Hi,
Honor RFC 5903 in isakmpd(8) as well. The member g_xy is always the
shared secret, but so far it has always been using the length of public
points. Since this is different now, as the shared secret for EC Groups
should only store the x point, we need another member to specify the
length of g_xy.
Note that this change breaks backwards compatibility to older peers that
don't run this change.
Similar change has been done to iked(8) recently.
OKs? Concerns?
Patrick
diff --git a/sbin/isakmpd/dh.c b/sbin/isakmpd/dh.c
index a0da694b5fb..5d3fcfaa7d0 100644
--- a/sbin/isakmpd/dh.c
+++ b/sbin/isakmpd/dh.c
@@ -36,10 +36,13 @@ int modp_create_shared(struct group *, u_int8_t *, u_int8_t
*);
int ec_init(struct group *);
int ec_getlen(struct group *);
+int ec_secretlen(struct group *);
int ec_create_exchange(struct group *, u_int8_t *);
int ec_create_shared(struct group *, u_int8_t *, u_int8_t *);
-int ec_point2raw(struct group *, const EC_POINT *, u_int8_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 *, u_int8_t *, size_t);
@@ -277,6 +280,7 @@ group_get(u_int32_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;
@@ -305,6 +309,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, u_int8_t *buf)
{
@@ -412,6 +425,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, u_int8_t *buf)
{
@@ -421,7 +448,7 @@ ec_create_exchange(struct group *group, u_int8_t *buf)
bzero(buf, len);
return (ec_point2raw(group, EC_KEY_get0_public_key(group->ec),
- buf, len));
+ buf, len, EC_POINT2RAW_FULL));
}
int
@@ -458,7 +485,8 @@ ec_create_shared(struct group *group, u_int8_t *secret,
u_int8_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)
@@ -473,7 +501,7 @@ ec_create_shared(struct group *group, u_int8_t *secret,
u_int8_t *exchange)
int
ec_point2raw(struct group *group, const EC_POINT *point,
- u_int8_t *buf, size_t len)
+ u_int8_t *buf, size_t len, int mode)
{
const EC_GROUP *ecgroup = NULL;
BN_CTX *bnctx = NULL;
@@ -490,9 +518,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;
@@ -513,10 +551,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/isakmpd/dh.h b/sbin/isakmpd/dh.h
index e04fdd90476..b258479fbf6 100644
--- a/sbin/isakmpd/dh.h
+++ b/sbin/isakmpd/dh.h
@@ -43,6 +43,7 @@ struct group {
int (*init)(struct group *);
int (*getlen)(struct group *);
+ int (*secretlen)(struct group *);
int (*exchange)(struct group *, u_int8_t *);
int (*shared)(struct group *, u_int8_t *, u_int8_t *);
};
@@ -54,6 +55,7 @@ void group_free(struct group *);
struct group *group_get(u_int32_t);
int dh_getlen(struct group *);
+int dh_secretlen(struct group *);
int dh_create_exchange(struct group *, u_int8_t *);
int dh_create_shared(struct group *, u_int8_t *, u_int8_t *);
diff --git a/sbin/isakmpd/ike_auth.c b/sbin/isakmpd/ike_auth.c
index d617b743349..3a201953a74 100644
--- a/sbin/isakmpd/ike_auth.c
+++ b/sbin/isakmpd/ike_auth.c
@@ -469,10 +469,10 @@ sig_gen_skeyid(struct exchange *exchange, size_t *sz)
LOG_DBG((LOG_NEGOTIATION, 80, "sig_gen_skeyid: g^xy length %lu",
(unsigned long)ie->g_x_len));
LOG_DBG_BUF((LOG_NEGOTIATION, 80,
- "sig_gen_skeyid: SKEYID fed with g^xy", ie->g_xy, ie->g_x_len));
+ "sig_gen_skeyid: SKEYID fed with g^xy", ie->g_xy, ie->g_xy_len));
prf->Init(prf->prfctx);
- prf->Update(prf->prfctx, ie->g_xy, ie->g_x_len);
+ prf->Update(prf->prfctx, ie->g_xy, ie->g_xy_len);
prf->Final(skeyid, prf->prfctx);
prf_free(prf);
return skeyid;
diff --git a/sbin/isakmpd/ike_phase_1.c b/sbin/isakmpd/ike_phase_1.c
index 21ff27727b8..a4544623fd6 100644
--- a/sbin/isakmpd/ike_phase_1.c
+++ b/sbin/isakmpd/ike_phase_1.c
@@ -606,11 +606,12 @@ ike_phase_1_post_exchange_KE_NONCE(struct message *msg)
enum cryptoerr err;
/* Compute Diffie-Hellman shared value. */
- ie->g_xy = malloc(ie->g_x_len);
+ ie->g_xy_len = dh_secretlen(ie->group);
+ ie->g_xy = malloc(ie->g_xy_len);
if (!ie->g_xy) {
/* XXX How to notify peer? */
log_error("ike_phase_1_post_exchange_KE_NONCE: "
- "malloc (%lu) failed", (unsigned long)ie->g_x_len);
+ "malloc (%lu) failed", (unsigned long)ie->g_xy_len);
return -1;
}
if (dh_create_shared(ie->group, ie->g_xy,
@@ -621,7 +622,7 @@ ike_phase_1_post_exchange_KE_NONCE(struct message *msg)
}
LOG_DBG_BUF((LOG_NEGOTIATION, 80,
"ike_phase_1_post_exchange_KE_NONCE: g^xy", ie->g_xy,
- ie->g_x_len));
+ ie->g_xy_len));
/* Compute the SKEYID depending on the authentication method. */
ie->skeyid = ie->ike_auth->gen_skeyid(exchange, &ie->skeyid_len);
@@ -647,7 +648,7 @@ ike_phase_1_post_exchange_KE_NONCE(struct message *msg)
return -1;
}
prf->Init(prf->prfctx);
- prf->Update(prf->prfctx, ie->g_xy, ie->g_x_len);
+ prf->Update(prf->prfctx, ie->g_xy, ie->g_xy_len);
prf->Update(prf->prfctx, exchange->cookies, ISAKMP_HDR_COOKIES_LEN);
prf->Update(prf->prfctx, (unsigned char *)"\0", 1);
prf->Final(ie->skeyid_d, prf->prfctx);
@@ -665,7 +666,7 @@ ike_phase_1_post_exchange_KE_NONCE(struct message *msg)
}
prf->Init(prf->prfctx);
prf->Update(prf->prfctx, ie->skeyid_d, ie->skeyid_len);
- prf->Update(prf->prfctx, ie->g_xy, ie->g_x_len);
+ prf->Update(prf->prfctx, ie->g_xy, ie->g_xy_len);
prf->Update(prf->prfctx, exchange->cookies, ISAKMP_HDR_COOKIES_LEN);
prf->Update(prf->prfctx, (unsigned char *)"\1", 1);
prf->Final(ie->skeyid_a, prf->prfctx);
@@ -684,7 +685,7 @@ ike_phase_1_post_exchange_KE_NONCE(struct message *msg)
}
prf->Init(prf->prfctx);
prf->Update(prf->prfctx, ie->skeyid_a, ie->skeyid_len);
- prf->Update(prf->prfctx, ie->g_xy, ie->g_x_len);
+ prf->Update(prf->prfctx, ie->g_xy, ie->g_xy_len);
prf->Update(prf->prfctx, exchange->cookies, ISAKMP_HDR_COOKIES_LEN);
prf->Update(prf->prfctx, (unsigned char *)"\2", 1);
prf->Final(ie->skeyid_e, prf->prfctx);
diff --git a/sbin/isakmpd/ike_quick_mode.c b/sbin/isakmpd/ike_quick_mode.c
index 20a87e23fbc..52749014b6b 100644
--- a/sbin/isakmpd/ike_quick_mode.c
+++ b/sbin/isakmpd/ike_quick_mode.c
@@ -1398,9 +1398,9 @@ post_quick_mode(struct message *msg)
LOG_DBG_BUF((LOG_NEGOTIATION,
90, "post_quick_mode: "
"g^xy", ie->g_xy,
- ie->g_x_len));
+ ie->g_xy_len));
prf->Update(prf->prfctx,
- ie->g_xy, ie->g_x_len);
+ ie->g_xy, ie->g_xy_len);
}
LOG_DBG((LOG_NEGOTIATION, 90,
"post_quick_mode: "
@@ -1902,10 +1902,11 @@ gen_g_xy(struct message *msg)
struct ipsec_exch *ie = exchange->data;
/* Compute Diffie-Hellman shared value. */
- ie->g_xy = malloc(ie->g_x_len);
+ ie->g_xy_len = dh_secretlen(ie->group);
+ ie->g_xy = malloc(ie->g_xy_len);
if (!ie->g_xy) {
log_error("gen_g_xy: malloc (%lu) failed",
- (unsigned long)ie->g_x_len);
+ (unsigned long)ie->g_xy_len);
return;
}
if (dh_create_shared(ie->group, ie->g_xy,
@@ -1914,7 +1915,7 @@ gen_g_xy(struct message *msg)
return;
}
LOG_DBG_BUF((LOG_NEGOTIATION, 80, "gen_g_xy: g^xy", ie->g_xy,
- ie->g_x_len));
+ ie->g_xy_len));
}
static int
diff --git a/sbin/isakmpd/ipsec.h b/sbin/isakmpd/ipsec.h
index 1c480017e08..3927140c863 100644
--- a/sbin/isakmpd/ipsec.h
+++ b/sbin/isakmpd/ipsec.h
@@ -73,6 +73,7 @@ struct ipsec_exch {
/* Diffie-Hellman values. */
size_t g_x_len;
+ size_t g_xy_len;
u_int8_t *g_xi;
u_int8_t *g_xr;
u_int8_t *g_xy;
