]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
Merge branch 'ikev1-transform-nr'
authorTobias Brunner <tobias@strongswan.org>
Fri, 6 Mar 2020 09:47:34 +0000 (10:47 +0100)
committerTobias Brunner <tobias@strongswan.org>
Fri, 6 Mar 2020 09:47:34 +0000 (10:47 +0100)
With these changes we return the lifetimes of the actually selected
transform back to the client, which is an issue if the peer uses
different lifetimes for different proposals.  We now also return the
correct transform and proposal IDs.

Fixes #3329.

src/libcharon/encoding/payloads/proposal_substructure.c
src/libcharon/encoding/payloads/proposal_substructure.h
src/libcharon/encoding/payloads/sa_payload.c
src/libcharon/encoding/payloads/sa_payload.h
src/libcharon/sa/ikev1/tasks/aggressive_mode.c
src/libcharon/sa/ikev1/tasks/main_mode.c
src/libcharon/sa/ikev1/tasks/quick_mode.c
src/libstrongswan/crypto/proposal/proposal.c
src/libstrongswan/crypto/proposal/proposal.h

index 2d0cb1f829f0f89be820a80eb4d618cb9dd4c921..127689eeccac84701d2bbe04e65bee77c00461f5 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2014 Tobias Brunner
+ * Copyright (C) 2012-2020 Tobias Brunner
  * Copyright (C) 2005-2010 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * HSR Hochschule fuer Technik Rapperswil
@@ -1004,18 +1004,25 @@ METHOD(proposal_substructure_t, get_proposals, void,
        enumerator = this->transforms->create_enumerator(this->transforms);
        while (enumerator->enumerate(enumerator, &transform))
        {
-               if (!proposal)
-               {
-                       proposal = proposal_create(this->protocol_id, this->proposal_number);
-                       proposal->set_spi(proposal, spi);
-                       proposals->insert_last(proposals, proposal);
-               }
                if (this->type == PLV2_PROPOSAL_SUBSTRUCTURE)
                {
+                       if (!proposal)
+                       {
+                               proposal = proposal_create(this->protocol_id,
+                                                                                  this->proposal_number);
+                               proposal->set_spi(proposal, spi);
+                               proposals->insert_last(proposals, proposal);
+                       }
                        add_to_proposal_v2(proposal, transform);
                }
                else
                {
+                       /* create a new proposal for each transform in IKEv1 */
+                       proposal = proposal_create_v1(
+                                                       this->protocol_id, this->proposal_number,
+                                                       transform->get_transform_type_or_number(transform));
+                       proposal->set_spi(proposal, spi);
+                       proposals->insert_last(proposals, proposal);
                        switch (this->protocol_id)
                        {
                                case PROTO_IKE:
@@ -1028,8 +1035,6 @@ METHOD(proposal_substructure_t, get_proposals, void,
                                default:
                                        break;
                        }
-                       /* create a new proposal for each transform in IKEv1 */
-                       proposal = NULL;
                }
        }
        enumerator->destroy(enumerator);
@@ -1074,8 +1079,8 @@ static uint64_t get_attr(private_proposal_substructure_t *this,
  * Look up a lifetime duration of a given kind in all transforms
  */
 static uint64_t get_life_duration(private_proposal_substructure_t *this,
-                               transform_attribute_type_t type_attr, ikev1_life_type_t type,
-                               transform_attribute_type_t dur_attr)
+                               uint8_t number, transform_attribute_type_t type_attr,
+                               ikev1_life_type_t type, transform_attribute_type_t dur_attr)
 {
        enumerator_t *transforms, *attributes;
        transform_substructure_t *transform;
@@ -1084,6 +1089,10 @@ static uint64_t get_life_duration(private_proposal_substructure_t *this,
        transforms = this->transforms->create_enumerator(this->transforms);
        while (transforms->enumerate(transforms, &transform))
        {
+               if (transform->get_transform_type_or_number(transform) != number)
+               {
+                       continue;
+               }
                attributes = transform->create_attribute_enumerator(transform);
                while (attributes->enumerate(attributes, &attr))
                {
@@ -1108,19 +1117,20 @@ static uint64_t get_life_duration(private_proposal_substructure_t *this,
 }
 
 METHOD(proposal_substructure_t, get_lifetime, uint32_t,
-       private_proposal_substructure_t *this)
+       private_proposal_substructure_t *this, uint8_t transform)
 {
        uint32_t duration;
 
        switch (this->protocol_id)
        {
                case PROTO_IKE:
-                       return get_life_duration(this, TATTR_PH1_LIFE_TYPE,
-                                               IKEV1_LIFE_TYPE_SECONDS, TATTR_PH1_LIFE_DURATION);
+                       return get_life_duration(this, transform, TATTR_PH1_LIFE_TYPE,
+                                                       IKEV1_LIFE_TYPE_SECONDS, TATTR_PH1_LIFE_DURATION);
                case PROTO_ESP:
                case PROTO_AH:
-                       duration = get_life_duration(this, TATTR_PH2_SA_LIFE_TYPE,
-                                               IKEV1_LIFE_TYPE_SECONDS, TATTR_PH2_SA_LIFE_DURATION);
+                       duration = get_life_duration(this, transform,
+                                                       TATTR_PH2_SA_LIFE_TYPE, IKEV1_LIFE_TYPE_SECONDS,
+                                                       TATTR_PH2_SA_LIFE_DURATION);
                        if (!duration)
                        {       /* default to 8 hours, RFC 2407 */
                                return 28800;
@@ -1132,14 +1142,15 @@ METHOD(proposal_substructure_t, get_lifetime, uint32_t,
 }
 
 METHOD(proposal_substructure_t, get_lifebytes, uint64_t,
-       private_proposal_substructure_t *this)
+       private_proposal_substructure_t *this, uint8_t transform)
 {
        switch (this->protocol_id)
        {
                case PROTO_ESP:
                case PROTO_AH:
-                       return 1000 * get_life_duration(this, TATTR_PH2_SA_LIFE_TYPE,
-                                       IKEV1_LIFE_TYPE_KILOBYTES, TATTR_PH2_SA_LIFE_DURATION);
+                       return 1000 * get_life_duration(this, transform,
+                                                       TATTR_PH2_SA_LIFE_TYPE, IKEV1_LIFE_TYPE_KILOBYTES,
+                                                       TATTR_PH2_SA_LIFE_DURATION);
                case PROTO_IKE:
                default:
                        return 0;
@@ -1525,7 +1536,9 @@ static void set_data(private_proposal_substructure_t *this, proposal_t *proposal
                default:
                        break;
        }
-       this->proposal_number = proposal->get_number(proposal);
+       /* default to 1 if no number is set (mainly for IKEv1, for IKEv2 the numbers
+        * are explicitly set when proposals are added to the SA payload) */
+       this->proposal_number = proposal->get_number(proposal) ?: 1;
        this->protocol_id = proposal->get_protocol(proposal);
        compute_length(this);
 }
@@ -1547,11 +1560,11 @@ proposal_substructure_t *proposal_substructure_create_from_proposal_v2(
 }
 
 /**
- * See header.
+ * Creates an IKEv1 proposal_substructure_t from a proposal_t.
  */
-proposal_substructure_t *proposal_substructure_create_from_proposal_v1(
+static proposal_substructure_t *proposal_substructure_create_from_proposal_v1(
                        proposal_t *proposal, uint32_t lifetime, uint64_t lifebytes,
-                       auth_method_t auth, ipsec_mode_t mode, encap_t udp)
+                       auth_method_t auth, ipsec_mode_t mode, encap_t udp, uint8_t number)
 {
        private_proposal_substructure_t *this;
 
@@ -1560,12 +1573,12 @@ proposal_substructure_t *proposal_substructure_create_from_proposal_v1(
        switch (proposal->get_protocol(proposal))
        {
                case PROTO_IKE:
-                       set_from_proposal_v1_ike(this, proposal, lifetime, auth, 1);
+                       set_from_proposal_v1_ike(this, proposal, lifetime, auth, number);
                        break;
                case PROTO_ESP:
                case PROTO_AH:
                        set_from_proposal_v1(this, proposal, lifetime,
-                                                                lifebytes, mode, udp, 1);
+                                                                lifebytes, mode, udp, number);
                        break;
                default:
                        break;
@@ -1585,17 +1598,18 @@ proposal_substructure_t *proposal_substructure_create_from_proposals_v1(
        private_proposal_substructure_t *this = NULL;
        enumerator_t *enumerator;
        proposal_t *proposal;
-       int number = 0;
+       int number = 1;
 
        enumerator = proposals->create_enumerator(proposals);
        while (enumerator->enumerate(enumerator, &proposal))
        {
                if (!this)
-               {
+               {       /* as responder the transform number is set and we only have a
+                        * single proposal, start with 1 otherwise */
                        this = (private_proposal_substructure_t*)
                                                proposal_substructure_create_from_proposal_v1(
-                                                               proposal, lifetime, lifebytes, auth, mode, udp);
-                       ++number;
+                                                       proposal, lifetime, lifebytes, auth, mode, udp,
+                                                       proposal->get_transform_number(proposal) ?: number);
                }
                else
                {
index 8101d9f386f63833535cb9f82a67268feb0339d4..65d72e0cbb96cb579cc82007e18349f3c27d3324 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Tobias Brunner
+ * Copyright (C) 2012-2020 Tobias Brunner
  * Copyright (C) 2005-2006 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * HSR Hochschule fuer Technik Rapperswil
@@ -131,18 +131,20 @@ struct proposal_substructure_t {
        enumerator_t* (*create_substructure_enumerator)(proposal_substructure_t *this);
 
        /**
-        * Get the (shortest) lifetime of a proposal (IKEv1 only).
+        * Get the lifetime of a transform (IKEv1 only).
         *
+        * @param transform                     transform number
         * @return                                      lifetime, in seconds
         */
-       uint32_t (*get_lifetime)(proposal_substructure_t *this);
+       uint32_t (*get_lifetime)(proposal_substructure_t *this, uint8_t transform);
 
        /**
-        * Get the (shortest) life duration of a proposal (IKEv1 only).
+        * Get the life duration of a transform (IKEv1 only).
         *
+        * @param transform                     transform number
         * @return                                      life duration, in bytes
         */
-       uint64_t (*get_lifebytes)(proposal_substructure_t *this);
+       uint64_t (*get_lifebytes)(proposal_substructure_t *this, uint8_t transform);
 
        /**
         * Get the first authentication method from the proposal (IKEv1 only).
@@ -181,20 +183,6 @@ proposal_substructure_t *proposal_substructure_create(payload_type_t type);
  */
 proposal_substructure_t *proposal_substructure_create_from_proposal_v2(
                                                                                                                proposal_t *proposal);
-/**
- * Creates an IKEv1 proposal_substructure_t from a proposal_t.
- *
- * @param proposal     proposal to build a substruct out of it
- * @param lifetime     lifetime in seconds
- * @param lifebytes    lifebytes, in bytes
- * @param auth         authentication method to use, or AUTH_NONE
- * @param mode         IPsec encapsulation mode, TRANSPORT or TUNNEL
- * @param udp          ENCAP_UDP to use UDP encapsulation
- * @return                     proposal_substructure_t object PLV1_PROPOSAL_SUBSTRUCTURE
- */
-proposal_substructure_t *proposal_substructure_create_from_proposal_v1(
-                       proposal_t *proposal,  uint32_t lifetime, uint64_t lifebytes,
-                       auth_method_t auth, ipsec_mode_t mode, encap_t udp);
 
 /**
  * Creates an IKEv1 proposal_substructure_t from a list of proposal_t.
index 5f0ffd326f3e82f18fc82c7cd0e32a2e68ff84e7..f254bd758dac1f104ada7729030a1e4e44e8ce99 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2014 Tobias Brunner
+ * Copyright (C) 2012-2020 Tobias Brunner
  * Copyright (C) 2005-2010 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * HSR Hochschule fuer Technik Rapperswil
@@ -404,16 +404,22 @@ METHOD(sa_payload_t, create_substructure_enumerator, enumerator_t*,
 }
 
 METHOD(sa_payload_t, get_lifetime, uint32_t,
-       private_sa_payload_t *this)
+       private_sa_payload_t *this, proposal_t *proposal)
 {
        proposal_substructure_t *substruct;
        enumerator_t *enumerator;
+       uint8_t number = proposal->get_number(proposal);
        uint32_t lifetime = 0;
 
        enumerator = this->proposals->create_enumerator(this->proposals);
-       if (enumerator->enumerate(enumerator, &substruct))
+       while (enumerator->enumerate(enumerator, &substruct))
        {
-               lifetime = substruct->get_lifetime(substruct);
+               if (substruct->get_proposal_number(substruct) == number)
+               {
+                       lifetime = substruct->get_lifetime(substruct,
+                                                                       proposal->get_transform_number(proposal));
+                       break;
+               }
        }
        enumerator->destroy(enumerator);
 
@@ -421,16 +427,22 @@ METHOD(sa_payload_t, get_lifetime, uint32_t,
 }
 
 METHOD(sa_payload_t, get_lifebytes, uint64_t,
-       private_sa_payload_t *this)
+       private_sa_payload_t *this, proposal_t *proposal)
 {
        proposal_substructure_t *substruct;
        enumerator_t *enumerator;
+       uint8_t number = proposal->get_number(proposal);
        uint64_t lifebytes = 0;
 
        enumerator = this->proposals->create_enumerator(this->proposals);
-       if (enumerator->enumerate(enumerator, &substruct))
+       while (enumerator->enumerate(enumerator, &substruct))
        {
-               lifebytes = substruct->get_lifebytes(substruct);
+               if (substruct->get_proposal_number(substruct) == number)
+               {
+                       lifebytes = substruct->get_lifebytes(substruct,
+                                                                       proposal->get_transform_number(proposal));
+                       break;
+               }
        }
        enumerator->destroy(enumerator);
 
index d6c73009ef219f5ca446acbb44ca262eeaf95db4..6bd78d428884e1e956aeef0f2c4813443ff5f5e3 100644 (file)
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2012-2020 Tobias Brunner
  * Copyright (C) 2005-2006 Martin Willi
  * Copyright (C) 2005 Jan Hutter
  * HSR Hochschule fuer Technik Rapperswil
@@ -60,18 +61,20 @@ struct sa_payload_t {
        linked_list_t *(*get_ipcomp_proposals) (sa_payload_t *this, uint16_t *cpi);
 
        /**
-        * Get the (shortest) lifetime of a proposal (IKEv1 only).
+        * Get the lifetime of a proposal/transform (IKEv1 only).
         *
+        * @param proposal                      proposal for which to get lifetime
         * @return                                      lifetime, in seconds
         */
-       uint32_t (*get_lifetime)(sa_payload_t *this);
+       uint32_t (*get_lifetime)(sa_payload_t *this, proposal_t *proposal);
 
        /**
-        * Get the (shortest) life duration of a proposal (IKEv1 only).
+        * Get the life duration of a proposal/transform (IKEv1 only).
         *
+        * @param proposal                      proposal for which to get life duration
         * @return                                      life duration, in bytes
         */
-       uint64_t (*get_lifebytes)(sa_payload_t *this);
+       uint64_t (*get_lifebytes)(sa_payload_t *this, proposal_t *proposal);
 
        /**
         * Get the first authentication method from the proposal (IKEv1 only).
index 94c3b7682183aaa1279e646bc9ba17d9b58e0177..7314f8872e53be85b6dda549c6abcb5c01356a27 100644 (file)
@@ -415,7 +415,8 @@ METHOD(task_t, process_r, status_t,
                        this->ike_sa->set_proposal(this->ike_sa, this->proposal);
 
                        this->method = sa_payload->get_auth_method(sa_payload);
-                       this->lifetime = sa_payload->get_lifetime(sa_payload);
+                       this->lifetime = sa_payload->get_lifetime(sa_payload,
+                                                                                                         this->proposal);
 
                        switch (this->method)
                        {
@@ -653,7 +654,7 @@ METHOD(task_t, process_i, status_t,
                }
                this->ike_sa->set_proposal(this->ike_sa, this->proposal);
 
-               lifetime = sa_payload->get_lifetime(sa_payload);
+               lifetime = sa_payload->get_lifetime(sa_payload, this->proposal);
                if (lifetime != this->lifetime)
                {
                        DBG1(DBG_IKE, "received lifetime %us does not match configured "
index 43848ad1a49058214c8c35c983877dbaed2f3a29..eb77f5cb8cf3cb9bd5c58cb131f54dbca57d0a11 100644 (file)
@@ -406,7 +406,8 @@ METHOD(task_t, process_r, status_t,
                        this->ike_sa->set_proposal(this->ike_sa, this->proposal);
 
                        this->method = sa_payload->get_auth_method(sa_payload);
-                       this->lifetime = sa_payload->get_lifetime(sa_payload);
+                       this->lifetime = sa_payload->get_lifetime(sa_payload,
+                                                                                                         this->proposal);
 
                        this->state = MM_SA;
                        return NEED_MORE;
@@ -654,7 +655,7 @@ METHOD(task_t, process_i, status_t,
                        }
                        this->ike_sa->set_proposal(this->ike_sa, this->proposal);
 
-                       lifetime = sa_payload->get_lifetime(sa_payload);
+                       lifetime = sa_payload->get_lifetime(sa_payload, this->proposal);
                        if (lifetime != this->lifetime)
                        {
                                DBG1(DBG_IKE, "received lifetime %us does not match configured "
index 9ded2dd5392e9653080893a5221409227c712e3d..89d74442501b6eb230276cfe945b4742284e1b8b 100644 (file)
@@ -744,8 +744,8 @@ static void apply_lifetimes(private_quick_mode_t *this, sa_payload_t *sa_payload
        uint32_t lifetime;
        uint64_t lifebytes;
 
-       lifetime = sa_payload->get_lifetime(sa_payload);
-       lifebytes = sa_payload->get_lifebytes(sa_payload);
+       lifetime = sa_payload->get_lifetime(sa_payload, this->proposal);
+       lifebytes = sa_payload->get_lifebytes(sa_payload, this->proposal);
        if (this->lifetime != lifetime)
        {
                DBG1(DBG_IKE, "received %us lifetime, configured %us",
index 560303f783858598217caefa91b63902c95aeb27..af2c6874a1eda3acea7d319f6e4e086900cec98b 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2018 Tobias Brunner
+ * Copyright (C) 2008-2020 Tobias Brunner
  * Copyright (C) 2006-2010 Martin Willi
  * Copyright (C) 2013-2015 Andreas Steffen
  * HSR Hochschule fuer Technik Rapperswil
@@ -70,7 +70,12 @@ struct private_proposal_t {
        /**
         * Proposal number
         */
-       u_int number;
+       uint8_t number;
+
+       /**
+        * Transform number (IKEv1 only)
+        */
+       uint8_t transform_number;
 };
 
 /**
@@ -455,12 +460,14 @@ METHOD(proposal_t, select_proposal, proposal_t*,
 
        if (flags & PROPOSAL_PREFER_SUPPLIED)
        {
-               selected = proposal_create(this->protocol, this->number);
+               selected = proposal_create_v1(this->protocol, this->number,
+                                                                         this->transform_number);
                selected->set_spi(selected, this->spi);
        }
        else
        {
-               selected = proposal_create(this->protocol, other->get_number(other));
+               selected = proposal_create_v1(this->protocol, other->get_number(other),
+                                                                         other->get_transform_number(other));
                selected->set_spi(selected, other->get_spi(other));
        }
 
@@ -539,12 +546,18 @@ static bool algo_list_equals(private_proposal_t *this, proposal_t *other,
        return equals;
 }
 
-METHOD(proposal_t, get_number, u_int,
+METHOD(proposal_t, get_number, uint8_t,
        private_proposal_t *this)
 {
        return this->number;
 }
 
+METHOD(proposal_t, get_transform_number, uint8_t,
+       private_proposal_t *this)
+{
+       return this->transform_number;
+}
+
 METHOD(proposal_t, equals, bool,
        private_proposal_t *this, proposal_t *other)
 {
@@ -598,6 +611,7 @@ METHOD(proposal_t, clone_, proposal_t*,
 
        clone->spi = this->spi;
        clone->number = this->number;
+       clone->transform_number = this->transform_number;
 
        return &clone->public;
 }
@@ -918,7 +932,8 @@ METHOD(proposal_t, destroy, void,
 /*
  * Described in header
  */
-proposal_t *proposal_create(protocol_id_t protocol, u_int number)
+proposal_t *proposal_create_v1(protocol_id_t protocol, uint8_t number,
+                                                          uint8_t transform)
 {
        private_proposal_t *this;
 
@@ -935,12 +950,14 @@ proposal_t *proposal_create(protocol_id_t protocol, u_int number)
                        .set_spi = _set_spi,
                        .get_spi = _get_spi,
                        .get_number = _get_number,
+                       .get_transform_number = _get_transform_number,
                        .equals = _equals,
                        .clone = _clone_,
                        .destroy = _destroy,
                },
                .protocol = protocol,
                .number = number,
+               .transform_number = transform,
                .transforms = array_create(sizeof(entry_t), 0),
                .types = array_create(sizeof(transform_type_t), 0),
        );
@@ -948,6 +965,14 @@ proposal_t *proposal_create(protocol_id_t protocol, u_int number)
        return &this->public;
 }
 
+/*
+ * Described in header
+ */
+proposal_t *proposal_create(protocol_id_t protocol, uint8_t number)
+{
+       return proposal_create_v1(protocol, number, 0);
+}
+
 /**
  * Add supported IKE algorithms to proposal
  */
index 6a9cdba62dc1b0098bacf00cc8ee1c183bc3c530..b8c2fba3ea7a01cb507183891c9eee1724990306 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2019 Tobias Brunner
+ * Copyright (C) 2009-2020 Tobias Brunner
  * Copyright (C) 2006 Martin Willi
  * HSR Hochschule fuer Technik Rapperswil
  *
@@ -181,7 +181,14 @@ struct proposal_t {
         *
         * @return                              proposal number
         */
-       u_int (*get_number)(proposal_t *this);
+       uint8_t (*get_number)(proposal_t *this);
+
+       /**
+        * Get number of the transform on which this proposal is based (IKEv1 only)
+        *
+        * @return                              transform number (or 0)
+        */
+       uint8_t (*get_transform_number)(proposal_t *this);
 
        /**
         * Check for the equality of two proposals.
@@ -212,7 +219,18 @@ struct proposal_t {
  * @param number                       proposal number, as encoded in SA payload
  * @return                                     proposal_t object
  */
-proposal_t *proposal_create(protocol_id_t protocol, u_int number);
+proposal_t *proposal_create(protocol_id_t protocol, uint8_t number);
+
+/**
+ * Create a proposal for IKE, ESP or AH that includes a transform number.
+ *
+ * @param protocol                     protocol, such as PROTO_ESP
+ * @param number                       proposal number, as encoded in SA payload
+ * @param transform                    transform number, as encoded in payload
+ * @return                                     proposal_t object
+ */
+proposal_t *proposal_create_v1(protocol_id_t protocol, uint8_t number,
+                                                          uint8_t transform);
 
 /**
  * Create a default proposal.