DKIM: More validation of DNS key record. Bug 1926

diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 7bfcdf4aac9ad10ff9a515392118c0696e4b7ac9..c4bdb5a931b9b71af20dad4605630d1f09b3fdc6 100644 (file)
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -603,90 +603,87 @@ pub = store_get(sizeof(pdkim_pubkey));
 memset(pub, 0, sizeof(pdkim_pubkey));
 
 for (p = raw_record; ; p++)
 memset(pub, 0, sizeof(pdkim_pubkey));
 
 for (p = raw_record; ; p++)

-  {
-  char c = *p;
-
-  /* Ignore FWS */
-  if (c == '\r' || c == '\n')
-    goto NEXT_CHAR;
-
-  if (where == PDKIM_HDR_LIMBO)

     {
     {

-    /* In limbo, just wait for a tag-char to appear */
-    if (!(c >= 'a' && c <= 'z'))
-      goto NEXT_CHAR;
-
-    where = PDKIM_HDR_TAG;
-    }
+    uschar c = *p;

 
 

-  if (where == PDKIM_HDR_TAG)
-    {
-    if (c >= 'a' && c <= 'z')
-      cur_tag = string_catn(cur_tag, &ts, &tl, p, 1);
-
-    if (c == '=')
+    /* Ignore FWS */
+    if (c != '\r' && c != '\n') switch (where)

       {
       {

-      cur_tag[tl] = '\0';
-      where = PDKIM_HDR_VALUE;
-      goto NEXT_CHAR;
-      }
-    }
+      case PDKIM_HDR_LIMBO:            /* In limbo, just wait for a tag-char to appear */
+       if (!(c >= 'a' && c <= 'z'))
+         break;
+       where = PDKIM_HDR_TAG;
+       /*FALLTHROUGH*/

 
 

-  if (where == PDKIM_HDR_VALUE)
-    {
-    if (c == ';' || c == '\0')
-      {
-      if (tl && vl)
-        {
-       cur_val[vl] = '\0';
-       pdkim_strtrim(cur_val);
-       DEBUG(D_acl) debug_printf(" %s=%s\n", cur_tag, cur_val);
+      case PDKIM_HDR_TAG:
+       if (c >= 'a' && c <= 'z')
+         cur_tag = string_catn(cur_tag, &ts, &tl, p, 1);

 
 

-       switch (cur_tag[0])
+       if (c == '=')

          {
          {

-         case 'v':
-           /* This tag isn't evaluated because:
-              - We only support version DKIM1.
-              - Which is the default for this value (set below)
-              - Other versions are currently not specified.      */
-           break;
-         case 'h':
-         case 'k':
-           pub->hashes = string_copy(cur_val); break;
-         case 'g':
-           pub->granularity = string_copy(cur_val); break;
-         case 'n':
-           pub->notes = pdkim_decode_qp(cur_val); break;
-         case 'p':
-           pdkim_decode_base64(US cur_val, &pub->key);
-            break;
-         case 's':
-           pub->srvtype = string_copy(cur_val); break;
-         case 't':
-           if (Ustrchr(cur_val, 'y') != NULL) pub->testing = 1;
-           if (Ustrchr(cur_val, 's') != NULL) pub->no_subdomaining = 1;
-           break;
-         default:
-           DEBUG(D_acl) debug_printf(" Unknown tag encountered\n");
-           break;
+         cur_tag[tl] = '\0';
+         where = PDKIM_HDR_VALUE;

          }
          }

-       }
-      tl = 0;
-      vl = 0;
-      where = PDKIM_HDR_LIMBO;
+       break;
+
+      case PDKIM_HDR_VALUE:
+       if (c == ';' || c == '\0')
+         {
+         if (tl && vl)
+           {
+           cur_val[vl] = '\0';
+           pdkim_strtrim(cur_val);
+           DEBUG(D_acl) debug_printf(" %s=%s\n", cur_tag, cur_val);
+
+           switch (cur_tag[0])
+             {
+             case 'v':
+               pub->version = string_copy(cur_val); break;
+             case 'h':
+             case 'k':
+/* This field appears to never be used. Also, unclear why
+a 'k' (key-type_ would go in this field name.  There is a field
+"keytype", also never used.
+               pub->hashes = string_copy(cur_val);
+*/
+               break;
+             case 'g':
+               pub->granularity = string_copy(cur_val); break;
+             case 'n':
+               pub->notes = pdkim_decode_qp(cur_val); break;
+             case 'p':
+               pdkim_decode_base64(US cur_val, &pub->key); break;
+             case 's':
+               pub->srvtype = string_copy(cur_val); break;
+             case 't':
+               if (Ustrchr(cur_val, 'y') != NULL) pub->testing = 1;
+               if (Ustrchr(cur_val, 's') != NULL) pub->no_subdomaining = 1;
+               break;
+             default:
+               DEBUG(D_acl) debug_printf(" Unknown tag encountered\n");
+               break;
+             }
+           }
+         tl = 0;
+         vl = 0;
+         where = PDKIM_HDR_LIMBO;
+         }
+       else
+         cur_val = string_catn(cur_val, &vs, &vl, p, 1);
+       break;

       }
       }

-    else
-      cur_val = string_catn(cur_val, &vs, &vl, p, 1);
-    }

 
 

-NEXT_CHAR:
-  if (c == '\0') break;
-  }
+    if (c == '\0') break;
+    }

 
 /* Set fallback defaults */
 if (!pub->version    ) pub->version     = string_copy(PDKIM_PUB_RECORD_VERSION);
 
 /* Set fallback defaults */
 if (!pub->version    ) pub->version     = string_copy(PDKIM_PUB_RECORD_VERSION);

+else if (Ustrcmp(pub->version, PDKIM_PUB_RECORD_VERSION) != 0) return NULL;
+

 if (!pub->granularity) pub->granularity = string_copy(US"*");
 if (!pub->granularity) pub->granularity = string_copy(US"*");

+/*

 if (!pub->keytype    ) pub->keytype     = string_copy(US"rsa");
 if (!pub->keytype    ) pub->keytype     = string_copy(US"rsa");

+*/

 if (!pub->srvtype    ) pub->srvtype     = string_copy(US"*");
 
 /* p= is required */
 if (!pub->srvtype    ) pub->srvtype     = string_copy(US"*");
 
 /* p= is required */


@@ -794,7 +791,7 @@ for (sig = ctx->sig; sig; sig = sig->next)
   DEBUG(D_acl)
     {
     debug_printf("PDKIM [%s] Body bytes hashed: %lu\n"
   DEBUG(D_acl)
     {
     debug_printf("PDKIM [%s] Body bytes hashed: %lu\n"

-                "PDKIM [%s] bh  computed: ",
+                "PDKIM [%s] Body hash computed: ",

                sig->domain, sig->signed_body_bytes, sig->domain);
     pdkim_hexprint(CUS bh.data, bh.len);
     }
                sig->domain, sig->signed_body_bytes, sig->domain);
     pdkim_hexprint(CUS bh.data, bh.len);
     }


@@ -822,7 +819,7 @@ for (sig = ctx->sig; sig; sig = sig->next)
       {
       DEBUG(D_acl)
         {
       {
       DEBUG(D_acl)
         {

-       debug_printf("PDKIM [%s] bh signature: ", sig->domain);
+       debug_printf("PDKIM [%s] Body hash signature from headers: ", sig->domain);

        pdkim_hexprint(sig->bodyhash.data,
                         exim_sha_hashlen(&sig->body_hash));
        debug_printf("PDKIM [%s] Body hash did NOT verify\n", sig->domain);
        pdkim_hexprint(sig->bodyhash.data,
                         exim_sha_hashlen(&sig->body_hash));
        debug_printf("PDKIM [%s] Body hash did NOT verify\n", sig->domain);


@@ -1341,7 +1338,7 @@ while (sig)
   exim_sha_init(&hhash_ctx, is_sha1 ? HASH_SHA1 : HASH_SHA256);
 
   DEBUG(D_acl) debug_printf(
   exim_sha_init(&hhash_ctx, is_sha1 ? HASH_SHA1 : HASH_SHA256);
 
   DEBUG(D_acl) debug_printf(

-      "PDKIM >> Hashed header data, canonicalized, in sequence >>>>>>>>>>>>>>\n");
+      "PDKIM >> Header data for hash, canonicalized, in sequence >>>>>>>>>>>>>>\n");

 
   /* SIGNING ---------------------------------------------------------------- */
   /* When signing, walk through our header list and add them to the hash. As we
 
   /* SIGNING ---------------------------------------------------------------- */
   /* When signing, walk through our header list and add them to the hash. As we


@@ -1471,7 +1468,7 @@ while (sig)
 
   DEBUG(D_acl)
     {
 
   DEBUG(D_acl)
     {

-    debug_printf("PDKIM [%s] hh computed: ", sig->domain);
+    debug_printf("PDKIM [%s] Header hash computed: ", sig->domain);

     pdkim_hexprint(hhash.data, hhash.len);
     }
 
     pdkim_hexprint(hhash.data, hhash.len);
     }
 


@@ -1520,6 +1517,7 @@ while (sig)
     {
     ev_ctx vctx;
     const uschar * errstr;
     {
     ev_ctx vctx;
     const uschar * errstr;

+    pdkim_pubkey * p;

 
     uschar *dns_txt_name, *dns_txt_reply;
 
 
     uschar *dns_txt_name, *dns_txt_reply;
 


@@ -1578,16 +1576,25 @@ while (sig)
       pdkim_quoteprint(CUS dns_txt_reply, Ustrlen(dns_txt_reply));
       }
 
       pdkim_quoteprint(CUS dns_txt_reply, Ustrlen(dns_txt_reply));
       }
 

-    if (!(sig->pubkey = pdkim_parse_pubkey_record(ctx, CUS dns_txt_reply)))
+    if (  !(p = pdkim_parse_pubkey_record(ctx, CUS dns_txt_reply))
+       || (Ustrcmp(p->srvtype, "*") != 0 && Ustrcmp(p->srvtype, "email") != 0)
+       )

       {
       sig->verify_status =      PDKIM_VERIFY_INVALID;
       sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD;
 
       {
       sig->verify_status =      PDKIM_VERIFY_INVALID;
       sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD;
 

-      DEBUG(D_acl) debug_printf(
-         " Error while parsing public key record\n"
+      DEBUG(D_acl)
+       {
+       if (p)
+         debug_printf(" Invalid public key service type '%s'\n", p->srvtype);
+       else
+         debug_printf(" Error while parsing public key record\n");
+       debug_printf(

          "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
          "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");

+       }

       goto NEXT_VERIFY;
       }
       goto NEXT_VERIFY;
       }

+    sig->pubkey = p;

 
     DEBUG(D_acl) debug_printf(
          "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
 
     DEBUG(D_acl) debug_printf(
          "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");

diff --git a/src/src/pdkim/pdkim.h b/src/src/pdkim/pdkim.h
index 536e5af690e297f53e57b2b4a90cb9884525b2bb..07ba5b5c4801b3888311e0d2d0c352717de9e809 100644 (file)
--- a/src/src/pdkim/pdkim.h
+++ b/src/src/pdkim/pdkim.h
@@ -100,13 +100,14 @@ typedef struct pdkim_pubkey {
   uschar *version;                /* v=  */
   uschar *granularity;            /* g=  */
 
   uschar *version;                /* v=  */
   uschar *granularity;            /* g=  */
 

+#ifdef notdef

   uschar *hashes;                 /* h=  */
   uschar *keytype;                /* k=  */
   uschar *hashes;                 /* h=  */
   uschar *keytype;                /* k=  */

+#endif

   uschar *srvtype;                /* s=  */
   uschar *notes;                  /* n=  */
 
   blob  key;                      /* p=  */
   uschar *srvtype;                /* s=  */
   uschar *notes;                  /* n=  */
 
   blob  key;                      /* p=  */

-

   int   testing;                  /* t=y */
   int   no_subdomaining;          /* t=s */
 } pdkim_pubkey;
   int   testing;                  /* t=y */
   int   no_subdomaining;          /* t=s */
 } pdkim_pubkey;

diff --git a/test/dnszones-src/db.test.ex b/test/dnszones-src/db.test.ex
index 6555ec81d20a18f25ecc981b1173d1d1fc28cb23..16468bf79f177c8eb4d8fc004772a93f2ecdf483 100644 (file)
--- a/test/dnszones-src/db.test.ex
+++ b/test/dnszones-src/db.test.ex
@@ -476,9 +476,12 @@ DELAY=1500 delay1500 A HOSTIPV4
 ;  openssl genrsa -out aux-fixed/dkim/dkim.private 1024
 ;  openssl rsa -in aux-fixed/dkim/dkim.private -out /dev/stdout -pubout -outform PEM
 ;
 ;  openssl genrsa -out aux-fixed/dkim/dkim.private 1024
 ;  openssl rsa -in aux-fixed/dkim/dkim.private -out /dev/stdout -pubout -outform PEM
 ;

+; Deliberate bad version, having extra backslashes
+;

 ; Another, 512-bit (with a Notes field)
 ;
 sel._domainkey TXT "v=DKIM1; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDXRFf+VhT+lCgFhhSkinZKcFNeRzjYdW8vT29Rbb3NadvTFwAd+cVLPFwZL8H5tUD/7JbUPqNTCPxmpgIL+V5T4tEZMorHatvvUM2qfcpQ45IfsZ+YdhbIiAslHCpy4xNxIR3zylgqRUF4+Dtsaqy3a5LhwMiKCLrnzhXk1F1hxwIDAQAB"
 ; Another, 512-bit (with a Notes field)
 ;
 sel._domainkey TXT "v=DKIM1; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDXRFf+VhT+lCgFhhSkinZKcFNeRzjYdW8vT29Rbb3NadvTFwAd+cVLPFwZL8H5tUD/7JbUPqNTCPxmpgIL+V5T4tEZMorHatvvUM2qfcpQ45IfsZ+YdhbIiAslHCpy4xNxIR3zylgqRUF4+Dtsaqy3a5LhwMiKCLrnzhXk1F1hxwIDAQAB"

+sel_bad._domainkey TXT "v=DKIM1\; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDXRFf+VhT+lCgFhhSkinZKcFNeRzjYdW8vT29Rbb3NadvTFwAd+cVLPFwZL8H5tUD/7JbUPqNTCPxmpgIL+V5T4tEZMorHatvvUM2qfcpQ45IfsZ+YdhbIiAslHCpy4xNxIR3zylgqRUF4+Dtsaqy3a5LhwMiKCLrnzhXk1F1hxwIDAQAB"

 
 ses._domainkey TXT "v=DKIM1; n=halfkilo; p=MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAL6eAQxd9didJ0/+05iDwJOqT6ly826Vi8aGPecsBiYK5/tAT97fxXk+dPWMZp9kQxtknEzYjYjAydzf+HQ2yJMCAwEAAQ=="
 
 
 ses._domainkey TXT "v=DKIM1; n=halfkilo; p=MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAL6eAQxd9didJ0/+05iDwJOqT6ly826Vi8aGPecsBiYK5/tAT97fxXk+dPWMZp9kQxtknEzYjYjAydzf+HQ2yJMCAwEAAQ=="
 

diff --git a/test/log/4502 b/test/log/4502
index ab5273ad03eccb0be63f8b30a6d50f4d99d23f0d..b7e4a8ddd31789cc07c9cd3e9fe25161ff126e9c 100644 (file)
--- a/test/log/4502
+++ b/test/log/4502
@@ -10,3 +10,6 @@
 1999-03-02 09:44:33 10HmaZ-0005vi-00 DKIM: d=test.ex s=sel c=relaxed/simple a=rsa-sha1 b=1024 [verification succeeded]
 1999-03-02 09:44:33 10HmaZ-0005vi-00 signer: test.ex bits: 1024
 1999-03-02 09:44:33 10HmaZ-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss
 1999-03-02 09:44:33 10HmaZ-0005vi-00 DKIM: d=test.ex s=sel c=relaxed/simple a=rsa-sha1 b=1024 [verification succeeded]
 1999-03-02 09:44:33 10HmaZ-0005vi-00 signer: test.ex bits: 1024
 1999-03-02 09:44:33 10HmaZ-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss

+1999-03-02 09:44:33 10HmbA-0005vi-00 DKIM: d=test.ex s=sel_bad c=relaxed/relaxed a=rsa-sha1 b=1024 [invalid - syntax error in public key record]
+1999-03-02 09:44:33 10HmbA-0005vi-00 signer: test.ex bits: 1024
+1999-03-02 09:44:33 10HmbA-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=564CFC9B.1040905@yahoo.com

diff --git a/test/scripts/4500-Domain-Keys-Identified-Mail/4502 b/test/scripts/4500-Domain-Keys-Identified-Mail/4502
index 5e63f129fc508eceb1b3a4f92891a3834589e8e8..1e0005b462639ae09182c9ebbeb4184326700018 100644 (file)
--- a/test/scripts/4500-Domain-Keys-Identified-Mail/4502
+++ b/test/scripts/4500-Domain-Keys-Identified-Mail/4502
@@ -1,11 +1,11 @@
-# DKIM relaxed canonicalisation
+# DKIM verify, relaxed canonicalisation

 #
 exim -DSERVER=server -bd -oX PORT_D
 ****
 #
 # This should pass.
 # Mail original in aux-fixed/4502.msg1.txt
 #
 exim -DSERVER=server -bd -oX PORT_D
 ****
 #
 # This should pass.
 # Mail original in aux-fixed/4502.msg1.txt

-# Sig generated by:  perl aux-fixed/dkim/sign.pl --method=relaxed/relaxed < aux_fixed/4502.msg1.txt
+# Sig generated by:  perl aux-fixed/dkim/sign.pl --method=relaxed/relaxed < aux-fixed/4502.msg1.txt

 client 127.0.0.1 PORT_D
 ??? 220
 HELO xxx
 client 127.0.0.1 PORT_D
 ??? 220
 HELO xxx


@@ -52,7 +52,7 @@ QUIT
 #
 # This should pass.
 # Mail original in aux-fixed/4502.msg2.txt
 #
 # This should pass.
 # Mail original in aux-fixed/4502.msg2.txt

-# Sig generated by:  perl aux-fixed/dkim/sign.pl --method=relaxed < aux_fixed/4502.msg2.txt
+# Sig generated by:  perl aux-fixed/dkim/sign.pl --method=relaxed < aux-fixed/4502.msg2.txt

 client 127.0.0.1 PORT_D
 ??? 220
 HELO xxx
 client 127.0.0.1 PORT_D
 ??? 220
 HELO xxx


@@ -93,7 +93,7 @@ QUIT
 #
 # This should pass.
 # Mail original in aux-fixed/4502.msg3.txt
 #
 # This should pass.
 # Mail original in aux-fixed/4502.msg3.txt

-# Sig generated by:  perl aux-fixed/dkim/sign.pl --method=relaxed < aux_fixed/4502.msg3.txt
+# Sig generated by:  perl aux-fixed/dkim/sign.pl --method=relaxed < aux-fixed/4502.msg3.txt

 client 127.0.0.1 PORT_D
 ??? 220
 HELO xxx
 client 127.0.0.1 PORT_D
 ??? 220
 HELO xxx


@@ -130,6 +130,52 @@ QUIT
 ??? 221
 ****
 #
 ??? 221
 ****
 #

+# This should fail, but passes - bug 1926 - due to an extra \ in the DNS record.
+# Mail original in aux-fixed/4502.msg1.txt
+# Sig generated by:  perl aux-fixed/dkim/sign.pl --method=relaxed/relaxed --selector=sel_bad < aux-fixed/4502.msg1.txt
+client 127.0.0.1 PORT_D
+??? 220
+HELO xxx
+??? 250
+MAIL FROM:<CALLER@bloggs.com>
+??? 250
+RCPT TO:<a@test.ex>
+??? 250
+DATA
+??? 354
+DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=test.ex; h=
+        message-id:date:from:mime-version:to:subject:content-type
+        :content-transfer-encoding; s=sel_bad; bh=rn0kk3aPKyhYbxzfi3WG8d
+        AxhNM=; b=kXWfssgeNTAHmr9u2U6VZvb8uXuzoeLtZqgxySmUERKBsjk9sV31yv
+        3rEMCwdtM38yBNFK9zuLsoBUO6M7fGnpfgbGv7BnDHx8AJcsPc1Ay/7JbLKhiCxo
+        zMTFil/4pj1s3bQGLCCOcN688IgerUUFqNBM5vq0nIOKzj2dwhQC8=
+Message-ID: <564CFC9B.1040905@yahoo.com>
+Date: Wed, 18 Nov 2015 14:32:59 -0800
+From: Joaquin Lopez <bakawolf@test.ex>
+User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:24.0) Gecko/20100101 Thunderbird/24.0
+MIME-Version: 1.0
+To: bakawolf@yahoo.com
+Subject: test
+Content-Type: text/plain; charset=ISO-8859-1; format=flowed
+Content-Transfer-Encoding: 7bit
+Content-Length: 13
+
+
+
+test
+
+
+
+
+
+
+      
+
+.
+??? 250
+QUIT
+??? 221
+****

 killdaemon
 no_stdout_check
 no_msglog_check
 killdaemon
 no_stdout_check
 no_msglog_check