From be24b950ae0db88b1c9811b3a028e95133c55efa Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sat, 25 Apr 2020 20:50:07 +0100 Subject: [PATCH] DKIM: dkim_verify_min_keysizes option --- doc/doc-docbook/spec.xfpt | 20 +++++++++++-- doc/doc-txt/ChangeLog | 3 ++ doc/doc-txt/NewStuff | 18 ++++++------ doc/doc-txt/OptionLists.txt | 1 + src/src/dkim.c | 9 ++++++ src/src/expand.c | 4 +-- src/src/functions.h | 2 ++ src/src/globals.c | 1 + src/src/globals.h | 1 + src/src/pdkim/pdkim.c | 14 +++++++++ src/src/pdkim/pdkim.h | 5 ++-- src/src/readconf.c | 1 + test/confs/4500 | 3 ++ test/confs/4520 | 1 + test/log/4500 | 5 ++++ test/log/4540 | 5 ++++ test/scripts/4500-DKIM/4500 | 44 +++++++++++++++++++++++++++-- test/scripts/4540-DKIM-Ed25519/4540 | 41 +++++++++++++++++++++++++++ test/stderr/4507 | 8 +++--- 19 files changed, 165 insertions(+), 21 deletions(-) diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index 870248570..bf042ac2f 100644 --- a/doc/doc-docbook/spec.xfpt +++ b/doc/doc-docbook/spec.xfpt @@ -14590,6 +14590,7 @@ See also the &'Policy controls'& section above. .table2 .row &%dkim_verify_hashes%& "DKIM hash methods accepted for signatures" .row &%dkim_verify_keytypes%& "DKIM key types accepted for signatures" +.row &%dkim_verify_min_keysizes%& "DKIM key sizes accepted for signatures" .row &%dkim_verify_signers%& "DKIM domains for which DKIM ACL is run" .row &%host_lookup%& "host name looked up for these hosts" .row &%host_lookup_order%& "order of DNS and local name lookups" @@ -15364,6 +15365,16 @@ This option gives a list of key types which are acceptable in signatures, and an order of processing. Signatures with algorithms not in the list will be ignored. + +.new +.option dkim_verify_min_keysizes main "string list" "rsa=1024 ed25519=250" +This option gives a list of key sizes which are acceptable in signatures. +The list is keyed by the algorithm type for the key; the values are in bits. +Signatures with keys smaller than given by this option will fail verification. + +The default enforces the RFC 8301 minimum key size for RSA signatures. +.wen + .option dkim_verify_minimal main boolean false If set to true, verification of signatures will terminate after the first success. @@ -40733,6 +40744,10 @@ Notes from the key record (tag n=). .vitem &%$dkim_key_length%& Number of bits in the key. +.new +Valid only once the key is loaded, which is at the time the header signature +is verified, which is after the body hash is. +.wen Note that RFC 8301 says: .code @@ -40740,9 +40755,8 @@ Verifiers MUST NOT consider signatures using RSA keys of less than 1024 bits as valid signatures. .endd -To enforce this you must have a DKIM ACL which checks this variable -and overwrites the &$dkim_verify_status$& variable as discussed above. -As EC keys are much smaller, the check should only do this for RSA keys. +This is enforced by the default setting for the &%dkim_verify_min_keysizes%& +option. .endlist diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 99ad7d1a5..9fd526b08 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -176,6 +176,9 @@ JH/38 Fix $dkim_key_length. This should, after a DKIM verification, present the size of the signing public-key. Previously it was instead giving the size of the signature hash. +JH/39 DKIM verification: the RFC 8301 restriction on sizes of RSA keys is now + the default. See the (new) dkim_verify_min_keysizes option. + Exim version 4.93 ----------------- diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff index 4ae49c2fa..b79802103 100644 --- a/doc/doc-txt/NewStuff +++ b/doc/doc-txt/NewStuff @@ -36,29 +36,31 @@ Version 4.94 9. The ACL control "queue_only" can also be spelled "queue", and now takes an option "first_pass_route" to do the same as a "-odqs" on the command line. - 9. Items specified for the router and transport headers_remove option can use +10. Items specified for the router and transport headers_remove option can use a trailing asterisk to specify globbing. -10. New $queue_size variable. +11. New $queue_size variable. -11. New variables $local_part_{pre,suf}fix_v. +12. New variables $local_part_{pre,suf}fix_v. -12. New main option "sqlite_dbfile", for use in preference to prefixing the +13. New main option "sqlite_dbfile", for use in preference to prefixing the lookup string. The older method fails when tainted variables are used in the lookup, as the filename becomes tainted. The new method keeps the filename separate. -13. Options on the dsearch lookup, to return the full path and to filter +14. Options on the dsearch lookup, to return the full path and to filter filetypes for matching. -14. Options on pgsql and mysql lookups, to specify server separate from the +15. Options on pgsql and mysql lookups, to specify server separate from the lookup string. -15. Expansion item ${listquote {}}. +16. Expansion item ${listquote {}}. -16. An option for the ${readsocket {}{}{}} expansion to make the result data +17. An option for the ${readsocket {}{}{}} expansion to make the result data cacheable. +18. dkim_verify_min_keysizes, a list of minimum acceptable public-key sizes. + Version 4.93 diff --git a/doc/doc-txt/OptionLists.txt b/doc/doc-txt/OptionLists.txt index bb5a32091..ce0c901a9 100644 --- a/doc/doc-txt/OptionLists.txt +++ b/doc/doc-txt/OptionLists.txt @@ -176,6 +176,7 @@ dkim_strict string* unset smtp dkim_timestamps integer* unset smtp 4.92 dkim_verify_hashes string sha256:sha512:sha1 main 4.93 dkim_verify_keytypes string ed25519:rsa main 4.93 +dkim_verify_min_keysizes string list "rsa=1024 ed25519=250" main 4.94 dkim_verify_minimal boolean false main 4.93 dkim_verify_signers string* $dkim_signers main 4.70 directory string* unset appendfile diff --git a/src/src/dkim.c b/src/src/dkim.c index 9c8458b87..17eb61603 100644 --- a/src/src/dkim.c +++ b/src/src/dkim.c @@ -268,6 +268,11 @@ else "(headers probably modified in transit)]"); break; + case PDKIM_VERIFY_INVALID_PUBKEY_KEYSIZE: + logmsg = string_cat(logmsg, + US"signature invalid (key too short)]"); + break; + default: logmsg = string_cat(logmsg, US"unspecified reason]"); } @@ -560,6 +565,7 @@ switch (what) return US"pubkey_unavailable"; case PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD:return US"pubkey_dns_syntax"; case PDKIM_VERIFY_INVALID_PUBKEY_IMPORT: return US"pubkey_der_syntax"; + case PDKIM_VERIFY_INVALID_PUBKEY_KEYSIZE: return US"pubkey_too_short"; case PDKIM_VERIFY_FAIL_BODY: return US"bodyhash_mismatch"; case PDKIM_VERIFY_FAIL_MESSAGE: return US"signature_incorrect"; } @@ -854,6 +860,9 @@ for (pdkim_signature * sig = dkim_signatures; sig; sig = sig->next) g = string_cat(g, US"fail (signature did not verify; headers probably modified in transit)\n\t\t"); break; + case PDKIM_VERIFY_INVALID_PUBKEY_KEYSIZE: /* should this really be "polcy"? */ + g = string_fmt_append(g, "fail (public key too short: %u bits)\n\t\t", sig->keybits); + break; default: g = string_cat(g, US"fail (unspecified reason)\n\t\t"); break; diff --git a/src/src/expand.c b/src/src/expand.c index 5ae74ef52..500b73916 100644 --- a/src/src/expand.c +++ b/src/src/expand.c @@ -1172,8 +1172,8 @@ Returns: NULL if the subfield was not found, or a pointer to the subfield's data */ -static uschar * -expand_getkeyed(uschar * key, const uschar * s) +uschar * +expand_getkeyed(const uschar * key, const uschar * s) { int length = Ustrlen(key); Uskip_whitespace(&s); diff --git a/src/src/functions.h b/src/src/functions.h index 8ea5e4ef6..bb81655c2 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -242,6 +242,8 @@ extern BOOL expand_check_condition(uschar *, uschar *, uschar *); extern uschar *expand_file_big_buffer(const uschar *); extern uschar *expand_string(uschar *); /* public, cannot make const */ extern const uschar *expand_cstring(const uschar *); /* ... so use this one */ +extern uschar *expand_getkeyed(const uschar *, const uschar *); + extern uschar *expand_hide_passwords(uschar * ); extern uschar *expand_string_copy(const uschar *); extern int_eximarith_t expand_string_integer(uschar *, BOOL); diff --git a/src/src/globals.c b/src/src/globals.c index b8bf7ced2..36ac24955 100644 --- a/src/src/globals.c +++ b/src/src/globals.c @@ -845,6 +845,7 @@ uschar *dkim_signing_domain = NULL; uschar *dkim_signing_selector = NULL; uschar *dkim_verify_hashes = US"sha256:sha512"; uschar *dkim_verify_keytypes = US"ed25519:rsa"; +uschar *dkim_verify_min_keysizes = US"rsa=1024 ed25519=250"; BOOL dkim_verify_minimal = FALSE; uschar *dkim_verify_overall = NULL; uschar *dkim_verify_signers = US"$dkim_signers"; diff --git a/src/src/globals.h b/src/src/globals.h index b46e20be4..6b69fd66a 100644 --- a/src/src/globals.h +++ b/src/src/globals.h @@ -515,6 +515,7 @@ extern uschar *dkim_signing_domain; /* Expansion variable, domain used for si extern uschar *dkim_signing_selector; /* Expansion variable, selector used for signing a message. */ extern uschar *dkim_verify_hashes; /* Preference order for signatures */ extern uschar *dkim_verify_keytypes; /* Preference order for signatures */ +extern uschar *dkim_verify_min_keysizes; /* list of minimum key sizes, keyed by algo */ extern BOOL dkim_verify_minimal; /* Shortcircuit signture verification */ extern uschar *dkim_verify_overall; /* First successful domain verified, or null */ extern uschar *dkim_verify_signers; /* Colon-separated list of domains for each of which we call the DKIM ACL */ diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c index 0d2674f50..f6d70bfc9 100644 --- a/src/src/pdkim/pdkim.c +++ b/src/src/pdkim/pdkim.c @@ -181,6 +181,7 @@ switch(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_PUBKEY_KEYSIZE: return "PDKIM_VERIFY_INVALID_PUBKEY_KEYSIZE"; 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"; @@ -1886,6 +1887,19 @@ for (pdkim_signature * sig = ctx->sig; sig; sig = sig->next) sig->verify_ext_status = PDKIM_VERIFY_FAIL_MESSAGE; goto NEXT_VERIFY; } + if (*dkim_verify_min_keysizes) + { + unsigned minbits; + uschar * ss = expand_getkeyed(US pdkim_keytypes[sig->keytype], + dkim_verify_min_keysizes); + if (ss && (minbits = atoi(CS ss)) > sig->keybits) + { + DEBUG(D_acl) debug_printf("Key too short: Actual: %s %u Minima '%s'\n", + pdkim_keytypes[sig->keytype], sig->keybits, dkim_verify_min_keysizes); + sig->verify_status = PDKIM_VERIFY_FAIL; + sig->verify_ext_status = PDKIM_VERIFY_INVALID_PUBKEY_KEYSIZE; + } + } /* We have a winner! (if bodyhash was correct earlier) */ diff --git a/src/src/pdkim/pdkim.h b/src/src/pdkim/pdkim.h index 0b21df8ab..e79d71e6e 100644 --- a/src/src/pdkim/pdkim.h +++ b/src/src/pdkim/pdkim.h @@ -76,8 +76,9 @@ #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 +#define PDKIM_VERIFY_INVALID_PUBKEY_KEYSIZE 8 +#define PDKIM_VERIFY_INVALID_SIGNATURE_ERROR 9 +#define PDKIM_VERIFY_INVALID_DKIM_VERSION 10 /* -------------------------------------------------------------------------- */ /* Some parameter values */ diff --git a/src/src/readconf.c b/src/src/readconf.c index 04b3e1161..a44f3223b 100644 --- a/src/src/readconf.c +++ b/src/src/readconf.c @@ -119,6 +119,7 @@ static optionlist optionlist_config[] = { #ifndef DISABLE_DKIM { "dkim_verify_hashes", opt_stringptr, {&dkim_verify_hashes} }, { "dkim_verify_keytypes", opt_stringptr, {&dkim_verify_keytypes} }, + { "dkim_verify_min_keysizes", opt_stringptr, {&dkim_verify_min_keysizes} }, { "dkim_verify_minimal", opt_bool, {&dkim_verify_minimal} }, { "dkim_verify_signers", opt_stringptr, {&dkim_verify_signers} }, #endif diff --git a/test/confs/4500 b/test/confs/4500 index c7335327e..9f0829c1a 100644 --- a/test/confs/4500 +++ b/test/confs/4500 @@ -14,6 +14,9 @@ acl_smtp_data = check_data log_selector = +dkim_verbose dkim_verify_hashes = sha256 : sha512 : sha1 +.ifdef MSIZE +dkim_verify_min_keysizes = MSIZE +.endif queue_only queue_run_in_order diff --git a/test/confs/4520 b/test/confs/4520 index 770d8cc62..1f9d75b80 100644 --- a/test/confs/4520 +++ b/test/confs/4520 @@ -15,6 +15,7 @@ acl_smtp_dkim = accept logwrite = dkim_acl: signer: $dkim_cur_signer bits: $dkim acl_smtp_data = accept logwrite = data acl: dkim status $dkim_verify_status dkim_verify_signers = $dkim_signers +dkim_verify_min_keysizes = rsa=512 ed25519=250 .ifdef FILTER dkim_verify_minimal = true .endif diff --git a/test/log/4500 b/test/log/4500 index 7dc89c5c2..ebb3ead85 100644 --- a/test/log/4500 +++ b/test/log/4500 @@ -23,3 +23,8 @@ 1999-03-02 09:44:33 10HmbB-0005vi-00 DKIM: d=test.ex s=sel c=simple/simple a=rsa-sha1 b=1024 [fail - hash too weak] 1999-03-02 09:44:33 10HmbB-0005vi-00 Authentication-Results: myhost.test.ex;\n dkim=policy (fail - hash too weak) header.d=test.ex header.s=sel header.a=rsa-sha1 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 +1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port PORT_D +1999-03-02 09:44:33 10HmbC-0005vi-00 signer: test.ex bits: 512 +1999-03-02 09:44:33 10HmbC-0005vi-00 DKIM: d=test.ex s=ses c=simple/simple a=rsa-sha1 b=512 [verification failed - signature invalid (key too short)] +1999-03-02 09:44:33 10HmbC-0005vi-00 Authentication-Results: myhost.test.ex;\n dkim=fail (public key too short: 512 bits)\n header.d=test.ex header.s=ses header.a=rsa-sha1 +1999-03-02 09:44:33 10HmbC-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/4540 b/test/log/4540 index 9724325ee..4018c7fa2 100644 --- a/test/log/4540 +++ b/test/log/4540 @@ -15,3 +15,8 @@ 1999-03-02 09:44:33 10HmbA-0005vi-00 DKIM: d=kitterman.org s=ed25519 c=relaxed/simple a=ed25519-sha256 b=512 i=@kitterman.org t=1517847601 [verification succeeded] 1999-03-02 09:44:33 10HmbA-0005vi-00 Authentication-Results: myhost.test.ex;\n dkim=pass header.d=kitterman.org header.i=@kitterman.org header.s=ed25519 header.a=ed25519-sha256 1999-03-02 09:44:33 10HmbA-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss DKIM=kitterman.org id=example@example.com +1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port PORT_D +1999-03-02 09:44:33 10HmbB-0005vi-00 signer: test.ex bits: 253 +1999-03-02 09:44:33 10HmbB-0005vi-00 DKIM: d=test.ex s=sed c=relaxed/relaxed a=ed25519-sha256 b=512 [verification failed - signature invalid (key too short)] +1999-03-02 09:44:33 10HmbB-0005vi-00 Authentication-Results: myhost.test.ex;\n dkim=fail (public key too short: 253 bits)\n header.d=test.ex header.s=sed header.a=ed25519-sha256 +1999-03-02 09:44:33 10HmbB-0005vi-00 <= CALLER@bloggs.com H=(xxx) [127.0.0.1] P=smtp S=sss id=E10HmaY-0005vi-00@myhost.test.ex diff --git a/test/scripts/4500-DKIM/4500 b/test/scripts/4500-DKIM/4500 index 3999d4988..81fa577d9 100644 --- a/test/scripts/4500-DKIM/4500 +++ b/test/scripts/4500-DKIM/4500 @@ -1,6 +1,6 @@ # DKIM verify, simple canonicalisation # -exim -DSERVER=server -bd -oX PORT_D +exim -DSERVER=server -DMSIZE='rsa=512 ed25519=250' -bd -oX PORT_D **** # # This should pass. @@ -138,7 +138,7 @@ QUIT killdaemon # # A verifier that refuses sha1 -exim -DSERVER=server -DOPTION -bd -oX PORT_D +exim -DSERVER=server -DOPTION -DMSIZE='rsa=512 ed25519c=32' -bd -oX PORT_D **** # # This should fail despite being a passing submission above (with the unlimited verifier). @@ -166,6 +166,46 @@ Date: Thu, 19 Nov 2015 17:00:07 -0700 Message-ID: Subject: simple test +This is a simple test. +. +??? 250 +QUIT +??? 221 +**** +killdaemon +# +# +# +# +# +# +# With the default keysize minima, a 512b key should fail +exim -DSERVER=server -bd -oX PORT_D +**** +# - sha1, 512b +# Mail original in aux-fixed/4500.msg1.txt +# Sig generated by: perl aux-fixed/dkim/sign.pl --method=simple/simple --selector=ses \ +# --keyfile=aux-fixed/dkim/dkim512.private < 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; bh=OB9dZVu7+5/ufs3TH9leIcEpXSo=; b= + cIErF1eueIT9AU4qG54FyT3yrlVDDM7RZnuU6fWTevZpAuMqhYcRO8tU3U4vtKWB + +I2vd+F1gzqCzBcRtfLhZg== +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/4540-DKIM-Ed25519/4540 b/test/scripts/4540-DKIM-Ed25519/4540 index 22558dfa7..317385041 100644 --- a/test/scripts/4540-DKIM-Ed25519/4540 +++ b/test/scripts/4540-DKIM-Ed25519/4540 @@ -112,5 +112,46 @@ QUIT **** # killdaemon +# +# +# +# +# This should fail because the signing pubkey is too small. +exim -DSERVER=server -DMSIZE='rsa=1024 ed25519=300' -bd -oX PORT_D +**** +# +# Duplicate test input to the first one, above. +# +client 127.0.0.1 PORT_D +??? 220 +HELO xxx +??? 250 +MAIL FROM: +??? 250 +RCPT TO: +??? 250 +DATA +??? 354 +DKIM-Signature: v=1; a=ed25519-sha256; q=dns/txt; c=relaxed/relaxed; d=test.ex + ; s=sed; h=From:To:Subject; bh=/Ab0giHZitYQbDhFszoqQRUkgqueaX9zatJttIU/plc=; + b=5fhyD3EILDrnL4DnkD4hDaeis7+GSzL9GMHrhIDZJjuJ00WD5iI8SQ1q9rDfzFL/Kdw0VIyB4R + Dq0a4H6HI+Bw==; +Received: from jgh by myhost.test.ex with local (Exim x.yz) + envelope-from ) + 1dtXln-0000YP-Hb + a@test.ex; Sun, 17 Sep 2017 12:29:51 +0100 +From: nobody@example.com +Message-Id: +Sender: CALLER_NAME +Date: Sun, 17 Sep 2017 12:29:51 +0100 + +content +. +??? 250 +QUIT +??? 221 +**** +killdaemon +# no_stdout_check no_msglog_check diff --git a/test/stderr/4507 b/test/stderr/4507 index 1c45d0955..492b2ddf2 100644 --- a/test/stderr/4507 +++ b/test/stderr/4507 @@ -9,22 +9,22 @@ >>> host in helo_try_verify_hosts? no (option unset) >>> host in helo_accept_junk_hosts? no (option unset) >>> xxx in helo_lookup_domains? no (end of list) ->>> processing "accept" (TESTSUITE/test-config 44) +>>> processing "accept" (TESTSUITE/test-config 47) >>> accept: condition test succeeded in inline ACL >>> end of inline ACL: ACCEPT >>> host in ignore_fromline_hosts? no (option unset) >>> using ACL "check_dkim" ->>> processing "warn" (TESTSUITE/test-config 35) +>>> processing "warn" (TESTSUITE/test-config 38) >>> check logwrite = signer: $dkim_cur_signer bits: $dkim_key_length >>> = signer: test.ex bits: 1024 LOG: 10HmaX-0005vi-00 signer: test.ex bits: 1024 >>> warn: condition test succeeded in ACL "check_dkim" ->>> processing "accept" (TESTSUITE/test-config 38) +>>> processing "accept" (TESTSUITE/test-config 41) >>> accept: condition test succeeded in ACL "check_dkim" >>> end of ACL "check_dkim": ACCEPT LOG: 10HmaX-0005vi-00 DKIM: d=test.ex s=sel c=simple/simple a=rsa-sha1 b=1024 [verification succeeded] >>> using ACL "check_data" ->>> processing "accept" (TESTSUITE/test-config 42) +>>> processing "accept" (TESTSUITE/test-config 45) >>> check logwrite = ${authresults {$primary_hostname}} >>> = Authentication-Results: myhost.test.ex; >>> dkim=pass header.d=test.ex header.s=sel header.a=rsa-sha1 -- 2.30.2