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 <mdw@distorted.org.uk>

diff --git a/dh.c b/dh.c
index 0b5e4d2..7a25ee1 100644 (file)
--- a/dh.c
+++ b/dh.c
@@ -42,10 +42,10 @@ struct dh {
     MP_INT p,g; /* prime modulus and generator */
 };
 
     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;
 {
     struct dh *st=sst;

-    string_t r;

     MP_INT a, b; /* a is secret key, b is public key */
 
     mpz_init(&a);
     MP_INT a, b; /* a is secret key, b is public key */
 
     mpz_init(&a);


@@ -55,17 +55,19 @@ static string_t dh_makepublic(void *sst, uint8_t *secret, int32_t secretlen)
 
     mpz_powm_sec(&b, &st->g, &a, &st->p);
 
 
     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);
 
     mpz_clear(&a);
     mpz_clear(&b);

-    return r;
+
+    return strlen(public);

 }
 
 static dh_makeshared_fn dh_makeshared;
 static bool_t dh_makeshared(void *sst, uint8_t *secret, int32_t secretlen,
 }
 
 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;
 {
     struct dh *st=sst;
     MP_INT a, b, c;


@@ -75,7 +77,7 @@ static bool_t dh_makeshared(void *sst, uint8_t *secret, int32_t secretlen,
     mpz_init(&c);
 
     read_mpbin(&a, secret, 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);
 
 
     mpz_powm_sec(&c, &b, &a, &st->p);
 


@@ -170,6 +172,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. */
 
     /* 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
     if (!dict)
        st->ops.capab_bit = CAPAB_BIT_TRADZP;
     else

diff --git a/secnet-wireshark.lua b/secnet-wireshark.lua
index 5166918..2a6761d 100644 (file)
--- 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.
 
 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)
 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
     },
       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
     },
     ["secnet.kx.sig"] = {
       name = "Sender's signature", type = ftypes.NONE

diff --git a/secnet.h b/secnet.h
index 554df4b..cda7f1d 100644 (file)
--- a/secnet.h
+++ b/secnet.h
@@ -611,26 +611,30 @@ struct netlink_if {
 
 /* DH interface */
 
 
 /* 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 */
                                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;
     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 4da99e3..3971e68 100644 (file)
--- a/site.c
+++ b/site.c
@@ -534,7 +534,7 @@ struct msg {
     uint8_t *nR;
     uint8_t *nL;
     int32_t pklen;
     uint8_t *nR;
     uint8_t *nL;
     int32_t pklen;

-    char *pk;
+    uint8_t *pk;

     int32_t hashlen;
     int32_t siglen;
     char *sig;
     int32_t hashlen;
     int32_t siglen;
     char *sig;


@@ -550,7 +550,7 @@ static int32_t wait_timeout(struct site *st) {
     return t;
 }
 
     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;
 
 {
     _Bool ok;
 


@@ -558,9 +558,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");
     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,
     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;
                                   st->sharedsecret,
                                   st->chosen_dh->shared_len))
        return False;


@@ -616,7 +618,10 @@ static bool_t generate_msg(struct site *st, uint32_t type, cstring_t what)
 {
     void *hst;
     uint8_t *hash;
 {
     void *hst;
     uint8_t *hash;

-    string_t dhpub, sig;
+    string_t sig;
+    uint8_t *pklen_addr;
+    int32_t pklen;
+    void *pk;

     unsigned minor;
 
     st->retries=st->setup_retries;
     unsigned minor;
 
     st->retries=st->setup_retries;


@@ -654,10 +659,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);
 
        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);
                                    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);

     hash=safe_malloc(st->hash->len, "generate_msg");
     hst=st->hash->init();
     st->hash->update(hst,st->buffer.start,st->buffer.size);
     hash=safe_malloc(st->hash->len, "generate_msg");
     hst=st->hash->init();
     st->hash->update(hst,st->buffer.start,st->buffer.size);


@@ -988,7 +996,7 @@ kind##_found:                                                               \
     generate_dhsecret(st);
 
     /* Generate the shared key and set up the transform */
     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;
 }
 
     return True;
 }


@@ -1019,7 +1027,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 */
     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;
 }
 
     return True;
 }