ok
2017-10-24 16:25 GMT+02:00 Patrick Wildt <[email protected]>:
> 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;
>
