Add optional 'group none' transform for child SAs and fix handling of

'group none'.   We currently send no transform of type DH by default,
which should be equivalent to explicitly sending a single DH transform
of type 'none'.  However, the proposal matching logic had a bug where
these two would not match, effectively breaking the ability to negotiate
optional PFS.  This commit fixes the bug but continues to send
no DH proposal by default to remain backwards compatible with older
versions.

ok patrick@

diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index 2ea7e7b..2f37cd8 100644 (file)
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ikev2.c,v 1.303 2021/02/04 20:38:26 tobhe Exp $       */
+/*     $OpenBSD: ikev2.c,v 1.304 2021/02/09 21:35:48 tobhe Exp $       */
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -2435,12 +2435,15 @@ ikev2_add_proposals(struct iked *env, struct iked_sa *sa, struct ibuf *buf,
        uint64_t                         spi64;
        uint32_t                         spi32, spi = 0;
        unsigned int                     i, xfi, nxforms;
+       int                              prop_skipdh;
 
        TAILQ_FOREACH(prop, proposals, prop_entry) {
                if ((protoid && prop->prop_protoid != protoid) ||
                    (!protoid && prop->prop_protoid == IKEV2_SAPROTO_IKE))
                        continue;
 
+               prop_skipdh = skipdh;
+
                if (protoid != IKEV2_SAPROTO_IKE && initiator) {
                        if (spi == 0) {
                                bzero(&csa, sizeof(csa));
@@ -2471,12 +2474,28 @@ ikev2_add_proposals(struct iked *env, struct iked_sa *sa, struct ibuf *buf,
                        prop->prop_localspi.spi_protoid = IKEV2_SAPROTO_IKE;
                }
 
+               /*
+                * A single DH transform of type NONE is equivalent with
+                * not sending a DH transform at all.
+                * Prefer the latter for downwards compatibility.
+                */
+               if (protoid != IKEV2_SAPROTO_IKE) {
+                       for (i = 0; i < prop->prop_nxforms; i++) {
+                               xform = prop->prop_xforms + i;
+                               if (xform->xform_type == IKEV2_XFORMTYPE_DH &&
+                                   xform->xform_id != IKEV2_XFORMDH_NONE)
+                                       break;
+                       }
+                       if (i == prop->prop_nxforms)
+                               prop_skipdh = 1;
+               }
+
                /*
                 * RFC 7296: 1.2. The Initial Exchanges
                 * IKE_AUTH messages do not contain KE/N payloads, thus
                 * SA payloads cannot contain groups.
                 */
-               if (skipdh) {
+               if (prop_skipdh) {
                        nxforms = 0;
                        for (i = 0; i < prop->prop_nxforms; i++) {
                                xform = prop->prop_xforms + i;
@@ -2514,7 +2533,7 @@ ikev2_add_proposals(struct iked *env, struct iked_sa *sa, struct ibuf *buf,
                for (i = 0, xfi = 0; i < prop->prop_nxforms; i++) {
                        xform = prop->prop_xforms + i;
 
-                       if (skipdh && xform->xform_type == IKEV2_XFORMTYPE_DH)
+                       if (prop_skipdh && xform->xform_type == IKEV2_XFORMTYPE_DH)
                                continue;
 
                        if ((xflen = ikev2_add_transform(buf,
@@ -3826,6 +3845,7 @@ ikev2_send_create_child_sa(struct iked *env, struct iked_sa *sa,
 {
        struct iked_policy              *pol = sa->sa_policy;
        struct iked_childsa             *csa = NULL, *csb = NULL;
+       struct iked_transform           *xform;
        struct ikev2_notify             *n;
        struct ikev2_payload            *pld = NULL;
        struct ikev2_keyexchange        *ke;
@@ -3918,8 +3938,8 @@ ikev2_send_create_child_sa(struct iked *env, struct iked_sa *sa,
                goto done;
        len = ibuf_size(nonce);
 
-       if (config_findtransform(&pol->pol_proposals, IKEV2_XFORMTYPE_DH,
-           protoid)) {
+       if ((xform = config_findtransform(&pol->pol_proposals, IKEV2_XFORMTYPE_DH,
+           protoid)) && group_get(xform->xform_id) != IKEV2_XFORMDH_NONE) {
                log_debug("%s: enable PFS", __func__);
                ikev2_sa_cleanup_dh(sa);
                if (proposed_group) {

diff --git a/sbin/iked/parse.y b/sbin/iked/parse.y
index ff6fa16..e9a57c9 100644 (file)
--- a/sbin/iked/parse.y
+++ b/sbin/iked/parse.y
@@ -1,4 +1,4 @@
-/*     $OpenBSD: parse.y,v 1.126 2021/02/07 00:51:53 tobhe Exp $       */
+/*     $OpenBSD: parse.y,v 1.127 2021/02/09 21:35:48 tobhe Exp $       */
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -192,6 +192,7 @@ struct iked_transform ikev2_default_esp_transforms[] = {
        { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_384_192 },
        { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_512_256 },
        { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 },
+       { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_NONE },
        { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_ESN },
        { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_NONE },
        { 0 }
@@ -202,6 +203,7 @@ size_t ikev2_default_nesp_transforms = ((sizeof(ikev2_default_esp_transforms) /
 struct iked_transform ikev2_default_esp_transforms_noauth[] = {
        { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_GCM_16, 128 },
        { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_GCM_16, 256 },
+       { IKEV2_XFORMTYPE_DH,   IKEV2_XFORMDH_NONE },
        { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_ESN },
        { IKEV2_XFORMTYPE_ESN,  IKEV2_XFORMESN_NONE },
        { 0 }
@@ -267,6 +269,7 @@ const struct ipsec_xf ipsecencxfs[] = {
 };
 
 const struct ipsec_xf groupxfs[] = {
+       { "none",               IKEV2_XFORMDH_NONE },
        { "modp768",            IKEV2_XFORMDH_MODP_768 },
        { "grp1",               IKEV2_XFORMDH_MODP_768 },
        { "modp1024",           IKEV2_XFORMDH_MODP_1024 },
@@ -2840,6 +2843,13 @@ create_ike(char *name, int af, uint8_t ipproto,
                                else
                                        auth++;
                        }
+                       for (j = 0; j < ike_sa->xfs[i]->ngroupxf; j++) {
+                               if (ike_sa->xfs[i]->groupxf[j]->id
+                                   == IKEV2_XFORMDH_NONE) {
+                                       yyerror("IKE group can not be \"none\".");
+                                       goto done;
+                               }
+                       }
                        if (ike_sa->xfs[i]->nauthxf)
                                auth++;
 

diff --git a/sbin/iked/policy.c b/sbin/iked/policy.c
index df7f267..d312980 100644 (file)
--- a/sbin/iked/policy.c
+++ b/sbin/iked/policy.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: policy.c,v 1.75 2021/02/01 16:37:48 tobhe Exp $       */
+/*     $OpenBSD: policy.c,v 1.76 2021/02/09 21:35:48 tobhe Exp $       */
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -1009,7 +1009,8 @@ proposals_match(struct iked_proposal *local, struct iked_proposal *peer,
                         */
                        if (rekey && requiredh == 0 &&
                            protoid == IKEV2_SAPROTO_ESP &&
-                           tlocal->xform_type == IKEV2_XFORMTYPE_DH)
+                           tlocal->xform_type == IKEV2_XFORMTYPE_DH &&
+                           tlocal->xform_id != IKEV2_XFORMDH_NONE)
                                requiredh = 1;
 
                        /* Compare peer and local proposals */