From 3dda94ca2d4bc9759521528b57115d4d1bbaff14 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Sat, 2 Nov 2019 11:15:36 -0400 Subject: [PATCH 1/5] Initial async support - works but so much overriding ... --- ChangeLog | 2 + README | 3 + dkim/__init__.py | 27 +++- dkim/asyncsupport.py | 286 +++++++++++++++++++++++++++++++++++++++++++ setup.py | 3 +- 5 files changed, 316 insertions(+), 5 deletions(-) create mode 100644 dkim/asyncsupport.py diff --git a/ChangeLog b/ChangeLog index 5774634..416048e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,8 @@ Version 1.0.0 - Add support for RFC 8460 tlsrpt DKIM signature processing (LP: #1847020) - Add new timeout parameter to enable DNS lookup timeouts to be adjusted + - Add async support with aiodns for DKIM verification (ARC not supported) + (LP: #1847002) - Drop usage of pymilter Milter.dns in dnsplug since it doesn't support havine a timeout passed to it diff --git a/README b/README index 0d3a0bd..f472afb 100644 --- a/README +++ b/README @@ -19,6 +19,8 @@ Dependencies will be automatically included for normal DKIM usage. The extras_requires feature 'ed25519' will add the dependencies needed for signing and verifying using the new DCRUP ed25519-sha256 algorithm. The extras_requires feature 'ARC' will add the extra dependencies needed for ARC. +Similarly, extras_requires feature 'asyncio' will add the extra dependencies +needed for asyncio. - Python 2.x >= 2.7, or Python 3.x >= 3.5. Recent versions have not been tested on python < 2.7 or python3 < 3.5, but may still work on python2.6 @@ -28,6 +30,7 @@ extras_requires feature 'ARC' will add the extra dependencies needed for ARC. - argparse. Standard library in python2.7 and later. - authres. Needed for ARC. - PyNaCl. Needed for use of ed25519 capability. + - aiodns. Needed for asycnio (Required python3.5 or later) INSTALLATION diff --git a/dkim/__init__.py b/dkim/__init__.py index e00908c..fd0c49f 100644 --- a/dkim/__init__.py +++ b/dkim/__init__.py @@ -36,6 +36,7 @@ import base64 import hashlib import logging import re +import sys import time # only needed for arc @@ -70,8 +71,13 @@ from dkim.crypto import ( try: from dkim.dnsplug import get_txt except ImportError: - def get_txt(s,timeout=5): - raise RuntimeError("DKIM.verify requires DNS or dnspython module") + try: + import aiodns + from dkim.asyncsupport import get_txt_async as get_txt + except: + # Only true if not using async + def get_txt(s,timeout=5): + raise RuntimeError("DKIM.verify requires DNS or dnspython module") from dkim.util import ( get_default_logger, InvalidTagValueList, @@ -419,7 +425,7 @@ def fold(header, namelen=0, linesep=b'\r\n'): return pre + header -def load_pk_from_dns(name, dnsfunc=get_txt, timeout=5): +def load_pk_from_dns(name, dnsfunc, timeout=5): s = dnsfunc(name, timeout=timeout) if not s: raise KeyFormatError("missing public key: %s"%name) @@ -1298,7 +1304,8 @@ def sign(message, selector, domain, privkey, identity=None, return d.sign(selector, domain, privkey, identity=identity, canonicalize=canonicalize, include_headers=include_headers, length=length) -def verify(message, logger=None, dnsfunc=get_txt, minkey=1024, timeout=5, tlsrpt=False): +def verify(message, logger=None, dnsfunc=get_txt, minkey=1024, + timeout=5, tlsrpt=False): """Verify the first (topmost) DKIM signature on an RFC822 formatted message. @param message: an RFC822 formatted message (with either \\n or \\r\\n line endings) @param logger: a logger to which debug info will be written (default None) @@ -1317,6 +1324,18 @@ def verify(message, logger=None, dnsfunc=get_txt, minkey=1024, timeout=5, tlsrpt logger.error("%s" % x) return False + +# aiodns requires Python 3.5+, so no async before that +if sys.version_info >= (3, 5): + try: + import aiodns + from dkim.asyncsupport import verify_async + dkim_verify_async = verify_async + except ImportError: + # If aiodns is not installed, then async verification is not available + pass + + # For consistency with ARC dkim_sign = sign dkim_verify = verify diff --git a/dkim/asyncsupport.py b/dkim/asyncsupport.py new file mode 100644 index 0000000..b9fa193 --- /dev/null +++ b/dkim/asyncsupport.py @@ -0,0 +1,286 @@ +# This software is provided 'as-is', without any express or implied +# warranty. In no event will the author be held liable for any damages +# arising from the use of this software. +# +# Permission is granted to anyone to use this software for any purpose, +# including commercial applications, and to alter it and redistribute it +# freely, subject to the following restrictions: +# +# 1. The origin of this software must not be misrepresented; you must not +# claim that you wrote the original software. If you use this software +# in a product, an acknowledgment in the product documentation would be +# appreciated but is not required. +# 2. Altered source versions must be plainly marked as such, and must not be +# misrepresented as being the original software. +# 3. This notice may not be removed or altered from any source distribution. +# +# Copyright (c) 2008 Greg Hewgill http://hewgill.com +# +# This has been modified from the original software. +# Copyright (c) 2011 William Grant +# +# This has been modified from the original software. +# Copyright (c) 2016 Google, Inc. +# Contact: Brandon Long +# +# This has been modified from the original software. +# Copyright (c) 2016, 2017, 2018, 2019 Scott Kitterman +# +# This has been modified from the original software. +# Copyright (c) 2017 Valimail Inc +# Contact: Gene Shuman + +import asyncio +import aiodns +import base64 +import dkim +import re + +__all__ = [ + 'get_txt_async', + 'load_pk_from_dns_async', + 'verify_async' + ] + + +async def get_txt_async(name, timeout=5): + """Return a TXT record associated with a DNS name in an asnyc loop. For + DKIM we can assume there is only one.""" + + # Note: This will use the existing loop or create one if needed + loop = asyncio.get_event_loop() + resolver = aiodns.DNSResolver(loop=loop, timeout=timeout) + + async def query(name, qtype): + return await resolver.query(name, qtype) + + #q = query(name, 'TXT') + try: + result = await query(name, 'TXT') + except aiodns.error.DNSError: + result = None + print('result', result) + + if result: + return result[0].text + else: + return None + + +async def load_pk_from_dns_async(name, dnsfunc, timeout=5): + s = await dnsfunc(name, timeout=timeout) + if not s: + raise dkim.KeyFormatError("missing public key: %s"%name) + try: + if type(s) is str: + s = s.encode('ascii') + pub = dkim.parse_tag_value(s) + except dkim.InvalidTagValueList as e: + raise dkim.KeyFormatError(e) + try: + if pub[b'v'] != b'DKIM1': + raise dkim.KeyFormatError("bad version") + except KeyError as e: + # Version not required in key record: RFC 6376 3.6.1 + pass + try: + if pub[b'k'] == b'ed25519': + pk = nacl.signing.VerifyKey(pub[b'p'], encoder=nacl.encoding.Base64Encoder) + keysize = 256 + ktag = b'ed25519' + except KeyError: + pub[b'k'] = b'rsa' + if pub[b'k'] == b'rsa': + try: + pk = dkim.parse_public_key(base64.b64decode(pub[b'p'])) + keysize = dkim.bitsize(pk['modulus']) + except KeyError: + raise dkim.KeyFormatError("incomplete public key: %s" % s) + except (TypeError,dkim.UnparsableKeyError) as e: + raise dkim.KeyFormatError("could not parse public key (%s): %s" % (pub[b'p'],e)) + ktag = b'rsa' + if pub[b'k'] != b'rsa' and pub[b'k'] != b'ed25519': + raise dkim.KeyFormatError('unknown algorithm in k= tag: {0}'.format(pub[b'k'])) + seqtlsrpt = False + try: + # Ignore unknown service types, RFC 6376 3.6.1 + if pub[b's'] != b'*' and pub[b's'] != b'email' and pub[b's'] != b'tlsrpt': + pk = None + keysize = None + ktag = None + raise dkim.KeyFormatError('unknown service type in s= tag: {0}'.format(pub[b's'])) + elif pub[b's'] == b'tlsrpt': + seqtlsrpt = True + except: + # Default is '*' - all service types, so no error if missing from key record + pass + return pk, keysize, ktag, seqtlsrpt + +class DKIM(dkim.DKIM): + #: Sign an RFC822 message and return the DKIM-Signature header line. + #: + #: The include_headers option gives full control over which header fields + #: are signed. Note that signing a header field that doesn't exist prevents + #: that field from being added without breaking the signature. Repeated + #: fields (such as Received) can be signed multiple times. Instances + #: of the field are signed from bottom to top. Signing a header field more + #: times than are currently present prevents additional instances + #: from being added without breaking the signature. + #: + #: The length option allows the message body to be appended to by MTAs + #: enroute (e.g. mailing lists that append unsubscribe information) + #: without breaking the signature. + #: + #: The default include_headers for this method differs from the backward + #: compatible sign function, which signs all headers not + #: in should_not_sign. The default list for this method can be modified + #: by tweaking should_sign and frozen_sign (or even should_not_sign). + #: It is only necessary to pass an include_headers list when precise control + #: is needed. + #: + #: @param selector: the DKIM selector value for the signature + #: @param domain: the DKIM domain value for the signature + #: @param privkey: a PKCS#1 private key in base64-encoded text form + #: @param identity: the DKIM identity value for the signature + #: (default "@"+domain) + #: @param canonicalize: the canonicalization algorithms to use + #: (default (Simple, Simple)) + #: @param include_headers: a list of strings indicating which headers + #: are to be signed (default rfc4871 recommended headers) + #: @param length: true if the l= tag should be included to indicate + #: body length signed (default False). + #: @return: DKIM-Signature header field terminated by '\r\n' + #: @raise DKIMException: when the message, include_headers, or key are badly + #: formed. + + # Abstract helper method to verify a signed header + #: @param sig: List of (key, value) tuples containing tag=values of the header + #: @param include_headers: headers to validate b= signature against + #: @param sig_header: (header_name, header_value) + #: @param dnsfunc: interface to dns + async def verify_sig(self, sig, include_headers, sig_header, dnsfunc): + name = sig[b's'] + b"._domainkey." + sig[b'd'] + b"." + try: + pk, self.keysize, ktag, self.seqtlsrpt = await load_pk_from_dns_async(name, dnsfunc, + timeout=self.timeout) + except dkim.KeyFormatError as e: + self.logger.error("%s" % e) + return False + + # RFC 8460 MAY ignore signatures without tlsrpt Service Type + if self.tlsrpt == 'strict' and not self.seqtlsrpt: + raise ValidationError("Message is tlsrpt and Service Type is not tlsrpt") + + # Inferred requirement from both RFC 8460 and RFC 6376 + if not self.tlsrpt and self.seqtlsrpt: + raise ValidationError("Message is not tlsrpt and Service Type is tlsrpt") + + try: + canon_policy = dkim.CanonicalizationPolicy.from_c_value(sig.get(b'c', b'simple/simple')) + except dkim.InvalidCanonicalizationPolicyError as e: + raise dkim.MessageFormatError("invalid c= value: %s" % e.args[0]) + + hasher = dkim.HASH_ALGORITHMS[sig[b'a']] + + # validate body if present + if b'bh' in sig: + h = dkim.HashThrough(hasher(), self.debug_content) + + body = canon_policy.canonicalize_body(self.body) + if b'l' in sig and not self.tlsrpt: + body = body[:int(sig[b'l'])] + h.update(body) + if self.debug_content: + self.logger.debug("body hashed: %r" % h.hashed()) + bodyhash = h.digest() + + self.logger.debug("bh: %s" % base64.b64encode(bodyhash)) + try: + bh = base64.b64decode(re.sub(br"\s+", b"", sig[b'bh'])) + except TypeError as e: + raise dkim.MessageFormatError(str(e)) + if bodyhash != bh: + raise dkim.ValidationError( + "body hash mismatch (got %s, expected %s)" % + (base64.b64encode(bodyhash), sig[b'bh'])) + + # address bug#644046 by including any additional From header + # fields when verifying. Since there should be only one From header, + # this shouldn't break any legitimate messages. This could be + # generalized to check for extras of other singleton headers. + if b'from' in include_headers: + include_headers.append(b'from') + h = dkim.HashThrough(hasher(), self.debug_content) + + headers = canon_policy.canonicalize_headers(self.headers) + self.signed_headers = dkim.hash_headers( + h, canon_policy, headers, include_headers, sig_header, sig) + if self.debug_content: + self.logger.debug("signed for %s: %r" % (sig_header[0], h.hashed())) + signature = base64.b64decode(re.sub(br"\s+", b"", sig[b'b'])) + if ktag == b'rsa': + try: + res = dkim.RSASSA_PKCS1_v1_5_verify(h, signature, pk) + self.logger.debug("%s valid: %s" % (sig_header[0], res)) + if res and self.keysize < self.minkey: + raise dkim.KeyFormatError("public key too small: %d" % self.keysize) + return res + except (TypeError,dkim.DigestTooLargeError) as e: + raise dkim.KeyFormatError("digest too large for modulus: %s"%e) + elif ktag == b'ed25519': + try: + pk.verify(h.digest(), signature) + self.logger.debug("%s valid" % (sig_header[0])) + return True + except (nacl.exceptions.BadSignatureError) as e: + return False + else: + raise dkim.UnknownKeyTypeError(ktag) + + async def verify(self,idx=0,dnsfunc=get_txt_async): + sigheaders = [(x,y) for x,y in self.headers if x.lower() == b"dkim-signature"] + if len(sigheaders) <= idx: + return False + + # By default, we validate the first DKIM-Signature line found. + try: + sig = dkim.parse_tag_value(sigheaders[idx][1]) + self.signature_fields = sig + except dkim.InvalidTagValueList as e: + raise dkim.MessageFormatError(e) + + self.logger.debug("sig: %r" % sig) + + dkim.validate_signature_fields(sig) + self.domain = sig[b'd'] + self.selector = sig[b's'] + + include_headers = [x.lower() for x in re.split(br"\s*:\s*", sig[b'h'])] + self.include_headers = tuple(include_headers) + + return await self.verify_sig(sig, include_headers, sigheaders[idx], dnsfunc) + + +async def verify_async(message, logger=None, dnsfunc=None, minkey=1024, + timeout=5, tlsrpt=False): + """Verify the first (topmost) DKIM signature on an RFC822 formatted message in an asyncio contxt. + @param message: an RFC822 formatted message (with either \\n or \\r\\n line endings) + @param logger: a logger to which debug info will be written (default None) + @param timeout: number of seconds for DNS lookup timeout (default = 5) + @param tlsrpt: message is an RFC 8460 TLS report (default False) + False: Not a tlsrpt, True: Is a tlsrpt, 'strict': tlsrpt, invalid if + service type is missing. For signing, if True, length is never used. + @return: True if signature verifies or False otherwise + """ + # type: (bytes, any, function, int) -> bool + # Note: This will use the existing loop or create one if needed + loop = asyncio.get_event_loop() + if not dnsfunc: + dnsfunc=get_txt_async + d = DKIM(message,logger=logger,minkey=minkey,timeout=timeout,tlsrpt=tlsrpt) + try: + return await d.verify(dnsfunc=dnsfunc) + except dkim.DKIMException as x: + if logger is not None: + logger.error("%s" % x) + return False diff --git a/setup.py b/setup.py index 752e168..29595f2 100644 --- a/setup.py +++ b/setup.py @@ -86,7 +86,8 @@ verification.""", 'pynacl', ], 'ed25519': ['pynacl'], - 'ARC': ['authres'] + 'ARC': ['authres'], + 'asyncio': ['aiodns'] }, **kw ) From e8ee183a7fce5ac96628e8227173171f0f0f3982 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Sat, 2 Nov 2019 11:16:13 -0400 Subject: [PATCH 2/5] Async version of dkimverify to demonstrate asyncio. --- dkimverify_async.py | 48 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 dkimverify_async.py diff --git a/dkimverify_async.py b/dkimverify_async.py new file mode 100644 index 0000000..a502700 --- /dev/null +++ b/dkimverify_async.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python + +# This software is provided 'as-is', without any express or implied +# warranty. In no event will the author be held liable for any damages +# arising from the use of this software. +# +# Permission is granted to anyone to use this software for any purpose, +# including commercial applications, and to alter it and redistribute it +# freely, subject to the following restrictions: +# +# 1. The origin of this software must not be misrepresented; you must not +# claim that you wrote the original software. If you use this software +# in a product, an acknowledgment in the product documentation would be +# appreciated but is not required. +# 2. Altered source versions must be plainly marked as such, and must not be +# misrepresented as being the original software. +# 3. This notice may not be removed or altered from any source distribution. +# +# Copyright (c) 2008 Greg Hewgill http://hewgill.com +# +# This has been modified from the original software. +# Copyright (c) 2011 William Grant + +from __future__ import print_function + +import sys +import asyncio +import dkim + + +if sys.version_info[0] >= 3: + # Make sys.stdin a binary stream. + sys.stdin = sys.stdin.detach() + +message = sys.stdin.read() + +async def main(): + res = await dkim.verify_async(message) + return res + + +if __name__ == "__main__": + res = asyncio.run(main()) + if not res: + print("signature verification failed") + sys.exit(1) + print("signature ok") + From 3de1dc0362221e313c2a2e8e99609b3fd7ee3e71 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Tue, 5 Nov 2019 08:34:13 -0500 Subject: [PATCH 3/5] Refactor load_pk_from_dns to reduce code duplication between async and non-async. --- dkim/__init__.py | 9 +++++++-- dkim/asyncsupport.py | 47 +------------------------------------------- 2 files changed, 8 insertions(+), 48 deletions(-) diff --git a/dkim/__init__.py b/dkim/__init__.py index fd0c49f..e714214 100644 --- a/dkim/__init__.py +++ b/dkim/__init__.py @@ -425,8 +425,7 @@ def fold(header, namelen=0, linesep=b'\r\n'): return pre + header -def load_pk_from_dns(name, dnsfunc, timeout=5): - s = dnsfunc(name, timeout=timeout) +def evaluate_pk(name, s): if not s: raise KeyFormatError("missing public key: %s"%name) try: @@ -475,6 +474,12 @@ def load_pk_from_dns(name, dnsfunc, timeout=5): return pk, keysize, ktag, seqtlsrpt +def load_pk_from_dns(name, dnsfunc=get_txt, timeout=5): + s = dnsfunc(name, timeout=timeout) + pk, keysize, ktag, seqtlsrpt = evaluate_pk(name, s) + return pk, keysize, ktag, seqtlsrpt + + #: Abstract base class for holding messages and options during DKIM/ARC signing and verification. class DomainSigner(object): # NOTE - the first 2 indentation levels are 2 instead of 4 diff --git a/dkim/asyncsupport.py b/dkim/asyncsupport.py index b9fa193..0e15b41 100644 --- a/dkim/asyncsupport.py +++ b/dkim/asyncsupport.py @@ -59,7 +59,6 @@ async def get_txt_async(name, timeout=5): result = await query(name, 'TXT') except aiodns.error.DNSError: result = None - print('result', result) if result: return result[0].text @@ -69,51 +68,7 @@ async def get_txt_async(name, timeout=5): async def load_pk_from_dns_async(name, dnsfunc, timeout=5): s = await dnsfunc(name, timeout=timeout) - if not s: - raise dkim.KeyFormatError("missing public key: %s"%name) - try: - if type(s) is str: - s = s.encode('ascii') - pub = dkim.parse_tag_value(s) - except dkim.InvalidTagValueList as e: - raise dkim.KeyFormatError(e) - try: - if pub[b'v'] != b'DKIM1': - raise dkim.KeyFormatError("bad version") - except KeyError as e: - # Version not required in key record: RFC 6376 3.6.1 - pass - try: - if pub[b'k'] == b'ed25519': - pk = nacl.signing.VerifyKey(pub[b'p'], encoder=nacl.encoding.Base64Encoder) - keysize = 256 - ktag = b'ed25519' - except KeyError: - pub[b'k'] = b'rsa' - if pub[b'k'] == b'rsa': - try: - pk = dkim.parse_public_key(base64.b64decode(pub[b'p'])) - keysize = dkim.bitsize(pk['modulus']) - except KeyError: - raise dkim.KeyFormatError("incomplete public key: %s" % s) - except (TypeError,dkim.UnparsableKeyError) as e: - raise dkim.KeyFormatError("could not parse public key (%s): %s" % (pub[b'p'],e)) - ktag = b'rsa' - if pub[b'k'] != b'rsa' and pub[b'k'] != b'ed25519': - raise dkim.KeyFormatError('unknown algorithm in k= tag: {0}'.format(pub[b'k'])) - seqtlsrpt = False - try: - # Ignore unknown service types, RFC 6376 3.6.1 - if pub[b's'] != b'*' and pub[b's'] != b'email' and pub[b's'] != b'tlsrpt': - pk = None - keysize = None - ktag = None - raise dkim.KeyFormatError('unknown service type in s= tag: {0}'.format(pub[b's'])) - elif pub[b's'] == b'tlsrpt': - seqtlsrpt = True - except: - # Default is '*' - all service types, so no error if missing from key record - pass + pk, keysize, ktag, seqtlsrpt = dkim.evaluate_pk(name, s) return pk, keysize, ktag, seqtlsrpt class DKIM(dkim.DKIM): From 9bdb451cd86645867339aa3e3a0460b0143dfd71 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Tue, 5 Nov 2019 21:10:28 -0500 Subject: [PATCH 4/5] DKIM.verify_sig: Refactor to minimize code duplication in dkim.asyncsupport. --- dkim/__init__.py | 43 +++++++++-------- dkim/asyncsupport.py | 109 ++----------------------------------------- 2 files changed, 29 insertions(+), 123 deletions(-) diff --git a/dkim/__init__.py b/dkim/__init__.py index e714214..ce84159 100644 --- a/dkim/__init__.py +++ b/dkim/__init__.py @@ -686,24 +686,13 @@ class DomainSigner(object): return header_value - # Abstract helper method to verify a signed header - #: @param sig: List of (key, value) tuples containing tag=values of the header - #: @param include_headers: headers to validate b= signature against - #: @param sig_header: (header_name, header_value) - #: @param dnsfunc: interface to dns - def verify_sig(self, sig, include_headers, sig_header, dnsfunc): - name = sig[b's'] + b"._domainkey." + sig[b'd'] + b"." - try: - pk, self.keysize, ktag, self.seqtlsrpt = load_pk_from_dns(name, dnsfunc, - timeout=self.timeout) - except KeyFormatError as e: - self.logger.error("%s" % e) - return False + def verify_sig_process(self, sig, include_headers, sig_header, dnsfunc): + """Non-async sensitive verify_sig elements. Separated to avoid async code + duplication.""" # RFC 8460 MAY ignore signatures without tlsrpt Service Type if self.tlsrpt == 'strict' and not self.seqtlsrpt: raise ValidationError("Message is tlsrpt and Service Type is not tlsrpt") - # Inferred requirement from both RFC 8460 and RFC 6376 if not self.tlsrpt and self.seqtlsrpt: raise ValidationError("Message is not tlsrpt and Service Type is tlsrpt") @@ -751,24 +740,40 @@ class DomainSigner(object): if self.debug_content: self.logger.debug("signed for %s: %r" % (sig_header[0], h.hashed())) signature = base64.b64decode(re.sub(br"\s+", b"", sig[b'b'])) - if ktag == b'rsa': + if self.ktag == b'rsa': try: - res = RSASSA_PKCS1_v1_5_verify(h, signature, pk) + res = RSASSA_PKCS1_v1_5_verify(h, signature, self.pk) self.logger.debug("%s valid: %s" % (sig_header[0], res)) if res and self.keysize < self.minkey: raise KeyFormatError("public key too small: %d" % self.keysize) return res except (TypeError,DigestTooLargeError) as e: raise KeyFormatError("digest too large for modulus: %s"%e) - elif ktag == b'ed25519': + elif self.ktag == b'ed25519': try: - pk.verify(h.digest(), signature) + self.pk.verify(h.digest(), signature) self.logger.debug("%s valid" % (sig_header[0])) return True except (nacl.exceptions.BadSignatureError) as e: return False else: - raise UnknownKeyTypeError(ktag) + raise UnknownKeyTypeError(self.ktag) + + + # Abstract helper method to verify a signed header + #: @param sig: List of (key, value) tuples containing tag=values of the header + #: @param include_headers: headers to validate b= signature against + #: @param sig_header: (header_name, header_value) + #: @param dnsfunc: interface to dns + def verify_sig(self, sig, include_headers, sig_header, dnsfunc): + name = sig[b's'] + b"._domainkey." + sig[b'd'] + b"." + try: + self.pk, self.keysize, self.ktag, self.seqtlsrpt = load_pk_from_dns(name, + dnsfunc, timeout=self.timeout) + except KeyFormatError as e: + self.logger.error("%s" % e) + return False + return self.verify_sig_process(sig, include_headers, sig_header, dnsfunc) #: Hold messages and options during DKIM signing and verification. diff --git a/dkim/asyncsupport.py b/dkim/asyncsupport.py index 0e15b41..dc856af 100644 --- a/dkim/asyncsupport.py +++ b/dkim/asyncsupport.py @@ -74,39 +74,8 @@ async def load_pk_from_dns_async(name, dnsfunc, timeout=5): class DKIM(dkim.DKIM): #: Sign an RFC822 message and return the DKIM-Signature header line. #: - #: The include_headers option gives full control over which header fields - #: are signed. Note that signing a header field that doesn't exist prevents - #: that field from being added without breaking the signature. Repeated - #: fields (such as Received) can be signed multiple times. Instances - #: of the field are signed from bottom to top. Signing a header field more - #: times than are currently present prevents additional instances - #: from being added without breaking the signature. - #: - #: The length option allows the message body to be appended to by MTAs - #: enroute (e.g. mailing lists that append unsubscribe information) - #: without breaking the signature. - #: - #: The default include_headers for this method differs from the backward - #: compatible sign function, which signs all headers not - #: in should_not_sign. The default list for this method can be modified - #: by tweaking should_sign and frozen_sign (or even should_not_sign). - #: It is only necessary to pass an include_headers list when precise control - #: is needed. - #: - #: @param selector: the DKIM selector value for the signature - #: @param domain: the DKIM domain value for the signature - #: @param privkey: a PKCS#1 private key in base64-encoded text form - #: @param identity: the DKIM identity value for the signature - #: (default "@"+domain) - #: @param canonicalize: the canonicalization algorithms to use - #: (default (Simple, Simple)) - #: @param include_headers: a list of strings indicating which headers - #: are to be signed (default rfc4871 recommended headers) - #: @param length: true if the l= tag should be included to indicate - #: body length signed (default False). - #: @return: DKIM-Signature header field terminated by '\r\n' - #: @raise DKIMException: when the message, include_headers, or key are badly - #: formed. + #: Identical to dkim.DKIM, except uses aiodns and can be awaited in an + #: ascyncio context. See dkim.DKIM for details. # Abstract helper method to verify a signed header #: @param sig: List of (key, value) tuples containing tag=values of the header @@ -116,81 +85,13 @@ class DKIM(dkim.DKIM): async def verify_sig(self, sig, include_headers, sig_header, dnsfunc): name = sig[b's'] + b"._domainkey." + sig[b'd'] + b"." try: - pk, self.keysize, ktag, self.seqtlsrpt = await load_pk_from_dns_async(name, dnsfunc, - timeout=self.timeout) + self.pk, self.keysize, self.ktag, self.seqtlsrpt = await load_pk_from_dns_async(name, + dnsfunc, timeout=self.timeout) except dkim.KeyFormatError as e: self.logger.error("%s" % e) return False + return self.verify_sig_process(sig, include_headers, sig_header, dnsfunc) - # RFC 8460 MAY ignore signatures without tlsrpt Service Type - if self.tlsrpt == 'strict' and not self.seqtlsrpt: - raise ValidationError("Message is tlsrpt and Service Type is not tlsrpt") - - # Inferred requirement from both RFC 8460 and RFC 6376 - if not self.tlsrpt and self.seqtlsrpt: - raise ValidationError("Message is not tlsrpt and Service Type is tlsrpt") - - try: - canon_policy = dkim.CanonicalizationPolicy.from_c_value(sig.get(b'c', b'simple/simple')) - except dkim.InvalidCanonicalizationPolicyError as e: - raise dkim.MessageFormatError("invalid c= value: %s" % e.args[0]) - - hasher = dkim.HASH_ALGORITHMS[sig[b'a']] - - # validate body if present - if b'bh' in sig: - h = dkim.HashThrough(hasher(), self.debug_content) - - body = canon_policy.canonicalize_body(self.body) - if b'l' in sig and not self.tlsrpt: - body = body[:int(sig[b'l'])] - h.update(body) - if self.debug_content: - self.logger.debug("body hashed: %r" % h.hashed()) - bodyhash = h.digest() - - self.logger.debug("bh: %s" % base64.b64encode(bodyhash)) - try: - bh = base64.b64decode(re.sub(br"\s+", b"", sig[b'bh'])) - except TypeError as e: - raise dkim.MessageFormatError(str(e)) - if bodyhash != bh: - raise dkim.ValidationError( - "body hash mismatch (got %s, expected %s)" % - (base64.b64encode(bodyhash), sig[b'bh'])) - - # address bug#644046 by including any additional From header - # fields when verifying. Since there should be only one From header, - # this shouldn't break any legitimate messages. This could be - # generalized to check for extras of other singleton headers. - if b'from' in include_headers: - include_headers.append(b'from') - h = dkim.HashThrough(hasher(), self.debug_content) - - headers = canon_policy.canonicalize_headers(self.headers) - self.signed_headers = dkim.hash_headers( - h, canon_policy, headers, include_headers, sig_header, sig) - if self.debug_content: - self.logger.debug("signed for %s: %r" % (sig_header[0], h.hashed())) - signature = base64.b64decode(re.sub(br"\s+", b"", sig[b'b'])) - if ktag == b'rsa': - try: - res = dkim.RSASSA_PKCS1_v1_5_verify(h, signature, pk) - self.logger.debug("%s valid: %s" % (sig_header[0], res)) - if res and self.keysize < self.minkey: - raise dkim.KeyFormatError("public key too small: %d" % self.keysize) - return res - except (TypeError,dkim.DigestTooLargeError) as e: - raise dkim.KeyFormatError("digest too large for modulus: %s"%e) - elif ktag == b'ed25519': - try: - pk.verify(h.digest(), signature) - self.logger.debug("%s valid" % (sig_header[0])) - return True - except (nacl.exceptions.BadSignatureError) as e: - return False - else: - raise dkim.UnknownKeyTypeError(ktag) async def verify(self,idx=0,dnsfunc=get_txt_async): sigheaders = [(x,y) for x,y in self.headers if x.lower() == b"dkim-signature"] From 79722177560725652b4844a8e68662ff7744a09a Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Tue, 5 Nov 2019 21:36:06 -0500 Subject: [PATCH 5/5] DKIM.verify: Refactor to minimize code duplication in dkim.asyncsupport. --- dkim/__init__.py | 25 ++++++++++++++++--------- dkim/asyncsupport.py | 21 +-------------------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/dkim/__init__.py b/dkim/__init__.py index ce84159..720af97 100644 --- a/dkim/__init__.py +++ b/dkim/__init__.py @@ -888,15 +888,11 @@ class DKIM(DomainSigner): self.signature_fields = dict(sigfields) return b'DKIM-Signature: ' + res - #: Verify a DKIM signature. - #: @type idx: int - #: @param idx: which signature to verify. The first (topmost) signature is 0. - #: @type dnsfunc: callable - #: @param dnsfunc: an option function to lookup TXT resource records - #: for a DNS domain. The default uses dnspython or pydns. - #: @return: True if signature verifies or False otherwise - #: @raise DKIMException: when the message, signature, or key are badly formed - def verify(self,idx=0,dnsfunc=get_txt): + + def verify_headerprep(self, idx=0): + """Non-DNS verify parts to minimize asyncio code duplication.""" + + sigheaders = [(x,y) for x,y in self.headers if x.lower() == b"dkim-signature"] if len(sigheaders) <= idx: return False @@ -916,7 +912,18 @@ class DKIM(DomainSigner): include_headers = [x.lower() for x in re.split(br"\s*:\s*", sig[b'h'])] self.include_headers = tuple(include_headers) + return sig, include_headers, sigheaders + #: Verify a DKIM signature. + #: @type idx: int + #: @param idx: which signature to verify. The first (topmost) signature is 0. + #: @type dnsfunc: callable + #: @param dnsfunc: an option function to lookup TXT resource records + #: for a DNS domain. The default uses dnspython or pydns. + #: @return: True if signature verifies or False otherwise + #: @raise DKIMException: when the message, signature, or key are badly formed + def verify(self,idx=0,dnsfunc=get_txt): + sig, include_headers, sigheaders = self.verify_headerprep(idx=0) return self.verify_sig(sig, include_headers, sigheaders[idx], dnsfunc) diff --git a/dkim/asyncsupport.py b/dkim/asyncsupport.py index dc856af..226cc42 100644 --- a/dkim/asyncsupport.py +++ b/dkim/asyncsupport.py @@ -94,26 +94,7 @@ class DKIM(dkim.DKIM): async def verify(self,idx=0,dnsfunc=get_txt_async): - sigheaders = [(x,y) for x,y in self.headers if x.lower() == b"dkim-signature"] - if len(sigheaders) <= idx: - return False - - # By default, we validate the first DKIM-Signature line found. - try: - sig = dkim.parse_tag_value(sigheaders[idx][1]) - self.signature_fields = sig - except dkim.InvalidTagValueList as e: - raise dkim.MessageFormatError(e) - - self.logger.debug("sig: %r" % sig) - - dkim.validate_signature_fields(sig) - self.domain = sig[b'd'] - self.selector = sig[b's'] - - include_headers = [x.lower() for x in re.split(br"\s*:\s*", sig[b'h'])] - self.include_headers = tuple(include_headers) - + sig, include_headers, sigheaders = self.verify_headerprep(idx=0) return await self.verify_sig(sig, include_headers, sigheaders[idx], dnsfunc)