Add checking for signature hashes

  Signatures include the first 2 octets of the hash the signature is
  over. Checking this matches what we expect is an easy way to drop
  corrupt or incorrect signatures. It doesn't provide any cryptographic
  verification but is a useful sanity check when accepting keys.

diff --git a/Makefile.in b/Makefile.in
index 2163b1a271257490e360fc9e21f84a4b014676bc..403a2704219c2f9dd08481caff07e12e808431cc 100644 (file)
--- a/Makefile.in
+++ b/Makefile.in
@@ -18,14 +18,14 @@ exec_prefix ?= @exec_prefix@
 
 PROGS = add lookup hashquery gpgwww onak splitkeys onak-mail.pl stripkey
 CORE_OBJS = armor.o charfuncs.o decodekey.o getcgi.o hash.o marshal.o \
 
 PROGS = add lookup hashquery gpgwww onak splitkeys onak-mail.pl stripkey
 CORE_OBJS = armor.o charfuncs.o decodekey.o getcgi.o hash.o marshal.o \

-       keyid.o keyindex.o ll.o mem.o onak-conf.o parsekey.o \
+       keyid.o keyindex.o ll.o mem.o onak-conf.o parsekey.o sigcheck.o \

        log.o photoid.o wordlist.o cleanup.o merge.o sendsync.o keyarray.o
 ifeq (x@NETTLE_LIBS@, x)
 CORE_OBJS += md5.o sha1.o
 endif
 SRCS = armor.c parsekey.c merge.c keyid.c md5.c sha1.c main.c getcgi.c mem.c \
        keyindex.c stats.c lookup.c add.c keydb_$(DBTYPE).c ll.c hash.c \
        log.o photoid.o wordlist.o cleanup.o merge.o sendsync.o keyarray.o
 ifeq (x@NETTLE_LIBS@, x)
 CORE_OBJS += md5.o sha1.o
 endif
 SRCS = armor.c parsekey.c merge.c keyid.c md5.c sha1.c main.c getcgi.c mem.c \
        keyindex.c stats.c lookup.c add.c keydb_$(DBTYPE).c ll.c hash.c \

-       gpgwww.c onak-conf.c charfuncs.c sendsync.c log.c photoid.c \
+       gpgwww.c onak-conf.c charfuncs.c sendsync.c log.c photoid.c sigcheck.c \

        wordlist.c cleankey.c cleanup.c keyarray.c hashquery.c marshal.c \
        $(foreach be,@BACKENDS@,keydb_$(be).c)
 PROGS_LDFLAGS_EXTRA =
        wordlist.c cleankey.c cleanup.c keyarray.c hashquery.c marshal.c \
        $(foreach be,@BACKENDS@,keydb_$(be).c)
 PROGS_LDFLAGS_EXTRA =

diff --git a/cleankey.c b/cleankey.c
index d4046f3e47dd6529bdae02b78a222bacb4a2fe01..7d0bfe4b8cb16315926399f395f58b2142061aad 100644 (file)
--- a/cleankey.c
+++ b/cleankey.c
@@ -1,7 +1,7 @@
 /*
  * cleankey.c - Routines to look for common key problems and clean them up.
  *
 /*
  * cleankey.c - Routines to look for common key problems and clean them up.
  *

- * Copyright 2004 Jonathan McDowell <noodles@earth.li>
+ * Copyright 2004,2012 Jonathan McDowell <noodles@earth.li>

  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the Free
  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the Free


@@ -23,9 +23,11 @@
 
 #include "cleankey.h"
 #include "keystructs.h"
 
 #include "cleankey.h"
 #include "keystructs.h"

+#include "log.h"

 #include "mem.h"
 #include "merge.h"
 #include "mem.h"
 #include "merge.h"

-#include "log.h"
+#include "onak-conf.h"
+#include "sigcheck.h"

 
 /**
  *     dedupuids - Merge duplicate uids on a key.
 
 /**
  *     dedupuids - Merge duplicate uids on a key.


@@ -71,6 +73,65 @@ int dedupuids(struct openpgp_publickey *key)
        return merged;
 }
 
        return merged;
 }
 

+/**
+ *     check_sighashes - Check that sig hashes are correct.
+ *     @key - the check to check the sig hashes of.
+ *
+ *     Given an OpenPGP key confirm that all of the sigs on it have the
+ *     appropriate 2 octet hash beginning, as stored as part of the sig.
+ *     This is a simple way to remove junk sigs and, for example, catches
+ *     subkey sig corruption as produced by old pksd implementations.
+ *     Any sig that has an incorrect hash is removed from the key. If the
+ *     hash cannot be checked (eg we don't support that hash type) we err
+ *     on the side of caution and keep it.
+ */
+int clean_sighashes(struct openpgp_publickey *key,
+               struct openpgp_packet *sigdata,
+               struct openpgp_packet_list **sigs)
+{
+       struct openpgp_packet_list *tmpsig;
+       int removed = 0;
+
+       while (*sigs != NULL) {
+               if (check_packet_sighash(key, sigdata, (*sigs)->packet) == 0) {
+                       tmpsig = *sigs;
+                       *sigs = (*sigs)->next;
+                       tmpsig->next = NULL;
+                       free_packet_list(tmpsig);
+                       removed++;
+               } else {
+                       sigs = &(*sigs)->next;
+               }
+       }
+
+       return removed;
+}
+
+int clean_list_sighashes(struct openpgp_publickey *key,
+                       struct openpgp_signedpacket_list *siglist)
+{
+       int removed = 0;
+
+       while (siglist != NULL) {
+               removed += clean_sighashes(key, siglist->packet,
+                       &siglist->sigs);
+               siglist = siglist->next;
+       }
+
+       return removed;
+}
+
+int clean_key_sighashes(struct openpgp_publickey *key)
+{
+       int removed;
+
+       removed = clean_sighashes(key, NULL, &key->sigs);
+       removed += clean_list_sighashes(key, key->uids);
+       removed += clean_list_sighashes(key, key->subkeys);
+
+       return removed;
+}
+

 /**
  *     cleankeys - Apply all available cleaning options on a list of keys.
  *     @keys: The list of keys to clean.
 /**
  *     cleankeys - Apply all available cleaning options on a list of keys.
  *     @keys: The list of keys to clean.


@@ -81,10 +142,14 @@ int dedupuids(struct openpgp_publickey *key)
  */
 int cleankeys(struct openpgp_publickey *keys)
 {
  */
 int cleankeys(struct openpgp_publickey *keys)
 {

-       int changed = 0;
+       int changed = 0, count;

 
        while (keys != NULL) {
 
        while (keys != NULL) {

-               if (dedupuids(keys) > 0) {
+               count = dedupuids(keys);
+               if (config.check_sighash) {
+                       count += clean_key_sighashes(keys);
+               }
+               if (count > 0) {

                        changed++;
                }
                keys = keys->next;
                        changed++;
                }
                keys = keys->next;

diff --git a/onak-conf.c b/onak-conf.c
index 6d72e6fd4e7e3d53ac86ab30760c45ffb7020d04..ec61cd2332b1eea0805f9052cf0b3a96e7b37117 100644 (file)
--- a/onak-conf.c
+++ b/onak-conf.c
@@ -1,7 +1,7 @@
 /*
  * onak-conf.c - Routines related to runtime config.
  *
 /*
  * onak-conf.c - Routines related to runtime config.
  *

- * Copyright 2002 Jonathan McDowell <noodles@earth.li>
+ * Copyright 2002,2012 Jonathan McDowell <noodles@earth.li>

  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the Free
  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the Free


@@ -66,6 +66,8 @@ struct onak_config config = {
        NULL,                   /* backends_dir */
 
        &DBFUNCS,               /* Default dbfuncs struct */
        NULL,                   /* backends_dir */
 
        &DBFUNCS,               /* Default dbfuncs struct */

+
+       true,                   /* Check packet sig hashes */

 };
 
 bool parsebool(char *str, bool fallback)
 };
 
 bool parsebool(char *str, bool fallback)


@@ -175,6 +177,9 @@ void readconfig(const char *configfile) {
                } else if (!strncmp("use_keyd ", curline, 9)) {
                        config.use_keyd = parsebool(&curline[9],
                                                config.use_keyd);
                } else if (!strncmp("use_keyd ", curline, 9)) {
                        config.use_keyd = parsebool(&curline[9],
                                                config.use_keyd);

+               } else if (!strncmp("check_sighash ", curline, 9)) {
+                       config.check_sighash = parsebool(&curline[9],
+                                               config.check_sighash);

                } else {
                        logthing(LOGTHING_ERROR,
                                "Unknown config line: %s", curline);
                } else {
                        logthing(LOGTHING_ERROR,
                                "Unknown config line: %s", curline);

diff --git a/onak-conf.h b/onak-conf.h
index 518c30616348d1af8cb1e442f1d7b713e3c15657..39283dc18c3751cc112090683ce3850e5908fcd5 100644 (file)
--- a/onak-conf.h
+++ b/onak-conf.h
@@ -77,6 +77,8 @@ struct onak_config {
        char *backends_dir;
 
        struct dbfuncs *dbbackend;
        char *backends_dir;
 
        struct dbfuncs *dbbackend;

+
+       bool check_sighash;

 };
 
 /*
 };
 
 /*

diff --git a/onak.conf.in b/onak.conf.in
index 2ee3eb4b1da250d77db1dbae9656d9aa14bd2914..17742112ee68b06dfb0208c9812b9f2702b97a79 100644 (file)
--- a/onak.conf.in
+++ b/onak.conf.in
@@ -67,3 +67,8 @@ max_last 1
 ### will allow any size reply.
 
 max_reply_keys 128
 ### will allow any size reply.
 
 max_reply_keys 128

+
+##
+## Verify signature hashes.
+##
+#check_sighash true

diff --git a/sigcheck.c b/sigcheck.c
new file mode 100644 (file)
index 0000000..fc04626
--- /dev/null
+++ b/sigcheck.c
@@ -0,0 +1,228 @@
+/*
+ * sigcheck.c - routines to check OpenPGP signatures
+ *
+ * Copyright 2012 Jonathan McDowell <noodles@earth.li>
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 51
+ * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#include <stdint.h>
+
+#include "config.h"
+#include "keystructs.h"
+#include "log.h"
+#include "openpgp.h"
+#include "sigcheck.h"
+
+#ifdef HAVE_NETTLE
+#include <nettle/md5.h>
+#include <nettle/sha.h>
+#else
+#include "md5.h"
+#include "sha1.h"
+#endif
+
+int check_packet_sighash(struct openpgp_publickey *key,
+                       struct openpgp_packet *packet,
+                       struct openpgp_packet *sig)
+{
+       uint8_t hashtype;
+       uint8_t *sighash;
+       size_t siglen, unhashedlen;
+       struct sha1_ctx sha1_context;
+       struct md5_ctx md5_context;
+#ifdef NETTLE_WITH_SHA224
+       struct sha224_ctx sha224_context;
+#endif
+#ifdef NETTLE_WITH_SHA256
+       struct sha256_ctx sha256_context;
+#endif
+#ifdef NETTLE_WITH_SHA384
+       struct sha384_ctx sha384_context;
+#endif
+#ifdef NETTLE_WITH_SHA512
+       struct sha512_ctx sha512_context;
+#endif
+       uint8_t keyheader[3];
+       uint8_t packetheader[5];
+       uint8_t v4trailer[6];
+       uint8_t hash[20];
+       uint8_t *hashdata[8];
+       size_t hashlen[8];
+       int chunks, i;
+
+       keyheader[0] = 0x99;
+       keyheader[1] = key->publickey->length >> 8;
+       keyheader[2] = key->publickey->length & 0xFF;
+       hashdata[0] = keyheader;
+       hashlen[0] = 3;
+       hashdata[1] = key->publickey->data;
+       hashlen[1] = key->publickey->length;
+       chunks = 2;
+
+       switch (sig->data[0]) {
+       case 2:
+       case 3:
+               hashtype = sig->data[16];
+
+               if (packet != NULL) {
+                       if (packet->tag == OPENPGP_PACKET_PUBLICSUBKEY) {
+                               packetheader[0] = 0x99;
+                               packetheader[1] = packet->length >> 8;
+                               packetheader[2] = packet->length & 0xFF;
+                               hashdata[chunks] = packetheader;
+                               hashlen[chunks] = 3;
+                               chunks++;
+                       }
+
+                       // TODO: Things other than UIDS/subkeys?
+                       hashdata[chunks] = packet->data;
+                       hashlen[chunks] = packet->length;
+                       chunks++;
+               }
+
+               hashdata[chunks] = &sig->data[2];
+               hashlen[chunks] = 5;
+               chunks++;
+               sighash = &sig->data[17];
+               break;
+       case 4:
+               hashtype = sig->data[3];
+
+               if (packet != NULL) {
+                       if (packet->tag == OPENPGP_PACKET_PUBLICSUBKEY) {
+                               packetheader[0] = 0x99;
+                               packetheader[1] = packet->length >> 8;
+                               packetheader[2] = packet->length & 0xFF;
+                               hashdata[chunks] = packetheader;
+                               hashlen[chunks] = 3;
+                               chunks++;
+                       } else if (packet->tag == OPENPGP_PACKET_UID ||
+                                       packet->tag == OPENPGP_PACKET_UAT) {
+                               packetheader[0] = (packet->tag ==
+                                       OPENPGP_PACKET_UID) ?  0xB4 : 0xD1;
+                               packetheader[1] = packet->length >> 24;
+                               packetheader[2] = (packet->length >> 16) & 0xFF;
+                               packetheader[3] = (packet->length >> 8) & 0xFF;
+                               packetheader[4] = packet->length & 0xFF;
+                               hashdata[chunks] = packetheader;
+                               hashlen[chunks] = 5;
+                               chunks++;
+                       }
+                       hashdata[chunks] = packet->data;
+                       hashlen[chunks] = packet->length;
+                       chunks++;
+               }
+
+               hashdata[chunks] = sig->data;
+               hashlen[chunks] = siglen = (sig->data[4] << 8) +
+                       sig->data[5] + 6;;
+               chunks++;
+
+               v4trailer[0] = 4;
+               v4trailer[1] = 0xFF;
+               v4trailer[2] = siglen >> 24;
+               v4trailer[3] = (siglen >> 16) & 0xFF;
+               v4trailer[4] = (siglen >> 8) & 0xFF;
+               v4trailer[5] = siglen & 0xFF;
+               hashdata[chunks] = v4trailer;
+               hashlen[chunks] = 6;
+               chunks++;
+
+               unhashedlen = (sig->data[siglen] << 8) +
+                       sig->data[siglen + 1];
+               sighash = &sig->data[siglen + unhashedlen + 2];
+               break;
+       default:
+               logthing(LOGTHING_ERROR, "Unknown signature version %d",
+                               sig->data[0]);
+               return -1;
+       }
+
+       switch (hashtype) {
+       case OPENPGP_HASH_MD5:
+               md5_init(&md5_context);
+               for (i = 0; i < chunks; i++) {
+                       md5_update(&md5_context, hashlen[i], hashdata[i]);
+               }
+               md5_digest(&md5_context, 16, hash);
+               break;
+       case OPENPGP_HASH_SHA1:
+               sha1_init(&sha1_context);
+               for (i = 0; i < chunks; i++) {
+                       sha1_update(&sha1_context, hashlen[i], hashdata[i]);
+               }
+               sha1_digest(&sha1_context, 20, hash);
+               break;
+       case OPENPGP_HASH_SHA224:
+#ifdef NETTLE_WITH_SHA224
+               sha224_init(&sha224_context);
+               for (i = 0; i < chunks; i++) {
+                       sha224_update(&sha224_context, hashlen[i],
+                               hashdata[i]);
+               }
+               sha224_digest(&sha224_context, SHA224_DIGEST_SIZE, hash);
+#else
+               logthing(LOGTHING_INFO, "SHA224 support not available.");
+#endif
+               break;
+       case OPENPGP_HASH_SHA256:
+#ifdef NETTLE_WITH_SHA256
+               sha256_init(&sha256_context);
+               for (i = 0; i < chunks; i++) {
+                       sha256_update(&sha256_context, hashlen[i],
+                               hashdata[i]);
+               }
+               sha256_digest(&sha256_context, SHA256_DIGEST_SIZE, hash);
+#else
+               logthing(LOGTHING_INFO, "SHA256 support not available.");
+#endif
+               break;
+       case OPENPGP_HASH_SHA384:
+#ifdef NETTLE_WITH_SHA384
+               sha384_init(&sha384_context);
+               for (i = 0; i < chunks; i++) {
+                       sha384_update(&sha384_context, hashlen[i],
+                               hashdata[i]);
+               }
+               sha384_digest(&sha384_context, SHA384_DIGEST_SIZE, hash);
+#else
+               logthing(LOGTHING_INFO, "SHA384 support not available.");
+#endif
+               break;
+       case OPENPGP_HASH_SHA512:
+#ifdef NETTLE_WITH_SHA512
+               sha512_init(&sha512_context);
+               for (i = 0; i < chunks; i++) {
+                       sha512_update(&sha512_context, hashlen[i],
+                               hashdata[i]);
+               }
+               sha512_digest(&sha512_context, SHA512_DIGEST_SIZE, hash);
+#else
+               logthing(LOGTHING_INFO, "SHA512 support not available.");
+#endif
+               break;
+       default:
+               logthing(LOGTHING_ERROR, "Unsupported signature hash type %d",
+                               hashtype);
+               return -1;
+       }
+
+       logthing(LOGTHING_DEBUG, "Hash type: %d, %d chunks, "
+               "calculated: %02X%02X / actual: %02X%02X\n",
+               hashtype, chunks,
+               hash[0], hash[1], sighash[0], sighash[1]);
+
+       return (hash[0] == sighash[0] && hash[1] == sighash[1]);
+}

diff --git a/sigcheck.h b/sigcheck.h
new file mode 100644 (file)
index 0000000..394dd65
--- /dev/null
+++ b/sigcheck.h
@@ -0,0 +1,9 @@
+#ifndef __SIGCHECK_H__
+#define __SIGCHECK_H__
+#include "keystructs.h"
+
+int check_packet_sighash(struct openpgp_publickey *key,
+                       struct openpgp_packet *packet,
+                       struct openpgp_packet *sig);
+
+#endif /* __SIGCHECK_H__ */