rsa: Bring hash selection in-house

In
  13b8fbf4548f3457b02afd36e9284d39839d6f85
  sig: Move hashing into algorithm
we introduced a scheme were for rsa1 the hash function is stored
in the signature scheme's key structure, but provided by the caller.

The intent was to allow defaulting, with context-specific overrides.
However, this does not work correctly.  In particular, most sites
have a single "local-key" setting at the top level in the main config,
but take "hash" keys from the sites file.

The result is that as the various sites are initialised, ->sethash is
called multiple times, once for each site.  Possibly with different
hash_if's.  I did not foresee this and it is clearly wrong.

If all the hash_if's are sha1 then this is harmless.  However, they
might not be, in particular if certain site(s) or vpn(s) in the sites
file(s) specify a different hash.  Such a configuration would be
rather wrong, because it would imply reuse of the same raw RSA key
material with a different hash function.  (Also since the default hash
is sha1 and historically the only alternative was md5, this is surely
wrong simply because it implies md5 is being used somewhere.)

But it has come to my attention that such installations exist.  Even a
non-operational, vestigial, use of a different hash, can cause
lossage.

To fix this properly and allow hash-agility with a single private key,
we would have to have call sites continue to look up the hash, but to
pass in into the signature function.  This is too annoying,
particularly when it is in support only of unreasonable and very old
configurations.

Instead, change the semantics so that the two rsa closure verbs nail
down their hash at key load time, defaulting to sha1.  The "hash"
config key is now looked up sort of implicitly in the context.  This
is slightly odd, but it has roughly the right effect with sites.conf
files generated by make-secnet-sites.  And it is contained within the
rsa1 signature scheme which is a thing we should be replacing anyway.

This change makes it more clearly impossible (as it has, in fact, been
since 0.4.x) to use the same loaded private key with different hashes.

Installations which are only using sha1 with their rsa1 will just keep
working an all is well.

Installations which are using md5 everywhere can be made to work by
adding a global config hash= setting in every instance.

Installations which are using a mixture have a more complicated task
to keep things working (maybe loading the key twice, or propagating
hash information in sites files, or something), if they don't want a
flag day transition to sha1.

In the future for rsa1, what hash a site is using becomes a property
which should be carried with public key; so a non-sha1 hash must be
specified in the config file (alongside `local-key') and also
documented in the sites file entry.

For forthcoming non-rsa1 algorithms hash choice will be handled within
the signature scheme in a less irregular way, and this "hash" key will
thereby become obsolete.

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

diff --git a/README b/README
index e23701c9151dc4d946523914dbbf81b2a69251fa..200da3742ba9ef91d7b4c81273c22184e02883ca 100644 (file)
--- a/README
+++ b/README
@@ -403,8 +403,6 @@ site: dict argument
   key (sigpubkey closure): our peer's public key (obsolete)
   transform (transform closure): how to mangle packets sent between sites
   dh (dh closure)
-  hash (hash closure): used for keys whose algorithm (or public
-    or private key file) does not imply the hash function
   key-lifetime (integer): max lifetime of a session key, in ms
     [one hour; mobile: 2 days]
   setup-retries (integer): max number of times to transmit a key negotiation
@@ -606,6 +604,11 @@ rsa-public: string,string
   arg1: encryption key (decimal)
   arg2: modulus (decimal)
 
+The sigscheme is hardcoded to use sha1.  Both rsa-private and
+rsa-public look for the following config key in their context:
+  hash (hash closure): hash function [sha1]
+
+
 ** dh
 
 Defines:

diff --git a/rsa.c b/rsa.c
index 145df5e464fc98b431534fed0dc49e2ff614f65a..b30fa01a5b10faf5bdf45588280c88b12ad76149 100644 (file)
--- a/rsa.c
+++ b/rsa.c
@@ -128,23 +128,22 @@ struct rsapub {
 
 static const char *hexchars="0123456789abcdef";
 
-static void rsa_sethash(struct rsacommon *c, struct hash_if *hash,
+static void rsa_sethash(struct load_ctx *l,
+                       struct rsacommon *c,
                        const struct hash_if **in_ops)
 {
-    free(c->hashbuf);
+    struct hash_if *hash=0;
+    if (l->deprdict)
+       hash=find_cl_if(l->deprdict,"hash",CL_HASH,False,"site",l->loc);
+    if (!hash)
+       hash=sha1_hash_if;
     c->hashbuf=safe_malloc(hash->hlen, "generate_msg");
     *in_ops=hash;
 }
-static void rsa_pub_sethash(void *sst, struct hash_if *hash)
-{
-    struct rsapub *st=sst;
-    rsa_sethash(&st->common, hash, &st->ops.hash);
-}
-static void rsa_priv_sethash(void *sst, struct hash_if *hash)
-{
-    struct rsapriv *st=sst;
-    rsa_sethash(&st->common, hash, &st->ops.hash);
-}
+
+static void rsa_pub_sethash(void *sst, struct hash_if *hash) { }
+static void rsa_priv_sethash(void *sst, struct hash_if *hash) { }
+
 static void rsacommon_dispose(struct rsacommon *c)
 {
     free(c->hashbuf);
@@ -365,6 +364,8 @@ static struct rsapub *rsa_loadpub_core(RSAPUB_BNS(RSAPUB_LOADCORE_DEFBN)
 
     RSAPUB_BNS(RSAPUB_LOADCORE_GETBN)
 
+    rsa_sethash(l,&st->common,&st->ops.hash);
+
     return st;
 
  error_out:
@@ -635,6 +636,8 @@ static struct rsapriv *rsa_loadpriv_core(struct load_ctx *l,
        fatal_perror("rsa-private (%s:%d): ferror",loc.file,loc.line);
     }
 
+    rsa_sethash(l,&st->common,&st->ops.hash);
+
     /*
      * Now verify the validity of the key, and set up the auxiliary
      * values for fast CRT signing.