using same reqid if a child sa rekeys an existing one

7 files changed, 37 insertions, 20 deletions

diff --git a/src/charon/sa/child_sa.c b/src/charon/sa/child_sa.c
index 2b2b6a732..58796e3ae 100644
--- a/src/charon/sa/child_sa.c
+++ b/src/charon/sa/child_sa.c
@@ -496,7 +496,8 @@ static void destroy(private_child_sa_t *this)
 /*
  * Described in header.
  */
-child_sa_t * child_sa_create(host_t *me, host_t* other, u_int32_t soft_lifetime, u_int32_t hard_lifetime)
+child_sa_t * child_sa_create(u_int32_t rekey, host_t *me, host_t* other, 
+							 u_int32_t soft_lifetime, u_int32_t hard_lifetime)
 {
 	static u_int32_t reqid = 2000000000;
 	private_child_sa_t *this = malloc_thing(private_child_sa_t);
@@ -521,7 +522,8 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, u_int32_t soft_lifetime,
 	this->other.spi = 0;
 	this->soft_lifetime = soft_lifetime;
 	this->hard_lifetime = hard_lifetime;
-	this->reqid = ++reqid;
+	/* reuse old reqid if we are rekeying an existing CHILD_SA */
+	this->reqid = rekey ? rekey : ++reqid;
 	this->policies = linked_list_create();
 	this->protocol = PROTO_NONE;
 	this->rekeyed = 0;
diff --git a/src/charon/sa/child_sa.h b/src/charon/sa/child_sa.h
index 2bbe95fe0..687a7d627 100644
--- a/src/charon/sa/child_sa.h
+++ b/src/charon/sa/child_sa.h
@@ -183,6 +183,7 @@ struct child_sa_t {
 /**
  * @brief Constructor to create a new child_sa_t.
  * 
+ * @param rekey_reqid	reqid of old CHILD_SA when rekeying, 0 otherwise
  * @param me			own address
  * @param other			remote address
  * @param soft_lifetime	time before rekeying
@@ -191,6 +192,7 @@ struct child_sa_t {
  * 
  * @ingroup sa
  */
-child_sa_t * child_sa_create(host_t *me, host_t *other, u_int32_t soft_lifetime, u_int32_t hard_lifetime);
+child_sa_t * child_sa_create(u_int32_t rekey_reqid, host_t *me, host_t *other, 
+							 u_int32_t soft_lifetime, u_int32_t hard_lifetime);
 
 #endif /*CHILD_SA_H_*/
diff --git a/src/charon/sa/ike_sa.c b/src/charon/sa/ike_sa.c
index 7ddf0d806..b4a123db3 100644
--- a/src/charon/sa/ike_sa.c
+++ b/src/charon/sa/ike_sa.c
@@ -1025,7 +1025,8 @@ static status_t rekey_child_sa(private_ike_sa_t *this, u_int32_t reqid)
 	request->add_payload(request, (payload_t*)notify);
 	
 	proposals = this->policy->get_proposals(this->policy);
-	child_sa = child_sa_create(this->connection->get_my_host(this->connection),
+	child_sa = child_sa_create(reqid,
+							   this->connection->get_my_host(this->connection),
 							   this->connection->get_other_host(this->connection),
 							   this->policy->get_soft_lifetime(this->policy),
 							   this->policy->get_hard_lifetime(this->policy));
diff --git a/src/charon/sa/states/ike_sa_established.c b/src/charon/sa/states/ike_sa_established.c
index 66f0c103b..46c181012 100644
--- a/src/charon/sa/states/ike_sa_established.c
+++ b/src/charon/sa/states/ike_sa_established.c
@@ -74,6 +74,11 @@ struct private_ike_sa_established_t {
 	 */
 	child_sa_t *child_sa;
 	
+	/**
+	 * Old child sa, if we are rekeying
+	 */
+	child_sa_t *old_child_sa;
+	
 	/** 
 	 * Assigned logger. Use logger of IKE_SA.
 	 */
@@ -93,6 +98,7 @@ static status_t build_sa_payload(private_ike_sa_established_t *this, sa_payload_
 	status_t status;
 	connection_t *connection;
 	policy_t *policy;
+	u_int32_t reqid = 0;
 	
 	/* prepare reply */
 	sa_response = sa_payload_create();
@@ -130,7 +136,12 @@ static status_t build_sa_payload(private_ike_sa_established_t *this, sa_payload_
 		
 		policy = this->ike_sa->get_policy(this->ike_sa);
 		connection = this->ike_sa->get_connection(this->ike_sa);
-		this->child_sa = child_sa_create(connection->get_my_host(connection),
+		if (this->old_child_sa)
+		{	/* reuse old reqid if we are rekeying */
+			reqid = this->old_child_sa->get_reqid(this->old_child_sa);
+		}
+		this->child_sa = child_sa_create(reqid,
+										 connection->get_my_host(connection),
 										 connection->get_other_host(connection),
 										 policy->get_soft_lifetime(policy),
 										 policy->get_hard_lifetime(policy));
@@ -243,7 +254,6 @@ static status_t process_create_child_sa(private_ike_sa_established_t *this, mess
 	notify_payload_t *notify = NULL;
 	iterator_t *payloads;
 	status_t status;
-	child_sa_t *old_child_sa;
 	
 	/* iterate over incoming payloads. Message is verified, we can be sure there are the required payloads */
 	payloads = request->get_payload_iterator(request);
@@ -280,7 +290,7 @@ static status_t process_create_child_sa(private_ike_sa_established_t *this, mess
 				 * for CHILD_SAs. */
 				u_int16_t no_group[1];
 				no_group[0] = htons(MODP_NONE);
-				chunk_t no_group_chunk = chunk_from_buf(no_group);
+				chunk_t no_group_chunk = chunk_from_buf((u_int8_t*)no_group);
 				this->ike_sa->send_notify(this->ike_sa, CREATE_CHILD_SA, INVALID_KE_PAYLOAD, no_group_chunk);
 				payloads->destroy(payloads);
 				return FAILED;
@@ -307,6 +317,11 @@ static status_t process_create_child_sa(private_ike_sa_established_t *this, mess
 		this->logger->log(this->logger, AUDIT, "CREATE_CHILD_SA request did not contain all required payloads. Ignored");
 		return FAILED;
 	}
+	
+	if (notify && notify->get_notify_message_type(notify) == REKEY_SA)
+	{
+		this->old_child_sa = this->ike_sa->get_child_sa(this->ike_sa, notify->get_spi(notify));
+	}
 		
 	/* build response */
 	this->ike_sa->build_message(this->ike_sa, CREATE_CHILD_SA, FALSE, &response);
@@ -364,15 +379,9 @@ static status_t process_create_child_sa(private_ike_sa_established_t *this, mess
 		{
 			this->logger->log(this->logger, AUDIT, "Could not install CHILD_SA policy!");
 		}
-		
-		if (notify && notify->get_notify_message_type(notify) == REKEY_SA)
-		{
-			/* mark old child sa as rekeyed */
-			old_child_sa = this->ike_sa->get_child_sa(this->ike_sa, notify->get_spi(notify));
-			if (old_child_sa)
-			{
-				old_child_sa->set_rekeyed(old_child_sa, this->child_sa->get_reqid(this->child_sa));
-			}
+		if (this->old_child_sa)
+		{	/* mark old child sa as rekeyed */
+			this->old_child_sa->set_rekeyed(this->old_child_sa, this->child_sa->get_reqid(this->child_sa));
 		}
 		this->ike_sa->add_child_sa(this->ike_sa, this->child_sa);
 	}
@@ -553,6 +562,7 @@ ike_sa_established_t *ike_sa_established_create(protected_ike_sa_t *ike_sa)
 	this->logger = logger_manager->get_logger(logger_manager, IKE_SA);
 	this->nonce_i = CHUNK_INITIALIZER;
 	this->nonce_r = CHUNK_INITIALIZER;
+	this->old_child_sa = NULL;
 	
 	return &(this->public);
 }
diff --git a/src/charon/sa/states/ike_sa_init_requested.c b/src/charon/sa/states/ike_sa_init_requested.c
index 40b0f9551..60288ae84 100644
--- a/src/charon/sa/states/ike_sa_init_requested.c
+++ b/src/charon/sa/states/ike_sa_init_requested.c
@@ -571,7 +571,8 @@ static status_t build_sa_payload (private_ike_sa_init_requested_t *this, message
 	proposal_list = policy->get_proposals(policy);
 	/* build child sa */
 	connection = this->ike_sa->get_connection(this->ike_sa);
-	this->child_sa = child_sa_create(connection->get_my_host(connection),
+	this->child_sa = child_sa_create(0,
+									 connection->get_my_host(connection),
 									 connection->get_other_host(connection),
 									 policy->get_soft_lifetime(policy),
 									 policy->get_hard_lifetime(policy));
diff --git a/src/charon/sa/states/ike_sa_init_responded.c b/src/charon/sa/states/ike_sa_init_responded.c
index f9d1e8405..ae4a084fa 100644
--- a/src/charon/sa/states/ike_sa_init_responded.c
+++ b/src/charon/sa/states/ike_sa_init_responded.c
@@ -477,7 +477,8 @@ static status_t build_sa_payload(private_ike_sa_init_responded_t *this, sa_paylo
 		
 		policy = this->ike_sa->get_policy(this->ike_sa);
 		connection = this->ike_sa->get_connection(this->ike_sa);
-		this->child_sa = child_sa_create(connection->get_my_host(connection),
+		this->child_sa = child_sa_create(0,
+										 connection->get_my_host(connection),
 										 connection->get_other_host(connection),
 										 policy->get_soft_lifetime(policy),
 										 policy->get_hard_lifetime(policy));
diff --git a/src/charon/testing/child_sa_test.c b/src/charon/testing/child_sa_test.c
index f36ab74d4..2c318a624 100644
--- a/src/charon/testing/child_sa_test.c
+++ b/src/charon/testing/child_sa_test.c
@@ -49,8 +49,8 @@ void test_child_sa(protected_tester_t *tester)
 	remote_me = host_create(AF_INET, "192.168.0.3", 0);
 	remote_other = host_create(AF_INET, "192.168.0.4", 0);
 	
-	local_sa = child_sa_create(local_me, local_other, 5, 10);
-	remote_sa = child_sa_create(remote_me, remote_other, 5, 10);
+	local_sa = child_sa_create(0, local_me, local_other, 5, 10);
+	remote_sa = child_sa_create(0, remote_me, remote_other, 5, 10);
 	
 	proposal1 = proposal_create(PROTO_ESP);
 	proposal1->add_algorithm(proposal1, ENCRYPTION_ALGORITHM, ENCR_AES_CBC, 16);