[PATCH 5/8] transform: Allow DH to set the key size

Ian Jackson ijackson at chiark.greenend.org.uk
Fri Jul 19 01:25:16 BST 2013


It turns out that the current Serpent CBC-MAC transform takes the raw
DH shared secret, and parcels it up in byte ranges for the various
uses (some of which are published).  Well, obviously this is not a
good idea.

But also it means the interface isn't set up to allow the size of the
key data provided to the transform to be determined by the size of the
DH modulus.

Fix this latter interface problem.  Now a transform can set its keylen
to 0, meaning it will be provided with the whole of the DH private
value.

We don't use this new feature yet.  We can't make the existing
transform use it without breaking compatibility, but it will be used
by the new EAX-based transform.

Signed-off-by: Ian Jackson <ijackson at chiark.greenend.org.uk>
---
 dh.c     |    4 ++++
 secnet.h |    3 ++-
 site.c   |   14 ++++++++------
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/dh.c b/dh.c
index 2383192..c37b538 100644
--- a/dh.c
+++ b/dh.c
@@ -125,6 +125,10 @@ static list_t *dh_apply(closure_t *self, struct cloc loc, dict_t *context,
 
     st->ops.len=sz;
 
+    st->ops.ceil_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. */
+
     return new_closure(&st->cl);
 }
 
diff --git a/secnet.h b/secnet.h
index 1a5aaad..23d62ba 100644
--- a/secnet.h
+++ b/secnet.h
@@ -412,7 +412,7 @@ struct transform_if {
     void *st;
     int32_t max_start_pad; /* these three are all <<< INT_MAX */
     int32_t max_end_pad;
-    int32_t keylen;
+    int32_t keylen; /* 0 means give the transform exactly as much as there is */
     transform_createinstance_fn *create;
 };
 
@@ -459,6 +459,7 @@ typedef void dh_makeshared_fn(void *st, uint8_t *secret,
 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 */
     dh_makepublic_fn *makepublic;
     dh_makeshared_fn *makeshared;
 };
diff --git a/site.c b/site.c
index e96d6f4..12b2df2 100644
--- a/site.c
+++ b/site.c
@@ -288,6 +288,7 @@ struct site {
     uint64_t timeout; /* Timeout for current state */
     uint8_t *dhsecret;
     uint8_t *sharedsecret;
+    uint32_t sharedsecretlen;
     struct transform_inst_if *new_transform; /* For key setup/verify */
 };
 
@@ -561,11 +562,11 @@ static bool_t process_msg3(struct site *st, struct buffer_if *msg3,
 
     /* Generate the shared key */
     st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->len,m.pk,
-		       st->sharedsecret,st->transform->keylen);
+		       st->sharedsecret,st->sharedsecretlen);
 
     /* Set up the transform */
     st->new_transform->setkey(st->new_transform->st,st->sharedsecret,
-			      st->transform->keylen);
+			      st->sharedsecretlen);
 
     return True;
 }
@@ -609,10 +610,10 @@ static bool_t process_msg4(struct site *st, struct buffer_if *msg4,
     m.pk[m.pklen]=0;
     /* Generate the shared key */
     st->dh->makeshared(st->dh->st,st->dhsecret,st->dh->len,m.pk,
-		       st->sharedsecret,st->transform->keylen);
+		       st->sharedsecret,st->sharedsecretlen);
     /* Set up the transform */
     st->new_transform->setkey(st->new_transform->st,st->sharedsecret,
-			      st->transform->keylen);
+			      st->sharedsecretlen);
 
     return True;
 }
@@ -1009,7 +1010,7 @@ static void enter_state_run(struct site *st)
     memset(st->remoteN,0,NONCELEN);
     st->new_transform->delkey(st->new_transform->st);
     memset(st->dhsecret,0,st->dh->len);
-    memset(st->sharedsecret,0,st->transform->keylen);
+    memset(st->sharedsecret,0,st->sharedsecretlen);
     set_link_quality(st);
 }
 
@@ -1557,7 +1558,8 @@ static list_t *site_apply(closure_t *self, struct cloc loc, dict_t *context,
     transport_peers_clear(st,&st->setup_peers);
     /* XXX mlock these */
     st->dhsecret=safe_malloc(st->dh->len,"site:dhsecret");
-    st->sharedsecret=safe_malloc(st->transform->keylen,"site:sharedsecret");
+    st->sharedsecretlen=st->transform->keylen?:st->dh->ceil_len;
+    st->sharedsecret=safe_malloc(st->sharedsecretlen,"site:sharedsecret");
 
     /* We need to compute some properties of our comms */
 #define COMPUTE_WORST(pad)			\
-- 
1.7.2.5




More information about the sgo-software-discuss mailing list