added CHILD_SA states, which allows us to detect further simultaneous transactions

reimplemented the buggy message id handling

src/charon/queues/jobs/delete_half_open_ike_sa_job.c

@@ -70,7 +70,7 @@ static status_t execute(private_delete_half_open_ike_sa_job_t *this)
 	
 	switch (ike_sa->get_state(ike_sa))
 	{
-		case SA_ESTABLISHED:
+		case IKE_ESTABLISHED:
 		{
 			/* IKE_SA is established and so is not getting destroyed */
 			charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa);

src/charon/sa/child_sa.c

@@ -29,6 +29,17 @@
 #include <daemon.h>
 
 
+/**
+ * String mappings for child_sa_state_t.
+ */
+mapping_t child_sa_state_m[] = {
+	{CHILD_CREATED, "CREATED"},
+	{CHILD_INSTALLED, "INSTALLED"},
+	{CHILD_REKEYING, "REKEYING"},
+	{CHILD_DELETING, "DELETING"},
+	{MAPPING_END, NULL}
+};
+
 typedef struct sa_policy_t sa_policy_t;
 
 /**
@@ -109,6 +120,11 @@ struct private_child_sa_t {
 	 */
 	u_int32_t hard_lifetime;
 	
+	/**
+	 * state of the CHILD_SA
+	 */
+	child_sa_state_t state;
+	
 	/**
 	 * transaction which is rekeying this CHILD_SA
 	 */
@@ -153,6 +169,22 @@ protocol_id_t get_protocol(private_child_sa_t *this)
 	return this->protocol;
 }
 
+/**
+ * Implements child_sa_t.get_state
+ */
+static child_sa_state_t get_state(private_child_sa_t *this)
+{
+	return this->state;
+}
+
+/**
+ * Implements child_sa_t.set_state
+ */
+static void set_state(private_child_sa_t *this, child_sa_state_t state)
+{
+	this->state = state;
+}
+
 /**
  * Allocate SPI for a single proposal
  */
@@ -354,6 +386,8 @@ static status_t add(private_child_sa_t *this, proposal_t *proposal, prf_plus_t *
 	}
 	proposal->set_spi(proposal, inbound_spi);
 	
+	this->state = CHILD_INSTALLED;
+	
 	return SUCCESS;
 }
 
@@ -378,6 +412,8 @@ static status_t update(private_child_sa_t *this, proposal_t *proposal, prf_plus_
 		return FAILED;
 	}
 	
+	this->state = CHILD_INSTALLED;
+	
 	return SUCCESS;
 }
 
@@ -556,8 +592,9 @@ static void log_status(private_child_sa_t *this, logger_t *logger, char* name)
 				this->reqid);
 	
 	logger->log(logger, CONTROL|LEVEL1, 
-				"  \"%s\":   rekeying: %s, last traffic (in/out): %s/%s",
+				"  \"%s\":   state: %s, rekeying: %s, last traffic (in/out): %s/%s",
-				name, rekey_str, use_in_str, use_out_str);
+				name, mapping_find(child_sa_state_m, this->state),
+				rekey_str, use_in_str, use_out_str);
 	
 	iterator = this->policies->create_iterator(this->policies, TRUE);
 	while (iterator->has_next(iterator))
@@ -844,6 +881,8 @@ child_sa_t * child_sa_create(u_int32_t rekey, host_t *me, host_t* other,
 	this->public.get_use_time = (status_t (*)(child_sa_t*,bool,time_t*))get_use_time;
 	this->public.set_rekeying_transaction = (void (*)(child_sa_t*,void*))set_rekeying_transaction;
 	this->public.get_rekeying_transaction = (void* (*)(child_sa_t*))get_rekeying_transaction;
+	this->public.set_state = (void(*)(child_sa_t*,child_sa_state_t))set_state;
+	this->public.get_state = (child_sa_state_t(*)(child_sa_t*))get_state;
 	this->public.log_status = (void (*)(child_sa_t*, logger_t*, char*))log_status;
 	this->public.destroy = (void(*)(child_sa_t*))destroy;
 
@@ -858,6 +897,7 @@ child_sa_t * child_sa_create(u_int32_t rekey, host_t *me, host_t* other,
 	this->use_natt = use_natt;
 	this->soft_lifetime = soft_lifetime;
 	this->hard_lifetime = hard_lifetime;
+	this->state = CHILD_CREATED;
 	/* reuse old reqid if we are rekeying an existing CHILD_SA */
 	this->reqid = rekey ? rekey : ++reqid;
 	this->policies = linked_list_create();

src/charon/sa/child_sa.h

@@ -36,6 +36,39 @@
  */
 #define REQID_START 2000000000
 
+typedef enum child_sa_state_t child_sa_state_t;
+
+/**
+ * @brief States of a CHILD_SA
+ */
+enum child_sa_state_t {
+	
+	/**
+	 * Just created, uninstalled CHILD_SA
+	 */
+	CHILD_CREATED,
+	
+	/**
+	 * Installed an in-use CHILD_SA
+	 */
+	CHILD_INSTALLED,
+	
+	/**
+	 * CHILD_SA which is rekeying
+	 */
+	CHILD_REKEYING,
+	
+	/**
+	 * CHILD_SA in progress of delete
+	 */
+	CHILD_DELETING,
+};
+
+/**
+ * String mappings for child_sa_state_t.
+ */
+extern mapping_t child_sa_state_m[];
+
 typedef struct child_sa_t child_sa_t;
 
 /**
@@ -167,6 +200,20 @@ struct child_sa_t {
 	 */	
 	status_t (*get_use_time) (child_sa_t *this, bool inbound, time_t *use_time);
 	
+	/**
+	 * @brief Get the state of the CHILD_SA.
+	 *
+	 * @param this 		calling object
+	 */	
+	child_sa_state_t (*get_state) (child_sa_t *this);
+	
+	/**
+	 * @brief Set the state of the CHILD_SA.
+	 *
+	 * @param this 		calling object
+	 */	
+	void (*set_state) (child_sa_t *this, child_sa_state_t state);
+	
 	/**
 	 * @brief Set the transaction which rekeys this CHILD_SA.
 	 *

src/charon/sa/ike_sa.c

@@ -60,10 +60,10 @@
  * String mappings for ike_sa_state_t.
  */
 mapping_t ike_sa_state_m[] = {
-	{SA_CREATED, "CREATED"},
+	{IKE_CREATED, "CREATED"},
-	{SA_CONNECTING, "CONNECTING"},
+	{IKE_CONNECTING, "CONNECTING"},
-	{SA_ESTABLISHED, "ESTABLISHED"},
+	{IKE_ESTABLISHED, "ESTABLISHED"},
-	{SA_DELETING, "DELETING"},
+	{IKE_DELETING, "DELETING"},
 	{MAPPING_END, NULL}
 };
 
@@ -525,23 +525,10 @@ static status_t process_request(private_ike_sa_t *this, message_t *request)
 	/* check if we already have a pre-created transaction for this request */
 	if (this->transaction_in_next)
 	{
-		u_int32_t trans_mid = this->transaction_in_next->get_message_id(this->transaction_in_next);
+		current = this->transaction_in_next;
-		
-		/* check message id consistency */
-		if (trans_mid == request_mid)
-		{
-			/* use it */
-			current = this->transaction_in_next;
-		}
-		else
-		{
-			/* discard queued transaction */
-			this->transaction_in_next->destroy(this->transaction_in_next);
-		}
 		this->transaction_in_next = NULL;
 	}
-	/* create new transaction if "next" unusable */
+	else
-	if (current == NULL)
 	{
 		current = transaction_create(&this->public, request);
 		if (current == NULL)
@@ -614,13 +601,6 @@ static status_t process_response(private_ike_sa_t *this, message_t *response)
 	current->destroy(current);
 	this->transaction_out = NULL;
 	
-	/* if conclude() created a new transaction, we increment the message_id
-	 * counter, as the new transaction used the next one */
-	if (new)
-	{
-		this->message_id_out = new->get_message_id(new) + 1;;
-	}
-	
 	/* queue new transaction */
 	return queue_transaction(this, new, TRUE);
 }
@@ -723,7 +703,7 @@ static status_t process_message(private_ike_sa_t *this, message_t *message)
 	else
 	{
 		/* check if message is trustworthy, and update connection information */
-		if ((this->state == SA_CREATED && this->connection) ||
+		if ((this->state == IKE_CREATED && this->connection) ||
 			message->get_exchange_type(message) != IKE_SA_INIT)
 		{
 			update_hosts(this, message->get_destination(message),
@@ -760,8 +740,8 @@ static status_t initiate(private_ike_sa_t *this, connection_t *connection)
 						  connection->get_name(connection));
 		return DESTROY_ME;
 	}
-	this->message_id_out = 0;
+	this->message_id_out = 1;
-	ike_sa_init = ike_sa_init_create(&this->public, this->message_id_out++);
+	ike_sa_init = ike_sa_init_create(&this->public);
 	return queue_transaction(this, (transaction_t*)ike_sa_init, TRUE);
 }
 
@@ -793,7 +773,7 @@ static status_t send_dpd(private_ike_sa_t *this)
 			/* to long ago, initiate dead peer detection */
 			dead_peer_detection_t *dpd;
 			this->logger->log(this->logger, CONTROL, "sending DPD request");
-			dpd = dead_peer_detection_create(&this->public, this->message_id_out++);
+			dpd = dead_peer_detection_create(&this->public);
 			status = queue_transaction(this, (transaction_t*)dpd, FALSE);
 			diff = 0;
 		}
@@ -859,7 +839,7 @@ static void set_state(private_ike_sa_t *this, ike_sa_state_t state)
 	this->logger->log(this->logger, CONTROL, "state change: %s => %s",
 					  mapping_find(ike_sa_state_m, this->state),
 					  mapping_find(ike_sa_state_m, state));
-	if (state == SA_ESTABLISHED)
+	if (state == IKE_ESTABLISHED)
 	{
 		host_t *my_host, *other_host;
 		identification_t *my_id, *other_id;
@@ -1162,7 +1142,7 @@ static status_t rekey_child_sa(private_ike_sa_t *this, protocol_id_t protocol, u
 		return NOT_FOUND;
 	}
 	
-	rekey = create_child_sa_create(&this->public, this->message_id_out++);
+	rekey = create_child_sa_create(&this->public);
 	rekey->rekeys_child(rekey, child_sa);
 	return queue_transaction(this, (transaction_t*)rekey, FALSE);
 }
@@ -1181,7 +1161,7 @@ static status_t delete_child_sa(private_ike_sa_t *this, protocol_id_t protocol,
 		return NOT_FOUND;
 	}
 	
-	del = delete_child_sa_create(&this->public, this->message_id_out++);
+	del = delete_child_sa_create(&this->public);
 	del->set_child_sa(del, child_sa);
 	return queue_transaction(this, (transaction_t*)del, FALSE);
 }
@@ -1276,11 +1256,19 @@ static void log_status(private_ike_sa_t *this, logger_t *logger, char *name)
 static status_t delete_(private_ike_sa_t *this)
 {
 	delete_ike_sa_t *delete_ike_sa;
-	delete_ike_sa = delete_ike_sa_create(&this->public, this->message_id_out++);
+	delete_ike_sa = delete_ike_sa_create(&this->public);
 	
 	return queue_transaction(this, (transaction_t*)delete_ike_sa, FALSE);
 }
 
+/**
+ * Implementation of ike_sa_t.get_next_message_id.
+ */
+static u_int32_t get_next_message_id (private_ike_sa_t *this)
+{
+	return this->message_id_out++;
+}
+
 /**
  * Implementation of ike_sa_t.is_natt_enabled.
  */
@@ -1322,7 +1310,7 @@ static void destroy(private_ike_sa_t *this)
 					  this->ike_sa_id->get_responder_spi(this->ike_sa_id),
 					  this->ike_sa_id->is_initiator(this->ike_sa_id) ? "initiator" : "responder");
 	
-	if (this->state == SA_ESTABLISHED)
+	if (this->state == IKE_ESTABLISHED)
 	{
 		this->logger->log(this->logger, ERROR, 
 						  "destroying an established IKE SA without knowledge from remote peer!");
@@ -1423,6 +1411,7 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id)
 	this->public.process_message = (status_t(*)(ike_sa_t*, message_t*)) process_message;
 	this->public.initiate = (status_t(*)(ike_sa_t*,connection_t*)) initiate;
 	this->public.get_id = (ike_sa_id_t*(*)(ike_sa_t*)) get_id;
+	this->public.get_next_message_id = (u_int32_t(*)(ike_sa_t*)) get_next_message_id;
 	this->public.get_connection = (connection_t*(*)(ike_sa_t*)) get_connection;
 	this->public.retransmit_request = (status_t (*) (ike_sa_t *, u_int32_t)) retransmit_request;
 	this->public.log_status = (void (*) (ike_sa_t*,logger_t*,char*))log_status;
@@ -1467,7 +1456,8 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id)
 	this->transaction_in = NULL;
 	this->transaction_in_next = NULL;
 	this->transaction_out = NULL;
-	this->state = SA_CREATED;
+	this->state = IKE_CREATED;
+	/* we start with message ID out, as ike_sa_init does not use this counter */
 	this->message_id_out = 0;
 	this->time_inbound = 0;
 	this->time_outbound = 0;

src/charon/sa/ike_sa.h

@@ -86,22 +86,22 @@ enum ike_sa_state_t {
 	/**
 	 * IKE_SA just got created, but is not initiating nor responding yet.
 	 */
-	SA_CREATED,
+	IKE_CREATED,
 	
 	/**
 	 * IKE_SA gets initiated actively or passively
 	 */
-	SA_CONNECTING,
+	IKE_CONNECTING,
 	
 	/**
 	 * IKE_SA is fully established
 	 */
-	SA_ESTABLISHED,
+	IKE_ESTABLISHED,
 	
 	/**
 	 * IKE_SA is in progress of deletion
 	 */
-	SA_DELETING,
+	IKE_DELETING,
 };
 
 /**
@@ -210,6 +210,14 @@ struct ike_sa_t {
 	 */
 	status_t (*process_message) (ike_sa_t *this, message_t *message);
 	
+	/**
+	 * @brief Get the next message ID for a request.
+	 *
+	 * @param this 			calling object
+	 * @return 				the next message id
+	 */
+	u_int32_t (*get_next_message_id) (ike_sa_t *this);
+	
 	/**
 	 * @brief Check if NAT traversal is enabled for this IKE_SA.
 	 *

src/charon/sa/transactions/create_child_sa.c

@@ -94,6 +94,11 @@ struct private_create_child_sa_t {
 	 */
 	chunk_t nonce_r;
 	
+	/**
+	 * lower of the nonces of a simultaneus rekeying request
+	 */
+	chunk_t nonce_s;
+	
 	/**
 	 * Negotiated traffic selectors for initiator
 	 */
@@ -172,12 +177,22 @@ static status_t get_request(private_create_child_sa_t *this, message_t **result)
 	host_t *me, *other;
 	
 	/* check if we are not already rekeying */
-	if (this->rekeyed_sa &&
+	if (this->rekeyed_sa)
-		this->rekeyed_sa->get_rekeying_transaction(this->rekeyed_sa))
 	{
-		this->logger->log(this->logger, ERROR,
+		switch (this->rekeyed_sa->get_state(this->rekeyed_sa))
-						  "rekeying a CHILD_SA which is already rekeying, aborted");
+		{
-		return FAILED;
+			case CHILD_REKEYING:
+				this->logger->log(this->logger, ERROR,
+								  "rekeying a CHILD_SA which is already rekeying, aborted");
+				return FAILED;
+			case CHILD_DELETING:
+				this->logger->log(this->logger, ERROR,
+								  "rekeying a CHILD_SA which is deleting, aborted");
+				return FAILED;
+			default:
+				break;
+		}
+		this->rekeyed_sa->set_state(this->rekeyed_sa, CHILD_REKEYING);
 	}
 	
 	/* check if we already have built a message (retransmission) */
@@ -198,7 +213,6 @@ static status_t get_request(private_create_child_sa_t *this, message_t **result)
 	request->set_destination(request, other->clone(other));
 	request->set_exchange_type(request, CREATE_CHILD_SA);
 	request->set_request(request, TRUE);
-	request->set_message_id(request, this->message_id);
 	request->set_ike_sa_id(request, this->ike_sa->get_id(this->ike_sa));
 	*result = request;
 	this->message = request;
@@ -275,6 +289,9 @@ static status_t get_request(private_create_child_sa_t *this, message_t **result)
 		this->rekeyed_sa->set_rekeying_transaction(this->rekeyed_sa, &this->public);
 	}
 	
+	this->message_id = this->ike_sa->get_next_message_id(this->ike_sa);
+	request->set_message_id(request, this->message_id);
+	
 	return SUCCESS;
 }
 
@@ -463,6 +480,7 @@ static status_t get_response(private_create_child_sa_t *this, message_t *request
 	me = this->connection->get_my_host(this->connection);
 	other = this->connection->get_other_host(this->connection);
 	this->policy = this->ike_sa->get_policy(this->ike_sa);
+	this->message_id = request->get_message_id(request);
 	
 	/* set up response */
 	response = message_create();
@@ -626,12 +644,29 @@ static status_t get_response(private_create_child_sa_t *this, message_t *request
 		ts_response = ts_payload_create_from_traffic_selectors(FALSE, this->tsr);
 		response->add_payload(response, (payload_t*)ts_response);
 	}
-	/* CHILD_SA successfully created. We set us as the rekeying transaction of
+	/* CHILD_SA successfully created. If another transaction is already rekeying
-	 * the rekeyed SA. If we already initiated rekeying of the same SA, we will detect
+	 * this SA, our lower nonce must be registered for a later nonce compare. */
-	 * this later in the conclude() call. */
 	if (this->rekeyed_sa)
 	{
-		this->rekeyed_sa->set_rekeying_transaction(this->rekeyed_sa, &this->public);
+		private_create_child_sa_t *other;
+		
+		other = this->rekeyed_sa->get_rekeying_transaction(this->rekeyed_sa);
+		if (other)
+		{
+			/* store our lower nonce in the simultaneus transaction, it 
+			 * will later compare it against his nonces when it calls conclude().
+			 */
+			if (memcmp(this->nonce_i.ptr, this->nonce_r.ptr,
+				min(this->nonce_i.len, this->nonce_r.len)) < 0)
+			{
+				other->nonce_s = chunk_clone(this->nonce_i);
+			}
+			else
+			{
+				other->nonce_s = chunk_clone(this->nonce_r);
+			}
+		}
+		this->rekeyed_sa->set_state(this->rekeyed_sa, CHILD_REKEYING);
 	}
 	return SUCCESS;
 }
@@ -763,20 +798,15 @@ static status_t conclude(private_create_child_sa_t *this, message_t *response,
 	 * If no simultaneous rekeying is going on, we just initiate the delete of
 	 * the superseded SA. */
 	if (this->rekeyed_sa)
-	{
+	{	
-		private_create_child_sa_t *other;
-		
-		other = (private_create_child_sa_t*)
-					this->rekeyed_sa->get_rekeying_transaction(this->rekeyed_sa);
-		
 		/* rekeying finished, update SA status */
 		this->rekeyed_sa->set_rekeying_transaction(this->rekeyed_sa, NULL);
 		
-		if (other != this)
+		if (this->nonce_s.ptr)
 		{	/* simlutaneous rekeying is going on, not so good */
-			chunk_t this_lowest, other_lowest;
+			chunk_t this_lowest;
 			
-			/* check if this has a lower nonce than other */
+			/* first get our lowest nonce */
 			if (memcmp(this->nonce_i.ptr, this->nonce_r.ptr, 
 				min(this->nonce_i.len, this->nonce_r.len)) < 0)
 			{
@@ -786,33 +816,24 @@ static status_t conclude(private_create_child_sa_t *this, message_t *response,
 			{
 				this_lowest = this->nonce_r;
 			}
-			if (memcmp(other->nonce_i.ptr, other->nonce_r.ptr, 
+			/* then compare against other lowest nonce */
-				min(other->nonce_i.len, other->nonce_r.len)) < 0)
+			if (memcmp(this_lowest.ptr, this->nonce_s.ptr, 
+				min(this_lowest.len, this->nonce_s.len)) < 0)
 			{
-				other_lowest = other->nonce_i;
-			}
-			else
-			{
-				other_lowest = other->nonce_r;
-			}
-			if (memcmp(this_lowest.ptr, other_lowest.ptr, 
-				min(this_lowest.len, other_lowest.len)) < 0)
-			{
-				this->logger->log(this->logger, ERROR,
-								  "detected simultaneous CHILD_SA rekeying, but ours is preferred");
-			}
-			else
-			{
-				
 				this->logger->log(this->logger, ERROR,
 								  "detected simultaneous CHILD_SA rekeying, deleting ours");
 				this->lost = TRUE;
 			}
+			else
+			{
+				this->logger->log(this->logger, ERROR,
+								  "detected simultaneous CHILD_SA rekeying, but ours is preferred");
+			}
 		}	
 		/* delete the old SA if we have won the rekeying nonce compare*/
 		if (!this->lost)
 		{
-			delete_child_sa = delete_child_sa_create(this->ike_sa, this->message_id + 1);
+			delete_child_sa = delete_child_sa_create(this->ike_sa);
 			delete_child_sa->set_child_sa(delete_child_sa, this->rekeyed_sa);
 			*next = (transaction_t*)delete_child_sa;
 		}
@@ -820,7 +841,7 @@ static status_t conclude(private_create_child_sa_t *this, message_t *response,
 	if (this->lost)
 	{
 		/* we have lost simlutaneous rekeying, delete the CHILD_SA we just have created */
-		delete_child_sa = delete_child_sa_create(this->ike_sa, this->message_id + 1);
+		delete_child_sa = delete_child_sa_create(this->ike_sa);
 		delete_child_sa->set_child_sa(delete_child_sa, new_child);
 		*next = (transaction_t*)delete_child_sa;
 	}
@@ -848,6 +869,7 @@ static void destroy(private_create_child_sa_t *this)
 	destroy_ts_list(this->tsr);
 	chunk_free(&this->nonce_i);
 	chunk_free(&this->nonce_r);
+	chunk_free(&this->nonce_s);
 	this->randomizer->destroy(this->randomizer);
 	free(this);
 }
@@ -855,7 +877,7 @@ static void destroy(private_create_child_sa_t *this)
 /*
  * Described in header.
  */
-create_child_sa_t *create_child_sa_create(ike_sa_t *ike_sa, u_int32_t message_id)
+create_child_sa_t *create_child_sa_create(ike_sa_t *ike_sa)
 {
 	private_create_child_sa_t *this = malloc_thing(private_create_child_sa_t);
 	
@@ -873,12 +895,13 @@ create_child_sa_t *create_child_sa_create(ike_sa_t *ike_sa, u_int32_t message_id
 	
 	/* private data */
 	this->ike_sa = ike_sa;
-	this->message_id = message_id;
+	this->message_id = 0;
 	this->message = NULL;
 	this->requested = 0;
 	this->rekey_spi = 0;
 	this->nonce_i = CHUNK_INITIALIZER;
 	this->nonce_r = CHUNK_INITIALIZER;
+	this->nonce_s = CHUNK_INITIALIZER;
 	this->child_sa = NULL;
 	this->rekeyed_sa = NULL;
 	this->lost = FALSE;

src/charon/sa/transactions/create_child_sa.h

@@ -95,11 +95,10 @@ struct create_child_sa_t {
  * @brief Create a new transaction which creates/rekeys CHILD_SAs.
  *
  * @param ike_sa		assigned IKE_SA
- * @param message_id	message ids used in this transaction
  * @return				created create_child_sa transaction
  *
  * @ingroup transactions
  */
-create_child_sa_t *create_child_sa_create(ike_sa_t *ike_sa, u_int32_t message_id);
+create_child_sa_t *create_child_sa_create(ike_sa_t *ike_sa);
 
 #endif /* CREATE_CHILD_SA_H_ */

src/charon/sa/transactions/dead_peer_detection.c

@@ -105,6 +105,7 @@ static status_t get_request(private_dead_peer_detection_t *this, message_t **res
 	request->set_destination(request, other->clone(other));
 	request->set_exchange_type(request, INFORMATIONAL);
 	request->set_request(request, TRUE);
+	this->message_id = this->ike_sa->get_next_message_id(this->ike_sa);
 	request->set_message_id(request, this->message_id);
 	request->set_ike_sa_id(request, this->ike_sa->get_id(this->ike_sa));
 	/* apply for caller */
@@ -135,6 +136,7 @@ static status_t get_response(private_dead_peer_detection_t *this, message_t *req
 	connection = this->ike_sa->get_connection(this->ike_sa);
 	me = connection->get_my_host(connection);
 	other = connection->get_other_host(connection);
+	this->message_id = request->get_message_id(request);
 	
 	/* set up response */
 	response = message_create();
@@ -175,7 +177,7 @@ static void destroy(private_dead_peer_detection_t *this)
 /*
  * Described in header.
  */
-dead_peer_detection_t *dead_peer_detection_create(ike_sa_t *ike_sa, u_int32_t message_id)
+dead_peer_detection_t *dead_peer_detection_create(ike_sa_t *ike_sa)
 {
 	private_dead_peer_detection_t *this = malloc_thing(private_dead_peer_detection_t);
 	
@@ -189,7 +191,7 @@ dead_peer_detection_t *dead_peer_detection_create(ike_sa_t *ike_sa, u_int32_t me
 	
 	/* private data */
 	this->ike_sa = ike_sa;
-	this->message_id = message_id;
+	this->message_id = 0;
 	this->message = NULL;
 	this->requested = 0;
 	this->logger = logger_manager->get_logger(logger_manager, IKE_SA);

src/charon/sa/transactions/dead_peer_detection.h

@@ -50,11 +50,10 @@ struct dead_peer_detection_t {
  * @brief Create a new transaction which detects dead peers.
  *
  * @param ike_sa		assigned IKE_SA
- * @param message_id	message ids used in this transaction
  * @return				created dead_peer_detection transaction
  *
  * @ingroup transactions
  */
-dead_peer_detection_t *dead_peer_detection_create(ike_sa_t *ike_sa, u_int32_t message_id);
+dead_peer_detection_t *dead_peer_detection_create(ike_sa_t *ike_sa);
 
 #endif /* DEAD_PEER_DETECTION_H_ */

src/charon/sa/transactions/delete_child_sa.c

@@ -120,6 +120,7 @@ static status_t get_request(private_delete_child_sa_t *this, message_t **result)
 	request->set_destination(request, other->clone(other));
 	request->set_exchange_type(request, INFORMATIONAL);
 	request->set_request(request, TRUE);
+	this->message_id = this->ike_sa->get_next_message_id(this->ike_sa);
 	request->set_message_id(request, this->message_id);
 	request->set_ike_sa_id(request, this->ike_sa->get_id(this->ike_sa));
 	*result = request;
@@ -141,6 +142,8 @@ static status_t get_request(private_delete_child_sa_t *this, message_t **result)
 		request->add_payload(request, (payload_t*)delete_payload);
 	}
 	
+	this->child_sa->set_state(this->child_sa, CHILD_DELETING);
+	
 	return SUCCESS;
 }
 
@@ -181,6 +184,8 @@ static status_t process_delete(private_delete_child_sa_t *this, delete_payload_t
 		{
 			create_child_sa_t *rekey;
 			
+			child_sa->set_state(child_sa, CHILD_DELETING);
+			
 			this->logger->log(this->logger, CONTROL,
 							  "received DELETE for %s CHILD_SA with SPI 0x%x, deleting",
 							  mapping_find(protocol_id_m, protocol), ntohl(spi));
@@ -192,11 +197,6 @@ static status_t process_delete(private_delete_child_sa_t *this, delete_payload_t
 				 * this means we have lost the nonce comparison, and the rekeying
 				 * will fail. We set a flag in the transaction for this special case.
 				 */
-				if (!response)
-				{	/* only whine as initiator */
-					this->logger->log(this->logger, CONTROL,
-									  "DELETE received while rekeying, rekeying cancelled");
-				}
 				rekey->cancel(rekey);
 			}
 			/* delete it, with inbound spi */
@@ -240,6 +240,7 @@ static status_t get_response(private_delete_child_sa_t *this, message_t *request
 	connection = this->ike_sa->get_connection(this->ike_sa);
 	me = connection->get_my_host(connection);
 	other = connection->get_other_host(connection);
+	this->message_id = request->get_message_id(request);
 	
 	/* set up response */
 	response = message_create();
@@ -344,7 +345,7 @@ static void destroy(private_delete_child_sa_t *this)
 /*
  * Described in header.
  */
-delete_child_sa_t *delete_child_sa_create(ike_sa_t *ike_sa, u_int32_t message_id)
+delete_child_sa_t *delete_child_sa_create(ike_sa_t *ike_sa)
 {
 	private_delete_child_sa_t *this = malloc_thing(private_delete_child_sa_t);
 	
@@ -361,7 +362,7 @@ delete_child_sa_t *delete_child_sa_create(ike_sa_t *ike_sa, u_int32_t message_id
 	
 	/* private data */
 	this->ike_sa = ike_sa;
-	this->message_id = message_id;
+	this->message_id = 0;
 	this->message = NULL;
 	this->requested = 0;
 	this->logger = logger_manager->get_logger(logger_manager, IKE_SA);

src/charon/sa/transactions/delete_child_sa.h

@@ -59,11 +59,10 @@ struct delete_child_sa_t {
  * @brief Create a new transaction which deletes a CHILD_SA.
  *
  * @param ike_sa		assigned IKE_SA
- * @param message_id	message ids used in this transaction
  * @return				created delete_child_sa transaction
  *
  * @ingroup transactions
  */
-delete_child_sa_t *delete_child_sa_create(ike_sa_t *ike_sa, u_int32_t message_id);
+delete_child_sa_t *delete_child_sa_create(ike_sa_t *ike_sa);
 
 #endif /* DELETE_CHILD_SA_H_ */

src/charon/sa/transactions/delete_ike_sa.c

@@ -107,6 +107,7 @@ static status_t get_request(private_delete_ike_sa_t *this, message_t **result)
 	request->set_destination(request, other->clone(other));
 	request->set_exchange_type(request, INFORMATIONAL);
 	request->set_request(request, TRUE);
+	this->message_id = this->ike_sa->get_next_message_id(this->ike_sa);
 	request->set_message_id(request, this->message_id);
 	request->set_ike_sa_id(request, this->ike_sa->get_id(this->ike_sa));
 	/* apply for caller */
@@ -118,7 +119,7 @@ static status_t get_request(private_delete_ike_sa_t *this, message_t **result)
 	request->add_payload(request, (payload_t*)delete_payload);
 	
 	/* transit to state SA_DELETING */
-	this->ike_sa->set_state(this->ike_sa, SA_DELETING);
+	this->ike_sa->set_state(this->ike_sa, IKE_DELETING);
 	
 	return SUCCESS;
 }
@@ -147,6 +148,7 @@ static status_t get_response(private_delete_ike_sa_t *this, message_t *request,
 	connection = this->ike_sa->get_connection(this->ike_sa);
 	me = connection->get_my_host(connection);
 	other = connection->get_other_host(connection);
+	this->message_id = request->get_message_id(request);
 	
 	/* set up response */
 	response = message_create();
@@ -204,13 +206,13 @@ static status_t get_response(private_delete_ike_sa_t *this, message_t *request,
 		this->logger->log(this->logger, CONTROL, 
 						  "received a weird DELETE request for IKE_SA, deleting anyway");
 	}
-	if (this->ike_sa->get_state(this->ike_sa) == SA_DELETING)
+	if (this->ike_sa->get_state(this->ike_sa) == IKE_DELETING)
 	{
 		/* if we are already deleting an IKE_SA, we do not destroy. We wait
 		 * until we get the response for our initiated delete. */
 		return SUCCESS;
 	}
-	this->ike_sa->set_state(this->ike_sa, SA_DELETING);
+	this->ike_sa->set_state(this->ike_sa, IKE_DELETING);
 	return DESTROY_ME;
 }
 
@@ -248,7 +250,7 @@ static void destroy(private_delete_ike_sa_t *this)
 /*
  * Described in header.
  */
-delete_ike_sa_t *delete_ike_sa_create(ike_sa_t *ike_sa, u_int32_t message_id)
+delete_ike_sa_t *delete_ike_sa_create(ike_sa_t *ike_sa)
 {
 	private_delete_ike_sa_t *this = malloc_thing(private_delete_ike_sa_t);
 	
@@ -262,7 +264,7 @@ delete_ike_sa_t *delete_ike_sa_create(ike_sa_t *ike_sa, u_int32_t message_id)
 	
 	/* private data */
 	this->ike_sa = ike_sa;
-	this->message_id = message_id;
+	this->message_id = 0;
 	this->message = NULL;
 	this->requested = 0;
 	this->logger = logger_manager->get_logger(logger_manager, IKE_SA);

src/charon/sa/transactions/delete_ike_sa.h

@@ -73,11 +73,10 @@ struct delete_ike_sa_t {
  * @brief Create a new transaction which deletes the IKE_SA.
  *
  * @param ike_sa		assigned IKE_SA
- * @param message_id	message ids used in this transaction
  * @return				created delete_ike_sa transaction
  *
  * @ingroup transactions
  */
-delete_ike_sa_t *delete_ike_sa_create(ike_sa_t *ike_sa, u_int32_t message_id);
+delete_ike_sa_t *delete_ike_sa_create(ike_sa_t *ike_sa);
 
 #endif /* DELETE_IKE_SA_H_ */

src/charon/sa/transactions/ike_auth.c

@@ -194,7 +194,6 @@ static status_t get_request(private_ike_auth_t *this, message_t **result)
 	request->set_destination(request, other->clone(other));
 	request->set_exchange_type(request, IKE_AUTH);
 	request->set_request(request, TRUE);
-	request->set_message_id(request, this->message_id);
 	request->set_ike_sa_id(request, this->ike_sa->get_id(this->ike_sa));
 	/* apply for caller */
 	*result = request;
@@ -295,6 +294,8 @@ static status_t get_request(private_ike_auth_t *this, message_t **result)
 		request->add_payload(request, (payload_t*)ts_payload);
 	}
 	
+	this->message_id = this->ike_sa->get_next_message_id(this->ike_sa);
+	request->set_message_id(request, this->message_id);
 	return SUCCESS;
 }
 
@@ -510,6 +511,7 @@ static status_t get_response(private_ike_auth_t *this, message_t *request,
 	this->connection = this->ike_sa->get_connection(this->ike_sa);
 	me = this->connection->get_my_host(this->connection);
 	other = this->connection->get_other_host(this->connection);
+	this->message_id = request->get_message_id(request);
 	
 	/* set up response */
 	response = message_create();
@@ -761,7 +763,7 @@ static status_t get_response(private_ike_auth_t *this, message_t *request,
 		response->add_payload(response, (payload_t*)ts_response);
 	}
 	/* set established state */
-	this->ike_sa->set_state(this->ike_sa, SA_ESTABLISHED);
+	this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
 	return SUCCESS;
 }
 
@@ -941,7 +943,7 @@ static status_t conclude(private_ike_auth_t *this, message_t *response,
 		}
 	}
 	/* set new state */
-	this->ike_sa->set_state(this->ike_sa, SA_ESTABLISHED);
+	this->ike_sa->set_state(this->ike_sa, IKE_ESTABLISHED);
 	return SUCCESS;
 }
 
@@ -974,7 +976,7 @@ static void destroy(private_ike_auth_t *this)
 /*
  * Described in header.
  */
-ike_auth_t *ike_auth_create(ike_sa_t *ike_sa, u_int32_t message_id)
+ike_auth_t *ike_auth_create(ike_sa_t *ike_sa)
 {
 	private_ike_auth_t *this = malloc_thing(private_ike_auth_t);
 
@@ -992,7 +994,7 @@ ike_auth_t *ike_auth_create(ike_sa_t *ike_sa, u_int32_t message_id)
 	
 	/* private data */
 	this->ike_sa = ike_sa;
-	this->message_id = message_id;
+	this->message_id = 0;
 	this->message = NULL;
 	this->requested = 0;
 	this->nonce_i = CHUNK_INITIALIZER;

src/charon/sa/transactions/ike_auth.h

@@ -76,11 +76,10 @@ struct ike_auth_t {
  * @brief Create a new transaction which processes IKE_AUTH exchanges.
  *
  * @param ike_sa		assigned IKE_SA
- * @param message_id	message ids used in this transaction
  * @return				created ike_auth transaction
  *
  * @ingroup transactions
  */
-ike_auth_t *ike_auth_create(ike_sa_t *ike_sa, u_int32_t message_id);
+ike_auth_t *ike_auth_create(ike_sa_t *ike_sa);
 
 #endif /* IKE_AUTH_H_ */

src/charon/sa/transactions/ike_sa_init.c

@@ -348,8 +348,7 @@ static status_t get_request(private_ike_sa_init_t *this, message_t **result)
 		request->add_payload(request, (payload_t*)notify);
 	}
 	
-	/* set new state */
+	this->ike_sa->set_state(this->ike_sa, IKE_CONNECTING);
-	this->ike_sa->set_state(this->ike_sa, SA_CONNECTING);
 	return SUCCESS;
 }
 
@@ -398,7 +397,7 @@ static status_t process_notifys(private_ike_sa_init_t *this, notify_payload_t *n
 								  "requested DH group not acceptable, aborting");
 				return DESTROY_ME;
 			}
-			retry = ike_sa_init_create(this->ike_sa, 0);
+			retry = ike_sa_init_create(this->ike_sa);
 			retry->use_dh_group(retry, dh_group);
 			*this->next = (transaction_t*)retry;
 			return FAILED;
@@ -489,6 +488,7 @@ static status_t get_response(private_ike_sa_init_t *this,
 	
 	me = request->get_destination(request);
 	other = request->get_source(request);
+	this->message_id = request->get_message_id(request);
 	
 	/* set up response */
 	response = message_create();
@@ -768,7 +768,7 @@ static status_t get_response(private_ike_sa_init_t *this,
 		response_chunk = response->get_packet_data(response);
 		
 		/* create next transaction, for which we except a message */
-		ike_auth = ike_auth_create(this->ike_sa, 1);
+		ike_auth = ike_auth_create(this->ike_sa);
 		ike_auth->set_nonces(ike_auth,
 							 chunk_clone(this->nonce_i),
 							 chunk_clone(this->nonce_r));
@@ -784,7 +784,7 @@ static status_t get_response(private_ike_sa_init_t *this,
 		charon->event_queue->add_relative(charon->event_queue, job, timeout);
 	}
 	/* set new state */
-	this->ike_sa->set_state(this->ike_sa, SA_CONNECTING);
+	this->ike_sa->set_state(this->ike_sa, IKE_CONNECTING);
 	return SUCCESS;
 }
 
@@ -999,7 +999,7 @@ static status_t conclude(private_ike_sa_init_t *this, message_t *response,
 		response_chunk = response->get_packet_data(response);
 		
 		/* create next transaction, for which we except a message */
-		ike_auth = ike_auth_create(this->ike_sa, this->message_id + 1);
+		ike_auth = ike_auth_create(this->ike_sa);
 		ike_auth->set_nonces(ike_auth,
 							 chunk_clone(this->nonce_i),
 							 chunk_clone(this->nonce_r));
@@ -1036,7 +1036,7 @@ static void destroy(private_ike_sa_init_t *this)
 /*
  * Described in header.
  */
-ike_sa_init_t *ike_sa_init_create(ike_sa_t *ike_sa, u_int32_t message_id)
+ike_sa_init_t *ike_sa_init_create(ike_sa_t *ike_sa)
 {
 	private_ike_sa_init_t *this = malloc_thing(private_ike_sa_init_t);
 
@@ -1053,7 +1053,7 @@ ike_sa_init_t *ike_sa_init_create(ike_sa_t *ike_sa, u_int32_t message_id)
 	
 	/* private data */
 	this->ike_sa = ike_sa;
-	this->message_id = message_id;
+	this->message_id = 0;
 	this->message = NULL;
 	this->requested = 0;
 	this->diffie_hellman = NULL;

src/charon/sa/transactions/ike_sa_init.h

@@ -63,11 +63,10 @@ struct ike_sa_init_t {
  * @brief Create a new transaction which processes IKE_SA_INIT exchanges.
  *
  * @param ike_sa		assigned IKE_SA
- * @param message_id	message ids used in this transaction
  * @return				created ike_sa_init transaction
  *
  * @ingroup transactions
  */
-ike_sa_init_t *ike_sa_init_create(ike_sa_t *ike_sa, u_int32_t message_id);
+ike_sa_init_t *ike_sa_init_create(ike_sa_t *ike_sa);
 
 #endif /* IKE_SA_INIT_H_ */

src/charon/sa/transactions/transaction.c

@@ -46,21 +46,19 @@ transaction_t *transaction_create(ike_sa_t *ike_sa, message_t *request)
 	payload_t *current;
 	notify_payload_t *notify;
 	transaction_t *transaction = NULL;
-	u_int32_t message_id;
 	
 	if (!request->get_request(request))
 	{
 		return NULL;
 	}
-	message_id = request->get_message_id(request);
 	
 	switch (request->get_exchange_type(request))
 	{
 		case IKE_SA_INIT:
 		{
-			if (ike_sa->get_state(ike_sa) == SA_CREATED)
+			if (ike_sa->get_state(ike_sa) == IKE_CREATED)
 			{
-				transaction = (transaction_t*)ike_sa_init_create(ike_sa, message_id);
+				transaction = (transaction_t*)ike_sa_init_create(ike_sa);
 			}
 			break;
 		}
@@ -72,7 +70,7 @@ transaction_t *transaction_create(ike_sa_t *ike_sa, message_t *request)
 		}
 		case CREATE_CHILD_SA:
 		{
-			if (ike_sa->get_state(ike_sa) != SA_ESTABLISHED)
+			if (ike_sa->get_state(ike_sa) != IKE_ESTABLISHED)
 			{
 				break;
 			}
@@ -93,14 +91,14 @@ transaction_t *transaction_create(ike_sa_t *ike_sa, message_t *request)
 				switch (notify->get_protocol_id(notify))
 				{
 					case PROTO_IKE:
-						/* TODO: transaction = rekey_ike_sa_create(ike_sa, message_id); */
+						/* TODO: transaction = rekey_ike_sa_create(ike_sa); */
 						break;
 					case PROTO_AH:
 					case PROTO_ESP:
 						/* we do not handle rekeying of CHILD_SAs in a special 
 						 * transaction, as the procedure is nearly equal 
 						 * to create a new CHILD_SA. */
-						transaction = (transaction_t*)create_child_sa_create(ike_sa, message_id);
+						transaction = (transaction_t*)create_child_sa_create(ike_sa);
 						break;
 					default:
 						break;
@@ -116,13 +114,13 @@ transaction_t *transaction_create(ike_sa_t *ike_sa, message_t *request)
 				/* we have not found a REKEY_SA notify for IKE. This means
 				 * we create a new CHILD_SA, or rekey an existing one.
 				 * Both cases are handled with the create_child_sa transaction. */
-				transaction = (transaction_t*)create_child_sa_create(ike_sa, message_id);
+				transaction = (transaction_t*)create_child_sa_create(ike_sa);
 			}
 			break;
 		}
 		case INFORMATIONAL:
 		{
-			if (ike_sa->get_state(ike_sa) == SA_CREATED)
+			if (ike_sa->get_state(ike_sa) == IKE_CREATED)
 			{
 				break;
 			}
@@ -142,12 +140,12 @@ transaction_t *transaction_create(ike_sa_t *ike_sa, message_t *request)
 						{
 							case PROTO_IKE:
 								transaction = (transaction_t*)
-										delete_ike_sa_create(ike_sa, message_id);
+										delete_ike_sa_create(ike_sa);
 								break;
 							case PROTO_AH:
 							case PROTO_ESP:
 								transaction = (transaction_t*)
-										delete_child_sa_create(ike_sa, message_id);
+										delete_child_sa_create(ike_sa);
 								break;
 							default:
 								break;
@@ -168,7 +166,7 @@ transaction_t *transaction_create(ike_sa_t *ike_sa, message_t *request)
 			if (payload_count == 0)
 			{
 				transaction = (transaction_t*)
-						dead_peer_detection_create(ike_sa, message_id);
+						dead_peer_detection_create(ike_sa);
 			}
 			break;
 		}