From: patrick Date: Fri, 1 Dec 2017 19:49:31 +0000 (+0000) Subject: Turns out that, as specified in the RFC, the initial Child SA does not X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=0313ffb557a660130f140caabf27430884cfcf5f;p=openbsd Turns out that, as specified in the RFC, the initial Child SA does not do PFS and is assumed to be secured using the DH exchange in the first handshake. Thus there is no KE/N payload in the IKE_AUTH exchange and we must not include a DH group other than None, which essentially means we must not supply any DH transforms in the IKE_AUTH messages. So now we skip adding the DH transforms for initiating and responding to IKE_AUTH messages. ok sthen@ --- diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c index e31322e1ac3..fbc487695a9 100644 --- a/sbin/iked/ikev2.c +++ b/sbin/iked/ikev2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ikev2.c,v 1.159 2017/11/30 12:18:44 patrick Exp $ */ +/* $OpenBSD: ikev2.c,v 1.160 2017/12/01 19:49:31 patrick Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -115,7 +115,7 @@ int ikev2_valid_proposal(struct iked_proposal *, struct iked_transform **, struct iked_transform **, int *); ssize_t ikev2_add_proposals(struct iked *, struct iked_sa *, struct ibuf *, - struct iked_proposals *, uint8_t, int, int); + struct iked_proposals *, uint8_t, int, int, int); ssize_t ikev2_add_cp(struct iked *, struct iked_sa *, struct ibuf *); ssize_t ikev2_add_transform(struct ibuf *, uint8_t, uint8_t, uint16_t, uint16_t); @@ -994,7 +994,7 @@ ikev2_init_ike_sa_peer(struct iked *env, struct iked_policy *pol, if ((pld = ikev2_add_payload(buf)) == NULL) goto done; if ((len = ikev2_add_proposals(env, sa, buf, &pol->pol_proposals, - IKEV2_SAPROTO_IKE, sa->sa_hdr.sh_initiator, 0)) == -1) + IKEV2_SAPROTO_IKE, sa->sa_hdr.sh_initiator, 0, 0)) == -1) goto done; if (ikev2_next_payload(pld, len, IKEV2_PAYLOAD_KE) == -1) @@ -1196,7 +1196,7 @@ ikev2_init_ike_auth(struct iked *env, struct iked_sa *sa) if ((pld = ikev2_add_payload(e)) == NULL) goto done; if ((len = ikev2_add_proposals(env, sa, e, &pol->pol_proposals, 0, - sa->sa_hdr.sh_initiator, 0)) == -1) + sa->sa_hdr.sh_initiator, 0, 1)) == -1) goto done; if ((len = ikev2_add_ts(e, &pld, len, sa, 0)) == -1) @@ -1963,7 +1963,7 @@ ikev2_add_cp(struct iked *env, struct iked_sa *sa, struct ibuf *buf) ssize_t ikev2_add_proposals(struct iked *env, struct iked_sa *sa, struct ibuf *buf, struct iked_proposals *proposals, uint8_t protoid, int initiator, - int sendikespi) + int sendikespi, int skipdh) { struct ikev2_sa_proposal *sap; struct iked_transform *xform; @@ -1972,7 +1972,7 @@ ikev2_add_proposals(struct iked *env, struct iked_sa *sa, struct ibuf *buf, ssize_t length = 0, saplength, xflen; uint64_t spi64; uint32_t spi32, spi; - unsigned int i; + unsigned int i, xfi, nxforms; TAILQ_FOREACH(prop, proposals, prop_entry) { if ((protoid && prop->prop_protoid != protoid) || @@ -2007,10 +2007,26 @@ ikev2_add_proposals(struct iked *env, struct iked_sa *sa, struct ibuf *buf, prop->prop_localspi.spi_protoid = IKEV2_SAPROTO_IKE; } + /* + * RFC 7296: 1.2. The Initial Exchanges + * IKE_AUTH messages do not contain KE/N payloads, thus + * SA payloads cannot contain groups. + */ + if (skipdh) { + nxforms = 0; + for (i = 0; i < prop->prop_nxforms; i++) { + xform = prop->prop_xforms + i; + if (xform->xform_type == IKEV2_XFORMTYPE_DH) + continue; + nxforms++; + } + } else + nxforms = prop->prop_nxforms; + sap->sap_proposalnr = prop->prop_id; sap->sap_protoid = prop->prop_protoid; sap->sap_spisize = prop->prop_localspi.spi_size; - sap->sap_transforms = prop->prop_nxforms; + sap->sap_transforms = nxforms; saplength = sizeof(*sap); switch (prop->prop_localspi.spi_size) { @@ -2030,16 +2046,20 @@ ikev2_add_proposals(struct iked *env, struct iked_sa *sa, struct ibuf *buf, break; } - for (i = 0; i < prop->prop_nxforms; i++) { + for (i = 0, xfi = 0; i < prop->prop_nxforms; i++) { xform = prop->prop_xforms + i; + if (skipdh && xform->xform_type == IKEV2_XFORMTYPE_DH) + continue; + if ((xflen = ikev2_add_transform(buf, - i == prop->prop_nxforms - 1 ? + xfi == nxforms - 1 ? IKEV2_XFORM_LAST : IKEV2_XFORM_MORE, xform->xform_type, xform->xform_id, xform->xform_length)) == -1) return (-1); + xfi++; saplength += xflen; } @@ -2340,7 +2360,7 @@ ikev2_resp_ike_sa_init(struct iked *env, struct iked_message *msg) if ((pld = ikev2_add_payload(buf)) == NULL) goto done; if ((len = ikev2_add_proposals(env, sa, buf, &sa->sa_proposals, - IKEV2_SAPROTO_IKE, sa->sa_hdr.sh_initiator, 0)) == -1) + IKEV2_SAPROTO_IKE, sa->sa_hdr.sh_initiator, 0, 0)) == -1) goto done; if (ikev2_next_payload(pld, len, IKEV2_PAYLOAD_KE) == -1) @@ -2713,7 +2733,7 @@ ikev2_resp_ike_auth(struct iked *env, struct iked_sa *sa) if ((pld = ikev2_add_payload(e)) == NULL) goto done; if ((len = ikev2_add_proposals(env, sa, e, &sa->sa_proposals, 0, - sa->sa_hdr.sh_initiator, 0)) == -1) + sa->sa_hdr.sh_initiator, 0, 1)) == -1) goto done; if ((len = ikev2_add_ts(e, &pld, len, sa, 0)) == -1) @@ -2974,7 +2994,7 @@ ikev2_send_create_child_sa(struct iked *env, struct iked_sa *sa, } if ((len = ikev2_add_proposals(env, sa, e, &sa->sa_proposals, - protoid, 1, 0)) == -1) + protoid, 1, 0, 0)) == -1) goto done; if (ikev2_next_payload(pld, len, IKEV2_PAYLOAD_NONCE) == -1) @@ -3114,7 +3134,7 @@ ikev2_ike_sa_rekey(struct iked *env, void *arg) /* just reuse the old IKE SA proposals */ if ((len = ikev2_add_proposals(env, nsa, e, &sa->sa_proposals, - IKEV2_SAPROTO_IKE, 1, 1)) == -1) + IKEV2_SAPROTO_IKE, 1, 1, 0)) == -1) goto done; if (ikev2_next_payload(pld, len, IKEV2_PAYLOAD_NONCE) == -1) @@ -3754,7 +3774,7 @@ ikev2_resp_create_child_sa(struct iked *env, struct iked_message *msg) if ((len = ikev2_add_proposals(env, nsa ? nsa : sa, e, nsa ? &nsa->sa_proposals : &proposals, - protoid, 0, nsa ? 1 : 0)) == -1) + protoid, 0, nsa ? 1 : 0, 0)) == -1) goto done; if (ikev2_next_payload(pld, len, IKEV2_PAYLOAD_NONCE) == -1)