The following reply was made to PR system/6601; it has been noted by GNATS.
From: Mike Belopuhov <[email protected]> To: [email protected] Cc: Subject: system/6601 Date: Thu, 9 Jun 2011 17:23:02 +0200 The problem is that BN_bn2bin skips leading zeroes when it converts a bignum to the char array and we have to prepend zeroes ourselves. Otherwise peers will compute different DH keys. The following diff seems to fix an issue. I've been running a tunnel for three days and got 0% ping loss, Pawel has similar results. Index: dh.c =================================================================== RCS file: /home/cvs/src/sbin/isakmpd/dh.c,v retrieving revision 1.13 diff -u -p -u -p -r1.13 dh.c --- dh.c 29 Nov 2010 22:49:26 -0000 1.13 +++ dh.c 7 Jun 2011 15:13:13 -0000 @@ -402,12 +402,22 @@ int modp_create_exchange(struct group *group, u_int8_t *buf) { DH *dh = group->dh; + int len, ret; if (!DH_generate_key(dh)) return (-1); - if (!BN_bn2bin(dh->pub_key, buf)) + ret = BN_bn2bin(dh->pub_key, buf); + if (!ret) return (-1); + len = dh_getlen(group); + + /* add zero padding */ + if (ret < len) { + bcopy(buf, buf + (len - ret), ret); + bzero(buf, len - ret); + } + return (0); } @@ -415,15 +425,23 @@ int modp_create_shared(struct group *group, u_int8_t *secret, u_int8_t *exchange) { BIGNUM *ex; - int ret; + int len, ret; - if ((ex = BN_bin2bn(exchange, dh_getlen(group), NULL)) == NULL) + len = dh_getlen(group); + + if ((ex = BN_bin2bn(exchange, len, NULL)) == NULL) return (-1); ret = DH_compute_key(secret, ex, group->dh); BN_clear_free(ex); if (!ret) return (-1); + + /* add zero padding */ + if (ret < len) { + bcopy(secret, secret + (len - ret), ret); + bzero(secret, len - ret); + } return (0); }
