From e872bd44b0622299fc154139ca0bdfd09ae448db Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Mon, 18 Feb 2019 08:02:27 -0500 Subject: [PATCH 01/15] ignore emacs turds --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 9e59230..8bd7e59 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ dist dkimpy_milter.egg-info *.pyc +*~ From f60ea12e86ac687cd557c9b220051c60a913305b Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 19:19:26 -0500 Subject: [PATCH 02/15] Prefer dnspython over PyDNS in setup.py README and dkimpy_milter/dnsplug.py both prefer dnspython if available, over PyDNS. setup.py should order the preferences in the same way. --- setup.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index 41a11a5..18c2ec9 100644 --- a/setup.py +++ b/setup.py @@ -23,10 +23,10 @@ description = "Domain Keys Identified Mail (DKIM) signing/verifying milter for P kw = {} # Work-around for lack of 'or' requires in setuptools. try: - import DNS - kw['install_requires'] = ['dkimpy>=0.7', 'pymilter', 'authres>=1.1.0', 'PyNaCl', 'ipaddress', 'PyDNS'] -except ImportError: # If PyDNS is not installed, prefer dnspython + import dns kw['install_requires'] = ['dkimpy>=0.7', 'pymilter', 'authres>=1.1.0', 'PyNaCl', 'ipaddress', 'dnspython'] +except ImportError: # If PyDNS is not installed, prefer dnspython + kw['install_requires'] = ['dkimpy>=0.7', 'pymilter', 'authres>=1.1.0', 'PyNaCl', 'ipaddress', 'PyDNS'] setup( name='dkimpy-milter', From 9e11b75ec34f9ae33cd024e3ecac9de1169bcd3c Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Sat, 16 Feb 2019 11:20:51 -0500 Subject: [PATCH 03/15] Avoid AttributeError on simple connection and disconnection Without this patch, this simple script for miltertest: ---- conn = mt.connect("unix:milter.sock") if conn == nil then error "mt.connect() failed" end if mt.conninfo(conn, nil, "unspec") ~= nil then error "mt.conninfo() failed" end if mt.getreply(conn) ~= SMFIR_CONTINUE then error "mt.conninfo() unexpected reply" end mt.disconnect(conn) ---- Produces the following error: Traceback (most recent call last): File "/usr/lib/python2.7/dist-packages/Milter/__init__.py", line 702, in connect_callback return m.connect(hostname,family,hostaddr) File "/usr/lib/python2.7/dist-packages/Milter/__init__.py", line 173, in wrapper rc = func(self,*args) File "/home/dkg/src/dkimpy-milter/dkimpy-milter/dkimpy_milter/__init__.py", line 64, in connect self.receiver = self.getsymval('j').strip() AttributeError: 'NoneType' object has no attribute 'strip' --- dkimpy_milter/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dkimpy_milter/__init__.py b/dkimpy_milter/__init__.py index 3791748..28971c3 100644 --- a/dkimpy_milter/__init__.py +++ b/dkimpy_milter/__init__.py @@ -61,7 +61,9 @@ class dkimMilter(Milter.Base): self.external_connection = False self.hello_name = None # sometimes people put extra space in sendmail config, so we strip - self.receiver = self.getsymval('j').strip() + self.receiver = self.getsymval('j') + if self.receiver is not None: + self.receiver = self.receiver.strip() try: self.AuthservID = milterconfig['AuthservID'] except: From bb44f36519f4bcfbc8b0aa34b245902a74c9e16c Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 13:18:44 -0500 Subject: [PATCH 04/15] When Socket is absolute path, do not strip leading / This appears to just be an untested codepath. --- dkimpy_milter/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dkimpy_milter/util.py b/dkimpy_milter/util.py index 5d3f69d..57b433c 100644 --- a/dkimpy_milter/util.py +++ b/dkimpy_milter/util.py @@ -151,7 +151,7 @@ def own_socketfile(milterconfig): import os user, group = user_group(milterconfig.get('UserID')) if milterconfig.get('Socket')[:1] == '/': - os.chown(milterconfig.get('Socket')[1:], user, group) + os.chown(milterconfig.get('Socket'), user, group) if milterconfig.get('Socket')[:6] == "local:": os.chown(milterconfig.get('Socket')[6:], user, group) From a9a6893c89516e8bb5e2c907b3e3941ceb276b8c Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 15:20:02 -0500 Subject: [PATCH 05/15] Handle unix: socket prefix the same as local: sendmail's milter.c treats these two declarations the same way, so what we do for one should also be done for the other. --- dkimpy_milter/util.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/dkimpy_milter/util.py b/dkimpy_milter/util.py index 57b433c..abab538 100644 --- a/dkimpy_milter/util.py +++ b/dkimpy_milter/util.py @@ -150,10 +150,17 @@ def own_socketfile(milterconfig): """If socket is Unix socket, chown to UserID before dropping privileges""" import os user, group = user_group(milterconfig.get('UserID')) - if milterconfig.get('Socket')[:1] == '/': - os.chown(milterconfig.get('Socket'), user, group) - if milterconfig.get('Socket')[:6] == "local:": - os.chown(milterconfig.get('Socket')[6:], user, group) + offset = None + sockname = milterconfig.get('Socket') + if sockname[:1] == '/': + offset = 0 + elif sockname[:6] == "local:": + offset = 6 + elif sockname[:5] == "unix:": + offset = 5 + + if offset is not None: + os.chown(sockname[offset:], user, group) def read_keyfile(milterconfig, keytype): From 71c0c3f20ae03c18e8f40284a0242fb404b76991 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 15:23:46 -0500 Subject: [PATCH 06/15] Avoid failing to chown non-existent Unix-domain sockets Changing ownership of sockets that doesn't exist isn't a great practice. A better approach would be to apply os.chown() to the file descriptor of the open socket, but at the very least dkimpy-milter shouldn't crash the way it currently does if the socket isn't already present. --- dkimpy_milter/util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dkimpy_milter/util.py b/dkimpy_milter/util.py index abab538..17857d6 100644 --- a/dkimpy_milter/util.py +++ b/dkimpy_milter/util.py @@ -160,7 +160,8 @@ def own_socketfile(milterconfig): offset = 5 if offset is not None: - os.chown(sockname[offset:], user, group) + if os.path.exists(sockname[offset:]): + os.chown(sockname[offset:], user, group) def read_keyfile(milterconfig, keytype): From 1c6030024d57d75e1ce9a3c3cdd645b5e34c173a Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Tue, 19 Feb 2019 10:35:45 -0500 Subject: [PATCH 07/15] add DNSOverride configuration for testing --- dkimpy_milter/__init__.py | 7 ++++++- dkimpy_milter/config.py | 2 ++ man/dkimpy-milter.conf.5 | 7 +++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/dkimpy_milter/__init__.py b/dkimpy_milter/__init__.py index 28971c3..5345fc7 100644 --- a/dkimpy_milter/__init__.py +++ b/dkimpy_milter/__init__.py @@ -260,7 +260,12 @@ class dkimMilter(Milter.Base): for y in range(self.has_dkim): # Verify _ALL_ the signatures d = dkim.DKIM(txt) try: - res = d.verify(idx=y) + dnsoverride = milterconfig.get('DNSOverride') + if isinstance(dnsoverride, str): + syslog.syslog("DNSOverride: {0}".format(dnsoverride)) + res = d.verify(idx=y, dnsfunc=lambda _x: dnsoverride) + else: + res = d.verify(idx=y) if res: if d.signature_fields.get(b'a') == 'ed25519-sha256': self.dkim_comment = ('Good {0} signature' diff --git a/dkimpy_milter/config.py b/dkimpy_milter/config.py index 9f42af2..3359246 100644 --- a/dkimpy_milter/config.py +++ b/dkimpy_milter/config.py @@ -48,6 +48,7 @@ defaultConfigData = { 'DiagnosticDirectory': '', 'MacroList': '', 'MacroListVerify': '', + 'DNSOverride': None, 'debugLevel': 0 # Undocumented config item for developer use } @@ -334,6 +335,7 @@ def _readConfigFile(path, configData=None, configGlobal={}): 'DiagnosticDirectory': 'str', 'MacroList': 'dataset', 'MacroListVerify': 'dataset', + 'DNSOverride': 'str', 'debugLevel': 'int' } diff --git a/man/dkimpy-milter.conf.5 b/man/dkimpy-milter.conf.5 index 3dd7612..a7e5d31 100644 --- a/man/dkimpy-milter.conf.5 +++ b/man/dkimpy-milter.conf.5 @@ -311,6 +311,13 @@ be set: (b) KeyTable, SigningTable, no Domain, no KeyFile, no Selector; [fooTable options NOT IMPLEMENTED] +.TP +.I DNSOverride (string) +Provide a text string that a verifying milter should use instead of +consulting the DNS on each message. This is useful primarily for +testing purposes in environments where it is awkward to modify the +system DNS resolution. It should not be used in production. + .TP .I PeerList (dataset) Identifies a set of "peers" that identifies clients whose connections From bd1d25d83ef40ae48fa1cdcf276e8ba8f39a2f19 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Tue, 19 Feb 2019 16:55:44 -0500 Subject: [PATCH 08/15] Set up correct AuthservID defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this fix, a verifying dkimpy-milter that has no explicit AuthservID produces the following crashing behavior as it tries to create the authres header: Traceback (most recent call last): File "/usr/lib/python2.7/dist-packages/Milter/__init__.py", line 772, in milter.set_eom_callback(lambda ctx: ctx.getpriv().eom()) File "…/dkimpy_milter/__init__.py", line 199, in eom h = fold(str(h)) File "/usr/lib/python2.7/dist-packages/authres/core.py", line 476, in __str__ return ''.join((self.HEADER_FIELD_NAME, ': ', self.header_value())) File "/usr/lib/python2.7/dist-packages/authres/core.py", line 496, in header_value return ''.join(strs) --- dkimpy_milter/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dkimpy_milter/config.py b/dkimpy_milter/config.py index 3359246..601017a 100644 --- a/dkimpy_milter/config.py +++ b/dkimpy_milter/config.py @@ -401,7 +401,7 @@ def _readConfigFile(path, configData=None, configGlobal={}): configData[name] = conversion(value) fp.close() try: - configData['AuthservID'] = _make_authserv_id(configData['AuthservID']) + configData['AuthservID'] = _make_authserv_id(configData.get('AuthservID', 'HOSTNAME')) configData['IntHosts'] = HostsDataset(configData['InternalHosts']) except: pass From b3db013754b8ce9d39828bc6de61859831ecfed0 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Tue, 19 Feb 2019 17:16:26 -0500 Subject: [PATCH 09/15] config: Reassemble strings sensibly If a string-based configuation entry had whitespace in it, it would be reassembled via a round-trip through the python interpreter, resulting in a line like this: PidFile /home/dkimpy-milter/pid file produces a string like "['/home/dkimpy-milter/pid', 'file']", which is clearly wrong. I don't want to encourage people to use paths or other strings with whitespace in them, but if we're going to fail on them we should be failing explicitly, not doing a weird transformation that will just break. This is concretely useful for the DNSOverride mechanism, which is where i ran into the problem when trying to set up testing that could work without setting up an emulated DNS system. --- dkimpy_milter/config.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dkimpy_milter/config.py b/dkimpy_milter/config.py index 601017a..d562e97 100644 --- a/dkimpy_milter/config.py +++ b/dkimpy_milter/config.py @@ -390,7 +390,10 @@ def _readConfigFile(path, configData=None, configGlobal={}): if conversion == 'bool': configData[name] = _find_boolean(value) elif conversion == 'str': - configData[name] = str(value) + if isinstance(value, list): + configData[name] = line.split(None, 1)[1] + else: + configData[name] = str(value) elif conversion == 'int': configData[name] = int(value) elif conversion == 'dataset': From 72ed000ccf2f4d814d39f29b1cbec03eb8f5defa Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Mon, 18 Feb 2019 07:55:33 -0500 Subject: [PATCH 10/15] simple testing framework --- tests/00_minimal.miltertest | 8 ++++++ tests/01_connect.miltertest | 14 ++++++++++ tests/runtests | 51 +++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 tests/00_minimal.miltertest create mode 100755 tests/01_connect.miltertest create mode 100755 tests/runtests diff --git a/tests/00_minimal.miltertest b/tests/00_minimal.miltertest new file mode 100644 index 0000000..a07b48e --- /dev/null +++ b/tests/00_minimal.miltertest @@ -0,0 +1,8 @@ +-- -*- lua -*- +mt.echo("beginning test") +conn = mt.connect("unix:signing.sock") +if conn == nil then + error "mt.connect() failed" +end +mt.disconnect(conn) +mt.echo("test complete") diff --git a/tests/01_connect.miltertest b/tests/01_connect.miltertest new file mode 100755 index 0000000..bd4c730 --- /dev/null +++ b/tests/01_connect.miltertest @@ -0,0 +1,14 @@ +-- -*- lua -*- +mt.echo("beginning test") +conn = mt.connect("unix:signing.sock") +if conn == nil then + error "mt.connect() failed" +end +if mt.conninfo(conn, "localhost", "127.0.0.1") ~= nil then + error "mt.conninfo() failed" +end +if mt.getreply(conn) ~= SMFIR_CONTINUE then + error "mt.conninfo() unexpected reply" +end +mt.disconnect(conn) +mt.echo("test complete") diff --git a/tests/runtests b/tests/runtests new file mode 100755 index 0000000..fbfceba --- /dev/null +++ b/tests/runtests @@ -0,0 +1,51 @@ +#!/bin/bash + +set -e +WORKDIR=$(mktemp -d) +TESTDIR=$(realpath "$(dirname "$0")") + +cd "$WORKDIR" + +dknewkey --ktype ed25519 testkey +cat > signing.conf < stderr:\n" + cat stderr + printf -- "-> end stderr\n" + fi + rm -rf "$WORKDIR" +} + +PYTHONPATH="$(dirname "$TESTDIR")" dkimpy-milter signing.conf 2>stderr & +trap cleanup EXIT + +# ugly ugly (how are we supposed to know that the filter is ready?): +sleep 2 + +# uses miltertest from opendkim: +for x in ${TESTS:-"$TESTDIR"/*.miltertest}; do + if ! [ -e "$x" ]; then + if [ -e "$TESTDIR/$x" ]; then + x="$TESTDIR/$x" + fi + fi + printf -- "-> running %s...\n" "$x" + miltertest -s "$x" +done From ae31730593d78e3c1fef9cf77b680fe83798957a Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Mon, 18 Feb 2019 23:33:51 -0500 Subject: [PATCH 11/15] check for actions claimed by the filter --- tests/01_connect.miltertest | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/01_connect.miltertest b/tests/01_connect.miltertest index bd4c730..4d20bd2 100755 --- a/tests/01_connect.miltertest +++ b/tests/01_connect.miltertest @@ -10,5 +10,27 @@ end if mt.getreply(conn) ~= SMFIR_CONTINUE then error "mt.conninfo() unexpected reply" end + +if mt.test_action(conn, SMFIF_ADDHDRS) then + print "could add headers" +else + error "mt.test_action() says could not add headers" +end + +if mt.test_action(conn, SMFIF_CHGHDRS) then + print "could change headers" +else + error "mt.test_action() says could not change headers" +end + +-- -- FIXME: this part of the test fails, as apparently the +-- -- dkimpy-milter claims the right to change the body of a message, +-- -- even though it shouldn't. How can we fix the negotiation? +-- if mt.test_action(conn, SMFIF_CHGBODY) then +-- error "mt.test_action() says could change body" +-- else +-- print "could not change body" +-- end + mt.disconnect(conn) mt.echo("test complete") From 5c1d5d6e52c0d84080e9c20df8cb0b43632163ea Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Tue, 19 Feb 2019 17:41:09 -0500 Subject: [PATCH 12/15] tests: Run a verifying milter as well as a signing milter Having a verifying milter will come in handy when we want to test both sides of the DKIM process. --- tests/runtests | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/tests/runtests b/tests/runtests index fbfceba..52874e0 100755 --- a/tests/runtests +++ b/tests/runtests @@ -12,28 +12,41 @@ Domain example.net KeyFileEd25519 testkey.key SelectorEd25519 testkey Socket unix:signing.sock -PidFile milter.pid +PidFile signing.pid Mode s UserID $(id --name --user):$(id --name --group) EOF -rm -f milter.pid milter.sock + +cat > verify.conf < stderr:\n" - cat stderr - printf -- "-> end stderr\n" + if [ -s verify.pid ] && kill -0 "$(cat verify.pid)"; then + kill "$(cat verify.pid)" fi + wait + for errdata in signing.stderr verify.stderr; do + if [ -s "$errdata" ]; then + printf -- "-> %s:\n" "$errdata" + cat "$errdata" + printf -- "-> end %s\n" "$errdata" + fi + done rm -rf "$WORKDIR" } -PYTHONPATH="$(dirname "$TESTDIR")" dkimpy-milter signing.conf 2>stderr & +PYTHONPATH="$(dirname "$TESTDIR")" dkimpy-milter signing.conf 2>signing.stderr & +PYTHONPATH="$(dirname "$TESTDIR")" dkimpy-milter verify.conf 2>verify.stderr & trap cleanup EXIT # ugly ugly (how are we supposed to know that the filter is ready?): From 7bfb87fab7720bd4c0f09d550020bd7218469ec7 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Tue, 19 Feb 2019 17:50:32 -0500 Subject: [PATCH 13/15] Set up __main__.py, use it in tests This allows us to invoke dkimpy-milter as "python -m dkimpy_milter dkimpy-milter.conf", which makes running the test suite easier. --- dkimpy_milter/__main__.py | 6 ++++++ tests/dkimpy-milter | 2 ++ tests/runtests | 7 +++++-- 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 dkimpy_milter/__main__.py create mode 100755 tests/dkimpy-milter diff --git a/dkimpy_milter/__main__.py b/dkimpy_milter/__main__.py new file mode 100644 index 0000000..8c5cf9c --- /dev/null +++ b/dkimpy_milter/__main__.py @@ -0,0 +1,6 @@ +#!/usr/bin/python2 + +from dkimpy_milter import main + +if __name__ == "__main__": + main() diff --git a/tests/dkimpy-milter b/tests/dkimpy-milter new file mode 100755 index 0000000..39b64d5 --- /dev/null +++ b/tests/dkimpy-milter @@ -0,0 +1,2 @@ +#!/bin/sh +python2 -m dkimpy_milter "$@" diff --git a/tests/runtests b/tests/runtests index 52874e0..d535e9f 100755 --- a/tests/runtests +++ b/tests/runtests @@ -3,9 +3,12 @@ set -e WORKDIR=$(mktemp -d) TESTDIR=$(realpath "$(dirname "$0")") +DKIMPY_MILTER=${DKIMPY_MILTER:-"$TESTDIR/dkimpy-milter"} cd "$WORKDIR" +printf "Testing %s from directory %s\n" "$DKIMPY_MILTER" "$WORKDIR" + dknewkey --ktype ed25519 testkey cat > signing.conf <signing.stderr & -PYTHONPATH="$(dirname "$TESTDIR")" dkimpy-milter verify.conf 2>verify.stderr & +PYTHONPATH="$(dirname "$TESTDIR")" "$DKIMPY_MILTER" signing.conf 2>signing.stderr & +PYTHONPATH="$(dirname "$TESTDIR")" "$DKIMPY_MILTER" verify.conf 2>verify.stderr & trap cleanup EXIT # ugly ugly (how are we supposed to know that the filter is ready?): From 479820a07d722a35eba89ebd70ef2d152bf3b88e Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Tue, 19 Feb 2019 00:31:19 -0500 Subject: [PATCH 14/15] tests: test DKIM signing and verification This test makes use of DNSOverride and the new verifying milter to ensure that signatures can be verified properly. It doesn't test the actual interaction with the public DNS, but getting that kind of test to work on arbitrary platforms might be more trouble than it's worth. I note that the DNSOverride only works as long as testkey.dns is a single line, which is fine for ed25519, but maybe not for RSA. --- tests/02_sign_message.miltertest | 98 ++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 tests/02_sign_message.miltertest diff --git a/tests/02_sign_message.miltertest b/tests/02_sign_message.miltertest new file mode 100644 index 0000000..6bd4f4b --- /dev/null +++ b/tests/02_sign_message.miltertest @@ -0,0 +1,98 @@ +-- -*- lua -*- +mt.echo("beginning test") + +msg = { + ['headers'] = { + ['From'] = 'Alice ', + ['Message-Id'] = '', + ['To'] = 'Bob ', + ['Date'] = 'Mon, 18 Feb 2019 08:32:50 -0500', + ['Subject'] = 'Signing test', + ['Content-Type'] = 'text/plain', + }, + ['body'] = "This is a test!\r\n", +} + +-- returns miltertest connection object +function connect_and_send (sockname, headers, body) + conn = mt.connect(sockname) + if conn == nil then + error "mt.connect() failed" + end + if mt.conninfo(conn, "localhost", "127.0.0.1") ~= nil then + error "mt.conninfo() failed" + end + if mt.getreply(conn) ~= SMFIR_CONTINUE then + error "mt.conninfo() unexpected reply" + end + + -- mt.macro(conn, SMFIC_MAIL, "i", "simple-message") + if mt.mailfrom(conn, "") ~= nil then + error "mt.mailfrom() failed" + end + if mt.getreply(conn) ~= SMFIR_CONTINUE then + error "mt.mailfrom() unexpected reply" + end + -- mt.rcptto() is called implicitly + + -- send headers + for key,value in pairs(headers) do + if mt.header(conn, key, value) ~= nil then + error("mt.header(" .. key .. ") failed") + end + if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.header(" .. key .. ") unexpected reply") + end + end + -- send EOH + if mt.eoh(conn) ~= nil then + error "mt.eoh() failed" + end + if mt.getreply(conn) ~= SMFIR_CONTINUE then + error "mt.eoh() unexpected reply" + end + + -- send body + if mt.bodystring(conn, body) ~= nil then + error "mt.bodystring() failed" + end + if mt.getreply(conn) ~= SMFIR_CONTINUE then + error "mt.bodystring() unexpected reply" + end + -- end of message; let the filter react + if mt.eom(conn) ~= nil then + error "mt.eom() failed" + end + reply = mt.getreply(conn) + if reply ~= SMFIR_CONTINUE then + error ("mt.eom() unexpected reply: " .. reply) + end + return conn +end + +signing = connect_and_send("unix:signing.sock", msg.headers, msg.body) +-- verify that a test header field got added +if not mt.eom_check(signing, MT_HDRINSERT) then + error "no header added by signer" +end + +signature = mt.getheader(signing, "DKIM-Signature", 0) + +mt.disconnect(signing) + +mt.echo("DKIM-Signature: " .. signature) + +msg.headers['DKIM-Signature'] = signature + +verify = connect_and_send("unix:verify.sock", msg.headers, msg.body) + +if not mt.eom_check(verify, MT_HDRINSERT) then + error "no header added in verify" +end + +authres = mt.getheader(verify, "Authentication-Results", 0) +mt.echo("Authentication-Results: "..authres) + +mt.disconnect(verify) + +mt.echo("test complete") From ad8f396db0700e5bc351d74d2fa57f71df464026 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 10:49:22 -0500 Subject: [PATCH 15/15] Expand test suite to cover RSA as well as ed25519 --- tests/00_minimal.miltertest | 16 +++++--- tests/01_connect.miltertest | 60 +++++++++++++++------------- tests/02_sign_message.miltertest | 54 ++++++++++++------------- tests/runtests | 67 ++++++++++++++++++++------------ 4 files changed, 112 insertions(+), 85 deletions(-) diff --git a/tests/00_minimal.miltertest b/tests/00_minimal.miltertest index a07b48e..fbe0849 100644 --- a/tests/00_minimal.miltertest +++ b/tests/00_minimal.miltertest @@ -1,8 +1,12 @@ -- -*- lua -*- -mt.echo("beginning test") -conn = mt.connect("unix:signing.sock") -if conn == nil then - error "mt.connect() failed" +for _, keytype in ipairs({"ed25519", "rsa"}) do + for _, func in ipairs({"signing", "verify"}) do + mt.echo("testing "..keytype.." "..func) + conn = mt.connect("unix:"..keytype.."."..func..".sock") + if conn == nil then + error("mt.connect() failed "..keytype.." "..func) + end + mt.disconnect(conn) + mt.echo(keytype.." "..func.." complete") + end end -mt.disconnect(conn) -mt.echo("test complete") diff --git a/tests/01_connect.miltertest b/tests/01_connect.miltertest index 4d20bd2..2f43eff 100755 --- a/tests/01_connect.miltertest +++ b/tests/01_connect.miltertest @@ -1,36 +1,40 @@ -- -*- lua -*- -mt.echo("beginning test") -conn = mt.connect("unix:signing.sock") -if conn == nil then - error "mt.connect() failed" -end -if mt.conninfo(conn, "localhost", "127.0.0.1") ~= nil then - error "mt.conninfo() failed" -end -if mt.getreply(conn) ~= SMFIR_CONTINUE then - error "mt.conninfo() unexpected reply" -end +for _, keytype in ipairs({"ed25519", "rsa"}) do + for _, func in ipairs({"signing", "verify"}) do + mt.echo("testing "..keytype.." "..func) + conn = mt.connect("unix:"..keytype.."."..func..".sock") + if conn == nil then + error("mt.connect() failed "..keytype.." "..func) + end + if mt.conninfo(conn, "localhost", "127.0.0.1") ~= nil then + error("mt.conninfo() failed "..keytype.." "..func) + end + if mt.getreply(conn) ~= SMFIR_CONTINUE then + error("mt.conninfo() unexpected reply "..keytype.." "..func) + end -if mt.test_action(conn, SMFIF_ADDHDRS) then - print "could add headers" -else - error "mt.test_action() says could not add headers" -end + if mt.test_action(conn, SMFIF_ADDHDRS) then + print("could add headers "..keytype.." "..func) + else + error("mt.test_action() says could not add headers "..keytype.." "..func) + end -if mt.test_action(conn, SMFIF_CHGHDRS) then - print "could change headers" -else - error "mt.test_action() says could not change headers" -end + if mt.test_action(conn, SMFIF_CHGHDRS) then + print("could change headers "..keytype.." "..func) + else + error("mt.test_action() says could not change headers "..keytype.." "..func) + end -- -- FIXME: this part of the test fails, as apparently the -- -- dkimpy-milter claims the right to change the body of a message, -- -- even though it shouldn't. How can we fix the negotiation? --- if mt.test_action(conn, SMFIF_CHGBODY) then --- error "mt.test_action() says could change body" --- else --- print "could not change body" --- end +-- if mt.test_action(conn, SMFIF_CHGBODY) then +-- error("mt.test_action() says could change body "..keytype.." "..func) +-- else +-- print("could not change body "..keytype.." "..func) +-- end -mt.disconnect(conn) -mt.echo("test complete") + mt.disconnect(conn) + mt.echo(keytype.." "..func.." test complete") + end +end diff --git a/tests/02_sign_message.miltertest b/tests/02_sign_message.miltertest index 6bd4f4b..cb5e7ff 100644 --- a/tests/02_sign_message.miltertest +++ b/tests/02_sign_message.miltertest @@ -1,5 +1,4 @@ -- -*- lua -*- -mt.echo("beginning test") msg = { ['headers'] = { @@ -70,29 +69,32 @@ function connect_and_send (sockname, headers, body) return conn end -signing = connect_and_send("unix:signing.sock", msg.headers, msg.body) --- verify that a test header field got added -if not mt.eom_check(signing, MT_HDRINSERT) then - error "no header added by signer" +for _, keytype in ipairs({"ed25519", "rsa"}) do + mt.echo("testing "..keytype) + signing = connect_and_send("unix:"..keytype..".signing.sock", msg.headers, msg.body) + -- verify that a test header field got added + if not mt.eom_check(signing, MT_HDRINSERT) then + error "no header added by signer" + end + + signature = mt.getheader(signing, "DKIM-Signature", 0) + + mt.disconnect(signing) + + mt.echo("DKIM-Signature: " .. signature) + + msg.headers['DKIM-Signature'] = signature + + verify = connect_and_send("unix:"..keytype..".verify.sock", msg.headers, msg.body) + + if not mt.eom_check(verify, MT_HDRINSERT) then + error "no header added in verify" + end + + authres = mt.getheader(verify, "Authentication-Results", 0) + mt.echo("Authentication-Results: "..authres) + + mt.disconnect(verify) + + mt.echo(keytype.." complete") end - -signature = mt.getheader(signing, "DKIM-Signature", 0) - -mt.disconnect(signing) - -mt.echo("DKIM-Signature: " .. signature) - -msg.headers['DKIM-Signature'] = signature - -verify = connect_and_send("unix:verify.sock", msg.headers, msg.body) - -if not mt.eom_check(verify, MT_HDRINSERT) then - error "no header added in verify" -end - -authres = mt.getheader(verify, "Authentication-Results", 0) -mt.echo("Authentication-Results: "..authres) - -mt.disconnect(verify) - -mt.echo("test complete") diff --git a/tests/runtests b/tests/runtests index d535e9f..7878f17 100755 --- a/tests/runtests +++ b/tests/runtests @@ -4,55 +4,72 @@ set -e WORKDIR=$(mktemp -d) TESTDIR=$(realpath "$(dirname "$0")") DKIMPY_MILTER=${DKIMPY_MILTER:-"$TESTDIR/dkimpy-milter"} +KEY_TYPES=(ed25519 rsa) cd "$WORKDIR" printf "Testing %s from directory %s\n" "$DKIMPY_MILTER" "$WORKDIR" -dknewkey --ktype ed25519 testkey -cat > signing.conf < "$keytype.signing.conf" < verify.conf < "$keytype.verify.conf" < %s:\n" "$errdata" - cat "$errdata" - printf -- "-> end %s\n" "$errdata" - fi + for keytype in "${KEY_TYPES[@]}"; do + for func in signing verify; do + errdata="$keytype.$func.stderr" + if [ -s "$errdata" ]; then + printf -- "-> %s:\n" "$errdata" + cat "$errdata" + printf -- "-> end %s\n" "$errdata" + fi + done done rm -rf "$WORKDIR" } -PYTHONPATH="$(dirname "$TESTDIR")" "$DKIMPY_MILTER" signing.conf 2>signing.stderr & -PYTHONPATH="$(dirname "$TESTDIR")" "$DKIMPY_MILTER" verify.conf 2>verify.stderr & +for keytype in "${KEY_TYPES[@]}"; do + for func in signing verify; do + PYTHONPATH="$(dirname "$TESTDIR")" "$DKIMPY_MILTER" "$keytype.$func.conf" 2>"$keytype.$func.stderr" & + done +done trap cleanup EXIT -# ugly ugly (how are we supposed to know that the filter is ready?): +# ugly ugly (how are we supposed to know that the milters are all ready?): sleep 2 # uses miltertest from opendkim: