]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
child-create: Change how DH group/QSKE mechanism is determined
authorTobias Brunner <tobias@strongswan.org>
Fri, 20 Jul 2018 15:44:14 +0000 (17:44 +0200)
committerAndreas Steffen <andreas.steffen@strongswan.org>
Fri, 18 Oct 2019 15:10:30 +0000 (17:10 +0200)
Either reuse algorithms previously used (rekeying) or use the IKE_SA's
proposal to determine a preferred group/mechanism.

src/libcharon/sa/ikev2/tasks/child_create.c
src/libcharon/sa/ikev2/tasks/child_create.h
src/libcharon/sa/ikev2/tasks/child_rekey.c

index 859e85b97447da8ff951bb7532412ab29d823225..04f4679123a880acddfa88cc6657326af2e7badc 100644 (file)
@@ -80,10 +80,15 @@ struct private_child_create_t {
        linked_list_t *proposals;
 
        /**
-        * selected proposal to use for CHILD_SA
+        * Selected proposal to use for CHILD_SA
         */
        proposal_t *proposal;
 
+       /**
+        * Proposal used previously during a rekeying
+        */
+       proposal_t *old_proposal;
+
        /**
         * traffic selectors for initiators side
         */
@@ -1165,6 +1170,80 @@ static status_t defer_child_sa(private_child_create_t *this)
        return NOT_SUPPORTED;
 }
 
+/**
+ * Assign algorithms from the given proposal, if we find them in our config.
+ *
+ * Returns TRUE if algorithms were found and assigned.
+ */
+static bool assign_pfs_algorithms(private_child_create_t *this,
+                                                                 proposal_t *proposal, char *origin)
+{
+       child_cfg_t *child_cfg = this->config;
+       uint16_t dh = 0, qske = 0;
+
+       if (proposal->get_algorithm(proposal, DIFFIE_HELLMAN_GROUP, &dh, NULL) ||
+               proposal->get_algorithm(proposal, QSKE_MECHANISM, &qske, NULL))
+       {
+               if (dh && this->dh_group != dh &&
+                       child_cfg->has_transform(child_cfg, DIFFIE_HELLMAN_GROUP, dh))
+               {
+                       DBG2(DBG_CFG, "prefer %s DH group %N to first in proposals %N",
+                                origin, diffie_hellman_group_names, dh,
+                                diffie_hellman_group_names, this->dh_group);
+                       this->dh_group = dh;
+               }
+               if (qske && this->qske_mechanism != qske &&
+                       child_cfg->has_transform(child_cfg, QSKE_MECHANISM, qske))
+               {
+                       DBG2(DBG_CFG, "prefer %s QSKE mechanism %N to first in proposals "
+                                "%N", origin, qske_mechanism_names, qske, qske_mechanism_names,
+                                this->qske_mechanism);
+                       this->qske_mechanism = qske;
+               }
+               return TRUE;
+       }
+       return FALSE;
+}
+
+/**
+ * Determine DH group/QSKE mechanism to use as initiator
+ */
+static void determine_pfs_algorithms(private_child_create_t *this)
+{
+       child_cfg_t *child_cfg = this->config;
+
+       /* determine default algorithms and whether we actually propose PFS */
+       this->dh_group = child_cfg->get_algorithm(child_cfg, DIFFIE_HELLMAN_GROUP);
+       this->qske_mechanism = child_cfg->get_algorithm(child_cfg, QSKE_MECHANISM);
+       if (this->dh_group == MODP_NONE && this->qske_mechanism == QSKE_NONE)
+       {
+               return;
+       }
+
+       /* reuse previous algorithms when rekeying, unless this is the first
+        * rekeying, which we assume is the case if the previous proposal contains
+        * no DH/QSKE algorithms (this could also be the case if we have proposals
+        * without PFS configured and the peer selected one of them) */
+       if (this->rekey && this->old_proposal &&
+               lib->settings->get_bool(lib->settings,
+                                                               "%s.prefer_previous_dh_group", TRUE, lib->ns))
+       {
+               if (assign_pfs_algorithms(this, this->old_proposal, "previously used"))
+               {
+                       return;
+               }
+       }
+
+       /* turn to the IKE_SA's proposal for pointers if we have no previous
+        * CHILD_SA proposal or it wasn't a PFS proposal */
+       if (lib->settings->get_bool(lib->settings,
+                                                       "%s.prefer_ike_dh_group", TRUE, lib->ns))
+       {
+               assign_pfs_algorithms(this, this->ike_sa->get_proposal(this->ike_sa),
+                                                         "IKE_SA's");
+       }
+}
+
 METHOD(task_t, build_i, status_t,
        private_child_create_t *this, message_t *message)
 {
@@ -1254,6 +1333,11 @@ METHOD(task_t, build_i, status_t,
        this->proposals = this->config->get_proposals(this->config, no_dh);
        this->mode = this->config->get_mode(this->config);
 
+       if (!no_dh && !this->retry)
+       {
+               determine_pfs_algorithms(this);
+       }
+
        this->child.if_id_in_def = this->ike_sa->get_if_id(this->ike_sa, TRUE);
        this->child.if_id_out_def = this->ike_sa->get_if_id(this->ike_sa, FALSE);
        this->child.encap = this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY);
@@ -1280,23 +1364,6 @@ METHOD(task_t, build_i, status_t,
                return FAILED;
        }
 
-       /* FIXME: get a group/mechanism from the IKE_SA if we are not rekeying */
-
-       /* during a rekeying the group might already be set */
-       if (!no_dh && !this->retry)
-       {
-               if (this->dh_group == MODP_NONE)
-               {
-                       this->dh_group = this->config->get_algorithm(this->config,
-                                                                                                                DIFFIE_HELLMAN_GROUP);
-               }
-               if (this->qske_mechanism == QSKE_NONE)
-               {
-                       this->qske_mechanism = this->config->get_algorithm(this->config,
-                                                                                                                          QSKE_MECHANISM);
-               }
-       }
-
        if (this->dh_group != MODP_NONE)
        {
                this->dh = this->keymat->keymat.create_dh(&this->keymat->keymat,
@@ -2116,10 +2183,11 @@ METHOD(child_create_t, use_if_ids, void,
        this->child.if_id_out = out;
 }
 
-METHOD(child_create_t, use_dh_group, void,
-       private_child_create_t *this, diffie_hellman_group_t dh_group)
+METHOD(child_create_t, use_proposal, void,
+       private_child_create_t *this, proposal_t *proposal)
 {
-       this->dh_group = dh_group;
+       DESTROY_IF(this->proposal);
+       this->old_proposal = proposal->clone(proposal);
 }
 
 METHOD(child_create_t, get_child, child_sa_t*,
@@ -2170,6 +2238,7 @@ METHOD(task_t, migrate, void,
        }
        DESTROY_IF(this->child_sa);
        DESTROY_IF(this->proposal);
+       DESTROY_IF(this->old_proposal);
        DESTROY_IF(this->nonceg);
        DESTROY_IF(this->qske);
        DESTROY_IF(this->dh);
@@ -2184,6 +2253,7 @@ METHOD(task_t, migrate, void,
        this->keymat = (keymat_v2_t*)ike_sa->get_keymat(ike_sa);
        this->proposal = NULL;
        this->proposals = NULL;
+       this->old_proposal = NULL;
        this->tsi = NULL;
        this->tsr = NULL;
        this->dh = NULL;
@@ -2217,6 +2287,7 @@ METHOD(task_t, destroy, void,
        }
        DESTROY_IF(this->packet_tsi);
        DESTROY_IF(this->packet_tsr);
+       DESTROY_IF(this->old_proposal);
        DESTROY_IF(this->proposal);
        DESTROY_IF(this->qske);
        DESTROY_IF(this->dh);
@@ -2246,7 +2317,7 @@ child_create_t *child_create_create(ike_sa_t *ike_sa,
                        .use_reqid = _use_reqid,
                        .use_marks = _use_marks,
                        .use_if_ids = _use_if_ids,
-                       .use_dh_group = _use_dh_group,
+                       .use_proposal = _use_proposal,
                        .task = {
                                .get_type = _get_type,
                                .migrate = _migrate,
index eae1f3532f548edcc56a693229812a752ab8f5e9..d40c4253d2be3a750375f98ee428bcc2c1cbd8a9 100644 (file)
@@ -69,13 +69,12 @@ struct child_create_t {
        void (*use_if_ids)(child_create_t *this, uint32_t in, uint32_t out);
 
        /**
-        * Initially propose a specific DH group to override configuration.
+        * Set a proposal the initiator can use to e.g. determine what DH group
+        * to propose during a rekeying.
         *
-        * This is used during rekeying to prefer the previously negotiated group.
-        *
-        * @param dh_group      DH group to use
+        * @param proposal      previous proposal to use as blueprint (cloned)
         */
-       void (*use_dh_group)(child_create_t *this, diffie_hellman_group_t dh_group);
+       void (*use_proposal)(child_create_t *this, proposal_t *proposal);
 
        /**
         * Get the lower of the two nonces, used for rekey collisions.
index 6b0f4278563d2ee1418286c6c0ff43df00ea3fdd..fdf4d420bf8f45400ce4e2c843fc1e63fc32b5d8 100644 (file)
@@ -190,17 +190,13 @@ METHOD(task_t, build_i, status_t,
        if (!this->child_create)
        {
                proposal_t *proposal;
-               uint16_t dh_group;
 
                this->child_create = child_create_create(this->ike_sa,
                                                                        config->get_ref(config), TRUE, NULL, NULL);
 
+               /* reuse algorithms negotiated previously */
                proposal = this->child_sa->get_proposal(this->child_sa);
-               if (proposal->get_algorithm(proposal, DIFFIE_HELLMAN_GROUP,
-                                                                       &dh_group, NULL))
-               {       /* reuse the DH group negotiated previously */
-                       this->child_create->use_dh_group(this->child_create, dh_group);
-               }
+               this->child_create->use_proposal(this->child_create, proposal);
        }
        reqid = this->child_sa->get_reqid(this->child_sa);
        this->child_create->use_reqid(this->child_create, reqid);