- reworked usage of IDs in various states

- using ID_ANY for any, not NULL as before
- initiator sends IDr payload in IKE_AUTH when ID unique

10 files changed, 125 insertions, 54 deletions

diff --git a/Source/charon/config/connections/connection.c b/Source/charon/config/connections/connection.c
index 2ce544cc9..74e6762b4 100644
--- a/Source/charon/config/connections/connection.c
+++ b/Source/charon/config/connections/connection.c
@@ -111,6 +111,24 @@ static identification_t *get_other_id(private_connection_t *this)
 }
 
 /**
+ * Implementation of connection_t.update_my_id
+ */
+static void update_my_id(private_connection_t *this, identification_t *my_id)
+{
+	this->my_id->destroy(this->my_id);
+	this->my_id = my_id;
+}
+
+/**
+ * Implementation of connection_t.update_other_id
+ */
+static void update_other_id(private_connection_t *this, identification_t *other_id)
+{
+	this->other_id->destroy(this->other_id);
+	this->other_id = other_id;
+}
+
+/**
  * Implementation of connection_t.get_my_host.
  */
 static host_t * get_my_host (private_connection_t *this)
@@ -305,6 +323,7 @@ static void destroy (private_connection_t *this)
 	this->other_host->destroy(this->other_host);
 	this->my_id->destroy(this->my_id);
 	this->other_id->destroy(this->other_id);
+	free(this->name);
 	free(this);
 }
 
@@ -322,6 +341,8 @@ connection_t * connection_create(char *name, host_t *my_host, host_t *other_host
 	this->public.get_my_host = (host_t*(*)(connection_t*))get_my_host;
 	this->public.update_my_host = (void(*)(connection_t*,host_t*))update_my_host;
 	this->public.update_other_host = (void(*)(connection_t*,host_t*))update_other_host;
+	this->public.update_my_id = (void(*)(connection_t*,identification_t*))update_my_id;
+	this->public.update_other_id = (void(*)(connection_t*,identification_t*))update_other_id;
 	this->public.get_other_host = (host_t*(*)(connection_t*))get_other_host;
 	this->public.get_proposals = (linked_list_t*(*)(connection_t*))get_proposals;
 	this->public.select_proposal = (proposal_t*(*)(connection_t*,linked_list_t*))select_proposal;
diff --git a/Source/charon/config/connections/connection.h b/Source/charon/config/connections/connection.h
index fb960d1a0..2cb3c20b8 100644
--- a/Source/charon/config/connections/connection.h
+++ b/Source/charon/config/connections/connection.h
@@ -143,6 +143,32 @@ struct connection_t {
 	 * @param my_host	new host to set as other_host
 	 */
 	void (*update_other_host) (connection_t *this, host_t *other_host);
+
+	/**
+	 * @brief Update own ID.
+	 * 
+	 * It may be necessary to uptdate own ID, as it 
+	 * is set to %any or to e.g. *@strongswan.org in 
+	 * some cases.
+	 * Old ID is destroyed, new one NOT cloned.
+	 * 
+	 * @param this		calling object
+	 * @param my_id		new ID to set as my_id
+	 */
+	void (*update_my_id) (connection_t *this, identification_t *my_id);
+
+	/**
+	 * @brief Update others ID.
+	 * 
+	 * It may be necessary to uptdate others ID, as it 
+	 * is set to %any or to e.g. *@strongswan.org in 
+	 * some cases.
+	 * Old ID is destroyed, new one NOT cloned.
+	 * 
+	 * @param this		calling object
+	 * @param other_id	new ID to set as other_id
+	 */
+	void (*update_other_id) (connection_t *this, identification_t *other_id);
 	
 	/**
 	 * @brief Returns a list of all supported proposals.
diff --git a/Source/charon/config/policies/local_policy_store.c b/Source/charon/config/policies/local_policy_store.c
index 7dcdf1728..ae02357ea 100644
--- a/Source/charon/config/policies/local_policy_store.c
+++ b/Source/charon/config/policies/local_policy_store.c
@@ -66,7 +66,7 @@ static policy_t *get_policy(private_local_policy_store_t *this, identification_t
 	iterator_t *iterator;
 	policy_t *current, *found = NULL;
 	
-	this->logger->log(this->logger, CONTROL|LEVEL0, "Looking for policy for IDs %s - %s",
+	this->logger->log(this->logger, CONTROL|LEVEL1, "Looking for policy for IDs %s - %s",
 					  my_id ? my_id->get_string(my_id) : "%any",
 					  other_id->get_string(other_id));
 	iterator = this->policies->create_iterator(this->policies, TRUE);
@@ -76,7 +76,7 @@ static policy_t *get_policy(private_local_policy_store_t *this, identification_t
 		identification_t *config_my_id = current->get_my_id(current);
 		identification_t *config_other_id = current->get_other_id(current);
 		
-		this->logger->log(this->logger, CONTROL|LEVEL0, "Found one for %s - %s",
+		this->logger->log(this->logger, CONTROL|LEVEL2, "Found one for %s - %s",
 						  config_my_id->get_string(config_my_id),
 						  config_other_id->get_string(config_other_id));
 		
@@ -84,11 +84,6 @@ static policy_t *get_policy(private_local_policy_store_t *this, identification_t
 		if (other_id->belongs_to(other_id, config_other_id))
 		{
 			/* get it if my_id not specified */
-			if (my_id == NULL)
-			{
-				found = current->clone(current);
-				break;
-			}
 			if (my_id->belongs_to(my_id, config_my_id))
 			{
 				found = current->clone(current);
@@ -101,10 +96,7 @@ static policy_t *get_policy(private_local_policy_store_t *this, identification_t
 	/* apply IDs as they are requsted, since they may be configured as %any or such */
 	if (found)
 	{
-		if (my_id)
-		{
-			found->update_my_id(found, my_id->clone(my_id));
-		}
+		found->update_my_id(found, my_id->clone(my_id));
 		found->update_other_id(found, other_id->clone(other_id));
 	}
 	return found;
diff --git a/Source/charon/config/policies/policy.h b/Source/charon/config/policies/policy.h
index 5a0823758..78cda1e8b 100644
--- a/Source/charon/config/policies/policy.h
+++ b/Source/charon/config/policies/policy.h
@@ -79,7 +79,7 @@ struct policy_t {
 	void (*update_my_id) (policy_t *this, identification_t *my_id);
 
 	/**
-	 * @brief Update others id.
+	 * @brief Update others ID.
 	 * 
 	 * It may be necessary to uptdate others ID, as it 
 	 * is set to %any or to e.g. *@strongswan.org in 
diff --git a/Source/charon/sa/child_sa.c b/Source/charon/sa/child_sa.c
index 8871b73a1..a678ea9b8 100644
--- a/Source/charon/sa/child_sa.c
+++ b/Source/charon/sa/child_sa.c
@@ -479,7 +479,7 @@ static void log_status(private_child_sa_t *this, logger_t *logger, char* name)
 	{
 		logger = this->logger;
 	}
-	logger->log(logger, CONTROL|LEVEL1, "\"%s\":   protected with ESP (%x/%x), AH (%x,%x):",
+	logger->log(logger, CONTROL|LEVEL1, "\"%s\":   protected with ESP (0x%x/0x%x), AH (0x%x,0x%x):",
 				name,
 				htonl(this->my_esp_spi), htonl(this->other_esp_spi), 
 				htonl(this->my_ah_spi), htonl(this->other_ah_spi));
diff --git a/Source/charon/sa/ike_sa.c b/Source/charon/sa/ike_sa.c
index 99531d75e..6322eb8e9 100644
--- a/Source/charon/sa/ike_sa.c
+++ b/Source/charon/sa/ike_sa.c
@@ -1007,7 +1007,7 @@ static void log_status(private_ike_sa_t *this, logger_t *logger, char *name)
 	{
 		logger = this->logger;
 	}
-	logger->log(logger, CONTROL|LEVEL1, "\"%s\": IKE_SA in state %s, SPIs: %llx %llx",
+	logger->log(logger, CONTROL|LEVEL1, "\"%s\": IKE_SA in state %s, SPIs: 0x%.16llx 0x%.16llx",
 				name,
 				mapping_find(ike_sa_state_m, this->current_state->get_state(this->current_state)),
 				this->ike_sa_id->get_initiator_spi(this->ike_sa_id),
diff --git a/Source/charon/sa/states/ike_auth_requested.c b/Source/charon/sa/states/ike_auth_requested.c
index 00c38a887..3d49f440f 100644
--- a/Source/charon/sa/states/ike_auth_requested.c
+++ b/Source/charon/sa/states/ike_auth_requested.c
@@ -373,26 +373,26 @@ static status_t process_message(private_ike_auth_requested_t *this, message_t *i
 static status_t process_idr_payload(private_ike_auth_requested_t *this, id_payload_t *idr_payload)
 {
 	identification_t *other_id, *configured_other_id;
+	connection_t *connection;
 	
 	other_id = idr_payload->get_identification(idr_payload);
-
 	configured_other_id = this->policy->get_other_id(this->policy);
-	if (configured_other_id)
+	
+	this->logger->log(this->logger, CONTROL|LEVEL1, "configured ID: %s, ID of responder: %s",
+						configured_other_id->get_string(configured_other_id),
+						other_id->get_string(other_id));
+	
+	if (!other_id->belongs_to(other_id, configured_other_id))
 	{
-		this->logger->log(this->logger, CONTROL|LEVEL1, "configured ID: %s, ID of responder: %s",
-							configured_other_id->get_string(configured_other_id),
-							other_id->get_string(other_id));
-
-		if (!other_id->equals(other_id, configured_other_id))
-		{
-			other_id->destroy(other_id);
-			this->logger->log(this->logger, AUDIT, "IKE_AUTH reply contained a not requested ID. Deleting IKE_SA");
-			return DELETE_ME;	
-		}
+		other_id->destroy(other_id);
+		this->logger->log(this->logger, AUDIT, "IKE_AUTH reply contained a not acceptable ID. Deleting IKE_SA");
+		return DELETE_ME;
 	}
 	
-	other_id->destroy(other_id);
-	/* TODO do we have to store other_id somewhere ? */
+	connection = this->ike_sa->get_connection(this->ike_sa);
+	connection->update_other_id(connection, other_id->clone(other_id));
+	
+	this->policy->update_other_id(this->policy, other_id);
 	return SUCCESS;
 }
 
diff --git a/Source/charon/sa/states/ike_sa_init_requested.c b/Source/charon/sa/states/ike_sa_init_requested.c
index e3769303c..311cdf0a0 100644
--- a/Source/charon/sa/states/ike_sa_init_requested.c
+++ b/Source/charon/sa/states/ike_sa_init_requested.c
@@ -135,6 +135,19 @@ struct private_ike_sa_init_requested_t {
 	status_t (*build_id_payload) (private_ike_sa_init_requested_t *this,id_payload_t **id_payload, message_t *response);
 	
 	/**
+	 * Build IDr payload for IKE_AUTH request.
+	 * 
+	 * Only built when the ID of the responder contains no wildcards.
+	 * 
+	 * @param this				calling object
+	 * @param response			created payload will be added to this message_t object
+	 * @return
+	 * 							- SUCCESS
+	 * 							- FAILED
+	 */
+	status_t (*build_idr_payload) (private_ike_sa_init_requested_t *this, message_t *response);
+	
+	/**
 	 * Build AUTH payload for IKE_AUTH request.
 	 * 
 	 * @param this				calling object
@@ -351,13 +364,19 @@ static status_t process_message(private_ike_sa_init_requested_t *this, message_t
 	/*  build empty message */
 	this->ike_sa->build_message(this->ike_sa, IKE_AUTH, TRUE, &request);
 	
-	status = this->build_id_payload(this, &id_payload,request);
+	status = this->build_id_payload(this, &id_payload, request);
+	if (status != SUCCESS)
+	{
+		request->destroy(request);
+		return status;
+	}	
+	status = this->build_idr_payload(this, request);
 	if (status != SUCCESS)
 	{
 		request->destroy(request);
 		return status;
 	}
-	status = this->build_auth_payload(this,(id_payload_t *) id_payload, request);
+	status = this->build_auth_payload(this, (id_payload_t*)id_payload, request);
 	if (status != SUCCESS)
 	{
 		request->destroy(request);
@@ -477,9 +496,8 @@ static status_t build_id_payload (private_ike_sa_init_requested_t *this,id_paylo
 	identification_t *identification;
 	
 	policy = this->ike_sa->get_policy(this->ike_sa);
-	/* identification_t object gets NOT cloned here */
 	identification = policy->get_my_id(policy);
-	new_id_payload = id_payload_create_from_identification(TRUE,identification);
+	new_id_payload = id_payload_create_from_identification(TRUE, identification);
 	
 	this->logger->log(this->logger, CONTROL|LEVEL2, "Add ID payload to message");
 	request->add_payload(request,(payload_t *) new_id_payload);
@@ -490,6 +508,27 @@ static status_t build_id_payload (private_ike_sa_init_requested_t *this,id_paylo
 }
 
 /**
+ * Implementation of private_ike_sa_init_requested_t.build_idr_payload.
+ */
+static status_t build_idr_payload (private_ike_sa_init_requested_t *this, message_t *request)
+{
+	policy_t *policy;
+	id_payload_t *idr_payload;
+	identification_t *identification;
+	
+	policy = this->ike_sa->get_policy(this->ike_sa);
+	identification = policy->get_other_id(policy);
+	if (!identification->contains_wildcards(identification))
+	{
+		idr_payload = id_payload_create_from_identification(FALSE, identification);
+	
+		this->logger->log(this->logger, CONTROL|LEVEL2, "Add IDr payload to message");
+		request->add_payload(request,(payload_t *) idr_payload);
+	}
+	return SUCCESS;
+}
+
+/**
  * Implementation of private_ike_sa_init_requested_t.build_auth_payload.
  */
 static status_t build_auth_payload (private_ike_sa_init_requested_t *this, id_payload_t *my_id_payload, message_t *request)
@@ -741,6 +780,7 @@ ike_sa_init_requested_t *ike_sa_init_requested_create(protected_ike_sa_t *ike_sa
 	this->build_tsi_payload = build_tsi_payload;
 	this->build_tsr_payload = build_tsr_payload;
 	this->build_id_payload = build_id_payload;
+	this->build_idr_payload = build_idr_payload;
 	this->build_sa_payload = build_sa_payload;
 	this->process_notify_payload = process_notify_payload;
 	
diff --git a/Source/charon/sa/states/ike_sa_init_responded.c b/Source/charon/sa/states/ike_sa_init_responded.c
index 54c0cc26b..e40b0cf22 100644
--- a/Source/charon/sa/states/ike_sa_init_responded.c
+++ b/Source/charon/sa/states/ike_sa_init_responded.c
@@ -382,39 +382,31 @@ static status_t build_idr_payload(private_ike_sa_init_responded_t *this, id_payl
 	connection_t *connection;
 	id_payload_t *idr_response;
 	
+	connection = this->ike_sa->get_connection(this->ike_sa);
+	
+	/* update adresses, as connection may contain wildcards, or wrong IDs */
 	other_id = request_idi->get_identification(request_idi);
 	if (request_idr)
 	{
 		my_id = request_idr->get_identification(request_idr);
+		connection->update_my_id(connection, my_id);
+	}
+	else
+	{
+		my_id = connection->get_my_id(connection);
 	}
+	connection->update_other_id(connection, other_id);
 
 	/* build new sa config */
-	connection = this->ike_sa->get_connection(this->ike_sa);
 	this->policy = charon->policies->get_policy(charon->policies, my_id, other_id);
 	if (this->policy == NULL)
-	{	
-		if (my_id)
-		{
-			this->logger->log(this->logger, AUDIT, "We don't have a policy for IDs %s - %s. Deleting IKE_SA", 
-							other_id->get_string(other_id),my_id->get_string(my_id));
-			my_id->destroy(my_id);	
-		}
-		else
-		{
-			this->logger->log(this->logger, AUDIT, "We don't have a policy for remote ID %s. Deleting IKE_SA", 
-							other_id->get_string(other_id));
-		}
-		other_id->destroy(other_id);
-		return DELETE_ME;
-	}
-	
-	if (my_id)
 	{
-		my_id->destroy(my_id);
+		this->logger->log(this->logger, AUDIT, "We don't have a policy for IDs %s - %s. Deleting IKE_SA", 
+						  my_id->get_string(my_id), other_id->get_string(other_id));
+		return DELETE_ME;
 	}
-	other_id->destroy(other_id);
 	
-	/* get my id, if not requested */
+	/* get my id from policy, which must contain a fully qualified valid id */
 	my_id = this->policy->get_my_id(this->policy);
 	
 	/* update others traffic selectors with actually used address */
diff --git a/Source/charon/threads/stroke_interface.c b/Source/charon/threads/stroke_interface.c
index 3078c03c6..ef5d5f1f6 100755
--- a/Source/charon/threads/stroke_interface.c
+++ b/Source/charon/threads/stroke_interface.c
@@ -312,7 +312,7 @@ static void stroke_initiate(private_stroke_t *this, stroke_msg_t *msg)
 	}
 	else
 	{
-		job = initiate_ike_sa_job_create(connection->clone(connection));
+		job = initiate_ike_sa_job_create(connection);
 		charon->job_queue->add(charon->job_queue, (job_t*)job);
 	}
 }