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.  Of course only if you use ECP.  Anyway, this
change makes us follow the RFC correctly.

ok markus@

3 files changed, 68 insertions, 20 deletions

diff --git a/sbin/iked/dh.c b/sbin/iked/dh.c
index a8308eec596..0a2a0244260 100644
--- a/sbin/iked/dh.c
+++ b/sbin/iked/dh.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dh.c,v 1.20 2017/05/21 02:37:52 deraadt Exp $	*/
+/*	$OpenBSD: dh.c,v 1.21 2017/10/27 14:26:35 patrick Exp $	*/
 
 /*
  * Copyright (c) 2010-2014 Reyk Floeter <reyk@openbsd.org>
@@ -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;
@@ -344,6 +348,15 @@ dh_getlen(struct group *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)
 {
 	return (group->exchange(group, 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..124fa73b6ac 100644
--- a/sbin/iked/dh.h
+++ b/sbin/iked/dh.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dh.h,v 1.10 2017/03/27 17:17:49 mikeb Exp $	*/
+/*	$OpenBSD: dh.h,v 1.11 2017/10/27 14:26:35 patrick Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -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..706f9ebbe1d 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ikev2.c,v 1.155 2017/06/01 15:23:43 sthen Exp $	*/
+/*	$OpenBSD: ikev2.c,v 1.156 2017/10/27 14:26:35 patrick Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -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;