improved MOBIKE:

  prefer address family already used
  do not change address implicit when mobike supported
  handle multiple simultaneous roaming requests more properly
  proper enabling/disabling of UDP encapsulation

10 files changed, 254 insertions, 88 deletions

diff --git a/src/charon/kernel/kernel_interface.c b/src/charon/kernel/kernel_interface.c
index 0e06368fd..50be720de 100644
--- a/src/charon/kernel/kernel_interface.c
+++ b/src/charon/kernel/kernel_interface.c
@@ -615,7 +615,8 @@ static void process_link(private_kernel_interface_t *this,
 	/* send an update to all IKE_SAs */
 	if (update && event)
 	{
-		charon->processor->queue_job(charon->processor, (job_t*)roam_job_create());
+		charon->processor->queue_job(charon->processor,
+									 (job_t*)roam_job_create(TRUE));
 	}
 }
 
@@ -633,7 +634,7 @@ static void process_addr(private_kernel_interface_t *this,
 	iface_entry_t *iface;
 	addr_entry_t *addr;
 	chunk_t local = chunk_empty, address = chunk_empty;
-	bool update = FALSE, found = FALSE;
+	bool update = FALSE, found = FALSE, changed = FALSE;
 	
 	while(RTA_OK(rta, rtasize))
 	{
@@ -681,6 +682,7 @@ static void process_addr(private_kernel_interface_t *this,
 					found = TRUE;
 					if (hdr->nlmsg_type == RTM_DELADDR)
 					{
+						changed = TRUE;
 						addrs->remove(addrs);
 						addr_entry_destroy(addr);
 						DBG1(DBG_KNL, "%H disappeared from %s", host, iface->ifname);
@@ -694,6 +696,7 @@ static void process_addr(private_kernel_interface_t *this,
 				if (!found)
 				{
 					found = TRUE;
+					changed = TRUE;
 					addr = malloc_thing(addr_entry_t);
 					addr->ip = host->clone(host);
 					addr->virtual = FALSE;
@@ -718,9 +721,10 @@ static void process_addr(private_kernel_interface_t *this,
 	host->destroy(host);
 	
 	/* send an update to all IKE_SAs */
-	if (update && event)
+	if (update && event && changed)
 	{
-		charon->processor->queue_job(charon->processor, (job_t*)roam_job_create());
+		charon->processor->queue_job(charon->processor,
+									 (job_t*)roam_job_create(TRUE));
 	}
 }
 
@@ -817,7 +821,7 @@ static job_requeue_t receive_events(private_kernel_interface_t *this)
 				case RTM_NEWROUTE:
 				case RTM_DELROUTE:
 					charon->processor->queue_job(charon->processor,
-												 (job_t*)roam_job_create());
+												 (job_t*)roam_job_create(FALSE));
 					break;
 				default:
 					break;
@@ -1763,11 +1767,11 @@ static status_t add_sa(private_kernel_interface_t *this,
 			return FAILED;
 		}
 
-		struct xfrm_encap_tmpl* encap = (struct xfrm_encap_tmpl*)RTA_DATA(rthdr);
-		encap->encap_type = UDP_ENCAP_ESPINUDP;
-		encap->encap_sport = htons(src->get_port(src));
-		encap->encap_dport = htons(dst->get_port(dst));
-		memset(&encap->encap_oa, 0, sizeof (xfrm_address_t));
+		struct xfrm_encap_tmpl* tmpl = (struct xfrm_encap_tmpl*)RTA_DATA(rthdr);
+		tmpl->encap_type = UDP_ENCAP_ESPINUDP;
+		tmpl->encap_sport = htons(src->get_port(src));
+		tmpl->encap_dport = htons(dst->get_port(dst));
+		memset(&tmpl->encap_oa, 0, sizeof (xfrm_address_t));
 		/* encap_oa could probably be derived from the 
 		 * traffic selectors [rfc4306, p39]. In the netlink kernel implementation 
 		 * pluto does the same as we do here but it uses encap_oa in the 
@@ -1794,13 +1798,16 @@ static status_t add_sa(private_kernel_interface_t *this,
 static status_t update_sa(private_kernel_interface_t *this,
 						  u_int32_t spi, protocol_id_t protocol,
 						  host_t *src, host_t *dst,
-						  host_t *new_src, host_t *new_dst)
+						  host_t *new_src, host_t *new_dst, bool encap)
 {
-	unsigned char request[BUFFER_SIZE];
+	unsigned char request[BUFFER_SIZE], *pos;
 	struct nlmsghdr *hdr, *out = NULL;
 	struct xfrm_usersa_id *sa_id;
-	struct xfrm_usersa_info *sa = NULL;
+	struct xfrm_usersa_info *out_sa = NULL, *sa;
 	size_t len;
+	struct rtattr *rta;
+	size_t rtasize;
+	struct xfrm_encap_tmpl* tmpl = NULL;
 	
 	memset(&request, 0, sizeof(request));
 	
@@ -1827,7 +1834,7 @@ static status_t update_sa(private_kernel_interface_t *this,
 			{
 				case XFRM_MSG_NEWSA:
 				{
-					sa = NLMSG_DATA(hdr);
+					out_sa = NLMSG_DATA(hdr);
 					break;
 				}
 				case NLMSG_ERROR:
@@ -1846,7 +1853,7 @@ static status_t update_sa(private_kernel_interface_t *this,
 			break;
 		}
 	}
-	if (sa == NULL ||
+	if (out_sa == NULL ||
 		this->public.del_sa(&this->public, dst, spi, protocol) != SUCCESS)
 	{
 		DBG1(DBG_KNL, "unable to update SAD entry with SPI 0x%x", spi);
@@ -1857,10 +1864,14 @@ static status_t update_sa(private_kernel_interface_t *this,
 	DBG2(DBG_KNL, "updating SAD entry with SPI 0x%x from %#H..%#H to %#H..%#H",
 		 spi, src, dst, new_src, new_dst);
 	
-	/* update the values in the queried SA */
-	hdr = out;
+	/* copy over the SA from out to request */
+	hdr = (struct nlmsghdr*)request;
+	memcpy(hdr, out, min(out->nlmsg_len, sizeof(request)));
 	hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;	
 	hdr->nlmsg_type = XFRM_MSG_NEWSA;
+	hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_info));
+	sa = NLMSG_DATA(hdr);
+	sa->family = new_dst->get_family(new_dst);
 	
 	if (!src->ip_equals(src, new_src))
 	{
@@ -1871,24 +1882,39 @@ static status_t update_sa(private_kernel_interface_t *this,
 		host2xfrm(new_dst, &sa->id.daddr);
 	}
 	
-	if (src->get_port(src) != new_src->get_port(new_src) ||
-		dst->get_port(dst) != new_dst->get_port(new_dst))
+	rta = XFRM_RTA(out, struct xfrm_usersa_info);
+	rtasize = XFRM_PAYLOAD(out, struct xfrm_usersa_info);
+	pos = (u_char*)XFRM_RTA(hdr, struct xfrm_usersa_info);
+	while(RTA_OK(rta, rtasize))
 	{
-		struct rtattr *rtattr = XFRM_RTA(hdr, struct xfrm_usersa_info);
-		size_t rtsize = XFRM_PAYLOAD(hdr, struct xfrm_usersa_info);
-		while (RTA_OK(rtattr, rtsize))
+		/* copy all attributes, but not XFRMA_ENCAP if we are disabling it */
+		if (rta->rta_type != XFRMA_ENCAP || encap)
 		{
-			if (rtattr->rta_type == XFRMA_ENCAP)
-			{
-				struct xfrm_encap_tmpl* encap;
-				encap = (struct xfrm_encap_tmpl*)RTA_DATA(rtattr);
-				encap->encap_sport = ntohs(new_src->get_port(new_src));
-				encap->encap_dport = ntohs(new_dst->get_port(new_dst));
-				break;
-			}
-			rtattr = RTA_NEXT(rtattr, rtsize);
+			if (rta->rta_type == XFRMA_ENCAP)
+			{	/* update encap tmpl */
+				tmpl = (struct xfrm_encap_tmpl*)RTA_DATA(rta);
+				tmpl->encap_sport = ntohs(new_src->get_port(new_src));
+				tmpl->encap_dport = ntohs(new_dst->get_port(new_dst));
+			}	
+			memcpy(pos, rta, rta->rta_len);
+			pos += rta->rta_len;
+			hdr->nlmsg_len += rta->rta_len;
 		}
+		rta = RTA_NEXT(rta, rtasize);
+	}
+	if (tmpl == NULL && encap)
+	{	/* add tmpl if we are enabling it */
+		rta = (struct rtattr*)pos;
+		rta->rta_type = XFRMA_ENCAP;
+		rta->rta_len = RTA_LENGTH(sizeof(struct xfrm_encap_tmpl));
+		hdr->nlmsg_len += rta->rta_len;
+		tmpl = (struct xfrm_encap_tmpl*)RTA_DATA(rta);
+		tmpl->encap_type = UDP_ENCAP_ESPINUDP;
+		tmpl->encap_sport = ntohs(new_src->get_port(new_src));
+		tmpl->encap_dport = ntohs(new_dst->get_port(new_dst));
+		memset(&tmpl->encap_oa, 0, sizeof (xfrm_address_t));
 	}
+	
 	if (netlink_send_ack(this, this->socket_xfrm, hdr) != SUCCESS)
 	{
 		DBG1(DBG_KNL, "unable to update SAD entry with SPI 0x%x", spi);
@@ -2320,7 +2346,7 @@ kernel_interface_t *kernel_interface_create()
 	/* public functions */
 	this->public.get_spi = (status_t(*)(kernel_interface_t*,host_t*,host_t*,protocol_id_t,u_int32_t,u_int32_t*))get_spi;
 	this->public.add_sa  = (status_t(*)(kernel_interface_t *,host_t*,host_t*,u_int32_t,protocol_id_t,u_int32_t,u_int64_t,u_int64_t,algorithm_t*,algorithm_t*,prf_plus_t*,mode_t,bool,bool))add_sa;
-	this->public.update_sa = (status_t(*)(kernel_interface_t*,u_int32_t,protocol_id_t,host_t*,host_t*,host_t*,host_t*))update_sa;
+	this->public.update_sa = (status_t(*)(kernel_interface_t*,u_int32_t,protocol_id_t,host_t*,host_t*,host_t*,host_t*,bool))update_sa;
 	this->public.query_sa = (status_t(*)(kernel_interface_t*,host_t*,u_int32_t,protocol_id_t,u_int32_t*))query_sa;
 	this->public.del_sa = (status_t(*)(kernel_interface_t*,host_t*,u_int32_t,protocol_id_t))del_sa;
 	this->public.add_policy = (status_t(*)(kernel_interface_t*,host_t*,host_t*,traffic_selector_t*,traffic_selector_t*,policy_dir_t,protocol_id_t,u_int32_t,bool,mode_t))add_policy;
diff --git a/src/charon/kernel/kernel_interface.h b/src/charon/kernel/kernel_interface.h
index a5d2d071e..256c20797 100644
--- a/src/charon/kernel/kernel_interface.h
+++ b/src/charon/kernel/kernel_interface.h
@@ -139,6 +139,7 @@ struct kernel_interface_t {
 	 * @param dst			current destination address
 	 * @param new_src		new source address
 	 * @param new_dst		new destination address
+	 * @param encap			use UDP encapsulation
 	 * @return
 	 * 						- SUCCESS
 	 * 						- FAILED if kernel comm failed
@@ -146,7 +147,7 @@ struct kernel_interface_t {
 	status_t (*update_sa)(kernel_interface_t *this,
 						  u_int32_t spi, protocol_id_t protocol,
 						  host_t *src, host_t *dst, 
-						  host_t *new_src, host_t *new_dst);
+						  host_t *new_src, host_t *new_dst, bool encap);
 	
 	/**
 	 * @brief Query the use time of an SA.
diff --git a/src/charon/processing/jobs/roam_job.c b/src/charon/processing/jobs/roam_job.c
index c5acad60c..3b5cd0ed2 100644
--- a/src/charon/processing/jobs/roam_job.c
+++ b/src/charon/processing/jobs/roam_job.c
@@ -39,6 +39,11 @@ struct private_roam_job_t {
 	 * public roam_job_t interface
 	 */
 	roam_job_t public;
+	
+	/**
+	 * has the address list changed, or the routing only?
+	 */
+	bool address;
 };
 
 /**
@@ -75,7 +80,7 @@ static void execute(private_roam_job_t *this)
 		ike_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager, id);
 		if (ike_sa)
 		{
-			if (ike_sa->roam(ike_sa) == DESTROY_ME)
+			if (ike_sa->roam(ike_sa, this->address) == DESTROY_ME)
 			{
 				charon->ike_sa_manager->checkin_and_destroy(
 											charon->ike_sa_manager, ike_sa);
@@ -95,13 +100,15 @@ static void execute(private_roam_job_t *this)
 /*
  * Described in header
  */
-roam_job_t *roam_job_create()
+roam_job_t *roam_job_create(bool address)
 {
 	private_roam_job_t *this = malloc_thing(private_roam_job_t);
 	
 	this->public.job_interface.destroy = (void (*) (job_t *)) destroy;
 	this->public.job_interface.execute = (void (*) (job_t *)) execute;
 	this->public.job_interface.destroy = (void (*) (job_t *)) destroy;
+	
+	this->address = address;
 
 	return &this->public;
 }
diff --git a/src/charon/processing/jobs/roam_job.h b/src/charon/processing/jobs/roam_job.h
index 23d374ccb..293b09f08 100644
--- a/src/charon/processing/jobs/roam_job.h
+++ b/src/charon/processing/jobs/roam_job.h
@@ -50,11 +50,12 @@ struct roam_job_t {
 /**
  * @brief Creates a job to inform IKE_SAs about an updated address list.
  * 
+ * @param address		TRUE if address list changed, FALSE if routing changed
  * @return				initiate_ike_sa_job_t object
  * 
  * @ingroup jobs
  */
-roam_job_t *roam_job_create();
+roam_job_t *roam_job_create(bool address);
 
 #endif /*ROAM_JOB_H_*/
 
diff --git a/src/charon/sa/child_sa.c b/src/charon/sa/child_sa.c
index 14a0502fa..73ccffd65 100644
--- a/src/charon/sa/child_sa.c
+++ b/src/charon/sa/child_sa.c
@@ -776,20 +776,22 @@ static status_t update_hosts(private_child_sa_t *this,
 							 host_t *me, host_t *other, bool encap) 
 {
 	/* anything changed at all? */
-	if (me->equals(me, this->me.addr) && other->equals(other, this->other.addr))
+	if (me->equals(me, this->me.addr) && 
+		other->equals(other, this->other.addr) && this->encap == encap)
 	{
 		return SUCCESS;
 	}
-	
 	/* run updown script to remove iptables rules */
 	updown(this, FALSE);
 	
+	this->encap = encap;
+	
 	/* update our (initator) SAs */
 	charon->kernel_interface->update_sa(charon->kernel_interface, this->me.spi,
-				this->protocol, this->other.addr, this->me.addr, other, me);
+			this->protocol, this->other.addr, this->me.addr, other, me, encap);
 	/* update his (responder) SAs */
 	charon->kernel_interface->update_sa(charon->kernel_interface, this->other.spi, 
-				this->protocol, this->me.addr, this->other.addr, me, other);
+			this->protocol, this->me.addr, this->other.addr, me, other, encap);
 	
 	/* update policies */
 	if (!me->ip_equals(me, this->me.addr) ||
diff --git a/src/charon/sa/ike_sa.c b/src/charon/sa/ike_sa.c
index 25731c654..b549f9d66 100644
--- a/src/charon/sa/ike_sa.c
+++ b/src/charon/sa/ike_sa.c
@@ -219,6 +219,11 @@ struct private_ike_sa_t {
 	 * list of peers additional addresses, transmitted via MOBIKE
 	 */
 	linked_list_t *additional_addresses;
+	
+	/**
+	 * number pending UPDATE_SA_ADDRESS (MOBIKE)
+	 */
+	u_int32_t pending_updates;
 
 	/**
 	 * Timestamps for this IKE_SA
@@ -715,6 +720,22 @@ static iterator_t* create_additional_address_iterator(private_ike_sa_t *this)
 	return this->additional_addresses->create_iterator(
 											this->additional_addresses, TRUE);
 }
+	
+/**
+ * Implementation of ike_sa_t.set_pending_updates.
+ */
+static void set_pending_updates(private_ike_sa_t *this, u_int32_t updates)
+{
+	this->pending_updates = updates;
+}
+
+/**
+ * Implementation of ike_sa_t.get_pending_updates.
+ */
+static u_int32_t get_pending_updates(private_ike_sa_t *this)
+{
+	return this->pending_updates;
+}
 
 /**
  * Update hosts, as addresses may change (NAT)
@@ -723,6 +744,11 @@ static void update_hosts(private_ike_sa_t *this, host_t *me, host_t *other)
 {
 	bool update = FALSE;
 	
+	if (supports_extension(this, EXT_MOBIKE))
+	{	/* if peer speaks mobike, address updates are explicit only */
+		return;
+	}
+	
 	if (me == NULL)
 	{
 		me = this->my_host;
@@ -1727,16 +1753,23 @@ static int get_path_prio(host_t *me, host_t *other)
 /**
  * Implementation of ike_sa_t.roam.
  */
-static status_t roam(private_ike_sa_t *this)
+static status_t roam(private_ike_sa_t *this, bool address)
 {
 	iterator_t *iterator;
 	host_t *me, *other, *cand_me, *cand_other;
 	ike_mobike_t *mobike;
-	int prio, best = 0;
+	int prio, best4 = 0, best6 = 0;
 	
-	/* only initiator handles address updated actively */
+	/* responder just updates the peer about changed address config */
 	if (!this->ike_sa_id->is_initiator(this->ike_sa_id))
 	{
+		if (supports_extension(this, EXT_MOBIKE) && address)
+		{
+			DBG1(DBG_IKE, "sending address list update using MOBIKE");
+			mobike = ike_mobike_create(&this->public, TRUE);
+			this->task_manager->queue_task(this->task_manager, (task_t*)mobike);
+			return this->task_manager->initiate(this->task_manager);
+		}
 		return SUCCESS;
 	}
 	
@@ -1746,11 +1779,20 @@ static status_t roam(private_ike_sa_t *this)
 												   other);
 	if (me)
 	{
-		best = get_path_prio(me, other);
+		if (me->get_family(me) == AF_INET)
+		{
+			best4 = get_path_prio(me, other);
+		}
+		else
+		{
+			best6 = get_path_prio(me, other);
+		}
 	}
 	iterator = create_additional_address_iterator(this);
 	while (iterator->iterate(iterator, (void**)&cand_other))
 	{
+		bool better = FALSE;
+	
 		cand_me = charon->kernel_interface->get_source_addr(
 										charon->kernel_interface, cand_other);
 		if (!cand_me)
@@ -1763,10 +1805,28 @@ static status_t roam(private_ike_sa_t *this)
 			cand_me->destroy(cand_me);
 			continue;
 		}
+		
 		prio = get_path_prio(cand_me, cand_other);
-		if (prio > best)
+		if (cand_me->get_family(cand_me) == AF_INET)
+		{
+			if (prio > best4 && (best6 == 0 ||
+						this->my_host->get_family(this->my_host) == AF_INET))
+			{
+				best4 = prio;
+				better = TRUE;
+			}
+		}
+		else
+		{
+			if (prio > best6 && (best4 == 0 ||
+						this->my_host->get_family(this->my_host) == AF_INET6))
+			{
+				best6 = prio;
+				better = TRUE;
+			}
+		}
+		if (better)
 		{
-			best = prio;
 			DESTROY_IF(me);
 			me = cand_me;
 			other = cand_other;
@@ -1797,19 +1857,20 @@ static status_t roam(private_ike_sa_t *this)
 	me->set_port(me, this->my_host->get_port(this->my_host));
 	other = other->clone(other);
 	other->set_port(other, this->other_host->get_port(this->other_host));
+	set_my_host(this, me);
+	set_other_host(this, other);
 	
 	/* update addresses with mobike, if supported ... */
 	if (supports_extension(this, EXT_MOBIKE))
 	{
 		DBG1(DBG_IKE, "requesting address change using MOBIKE");
 		mobike = ike_mobike_create(&this->public, TRUE);
-		mobike->roam(mobike, me, other);
+		mobike->roam(mobike, address);
 		this->task_manager->queue_task(this->task_manager, (task_t*)mobike);
 		return this->task_manager->initiate(this->task_manager);
 	}
 	DBG1(DBG_IKE, "reestablishing IKE_SA due address change");
 	/* ... reestablish if not */
-	set_my_host(this, me);
 	return reestablish(this);
 }
 
@@ -2078,6 +2139,8 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id)
 	this->public.supports_extension = (bool(*)(ike_sa_t*, ike_extension_t extension))supports_extension;
 	this->public.set_condition = (void (*)(ike_sa_t*, ike_condition_t,bool)) set_condition;
 	this->public.has_condition = (bool (*)(ike_sa_t*,ike_condition_t)) has_condition;
+	this->public.set_pending_updates = (void(*)(ike_sa_t*, u_int32_t updates))set_pending_updates;
+	this->public.get_pending_updates = (u_int32_t(*)(ike_sa_t*))get_pending_updates;
 	this->public.create_additional_address_iterator = (iterator_t*(*)(ike_sa_t*))create_additional_address_iterator;
 	this->public.add_additional_address = (void(*)(ike_sa_t*, host_t *host))add_additional_address;
 	this->public.retransmit = (status_t (*)(ike_sa_t *, u_int32_t)) retransmit;
@@ -2098,7 +2161,7 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id)
 	this->public.destroy_child_sa = (status_t (*)(ike_sa_t*,protocol_id_t,u_int32_t))destroy_child_sa;
 	this->public.rekey = (status_t (*)(ike_sa_t*))rekey;
 	this->public.reestablish = (status_t (*)(ike_sa_t*))reestablish;
-	this->public.roam = (status_t(*)(ike_sa_t*))roam;
+	this->public.roam = (status_t(*)(ike_sa_t*,bool))roam;
 	this->public.inherit = (status_t (*)(ike_sa_t*,ike_sa_t*))inherit;
 	this->public.generate_message = (status_t (*)(ike_sa_t*,message_t*,packet_t**))generate_message;
 	this->public.reset = (void (*)(ike_sa_t*))reset;
@@ -2138,6 +2201,7 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id)
 	this->other_virtual_ip = NULL;
 	this->dns_servers = linked_list_create();
 	this->additional_addresses = linked_list_create();
+	this->pending_updates = 0;
 	this->keyingtry = 0;
 	
 	return &this->public;
diff --git a/src/charon/sa/ike_sa.h b/src/charon/sa/ike_sa.h
index b7eceedd2..ba189577c 100644
--- a/src/charon/sa/ike_sa.h
+++ b/src/charon/sa/ike_sa.h
@@ -433,6 +433,22 @@ struct ike_sa_t {
 	bool (*has_condition) (ike_sa_t *this, ike_condition_t condition);
 	
 	/**
+	 * @brief Get the number of queued MOBIKE address updates.
+	 *
+	 * @param this			calling object
+	 * @return				number of pending updates
+	 */
+	u_int32_t (*get_pending_updates)(ike_sa_t *this);
+	
+	/**
+	 * @brief Set the number of queued MOBIKE address updates.
+	 *
+	 * @param this			calling object
+	 * @param updates		number of pending updates
+	 */
+	void (*set_pending_updates)(ike_sa_t *this, u_int32_t updates);
+	
+	/**
 	 * @brief Initiate a new connection.
 	 *
 	 * The configs are owned by the IKE_SA after the call.
@@ -510,10 +526,11 @@ struct ike_sa_t {
 	 * If MOBIKE is supported, addresses are updated; If not, the tunnel is
 	 * restarted.
 	 *
-	 * @param 
-	 * @return
+	 * @param this			calling object
+	 * @param address		TRUE if address list changed, FALSE otherwise
+	 * @return				SUCCESS, FAILED, DESTROY_ME
 	 */
-	status_t (*roam)(ike_sa_t *this);
+	status_t (*roam)(ike_sa_t *this, bool address);
 	
 	/**
 	 * @brief Processes a incoming IKEv2-Message.
diff --git a/src/charon/sa/task_manager.c b/src/charon/sa/task_manager.c
index 6d77e3be8..55592f437 100644
--- a/src/charon/sa/task_manager.c
+++ b/src/charon/sa/task_manager.c
@@ -719,6 +719,10 @@ static status_t process_request(private_task_manager_t *this,
 					default:
 						break;
 				}
+				if (task)
+				{
+					break;
+				}
 			}
 			iterator->destroy(iterator);
 			
diff --git a/src/charon/sa/tasks/ike_mobike.c b/src/charon/sa/tasks/ike_mobike.c
index 096199c7a..8d4dce36c 100644
--- a/src/charon/sa/tasks/ike_mobike.c
+++ b/src/charon/sa/tasks/ike_mobike.c
@@ -52,24 +52,24 @@ struct private_ike_mobike_t {
 	bool initiator;
 	
 	/**
-	 * local host to roam to
+	 * cookie2 value to verify new addresses
 	 */
-	host_t *me;
+	chunk_t cookie2;
 	
 	/**
-	 * remote host to roam to
+	 * NAT discovery reusing the IKE_NATD task
 	 */
-	host_t *other;
+	ike_natd_t *natd;
 	
 	/**
-	 * cookie2 value to verify new addresses
+	 * use task to update addresses
 	 */
-	chunk_t cookie2;
+	bool roam;
 	
 	/**
-	 * NAT discovery reusing the IKE_NATD task
+	 * include address list update
 	 */
-	ike_natd_t *natd;
+	bool address;
 };
 
 /**
@@ -138,6 +138,11 @@ static void process_payloads(private_ike_mobike_t *this, message_t *message)
 				this->ike_sa->add_additional_address(this->ike_sa, host);
 				break;
 			}
+			case UPDATE_SA_ADDRESSES:
+			{
+				this->roam = TRUE;
+				break;
+			}
 			case NO_ADDITIONAL_ADDRESSES:
 			{
 				flush_additional_addresses(this);
@@ -201,6 +206,25 @@ static void build_address_list(private_ike_mobike_t *this, message_t *message)
 }
 
 /**
+ * update addresses of associated CHILD_SAs
+ */
+static void update_children(private_ike_mobike_t *this)
+{
+	iterator_t *iterator;
+	child_sa_t *child_sa;
+	
+	iterator = this->ike_sa->create_child_sa_iterator(this->ike_sa);
+	while (iterator->iterate(iterator, (void**)&child_sa))
+	{
+		child_sa->update_hosts(child_sa,
+						this->ike_sa->get_my_host(this->ike_sa), 
+						this->ike_sa->get_other_host(this->ike_sa),
+						this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY));
+	}
+	iterator->destroy(iterator);
+}
+
+/**
  * Implementation of task_t.process for initiator
  */
 static status_t build_i(private_ike_mobike_t *this, message_t *message)
@@ -211,16 +235,20 @@ static status_t build_i(private_ike_mobike_t *this, message_t *message)
 		message->add_notify(message, FALSE, MOBIKE_SUPPORTED, chunk_empty);
 		build_address_list(this, message);
 	}
-	else if (this->me || this->other)
-	{	/* address change */
-		message->add_notify(message, FALSE, UPDATE_SA_ADDRESSES, chunk_empty);
-		build_address_list(this, message);
-		/* set new addresses */
-		this->ike_sa->update_hosts(this->ike_sa, this->me, this->other);
-		if (this->natd)
+	else 
+	{
+		if (this->roam)
 		{
-			this->natd->task.build(&this->natd->task, message);
+			message->add_notify(message, FALSE, UPDATE_SA_ADDRESSES, chunk_empty);
 		}
+		if (this->address)
+		{
+			build_address_list(this, message);
+		}
+		
+		this->natd = ike_natd_create(this->ike_sa, this->initiator);
+		this->natd->task.build(&this->natd->task, message);
+		update_children(this);
 	}
 	
 	return NEED_MORE;
@@ -239,6 +267,16 @@ static status_t process_r(private_ike_mobike_t *this, message_t *message)
 	else if (message->get_exchange_type(message) == INFORMATIONAL)
 	{
 		process_payloads(this, message);
+		if (this->roam)
+		{
+			host_t *me, *other;
+			
+			me = message->get_destination(message);
+			other = message->get_source(message);
+			this->ike_sa->set_my_host(this->ike_sa, me->clone(me));
+			this->ike_sa->set_other_host(this->ike_sa, other->clone(other));
+		}
+		
 		if (this->natd)
 		{
 			this->natd->task.process(&this->natd->task, message);
@@ -268,6 +306,10 @@ static status_t build_r(private_ike_mobike_t *this, message_t *message)
 		{
 			this->natd->task.build(&this->natd->task, message);
 		}
+		if (this->roam)
+		{
+			update_children(this);
+		}
 		return SUCCESS;
 	}
 	return NEED_MORE;
@@ -287,11 +329,23 @@ static status_t process_i(private_ike_mobike_t *this, message_t *message)
 	}
 	else if (message->get_exchange_type(message) == INFORMATIONAL)
 	{
+		u_int32_t updates = this->ike_sa->get_pending_updates(this->ike_sa) - 1;
+		this->ike_sa->set_pending_updates(this->ike_sa, updates);
+		if (updates > 0)
+		{
+			/* newer update queued, ignore this one */
+			return SUCCESS;
+		}
 		process_payloads(this, message);
 		if (this->natd)
 		{
 			this->natd->task.process(&this->natd->task, message);
 		}
+		if (this->roam)
+		{
+			/* update again, as NAT state may have changed */
+			update_children(this);
+		}
 		return SUCCESS;
 	}
 	return NEED_MORE;
@@ -300,13 +354,12 @@ static status_t process_i(private_ike_mobike_t *this, message_t *message)
 /**
  * Implementation of ike_mobike_t.roam.
  */
-static void roam(private_ike_mobike_t *this, host_t *me, host_t *other)
+static void roam(private_ike_mobike_t *this, bool address)
 {
-	this->me = me;
-	this->other = other;
-	
-	/* include NAT detection when roaming */
-	this->natd = ike_natd_create(this->ike_sa, this->initiator);
+	this->roam = TRUE;
+	this->address = address;
+	this->ike_sa->set_pending_updates(this->ike_sa, 
+							this->ike_sa->get_pending_updates(this->ike_sa) + 1);
 }
 
 /**
@@ -322,12 +375,8 @@ static task_type_t get_type(private_ike_mobike_t *this)
  */
 static void migrate(private_ike_mobike_t *this, ike_sa_t *ike_sa)
 {
-	DESTROY_IF(this->me);
-	DESTROY_IF(this->other);
 	chunk_free(&this->cookie2);
 	this->ike_sa = ike_sa;
-	this->me = NULL;
-	this->other = NULL;
 	if (this->natd)
 	{
 		this->natd->task.migrate(&this->natd->task, ike_sa);
@@ -339,8 +388,6 @@ static void migrate(private_ike_mobike_t *this, ike_sa_t *ike_sa)
  */
 static void destroy(private_ike_mobike_t *this)
 {
-	DESTROY_IF(this->me);
-	DESTROY_IF(this->other);
 	chunk_free(&this->cookie2);
 	if (this->natd)
 	{
@@ -356,7 +403,7 @@ ike_mobike_t *ike_mobike_create(ike_sa_t *ike_sa, bool initiator)
 {
 	private_ike_mobike_t *this = malloc_thing(private_ike_mobike_t);
 
-	this->public.roam = (void(*)(ike_mobike_t*, host_t *, host_t *))roam;
+	this->public.roam = (void(*)(ike_mobike_t*,bool))roam;
 	this->public.task.get_type = (task_type_t(*)(task_t*))get_type;
 	this->public.task.migrate = (void(*)(task_t*,ike_sa_t*))migrate;
 	this->public.task.destroy = (void(*)(task_t*))destroy;
@@ -374,8 +421,8 @@ ike_mobike_t *ike_mobike_create(ike_sa_t *ike_sa, bool initiator)
 	
 	this->ike_sa = ike_sa;
 	this->initiator = initiator;
-	this->me = NULL;
-	this->other = NULL;
+	this->roam = FALSE;
+	this->address = TRUE;
 	this->cookie2 = chunk_empty;
 	this->natd = NULL;
 	
diff --git a/src/charon/sa/tasks/ike_mobike.h b/src/charon/sa/tasks/ike_mobike.h
index f815e4998..db493c459 100644
--- a/src/charon/sa/tasks/ike_mobike.h
+++ b/src/charon/sa/tasks/ike_mobike.h
@@ -54,13 +54,10 @@ struct ike_mobike_t {
 	/**
 	 * @brief Use the task to roam to other addresses.
 	 *
-	 * Supplied hosts may be NULL to reuse existing IKE_SA hosts.
-	 *
 	 * @param this			calling object
-	 * @param me			local host to roam to, or NULL
-	 * @param other			remote host to roam to, or NULL
+	 * @param address		TRUE to include address list update
 	 */
-	void (*roam)(ike_mobike_t *this, host_t *me, host_t *other);
+	void (*roam)(ike_mobike_t *this, bool address);
 };
 
 /**