summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeremy Harris <jgh146exb@wizmail.org>2016-11-22 15:22:11 +0000
committerJeremy Harris <jgh146exb@wizmail.org>2016-12-29 19:33:13 +0000
commite21a4d0042e48109cef06e48b8a73dd79d7a4330 (patch)
tree5b9dae46a46987570b3f0576c687ad94a59ae740
parentb999c48327cd31b50bfdbda9ce6a23ea3ee25730 (diff)
DKIM: More validation of DNS key record. Bug 1926
-rw-r--r--src/src/pdkim/pdkim.c161
-rw-r--r--src/src/pdkim/pdkim.h3
-rw-r--r--test/dnszones-src/db.test.ex3
-rw-r--r--test/log/45023
-rw-r--r--test/scripts/4500-Domain-Keys-Identified-Mail/450254
5 files changed, 142 insertions, 82 deletions
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 7bfcdf4aa..c4bdb5a93 100644
--- 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++)
- {
- 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);
+else if (Ustrcmp(pub->version, PDKIM_PUB_RECORD_VERSION) != 0) return NULL;
+
if (!pub->granularity) pub->granularity = string_copy(US"*");
+/*
if (!pub->keytype ) pub->keytype = string_copy(US"rsa");
+*/
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"
- "PDKIM [%s] bh computed: ",
+ "PDKIM [%s] Body hash computed: ",
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_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);
@@ -1341,7 +1338,7 @@ while (sig)
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
@@ -1471,7 +1468,7 @@ while (sig)
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);
}
@@ -1520,6 +1517,7 @@ while (sig)
{
ev_ctx vctx;
const uschar * errstr;
+ pdkim_pubkey * p;
uschar *dns_txt_name, *dns_txt_reply;
@@ -1578,16 +1576,25 @@ while (sig)
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;
- 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");
+ }
goto NEXT_VERIFY;
}
+ sig->pubkey = p;
DEBUG(D_acl) debug_printf(
"PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
diff --git a/src/src/pdkim/pdkim.h b/src/src/pdkim/pdkim.h
index 536e5af69..07ba5b5c4 100644
--- 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= */
+#ifdef notdef
uschar *hashes; /* h= */
uschar *keytype; /* k= */
+#endif
uschar *srvtype; /* s= */
uschar *notes; /* n= */
blob key; /* p= */
-
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 6555ec81d..16468bf79 100644
--- 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
;
+; 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"
+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=="
diff --git a/test/log/4502 b/test/log/4502
index ab5273ad0..b7e4a8ddd 100644
--- 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 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 5e63f129f..1e0005b46 100644
--- 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
-# 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
@@ -52,7 +52,7 @@ QUIT
#
# 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
@@ -93,7 +93,7 @@ QUIT
#
# 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
@@ -130,6 +130,52 @@ QUIT
??? 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