ikev1: Get and set the lifetimes of the selected proposal/transform

Previously, we simply used the lifetimes of the first
proposal/transform, which is not correct if the initiator uses different
lifetimes in its proposals/transforms.

diff --git a/src/libcharon/encoding/payloads/proposal_substructure.c b/src/libcharon/encoding/payloads/proposal_substructure.c
index d7e1d2814cc42c136cc91debdccecce99da4b09b..127689eeccac84701d2bbe04e65bee77c00461f5 100644 (file)
--- a/src/libcharon/encoding/payloads/proposal_substructure.c
+++ b/src/libcharon/encoding/payloads/proposal_substructure.c
@@ -1079,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;
@@ -1089,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))
                {
@@ -1113,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;
@@ -1137,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;

diff --git a/src/libcharon/encoding/payloads/proposal_substructure.h b/src/libcharon/encoding/payloads/proposal_substructure.h
index 0395ac8cf46c668de30a732cf0fbd1acfc837efb..65d72e0cbb96cb579cc82007e18349f3c27d3324 100644 (file)
--- a/src/libcharon/encoding/payloads/proposal_substructure.h
+++ b/src/libcharon/encoding/payloads/proposal_substructure.h
@@ -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).

diff --git a/src/libcharon/encoding/payloads/sa_payload.c b/src/libcharon/encoding/payloads/sa_payload.c
index 5f0ffd326f3e82f18fc82c7cd0e32a2e68ff84e7..f254bd758dac1f104ada7729030a1e4e44e8ce99 100644 (file)
--- a/src/libcharon/encoding/payloads/sa_payload.c
+++ b/src/libcharon/encoding/payloads/sa_payload.c
@@ -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);
 

diff --git a/src/libcharon/encoding/payloads/sa_payload.h b/src/libcharon/encoding/payloads/sa_payload.h
index d6c73009ef219f5ca446acbb44ca262eeaf95db4..6bd78d428884e1e956aeef0f2c4813443ff5f5e3 100644 (file)
--- a/src/libcharon/encoding/payloads/sa_payload.h
+++ b/src/libcharon/encoding/payloads/sa_payload.h
@@ -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).

diff --git a/src/libcharon/sa/ikev1/tasks/aggressive_mode.c b/src/libcharon/sa/ikev1/tasks/aggressive_mode.c
index 94c3b7682183aaa1279e646bc9ba17d9b58e0177..7314f8872e53be85b6dda549c6abcb5c01356a27 100644 (file)
--- a/src/libcharon/sa/ikev1/tasks/aggressive_mode.c
+++ b/src/libcharon/sa/ikev1/tasks/aggressive_mode.c
@@ -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 "

diff --git a/src/libcharon/sa/ikev1/tasks/main_mode.c b/src/libcharon/sa/ikev1/tasks/main_mode.c
index 43848ad1a49058214c8c35c983877dbaed2f3a29..eb77f5cb8cf3cb9bd5c58cb131f54dbca57d0a11 100644 (file)
--- a/src/libcharon/sa/ikev1/tasks/main_mode.c
+++ b/src/libcharon/sa/ikev1/tasks/main_mode.c
@@ -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 "

diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c
index 9ded2dd5392e9653080893a5221409227c712e3d..89d74442501b6eb230276cfe945b4742284e1b8b 100644 (file)
--- a/src/libcharon/sa/ikev1/tasks/quick_mode.c
+++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c
@@ -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",