ike-sa: Always set ike_cfg_t when setting peer_cfg_t

This is more consistent and e.g. allows to properly take into account
some settings that are also relevant during IKE_AUTH (e.g. childless).

We also already use the peer_cfg_t's ike_cfg_t when rekeying,
reauthenticating and reestablishing an IKE_SA (and e.g. for DSCP).

Also changed are some IKEv1 cases where get_ike_cfg() is called before
set_peer_cfg() without taking a reference to the ike_cfg_t that might
get replaced/destroyed (none of the cases were problematic, though, but
it also wasn't necessary to keep the ike_cfg_t around).

Closes strongswan/strongswan#1238

diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c
index 0d554204d25438a3861b4721ca48eac4dd471394..b0b2ab27d612f1ff3a68424bed3993b8d4aa7d2e 100644 (file)
--- a/src/libcharon/sa/ike_sa.c
+++ b/src/libcharon/sa/ike_sa.c
@@ -436,11 +436,9 @@ METHOD(ike_sa_t, set_peer_cfg, void,
        DESTROY_IF(this->peer_cfg);
        this->peer_cfg = peer_cfg;
 
-       if (!this->ike_cfg)
-       {
-               this->ike_cfg = peer_cfg->get_ike_cfg(peer_cfg);
-               this->ike_cfg->get_ref(this->ike_cfg);
-       }
+       DESTROY_IF(this->ike_cfg);
+       this->ike_cfg = peer_cfg->get_ike_cfg(peer_cfg);
+       this->ike_cfg->get_ref(this->ike_cfg);
 
        this->if_id_in = peer_cfg->get_if_id(peer_cfg, TRUE);
        this->if_id_out = peer_cfg->get_if_id(peer_cfg, FALSE);
@@ -644,21 +642,9 @@ METHOD(ike_sa_t, get_message_id, uint32_t,
  */
 static void set_dscp(private_ike_sa_t *this, packet_t *packet)
 {
-       ike_cfg_t *ike_cfg;
-
-       /* prefer IKE config on peer_cfg, as its selection is more accurate
-        * then the initial IKE config */
-       if (this->peer_cfg)
-       {
-               ike_cfg = this->peer_cfg->get_ike_cfg(this->peer_cfg);
-       }
-       else
-       {
-               ike_cfg = this->ike_cfg;
-       }
-       if (ike_cfg)
+       if (this->ike_cfg)
        {
-               packet->set_dscp(packet, ike_cfg->get_dscp(ike_cfg));
+               packet->set_dscp(packet, this->ike_cfg->get_dscp(this->ike_cfg));
        }
 }
 

diff --git a/src/libcharon/sa/ikev1/tasks/aggressive_mode.c b/src/libcharon/sa/ikev1/tasks/aggressive_mode.c
index 517843d3c8949b625b2b16f56482fc43bdcdbba8..6dbaabe9cb803b4d15c2e559bfddece0c2118353 100644 (file)
--- a/src/libcharon/sa/ikev1/tasks/aggressive_mode.c
+++ b/src/libcharon/sa/ikev1/tasks/aggressive_mode.c
@@ -58,11 +58,6 @@ struct private_aggressive_mode_t {
         */
        phase1_t *ph1;
 
-       /**
-        * IKE config to establish
-        */
-       ike_cfg_t *ike_cfg;
-
        /**
         * Peer config to use
         */
@@ -213,6 +208,7 @@ METHOD(task_t, build_i, status_t,
        {
                case AM_INIT:
                {
+                       ike_cfg_t *ike_cfg;
                        sa_payload_t *sa_payload;
                        id_payload_t *id_payload;
                        linked_list_t *proposals;
@@ -226,7 +222,7 @@ METHOD(task_t, build_i, status_t,
                                 this->ike_sa->get_other_host(this->ike_sa));
                        this->ike_sa->set_state(this->ike_sa, IKE_CONNECTING);
 
-                       this->ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
+                       ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
                        this->peer_cfg = this->ike_sa->get_peer_cfg(this->ike_sa);
                        this->peer_cfg->get_ref(this->peer_cfg);
 
@@ -244,7 +240,7 @@ METHOD(task_t, build_i, status_t,
                                                                                                                                 FALSE);
                        }
                        this->lifetime += this->peer_cfg->get_over_time(this->peer_cfg);
-                       proposals = this->ike_cfg->get_proposals(this->ike_cfg);
+                       proposals = ike_cfg->get_proposals(ike_cfg);
                        sa_payload = sa_payload_create_from_proposals_v1(proposals,
                                                                        this->lifetime, 0, this->method, MODE_NONE,
                                                                        ENCAP_NONE, 0);
@@ -252,8 +248,7 @@ METHOD(task_t, build_i, status_t,
 
                        message->add_payload(message, &sa_payload->payload_interface);
 
-                       group = this->ike_cfg->get_algorithm(this->ike_cfg,
-                                                                                                KEY_EXCHANGE_METHOD);
+                       group = ike_cfg->get_algorithm(ike_cfg, KEY_EXCHANGE_METHOD);
                        if (!group)
                        {
                                DBG1(DBG_IKE, "DH group selection failed");
@@ -370,6 +365,7 @@ METHOD(task_t, process_r, status_t,
        {
                case AM_INIT:
                {
+                       ike_cfg_t *ike_cfg;
                        sa_payload_t *sa_payload;
                        id_payload_t *id_payload;
                        identification_t *id;
@@ -377,7 +373,7 @@ METHOD(task_t, process_r, status_t,
                        proposal_selection_flag_t flags = PROPOSAL_SKIP_PRIVATE;
                        uint16_t group;
 
-                       this->ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
+                       ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
                        DBG0(DBG_IKE, "%H is initiating a Aggressive Mode IKE_SA",
                                 message->get_source(message));
                        this->ike_sa->set_state(this->ike_sa, IKE_CONNECTING);
@@ -405,8 +401,7 @@ METHOD(task_t, process_r, status_t,
                        {
                                flags = PROPOSAL_PREFER_SUPPLIED;
                        }
-                       this->proposal = this->ike_cfg->select_proposal(this->ike_cfg, list,
-                                                                                                                       flags);
+                       this->proposal = ike_cfg->select_proposal(ike_cfg, list, flags);
                        list->destroy_offset(list, offsetof(proposal_t, destroy));
                        if (!this->proposal)
                        {
@@ -635,6 +630,7 @@ METHOD(task_t, process_i, status_t,
                auth_method_t method;
                sa_payload_t *sa_payload;
                id_payload_t *id_payload;
+               ike_cfg_t *ike_cfg;
                identification_t *id, *cid;
                linked_list_t *list;
                uint32_t lifetime;
@@ -647,7 +643,8 @@ METHOD(task_t, process_i, status_t,
                        return send_notify(this, INVALID_PAYLOAD_TYPE);
                }
                list = sa_payload->get_proposals(sa_payload);
-               this->proposal = this->ike_cfg->select_proposal(this->ike_cfg, list, 0);
+               ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
+               this->proposal = ike_cfg->select_proposal(ike_cfg, list, 0);
                list->destroy_offset(list, offsetof(proposal_t, destroy));
                if (!this->proposal)
                {

diff --git a/src/libcharon/sa/ikev1/tasks/main_mode.c b/src/libcharon/sa/ikev1/tasks/main_mode.c
index 9ab34ce92bee931e2c0f9d14cbd210781d8e3e97..4248700d5e02e2fb00b0c9565ac39664b5e69a35 100644 (file)
--- a/src/libcharon/sa/ikev1/tasks/main_mode.c
+++ b/src/libcharon/sa/ikev1/tasks/main_mode.c
@@ -58,11 +58,6 @@ struct private_main_mode_t {
         */
        phase1_t *ph1;
 
-       /**
-        * IKE config to establish
-        */
-       ike_cfg_t *ike_cfg;
-
        /**
         * Peer config to use
         */
@@ -247,6 +242,7 @@ METHOD(task_t, build_i, status_t,
        {
                case MM_INIT:
                {
+                       ike_cfg_t *ike_cfg;
                        sa_payload_t *sa_payload;
                        linked_list_t *proposals;
                        packet_t *packet;
@@ -257,7 +253,7 @@ METHOD(task_t, build_i, status_t,
                                 this->ike_sa->get_other_host(this->ike_sa));
                        this->ike_sa->set_state(this->ike_sa, IKE_CONNECTING);
 
-                       this->ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
+                       ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
                        this->peer_cfg = this->ike_sa->get_peer_cfg(this->ike_sa);
                        this->peer_cfg->get_ref(this->peer_cfg);
 
@@ -275,7 +271,7 @@ METHOD(task_t, build_i, status_t,
                                                                                                                                 FALSE);
                        }
                        this->lifetime += this->peer_cfg->get_over_time(this->peer_cfg);
-                       proposals = this->ike_cfg->get_proposals(this->ike_cfg);
+                       proposals = ike_cfg->get_proposals(ike_cfg);
                        sa_payload = sa_payload_create_from_proposals_v1(proposals,
                                                                        this->lifetime, 0, this->method, MODE_NONE,
                                                                        ENCAP_NONE, 0);
@@ -364,11 +360,12 @@ METHOD(task_t, process_r, status_t,
        {
                case MM_INIT:
                {
+                       ike_cfg_t *ike_cfg;
                        linked_list_t *list;
                        sa_payload_t *sa_payload;
                        proposal_selection_flag_t flags = 0;
 
-                       this->ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
+                       ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
                        DBG0(DBG_IKE, "%H is initiating a Main Mode IKE_SA",
                                 message->get_source(message));
                        this->ike_sa->set_state(this->ike_sa, IKE_CONNECTING);
@@ -402,8 +399,7 @@ METHOD(task_t, process_r, status_t,
                        {
                                flags |= PROPOSAL_PREFER_SUPPLIED;
                        }
-                       this->proposal = this->ike_cfg->select_proposal(this->ike_cfg,
-                                                                                       list, flags);
+                       this->proposal = ike_cfg->select_proposal(ike_cfg, list, flags);
                        list->destroy_offset(list, offsetof(proposal_t, destroy));
                        if (!this->proposal)
                        {
@@ -636,6 +632,7 @@ METHOD(task_t, process_i, status_t,
                {
                        linked_list_t *list;
                        sa_payload_t *sa_payload;
+                       ike_cfg_t *ike_cfg;
                        auth_method_t method;
                        proposal_selection_flag_t flags = 0;
                        uint32_t lifetime;
@@ -654,8 +651,8 @@ METHOD(task_t, process_i, status_t,
                        {
                                flags |= PROPOSAL_SKIP_PRIVATE;
                        }
-                       this->proposal = this->ike_cfg->select_proposal(this->ike_cfg,
-                                                                                                                       list, flags);
+                       ike_cfg = this->ike_sa->get_ike_cfg(this->ike_sa);
+                       this->proposal = ike_cfg->select_proposal(ike_cfg, list, flags);
                        list->destroy_offset(list, offsetof(proposal_t, destroy));
                        if (!this->proposal)
                        {