From 135e949699b889c8c9088bb05f810d44adc74246 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Wed, 28 Jun 2017 15:25:12 +0100 Subject: [PATCH] DKIM: Enforce any "h" field present in the DNS publickey record. This can be set to require specific hash types, eg sha256, in signatues. There is an IETF draft in discussion which deprecates sha1 so this feature may start to be used. --- doc/doc-txt/ChangeLog | 3 +++ src/src/pdkim/pdkim.c | 33 +++++++++++++++++++++++++++------ src/src/pdkim/pdkim.h | 15 ++++++++------- test/dnszones-src/db.test.ex | 6 ++++++ test/log/4500 | 3 +++ test/log/4506 | 3 +++ test/scripts/4500-DKIM/4500 | 34 ++++++++++++++++++++++++++++++++++ test/scripts/4500-DKIM/4506 | 35 +++++++++++++++++++++++++++++++++++ test/stderr/4520 | 1 + 9 files changed, 120 insertions(+), 13 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 0b4076c20..ecb6b33a9 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -138,6 +138,9 @@ JH/21 Bug 2151: Avoid using SIZE on the MAIL for a callout verify, on any but JH/22 Retire historical build files to an "unsupported" subdir. These are defined as "ones for which we have no current evidence of testing". +JH/23 DKIM: enforce the DNS pubkey record "h" permitted-hashes optional field, + if present. Previously it was ignored. + Exim version 4.89 ----------------- diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c index d41b60f13..d289ec7a3 100644 --- a/src/src/pdkim/pdkim.c +++ b/src/src/pdkim/pdkim.c @@ -132,6 +132,7 @@ switch(ext_status) { case PDKIM_VERIFY_FAIL_BODY: return "PDKIM_VERIFY_FAIL_BODY"; case PDKIM_VERIFY_FAIL_MESSAGE: return "PDKIM_VERIFY_FAIL_MESSAGE"; + case PDKIM_VERIFY_FAIL_SIG_ALGO_MISMATCH: return "PDKIM_VERIFY_FAIL_SIG_ALGO_MISMATCH"; case PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE: return "PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE"; case PDKIM_VERIFY_INVALID_BUFFER_SIZE: return "PDKIM_VERIFY_INVALID_BUFFER_SIZE"; case PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD: return "PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD"; @@ -644,12 +645,8 @@ for (p = raw_record; ; p++) case 'v': pub->version = string_copy(cur_val); break; case 'h': + pub->hashes = string_copy(cur_val); break; 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; @@ -682,7 +679,11 @@ a 'k' (key-type_ would go in this field name. There is a field /* 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; +else if (Ustrcmp(pub->version, PDKIM_PUB_RECORD_VERSION) != 0) + { + DEBUG(D_acl) debug_printf(" Bad v= field\n"); + return NULL; + } if (!pub->granularity) pub->granularity = string_copy(US"*"); /* @@ -694,6 +695,7 @@ if (!pub->srvtype ) pub->srvtype = string_copy(US"*"); if (pub->key.data) return pub; +DEBUG(D_acl) debug_printf(" Missing p= field\n"); return NULL; } @@ -1631,6 +1633,25 @@ while (sig) if (!(sig->pubkey = pdkim_key_from_dns(ctx, sig, &vctx, err))) goto NEXT_VERIFY; + /* If the pubkey limits to a list of specific hashes, ignore sigs that + do not have the hash part of the sig algorithm matching */ + + if (sig->pubkey->hashes) + { + const uschar * list = sig->pubkey->hashes, * ele; + int sep = ':'; + while ((ele = string_nextinlist(&list, &sep, NULL, 0))) + if (Ustrcmp(ele, pdkim_algos[sig->algo] + 4) == 0) break; + if (!ele) + { + DEBUG(D_acl) debug_printf("pubkey h=%s vs sig a=%s\n", + sig->pubkey->hashes, pdkim_algos[sig->algo]); + sig->verify_status = PDKIM_VERIFY_FAIL; + sig->verify_ext_status = PDKIM_VERIFY_FAIL_SIG_ALGO_MISMATCH; + goto NEXT_VERIFY; + } + } + /* Check the signature */ if ((*err = exim_rsa_verify(&vctx, is_sha1, &hhash, &sig->sighash))) { diff --git a/src/src/pdkim/pdkim.h b/src/src/pdkim/pdkim.h index 78e3c3c8b..9899356d5 100644 --- a/src/src/pdkim/pdkim.h +++ b/src/src/pdkim/pdkim.h @@ -51,12 +51,13 @@ #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 +#define PDKIM_VERIFY_FAIL_SIG_ALGO_MISMATCH 3 +#define PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE 4 +#define PDKIM_VERIFY_INVALID_BUFFER_SIZE 5 +#define PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD 6 +#define PDKIM_VERIFY_INVALID_PUBKEY_IMPORT 7 +#define PDKIM_VERIFY_INVALID_SIGNATURE_ERROR 8 +#define PDKIM_VERIFY_INVALID_DKIM_VERSION 9 /* -------------------------------------------------------------------------- */ /* Some parameter values */ @@ -100,8 +101,8 @@ typedef struct pdkim_pubkey { uschar *version; /* v= */ uschar *granularity; /* g= */ -#ifdef notdef uschar *hashes; /* h= */ +#ifdef notdef uschar *keytype; /* k= */ #endif uschar *srvtype; /* s= */ diff --git a/test/dnszones-src/db.test.ex b/test/dnszones-src/db.test.ex index f7c9e313b..73db57f9c 100644 --- a/test/dnszones-src/db.test.ex +++ b/test/dnszones-src/db.test.ex @@ -491,13 +491,19 @@ DELAY=1500 delay1500 A HOSTIPV4 ; openssl rsa -in aux-fixed/dkim/dkim.private -out /dev/stdout -pubout -outform PEM ; ; Deliberate bad version, having extra backslashes +; sha256-hash-only version.... appears to be too long, gets truncated ; ; Another, 512-bit (with a Notes field) +; 512 requiring sha1 hash +; 512 requiring sha256 hash ; 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" +sel_sha256._domainkey TXT "v=DKIM1; h=sha256; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDXRFf+VhT+lCgFhhSkinZKcFNeRzjYdW8vT29Rbb3NadvTFwAd+cVLPFwZL8H5tUD/7JbUPqNTCPxmpgIL+V5T4tEZMorHatvvUM2qfcpQ45IfsZ+YdhbIiAslHCpy4xNxIR3zylgqRUF4+Dtsaqy3a5LhwMiKCLrnzhXk1F1hxwIDAQAB" ses._domainkey TXT "v=DKIM1; n=halfkilo; p=MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAL6eAQxd9didJ0/+05iDwJOqT6ly826Vi8aGPecsBiYK5/tAT97fxXk+dPWMZp9kQxtknEzYjYjAydzf+HQ2yJMCAwEAAQ==" +ses_sha1._domainkey TXT "v=DKIM1; h=sha1; p=MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAL6eAQxd9didJ0/+05iDwJOqT6ly826Vi8aGPecsBiYK5/tAT97fxXk+dPWMZp9kQxtknEzYjYjAydzf+HQ2yJMCAwEAAQ==" +ses_sha256._domainkey TXT "v=DKIM1; h=sha256; p=MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAL6eAQxd9didJ0/+05iDwJOqT6ly826Vi8aGPecsBiYK5/tAT97fxXk+dPWMZp9kQxtknEzYjYjAydzf+HQ2yJMCAwEAAQ==" ; End diff --git a/test/log/4500 b/test/log/4500 index 0e0f8400d..ec8ef088e 100644 --- a/test/log/4500 +++ b/test/log/4500 @@ -10,3 +10,6 @@ 1999-03-02 09:44:33 10HmaZ-0005vi-00 DKIM: d=test.ex s=sel c=simple/simple a=rsa-sha256 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 id=qwerty1234@disco-zombie.net +1999-03-02 09:44:33 10HmbA-0005vi-00 DKIM: d=test.ex s=ses_sha1 c=simple/simple a=rsa-sha1 b=512 [verification succeeded] +1999-03-02 09:44:33 10HmbA-0005vi-00 signer: test.ex bits: 512 +1999-03-02 09:44:33 10HmbA-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=qwerty1234@disco-zombie.net diff --git a/test/log/4506 b/test/log/4506 index fb0f22567..027169df0 100644 --- a/test/log/4506 +++ b/test/log/4506 @@ -13,3 +13,6 @@ 1999-03-02 09:44:33 10HmbA-0005vi-00 DKIM: validation error: RSA_LONG_LINE 1999-03-02 09:44:33 10HmbA-0005vi-00 DKIM: Error during validation, disabling signature verification: RSA_LONG_LINE 1999-03-02 09:44:33 10HmbA-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=qwerty1234@disco-zombie.net +1999-03-02 09:44:33 10HmbB-0005vi-00 DKIM: d=test.ex s=ses_sha256 c=simple/simple a=rsa-sha1 b=512 [verification failed - unspecified reason] +1999-03-02 09:44:33 10HmbB-0005vi-00 signer: test.ex bits: 512 +1999-03-02 09:44:33 10HmbB-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=qwerty1234@disco-zombie.net diff --git a/test/scripts/4500-DKIM/4500 b/test/scripts/4500-DKIM/4500 index 6728b141d..6b3ff5fcf 100644 --- a/test/scripts/4500-DKIM/4500 +++ b/test/scripts/4500-DKIM/4500 @@ -93,6 +93,40 @@ Date: Thu, 19 Nov 2015 17:00:07 -0700 Message-ID: Subject: simple test +This is a simple test. +. +??? 250 +QUIT +??? 221 +**** +# +# +# This should pass. The pubkey dns decord has a additional sha1-only h= field +# +# - sha1, 512b +# Mail original in aux-fixed/4500.msg1.txt +# Sig generated by: perl aux-fixed/dkim/sign.pl --keyfile=aux-fixed/dkim/dkim512.private \ +# --method=simple/simple --selector=ses_sha1 < aux-fixed/4500.msg1.txt +client 127.0.0.1 PORT_D +??? 220 +HELO xxx +??? 250 +MAIL FROM: +??? 250 +RCPT TO: +??? 250 +DATA +??? 354 +DKIM-Signature: v=1; a=rsa-sha1; c=simple/simple; d=test.ex; h=from:to + :date:message-id:subject; s=ses_sha1; bh=OB9dZVu7+5/ufs3TH9leIcE + pXSo=; b=hG14R3Eb/f13Pw6J0LmovHAL01KHVmVrTZ7KJrqieYTQemUaseoU2pB + 7/g8NUwG/AsYoaw3gaAK8PqxSk2lcIQ== +From: mrgus@text.ex +To: bakawolf@yahoo.com +Date: Thu, 19 Nov 2015 17:00:07 -0700 +Message-ID: +Subject: simple test + This is a simple test. . ??? 250 diff --git a/test/scripts/4500-DKIM/4506 b/test/scripts/4500-DKIM/4506 index 6eb81cc16..e8d7c41f0 100644 --- a/test/scripts/4500-DKIM/4506 +++ b/test/scripts/4500-DKIM/4506 @@ -134,6 +134,41 @@ QUIT **** # # +# This should fail as the sig on the mail uses sha1 but the dns record requires sha256 +# +# - sha256, 512b +# Mail original in aux-fixed/4500.msg1.txt +# Sig generated by: perl aux-fixed/dkim/sign.pl --keyfile=aux-fixed/dkim/dkim512.private \ +# --method=simple/simple --selector=ses_sha1 < aux-fixed/4500.msg1.txt +# and then modifying the s= manually +client 127.0.0.1 PORT_D +??? 220 +HELO xxx +??? 250 +MAIL FROM: +??? 250 +RCPT TO: +??? 250 +DATA +??? 354 +DKIM-Signature: v=1; a=rsa-sha1; c=simple/simple; d=test.ex; h=from:to + :date:message-id:subject; s=ses_sha256; bh=OB9dZVu7+5/ufs3TH9leIcE + pXSo=; b=hG14R3Eb/f13Pw6J0LmovHAL01KHVmVrTZ7KJrqieYTQemUaseoU2pB + 7/g8NUwG/AsYoaw3gaAK8PqxSk2lcIQ== +From: mrgus@text.ex +To: bakawolf@yahoo.com +Date: Thu, 19 Nov 2015 17:00:07 -0700 +Message-ID: +Subject: simple test + +This is a simple test. +. +??? 250 +QUIT +??? 221 +**** +# +# killdaemon no_stdout_check no_msglog_check diff --git a/test/stderr/4520 b/test/stderr/4520 index c1bafcdcc..fc64a9e93 100644 --- a/test/stderr/4520 +++ b/test/stderr/4520 @@ -28,6 +28,7 @@ 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 + Bad v= field Error while parsing public key record WARNING: bad dkim key in dns PDKIM (finished checking verify key)<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< -- 2.25.1