From: Mark Wooding Date: Fri, 28 Apr 2017 21:51:36 +0000 (+0100) Subject: Adjust the DH closure protocol to handle public values as raw binary. X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ian/git?a=commitdiff_plain;h=0eb41ca6ccc1d6adf5205924e55a4ecafd53e193;p=secnet.git Adjust the DH closure protocol to handle public values as raw binary. Responsibility for hex-encoding the public value now lies with the individual DH group implementation, rather than the common site-level machinery. Signed-off-by: Mark Wooding --- diff --git a/dh.c b/dh.c index bb79b24..bf41607 100644 --- a/dh.c +++ b/dh.c @@ -42,10 +42,10 @@ struct dh { MP_INT p,g; /* prime modulus and generator */ }; -static string_t dh_makepublic(void *sst, uint8_t *secret, int32_t secretlen) +static int32_t dh_makepublic(void *sst, void *public, int32_t publiclen, + uint8_t *secret, int32_t secretlen) { struct dh *st=sst; - string_t r; MP_INT a, b; /* a is secret key, b is public key */ mpz_init(&a); @@ -55,11 +55,13 @@ static string_t dh_makepublic(void *sst, uint8_t *secret, int32_t secretlen) mpz_powm_sec(&b, &st->g, &a, &st->p); - r=write_mpstring(&b); + assert(mpz_sizeinbase(&b, 16) + 2 <= (size_t)publiclen); + mpz_get_str(public, 16, &b); mpz_clear(&a); mpz_clear(&b); - return r; + + return strlen(public); } static void write_mpbin_anomalous(MP_INT *a, uint8_t *buffer, @@ -76,8 +78,8 @@ static void write_mpbin_anomalous(MP_INT *a, uint8_t *buffer, static dh_makeshared_fn dh_makeshared; static bool_t dh_makeshared(void *sst, uint8_t *secret, int32_t secretlen, - cstring_t rempublic, uint8_t *sharedsecret, - int32_t buflen) + const void *public, int32_t publiclen, + uint8_t *sharedsecret, int32_t buflen) { struct dh *st=sst; MP_INT a, b, c; @@ -87,7 +89,7 @@ static bool_t dh_makeshared(void *sst, uint8_t *secret, int32_t secretlen, mpz_init(&c); read_mpbin(&a, secret, secretlen); - mpz_set_str(&b, rempublic, 16); + mpz_set_str(&b, public, 16); mpz_powm_sec(&c, &b, &a, &st->p); @@ -182,6 +184,8 @@ static list_t *dh_apply(closure_t *self, struct cloc loc, dict_t *context, /* According to the docs, mpz_sizeinbase(,256) is allowed to return * an answer which is 1 too large. But mpz_sizeinbase(,2) isn't. */ + st->ops.public_len=(mpz_sizeinbase(&st->p,16)+2); + if (!dict) st->ops.capab_bit = CAPAB_BIT_TRADZP; else diff --git a/secnet-wireshark.lua b/secnet-wireshark.lua index 5166918..2a6761d 100644 --- a/secnet-wireshark.lua +++ b/secnet-wireshark.lua @@ -476,7 +476,11 @@ end local function dissect_dhval(st, buf, tree, pos, sz) -- Dissect a Diffie--Hellman public value. - return dissect_lenstr(st, buf, tree, "secnet.kx.dhval", pos, sz) + local len = buf(pos, 2):uint() + local sub = tree:add(PF["secnet.kx.dhval"], buf(pos, len + 2)) + sub:add(PF["secnet.kx.dhval.len"], buf(pos, 2)); pos = pos + 2 + sub:add(PF["secnet.kx.dhval.bytes"], buf(pos, len)); pos = pos + len + return pos end local function dissect_sig(st, buf, tree, pos, sz) @@ -784,9 +788,9 @@ do name = "Sender's public Diffie--Hellman length", type = ftypes.UINT16, base = base.DEC }, - ["secnet.kx.dhval.text"] = { - name = "Sender's public Diffie--Hellman text", type = ftypes.STRING, - base = base.ASCII + ["secnet.kx.dhval.bytes"] = { + name = "Sender's public Diffie--Hellman value bytes", + type = ftypes.BYTES, base = base.SPACE }, ["secnet.kx.sig"] = { name = "Sender's signature", type = ftypes.NONE diff --git a/secnet.h b/secnet.h index 4110b4c..ecf9d7e 100644 --- a/secnet.h +++ b/secnet.h @@ -758,26 +758,30 @@ struct netlink_if { /* DH interface */ -/* Returns public key as a malloced hex string. The secretlen will be the - * secret_len reported by the closure. This operation is not expected to - * fail. +/* Write public value corresponding to the secret to the public buffer, and + * return its actual length. The size of the buffer is given in publiclen, + * which will be at least the public_len reported by the closure. The + * secretlen will be the secret_len reported by the closure. This operation + * is not expected to fail. */ -typedef string_t dh_makepublic_fn(void *st, uint8_t *secret, - int32_t secretlen); - -/* Fills buffer (up to buflen) with shared secret. The rempublic string - * comes from the remote site, and may not be acceptable, though it has been - * checked for memory-safety. The secretlen and buflen are the secret_len - * and shared_len reported by the closure, respectively. Return false on - * faliure (e.g., if the publiclen is unacceptable). +typedef int32_t dh_makepublic_fn(void *st, void *public, int32_t publiclen, + uint8_t *secret, int32_t secretlen); + +/* Fills buffer (up to buflen) with shared secret. The publiclen comes from + * the remote site, and may not be acceptable, though it has been checked for + * memory-safety. The secretlen and buflen are the secret_len and shared_len + * reported by the closure, respectively. Return false on faliure (e.g., if + * the publiclen is unacceptable). */ -typedef bool_t dh_makeshared_fn(void *st, uint8_t *secret, - int32_t secretlen, cstring_t rempublic, +typedef bool_t dh_makeshared_fn(void *st, + uint8_t *secret, int32_t secretlen, + const void *public, int32_t publiclen, uint8_t *sharedsecret, int32_t buflen); struct dh_if { void *st; int32_t secret_len; /* Size of random secret to generate */ + int32_t public_len; /* Maximum number of bytes needed for public value */ int32_t shared_len; /* Size of generated shared secret */ int capab_bit; dh_makepublic_fn *makepublic; diff --git a/site.c b/site.c index 093b76a..65f543a 100644 --- a/site.c +++ b/site.c @@ -562,7 +562,7 @@ struct msg { uint8_t *nR; uint8_t *nL; int32_t pklen; - char *pk; + uint8_t *pk; int32_t hashlen; struct alg_msg_data sig; int n_pubkeys_accepted_nom; /* may be > MAX_SIG_KEYS ! */ @@ -582,7 +582,7 @@ static int32_t wait_timeout(struct site *st) { return t; } -static _Bool set_new_transform(struct site *st, char *pk) +static _Bool set_new_transform(struct site *st, uint8_t *pk, int32_t pklen) { _Bool ok; @@ -590,9 +590,11 @@ static _Bool set_new_transform(struct site *st, char *pk) assert(!st->sharedsecret); st->sharedsecret = safe_malloc(st->chosen_dh->shared_len, "site:sharedsecret"); + pk[pklen]=0; /* clobbers the following signature length, which we've + * already copied */ if (!st->chosen_dh->makeshared(st->chosen_dh->st, st->dhsecret,st->chosen_dh->secret_len, - pk, + pk,pklen, st->sharedsecret, st->chosen_dh->shared_len)) return False; @@ -648,7 +650,9 @@ static bool_t generate_msg(struct site *st, uint32_t type, cstring_t what, const struct msg *prompt /* may be 0 for MSG1 */) { - string_t dhpub; + uint8_t *pklen_addr; + int32_t pklen; + void *pk; unsigned minor; int ki; @@ -730,10 +734,13 @@ static bool_t generate_msg(struct site *st, uint32_t type, cstring_t what, buf_append_uint8(&st->buffer,st->chosen_dh->capab_bit); } while (0); - dhpub=st->chosen_dh->makepublic(st->chosen_dh->st, + pklen_addr=buf_append(&st->buffer,2); + pk=buf_append(&st->buffer,st->chosen_dh->public_len); + pklen=st->chosen_dh->makepublic(st->chosen_dh->st, + pk,st->chosen_dh->public_len, st->dhsecret,st->chosen_dh->secret_len); - buf_append_string(&st->buffer,dhpub); - free(dhpub); + put_uint16(pklen_addr,pklen); + buf_unappend(&st->buffer,st->chosen_dh->public_len-pklen); bool_t ok=privkey->sign(privkey->st, st->buffer.start, @@ -1190,7 +1197,7 @@ kind##_found: \ generate_dhsecret(st); /* Generate the shared key and set up the transform */ - if (!set_new_transform(st,m->pk)) return False; + if (!set_new_transform(st,m->pk,m->pklen)) return False; return True; } @@ -1221,7 +1228,7 @@ static bool_t process_msg4(struct site *st, struct buffer_if *msg4, m->pk[m->pklen]=0; /* Generate the shared key and set up the transform */ - if (!set_new_transform(st,m->pk)) return False; + if (!set_new_transform(st,m->pk,m->pklen)) return False; return True; }