ikev1: Adopt virtual IPs on new IKE_SA during re-authentication
authorTobias Brunner <tobias@strongswan.org>
Fri, 20 Feb 2015 15:57:13 +0000 (16:57 +0100)
committerTobias Brunner <tobias@strongswan.org>
Thu, 19 Mar 2015 09:32:06 +0000 (10:32 +0100)
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.

src/libcharon/processing/jobs/adopt_children_job.c
src/libcharon/sa/ike_sa_manager.c
src/libcharon/sa/ikev1/tasks/mode_config.c

index fb480ee..c8a9c17 100644 (file)
@@ -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))
                {
index 5e2b925..13fc74f 100644 (file)
@@ -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
index 160d4af..d0994a9 100644 (file)
@@ -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);
 
@@ -455,6 +472,28 @@ METHOD(task_t, process_r, status_t,
 }
 
 /**
+ * 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
  */
 static status_t build_reply(private_mode_config_t *this, message_t *message)
@@ -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);