Untangle digestalg from verifyalg; better separation, less code duplication.

src/dnssec-crypto.h

@@ -15,13 +15,13 @@ struct keydata;
  *    // nor consumed, so the pointer must stay valid.
  *    alg->set_signature(sig, 16);
  *
- *    // Second, push the data in; data is consumed immediately, so the buffer
- *    // can be freed or modified.
- *    alg->begin_data();
- *    alg->add_data(buf1, 123);
- *    alg->add_data(buf2, 45);
- *    alg->add_data(buf3, 678);
- *    alg->end_data();
+ *    // Second, get push the data through the corresponding digest algorithm;
+ *    // data is consumed immediately, so the buffers can be freed or modified.
+ *    digestalg_begin(alg->get_digestalgo());
+ *    digestalg_add_data(buf1, 123);
+ *    digestalg_add_data(buf2, 45);
+ *    digestalg_add_data(buf3, 678);
+ *    alg->set_digest(digestalg_final());
  *
  *    // Third, verify if we got the correct key for this signature.
  *    alg->verify(key1, 16);
@@ -33,9 +33,8 @@ typedef struct VerifyAlgCtx VerifyAlgCtx;
 typedef struct
 {
   int (*set_signature)(VerifyAlgCtx *ctx, unsigned char *data, unsigned len);
-  void (*begin_data)(VerifyAlgCtx *ctx);
-  void (*add_data)(VerifyAlgCtx *ctx, void *data, unsigned len);
-  void (*end_data)(VerifyAlgCtx *ctx);
+  int (*get_digestalgo)(VerifyAlgCtx *ctx);
+  void (*set_digest)(VerifyAlgCtx *ctx, unsigned char *digest);
   int (*verify)(VerifyAlgCtx *ctx, struct keydata *key, unsigned key_len);
 } VerifyAlg;
 
@@ -50,10 +49,16 @@ void verifyalg_free(VerifyAlgCtx *a);
 int verifyalg_algonum(VerifyAlgCtx *a);
 
 /* Functions to calculate the digest of a key */
+
+/* RFC4034 digest algorithms */
+#define DIGESTALG_SHA1     1
+#define DIGESTALG_SHA256   2
+
 int digestalg_supported(int algo);
 int digestalg_begin(int algo);
 void digestalg_add_data(void *data, unsigned len);
 void digestalg_add_keydata(struct keydata *key, size_t len);
-int digestalg_final(struct keydata *digest);
+unsigned char *digestalg_final(void);
+int digestalg_len(void);
 
 #endif /* DNSSEC_CRYPTO_H */

src/dnssec-openssl.c

@@ -9,11 +9,7 @@ typedef struct VACTX_rsasha1
   VerifyAlgCtx base;
   unsigned char *sig;
   unsigned siglen;
-  union
-    {
-      EVP_MD_CTX hash;
-      unsigned char digest[20];
-    };
+  unsigned char digest[20];
 } VACTX_rsasha1;
 
 typedef struct VACTX_rsasha256
@@ -21,11 +17,7 @@ typedef struct VACTX_rsasha256
   VerifyAlgCtx base;
   unsigned char *sig;
   unsigned siglen;
-  union
-    {
-      EVP_MD_CTX hash;
-      unsigned char digest[32];
-    };
+  unsigned char digest[32];
 } VACTX_rsasha256;
 
 #define POOL_SIZE 1
@@ -62,43 +54,26 @@ static int rsasha256_set_signature(VerifyAlgCtx *ctx_, unsigned char *data, unsi
   return 1;
 }
 
-static void rsasha1_begin_data(VerifyAlgCtx *ctx_)
-{
-  VACTX_rsasha1 *ctx = (VACTX_rsasha1 *)ctx_;
-  EVP_MD_CTX_init(&ctx->hash);
-  EVP_DigestInit_ex(&ctx->hash, EVP_sha1(), NULL);
-}
-static void rsasha256_begin_data(VerifyAlgCtx *ctx_)
-{
-  VACTX_rsasha256 *ctx = (VACTX_rsasha256 *)ctx_;
-  EVP_MD_CTX_init(&ctx->hash);
-  EVP_DigestInit_ex(&ctx->hash, EVP_sha256(), NULL);
-}
-
-static void rsasha1_add_data(VerifyAlgCtx *ctx_, void *data, unsigned len)
+static int rsasha1_get_digestalgo(VerifyAlgCtx *ctx_)
 {
-  VACTX_rsasha1 *ctx = (VACTX_rsasha1 *)ctx_;
-  EVP_DigestUpdate(&ctx->hash, data, len);
+  (void)ctx_;
+  return DIGESTALG_SHA1;
 }
-static void rsasha256_add_data(VerifyAlgCtx *ctx_, void *data, unsigned len)
+static int rsasha256_get_digestalgo(VerifyAlgCtx *ctx_)
 {
-  VACTX_rsasha256 *ctx = (VACTX_rsasha256 *)ctx_;
-  EVP_DigestUpdate(&ctx->hash, data, len);
+  (void)ctx_;
+  return DIGESTALG_SHA256;
 }
 
-static void rsasha1_end_data(VerifyAlgCtx *ctx_)
+static void rsasha1_set_digest(VerifyAlgCtx *ctx_, unsigned char *digest)
 {
   VACTX_rsasha1 *ctx = (VACTX_rsasha1 *)ctx_;
-  unsigned char digest[20];
-  EVP_DigestFinal(&ctx->hash, digest, NULL);
-  memcpy(ctx->digest, digest, 20);
+  memcpy(ctx->digest, digest, sizeof(ctx->digest));
 }
-static void rsasha256_end_data(VerifyAlgCtx *ctx_)
+static void rsasha256_set_digest(VerifyAlgCtx *ctx_, unsigned char *digest)
 {
   VACTX_rsasha256 *ctx = (VACTX_rsasha256 *)ctx_;
-  unsigned char digest[32];
-  EVP_DigestFinal(&ctx->hash, digest, NULL);
-  memcpy(ctx->digest, digest, 32);
+  memcpy(ctx->digest, digest, sizeof(ctx->digest));
 }
 
 static int keydata_to_bn(BIGNUM *ret, struct keydata **key_data, unsigned char **p, unsigned len)
@@ -174,44 +149,36 @@ static int rsasha256_verify(VerifyAlgCtx *ctx_, struct keydata *key_data, unsign
   return validated;
 }
 
-#define DEFINE_VALG(alg) \
-  int alg ## _set_signature(VerifyAlgCtx *ctx, unsigned char *data, unsigned len); \
-  void alg ## _begin_data(VerifyAlgCtx *ctx); \
-  void alg ## _add_data(VerifyAlgCtx *ctx, void *data, unsigned len); \
-  void alg ## _end_data(VerifyAlgCtx *ctx); \
-  int alg ## _verify(VerifyAlgCtx *ctx, struct keydata *key, unsigned key_len) \
-  /**/
+#define VALG_UNSUPPORTED() { \
+    0,0,0,0 \
+  } /**/
 
 #define VALG_VTABLE(alg) { \
   alg ## _set_signature, \
-  alg ## _begin_data, \
-  alg ## _add_data, \
-  alg ## _end_data, \
+  alg ## _get_digestalgo, \
+  alg ## _set_digest, \
   alg ## _verify \
   } /**/
 
-DEFINE_VALG(rsasha1);
-DEFINE_VALG(rsasha256);
-
 /* Updated registry that merges various RFCs:
    https://www.iana.org/assignments/dns-sec-alg-numbers/dns-sec-alg-numbers.xml */
 static const VerifyAlg valgs[] =
 {
-  {0,0,0,0,0},            /*  0: reserved */
-  {0,0,0,0,0},            /*  1: RSAMD5 */
-  {0,0,0,0,0},            /*  2: DH */
-  {0,0,0,0,0},            /*  3: DSA */
-  {0,0,0,0,0},            /*  4: ECC */
-  VALG_VTABLE(rsasha1),   /*  5: RSASHA1 */
-  {0,0,0,0,0},            /*  6: DSA-NSEC3-SHA1 */
-  VALG_VTABLE(rsasha1),   /*  7: RSASHA1-NSEC3-SHA1 */
-  VALG_VTABLE(rsasha256), /*  8: RSASHA256 */
-  {0,0,0,0,0},            /*  9: unassigned */
-  {0,0,0,0,0},            /* 10: RSASHA512 */
-  {0,0,0,0,0},            /* 11: unassigned */
-  {0,0,0,0,0},            /* 12: ECC-GOST */
-  {0,0,0,0,0},            /* 13: ECDSAP256SHA256 */
-  {0,0,0,0,0},            /* 14: ECDSAP384SHA384 */
+  VALG_UNSUPPORTED(),            /*  0: reserved */
+  VALG_UNSUPPORTED(),            /*  1: RSAMD5 */
+  VALG_UNSUPPORTED(),            /*  2: DH */
+  VALG_UNSUPPORTED(),            /*  3: DSA */
+  VALG_UNSUPPORTED(),            /*  4: ECC */
+  VALG_VTABLE(rsasha1),          /*  5: RSASHA1 */
+  VALG_UNSUPPORTED(),            /*  6: DSA-NSEC3-SHA1 */
+  VALG_VTABLE(rsasha1),          /*  7: RSASHA1-NSEC3-SHA1 */
+  VALG_VTABLE(rsasha256),        /*  8: RSASHA256 */
+  VALG_UNSUPPORTED(),            /*  9: unassigned */
+  VALG_UNSUPPORTED(),            /* 10: RSASHA512 */
+  VALG_UNSUPPORTED(),            /* 11: unassigned */
+  VALG_UNSUPPORTED(),            /* 12: ECC-GOST */
+  VALG_UNSUPPORTED(),            /* 13: ECDSAP256SHA256 */
+  VALG_UNSUPPORTED(),            /* 14: ECDSAP384SHA384 */
 };
 
 static const int valgctx_size[] =
@@ -286,7 +253,7 @@ static EVP_MD_CTX digctx;
 
 int digestalg_supported(int algo)
 {
-  return (algo == 1 || algo == 2);
+  return (algo == DIGESTALG_SHA1 || algo == DIGESTALG_SHA256);
 }
 
 int digestalg_begin(int algo)
@@ -301,6 +268,11 @@ int digestalg_begin(int algo)
   return 1;
 }
 
+int digestalg_len()
+{
+  return EVP_MD_CTX_size(&digctx);
+}
+
 void digestalg_add_data(void *data, unsigned len)
 {
   EVP_DigestUpdate(&digctx, data, len);
@@ -318,10 +290,10 @@ void digestalg_add_keydata(struct keydata *key, size_t len)
     }
 }
 
-int digestalg_final(struct keydata *expected)
+unsigned char* digestalg_final(void)
 {
-  unsigned char digest[32];
+  static unsigned char digest[32];
   EVP_DigestFinal(&digctx, digest, NULL);
-  /* FIXME: why EVP_MD_CTX_size() crashes? */
-  return (memcmp(digest, expected->key, 20) == 0);
+  return digest;
 }
+

src/dnssec.c

@@ -443,11 +443,11 @@ static int convert_domain_to_wire(char *name, unsigned char* out)
 }
 
 
-/* Pass a resource record's rdata field through a verification hash function.
+/* Pass a resource record's rdata field through the currently-initailized digest algorithm.
 
    We must pass the record in DNS wire format, but if the record contains domain names,
    they must be uncompressed. This makes things very tricky, because  */
-static int verifyalg_add_rdata(VerifyAlgCtx *alg, int sigtype, struct dns_header *header, size_t pktlen,
+static int digestalg_add_rdata(int sigtype, struct dns_header *header, size_t pktlen,
                                unsigned char *rdata)
 {
   size_t len;
@@ -471,10 +471,10 @@ static int verifyalg_add_rdata(VerifyAlgCtx *alg, int sigtype, struct dns_header
 
   /* Iteration 2: process the canonical record through the hash function */
   total = htons(total);
-  alg->vtbl->add_data(alg, &total, 2);
+  digestalg_add_data(&total, 2);
 
   while ((p = rdata_cform_next(&cf2, &len)))
-    alg->vtbl->add_data(alg, p, len);
+    digestalg_add_data(p, len);
 
   return 1;
 }
@@ -575,22 +575,22 @@ static int begin_rrsig_validation(struct dns_header *header, size_t pktlen,
   unsigned char owner_wire[MAXCDNAME];
   int owner_wire_len = convert_domain_to_wire(owner, owner_wire);
 
-  alg->vtbl->begin_data(alg);
-  alg->vtbl->add_data(alg, sigrdata, 18+signer_name_rdlen);
+  digestalg_begin(alg->vtbl->get_digestalgo(alg));
+  digestalg_add_data(sigrdata, 18+signer_name_rdlen);
   for (i = 0; i < rrsetidx; ++i)
     {
       p = (unsigned char*)(rrset[i]);
 
-      alg->vtbl->add_data(alg, owner_wire, owner_wire_len);
-      alg->vtbl->add_data(alg, &sigtype, 2);
-      alg->vtbl->add_data(alg, &sigclass, 2);
-      alg->vtbl->add_data(alg, &sigttl, 4);
+      digestalg_add_data(owner_wire, owner_wire_len);
+      digestalg_add_data(&sigtype, 2);
+      digestalg_add_data(&sigclass, 2);
+      digestalg_add_data(&sigttl, 4);
     
       p += 8;
-      if (!verifyalg_add_rdata(alg, ntohs(sigtype), header, pktlen, p))
+      if (!digestalg_add_rdata(ntohs(sigtype), header, pktlen, p))
         return 0;
     }
-  alg->vtbl->end_data(alg);
+  alg->vtbl->set_digest(alg, digestalg_final());
 
   /* We don't need the owner name anymore; now extract the signer name */
   if (!extract_name_no_compression(sigrdata+18, signer_name_rdlen, signer_name))
@@ -672,14 +672,16 @@ static int dnskey_ds_match(struct crec *dnskey, struct crec *ds)
   int owner_len = convert_domain_to_wire(cache_get_name(ds), owner);
   size_t keylen = dnskey->uid;
   int dig = ds->uid;
+  int digsize;
 
   if (!digestalg_begin(dig))
     return 0;
+  digsize = digestalg_len();
   digestalg_add_data(owner, owner_len);
   digestalg_add_data("\x01\x01\x03", 3);
   digestalg_add_data(&ds->addr.key.algo, 1);
   digestalg_add_keydata(dnskey->addr.key.keydata, keylen);
-  return digestalg_final(ds->addr.key.keydata);
+  return (memcmp(digestalg_final(), ds->addr.key.keydata->key, digsize) == 0);
 }
 
 int dnssec_parsekey(struct dns_header *header, size_t pktlen, char *owner, unsigned long ttl,