Hi,

in the final RFC 5903 the computation for the DH shared secret changed.
Instead of the full point, only the X point is included.  Unfortunately
this is a backwards incompatible change, so older ikeds won't be com-
patible with this change is committed.  Of course only if you use ECP.
Anyway, this change makes us follow the RFC correctly.

Source: https://tools.ietf.org/html/rfc5903 - 9.  Changes from RFC 4753

ok?

Patrick

diff --git a/sbin/iked/dh.c b/sbin/iked/dh.c
index a8308eec596..a3ef5f80906 100644
--- a/sbin/iked/dh.c
+++ b/sbin/iked/dh.c
@@ -38,10 +38,13 @@ int modp_create_shared(struct group *, uint8_t *, uint8_t 
*);
 /* EC2N/ECP */
 int    ec_init(struct group *);
 int    ec_getlen(struct group *);
+int    ec_secretlen(struct group *);
 int    ec_create_exchange(struct group *, uint8_t *);
 int    ec_create_shared(struct group *, uint8_t *, uint8_t *);
 
-int    ec_point2raw(struct group *, const EC_POINT *, uint8_t *, size_t);
+#define EC_POINT2RAW_FULL      0
+#define EC_POINT2RAW_XONLY     1
+int    ec_point2raw(struct group *, const EC_POINT *, uint8_t *, size_t, int);
 EC_POINT *
        ec_raw2point(struct group *, uint8_t *, size_t);
 
@@ -293,6 +296,7 @@ group_get(uint32_t id)
        case GROUP_ECP:
                group->init = ec_init;
                group->getlen = ec_getlen;
+               group->secretlen = ec_secretlen;
                group->exchange = ec_create_exchange;
                group->shared = ec_create_shared;
                break;
@@ -343,6 +347,15 @@ dh_getlen(struct group *group)
        return (group->getlen(group));
 }
 
+int
+dh_secretlen(struct group *group)
+{
+       if (group->secretlen)
+               return (group->secretlen(group));
+       else
+               return (group->getlen(group));
+}
+
 int
 dh_create_exchange(struct group *group, uint8_t *buf)
 {
@@ -450,6 +463,20 @@ ec_getlen(struct group *group)
        return ((roundup(group->spec->bits, 8) * 2) / 8);
 }
 
+/*
+ * Note that the shared secret only includes the x value:
+ *
+ * See RFC 5903, 7. ECP Key Exchange Data Formats:
+ *   The Diffie-Hellman shared secret value consists of the x value of the
+ *   Diffie-Hellman common value.
+ * See also RFC 5903, 9. Changes from RFC 4753.
+ */
+int
+ec_secretlen(struct group *group)
+{
+       return (ec_getlen(group) / 2);
+}
+
 int
 ec_create_exchange(struct group *group, uint8_t *buf)
 {
@@ -459,7 +486,7 @@ ec_create_exchange(struct group *group, uint8_t *buf)
        bzero(buf, len);
 
        return (ec_point2raw(group, EC_KEY_get0_public_key(group->ec),
-           buf, len));
+           buf, len, EC_POINT2RAW_FULL));
 }
 
 int
@@ -496,7 +523,8 @@ ec_create_shared(struct group *group, uint8_t *secret, 
uint8_t *exchange)
        if (!EC_POINT_mul(ecgroup, secretp, NULL, exchangep, privkey, NULL))
                goto done;
 
-       ret = ec_point2raw(group, secretp, secret, ec_getlen(group));
+       ret = ec_point2raw(group, secretp, secret, ec_secretlen(group),
+           EC_POINT2RAW_XONLY);
 
  done:
        if (exkey != NULL)
@@ -511,7 +539,7 @@ ec_create_shared(struct group *group, uint8_t *secret, 
uint8_t *exchange)
 
 int
 ec_point2raw(struct group *group, const EC_POINT *point,
-    uint8_t *buf, size_t len)
+    uint8_t *buf, size_t len, int mode)
 {
        const EC_GROUP  *ecgroup = NULL;
        BN_CTX          *bnctx = NULL;
@@ -528,9 +556,19 @@ ec_point2raw(struct group *group, const EC_POINT *point,
                goto done;
 
        eclen = ec_getlen(group);
-       if (len < eclen)
+       switch (mode) {
+       case EC_POINT2RAW_XONLY:
+               xlen = eclen / 2;
+               ylen = 0;
+               break;
+       case EC_POINT2RAW_FULL:
+               xlen = ylen = eclen / 2;
+               break;
+       default:
+               goto done;
+       }
+       if (len < xlen + ylen)
                goto done;
-       xlen = ylen = eclen / 2;
 
        if ((ecgroup = EC_KEY_get0_group(group->ec)) == NULL)
                goto done;
@@ -551,10 +589,12 @@ ec_point2raw(struct group *group, const EC_POINT *point,
        if (!BN_bn2bin(x, buf + xoff))
                goto done;
 
-       yoff = (ylen - BN_num_bytes(y)) + xlen;
-       bzero(buf + xlen, yoff - xlen);
-       if (!BN_bn2bin(y, buf + yoff))
-               goto done;
+       if (ylen > 0) {
+               yoff = (ylen - BN_num_bytes(y)) + xlen;
+               bzero(buf + xlen, yoff - xlen);
+               if (!BN_bn2bin(y, buf + yoff))
+                       goto done;
+       }
 
        ret = 0;
  done:
diff --git a/sbin/iked/dh.h b/sbin/iked/dh.h
index 77bb4b5ef16..7e24d4d6746 100644
--- a/sbin/iked/dh.h
+++ b/sbin/iked/dh.h
@@ -46,6 +46,7 @@ struct group {
 
        int             (*init)(struct group *);
        int             (*getlen)(struct group *);
+       int             (*secretlen)(struct group *);
        int             (*exchange)(struct group *, uint8_t *);
        int             (*shared)(struct group *, uint8_t *, uint8_t *);
 };
@@ -59,6 +60,7 @@ const struct group_id
                *group_getid(uint32_t);
 
 int             dh_getlen(struct group *);
+int             dh_secretlen(struct group *);
 int             dh_create_exchange(struct group *, uint8_t *);
 int             dh_create_shared(struct group *, uint8_t *, uint8_t *);
 
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index 045e499aaed..3b054d5c994 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -4290,19 +4290,24 @@ ikev2_sa_keys(struct iked *env, struct iked_sa *sa, 
struct ibuf *key)
        /*
         *  Generate g^ir
         */
-       if ((dhsecret = ibuf_new(NULL, dh_getlen(group))) == NULL) {
+       if ((dhsecret = ibuf_new(NULL, dh_secretlen(group))) == NULL) {
                log_debug("%s: failed to alloc dh secret", __func__);
                goto done;
        }
        if (dh_create_shared(group, dhsecret->buf,
            sa->sa_dhpeer->buf) == -1) {
                log_debug("%s: failed to get dh secret"
-                   " group %d len %d secret %zu exchange %zu", __func__,
-                   group->id, dh_getlen(group), ibuf_length(dhsecret),
-                   ibuf_length(sa->sa_dhpeer));
+                   " group %d len %d secretlen %d secret %zu exchange %zu",
+                    __func__,
+                   group->id, dh_getlen(group), dh_secretlen(group),
+                   ibuf_length(dhsecret), ibuf_length(sa->sa_dhpeer));
                goto done;
        }
 
+       log_debug("%s: DHSECRET with %zu bytes", __func__,
+           ibuf_length(dhsecret));
+       print_hex(dhsecret->buf, 0, ibuf_length(dhsecret));
+
        if (!key) {
                /*
                 * Set PRF key to generate SKEEYSEED = prf(Ni | Nr, g^ir)
@@ -4725,15 +4730,16 @@ ikev2_childsa_negotiate(struct iked *env, struct 
iked_sa *sa,
                        log_debug("%s: no dh group for pfs", __func__);
                        goto done;
                }
-               if ((dhsecret = ibuf_new(NULL, dh_getlen(group))) == NULL) {
+               if ((dhsecret = ibuf_new(NULL, dh_secretlen(group))) == NULL) {
                        log_debug("%s: failed to alloc dh secret", __func__);
                        goto done;
                }
                if (dh_create_shared(group, dhsecret->buf,
                    kex->kex_dhpeer->buf) == -1) {
                        log_debug("%s: failed to get dh secret"
-                           " group %d len %d secret %zu exchange %zu",
-                           __func__, group->id, dh_getlen(group),
+                           " group %d len %d secretlen %d secret %zu"
+                           " exchange %zu", __func__, group->id,
+                           dh_getlen(group), dh_secretlen(group),
                            ibuf_length(dhsecret),
                            ibuf_length(kex->kex_dhpeer));
                        goto done;

Reply via email to