DKIM: Under debug, when signing do an extra check on the dns record that will be

used for verification.  Bug 1926

11 files changed, 180 insertions, 72 deletions

diff --git a/src/src/dkim.c b/src/src/dkim.c
index 70c9547ec..a2574c15b 100644
--- a/src/src/dkim.c
+++ b/src/src/dkim.c
@@ -607,11 +607,13 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep,
     dkim_private_key_expanded = big_buffer;
     }
 
-  ctx = pdkim_init_sign( CS dkim_signing_domain,
-			 CS dkim_signing_selector,
-			 CS dkim_private_key_expanded,
-			 PDKIM_ALGO_RSA_SHA256,
-			 dkim->dot_stuffed);
+  ctx = pdkim_init_sign(CS dkim_signing_domain,
+			CS dkim_signing_selector,
+			CS dkim_private_key_expanded,
+			PDKIM_ALGO_RSA_SHA256,
+			dkim->dot_stuffed,
+			&dkim_exim_query_dns_txt
+			);
   dkim_private_key_expanded[0] = '\0';
   pdkim_set_optional(ctx,
 		      CS dkim_sign_headers_expanded,
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index c4bdb5a93..d3ff03108 100644
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -1302,6 +1302,74 @@ return hdr;
 
 /* -------------------------------------------------------------------------- */
 
+static pdkim_pubkey *
+pdkim_key_from_dns(pdkim_ctx * ctx, pdkim_signature * sig, ev_ctx * vctx)
+{
+uschar * dns_txt_name, * dns_txt_reply;
+pdkim_pubkey * p;
+const uschar * errstr;
+
+/* Fetch public key for signing domain, from DNS */
+
+dns_txt_name = string_sprintf("%s._domainkey.%s.", sig->selector, sig->domain);
+
+dns_txt_reply = store_get(PDKIM_DNS_TXT_MAX_RECLEN);
+memset(dns_txt_reply, 0, PDKIM_DNS_TXT_MAX_RECLEN);
+
+if (  ctx->dns_txt_callback(CS dns_txt_name, CS dns_txt_reply) != PDKIM_OK 
+   || dns_txt_reply[0] == '\0'
+   )
+  {
+  sig->verify_status =      PDKIM_VERIFY_INVALID;
+  sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE;
+  return NULL;
+  }
+
+DEBUG(D_acl)
+  {
+  debug_printf(
+    "PDKIM >> Parsing public key record >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n"
+    " Raw record: ");
+  pdkim_quoteprint(CUS dns_txt_reply, Ustrlen(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;
+
+  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");
+    }
+  return NULL;
+  }
+
+DEBUG(D_acl) debug_printf(
+      "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
+
+/* Import public key */
+if ((errstr = exim_rsa_verify_init(&p->key, vctx)))
+  {
+  DEBUG(D_acl) debug_printf("verify_init: %s\n", errstr);
+  sig->verify_status =      PDKIM_VERIFY_INVALID;
+  sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_IMPORT;
+  return NULL;
+  }
+
+return p;
+}
+
+
+/* -------------------------------------------------------------------------- */
+
 DLLEXPORT int
 pdkim_feed_finish(pdkim_ctx *ctx, pdkim_signature **return_signatures)
 {
@@ -1519,8 +1587,6 @@ while (sig)
     const uschar * errstr;
     pdkim_pubkey * p;
 
-    uschar *dns_txt_name, *dns_txt_reply;
-
     /* Make sure we have all required signature tags */
     if (!(  sig->domain        && *sig->domain
 	 && sig->selector      && *sig->selector
@@ -1552,61 +1618,8 @@ while (sig)
       goto NEXT_VERIFY;
       }
 
-    /* Fetch public key for signing domain, from DNS */
-
-    dns_txt_name = string_sprintf("%s._domainkey.%s.",
-				 sig->selector, sig->domain);
-
-    dns_txt_reply = store_get(PDKIM_DNS_TXT_MAX_RECLEN);
-    memset(dns_txt_reply, 0, PDKIM_DNS_TXT_MAX_RECLEN);
-
-    if (  ctx->dns_txt_callback(CS dns_txt_name, CS dns_txt_reply) != PDKIM_OK 
-       || dns_txt_reply[0] == '\0')
-      {
-      sig->verify_status =      PDKIM_VERIFY_INVALID;
-      sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE;
-      goto NEXT_VERIFY;
-      }
-
-    DEBUG(D_acl)
-      {
-      debug_printf(
-          "PDKIM >> Parsing public key record >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n"
-          " Raw record: ");
-      pdkim_quoteprint(CUS dns_txt_reply, Ustrlen(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;
-
-      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");
-	}
+    if (!(sig->pubkey = pdkim_key_from_dns(ctx, sig, &vctx)))
       goto NEXT_VERIFY;
-      }
-    sig->pubkey = p;
-
-    DEBUG(D_acl) debug_printf(
-	  "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
-
-    /* Import public key */
-    if ((errstr = exim_rsa_verify_init(&sig->pubkey->key, &vctx)))
-      {
-      DEBUG(D_acl) debug_printf("verify_init: %s\n", errstr);
-      sig->verify_status =      PDKIM_VERIFY_INVALID;
-      sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_IMPORT;
-      goto NEXT_VERIFY;
-      }
 
     /* Check the signature */
     if ((errstr = exim_rsa_verify(&vctx, is_sha1, &hhash, &sig->sigdata)))
@@ -1668,22 +1681,24 @@ return ctx;
 /* -------------------------------------------------------------------------- */
 
 DLLEXPORT pdkim_ctx *
-pdkim_init_sign(char *domain, char *selector, char *rsa_privkey, int algo,
-  BOOL dot_stuffed)
+pdkim_init_sign(char * domain, char * selector, char * rsa_privkey, int algo,
+  BOOL dot_stuffed, int(*dns_txt_callback)(char *, char *))
 {
-pdkim_ctx *ctx;
-pdkim_signature *sig;
+pdkim_ctx * ctx;
+pdkim_signature * sig;
 
 if (!domain || !selector || !rsa_privkey)
   return NULL;
 
-ctx = store_get(sizeof(pdkim_ctx));
+ctx = store_get(sizeof(pdkim_ctx) + PDKIM_MAX_BODY_LINE_LEN + sizeof(pdkim_signature));
 memset(ctx, 0, sizeof(pdkim_ctx));
 
 ctx->flags = dot_stuffed ? PDKIM_MODE_SIGN | PDKIM_DOT_TERM : PDKIM_MODE_SIGN;
-ctx->linebuf = store_get(PDKIM_MAX_BODY_LINE_LEN);
+ctx->linebuf = CS (ctx+1);
 
-sig = store_get(sizeof(pdkim_signature));
+DEBUG(D_acl) ctx->dns_txt_callback = dns_txt_callback;
+
+sig = (pdkim_signature *)(ctx->linebuf + PDKIM_MAX_BODY_LINE_LEN);
 memset(sig, 0, sizeof(pdkim_signature));
 
 sig->bodylength = -1;
@@ -1695,6 +1710,18 @@ sig->rsa_privkey = string_copy(US rsa_privkey);
 sig->algo = algo;
 
 exim_sha_init(&sig->body_hash, algo == PDKIM_ALGO_RSA_SHA1 ? HASH_SHA1 : HASH_SHA256);
+
+DEBUG(D_acl)
+  {
+  pdkim_signature s = *sig;
+  ev_ctx vctx;
+
+  debug_printf("PDKIM (checking verify key)<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
+  if (!pdkim_key_from_dns(ctx, &s, &vctx))
+    debug_printf("WARNING: bad dkim key in dns\n");
+  debug_printf("PDKIM (finished checking verify key)<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
+  }
+
 return ctx;
 }
 
diff --git a/src/src/pdkim/pdkim.h b/src/src/pdkim/pdkim.h
index 07ba5b5c4..c1c8c262e 100644
--- a/src/src/pdkim/pdkim.h
+++ b/src/src/pdkim/pdkim.h
@@ -285,7 +285,8 @@ extern "C" {
 void	   pdkim_init         (void);
 
 DLLEXPORT
-pdkim_ctx *pdkim_init_sign    (char *, char *, char *, int, BOOL);
+pdkim_ctx *pdkim_init_sign    (char *, char *, char *, int,
+			      BOOL, int(*)(char *, char *));
 
 DLLEXPORT
 pdkim_ctx *pdkim_init_verify  (int(*)(char *, char *), BOOL);
diff --git a/test/confs/4503 b/test/confs/4503
index 47f4b74c9..cbd82d8f5 100644
--- a/test/confs/4503
+++ b/test/confs/4503
@@ -37,7 +37,11 @@ send_to_server:
   port = PORT_D
 
   dkim_domain =		test.ex
+.ifdef SELECTOR
+  dkim_selector =	SELECTOR
+.else
   dkim_selector =	sel
+.endif
   dkim_private_key =	DIR/aux-fixed/dkim/dkim.private
 .ifndef HEADERS_MAXSIZE
   dkim_sign_headers =	OPT
diff --git a/test/log/4503 b/test/log/4503
index 53781b966..e9736fd6f 100644
--- a/test/log/4503
+++ b/test/log/4503
@@ -7,6 +7,9 @@
 1999-03-02 09:44:33 10HmbB-0005vi-00 <= CALLER@myhost.test.ex U=CALLER P=local S=sss
 1999-03-02 09:44:33 10HmbB-0005vi-00 => c@test.ex R=client T=send_to_server H=ip4.ip4.ip4.ip4 [ip4.ip4.ip4.ip4] C="250 OK id=10HmbC-0005vi-00"
 1999-03-02 09:44:33 10HmbB-0005vi-00 Completed
+1999-03-02 09:44:33 10HmbD-0005vi-00 <= CALLER@myhost.test.ex U=CALLER P=local S=sss
+1999-03-02 09:44:33 10HmbD-0005vi-00 => d@test.ex R=client T=send_to_server H=ip4.ip4.ip4.ip4 [ip4.ip4.ip4.ip4] C="250 OK id=10HmbE-0005vi-00"
+1999-03-02 09:44:33 10HmbD-0005vi-00 Completed
 
 ******** SERVER ********
 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
@@ -25,3 +28,8 @@
 1999-03-02 09:44:33 10HmbC-0005vi-00 <= CALLER@myhost.test.ex H=the.local.host.name (myhost.test.ex) [ip4.ip4.ip4.ip4] P=esmtp S=sss id=E10HmbB-0005vi-00@myhost.test.ex
 1999-03-02 09:44:33 10HmbC-0005vi-00 => :blackhole: <c@test.ex> R=server_dump
 1999-03-02 09:44:33 10HmbC-0005vi-00 Completed
+1999-03-02 09:44:33 10HmbE-0005vi-00 DKIM: d=test.ex s=sel_bad c=relaxed/relaxed a=rsa-sha256 b=1024 [invalid - syntax error in public key record]
+1999-03-02 09:44:33 10HmbE-0005vi-00 signer: test.ex bits: 1024 h=Date:Sender:Message-Id:From:Reply-To:Subject:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive
+1999-03-02 09:44:33 10HmbE-0005vi-00 <= CALLER@myhost.test.ex H=the.local.host.name (myhost.test.ex) [ip4.ip4.ip4.ip4] P=esmtp S=sss id=E10HmbD-0005vi-00@myhost.test.ex
+1999-03-02 09:44:33 10HmbE-0005vi-00 => :blackhole: <d@test.ex> R=server_dump
+1999-03-02 09:44:33 10HmbE-0005vi-00 Completed
diff --git a/test/runtest b/test/runtest
index 1ee61a5a1..bbb46542a 100755
--- a/test/runtest
+++ b/test/runtest
@@ -1081,6 +1081,10 @@ RESET_AFTER_EXTRA_LINE_READ:
     # Not all platforms build with DKIM enabled
     next if /^PDKIM >> Body data for hash, canonicalized/;
 
+    #  Parts of DKIM-specific debug output depend on the time/date
+    next if /^date:\w+,{SP}/;
+    next if /^PDKIM \[[^[]+\] (Header hash|b) computed:/;
+
     # Not all platforms support TCP Fast Open, and the compile omits the check
     if (s/\S+ in hosts_try_fastopen\? no \(option unset\)\n$//)
       {
diff --git a/test/scripts/4500-Domain-Keys-Identified-Mail/4500 b/test/scripts/4500-Domain-Keys-Identified-Mail/4500
index 56283992b..6728b141d 100644
--- a/test/scripts/4500-Domain-Keys-Identified-Mail/4500
+++ b/test/scripts/4500-Domain-Keys-Identified-Mail/4500
@@ -1,4 +1,4 @@
-# DKIM simple canonicalisation
+# DKIM verify, simple canonicalisation
 #
 exim -DSERVER=server -bd -oX PORT_D
 ****
diff --git a/test/scripts/4500-Domain-Keys-Identified-Mail/4501 b/test/scripts/4500-Domain-Keys-Identified-Mail/4501
index 5a4bf2be6..f2c78fa9f 100644
--- a/test/scripts/4500-Domain-Keys-Identified-Mail/4501
+++ b/test/scripts/4500-Domain-Keys-Identified-Mail/4501
@@ -1,4 +1,4 @@
-# DKIM simple canonicalisation, with spaces
+# DKIM verify, simple canonicalisation, with spaces
 #
 exim -DSERVER=server -bd -oX PORT_D
 ****
diff --git a/test/scripts/4500-Domain-Keys-Identified-Mail/4502 b/test/scripts/4500-Domain-Keys-Identified-Mail/4502
index 1e0005b46..fb61997c2 100644
--- a/test/scripts/4500-Domain-Keys-Identified-Mail/4502
+++ b/test/scripts/4500-Domain-Keys-Identified-Mail/4502
@@ -130,7 +130,7 @@ QUIT
 ??? 221
 ****
 #
-# This should fail, but passes - bug 1926 - due to an extra \ in the DNS record.
+# This should fail, 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
diff --git a/test/scripts/4500-Domain-Keys-Identified-Mail/4503 b/test/scripts/4500-Domain-Keys-Identified-Mail/4503
index 140e6d6bf..6efe3545a 100644
--- a/test/scripts/4500-Domain-Keys-Identified-Mail/4503
+++ b/test/scripts/4500-Domain-Keys-Identified-Mail/4503
@@ -24,6 +24,14 @@ From: nobody@example.com
 
 content
 ****
+#
+# check that on signing we warn in debug mode about verify problems
+exim -d-all+acl -DHEADERS_MAXSIZE=y -DSELECTOR=sel_bad -odf d@test.ex
+From: nobody@example.com
+
+content
+****
+#
 millisleep 500
 killdaemon
 no_msglog_check
diff --git a/test/stderr/4503 b/test/stderr/4503
new file mode 100644
index 000000000..c2a856bb1
--- /dev/null
+++ b/test/stderr/4503
@@ -0,0 +1,54 @@
+Exim version x.yz ....
+configuration file is TESTSUITE/test-config
+admin user
+LOG: MAIN
+  <= CALLER@myhost.test.ex U=CALLER P=local S=sss
+Exim version x.yz ....
+configuration file is TESTSUITE/test-config
+trusted user
+admin user
+Connecting to ip4.ip4.ip4.ip4 [ip4.ip4.ip4.ip4]:1225 ... connected
+  SMTP<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
+  SMTP>> EHLO myhost.test.ex
+cmd buf flush ddd bytes
+  SMTP<< 250-myhost.test.ex Hello the.local.host.name [ip4.ip4.ip4.ip4]
+         250-SIZE 52428800
+         250-8BITMIME
+         250-PIPELINING
+         250 HELP
+  SMTP>> MAIL FROM:<CALLER@myhost.test.ex> SIZE=ssss
+  SMTP>> RCPT TO:<d@test.ex>
+  SMTP>> DATA
+cmd buf flush ddd bytes
+  SMTP<< 250 OK
+  SMTP<< 250 Accepted
+  SMTP<< 354 Enter message, ending with "." on a line by itself
+PDKIM (checking verify key)<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
+PDKIM >> Parsing public key record >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
+ Raw record: v=DKIM1\;{SP}p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDXRFf+VhT+lCgFhhSkinZKcFNeRzjYdW8vT29Rbb3NadvTFwAd+cVLPFwZL8H5tUD/7JbUPqNTCPxmpgIL+V5T4tEZMorHatvvUM2qfcpQ45IfsZ+YdhbIiAslHCpy4xNxIR3zylgqRUF4+Dtsaqy3a5LhwMiKCLrnzhXk1F1hxwIDAQAB
+ v=DKIM1\
+ p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDXRFf+VhT+lCgFhhSkinZKcFNeRzjYdW8vT29Rbb3NadvTFwAd+cVLPFwZL8H5tUD/7JbUPqNTCPxmpgIL+V5T4tEZMorHatvvUM2qfcpQ45IfsZ+YdhbIiAslHCpy4xNxIR3zylgqRUF4+Dtsaqy3a5LhwMiKCLrnzhXk1F1hxwIDAQAB
+ Error while parsing public key record
+WARNING: bad dkim key in dns
+PDKIM (finished checking verify key)<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
+content{CR}{LF}
+PDKIM [test.ex] Body bytes hashed: 9
+PDKIM [test.ex] Body hash computed: fc06f48221d98ad6106c3845b33a2a41152482ab9e697f736ad26db4853fa657
+PDKIM >> Header data for hash, canonicalized, in sequence >>>>>>>>>>>>>>
+sender:CALLER_NAME{SP}<CALLER@myhost.test.ex>{CR}{LF}
+message-id:<E10HmbD-0005vi-00@myhost.test.ex>{CR}{LF}
+from:nobody@example.com{CR}{LF}
+PDKIM >> Signed DKIM-Signature header, canonicalized >>>>>>>>>>>>>>>>>
+dkim-signature:v=1;{SP}a=rsa-sha256;{SP}q=dns/txt;{SP}c=relaxed/relaxed;{SP}d=test.ex;{SP}s=sel_bad;{SP}h=Date:Sender:Message-Id:From:Reply-To:Subject:To:Cc:MIME-Version:{SP}Content-Type:Content-Transfer-Encoding:Content-ID:Content-Description:{SP}Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:{SP}In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:{SP}List-Post:List-Owner:List-Archive;{SP}bh=/Ab0giHZitYQbDhFszoqQRUkgqueaX9zatJttIU/plc=;{SP}b=;
+  SMTP<< 250 OK id=10HmbE-0005vi-00
+  SMTP>> QUIT
+cmd buf flush ddd bytes
+  SMTP(close)>>
+LOG: MAIN
+  => d@test.ex R=client T=send_to_server H=ip4.ip4.ip4.ip4 [ip4.ip4.ip4.ip4] C="250 OK id=10HmbE-0005vi-00"
+LOG: MAIN
+  Completed
+>>>>>>>>>>>>>>>> Exim pid=pppp terminating with rc=0 >>>>>>>>>>>>>>>>
+>>>>>>>>>>>>>>>> Exim pid=pppp terminating with rc=0 >>>>>>>>>>>>>>>>
+
+******** SERVER ********