From b4bdbed326a12b05657c3cc9c3a93269e4a9e938 Mon Sep 17 00:00:00 2001 From: tobhe Date: Tue, 12 Oct 2021 10:01:59 +0000 Subject: [PATCH] Change responder to prefer DH group from KE payload. Without this change the responder would always prefer the first DH group configured in its policy. This would lead to invalid KE messages that cause an additional exchange which old implementations do not support correctly. Now we ignore the order of DH groups in the policy and prefer the group from the policy that matches the KE payload. from markus@ ok patrick@ --- iked/iked.h | 4 ++-- iked/ikev2.c | 10 +++++----- iked/policy.c | 25 +++++++++++++++++++------ 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/iked/iked.h b/iked/iked.h index fb87353d..44d94193 100644 --- a/iked/iked.h +++ b/iked/iked.h @@ -1,4 +1,4 @@ -/* $OpenBSD: iked.h,v 1.193 2021/09/01 15:30:06 tobhe Exp $ */ +/* $OpenBSD: iked.h,v 1.194 2021/10/12 10:01:59 tobhe Exp $ */ /* * Copyright (c) 2019-2021 Tobias Heider @@ -920,7 +920,7 @@ struct iked_sa * sa_dstid_insert(struct iked *, struct iked_sa *); void sa_dstid_remove(struct iked *, struct iked_sa *); int proposals_negotiate(struct iked_proposals *, struct iked_proposals *, - struct iked_proposals *, int); + struct iked_proposals *, int, int); RB_PROTOTYPE(iked_sas, iked_sa, sa_entry, sa_cmp); RB_PROTOTYPE(iked_dstid_sas, iked_sa, sa_dstid_entry, sa_dstid_cmp); RB_PROTOTYPE(iked_addrpool, iked_sa, sa_addrpool_entry, sa_addrpool_cmp); diff --git a/iked/ikev2.c b/iked/ikev2.c index ec6c6e22..0ea3c588 100644 --- a/iked/ikev2.c +++ b/iked/ikev2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ikev2.c,v 1.328 2021/10/12 09:27:21 tobhe Exp $ */ +/* $OpenBSD: ikev2.c,v 1.329 2021/10/12 10:01:59 tobhe Exp $ */ /* * Copyright (c) 2019 Tobias Heider @@ -974,7 +974,7 @@ ikev2_ike_auth_recv(struct iked *env, struct iked_sa *sa, if (!TAILQ_EMPTY(&msg->msg_proposals)) { if (proposals_negotiate(&sa->sa_proposals, &sa->sa_policy->pol_proposals, &msg->msg_proposals, - 0) != 0) { + 0, -1) != 0) { log_info("%s: no proposal chosen", __func__); msg->msg_error = IKEV2_N_NO_PROPOSAL_CHOSEN; return (-1); @@ -4227,7 +4227,7 @@ ikev2_init_create_child_sa(struct iked *env, struct iked_message *msg) } if (proposals_negotiate(&sa->sa_proposals, &sa->sa_proposals, - &msg->msg_proposals, 1) != 0) { + &msg->msg_proposals, 1, -1) != 0) { log_info("%s: no proposal chosen", SPI_SA(sa, __func__)); return (-1); } @@ -4730,7 +4730,7 @@ ikev2_resp_create_child_sa(struct iked *env, struct iked_message *msg) if (proposals_negotiate(&proposals, &sa->sa_policy->pol_proposals, &msg->msg_proposals, - 1) != 0) { + 1, msg->msg_dhgroup) != 0) { log_info("%s: no proposal chosen", __func__); msg->msg_error = IKEV2_N_NO_PROPOSAL_CHOSEN; goto fail; @@ -5245,7 +5245,7 @@ ikev2_sa_negotiate_common(struct iked *env, struct iked_sa *sa, struct iked_mess /* XXX we need a better way to get this */ if (proposals_negotiate(&sa->sa_proposals, - &msg->msg_policy->pol_proposals, &msg->msg_proposals, 0) != 0) { + &msg->msg_policy->pol_proposals, &msg->msg_proposals, 0, -1) != 0) { log_info("%s: proposals_negotiate", __func__); return (-1); } diff --git a/iked/policy.c b/iked/policy.c index 4f60c411..c80b180e 100644 --- a/iked/policy.c +++ b/iked/policy.c @@ -1,4 +1,4 @@ -/* $OpenBSD: policy.c,v 1.83 2021/09/01 15:30:06 tobhe Exp $ */ +/* $OpenBSD: policy.c,v 1.84 2021/10/12 10:01:59 tobhe Exp $ */ /* * Copyright (c) 2020-2021 Tobias Heider @@ -53,7 +53,7 @@ static __inline int static int policy_test_flows(struct iked_policy *, struct iked_policy *); static int proposals_match(struct iked_proposal *, struct iked_proposal *, - struct iked_transform **, int); + struct iked_transform **, int, int); void policy_init(struct iked *env) @@ -276,7 +276,7 @@ policy_test(struct iked *env, struct iked_policy *key) /* Make sure the proposals are compatible */ if (TAILQ_FIRST(&key->pol_proposals) && proposals_negotiate(NULL, &p->pol_proposals, - &key->pol_proposals, 0) == -1) { + &key->pol_proposals, 0, -1) == -1) { p = TAILQ_NEXT(p, pol_entry); continue; } @@ -973,7 +973,7 @@ user_cmp(struct iked_user *a, struct iked_user *b) */ int proposals_negotiate(struct iked_proposals *result, struct iked_proposals *local, - struct iked_proposals *peer, int rekey) + struct iked_proposals *peer, int rekey, int groupid) { struct iked_proposal *ppeer = NULL, *plocal, *prop, vpeer, vlocal; struct iked_transform chosen[IKEV2_XFORMTYPE_MAX]; @@ -998,7 +998,7 @@ proposals_negotiate(struct iked_proposals *result, struct iked_proposals *local, continue; bzero(match, sizeof(match)); score = proposals_match(plocal, ppeer, match, - rekey); + rekey, groupid); log_debug("%s: score %d", __func__, score); if (score && (!chosen_score || score < chosen_score)) { chosen_score = score; @@ -1053,10 +1053,11 @@ proposals_negotiate(struct iked_proposals *result, struct iked_proposals *local, static int proposals_match(struct iked_proposal *local, struct iked_proposal *peer, - struct iked_transform **xforms, int rekey) + struct iked_transform **xforms, int rekey, int dhgroup) { struct iked_transform *tpeer, *tlocal; unsigned int i, j, type, score, requiredh = 0, nodh = 0, noauth = 0; + unsigned int dhforced = 0; uint8_t protoid = peer->prop_protoid; uint8_t peerxfs[IKEV2_XFORMTYPE_MAX]; @@ -1114,6 +1115,18 @@ proposals_match(struct iked_proposal *local, struct iked_proposal *peer, continue; type = tpeer->xform_type; + if (rekey && nodh == 0 && dhgroup >= 0 && + protoid == IKEV2_SAPROTO_ESP && + type == IKEV2_XFORMTYPE_DH) { + if (dhforced) + continue; + /* reset xform, so this xform w/matching group is enforced */ + if (tlocal->xform_id == dhgroup) { + xforms[type] = NULL; + dhforced = 1; + } + } + if (xforms[type] == NULL || tlocal->xform_score < xforms[type]->xform_score) { xforms[type] = tlocal;