From a9f14ada34554bc42b819f8196899a002bade27b Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 10:56:08 +0200 Subject: [PATCH 01/14] kernel-pfkey: Correctly handle IPSEC_PROTO_ANY in an acquire --- src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index d975f15b9..dd9d354b1 100644 --- a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -981,6 +981,10 @@ static traffic_selector_t* sadb_address2ts(struct sadb_address *address) { traffic_selector_t *ts; host_t *host; + u_int8_t proto; + + proto = address->sadb_address_proto; + proto = proto == IPSEC_PROTO_ANY ? 0 : proto; /* The Linux 2.6 kernel does not set the protocol and port information * in the src and dst sadb_address extensions of the SADB_ACQUIRE message. @@ -988,8 +992,7 @@ static traffic_selector_t* sadb_address2ts(struct sadb_address *address) host = host_create_from_sockaddr((sockaddr_t*)&address[1]); ts = traffic_selector_create_from_subnet(host, address->sadb_address_prefixlen, - address->sadb_address_proto, - host->get_port(host), + proto, host->get_port(host), host->get_port(host) ?: 65535); return ts; } From bbd9df25a9410fcde8cb594e5c789cad5506d5ed Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 11:08:01 +0200 Subject: [PATCH 02/14] kernel-pfkey: Remove latest IPsec SA mapping when deleting a policy If IPsec SAs are rekeyed due to an address change (e.g. because update_sa is not supported) the exact same policy with the same reqid will be installed, but with different addresses. After the rekeying the old SA and its policies are removed, using the first matching mapping breaks the mapping between the policies and the new SA (at least on FreeBSD, the Linux kernel might only use the reqid for this). Using the oldest matching SA is still an approximation but it solves the above issue. --- .../plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index dd9d354b1..86853e658 100644 --- a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -2479,9 +2479,9 @@ METHOD(kernel_ipsec_t, del_policy, status_t, struct sadb_msg *msg, *out; struct sadb_x_policy *pol; policy_entry_t *policy, *found = NULL; - policy_sa_t *mapping; + policy_sa_t *mapping, *to_remove = NULL; enumerator_t *enumerator; - bool is_installed = TRUE; + bool first = TRUE, is_installed = TRUE; u_int32_t priority; size_t len; @@ -2511,19 +2511,26 @@ METHOD(kernel_ipsec_t, del_policy, status_t, policy_entry_destroy(policy, this); policy = found; - /* remove mapping to SA by reqid and priority */ + /* remove mapping to SA by reqid and priority, if multiple match, which + * could happen when rekeying due to an address change, remove the oldest */ priority = get_priority(policy, prio); enumerator = policy->used_by->create_enumerator(policy->used_by); while (enumerator->enumerate(enumerator, (void**)&mapping)) { if (reqid == mapping->sa->cfg.reqid && priority == mapping->priority) { - policy->used_by->remove_at(policy->used_by, enumerator); + to_remove = mapping; + is_installed = first; + } + else if (priority < mapping->priority) + { break; } - is_installed = FALSE; + first = FALSE; } enumerator->destroy(enumerator); + policy->used_by->remove(policy->used_by, to_remove, NULL); + mapping = to_remove; if (policy->used_by->get_count(policy->used_by) > 0) { /* policy is used by more SAs, keep in kernel */ From dc8b083d9fd2c55dec4412fd6116804064d826b6 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 11:22:57 +0200 Subject: [PATCH 03/14] kernel-pfroute: Retry route lookup without source address on failure The known source address might be gone resulting in an error, making learning a new source address impossible. --- .../plugins/kernel_pfroute/kernel_pfroute_net.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index c05ca56ec..2d0e252ac 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -1118,8 +1118,10 @@ static host_t *get_route(private_kernel_pfroute_net_t *this, bool nexthop, host_t *host = NULL; enumerator_t *enumerator; struct sockaddr *addr; + bool failed = FALSE; int type; +retry: msg.hdr.rtm_msglen = sizeof(struct rt_msghdr); for (type = 0; type < RTAX_MAX; type++) { @@ -1196,13 +1198,26 @@ static host_t *get_route(private_kernel_pfroute_net_t *this, bool nexthop, } else { - DBG1(DBG_KNL, "PF_ROUTE lookup failed: %s", strerror(errno)); + failed = TRUE; } /* signal completion of query to a waiting thread */ this->waiting_seq = 0; this->condvar->signal(this->condvar); this->mutex->unlock(this->mutex); + if (failed) + { + if (src) + { /* the given source address might be gone, try again without */ + src = NULL; + msg.hdr.rtm_seq = ++this->seq; + msg.hdr.rtm_addrs = 0; + memset(msg.buf, sizeof(msg.buf), 0); + goto retry; + } + DBG1(DBG_KNL, "PF_ROUTE lookup failed: %s", strerror(errno)); + } + if (host) { DBG2(DBG_KNL, "using %H as %s to reach %H", host, From baa6419ec1fdf07c0d0563f005276ed05eb80a0a Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 11:31:56 +0200 Subject: [PATCH 04/14] kernel-pfroute: Make time that is waited for VIPs to appear configurable One second might be too short for IPs to appear/disappear, especially on virtualized hosts. --- man/strongswan.conf.5.in | 3 +++ .../plugins/kernel_pfroute/kernel_pfroute_net.c | 13 +++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/man/strongswan.conf.5.in b/man/strongswan.conf.5.in index f86e9ea10..4192bc9af 100644 --- a/man/strongswan.conf.5.in +++ b/man/strongswan.conf.5.in @@ -610,6 +610,9 @@ Set MTU of ipsecN device .BR charon.plugins.kernel-netlink.roam_events " [yes]" Whether to trigger roam events when interfaces, addresses or routes change .TP +.BR charon.plugins.kernel-pfroute.vip_wait " [1000]" +Time in ms to wait until virtual IP addresses appear/disappear before failing. +.TP .BR charon.plugins.load-tester Section to configure the load-tester plugin, see LOAD TESTS .TP diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index 2d0e252ac..011cc1b64 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -247,6 +247,11 @@ struct private_kernel_pfroute_net_t * time of last roam event */ timeval_t last_roam; + + /** + * Time in ms to wait for IP addresses to appear/disappear + */ + int vip_wait; }; /** @@ -852,7 +857,8 @@ METHOD(kernel_net_t, add_ip, status_t, this->mutex->lock(this->mutex); while (!timeout && !get_interface_name(this, vip, NULL)) { - timeout = this->condvar->timed_wait(this->condvar, this->mutex, 1000); + timeout = this->condvar->timed_wait(this->condvar, this->mutex, + this->vip_wait); } this->mutex->unlock(this->mutex); if (timeout) @@ -929,7 +935,8 @@ METHOD(kernel_net_t, del_ip, status_t, this->mutex->lock(this->mutex); while (!timeout && get_interface_name(this, vip, NULL)) { - timeout = this->condvar->timed_wait(this->condvar, this->mutex, 1000); + timeout = this->condvar->timed_wait(this->condvar, this->mutex, + this->vip_wait); } this->mutex->unlock(this->mutex); if (timeout) @@ -1382,6 +1389,8 @@ kernel_pfroute_net_t *kernel_pfroute_net_create() .lock = rwlock_create(RWLOCK_TYPE_DEFAULT), .mutex = mutex_create(MUTEX_TYPE_DEFAULT), .condvar = condvar_create(CONDVAR_TYPE_DEFAULT), + .vip_wait = lib->settings->get_int(lib->settings, + "%s.plugins.kernel-pfroute.vip_wait", 1000, hydra->daemon), ); /* create a PF_ROUTE socket to communicate with the kernel */ From e50b20539b8a809021a1337288df332c825ccd3d Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 11:42:00 +0200 Subject: [PATCH 05/14] kernel-pfroute: Use ref_get() to allocate sequence numbers --- src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index 011cc1b64..13da679e4 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -1015,7 +1015,7 @@ static status_t manage_route(private_kernel_pfroute_net_t *this, int op, .rtm_type = op, .rtm_flags = RTF_UP | RTF_STATIC, .rtm_pid = this->pid, - .rtm_seq = ++this->seq, + .rtm_seq = ref_get(&this->seq), }, }; host_t *dst; @@ -1119,7 +1119,7 @@ static host_t *get_route(private_kernel_pfroute_net_t *this, bool nexthop, .rtm_version = RTM_VERSION, .rtm_type = RTM_GET, .rtm_pid = this->pid, - .rtm_seq = ++this->seq, + .rtm_seq = ref_get(&this->seq), }, }; host_t *host = NULL; @@ -1217,7 +1217,7 @@ retry: if (src) { /* the given source address might be gone, try again without */ src = NULL; - msg.hdr.rtm_seq = ++this->seq; + msg.hdr.rtm_seq = ref_get(&this->seq); msg.hdr.rtm_addrs = 0; memset(msg.buf, sizeof(msg.buf), 0); goto retry; From 7b9c3fb41f40f667d6b82d4dab85f001ac3e8978 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 11:57:31 +0200 Subject: [PATCH 06/14] kernel-pfroute: Trigger a roam event if a new interface appears --- src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index 13da679e4..88ccb97b7 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -569,6 +569,10 @@ static void process_link(private_kernel_pfroute_net_t *this, hydra->kernel_interface, iface->ifname); repopulate_iface(this, iface); this->ifaces->insert_last(this->ifaces, iface); + if (iface->usable) + { + roam = TRUE; + } } else { From 0745f846d0b5dac637938e63f0722ec20fdee1a3 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 12:14:19 +0200 Subject: [PATCH 07/14] kernel-pfroute: Reinstall routes on interface/address changes --- .../kernel_pfroute/kernel_pfroute_net.c | 327 +++++++++++++++++- 1 file changed, 320 insertions(+), 7 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index 88ccb97b7..28802efd8 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2012 Tobias Brunner + * Copyright (C) 2009-2013 Tobias Brunner * Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -53,6 +53,9 @@ /** delay before firing roam events (ms) */ #define ROAM_DELAY 100 +/** delay before reinstalling routes (ms) */ +#define ROUTE_DELAY 100 + typedef struct addr_entry_t addr_entry_t; /** @@ -176,6 +179,110 @@ static bool addr_map_entry_match_up(addr_map_entry_t *a, addr_map_entry_t *b) return iface_entry_up(b->iface) && a->ip->ip_equals(a->ip, b->ip); } +typedef struct route_entry_t route_entry_t; + +/** + * Installed routing entry + */ +struct route_entry_t { + /** Name of the interface the route is bound to */ + char *if_name; + + /** Gateway for this route */ + host_t *gateway; + + /** Destination net */ + chunk_t dst_net; + + /** Destination net prefixlen */ + u_int8_t prefixlen; +}; + +/** + * Clone a route_entry_t object. + */ +static route_entry_t *route_entry_clone(route_entry_t *this) +{ + route_entry_t *route; + + INIT(route, + .if_name = strdup(this->if_name), + .gateway = this->gateway ? this->gateway->clone(this->gateway) : NULL, + .dst_net = chunk_clone(this->dst_net), + .prefixlen = this->prefixlen, + ); + return route; +} + +/** + * Destroy a route_entry_t object + */ +static void route_entry_destroy(route_entry_t *this) +{ + free(this->if_name); + DESTROY_IF(this->gateway); + chunk_free(&this->dst_net); + free(this); +} + +/** + * Hash a route_entry_t object + */ +static u_int route_entry_hash(route_entry_t *this) +{ + return chunk_hash_inc(chunk_from_thing(this->prefixlen), + chunk_hash(this->dst_net)); +} + +/** + * Compare two route_entry_t objects + */ +static bool route_entry_equals(route_entry_t *a, route_entry_t *b) +{ + if (a->if_name && b->if_name && streq(a->if_name, b->if_name) && + chunk_equals(a->dst_net, b->dst_net) && a->prefixlen == b->prefixlen) + { + return (!a->gateway && !b->gateway) || (a->gateway && b->gateway && + a->gateway->ip_equals(a->gateway, b->gateway)); + } + return FALSE; +} + +typedef struct net_change_t net_change_t; + +/** + * Queued network changes + */ +struct net_change_t { + /** Name of the interface that got activated (or an IP appeared on) */ + char *if_name; +}; + +/** + * Destroy a net_change_t object + */ +static void net_change_destroy(net_change_t *this) +{ + free(this->if_name); + free(this); +} + +/** + * Hash a net_change_t object + */ +static u_int net_change_hash(net_change_t *this) +{ + return chunk_hash(chunk_create(this->if_name, strlen(this->if_name))); +} + +/** + * Compare two net_change_t objects + */ +static bool net_change_equals(net_change_t *a, net_change_t *b) +{ + return streq(a->if_name, b->if_name); +} + typedef struct private_kernel_pfroute_net_t private_kernel_pfroute_net_t; /** @@ -218,6 +325,31 @@ struct private_kernel_pfroute_net_t */ condvar_t *condvar; + /** + * installed routes + */ + hashtable_t *routes; + + /** + * mutex for routes + */ + mutex_t *routes_lock; + + /** + * interface changes which may trigger route reinstallation + */ + hashtable_t *net_changes; + + /** + * mutex for route reinstallation triggers + */ + mutex_t *net_changes_lock; + + /** + * time of last route reinstallation + */ + timeval_t last_route_reinstall; + /** * pid to send PF_ROUTE messages with */ @@ -254,6 +386,101 @@ struct private_kernel_pfroute_net_t int vip_wait; }; + +/** + * Forward declaration + */ +static status_t manage_route(private_kernel_pfroute_net_t *this, int op, + chunk_t dst_net, u_int8_t prefixlen, + host_t *gateway, char *if_name); + +/** + * Clear the queued network changes. + */ +static void net_changes_clear(private_kernel_pfroute_net_t *this) +{ + enumerator_t *enumerator; + net_change_t *change; + + enumerator = this->net_changes->create_enumerator(this->net_changes); + while (enumerator->enumerate(enumerator, NULL, (void**)&change)) + { + this->net_changes->remove_at(this->net_changes, enumerator); + net_change_destroy(change); + } + enumerator->destroy(enumerator); +} + +/** + * Act upon queued network changes. + */ +static job_requeue_t reinstall_routes(private_kernel_pfroute_net_t *this) +{ + enumerator_t *enumerator; + route_entry_t *route; + + this->net_changes_lock->lock(this->net_changes_lock); + this->routes_lock->lock(this->routes_lock); + + enumerator = this->routes->create_enumerator(this->routes); + while (enumerator->enumerate(enumerator, NULL, (void**)&route)) + { + net_change_t *change, lookup = { + .if_name = route->if_name, + }; + /* check if a change for the outgoing interface is queued */ + change = this->net_changes->get(this->net_changes, &lookup); + if (change) + { + manage_route(this, RTM_ADD, route->dst_net, route->prefixlen, + route->gateway, route->if_name); + } + } + enumerator->destroy(enumerator); + this->routes_lock->unlock(this->routes_lock); + + net_changes_clear(this); + this->net_changes_lock->unlock(this->net_changes_lock); + return JOB_REQUEUE_NONE; +} + +/** + * Queue route reinstallation caused by network changes for a given interface. + * + * The route reinstallation is delayed for a while and only done once for + * several calls during this delay, in order to avoid doing it too often. + * The interface name is freed. + */ +static void queue_route_reinstall(private_kernel_pfroute_net_t *this, + char *if_name) +{ + net_change_t *update, *found; + timeval_t now; + job_t *job; + + INIT(update, + .if_name = if_name + ); + + this->net_changes_lock->lock(this->net_changes_lock); + found = this->net_changes->put(this->net_changes, update, update); + if (found) + { + net_change_destroy(found); + } + time_monotonic(&now); + if (timercmp(&now, &this->last_route_reinstall, >)) + { + timeval_add_ms(&now, ROUTE_DELAY); + this->last_route_reinstall = now; + + job = (job_t*)callback_job_create((callback_job_cb_t)reinstall_routes, + this, NULL, NULL); + lib->scheduler->schedule_job_ms(lib->scheduler, job, ROUTE_DELAY); + } + this->net_changes_lock->unlock(this->net_changes_lock); +} + /** * Add an address map entry */ @@ -401,6 +628,7 @@ static void process_addr(private_kernel_pfroute_net_t *this, addr_entry_t *addr; bool found = FALSE, changed = FALSE, roam = FALSE; enumerator_t *enumerator; + char *ifname = NULL; int type; enumerator = create_rtmsg_enumerator(ifa, sizeof(*ifa)); @@ -453,6 +681,7 @@ static void process_addr(private_kernel_pfroute_net_t *this, .ip = host->clone(host), ); changed = TRUE; + ifname = strdup(iface->ifname); iface->addrs->insert_last(iface->addrs, addr); addr_map_entry_add(this, addr, iface); if (iface->usable) @@ -472,6 +701,15 @@ static void process_addr(private_kernel_pfroute_net_t *this, this->lock->unlock(this->lock); host->destroy(host); + if (roam && ifname) + { + queue_route_reinstall(this, ifname); + } + else + { + free(ifname); + } + if (roam) { fire_roam_event(this, TRUE); @@ -526,7 +764,7 @@ static void process_link(private_kernel_pfroute_net_t *this, { enumerator_t *enumerator; iface_entry_t *iface; - bool roam = FALSE, found = FALSE; + bool roam = FALSE, found = FALSE, update_routes = FALSE; this->lock->write_lock(this->lock); enumerator = this->ifaces->create_enumerator(this->ifaces); @@ -538,7 +776,7 @@ static void process_link(private_kernel_pfroute_net_t *this, { if (!(iface->flags & IFF_UP) && (msg->ifm_flags & IFF_UP)) { - roam = TRUE; + roam = update_routes = TRUE; DBG1(DBG_KNL, "interface %s activated", iface->ifname); } else if ((iface->flags & IFF_UP) && !(msg->ifm_flags & IFF_UP)) @@ -571,7 +809,7 @@ static void process_link(private_kernel_pfroute_net_t *this, this->ifaces->insert_last(this->ifaces, iface); if (iface->usable) { - roam = TRUE; + roam = update_routes = TRUE; } } else @@ -581,6 +819,11 @@ static void process_link(private_kernel_pfroute_net_t *this, } this->lock->unlock(this->lock); + if (update_routes) + { + queue_route_reinstall(this, strdup(iface->ifname)); + } + if (roam) { fire_roam_event(this, TRUE); @@ -890,11 +1133,16 @@ METHOD(kernel_net_t, add_ip, status_t, } } addrs->destroy(addrs); + /* during IKEv1 reauthentication, children get moved from + * old the new SA before the virtual IP is available. This + * kills the route for our virtual IP, reinstall. */ + queue_route_reinstall(this, strdup(iface->ifname)); + break; } } ifaces->destroy(ifaces); /* lets do this while holding the lock, thus preventing another thread - * from deleting the TUN device concurrently, hopefully listeneres are quick + * from deleting the TUN device concurrently, hopefully listeners are quick * and cause no deadlocks */ hydra->kernel_interface->tun(hydra->kernel_interface, tun, TRUE); this->lock->unlock(this->lock); @@ -1098,14 +1346,53 @@ METHOD(kernel_net_t, add_route, status_t, private_kernel_pfroute_net_t *this, chunk_t dst_net, u_int8_t prefixlen, host_t *gateway, host_t *src_ip, char *if_name) { - return manage_route(this, RTM_ADD, dst_net, prefixlen, gateway, if_name); + status_t status; + route_entry_t *found, route = { + .dst_net = dst_net, + .prefixlen = prefixlen, + .gateway = gateway, + .if_name = if_name, + }; + + this->routes_lock->lock(this->routes_lock); + found = this->routes->get(this->routes, &route); + if (found) + { + this->routes_lock->unlock(this->routes_lock); + return ALREADY_DONE; + } + found = route_entry_clone(&route); + this->routes->put(this->routes, found, found); + status = manage_route(this, RTM_ADD, dst_net, prefixlen, gateway, if_name); + this->routes_lock->unlock(this->routes_lock); + return status; } METHOD(kernel_net_t, del_route, status_t, private_kernel_pfroute_net_t *this, chunk_t dst_net, u_int8_t prefixlen, host_t *gateway, host_t *src_ip, char *if_name) { - return manage_route(this, RTM_DELETE, dst_net, prefixlen, gateway, if_name); + status_t status; + route_entry_t *found, route = { + .dst_net = dst_net, + .prefixlen = prefixlen, + .gateway = gateway, + .if_name = if_name, + }; + + this->routes_lock->lock(this->routes_lock); + found = this->routes->get(this->routes, &route); + if (!found) + { + this->routes_lock->unlock(this->routes_lock); + return NOT_FOUND; + } + this->routes->remove(this->routes, found); + route_entry_destroy(found); + status = manage_route(this, RTM_DELETE, dst_net, prefixlen, gateway, + if_name); + this->routes_lock->unlock(this->routes_lock); + return status; } /** @@ -1340,12 +1627,29 @@ METHOD(kernel_net_t, destroy, void, private_kernel_pfroute_net_t *this) { enumerator_t *enumerator; + route_entry_t *route; addr_entry_t *addr; + enumerator = this->routes->create_enumerator(this->routes); + while (enumerator->enumerate(enumerator, NULL, (void**)&route)) + { + manage_route(this, RTM_DELETE, route->dst_net, route->prefixlen, + route->gateway, route->if_name); + route_entry_destroy(route); + } + enumerator->destroy(enumerator); + this->routes->destroy(this->routes); + this->routes_lock->destroy(this->routes_lock); + if (this->socket != -1) { close(this->socket); } + + net_changes_clear(this); + this->net_changes->destroy(this->net_changes); + this->net_changes_lock->destroy(this->net_changes_lock); + enumerator = this->addrs->create_enumerator(this->addrs); while (enumerator->enumerate(enumerator, NULL, (void**)&addr)) { @@ -1389,13 +1693,22 @@ kernel_pfroute_net_t *kernel_pfroute_net_create() .addrs = hashtable_create( (hashtable_hash_t)addr_map_entry_hash, (hashtable_equals_t)addr_map_entry_equals, 16), + .routes = hashtable_create((hashtable_hash_t)route_entry_hash, + (hashtable_equals_t)route_entry_equals, 16), + .net_changes = hashtable_create( + (hashtable_hash_t)net_change_hash, + (hashtable_equals_t)net_change_equals, 16), .tuns = linked_list_create(), .lock = rwlock_create(RWLOCK_TYPE_DEFAULT), .mutex = mutex_create(MUTEX_TYPE_DEFAULT), .condvar = condvar_create(CONDVAR_TYPE_DEFAULT), + .routes_lock = mutex_create(MUTEX_TYPE_DEFAULT), + .net_changes_lock = mutex_create(MUTEX_TYPE_DEFAULT), .vip_wait = lib->settings->get_int(lib->settings, "%s.plugins.kernel-pfroute.vip_wait", 1000, hydra->daemon), ); + timerclear(&this->last_route_reinstall); + timerclear(&this->last_roam); /* create a PF_ROUTE socket to communicate with the kernel */ this->socket = socket(PF_ROUTE, SOCK_RAW, AF_UNSPEC); From 8afd0f05e34a5443ae2bda631a18b9b0c5a747d4 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 12:21:58 +0200 Subject: [PATCH 08/14] kernel-pfkey: Provide interface name when installing exclude route --- .../plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index 86853e658..fb90f5b21 100644 --- a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -1993,10 +1993,14 @@ static void add_exclude_route(private_kernel_pfkey_ipsec_t *this, dst, NULL); if (gtw) { - if (hydra->kernel_interface->add_route(hydra->kernel_interface, + char *if_name = NULL; + + if (hydra->kernel_interface->get_interface( + hydra->kernel_interface, src, &if_name) && + hydra->kernel_interface->add_route(hydra->kernel_interface, dst->get_address(dst), dst->get_family(dst) == AF_INET ? 32 : 128, - gtw, src, NULL) == SUCCESS) + gtw, src, if_name) == SUCCESS) { INIT(exclude, .dst = dst->clone(dst), @@ -2012,6 +2016,7 @@ static void add_exclude_route(private_kernel_pfkey_ipsec_t *this, DBG1(DBG_KNL, "installing exclude route for %H failed", dst); } gtw->destroy(gtw); + free(if_name); } else { @@ -2050,18 +2055,24 @@ static void remove_exclude_route(private_kernel_pfkey_ipsec_t *this, if (removed) { + char *if_name = NULL; + dst = route->exclude->dst; DBG2(DBG_KNL, "uninstalling exclude route for %H src %H", dst, route->exclude->src); - if (hydra->kernel_interface->del_route(hydra->kernel_interface, + if (hydra->kernel_interface->get_interface( + hydra->kernel_interface, + route->exclude->src, &if_name) && + hydra->kernel_interface->del_route(hydra->kernel_interface, dst->get_address(dst), dst->get_family(dst) == AF_INET ? 32 : 128, route->exclude->gtw, route->exclude->src, - NULL) != SUCCESS) + if_name) != SUCCESS) { DBG1(DBG_KNL, "uninstalling exclude route for %H failed", dst); } exclude_route_destroy(route->exclude); + free(if_name); } route->exclude = NULL; } From 527663d6b6f3e9d7ca76869771a758491a5692c2 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 12:38:21 +0200 Subject: [PATCH 09/14] kernel-pfroute: Don't report an error when trying to reinstall a route --- src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index 28802efd8..0760513b8 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -1335,6 +1335,10 @@ static status_t manage_route(private_kernel_pfroute_net_t *this, int op, if (send(this->socket, &msg, msg.hdr.rtm_msglen, 0) != msg.hdr.rtm_msglen) { + if (errno == EEXIST) + { + return ALREADY_DONE; + } DBG1(DBG_KNL, "%s PF_ROUTE route failed: %s", op == RTM_ADD ? "adding" : "deleting", strerror(errno)); return FAILED; From cb082d15eff3948f9bfbb4d43d48cdf8e4ee993e Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 15:02:48 +0200 Subject: [PATCH 10/14] kernel-pfroute: Make sure source addresses are not virtual and usable It seems we sometimes get the virtual IP as source (with rightsubnet=0.0.0.0/0) even if the exclude route is already installed. Might be a timing issue because shortly afterwards the lookup seems to succeed. --- .../kernel_pfroute/kernel_pfroute_net.c | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index 0760513b8..c1224cc98 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -1519,12 +1519,28 @@ retry: } DBG1(DBG_KNL, "PF_ROUTE lookup failed: %s", strerror(errno)); } - - if (host) + if (!host) { - DBG2(DBG_KNL, "using %H as %s to reach %H", host, - nexthop ? "nexthop" : "address", dest); + return NULL; } + if (!nexthop) + { /* make sure the source address is not virtual and usable */ + addr_entry_t *entry, lookup = { + .ip = host, + }; + + this->lock->read_lock(this->lock); + entry = this->addrs->get_match(this->addrs, &lookup, + (void*)addr_map_entry_match_up_and_usable); + this->lock->unlock(this->lock); + if (!entry) + { + host->destroy(host); + return NULL; + } + } + DBG2(DBG_KNL, "using %H as %s to reach %H", host, + nexthop ? "nexthop" : "address", dest); return host; } From e9c1ca0278a0f512545c8b3aa6be0983e57412ec Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 15:29:38 +0200 Subject: [PATCH 11/14] kernel-pfroute: Ignore virtual IPs in address map As the virtual flag is set after the address has been added to the map, we make sure we ignore virtual IPs when doing lookups. --- .../kernel_pfroute/kernel_pfroute_net.c | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index c1224cc98..d4a595762 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -136,6 +136,9 @@ struct addr_map_entry_t { /** The IP address */ host_t *ip; + /** The address entry for this IP address */ + addr_entry_t *addr; + /** The interface this address is installed on */ iface_entry_t *iface; }; @@ -166,8 +169,8 @@ static bool addr_map_entry_equals(addr_map_entry_t *a, addr_map_entry_t *b) static bool addr_map_entry_match_up_and_usable(addr_map_entry_t *a, addr_map_entry_t *b) { - return iface_entry_up_and_usable(b->iface) && - a->ip->ip_equals(a->ip, b->ip); + return !b->addr->virtual && iface_entry_up_and_usable(b->iface) && + a->ip->ip_equals(a->ip, b->ip); } /** @@ -176,7 +179,8 @@ static bool addr_map_entry_match_up_and_usable(addr_map_entry_t *a, */ static bool addr_map_entry_match_up(addr_map_entry_t *a, addr_map_entry_t *b) { - return iface_entry_up(b->iface) && a->ip->ip_equals(a->ip, b->ip); + return !b->addr->virtual && iface_entry_up(b->iface) && + a->ip->ip_equals(a->ip, b->ip); } typedef struct route_entry_t route_entry_t; @@ -489,13 +493,9 @@ static void addr_map_entry_add(private_kernel_pfroute_net_t *this, { addr_map_entry_t *entry; - if (addr->virtual) - { /* don't map virtual IPs */ - return; - } - INIT(entry, .ip = addr->ip, + .addr = addr, .iface = iface, ); entry = this->addrs->put(this->addrs, entry, entry); @@ -511,14 +511,10 @@ static void addr_map_entry_remove(addr_entry_t *addr, iface_entry_t *iface, { addr_map_entry_t *entry, lookup = { .ip = addr->ip, + .addr = addr, .iface = iface, }; - if (addr->virtual) - { /* these are never mapped, but this check avoid problems if a virtual IP - * equals a regular one */ - return; - } entry = this->addrs->remove(this->addrs, &lookup); free(entry); } From 5310f485d98313812a8bad4bbd7030c526091634 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 15:37:35 +0200 Subject: [PATCH 12/14] kernel-pfroute: Provide name of interfaces on which virtual IPs are installed --- .../kernel_netlink/kernel_netlink_net.c | 2 +- .../kernel_pfroute/kernel_pfroute_net.c | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c index 020b36a0b..c29aff433 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c @@ -1266,7 +1266,7 @@ METHOD(kernel_net_t, get_interface_name, bool, if (name) { *name = strdup(entry->iface->ifname); - DBG2(DBG_KNL, "virtual %H is on interface %s", ip, *name); + DBG2(DBG_KNL, "virtual IP %H is on interface %s", ip, *name); } this->lock->unlock(this->lock); return TRUE; diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index d4a595762..ca9e06975 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -173,6 +173,15 @@ static bool addr_map_entry_match_up_and_usable(addr_map_entry_t *a, a->ip->ip_equals(a->ip, b->ip); } +/** + * Used with get_match this finds an address entry if it is installed as virtual + * IP address + */ +static bool addr_map_entry_match_virtual(addr_map_entry_t *a, addr_map_entry_t *b) +{ + return b->addr->virtual && a->ip->ip_equals(a->ip, b->ip); +} + /** * Used with get_match this finds an address entry if it is installed on * any active local interface @@ -1060,6 +1069,19 @@ METHOD(kernel_net_t, get_interface_name, bool, this->lock->unlock(this->lock); return TRUE; } + /* check if it is a virtual IP */ + entry = this->addrs->get_match(this->addrs, &lookup, + (void*)addr_map_entry_match_virtual); + if (entry) + { + if (name) + { + *name = strdup(entry->iface->ifname); + DBG2(DBG_KNL, "virtual IP %H is on interface %s", ip, *name); + } + this->lock->unlock(this->lock); + return TRUE; + } /* maybe it is installed on an ignored interface */ entry = this->addrs->get_match(this->addrs, &lookup, (void*)addr_map_entry_match_up); From b308a97944511155b3bcc51d52ebbb0d8992da55 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 16:08:56 +0200 Subject: [PATCH 13/14] kernel-pfroute: Properly enumerate sockaddrs in interface messages The ifa_msghdr and rt_msghdr structs are not compatible (at least not on FreeBSD). --- .../kernel_pfroute/kernel_pfroute_net.c | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index ca9e06975..6a191842e 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -601,11 +601,11 @@ METHOD(enumerator_t, rt_enumerate, bool, } /** - * Create a safe enumerator over sockaddrs in ifa/ifam/rt_msg + * Create an enumerator over sockaddrs in rt/if messages */ -static enumerator_t *create_rtmsg_enumerator(void *hdr, size_t hdrlen) +static enumerator_t *create_rt_enumerator(int types, int remaining, + struct sockaddr *addr) { - struct rt_msghdr *rthdr = hdr; rt_enumerator_t *this; INIT(this, @@ -613,13 +613,31 @@ static enumerator_t *create_rtmsg_enumerator(void *hdr, size_t hdrlen) .enumerate = (void*)_rt_enumerate, .destroy = (void*)free, }, - .types = rthdr->rtm_addrs, - .remaining = rthdr->rtm_msglen - hdrlen, - .addr = hdr + hdrlen, + .types = types, + .remaining = remaining, + .addr = addr, ); return &this->public; } +/** + * Create a safe enumerator over sockaddrs in rt_msghdr + */ +static enumerator_t *create_rtmsg_enumerator(struct rt_msghdr *hdr) +{ + return create_rt_enumerator(hdr->rtm_addrs, hdr->rtm_msglen - sizeof(*hdr), + (struct sockaddr *)(hdr + 1)); +} + +/** + * Create a safe enumerator over sockaddrs in ifa_msghdr + */ +static enumerator_t *create_ifamsg_enumerator(struct ifa_msghdr *hdr) +{ + return create_rt_enumerator(hdr->ifam_addrs, hdr->ifam_msglen - sizeof(*hdr), + (struct sockaddr *)(hdr + 1)); +} + /** * Process an RTM_*ADDR message from the kernel */ @@ -636,7 +654,7 @@ static void process_addr(private_kernel_pfroute_net_t *this, char *ifname = NULL; int type; - enumerator = create_rtmsg_enumerator(ifa, sizeof(*ifa)); + enumerator = create_ifamsg_enumerator(ifa); while (enumerator->enumerate(enumerator, &type, &sockaddr)) { if (type == RTAX_IFA) @@ -1483,8 +1501,7 @@ retry: { continue; } - enumerator = create_rtmsg_enumerator(this->reply, - sizeof(*this->reply)); + enumerator = create_rtmsg_enumerator(this->reply); while (enumerator->enumerate(enumerator, &type, &addr)) { if (nexthop) From fae4d67adc20d0efc00775d441a16f1c18956c2b Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 10 Jul 2013 16:28:55 +0200 Subject: [PATCH 14/14] kernel-pfroute: Ignore IP address changes if address is %any --- src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c index 6a191842e..a5453d0bb 100644 --- a/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c +++ b/src/libhydra/plugins/kernel_pfroute/kernel_pfroute_net.c @@ -665,8 +665,9 @@ static void process_addr(private_kernel_pfroute_net_t *this, } enumerator->destroy(enumerator); - if (!host) + if (!host || host->is_anyaddr(host)) { + DESTROY_IF(host); return; }