DKIM: error verification on missing tags. Bug 1853

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 128aef3d139a593c9c63201737dad49b43f5b4fd..ca64f9f1187872a0e6ee118c24c3b036fd88e833 100644 (file)
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -38115,7 +38115,7 @@ used.
 .section "Verifying DKIM signatures in incoming mail" "SECID514"
 .cindex "DKIM" "verification"
 
-Verification of DKIM signatures in incoming email is implemented via the
+Verification of DKIM signatures in SMTP incoming email is implemented via the
 &%acl_smtp_dkim%& ACL. By default, this ACL is called once for each
 syntactically(!) correct signature in the incoming message.
 A missing ACL definition defaults to accept.

diff --git a/src/src/dkim.c b/src/src/dkim.c
index 4e20f14f26b8ca0246f97504538edc7337fd34fe..24de2bc332f090d317ed9a5f66bf20f510cef232 100644 (file)
--- a/src/src/dkim.c
+++ b/src/src/dkim.c
@@ -148,10 +148,12 @@ for (sig = dkim_signatures; sig; sig = sig->next)
        string_sprintf("d=%s s=%s c=%s/%s a=%s b=%d ",
              sig->domain,
              sig->selector,
-             sig->canon_headers == PDKIM_CANON_SIMPLE ?  "simple" : "relaxed",
-             sig->canon_body == PDKIM_CANON_SIMPLE ?  "simple" : "relaxed",
-             sig->algo == PDKIM_ALGO_RSA_SHA256 ?  "rsa-sha256" : "rsa-sha1",
-             sig->sigdata.len * 8
+             sig->canon_headers == PDKIM_CANON_SIMPLE ? "simple" : "relaxed",
+             sig->canon_body == PDKIM_CANON_SIMPLE ? "simple" : "relaxed",
+             sig->algo == PDKIM_ALGO_RSA_SHA256
+             ? "rsa-sha256"
+             : sig->algo == PDKIM_ALGO_RSA_SHA1 ? "rsa-sha1" : "err",
+             (int)sig->sigdata.len > -1 ? sig->sigdata.len * 8 : 0
              ),
 
        sig->identity ? string_sprintf("i=%s ", sig->identity) : US"",
@@ -186,6 +188,16 @@ for (sig = dkim_signatures; sig; sig = sig->next)
                       "syntax error in public key record]");
          break;
 
+        case PDKIM_VERIFY_INVALID_SIGNATURE_ERROR:
+          logmsg = string_append(logmsg, &size, &ptr, 1,
+                       "signature tag missing or invalid]");
+          break;
+
+        case PDKIM_VERIFY_INVALID_DKIM_VERSION:
+          logmsg = string_append(logmsg, &size, &ptr, 1,
+                       "unsupported DKIM version]");
+          break;
+
        default:
          logmsg = string_append(logmsg, &size, &ptr, 1,
                        "unspecified problem]");

diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 29277baeb60acb641b4c3e53aad128931031e4a9..a0f0226859d8e22f3f10aea441fe32cc2e893888 100644 (file)
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -134,6 +134,8 @@ pdkim_verify_ext_status_str(int ext_status)
     case PDKIM_VERIFY_INVALID_BUFFER_SIZE: return "PDKIM_VERIFY_INVALID_BUFFER_SIZE";
     case PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD: return "PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD";
     case PDKIM_VERIFY_INVALID_PUBKEY_IMPORT: return "PDKIM_VERIFY_INVALID_PUBKEY_IMPORT";
+    case PDKIM_VERIFY_INVALID_SIGNATURE_ERROR: return "PDKIM_VERIFY_INVALID_SIGNATURE_ERROR";
+    case PDKIM_VERIFY_INVALID_DKIM_VERSION: return "PDKIM_VERIFY_INVALID_DKIM_VERSION";
     default: return "PDKIM_VERIFY_UNKNOWN";
   }
 }
@@ -407,6 +409,10 @@ sig = store_get(sizeof(pdkim_signature));
 memset(sig, 0, sizeof(pdkim_signature));
 sig->bodylength = -1;
 
+/* Set so invalid/missing data error display is accurate */
+sig->algo = -1;
+sig->version = 0;
+
 q = sig->rawsig_no_b_val = store_get(Ustrlen(raw_hdr)+1);
 
 for (p = raw_hdr; ; p++)
@@ -476,8 +482,8 @@ for (p = raw_hdr; ; p++)
          case 'v':
              /* We only support version 1, and that is currently the
                 only version there is. */
-           if (Ustrcmp(cur_val, PDKIM_SIGNATURE_VERSION) == 0)
-             sig->version = 1;
+           sig->version =
+             Ustrcmp(cur_val, PDKIM_SIGNATURE_VERSION) == 0 ? 1 : -1;
            break;
          case 'a':
            for (i = 0; pdkim_algos[i]; i++)
@@ -542,10 +548,6 @@ NEXT_CHAR:
     *q++ = c;
   }
 
-/* Make sure the most important bits are there. */
-if (!sig->version)
-  return NULL;
-
 *q = '\0';
 /* Chomp raw header. The final newline must not be added to the signature. */
 while (--q > sig->rawsig_no_b_val  && (*q == '\r' || *q == '\n'))
@@ -1475,6 +1477,37 @@ while (sig)
 
     uschar *dns_txt_name, *dns_txt_reply;
 
+    /* Make sure we have all required signature tags */
+    if (!(  sig->domain        && *sig->domain
+        && sig->selector      && *sig->selector
+        && sig->headernames   && *sig->headernames
+        && sig->bodyhash.data
+        && sig->sigdata.data
+        && sig->algo > -1
+        && sig->version
+       ) )
+      {
+      sig->verify_status     = PDKIM_VERIFY_INVALID;
+      sig->verify_ext_status = PDKIM_VERIFY_INVALID_SIGNATURE_ERROR;
+
+      DEBUG(D_acl) debug_printf(
+         " Error in DKIM-Signature header: tags missing or invalid\n"
+         "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
+      goto NEXT_VERIFY;
+      }
+
+    /* Make sure sig uses supported DKIM version (only v1) */
+    if (sig->version != 1)
+      {
+      sig->verify_status     = PDKIM_VERIFY_INVALID;
+      sig->verify_ext_status = PDKIM_VERIFY_INVALID_DKIM_VERSION;
+
+      DEBUG(D_acl) debug_printf(
+          " Error in DKIM-Signature header: unsupported DKIM version\n"
+          "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
+      goto NEXT_VERIFY;
+      }
+
     /* Fetch public key for signing domain, from DNS */
 
     dns_txt_name = string_sprintf("%s._domainkey.%s.",

diff --git a/src/src/pdkim/pdkim.h b/src/src/pdkim/pdkim.h
index ba984c1d9e51db8183af81c309a0d1e2d9ee1aba..0803ea0b08b8cb8d1125febfc203f20bbf04fe7b 100644 (file)
--- a/src/src/pdkim/pdkim.h
+++ b/src/src/pdkim/pdkim.h
@@ -49,12 +49,14 @@
 #define PDKIM_VERIFY_FAIL      2
 #define PDKIM_VERIFY_PASS      3
 
-#define PDKIM_VERIFY_FAIL_BODY                  1
-#define PDKIM_VERIFY_FAIL_MESSAGE               2
-#define PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE 3
-#define PDKIM_VERIFY_INVALID_BUFFER_SIZE        4
-#define PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD   5
-#define PDKIM_VERIFY_INVALID_PUBKEY_IMPORT      6
+#define PDKIM_VERIFY_FAIL_BODY                    1
+#define PDKIM_VERIFY_FAIL_MESSAGE                 2
+#define PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE   3
+#define PDKIM_VERIFY_INVALID_BUFFER_SIZE          4
+#define PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD     5
+#define PDKIM_VERIFY_INVALID_PUBKEY_IMPORT        6
+#define PDKIM_VERIFY_INVALID_SIGNATURE_ERROR      7
+#define PDKIM_VERIFY_INVALID_DKIM_VERSION         8
 
 /* -------------------------------------------------------------------------- */
 /* Some parameter values */

diff --git a/src/src/verify.c b/src/src/verify.c
index 3624af0bc9e428c311d199b7ffc6acd26fd10b26..b52533f7abbfd08d35cf5a3a9487fc1448d50b44 100644 (file)
--- a/src/src/verify.c
+++ b/src/src/verify.c
@@ -889,7 +889,9 @@ can do it there for the non-rcpt-verify case.  For this we keep an addresscount.
     /* Need proper integration with the proper transport mechanism. */
     if (cutthrough.delivery)
       {
+#ifndef DISABLE_DKIM
       uschar * s;
+#endif
       if (addr->transport->filter_command)
         {
         cutthrough.delivery = FALSE;