From 98c72db3b13ac9b2705d071e8c59d42fac5b4f33 Mon Sep 17 00:00:00 2001 From: William Grant Date: Wed, 9 Mar 2011 22:46:17 +1100 Subject: [PATCH 01/10] Add a (hopefully correct) Tag=Value list parser, with tests. --- dkim/tests/test_dkim.py | 32 ++++++++++++++++++++++++++++ dkim/util.py | 46 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 dkim/util.py diff --git a/dkim/tests/test_dkim.py b/dkim/tests/test_dkim.py index 1baff54..c442a96 100644 --- a/dkim/tests/test_dkim.py +++ b/dkim/tests/test_dkim.py @@ -2,6 +2,10 @@ import os.path import unittest import dkim +from dkim.util import ( + InvalidTagValueList, + parse_tag_value, + ) def read_test_data(filename): @@ -50,5 +54,33 @@ class TestSignAndVerify(unittest.TestCase): self.assertFalse(res) +class TestParseTagValue(unittest.TestCase): + """Tag=Value parsing tests.""" + + def test_single(self): + self.assertEqual( + {'foo': 'bar'}, + parse_tag_value('foo=bar')) + + def test_trailing_separator_ignored(self): + self.assertEqual( + {'foo': 'bar'}, + parse_tag_value('foo=bar;')) + + def test_multiple(self): + self.assertEqual( + {'foo': 'bar', 'baz': 'foo'}, + parse_tag_value('foo=bar;baz=foo')) + + def test_value_with_equals(self): + self.assertEqual( + {'foo': 'bar', 'baz': 'foo=bar'}, + parse_tag_value('foo=bar;baz=foo=bar')) + + def test_no_value(self): + self.assertRaises( + InvalidTagValueList, + parse_tag_value, 'foo=bar;baz') + if __name__ == '__main__': unittest.main() diff --git a/dkim/util.py b/dkim/util.py new file mode 100644 index 0000000..1b73102 --- /dev/null +++ b/dkim/util.py @@ -0,0 +1,46 @@ +# 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) 2011 William Grant + +__all__ = [ + 'InvalidTagValueList', + 'parse_tag_value', + ] + + +class InvalidTagValueList(Exception): + pass + + +def parse_tag_value(tag_list): + """Parse a DKIM Tag=Value list. + + Interprets the syntax specified by RFC4871 section 3.2. + Assumes that folding whitespace is already unfolded. + """ + tags = {} + tag_specs = tag_list.split(';') + # Trailing semicolons are valid. + if not tag_specs[-1]: + tag_specs.pop() + for tag_spec in tag_specs: + try: + key, value = tag_spec.split('=', 1) + except ValueError: + raise InvalidTagValueList() + tags[key.strip()] = value.strip() + return tags From f8fb2d952d35a4f73ea90bf2d4519dfc44f6b088 Mon Sep 17 00:00:00 2001 From: William Grant Date: Wed, 9 Mar 2011 22:48:34 +1100 Subject: [PATCH 02/10] Fail if there is a duplicate tag. --- dkim/tests/test_dkim.py | 8 +++++++- dkim/util.py | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/dkim/tests/test_dkim.py b/dkim/tests/test_dkim.py index c442a96..e5817a8 100644 --- a/dkim/tests/test_dkim.py +++ b/dkim/tests/test_dkim.py @@ -77,10 +77,16 @@ class TestParseTagValue(unittest.TestCase): {'foo': 'bar', 'baz': 'foo=bar'}, parse_tag_value('foo=bar;baz=foo=bar')) - def test_no_value(self): + def test_missing_value_is_an_error(self): self.assertRaises( InvalidTagValueList, parse_tag_value, 'foo=bar;baz') + def test_duplicate_tag_is_an_error(self): + self.assertRaises( + InvalidTagValueList, + parse_tag_value, 'foo=bar;foo=baz') + + if __name__ == '__main__': unittest.main() diff --git a/dkim/util.py b/dkim/util.py index 1b73102..d5d6f1e 100644 --- a/dkim/util.py +++ b/dkim/util.py @@ -42,5 +42,7 @@ def parse_tag_value(tag_list): key, value = tag_spec.split('=', 1) except ValueError: raise InvalidTagValueList() + if key.strip() in tags: + raise InvalidTagValueList() tags[key.strip()] = value.strip() return tags From 42a463cc23d96988eda413975f854d8fc40e4108 Mon Sep 17 00:00:00 2001 From: William Grant Date: Wed, 9 Mar 2011 22:50:08 +1100 Subject: [PATCH 03/10] Test that whitespace around tags and values is stripped. --- dkim/tests/test_dkim.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dkim/tests/test_dkim.py b/dkim/tests/test_dkim.py index e5817a8..4d88eef 100644 --- a/dkim/tests/test_dkim.py +++ b/dkim/tests/test_dkim.py @@ -77,6 +77,11 @@ class TestParseTagValue(unittest.TestCase): {'foo': 'bar', 'baz': 'foo=bar'}, parse_tag_value('foo=bar;baz=foo=bar')) + def test_whitespace_is_stripped(self): + self.assertEqual( + {'foo': 'bar', 'baz': 'f oo=bar'}, + parse_tag_value(' foo \t= bar;\tbaz= f oo=bar ')) + def test_missing_value_is_an_error(self): self.assertRaises( InvalidTagValueList, From fb2d8678fd6c5ea418884f01e4a32ed074800e59 Mon Sep 17 00:00:00 2001 From: William Grant Date: Wed, 9 Mar 2011 22:54:41 +1100 Subject: [PATCH 04/10] Use parse_tag_value in verify(). --- dkim/__init__.py | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/dkim/__init__.py b/dkim/__init__.py index 5966d71..1c9debe 100644 --- a/dkim/__init__.py +++ b/dkim/__init__.py @@ -23,6 +23,11 @@ import time import dns.resolver +from dkim.util import ( + InvalidTagValueList, + parse_tag_value, + ) + __all__ = [ "Simple", "Relaxed", @@ -498,19 +503,10 @@ def verify(message, debuglog=None, dnsfunc=dnstxt): return False # Currently, we only validate the first DKIM-Signature line found. - - a = re.split(r"\s*;\s*", sigheaders[0][1].strip()) - if debuglog is not None: - print >>debuglog, "a:", a - sig = {} - for x in a: - if x: - m = re.match(r"(\w+)\s*=\s*(.*)", x, re.DOTALL) - if m is None: - if debuglog is not None: - print >>debuglog, "invalid format of signature part: %s" % x - return False - sig[m.group(1)] = m.group(2) + try: + sig = parse_tag_value(sigheaders[0][1]) + except InvalidTagValueList: + return False if debuglog is not None: print >>debuglog, "sig:", sig @@ -575,20 +571,10 @@ def verify(message, debuglog=None, dnsfunc=dnstxt): s = dnsfunc(sig['s']+"._domainkey."+sig['d']+".") if not s: return False - a = re.split(r"\s*;\s*", s) - # Trailing ';' on signature record is valid, see RFC 4871 3.2 - # tag-list = tag-spec 0*( ";" tag-spec ) [ ";" ] - if a[-1] == '': - a.pop(-1) - pub = {} - for f in a: - m = re.match(r"(\w+)=(.*)", f) - if m is not None: - pub[m.group(1)] = m.group(2) - else: - if debuglog is not None: - print >>debuglog, "invalid format in _domainkey txt record" - return False + try: + pub = parse_tag_value(s) + except InvalidTagValueList: + return False pk = parse_public_key(base64.b64decode(pub['p'])) modlen = len(int2str(pk['modulus'])) if debuglog is not None: From f9c2c2dcd93ce4ab1cc57a055cc96576e7efb03e Mon Sep 17 00:00:00 2001 From: William Grant Date: Thu, 10 Mar 2011 09:32:15 +1100 Subject: [PATCH 05/10] Add license header to test_dkim. --- dkim/tests/test_dkim.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dkim/tests/test_dkim.py b/dkim/tests/test_dkim.py index 1baff54..aecc326 100644 --- a/dkim/tests/test_dkim.py +++ b/dkim/tests/test_dkim.py @@ -1,3 +1,21 @@ +# 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) 2011 William Grant + import os.path import unittest From 44cb4c357df52009a32ee8fdccf77cc2074d3933 Mon Sep 17 00:00:00 2001 From: William Grant Date: Sat, 12 Mar 2011 11:20:30 +1100 Subject: [PATCH 06/10] Only exclude .testrepository at the root. --- .bzrignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bzrignore b/.bzrignore index a9b310f..6a691e0 100644 --- a/.bzrignore +++ b/.bzrignore @@ -1 +1 @@ -.testrepository +/.testrepository From ed497a40d65a3a8241c9f592ab66060029749d10 Mon Sep 17 00:00:00 2001 From: William Grant Date: Sat, 12 Mar 2011 12:05:15 +1100 Subject: [PATCH 07/10] Add docstrings to the two crypto functions. --- dkim/__init__.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/dkim/__init__.py b/dkim/__init__.py index 5966d71..deb69ef 100644 --- a/dkim/__init__.py +++ b/dkim/__init__.py @@ -94,7 +94,16 @@ def _remove(s, t): assert i >= 0 return s[:i] + s[i+len(t):] + def EMSA_PKCS1_v1_5_encode(digest, modlen, hashid): + """Encode a digest with EMSA-PKCS1-v1_5. + + Defined in RFC3447 section 9.2. + + @param digest: A digest value to encode. + @param modlen: The desired message length. + @param hashid: The ID of the hash used to generate the digest. + """ dinfo = asn1_build( (SEQUENCE, [ (SEQUENCE, [ @@ -102,8 +111,7 @@ def EMSA_PKCS1_v1_5_encode(digest, modlen, hashid): (NULL, None), ]), (OCTET_STRING, digest), - ]) - ) + ])) if len(dinfo)+3 > modlen: raise ParameterError("Hash too large for modulus") return "\x00\x01"+"\xff"*(modlen-len(dinfo)-3)+"\x00"+dinfo @@ -133,6 +141,11 @@ def hash_headers(hasher, canonicalize_headers, headers, include_headers, def parse_public_key(data): + """Parse an RSA public key. + + @param data: A DER-encoded X.509 subjectPublicKeyInfo + containing an RFC3447 RSAPublicKey. + """ x = asn1_parse(ASN1_Object, data) # Not sure why the [1:] is necessary to skip a byte. pkd = asn1_parse(ASN1_RSAPublicKey, x[0][1][1:]) From 142285bc0328c686d81d6c0935170533b3371984 Mon Sep 17 00:00:00 2001 From: William Grant Date: Sat, 12 Mar 2011 12:09:43 +1100 Subject: [PATCH 08/10] More docstrings. --- dkim/__init__.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dkim/__init__.py b/dkim/__init__.py index deb69ef..71e906f 100644 --- a/dkim/__init__.py +++ b/dkim/__init__.py @@ -119,6 +119,7 @@ def EMSA_PKCS1_v1_5_encode(digest, modlen, hashid): def hash_headers(hasher, canonicalize_headers, headers, include_headers, sigheaders, sig): + """Sign message header fields.""" sign_headers = [] lastindex = {} for h in include_headers: @@ -157,6 +158,14 @@ def parse_public_key(data): def validate_signature_fields(sig, debuglog=None): + """Validate DKIM-Signature fields. + + Basic checks for presence and correct formatting of mandatory fields. + + @param sig: A dict mapping field keys to values. + @param debuglog: A file-like object to which details will be written + on error. + """ mandatory_fields = ('v', 'a', 'b', 'bh', 'd', 'h', 's') for field in mandatory_fields: if field not in sig: From 160718f9b227bc16d0a6ac6a3a19f0c9ae52a448 Mon Sep 17 00:00:00 2001 From: William Grant Date: Sat, 12 Mar 2011 12:18:22 +1100 Subject: [PATCH 09/10] Add @param. --- dkim/util.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dkim/util.py b/dkim/util.py index d5d6f1e..b977c79 100644 --- a/dkim/util.py +++ b/dkim/util.py @@ -31,6 +31,8 @@ def parse_tag_value(tag_list): Interprets the syntax specified by RFC4871 section 3.2. Assumes that folding whitespace is already unfolded. + + @param tag_list: A string containing a DKIM Tag=Value list. """ tags = {} tag_specs = tag_list.split(';') From e74405d27a63434c9114c906a7250ab59570ff93 Mon Sep 17 00:00:00 2001 From: William Grant Date: Sat, 12 Mar 2011 12:24:55 +1100 Subject: [PATCH 10/10] Raise more descriptive exceptions, DuplicateTag and InvalidTagSpec. --- dkim/tests/test_dkim.py | 11 ++++++----- dkim/util.py | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/dkim/tests/test_dkim.py b/dkim/tests/test_dkim.py index 4d88eef..d8d2f58 100644 --- a/dkim/tests/test_dkim.py +++ b/dkim/tests/test_dkim.py @@ -3,7 +3,8 @@ import unittest import dkim from dkim.util import ( - InvalidTagValueList, + DuplicateTag, + InvalidTagSpec, parse_tag_value, ) @@ -83,13 +84,13 @@ class TestParseTagValue(unittest.TestCase): parse_tag_value(' foo \t= bar;\tbaz= f oo=bar ')) def test_missing_value_is_an_error(self): - self.assertRaises( - InvalidTagValueList, + self.assertRaisesRegexp( + InvalidTagSpec, 'baz', parse_tag_value, 'foo=bar;baz') def test_duplicate_tag_is_an_error(self): - self.assertRaises( - InvalidTagValueList, + self.assertRaisesRegexp( + DuplicateTag, 'foo', parse_tag_value, 'foo=bar;foo=baz') diff --git a/dkim/util.py b/dkim/util.py index b977c79..8511ca2 100644 --- a/dkim/util.py +++ b/dkim/util.py @@ -17,6 +17,8 @@ # Copyright (c) 2011 William Grant __all__ = [ + 'DuplicateTag', + 'InvalidTagSpec', 'InvalidTagValueList', 'parse_tag_value', ] @@ -26,6 +28,14 @@ class InvalidTagValueList(Exception): pass +class DuplicateTag(InvalidTagValueList): + pass + + +class InvalidTagSpec(InvalidTagValueList): + pass + + def parse_tag_value(tag_list): """Parse a DKIM Tag=Value list. @@ -43,8 +53,8 @@ def parse_tag_value(tag_list): try: key, value = tag_spec.split('=', 1) except ValueError: - raise InvalidTagValueList() + raise InvalidTagSpec(tag_spec) if key.strip() in tags: - raise InvalidTagValueList() + raise DuplicateTag(key.strip()) tags[key.strip()] = value.strip() return tags