From 98c72db3b13ac9b2705d071e8c59d42fac5b4f33 Mon Sep 17 00:00:00 2001 From: William Grant Date: Wed, 9 Mar 2011 22:46:17 +1100 Subject: [PATCH 1/6] 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 2/6] 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 3/6] 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 4/6] 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 160718f9b227bc16d0a6ac6a3a19f0c9ae52a448 Mon Sep 17 00:00:00 2001 From: William Grant Date: Sat, 12 Mar 2011 12:18:22 +1100 Subject: [PATCH 5/6] 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 6/6] 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