sig: Move unmarshalling responsibility into algorithm

Because site wants to first unpick the packet, and only later actually
check the signature, we provide two entrypoints.  The first, `unpick',
basically just computes the length.  So the result of `unpick' is
simply a note of the part of the buffer which contains the signature.

The alternative would be to have site.c handle the length, so there
would be one entrypoint `check' which would get a byte block.  This
would move complexity from the `unpick'/`check' interface to the
`sign' interface (which would have to negotiate about space).  It
would mean that for algorithms where signatures are of fixed size, we
couldn't omit the length field.

rsa.c needs to do some shenanigans: because it wants to use
mpz_set_str (for historical reasons), it needs the buffer to be
nul-terminated.  So `unpick' checks that there will be a spare byte
afterwards into which we can write the nul.  `check' writes the nul -
and puts the previous character back, so that we don't have to write
weird stuff in the algorithm api.  Doing better than this would be
turd-polishing since this algorithm is obsolete.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/rsa.c b/rsa.c
index dfbc2372f605670842382c5cbe8c1e9af742c916..49672c2511bb26a5ed38dfb841ba4b4f1ab0b755 100644 (file)
--- a/rsa.c
+++ b/rsa.c
@@ -183,9 +183,27 @@ static bool_t rsa_sign(void *sst, uint8_t *data, int32_t datalen,
     return ok;
 }
 
+static bool_t rsa_sig_unpick(void *sst, struct buffer_if *msg,
+                            struct alg_msg_data *sig)
+{
+    uint8_t *lp = buf_unprepend(msg, 2);
+    if (!lp) return False;
+    sig->siglen = get_uint16(lp);
+    sig->sigstart = buf_unprepend(msg, sig->siglen);
+    if (!sig->sigstart) return False;
+
+    /* In `rsa_sig_check' below, we assume that we can write a nul
+     * terminator following the signature.  Make sure there's enough space.
+     */
+    if (msg->start >= msg->base + msg->alloclen)
+       return False;
+
+    return True;
+}
+
 static sig_checksig_fn rsa_sig_check;
 static bool_t rsa_sig_check(void *sst, uint8_t *data, int32_t datalen,
-                           cstring_t signature)
+                           const struct alg_msg_data *sig)
 {
     struct rsapub *st=sst;
     MP_INT a, b, c;
@@ -197,7 +215,11 @@ static bool_t rsa_sig_check(void *sst, uint8_t *data, int32_t datalen,
 
     emsa_pkcs1(&st->n, &a, data, datalen);
 
-    mpz_set_str(&b, signature, 16);
+    /* Terminate signature with a '0' - already checked that this will fit */
+    int save = sig->sigstart[sig->siglen];
+    sig->sigstart[sig->siglen] = 0;
+    mpz_set_str(&b, sig->sigstart, 16);
+    sig->sigstart[sig->siglen] = save;
 
     mpz_powm(&c, &b, &st->e, &st->n);
 
@@ -223,6 +245,7 @@ static list_t *rsapub_apply(closure_t *self, struct cloc loc, dict_t *context,
     st->cl.apply=NULL;
     st->cl.interface=&st->ops;
     st->ops.st=st;
+    st->ops.unpick=rsa_sig_unpick;
     st->ops.check=rsa_sig_check;
     st->loc=loc;
 

diff --git a/secnet.h b/secnet.h
index bd4362e1400fd30608f954a193dc3f8ac01da9e5..5fafdcb5040556e355d809ed766fe2ed6b8d138e 100644 (file)
--- a/secnet.h
+++ b/secnet.h
@@ -377,6 +377,11 @@ extern init_module log_module;
 
 struct buffer_if;
 
+struct alg_msg_data {
+    uint8_t *sigstart;
+    int32_t siglen;
+};
+
 /* PURE closure requires no interface */
 
 /* RESOLVER interface */
@@ -413,10 +418,13 @@ struct random_if {
 
 /* SIGPUBKEY interface */
 
+typedef bool_t sig_unpick_fn(void *sst, struct buffer_if *msg,
+                            struct alg_msg_data *sig);
 typedef bool_t sig_checksig_fn(void *st, uint8_t *data, int32_t datalen,
-                              cstring_t signature);
+                              const struct alg_msg_data *sig);
 struct sigpubkey_if {
     void *st;
+    sig_unpick_fn *unpick;
     sig_checksig_fn *check;
 };
 

diff --git a/site.c b/site.c
index 6e5c5a67d56ed0d8b2bf8a06662298c9039a6c27..73d5b64aae560f9b244a333e78d564ed5cc2aa58 100644 (file)
--- a/site.c
+++ b/site.c
@@ -534,8 +534,7 @@ struct msg {
     int32_t pklen;
     char *pk;
     int32_t hashlen;
-    int32_t siglen;
-    char *sig;
+    struct alg_msg_data sig;
 };
 
 static int32_t wait_timeout(struct site *st) {
@@ -745,17 +744,12 @@ static bool_t unpick_msg(struct site *st, uint32_t type,
     CHECK_AVAIL(msg,m->pklen);
     m->pk=buf_unprepend(msg,m->pklen);
     m->hashlen=msg->start-m->hashstart;
-    CHECK_AVAIL(msg,2);
-    m->siglen=buf_unprepend_uint16(msg);
-    CHECK_AVAIL(msg,m->siglen);
-    m->sig=buf_unprepend(msg,m->siglen);
-    CHECK_EMPTY(msg);
 
-    /* In `process_msg3_msg4' below, we assume that we can write a nul
-     * terminator following the signature.  Make sure there's enough space.
-     */
-    if (msg->start >= msg->base + msg->alloclen)
+    if (!st->pubkey->unpick(st->pubkey->st,msg,&m->sig)) {
        return False;
+    }
+
+    CHECK_EMPTY(msg);
 
     return True;
 }
@@ -898,9 +892,9 @@ static bool_t process_msg3_msg4(struct site *st, struct msg *m)
     hst=st->hash->init();
     st->hash->update(hst,m->hashstart,m->hashlen);
     st->hash->final(hst,hash);
-    /* Terminate signature with a '0' - already checked that this will fit */
-    m->sig[m->siglen]=0;
-    if (!st->pubkey->check(st->pubkey->st,hash,st->hash->len,m->sig)) {
+    if (!st->pubkey->check(st->pubkey->st,
+                          hash,st->hash->len,
+                          &m->sig)) {
        slog(st,LOG_SEC,"msg3/msg4 signature failed check!");
        free(hash);
        return False;