From: Tobias Brunner Date: Fri, 20 Feb 2015 15:57:13 +0000 (+0100) Subject: ikev1: Adopt virtual IPs on new IKE_SA during re-authentication X-Git-Tag: 5.3.0rc1~23 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=31be582399;p=thirdparty%2Fstrongswan.git ikev1: Adopt virtual IPs on new IKE_SA during re-authentication Some clients like iOS/Mac OS X don't do a mode config exchange on the new SA during re-authentication. If we don't adopt the previous virtual IP Quick Mode rekeying will later fail. If a client does do Mode Config we directly reassign the VIPs we migrated from the old SA, without querying the attributes framework. Fixes #807, #810. --- diff --git a/src/libcharon/processing/jobs/adopt_children_job.c b/src/libcharon/processing/jobs/adopt_children_job.c index fb480eee22..c8a9c17ded 100644 --- a/src/libcharon/processing/jobs/adopt_children_job.c +++ b/src/libcharon/processing/jobs/adopt_children_job.c @@ -1,4 +1,7 @@ /* + * Copyright (C) 2015 Tobias Brunner + * Hochschule fuer Technik Rapperswil + * * Copyright (C) 2012 Martin Willi * Copyright (C) 2012 revosec AG * @@ -54,10 +57,10 @@ METHOD(job_t, execute, job_requeue_t, private_adopt_children_job_t *this) { identification_t *my_id, *other_id, *xauth; - host_t *me, *other; + host_t *me, *other, *vip; peer_cfg_t *cfg; - linked_list_t *children; - enumerator_t *enumerator, *childenum; + linked_list_t *children, *vips; + enumerator_t *enumerator, *subenum; ike_sa_id_t *id; ike_sa_t *ike_sa; child_sa_t *child_sa; @@ -81,7 +84,8 @@ METHOD(job_t, execute, job_requeue_t, charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); - /* find old SA to adopt children from */ + /* find old SA to adopt children and virtual IPs from */ + vips = linked_list_create(); children = linked_list_create(); enumerator = charon->ike_sa_manager->create_id_enumerator( charon->ike_sa_manager, my_id, xauth, @@ -102,18 +106,29 @@ METHOD(job_t, execute, job_requeue_t, other_id->equals(other_id, ike_sa->get_other_id(ike_sa)) && cfg->equals(cfg, ike_sa->get_peer_cfg(ike_sa))) { - childenum = ike_sa->create_child_sa_enumerator(ike_sa); - while (childenum->enumerate(childenum, &child_sa)) + subenum = ike_sa->create_child_sa_enumerator(ike_sa); + while (subenum->enumerate(subenum, &child_sa)) { - ike_sa->remove_child_sa(ike_sa, childenum); + ike_sa->remove_child_sa(ike_sa, subenum); children->insert_last(children, child_sa); } - childenum->destroy(childenum); - if (children->get_count(children)) + subenum->destroy(subenum); + + subenum = ike_sa->create_virtual_ip_enumerator(ike_sa, FALSE); + while (subenum->enumerate(subenum, &vip)) + { + vips->insert_last(vips, vip->clone(vip)); + } + subenum->destroy(subenum); + /* this does not release the addresses, which is good, but + * it does trigger an assign_vips(FALSE) event, so we also + * trigger one below */ + ike_sa->clear_virtual_ips(ike_sa, FALSE); + if (children->get_count(children) || vips->get_count(vips)) { DBG1(DBG_IKE, "detected reauth of existing IKE_SA, " - "adopting %d children", - children->get_count(children)); + "adopting %d children and %d virtual IPs", + children->get_count(children), vips->get_count(vips)); } ike_sa->set_state(ike_sa, IKE_DELETING); charon->bus->ike_updown(charon->bus, ike_sa, FALSE); @@ -125,7 +140,7 @@ METHOD(job_t, execute, job_requeue_t, charon->ike_sa_manager->checkin( charon->ike_sa_manager, ike_sa); } - if (children->get_count(children)) + if (children->get_count(children) || vips->get_count(vips)) { break; } @@ -140,7 +155,7 @@ METHOD(job_t, execute, job_requeue_t, xauth->destroy(xauth); cfg->destroy(cfg); - if (children->get_count(children)) + if (children->get_count(children) || vips->get_count(vips)) { /* adopt children by new SA */ ike_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager, @@ -152,10 +167,27 @@ METHOD(job_t, execute, job_requeue_t, { ike_sa->add_child_sa(ike_sa, child_sa); } + if (vips->get_count(vips)) + { + while (vips->remove_first(vips, (void**)&vip) == SUCCESS) + { + ike_sa->add_virtual_ip(ike_sa, FALSE, vip); + vip->destroy(vip); + } + charon->bus->assign_vips(charon->bus, ike_sa, TRUE); + } charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); } } children->destroy_offset(children, offsetof(child_sa_t, destroy)); + /* FIXME: If we still have addresses here it means we weren't able to + * find the new SA anymore (while not very likely during a proper + * reauthentication, this theoretically could happen because the SA is + * not locked while we search for the old one). So the addresses here + * should be released properly to avoid leaking these leases. This is + * currently not possible, though, due to the changed interface of + * release_address(), which now takes a complete IKE_SA object. */ + vips->destroy_offset(vips, offsetof(host_t, destroy)); if (array_count(this->tasks)) { diff --git a/src/libcharon/sa/ike_sa_manager.c b/src/libcharon/sa/ike_sa_manager.c index 5e2b925978..13fc74ff7f 100644 --- a/src/libcharon/sa/ike_sa_manager.c +++ b/src/libcharon/sa/ike_sa_manager.c @@ -1728,20 +1728,45 @@ METHOD(ike_sa_manager_t, create_id_enumerator, enumerator_t*, } /** - * Move all CHILD_SAs from old to new + * Move all CHILD_SAs and virtual IPs from old to new */ -static void adopt_children(ike_sa_t *old, ike_sa_t *new) +static void adopt_children_and_vips(ike_sa_t *old, ike_sa_t *new) { enumerator_t *enumerator; child_sa_t *child_sa; + host_t *vip; + int chcount = 0, vipcount = 0; + enumerator = old->create_child_sa_enumerator(old); while (enumerator->enumerate(enumerator, &child_sa)) { old->remove_child_sa(old, enumerator); new->add_child_sa(new, child_sa); + chcount++; + } + enumerator->destroy(enumerator); + + enumerator = old->create_virtual_ip_enumerator(old, FALSE); + while (enumerator->enumerate(enumerator, &vip)) + { + new->add_virtual_ip(new, FALSE, vip); + vipcount++; } enumerator->destroy(enumerator); + /* this does not release the addresses, which is good, but it does trigger + * an assign_vips(FALSE) event... */ + old->clear_virtual_ips(old, FALSE); + /* ...trigger the analogous event on the new SA */ + charon->bus->set_sa(charon->bus, new); + charon->bus->assign_vips(charon->bus, new, TRUE); + charon->bus->set_sa(charon->bus, old); + + if (chcount || vipcount) + { + DBG1(DBG_IKE, "detected reauth of existing IKE_SA, adopting %d " + "children and %d virtual IPs", chcount, vipcount); + } } /** @@ -1761,7 +1786,7 @@ static status_t enforce_replace(private_ike_sa_manager_t *this, { /* IKEv1 implicitly takes over children, IKEv2 recreates them * explicitly. */ - adopt_children(duplicate, new); + adopt_children_and_vips(duplicate, new); } /* For IKEv1 we have to delay the delete for the old IKE_SA. Some * peers need to complete the new SA first, otherwise the quick modes diff --git a/src/libcharon/sa/ikev1/tasks/mode_config.c b/src/libcharon/sa/ikev1/tasks/mode_config.c index 160d4af57c..d0994a9616 100644 --- a/src/libcharon/sa/ikev1/tasks/mode_config.c +++ b/src/libcharon/sa/ikev1/tasks/mode_config.c @@ -350,7 +350,7 @@ static status_t build_set(private_mode_config_t *this, message_t *message) cp_payload_t *cp; peer_cfg_t *config; identification_t *id; - linked_list_t *pools; + linked_list_t *pools, *migrated, *vips; host_t *any4, *any6, *found; char *name; @@ -358,37 +358,54 @@ static status_t build_set(private_mode_config_t *this, message_t *message) id = this->ike_sa->get_other_eap_id(this->ike_sa); config = this->ike_sa->get_peer_cfg(this->ike_sa); - any4 = host_create_any(AF_INET); - any6 = host_create_any(AF_INET6); + /* if we migrated virtual IPs during reauthentication, reassign them */ + migrated = linked_list_create_from_enumerator( + this->ike_sa->create_virtual_ip_enumerator(this->ike_sa, + FALSE)); + vips = migrated->clone_offset(migrated, offsetof(host_t, clone)); + migrated->destroy(migrated); this->ike_sa->clear_virtual_ips(this->ike_sa, FALSE); /* in push mode, we ask each configured pool for an address */ - enumerator = config->create_pool_enumerator(config); - while (enumerator->enumerate(enumerator, &name)) + if (!vips->get_count(vips)) { - pools = linked_list_create_with_items(name, NULL); - /* try IPv4, then IPv6 */ - found = charon->attributes->acquire_address(charon->attributes, - pools, this->ike_sa, any4); - if (!found) + any4 = host_create_any(AF_INET); + any6 = host_create_any(AF_INET6); + enumerator = config->create_pool_enumerator(config); + while (enumerator->enumerate(enumerator, &name)) { + pools = linked_list_create_with_items(name, NULL); + /* try IPv4, then IPv6 */ found = charon->attributes->acquire_address(charon->attributes, + pools, this->ike_sa, any4); + if (!found) + { + found = charon->attributes->acquire_address(charon->attributes, pools, this->ike_sa, any6); + } + pools->destroy(pools); + if (found) + { + vips->insert_last(vips, found); + } } - pools->destroy(pools); - if (found) - { - DBG1(DBG_IKE, "assigning virtual IP %H to peer '%Y'", found, id); - this->ike_sa->add_virtual_ip(this->ike_sa, FALSE, found); - cp->add_attribute(cp, build_vip(found)); - this->vips->insert_last(this->vips, found); - } + enumerator->destroy(enumerator); + any4->destroy(any4); + any6->destroy(any6); } - enumerator->destroy(enumerator); - any4->destroy(any4); - any6->destroy(any6); + enumerator = vips->create_enumerator(vips); + while (enumerator->enumerate(enumerator, &found)) + { + DBG1(DBG_IKE, "assigning virtual IP %H to peer '%Y'", found, id); + this->ike_sa->add_virtual_ip(this->ike_sa, FALSE, found); + cp->add_attribute(cp, build_vip(found)); + this->vips->insert_last(this->vips, found); + vips->remove_at(vips, enumerator); + } + enumerator->destroy(enumerator); + vips->destroy(vips); charon->bus->assign_vips(charon->bus, this->ike_sa, TRUE); @@ -454,6 +471,28 @@ METHOD(task_t, process_r, status_t, return NEED_MORE; } +/** + * Assign a migrated virtual IP + */ +static host_t *assign_migrated_vip(linked_list_t *migrated, host_t *requested) +{ + enumerator_t *enumerator; + host_t *found = NULL, *vip; + + enumerator = migrated->create_enumerator(migrated); + while (enumerator->enumerate(enumerator, &vip)) + { + if (vip->ip_equals(vip, requested)) + { + migrated->remove_at(migrated, enumerator); + found = vip; + break; + } + } + enumerator->destroy(enumerator); + return found; +} + /** * Build CFG_REPLY message after receiving CFG_REQUEST */ @@ -465,29 +504,35 @@ static status_t build_reply(private_mode_config_t *this, message_t *message) cp_payload_t *cp; peer_cfg_t *config; identification_t *id; - linked_list_t *vips, *pools; - host_t *requested; + linked_list_t *vips, *pools, *migrated; + host_t *requested, *found; cp = cp_payload_create_type(PLV1_CONFIGURATION, CFG_REPLY); id = this->ike_sa->get_other_eap_id(this->ike_sa); config = this->ike_sa->get_peer_cfg(this->ike_sa); - vips = linked_list_create(); pools = linked_list_create_from_enumerator( config->create_pool_enumerator(config)); - + /* if we migrated virtual IPs during reauthentication, reassign them */ + vips = linked_list_create_from_enumerator( + this->ike_sa->create_virtual_ip_enumerator(this->ike_sa, + FALSE)); + migrated = vips->clone_offset(vips, offsetof(host_t, clone)); + vips->destroy(vips); this->ike_sa->clear_virtual_ips(this->ike_sa, FALSE); + vips = linked_list_create(); enumerator = this->vips->create_enumerator(this->vips); while (enumerator->enumerate(enumerator, &requested)) { - host_t *found = NULL; - - /* query all pools until we get an address */ DBG1(DBG_IKE, "peer requested virtual IP %H", requested); - found = charon->attributes->acquire_address(charon->attributes, + found = assign_migrated_vip(migrated, requested); + if (!found) + { + found = charon->attributes->acquire_address(charon->attributes, pools, this->ike_sa, requested); + } if (found) { DBG1(DBG_IKE, "assigning virtual IP %H to peer '%Y'", found, id); @@ -515,6 +560,15 @@ static status_t build_reply(private_mode_config_t *this, message_t *message) type, value)); } enumerator->destroy(enumerator); + /* if a client did not re-request all adresses, release them */ + enumerator = migrated->create_enumerator(migrated); + while (enumerator->enumerate(enumerator, &found)) + { + charon->attributes->release_address(charon->attributes, + pools, found, this->ike_sa); + } + enumerator->destroy(enumerator); + migrated->destroy_offset(migrated, offsetof(host_t, destroy)); vips->destroy_offset(vips, offsetof(host_t, destroy)); pools->destroy(pools);