fixed double free bug
authorMartin Willi <martin@strongswan.org>
Mon, 5 Mar 2007 22:02:14 +0000 (22:02 -0000)
committerMartin Willi <martin@strongswan.org>
Mon, 5 Mar 2007 22:02:14 +0000 (22:02 -0000)
src/charon/config/policies/local_policy_store.c
src/charon/config/policies/policy.c
src/charon/config/proposal.c
src/charon/config/traffic_selector.c
src/charon/config/traffic_selector.h
src/charon/encoding/payloads/sa_payload.c
src/charon/sa/tasks/child_create.c
src/charon/sa/tasks/ike_auth.c
src/charon/sa/tasks/ike_config.c
src/charon/sa/tasks/ike_init.c

index 7eef382..54c0163 100644 (file)
@@ -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) ||
index 3ff0e32..b853a5a 100644 (file)
@@ -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;
 }
 
index 0faef3d..5aa4ac0 100644 (file)
@@ -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);
index b66b77f..cee45c2 100644 (file)
@@ -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_;
index 33152b0..0e798fc 100644 (file)
@@ -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 
index 7ae0f95..e264b21 100644 (file)
@@ -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",
index 16b21eb..90ac2d0 100644 (file)
@@ -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 */
index 4ab486a..bbc1742 100644 (file)
@@ -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
        {
index b6e883b..d5e4dd7 100644 (file)
@@ -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);
index 9149aab..9c859f4 100644 (file)
@@ -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);
 }
 
 /**