From: Mark Wooding <mdw@distorted.org.uk>
Date: Sat, 29 Apr 2017 12:55:40 +0000 (+0100)
Subject: Change our view of the Diffie--Hellman closure protocol.
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ian/git?a=commitdiff_plain;h=e2b810f4afa905a7108a1d31904370f891264ef6;p=secnet.git

Change our view of the Diffie--Hellman closure protocol.

Rename `len' to `secret_len' and `ceil_len' to `shared_len', and label
these as the required private secret length and generated shared-secret
length respectively.  There's no functional change: that's what these
variables were being used for anyway.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
---

diff --git a/dh.c b/dh.c
index 6860bfa..68318ce 100644
--- a/dh.c
+++ b/dh.c
@@ -154,9 +154,9 @@ static list_t *dh_apply(closure_t *self, struct cloc loc, dict_t *context,
 	cfgfatal(loc,"diffie-hellman","generator must be less than modulus\n");
     }
 
-    st->ops.len=sz;
+    st->ops.secret_len=sz;
 
-    st->ops.ceil_len=(mpz_sizeinbase(&st->p,2)+7)/8;
+    st->ops.shared_len=(mpz_sizeinbase(&st->p,2)+7)/8;
     /* According to the docs, mpz_sizeinbase(,256) is allowed to return
      * an answer which is 1 too large.  But mpz_sizeinbase(,2) isn't. */
 
diff --git a/secnet.h b/secnet.h
index 5063e2e..35c48d7 100644
--- a/secnet.h
+++ b/secnet.h
@@ -757,17 +757,27 @@ struct netlink_if {
 
 /* DH interface */
 
-/* Returns public key as a malloced hex string */
+/* 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.
+ */
 typedef string_t dh_makepublic_fn(void *st, uint8_t *secret,
 				  int32_t secretlen);
-/* Fills buffer (up to buflen) with shared secret */
+
+/* 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 bool_t dh_makeshared_fn(void *st, uint8_t *secret,
 				int32_t secretlen, cstring_t rempublic,
 				uint8_t *sharedsecret, int32_t buflen);
+
 struct dh_if {
     void *st;
-    int32_t len; /* Approximate size of modulus in bytes */
-    int32_t ceil_len; /* Number of bytes just sufficient to contain modulus */
+    int32_t secret_len; /* Size of random secret to generate */
+    int32_t shared_len; /* Size of generated shared secret */
     dh_makepublic_fn *makepublic;
     dh_makeshared_fn *makeshared;
 };
diff --git a/site.c b/site.c
index 90db333..c377ebb 100644
--- a/site.c
+++ b/site.c
@@ -585,7 +585,7 @@ static _Bool set_new_transform(struct site *st, char *pk)
     _Bool ok;
 
     /* Make room for the shared key */
-    st->sharedsecretlen=st->chosen_transform->keylen?:st->dh->ceil_len;
+    st->sharedsecretlen=st->chosen_transform->keylen?:st->dh->shared_len;
     assert(st->sharedsecretlen);
     if (st->sharedsecretlen > st->sharedsecretallocd) {
 	st->sharedsecretallocd=st->sharedsecretlen;
@@ -595,8 +595,8 @@ static _Bool set_new_transform(struct site *st, char *pk)
     }
 
     /* Generate the shared key */
-    if (!st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->len,pk,
-			    st->sharedsecret,st->sharedsecretlen))
+    if (!st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->secret_len,
+			    pk, st->sharedsecret,st->sharedsecretlen))
 	return False;
 
     /* Set up the transform */
@@ -733,7 +733,7 @@ static bool_t generate_msg(struct site *st, uint32_t type, cstring_t what,
 	buf_append_uint8(&st->buffer,st->chosen_transform->capab_bit);
     } while (0);
 
-    dhpub=st->dh->makepublic(st->dh->st,st->dhsecret,st->dh->len);
+    dhpub=st->dh->makepublic(st->dh->st,st->dhsecret,st->dh->secret_len);
     buf_append_string(&st->buffer,dhpub);
     free(dhpub);
 
@@ -1059,7 +1059,7 @@ static bool_t generate_msg3(struct site *st, const struct msg *prompt)
 {
     /* Now we have our nonce and their nonce. Think of a secret key,
        and create message number 3. */
-    st->random->generate(st->random->st,st->dh->len,st->dhsecret);
+    st->random->generate(st->random->st,st->dh->secret_len,st->dhsecret);
     return generate_msg(st,
 			(st->remote_capabilities & CAPAB_TRANSFORM_MASK)
 			? LABEL_MSG3BIS
@@ -1165,7 +1165,7 @@ kind##_found:								\
     /* Terminate their DH public key with a '0' */
     m->pk[m->pklen]=0;
     /* Invent our DH secret key */
-    st->random->generate(st->random->st,st->dh->len,st->dhsecret);
+    st->random->generate(st->random->st,st->dh->secret_len,st->dhsecret);
 
     /* Generate the shared key and set up the transform */
     if (!set_new_transform(st,m->pk)) return False;
@@ -1727,7 +1727,7 @@ static void enter_state_run(struct site *st)
     FILLZERO(st->localN);
     FILLZERO(st->remoteN);
     dispose_transform(&st->new_transform);
-    memset(st->dhsecret,0,st->dh->len);
+    memset(st->dhsecret,0,st->dh->secret_len);
     if (st->sharedsecret) memset(st->sharedsecret,0,st->sharedsecretlen);
     set_link_quality(st);
 
@@ -2535,7 +2535,7 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
     transport_peers_clear(st,&st->peers);
     transport_peers_clear(st,&st->setup_peers);
     /* XXX mlock these */
-    st->dhsecret=safe_malloc(st->dh->len,"site:dhsecret");
+    st->dhsecret=safe_malloc(st->dh->secret_len,"site:dhsecret");
     st->sharedsecretlen=st->sharedsecretallocd=0;
     st->sharedsecret=0;