fixed double free bug

10 files changed, 96 insertions, 24 deletions

diff --git a/src/charon/config/policies/local_policy_store.c b/src/charon/config/policies/local_policy_store.c
index 7eef382f0..54c01630b 100644
--- a/src/charon/config/policies/local_policy_store.c
+++ b/src/charon/config/policies/local_policy_store.c
@@ -127,10 +127,11 @@ static policy_t *get_policy(private_local_policy_store_t *this,
 			prio_t prio = PRIO_UNDEFINED;
 
 			/* wildcard match for other_id */
-			if (other_id->matches(other_id, candidate_other_id, &wildcards))
+			if (!other_id->matches(other_id, candidate_other_id, &wildcards))
 			{
-				prio = PRIO_ID_MATCH - wildcards;
+				continue;
 			}
+			prio = PRIO_ID_MATCH - wildcards;
 			
 			/* only accept if traffic selectors match */
 			if (!contains_traffic_selectors(candidate, TRUE, my_ts, my_host) ||
diff --git a/src/charon/config/policies/policy.c b/src/charon/config/policies/policy.c
index 3ff0e32ea..b853a5a1e 100644
--- a/src/charon/config/policies/policy.c
+++ b/src/charon/config/policies/policy.c
@@ -268,8 +268,8 @@ static linked_list_t *select_traffic_selectors(private_policy_t *this,
 											   linked_list_t *supplied,
 											   host_t *host)
 {
-	iterator_t *supplied_iter, *stored_iter;
-	traffic_selector_t *supplied_ts, *stored_ts, *selected_ts;
+	iterator_t *supplied_iter, *stored_iter, *i1, *i2;
+	traffic_selector_t *supplied_ts, *stored_ts, *selected_ts, *ts1, *ts2;
 	linked_list_t *selected = linked_list_create();
 	
 	DBG2(DBG_CFG, "selecting traffic selectors");
@@ -310,6 +310,26 @@ static linked_list_t *select_traffic_selectors(private_policy_t *this,
 	stored_iter->destroy(stored_iter);
 	supplied_iter->destroy(supplied_iter);
 	
+	/* remove any redundant traffic selectors in the list */
+	i1 = selected->create_iterator(selected, TRUE);
+	i2 = selected->create_iterator(selected, TRUE);
+	while (i1->iterate(i1, (void**)&ts1))
+	{
+		while (i2->iterate(i2, (void**)&ts2))
+		{
+			if (ts1 != ts2 && ts2->is_contained_in(ts2, ts1))
+			{
+				i2->remove(i2);
+				ts2->destroy(ts2);
+				i1->reset(i1);
+				break;
+			}
+		}
+	}
+	i1->destroy(i1);
+	i2->destroy(i2);
+	
+	
 	return selected;
 }
 
diff --git a/src/charon/config/proposal.c b/src/charon/config/proposal.c
index 0faef3dad..5aa4ac053 100644
--- a/src/charon/config/proposal.c
+++ b/src/charon/config/proposal.c
@@ -24,6 +24,7 @@
 
 #include "proposal.h"
 
+#include <daemon.h>
 #include <utils/linked_list.h>
 #include <utils/identification.h>
 #include <utils/lexparser.h>
@@ -221,6 +222,9 @@ static bool select_algo(linked_list_t *first, linked_list_t *second, bool *add,
 		second_iter->reset(second_iter);
 		while (second_iter->iterate(second_iter, (void**)&second_alg))
 		{
+			DBG2(DBG_CFG, "comparing algo %d - %d, keylen %d - %d", 
+				 first_alg->algorithm, second_alg->algorithm,
+				 first_alg->key_size, second_alg->key_size);
 			if (first_alg->algorithm == second_alg->algorithm &&
 				first_alg->key_size == second_alg->key_size)
 			{
@@ -250,9 +254,12 @@ static proposal_t *select_proposal(private_proposal_t *this, private_proposal_t
 	size_t key_size;
 	bool add;
 	
+	DBG2(DBG_CFG, "selecting proposal:");
+	
 	/* check protocol */
 	if (this->protocol != other->protocol)
 	{
+		DBG2(DBG_CFG, "  protocol mismatch, skipping");
 		return NULL;
 	}
 	
@@ -269,6 +276,8 @@ static proposal_t *select_proposal(private_proposal_t *this, private_proposal_t
 	else
 	{
 		selected->destroy(selected);
+		DBG2(DBG_CFG, "  no acceptable ENCRYPTION_ALGORITHM found contained %d - %d, skipping",
+		this->encryption_algos->get_count(this->encryption_algos), other->encryption_algos->get_count(other->encryption_algos));
 		return NULL;
 	}
 	/* select integrity algorithm */
@@ -282,6 +291,7 @@ static proposal_t *select_proposal(private_proposal_t *this, private_proposal_t
 	else
 	{
 		selected->destroy(selected);
+		DBG2(DBG_CFG, "  no acceptable INTEGRITY_ALGORITHM found, skipping");
 		return NULL;
 	}
 	/* select prf algorithm */
@@ -295,6 +305,7 @@ static proposal_t *select_proposal(private_proposal_t *this, private_proposal_t
 	else
 	{
 		selected->destroy(selected);
+		DBG2(DBG_CFG, "  no acceptable PSEUDO_RANDOM_FUNCTION found, skipping");
 		return NULL;
 	}
 	/* select a DH-group */
@@ -308,6 +319,7 @@ static proposal_t *select_proposal(private_proposal_t *this, private_proposal_t
 	else
 	{
 		selected->destroy(selected);
+		DBG2(DBG_CFG, "  no acceptable DIFFIE_HELLMAN_GROUP found, skipping");
 		return NULL;
 	}
 	/* select if we use ESNs */
@@ -321,8 +333,10 @@ static proposal_t *select_proposal(private_proposal_t *this, private_proposal_t
 	else
 	{
 		selected->destroy(selected);
+		DBG2(DBG_CFG, "  no acceptable EXTENDED_SEQUENCE_NUMBERS found, skipping");
 		return NULL;
 	}
+	DBG2(DBG_CFG, "  proposal matches");
 	
 	/* apply SPI from "other" */
 	selected->set_spi(selected, other->spi);
@@ -443,6 +457,10 @@ static status_t add_string_algo(private_proposal_t *this, chunk_t alg)
 			add_algorithm(this, PSEUDO_RANDOM_FUNCTION, PRF_HMAC_MD5, 0);
 		}
 	}
+	else if (strncmp(alg.ptr, "modp768", alg.len) == 0)
+	{
+		add_algorithm(this, DIFFIE_HELLMAN_GROUP, MODP_768_BIT, 0);
+	}
 	else if (strncmp(alg.ptr, "modp1024", alg.len) == 0)
 	{
 		add_algorithm(this, DIFFIE_HELLMAN_GROUP, MODP_1024_BIT, 0);
diff --git a/src/charon/config/traffic_selector.c b/src/charon/config/traffic_selector.c
index b66b77f4a..cee45c287 100644
--- a/src/charon/config/traffic_selector.c
+++ b/src/charon/config/traffic_selector.c
@@ -504,6 +504,28 @@ static void set_address(private_traffic_selector_t *this, host_t *host)
 }
 
 /**
+ * Implements traffic_selector_t.is_contained_in.
+ */
+static bool is_contained_in(private_traffic_selector_t *this,
+							private_traffic_selector_t *other)
+{
+	private_traffic_selector_t *subset;
+	bool contained_in = FALSE;
+	
+	subset = (private_traffic_selector_t*)get_subset(this, other);
+	
+	if (subset)
+	{
+		if (equals(subset, this))
+		{
+			contained_in = TRUE;
+		}
+		free(subset);
+	}
+	return contained_in;	
+}
+
+/**
  * Implements traffic_selector_t.includes.
  */
 static bool includes(private_traffic_selector_t *this, host_t *host)
@@ -754,6 +776,7 @@ static private_traffic_selector_t *traffic_selector_create(u_int8_t protocol,
 	this->public.get_type = (ts_type_t(*)(traffic_selector_t*))get_type;
 	this->public.get_protocol = (u_int8_t(*)(traffic_selector_t*))get_protocol;
 	this->public.is_host = (bool(*)(traffic_selector_t*,host_t*))is_host;
+	this->public.is_contained_in = (bool(*)(traffic_selector_t*,traffic_selector_t*))is_contained_in;
 	this->public.includes = (bool(*)(traffic_selector_t*,host_t*))includes;
 	this->public.set_address = (void(*)(traffic_selector_t*,host_t*))set_address;
 	this->public.clone = (traffic_selector_t*(*)(traffic_selector_t*))clone_;
diff --git a/src/charon/config/traffic_selector.h b/src/charon/config/traffic_selector.h
index 33152b008..0e798fc6a 100644
--- a/src/charon/config/traffic_selector.h
+++ b/src/charon/config/traffic_selector.h
@@ -192,6 +192,17 @@ struct traffic_selector_t {
 	 * @return 			pointer to a string.
 	 */
 	bool (*equals) (traffic_selector_t *this, traffic_selector_t *other);
+	
+	/**
+	 * @brief Check if a traffic selector is contained completly in another.
+	 *
+	 * contains() allows to check if multiple traffic selectors are redundant.
+	 *
+	 * @param this		ts that is contained in another
+	 * @param other		ts that contains this
+	 * @return			TRUE if other contains this completly, FALSE otherwise
+	 */
+	bool (*is_contained_in) (traffic_selector_t *this, traffic_selector_t *other);
 
 	/**
 	 * @brief Check if a specific host is included in the address range of 
diff --git a/src/charon/encoding/payloads/sa_payload.c b/src/charon/encoding/payloads/sa_payload.c
index 7ae0f9532..e264b2123 100644
--- a/src/charon/encoding/payloads/sa_payload.c
+++ b/src/charon/encoding/payloads/sa_payload.c
@@ -119,15 +119,8 @@ static status_t verify(private_sa_payload_t *this)
 	while(iterator->iterate(iterator, (void**)&current_proposal))
 	{
 		current_number = current_proposal->get_proposal_number(current_proposal);
-		if (current_number > expected_number)
-		{
-			if (first)
-			{
-				DBG1(DBG_ENC, "first proposal is not proposal #1");
-				status = FAILED;
-				break;
-			}
-			
+		if (current_number < expected_number)
+		{			
 			if (current_number != (expected_number + 1))
 			{
 				DBG1(DBG_ENC, "proposal number is %d, excepted %d or %d",
diff --git a/src/charon/sa/tasks/child_create.c b/src/charon/sa/tasks/child_create.c
index 16b21eb4a..90ac2d06b 100644
--- a/src/charon/sa/tasks/child_create.c
+++ b/src/charon/sa/tasks/child_create.c
@@ -193,7 +193,9 @@ static status_t select_and_install(private_child_create_t *this)
 	my_vip = this->ike_sa->get_virtual_ip(this->ike_sa, TRUE);
 	other_vip = this->ike_sa->get_virtual_ip(this->ike_sa, FALSE);
 
+	DBG1(DBG_IKE, "received %d proposals, selecting:", this->proposals->get_count(this->proposals));
 	this->proposal = this->policy->select_proposal(this->policy, this->proposals);
+	DBG1(DBG_IKE, "proposal is %p", this->proposal);
 	
 	if (this->initiator && my_vip)
 	{	/* if we have a virtual IP, shorten our TS to the minimum */
@@ -230,14 +232,19 @@ static status_t select_and_install(private_child_create_t *this)
 		this->tsi = other_ts;
 	}
 	
-	if (this->proposal == NULL ||
-		this->tsi->get_count(this->tsi) == 0 ||
-		this->tsr->get_count(this->tsr) == 0)
+	if (this->proposal == NULL)
 	{
 		SIG(CHILD_UP_FAILED, "no acceptable proposal found");
 		return FAILED;
 	}
 	
+	if (this->tsi->get_count(this->tsi) == 0 ||
+		this->tsr->get_count(this->tsr) == 0)
+	{
+		SIG(CHILD_UP_FAILED, "no acceptable traffic selectors found");
+		return FAILED;
+	}
+	
 	if (!this->initiator)
 	{
 		/* check if requested mode is acceptable, downgrade if required */
diff --git a/src/charon/sa/tasks/ike_auth.c b/src/charon/sa/tasks/ike_auth.c
index 4ab486ac7..bbc174246 100644
--- a/src/charon/sa/tasks/ike_auth.c
+++ b/src/charon/sa/tasks/ike_auth.c
@@ -108,7 +108,6 @@ static status_t build_payloads(private_ike_auth_t *this, message_t *message)
 	me = this->ike_sa->get_my_id(this->ike_sa);
 	other = this->ike_sa->get_other_id(this->ike_sa);
 	
-	
 	/* create own authenticator and add auth payload */
 	policy = this->ike_sa->get_policy(this->ike_sa);
 	if (!policy)
@@ -126,7 +125,7 @@ static status_t build_payloads(private_ike_auth_t *this, message_t *message)
 			SIG(IKE_UP_FAILED, "negotiation of own ID failed");
 			return FAILED;
 		}
-		this->ike_sa->set_my_id(this->ike_sa, me);
+		this->ike_sa->set_my_id(this->ike_sa, me->clone(me));
 	}
 		
 	id_payload = id_payload_create_from_identification(this->initiator, me);
@@ -214,6 +213,7 @@ static void process_payloads(private_ike_auth_t *this, message_t *message)
 	if (this->initiator)
 	{
 		this->ike_sa->set_other_id(this->ike_sa, idr);
+		DESTROY_IF(idi);
 	}
 	else
 	{
diff --git a/src/charon/sa/tasks/ike_config.c b/src/charon/sa/tasks/ike_config.c
index b6e883b48..d5e4dd7ef 100644
--- a/src/charon/sa/tasks/ike_config.c
+++ b/src/charon/sa/tasks/ike_config.c
@@ -300,12 +300,12 @@ static status_t build_r(private_ike_config_t *this, message_t *message)
 			
 			DBG1(DBG_IKE, "peer requested virtual IP %H", this->virtual_ip);
 			ip = this->policy->get_virtual_ip(this->policy, this->virtual_ip);
-			if (ip == NULL)
+			if (ip == NULL || ip->is_anyaddr(ip))
 			{
 				DBG1(DBG_IKE, "not assigning a virtual IP to peer");
 				return SUCCESS;
 			}
-				DBG1(DBG_IKE, "assigning virtual IP %H to peer", ip);
+			DBG1(DBG_IKE, "assigning virtual IP %H to peer", ip);
 			this->ike_sa->set_virtual_ip(this->ike_sa, FALSE, ip);
 			
 			this->virtual_ip->destroy(this->virtual_ip);
diff --git a/src/charon/sa/tasks/ike_init.c b/src/charon/sa/tasks/ike_init.c
index 9149aab91..9c859f4f8 100644
--- a/src/charon/sa/tasks/ike_init.c
+++ b/src/charon/sa/tasks/ike_init.c
@@ -135,13 +135,12 @@ static void build_payloads(private_ike_init_t *this, message_t *message)
 	}
 	message->add_payload(message, (payload_t*)sa_payload);
 	
-	ke_payload = ke_payload_create_from_diffie_hellman(this->diffie_hellman);
-	message->add_payload(message, (payload_t*)ke_payload);
-	
 	nonce_payload = nonce_payload_create();
 	nonce_payload->set_nonce(nonce_payload, this->my_nonce);
-	
 	message->add_payload(message, (payload_t*)nonce_payload);
+	
+	ke_payload = ke_payload_create_from_diffie_hellman(this->diffie_hellman);
+	message->add_payload(message, (payload_t*)ke_payload);
 }
 
 /**