From: Tobias Brunner Date: Fri, 20 Jul 2018 15:44:14 +0000 (+0200) Subject: child-create: Change how DH group/QSKE mechanism is determined X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=d45913ddb8fa465c286be283a9dbfccdfda6fd8f;p=thirdparty%2Fstrongswan.git child-create: Change how DH group/QSKE mechanism is determined Either reuse algorithms previously used (rekeying) or use the IKE_SA's proposal to determine a preferred group/mechanism. --- diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 859e85b974..04f4679123 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -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, diff --git a/src/libcharon/sa/ikev2/tasks/child_create.h b/src/libcharon/sa/ikev2/tasks/child_create.h index eae1f3532f..d40c4253d2 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.h +++ b/src/libcharon/sa/ikev2/tasks/child_create.h @@ -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. diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index 6b0f427856..fdf4d420bf 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -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);