From 2cda1758c1287a124db31e4c91412fa526c8983e Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Fri, 1 Feb 2019 15:11:00 -0500 Subject: [PATCH 01/32] Fix spelling error in README --- README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README b/README index 53f75aa..a09a854 100644 --- a/README +++ b/README @@ -12,7 +12,7 @@ default is a feature: python setup.py install --single-version-externally-managed --record=/dev/null -For users of Debian Stable (Debian 9, Codename Squueze), all dependencies are +For users of Debian Stable (Debian 9, Codename Squeeze), all dependencies are available in either the main or backports repositories: [sudo] apt install python-milter python-nacl python-ipaddress python-dnspython From 03c86a2b08cb3c74ebce92bcbeadc0d357eabeab Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Fri, 1 Feb 2019 15:15:50 -0500 Subject: [PATCH 02/32] Fix grammar error in README --- README | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README b/README index a09a854..d91a378 100644 --- a/README +++ b/README @@ -80,9 +80,9 @@ http://www.elandsys.com/resources/sendmail/milter.html For Postfix: Integration of dkimpy-milter into Postfix is like any milter (See Postfix's -README_FILES/MILTER_README). Here's an example master.cf excerpt the talks to -two dkimpy-milter instances, one configured for signing and one configured for -verification: +README_FILES/MILTER_README). Here's an example master.cf excerpt that talks +to two dkimpy-milter instances, one configured for signing and one configured +for verification: smtp inet n - - - - smtpd ... From e951ab6c5e6f39e462a9e65cd8cfff5359978580 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Fri, 8 Feb 2019 01:12:21 -0500 Subject: [PATCH 03/32] Remove obsolete .IX macro from man pages Conflicts: man/dkimpy-milter.conf.5 --- man/dkimpy-milter.8 | 1 - man/dkimpy-milter.conf.5 | 9 --------- 2 files changed, 10 deletions(-) diff --git a/man/dkimpy-milter.8 b/man/dkimpy-milter.8 index 1a996d9..4ed9746 100644 --- a/man/dkimpy-milter.8 +++ b/man/dkimpy-milter.8 @@ -127,7 +127,6 @@ .rm #[ #] #H #V #F C .\" ======================================================================== .\" -.IX Title "dkimpy-milter 8" .TH dkimpyy-milter 8 .SH NAME .B dkimpy diff --git a/man/dkimpy-milter.conf.5 b/man/dkimpy-milter.conf.5 index a6f8b5e..3dd7612 100644 --- a/man/dkimpy-milter.conf.5 +++ b/man/dkimpy-milter.conf.5 @@ -127,16 +127,13 @@ .rm #[ #] #H #V #F C .\" ======================================================================== .\" -.IX Title "dkimpy-milter.conf 5" .TH dkimpy-milter.conf 5 "2018-02-12" .SH "NAME" dkimpy-milter \- Python milter for DKIM signing and validation .SH "VERSION" -.IX Header "VERSION" 0\.9\.2 .SH "DESCRIPTION" -.IX Header "DESCRIPTION" .I dkimpy-milter(8) implements the .B DKIM @@ -160,18 +157,15 @@ The provided setup.py installs this configuration file in /etc or Command line invocation of parameters as is done by OpenDKIM is not supported. .SH "USAGE" -.IX Header "USAGE" Usage: dkimpy-milter [/etc/dkimpy-milter.conf] .SH "OTHER DOCUMENTATION" -.IX Header "OTHER DOCUMENTATION" This documentation assumes you have read Postfix's README_FILES/MILTER_README (or Sendmail equivalent) and are generally familiar with Domain Keys Identified Mail (DKIM). See RFC 6376 for details. .SH "SYNOPSIS" -.IX Header "SYNOPSIS" dkimpy-milter operates with a default installed configuration file and set of default configuration options that are used if the configuration file @@ -181,14 +175,12 @@ files can be used directly. Not all OpenDKIM options are supported. If an unsupported option from OpenDKIM is specified, an error will be raised. .SH "DESCRIPTION" -.IX Header "DESCRIPTION" Configuration options are described here and in the configuration file provided with the package. The provided setup.py installs this configuration file in /etc or /usr/local/etc. .SH "OPTIONS" -.IX Header "OPTIONS" .TP .I AuthservID (string) @@ -441,7 +433,6 @@ unless an alternate is specified. .SH "AUTHORS" -.IX Header "AUTHORS" \ddkimpy-milter\fR was written by Scott Kitterman . It is based on dkimpy-milter.py Copyright (c) 2001-2013 Business Management Systems, Inc. Copyright (c) 2013-2015 Stuart D. Gathman From 06948b3dbc023a1c18880e1293ce7a130eb7cd88 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Fri, 8 Feb 2019 04:02:34 -0500 Subject: [PATCH 04/32] Update references in man/dkimpy-milter.8 --- man/dkimpy-milter.8 | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/man/dkimpy-milter.8 b/man/dkimpy-milter.8 index 4ed9746..0900c4f 100644 --- a/man/dkimpy-milter.8 +++ b/man/dkimpy-milter.8 @@ -268,7 +268,7 @@ proposal, and Cisco's .B Internet Identified Mail (IIM) proposal. .SH VERSION -This man page covers version 0.9.4 of +This man page covers version 1.1.0 of .I dkimpy-milter. .SH COPYRIGHT Copyright (c) 2005-2008, Sendmail, Inc. and its suppliers. All rights @@ -277,7 +277,7 @@ reserved. Copyright (c) 2009-2013, 2015, The Trusted Domain Project. All rights reserved. -Copyright (c) 2018 Scott Kitterman +Copyright (c) 2018, 2019 Scott Kitterman .SH SEE ALSO .I dkimpy-milter.conf(5), sendmail(8) .P @@ -291,4 +291,6 @@ RFC6376 - DomainKeys Identified Mail .P RFC7601 - Message Header Field for Indicating Message Authentication Status .P -draft-ietf-dcrup-dkim-crypto - A new cryptographic signature method for DKIM +RFC8301 - Cryptographic Algorithm and Key Usage Update to DomainKeys Identified Mail (DKIM) +.P +RFC8463 - A New Cryptographic Signature Method for DomainKeys Identified Mail (DKIM) From f38fed3beec91679ca20e8334fbf2ae7eda02842 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Mon, 11 Feb 2019 03:16:53 -0500 Subject: [PATCH 05/32] Rip out unused whichbd module in preparation for python3 port --- dkimpy_milter/config.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/dkimpy_milter/config.py b/dkimpy_milter/config.py index 6086e96..9f42af2 100644 --- a/dkimpy_milter/config.py +++ b/dkimpy_milter/config.py @@ -294,19 +294,7 @@ def _dataset_to_list(dataset): else: return [dataset.strip().strip(',')] if dataset[-3:] == '.db' or dataset[:3] == 'db:': - # This is a Sleepycat (Oracle) DB dataset - import whichdb # Will need rewriting someday for python3 - if dataset[-3:] == '.db': - dbname = dataset - elif dataset[:3] == 'db:': - dbname = dataset[3:] - else: - raise dkim.ParameterError('Unimplmented dataset type: {0}' - .format(type(dataset))) - if whichdb.whichdb(dbname) != 'dbhash': - raise dkim.ParameterError('Unimplmented dataset type: {0}' - .format(type(dataset))) - #TODO replace this with code to use db maps + # This is a Sleepycat (Oracle) DB dataset, which we dont support raise dkim.ParameterError('Unsupported dataset db datase: {0}' .format(type(dataset))) From 5945e818ca7428688f274669fc16d381ebbc7334 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Mon, 11 Feb 2019 13:32:37 -0500 Subject: [PATCH 06/32] =?UTF-8?q?=20-=20Add=20additional=20Sendmail=20conf?= =?UTF-8?q?iguration=20information=20to=20README=20from=20OpenDKIM=20=20?= =?UTF-8?q?=20=20update=20based=20on=20input=20from=20=D0=94=D0=B8=D0=BB?= =?UTF-8?q?=D1=8F=D0=BD=20=D0=9F=D0=B0=D0=BB=D0=B0=D1=83=D0=B7=D0=BE=D0=B2?= =?UTF-8?q?=20(LP:=20#1801619)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGES | 4 ++++ README | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/CHANGES b/CHANGES index 06cd982..ab6c19e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +1.0.1 UNRELEASED + - Add additional Sendmail configuration information to README from OpenDKIM + update based on input from Дилян Палаузов (LP: #1801619) + 1.0.0 2018-05-11 - Minor documentation updates - Deleted reference to obsolete syslog target in unit file diff --git a/README b/README index d91a378..5efdeff 100644 --- a/README +++ b/README @@ -77,6 +77,57 @@ Milter support should be present by default in most versions of sendmail these days, but if not included in your Sendmail build, see: http://www.elandsys.com/resources/sendmail/milter.html +ISSUES USING SENDMAIL TO SIGN AND VERIFY +======================================== + +When using the sendmail MTA in both signing and verifying mode, there are +a few issues of which to be aware that might cause operational problems +and deserve consideration. + +(a) When the MTA will be used for relaying emails, e.g. delivering to other + hosts using the aliases mechanism, it is important not to break + signatures inserted by the original sender. This is particularly sensitive + particular when the sending domain has published a "reject" DMARC policy. + + By default, sendmail quotes to address header fields when there are no + quotes and the display part of the address contains a period or an + apostrophe. However, opendkim only sees the raw, unmodified form of + the header field, and so the content that gets verified and what gets + signed will not be the same, guaranteeing the attached signature is not + valid. + + To direct sendmail not to modify the headers, add this to your sendmail.mc: + + conf(`confMUST_QUOTE_CHARS', `') + +(b) As stated in sendmail's KNOWNBUGS file, sendmail truncates header field + values longer than 256 characters, which could mean truncating the domain + of a long From: header field value and invalidating the signature. + You may wish to consider increasing MAXNAME in sendmail/conf.h to mitigate + changing the messages and invalidating their signatures. This change + requires recompiling sendmail. + +(c) Similar to (a) above, sendmail may wrap very long single-line recipient + fields for presentation purposes; for example: + + To: very long name ,anotherloo...ong name b + + ...might be rewritten as: + + To: very long name , + anotherloo...ong name b + + This rewrite is also done after opendkim has seen the message, meaning + the signature opendkim attaches to the message does not match the + content it signed. There is not a known configuration change to + mitigate this mutation. + + The only known mechanism for dealing with this is to have distinct + instances of opendkim do the verifying (inbound) and signing (outbound) + so that the version that arrives at the signing instance is already + in the rewritten form, guaranteeing the input and output are the same + and thus the signature matches the payload. + For Postfix: Integration of dkimpy-milter into Postfix is like any milter (See Postfix's From ea2b612e8d6ebad3caf270549392aff17fcee1f0 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Mon, 11 Feb 2019 14:23:55 -0500 Subject: [PATCH 07/32] - Add information on Ed25519 key creation to README (LP: #1815313) --- CHANGES | 1 + README | 74 ++++++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/CHANGES b/CHANGES index ab6c19e..697f21c 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,7 @@ 1.0.1 UNRELEASED - Add additional Sendmail configuration information to README from OpenDKIM update based on input from Дилян Палаузов (LP: #1801619) + - Add information on Ed25519 key creation to README (LP: #1815313) 1.0.0 2018-05-11 - Minor documentation updates diff --git a/README b/README index 5efdeff..f7de656 100644 --- a/README +++ b/README @@ -1,10 +1,17 @@ -This is a DKIM signing and verification milter. In theory it has been tested -with both Postfix and Sendmail. +OVERVIEW +======== + +This is a DKIM signing and verification milter. It has been tested with both +Postfix and Sendmail. The configuration file is designed to be compatible with OpenDKIM, but only a subset of OpenDKIM options are supported. If an unsupported option is specified, an error will be raised. + +INSTALLATION +=========== + This package includes a default configuration file and man pages. For those to be installed when installing using setup.py, the following incantation is required because setuptools developers decided not being able to do this by @@ -33,6 +40,48 @@ The milter will work with either pydns (DNS) or dnspython (dns), preferring dnspython is both are available. The dkimpy DKIM module also works with either. + +SETUP +==== + +SIGNING KEYS +============ + +In order to create DKIM signatures, a private key must be available. Signing +keys should be protected (owned by root:root with permissions 600 in a +directory that is not world readable). Different keys are required for RSA +and (if used) Ed25519. + +RSA +=== + +Both public and private keys for RSA have standard formats and there are many +tools available to create them. Keys must (RFC 8302) have a minimum size of +1024 bits and should have a size of at least 2048 bits. The dknewkey script +that is provided with dkimpy is one such tool: + +dknewkey exampleprivkey + +will produce both the private key file (.key suffix) and a file with the DKIM +public key record to be published DNS (.dns suffix). RSA is the default key +type. 2048 bits is the default key size. + +ED25519 +======= + +There is no standardized non-binary representation for Ed25519 private keys, +so in order to generate Ed25519 keys for dkimpy-milter, dkimpy specific tools +must be used to be compatible. The same dknewkey script support Ed25519: + +dknewkey --ktype ed25519 anothernewkey + +will provide both the private key file (.key suffix) and a file with the DKIM +public key record to be published DNS (.dns suffix). Ed25519 keys do not have +variable bit lengths. + +MTA INTEGRATION +============== + Both a systemd unit file and a sysv init file are provided. Both make assumptions about defaults being used, e.g. if a non-standard pidfile name is used, they will need to be updated. The sysv init file is Debian specific and @@ -61,7 +110,8 @@ the following steps: As with all milters, dkimpy-milter needs to be integrated with your MTA of choice (Sendmail or Postfix). -For Sendmail: +SENDMAIL +======== Configuration is very similar to opendkim, but needs some adjustment for dkimpy-milter. Here's an example configuration line to include in your @@ -128,7 +178,8 @@ and deserve consideration. in the rewritten form, guaranteeing the input and output are the same and thus the signature matches the payload. -For Postfix: +POSTFIX +======= Integration of dkimpy-milter into Postfix is like any milter (See Postfix's README_FILES/MILTER_README). Here's an example master.cf excerpt that talks @@ -178,14 +229,15 @@ MacroListVerify daemon_name|VERIFYING ... +NOTES +===== + The python DKIM library, dkimpy, requires the entire message being signed or verified to be in memory, so dkimpy-milter does not write messages out to a temp file. This may impact performance on low-memory systems. -This is an initial production release to support interoperability testing with -Ed25519 signatures sufficient functionality for basic use. The documented -functionality has been implemented and at generally partially tested. It is -free of known defects, but is not fully tested in a variety of environments. - -DKIM Ed25519 signatures are still in development, but the specification is -technically stable. Version 1.0.0 supports draft-ietf-dcrup-dkim-crypto-09. +DKIM with Ed25519 signatures are described in RFC 8463. Version 1.0.0 and +later support Ed25519 signing and verification. RFC 8301 removed rsa-sha1 +from DKIM. dkimpy-milter does not sign with rsa-sha1, but still considers +rsa-sha1 signatures as valid for verification because they are still in +common use and are not known to be cryptographically broken. From b1abbf9d6133a08e8db149d41c2a1bf7ddfc77a8 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Mon, 11 Feb 2019 14:55:35 -0500 Subject: [PATCH 08/32] - Make domain checks case insensitive for determining if signing should be done (LP: #1815311) --- CHANGES | 2 ++ dkimpy_milter/__init__.py | 12 ++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/CHANGES b/CHANGES index 697f21c..8fc3de5 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,6 @@ 1.0.1 UNRELEASED + - Make domain checks case insensitive for determining if signing should be + done (LP: #1815311) - Add additional Sendmail configuration information to README from OpenDKIM update based on input from Дилян Палаузов (LP: #1801619) - Add information on Ed25519 key creation to README (LP: #1815313) diff --git a/dkimpy_milter/__init__.py b/dkimpy_milter/__init__.py index 61bb1dd..07c8bfb 100644 --- a/dkimpy_milter/__init__.py +++ b/dkimpy_milter/__init__.py @@ -131,7 +131,7 @@ class dkimMilter(Milter.Base): if lname == 'from': fname, self.author = parseaddr(val) try: - self.fdomain = self.author.split('@')[1] + self.fdomain = self.author.split('@')[1].lower() except IndexError as er: self.fdomain = '' # self.author was not a proper email address if (milterconfig.get('Syslog') and @@ -228,7 +228,7 @@ class dkimMilter(Milter.Base): 'd={3})'.format(self.getsymval('i'), d.signature_fields.get(b'a'), d.signature_fields.get(b's'), - d.domain)) + d.domain.lower())) if privateEd25519: d = dkim.DKIM(txt) h = d.sign(milterconfig.get('SelectorEd25519'), self.fdomain, @@ -244,7 +244,7 @@ class dkimMilter(Milter.Base): 'd={3})'.format(self.getsymval('i'), d.signature_fields.get(b'a'), d.signature_fields.get(b's'), - d.domain)) + d.domain.lower())) except dkim.DKIMException as x: if milterconfig.get('Syslog'): syslog.syslog('DKIM: {0}'.format(x)) @@ -292,8 +292,8 @@ class dkimMilter(Milter.Base): 'd={3})'.format(self.getsymval('i'), d.signature_fields.get(b'a'), d.signature_fields.get(b's'), - d.domain)) - self.dkim_domain = d.domain + d.domain.lower())) + self.dkim_domain = d.domain.lower() else: if milterconfig.get('DiagnosticDirectory'): fd, fname = tempfile.mkstemp(".dkim") @@ -303,7 +303,7 @@ class dkimMilter(Milter.Base): syslog.syslog('DKIM: Fail (saved as {0})' .format(fname)) else: - syslog.syslog('DKIM: Fail ({0})'.format(d.domain)) + syslog.syslog('DKIM: Fail ({0})'.format(d.domain.lower())) if res: result = 'pass' else: From aa4dadc22f9478df8e66f753e012b7aae7ddbf34 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Mon, 11 Feb 2019 15:09:34 -0500 Subject: [PATCH 09/32] * Reorder milter start and dropping privileges so permissions on Unix socket are correct (LP: 1797720) --- CHANGES | 2 ++ dkimpy_milter/__init__.py | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 8fc3de5..5e6358e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,6 @@ 1.0.1 UNRELEASED + * Reorder milter start and dropping privileges so permissions on Unix socket + are correct (LP: 1797720) - Make domain checks case insensitive for determining if signing should be done (LP: #1815311) - Add additional Sendmail configuration information to README from OpenDKIM diff --git a/dkimpy_milter/__init__.py b/dkimpy_milter/__init__.py index 07c8bfb..56dd228 100644 --- a/dkimpy_milter/__init__.py +++ b/dkimpy_milter/__init__.py @@ -348,13 +348,13 @@ def main(): Milter.set_flags(Milter.CHGHDRS + Milter.ADDHDRS) miltername = 'dkimpy-filter' socketname = milterconfig.get('Socket') + own_socketfile(milterconfig) + drop_privileges(milterconfig) + sys.stdout.flush() + Milter.runmilter(miltername, socketname, 240) if milterconfig.get('Syslog'): syslog.syslog('dkimpy-milter started:{0} user:{1}' .format(pid, milterconfig.get('UserID'))) - sys.stdout.flush() - Milter.runmilter(miltername, socketname, 240) - own_socketfile(milterconfig) - drop_privileges(milterconfig) if __name__ == "__main__": main() From 51c8fdcb6cc08cedd2e1217083df6c8cac74fae1 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Mon, 11 Feb 2019 15:14:11 -0500 Subject: [PATCH 10/32] Bump version to 1.0.1, update TODO, set release date --- CHANGES | 2 +- TODO | 14 ++++++++++---- dkimpy_milter/__init__.py | 2 +- setup.py | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/CHANGES b/CHANGES index 5e6358e..4c11ec8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,4 @@ -1.0.1 UNRELEASED +1.0.1 2019-02-11 * Reorder milter start and dropping privileges so permissions on Unix socket are correct (LP: 1797720) - Make domain checks case insensitive for determining if signing should be diff --git a/TODO b/TODO index f9ca585..cbb4a2d 100644 --- a/TODO +++ b/TODO @@ -39,10 +39,16 @@ MacroListVerify implemented verified SyslogSuccess implemented verified 1.0.0 -No additional features planned +No additional features -Plannedataset type support (if needed): -db:/.db +1.0.1 +Bug fix only, improved documentation + +1.1.0 (planned) +Port to Python 3 +Subdomain support + +Planned dataset type support (if needed): mdb: Considered for near-term feature release @@ -50,7 +56,7 @@ Considered for near-term feature release AlwaysAddARHeader ChangeRootDirectory ClockDrift (requires dkimpy change) -DNSTimeout (requires dkmpy change) +DNSTimeout (requires dkimpy change) MilterDebug MinimumKeyBits OversignHeaders (may require dkimpy changes) diff --git a/dkimpy_milter/__init__.py b/dkimpy_milter/__init__.py index 56dd228..3791748 100644 --- a/dkimpy_milter/__init__.py +++ b/dkimpy_milter/__init__.py @@ -39,7 +39,7 @@ from dkimpy_milter.util import read_keyfile from dkimpy_milter.util import own_socketfile from dkimpy_milter.util import fold -__version__ = "1.0.0" +__version__ = "1.0.1" FWS = re.compile(r'\r?\n[ \t]+') diff --git a/setup.py b/setup.py index d5180af..41a11a5 100644 --- a/setup.py +++ b/setup.py @@ -30,7 +30,7 @@ except ImportError: # If PyDNS is not installed, prefer dnspython setup( name='dkimpy-milter', - version='1.0.0', + version='1.0.1', author='Scott Kitterman', author_email='scott@kitterman.com', url='https://launchpad.net/dkimpy-milter', From e872bd44b0622299fc154139ca0bdfd09ae448db Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Mon, 18 Feb 2019 08:02:27 -0500 Subject: [PATCH 11/32] 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 12/32] 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 13/32] 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 14/32] 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 15/32] 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 16/32] 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 17/32] 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 18/32] 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 19/32] 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 20/32] 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 21/32] 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 22/32] 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 23/32] 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 24/32] 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 25/32] 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: From 391b5352f396d8fed9b497558a3566c06fc3bdf8 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Wed, 20 Feb 2019 16:57:32 -0500 Subject: [PATCH 26/32] Convert mostly to python3 (still need strings/bytes conversions) This covers conversion of the whole project to python3, *except* for the strings/bytes distinction in __init__.py, which i'm leaving for a second commit. The changes in this commit are intended to be relatively uncontroversial, so that the following commit contains the tricky bits. --- dkimpy_milter/__init__.py | 2 +- dkimpy_milter/__main__.py | 2 +- dkimpy_milter/config.py | 20 ++++++++++---------- dkimpy_milter/dnsplug.py | 16 ++++++++-------- setup.py | 8 ++++---- tests/dkimpy-milter | 2 +- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/dkimpy_milter/__init__.py b/dkimpy_milter/__init__.py index 5345fc7..c4c407b 100644 --- a/dkimpy_milter/__init__.py +++ b/dkimpy_milter/__init__.py @@ -1,4 +1,4 @@ -#! /usr/bin/python2 +#! /usr/bin/python3 # Original dkim-milter.py code: # Author: Stuart D. Gathman # Copyright 2007 Business Management Systems, Inc. diff --git a/dkimpy_milter/__main__.py b/dkimpy_milter/__main__.py index 8c5cf9c..f95075f 100644 --- a/dkimpy_milter/__main__.py +++ b/dkimpy_milter/__main__.py @@ -1,4 +1,4 @@ -#!/usr/bin/python2 +#!/usr/bin/python3 from dkimpy_milter import main diff --git a/dkimpy_milter/config.py b/dkimpy_milter/config.py index d562e97..bf6551a 100644 --- a/dkimpy_milter/config.py +++ b/dkimpy_milter/config.py @@ -31,13 +31,13 @@ import stat import dkim import socket import ipaddress -from dnsplug import Session +from .dnsplug import Session # default values defaultConfigData = { 'Syslog': 'yes', 'SyslogFacility': 'mail', - 'UMask': 007, + 'UMask': 0o07, 'Mode': 'sv', 'Socket': 'local:/var/run/dkimpy-milter/dkimpy-milter.sock', 'PidFile': '/var/run/dkimpy-milter/dkimpy-milter.pid', @@ -85,14 +85,14 @@ class HostsDataset(object): self.item = item[1:] self.negative = True try: - self.item = ipaddress.ip_address(unicode(self.item, "utf-8")) + self.item = ipaddress.ip_address(str(self.item, "utf-8")) if isinstance(self.item, ipaddress.IPv4Address): self.isipv4 = True elif isinstance(self.item, ipaddress.IPv6Address): self.isipv6 = True except ValueError as e: try: - self.item = ipaddress.ip_network(unicode + self.item = ipaddress.ip_network(str (self.item, "utf-8"), strict=False) if isinstance(self.item, ipaddress.IPv4Network): @@ -110,7 +110,7 @@ class HostsDataset(object): def match(self, connectip): '''Check if the connect IP is part of the dataset''' - source = ipaddress.ip_address(unicode(connectip, "utf-8")) + source = ipaddress.ip_address(str(connectip, "utf-8")) for item in self.dataset: if item.isdomain or item.ishostname: result = self.matchname(source) # Match host/domains first @@ -160,13 +160,13 @@ class HostsDataset(object): if isinstance(source, ipaddress.IPv4Address): ips = s.dns(name, 'A') for ip in ips: - ip = ipaddress.IPv4Address(unicode(ip, 'UTF-8')) + ip = ipaddress.IPv4Address(str(ip, 'UTF-8')) if ip == source: results.append(name) if isinstance(source, ipaddress.IPv6Address): ips = s.dns(name, 'AAAA') for ip in ips: - ip = ipaddress.IPv6Address(unicode(ip, 'UTF-8')) + ip = ipaddress.IPv6Address(str(ip, 'UTF-8')) if ip == source: results.append(name) return results @@ -225,13 +225,13 @@ def _processConfigFile(filename=None, configdata=None, useSyslog=1, '''Load the specified config file, exit and log errors if it fails, otherwise return a config dictionary.''' - import config + from . import config if configdata is None: configdata = config.defaultConfigData if filename is not None: try: _readConfigFile(filename, configdata) - except Exception, e: + except Exception as e: raise if useSyslog: syslog.syslog(e.args[0]) @@ -342,7 +342,7 @@ def _readConfigFile(path, configData=None, configGlobal={}): # check to see if it's a file try: mode = os.stat(path)[0] - except OSError, e: + except OSError as e: syslog.syslog(syslog.LOG_ERR, 'ERROR stating "%s": %s' % (path, e.strerror)) return(configData) diff --git a/dkimpy_milter/dnsplug.py b/dkimpy_milter/dnsplug.py index aa357ec..33067e9 100644 --- a/dkimpy_milter/dnsplug.py +++ b/dkimpy_milter/dnsplug.py @@ -84,7 +84,7 @@ class Session(object): raise DNSError('Length of CNAME chain exceeds %d' % MAX_CNAME) cnames[name] = cname if cname in cnames: - raise DNSError, 'CNAME loop' + raise DNSError('CNAME loop') result = self.dns(cname, qtype, cnames=cnames) return result @@ -103,16 +103,16 @@ def DNSLookup_pydns(name, qtype, tcpfallback=True, timeout=30): # if resp.header['tc'] == True: if not tcpfallback: - raise DNS.DNSError, 'DNS: Truncated UDP Reply, SPF records should fit in a UDP packet' + raise DNS.DNSError('DNS: Truncated UDP Reply, SPF records should fit in a UDP packet') try: req = DNS.DnsRequest(name, qtype=qtype, protocol='tcp', timeout=timeout) resp = req.req() - except DNS.DNSError, x: - raise DNS.DNSError, 'TCP Fallback error: ' + str(x) + except DNS.DNSError as x: + raise DNS.DNSError('TCP Fallback error: ' + str(x)) return [((a['name'], a['typename']), a['data']) for a in resp.answers] - except IOError, x: - raise DNS.DNSError, 'DNS: ' + str(x) + except IOError as x: + raise DNS.DNSError('DNS: ' + str(x)) def DNSLookup_dnspython(name,qtype,tcpfallback=True,timeout=30): retVal = [] @@ -164,5 +164,5 @@ if __name__ == '__main__': import sys s = Session() for n,t in zip(*[iter(sys.argv[1:])]*2): - print n,t - print s.dns(n,t) + print(n,t) + print(s.dns(n,t)) diff --git a/setup.py b/setup.py index 18c2ec9..c7990d7 100644 --- a/setup.py +++ b/setup.py @@ -1,4 +1,4 @@ -#! /usr/bin/python +#! /usr/bin/python3 # dkimpy-milter: A DKIM signing/verification Milter application # Author: Scott Kitterman # Copyright 2018 Scott Kitterman @@ -24,9 +24,9 @@ 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', 'dnspython'] + kw['install_requires'] = ['dkimpy>=0.7', 'pymilter', 'authres>=1.1.0', 'PyNaCl', 'dnspython'] except ImportError: # If PyDNS is not installed, prefer dnspython - kw['install_requires'] = ['dkimpy>=0.7', 'pymilter', 'authres>=1.1.0', 'PyNaCl', 'ipaddress', 'PyDNS'] + kw['install_requires'] = ['dkimpy>=0.7', 'pymilter', 'authres>=1.1.0', 'PyNaCl', 'PyDNS'] setup( name='dkimpy-milter', @@ -43,7 +43,7 @@ setup( 'License :: OSI Approved :: GNU General Public License (GPL)', 'Natural Language :: English', 'Operating System :: POSIX', - 'Programming Language :: Python :: 2 :: Only', + 'Programming Language :: Python :: 3 :: Only', 'Topic :: Communications :: Email :: Mail Transport Agents', 'Topic :: Communications :: Email :: Filters', 'Topic :: Security', diff --git a/tests/dkimpy-milter b/tests/dkimpy-milter index 39b64d5..9ee02e9 100755 --- a/tests/dkimpy-milter +++ b/tests/dkimpy-milter @@ -1,2 +1,2 @@ #!/bin/sh -python2 -m dkimpy_milter "$@" +python3 -m dkimpy_milter "$@" From 9d5316ca0e6c573db6a0d3aeaad4e23815e8e301 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 15:41:35 -0500 Subject: [PATCH 27/32] Handle defaults for Socket differently We want to be able to select the default for Socket differently in the future. This change augments the API for dkimpy_milter.util.own_socketfile() by adding an optional sockname argument. This is a backward-compatible change. If we aren't committed to API stability for this function, we could make a more invasive change that would probably be a more reasonable API going forward, but this is probably good enough. --- dkimpy_milter/__init__.py | 4 +++- dkimpy_milter/config.py | 2 +- dkimpy_milter/util.py | 7 +++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/dkimpy_milter/__init__.py b/dkimpy_milter/__init__.py index bcdf2a7..3f9eef9 100644 --- a/dkimpy_milter/__init__.py +++ b/dkimpy_milter/__init__.py @@ -354,7 +354,9 @@ def main(): Milter.set_flags(Milter.CHGHDRS + Milter.ADDHDRS) miltername = 'dkimpy-filter' socketname = milterconfig.get('Socket') - own_socketfile(milterconfig) + if socketname is None: + socketname = 'local:/var/run/dkimpy-milter/dkimpy-milter.sock' + own_socketfile(milterconfig, socketname) drop_privileges(milterconfig) sys.stdout.flush() Milter.runmilter(miltername, socketname, 240) diff --git a/dkimpy_milter/config.py b/dkimpy_milter/config.py index bf6551a..c59ce55 100644 --- a/dkimpy_milter/config.py +++ b/dkimpy_milter/config.py @@ -39,7 +39,7 @@ defaultConfigData = { 'SyslogFacility': 'mail', 'UMask': 0o07, 'Mode': 'sv', - 'Socket': 'local:/var/run/dkimpy-milter/dkimpy-milter.sock', + 'Socket': None, 'PidFile': '/var/run/dkimpy-milter/dkimpy-milter.pid', 'UserID': 'dkimpy-milter', 'Canonicalization': 'relaxed/simple', diff --git a/dkimpy_milter/util.py b/dkimpy_milter/util.py index 17857d6..1d5788d 100644 --- a/dkimpy_milter/util.py +++ b/dkimpy_milter/util.py @@ -146,12 +146,15 @@ def write_pid(milterconfig): return pid -def own_socketfile(milterconfig): +def own_socketfile(milterconfig, sockname=None): """If socket is Unix socket, chown to UserID before dropping privileges""" import os user, group = user_group(milterconfig.get('UserID')) offset = None - sockname = milterconfig.get('Socket') + if sockname is None: + sockname = milterconfig.get('Socket') + if sockname is None: + return if sockname[:1] == '/': offset = 0 elif sockname[:6] == "local:": From 25fdd3b81c2d867858f01a9d34c8741f698942a5 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 16:22:50 -0500 Subject: [PATCH 28/32] Do not create PidFile by default By default, avoid creating a PIDFile. PIDFiles are racy and potentially dangerous. Modern system supervision systems don't need them, because they manage the process groups directly. If the configuration file doesn't specify a PidFile, dkimpy-milter shouldn't try to create one. --- dkimpy_milter/config.py | 2 +- dkimpy_milter/util.py | 19 +++++++++++-------- man/dkimpy-milter.conf.5 | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/dkimpy_milter/config.py b/dkimpy_milter/config.py index c59ce55..3e2c736 100644 --- a/dkimpy_milter/config.py +++ b/dkimpy_milter/config.py @@ -40,7 +40,7 @@ defaultConfigData = { 'UMask': 0o07, 'Mode': 'sv', 'Socket': None, - 'PidFile': '/var/run/dkimpy-milter/dkimpy-milter.pid', + 'PidFile': None, 'UserID': 'dkimpy-milter', 'Canonicalization': 'relaxed/simple', 'InternalHosts': '127.0.0.1', diff --git a/dkimpy_milter/util.py b/dkimpy_milter/util.py index 1d5788d..56a57a4 100644 --- a/dkimpy_milter/util.py +++ b/dkimpy_milter/util.py @@ -115,34 +115,37 @@ def write_pid(milterconfig): """Write PID in pidfile. Will not overwrite an existing file.""" import os import syslog - if not os.path.isfile(milterconfig.get('PidFile')): + pidfile = milterconfig.get('PidFile') + if pidfile is None: + return + if not os.path.isfile(pidfile): pid = str(os.getpid()) try: - f = open(milterconfig.get('PidFile'), 'w') + f = open(pidfile, 'w') except IOError as e: if str(e)[:35] == '[Errno 2] No such file or directory': - piddir = milterconfig.get('PidFile').rsplit('/', 1)[0] + piddir = pidfile.rsplit('/', 1)[0] os.mkdir(piddir) user, group = user_group(milterconfig.get('UserID')) os.chown(piddir, user, group) - f = open(milterconfig.get('PidFile'), 'w') + f = open(pidfile, 'w') if milterconfig.get('Syslog'): syslog.syslog('PID dir created: {0}'.format(piddir)) else: if milterconfig.get('Syslog'): syslog.syslog('Unable to write pidfle {0}. IOError: {1}' - .format(milterconfig.get('PidFile'), e)) + .format(pidfile, e)) raise f.write(pid) f.close() user, group = user_group(milterconfig.get('UserID')) - os.chown(milterconfig.get('PidFile'), user, group) + os.chown(pidfile, user, group) else: if milterconfig.get('Syslog'): syslog.syslog('Unable to write pidfle {0}. File exists.' - .format(milterconfig.get('PidFile'))) + .format(pidfile)) raise RuntimeError('Unable to write pidfle {0}. File exists.' - .format(milterconfig.get('PidFile'))) + .format(pidfile)) return pid diff --git a/man/dkimpy-milter.conf.5 b/man/dkimpy-milter.conf.5 index a7e5d31..bb2a019 100644 --- a/man/dkimpy-milter.conf.5 +++ b/man/dkimpy-milter.conf.5 @@ -338,7 +338,7 @@ will be checked. [PeerList NOT IMPLEMENTED - included for reference only] .TP .I PidFile (string) Specifies the path to a file that should be created at process start -containing the process ID. +containing the process ID. If not specified, no such file will be created. .TP .I Selector (string) From ea09bab1a8fb9eeed3429e9a8411ee42f9c423f3 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Wed, 20 Feb 2019 19:01:30 -0500 Subject: [PATCH 29/32] Convert __init__.py to python3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main work here is about bytes vs. strings. This work was confusing for several reasons: * pymilter thinks that headers are all strings, but body is bytes * dkimpy wants to deal with bytes objects generally (though it accepts a string object as an ed25519 secret key for some reason, despite requiring bytes as an RSA secret key) * authres.AuthenticationResultsHeader object converts easily to a string, but has no direct bytes conversion. meanwhile, it wants its arguments as strings, but will accept them if they are bytes and convert them with something like str(), which leaves weird cruft like "header.a=b'ed25519-sha256'" * dkimpy_milter/utils.py contains fold() which expects bytes * self.fp needs to accumulate the on-the-wire version of the message as a whole (so it needs to be bytes). That means converting the headers. Header names and values are US-ASCII, per §2.2 of RFC 5322, so they should be convertible cleanly, but we still have to convert them explicitly so that python knows the right thing to do. At any rate, tests/runtests all passes with these changes, and the output for both Authentication-Results: and DKIM-Signature headers looks the same. --- dkimpy_milter/__init__.py | 61 +++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/dkimpy_milter/__init__.py b/dkimpy_milter/__init__.py index c4c407b..bcdf2a7 100644 --- a/dkimpy_milter/__init__.py +++ b/dkimpy_milter/__init__.py @@ -28,8 +28,9 @@ import dkim import authres import os import tempfile -import StringIO +import io import re +import codecs from Milter.utils import parse_addr, parseaddr import dkimpy_milter.config as config from dkimpy_milter.util import drop_privileges @@ -110,7 +111,7 @@ class dkimMilter(Milter.Base): def envfrom(self, f, *str): if milterconfig.get('Syslog') and milterconfig.get('debugLevel') >= 2: syslog.syslog("mail from: {0} {1}".format(f, str)) - self.fp = StringIO.StringIO() + self.fp = io.BytesIO() self.mailfrom = f t = parse_addr(f) if len(t) == 2: @@ -142,13 +143,13 @@ class dkimMilter(Milter.Base): elif lname == 'authentication-results': self.arheaders.append(val) if self.fp: - self.fp.write("%s: %s\n" % (name, val)) + self.fp.write(b"%s: %s\n" % (codecs.encode(name, 'ascii'), codecs.encode(val, 'ascii'))) return Milter.CONTINUE @Milter.noreply def eoh(self): if self.fp: - self.fp.write("\n") # terminate headers + self.fp.write(b"\n") # terminate headers self.bodysize = 0 return Milter.CONTINUE @@ -195,20 +196,20 @@ class dkimMilter(Milter.Base): h = authres.AuthenticationResultsHeader(authserv_id= self.AuthservID, results=self.arresults) - h = fold(str(h)) + h = fold(codecs.encode(str(h), 'ascii')) if (milterconfig.get('Syslog') and milterconfig.get('debugLevel') >= 2): - syslog.syslog(str(h)) - name, val = str(h).split(': ', 1) + syslog.syslog(codecs.decode(h, 'ascii')) + name, val = codecs.decode(h, 'ascii').split(': ', 1) self.addheader(name, val, 0) return Milter.CONTINUE def sign_dkim(self, txt): - canon = milterconfig.get('Canonicalization') + canon = codecs.encode(milterconfig.get('Canonicalization'), 'ascii') canonicalize = [] - if len(canon.split('/')) == 2: - canonicalize.append(canon.split('/')[0]) - canonicalize.append(canon.split('/')[1]) + if len(canon.split(b'/')) == 2: + canonicalize.append(canon.split(b'/')[0]) + canonicalize.append(canon.split(b'/')[1]) else: canonicalize.append(canon) canonicalize.append(canon) @@ -218,11 +219,12 @@ class dkimMilter(Milter.Base): try: if privateRSA: d = dkim.DKIM(txt) - h = d.sign(milterconfig.get('Selector'), self.fdomain, - privateRSA, canonicalize=(canonicalize[0], - canonicalize[1])) - name, val = h.split(': ', 1) - self.addheader(name, val.strip().replace('\r\n', '\n'), 0) + h = d.sign(codecs.encode(milterconfig.get('Selector'), 'ascii'), codecs.encode(self.fdomain, 'ascii'), + codecs.encode(privateRSA, 'ascii'), + canonicalize=(canonicalize[0], + canonicalize[1])) + name, val = h.split(b': ', 1) + self.addheader(codecs.decode(name, 'ascii'), codecs.decode(val, 'ascii').strip().replace('\r\n', '\n'), 0) if (milterconfig.get('Syslog') and (milterconfig.get('SyslogSuccess') or milterconfig.get('debugLevel') >= 1)): @@ -233,12 +235,12 @@ class dkimMilter(Milter.Base): d.domain.lower())) if privateEd25519: d = dkim.DKIM(txt) - h = d.sign(milterconfig.get('SelectorEd25519'), self.fdomain, + h = d.sign(codecs.encode(milterconfig.get('SelectorEd25519'), 'ascii'), codecs.encode(self.fdomain, 'ascii'), privateEd25519, canonicalize=(canonicalize[0], canonicalize[1]), - signature_algorithm='ed25519-sha256') - name, val = h.split(': ', 1) - self.addheader(name, val.strip().replace('\r\n', '\n'), 0) + signature_algorithm=b'ed25519-sha256') + name, val = h.split(b': ', 1) + self.addheader(codecs.decode(name, 'ascii'), codecs.decode(val, 'ascii').strip().replace('\r\n', '\n'), 0) if (milterconfig.get('Syslog') and (milterconfig.get('SyslogSuccess') or milterconfig.get('debugLevel') >= 1)): @@ -266,20 +268,17 @@ class dkimMilter(Milter.Base): res = d.verify(idx=y, dnsfunc=lambda _x: dnsoverride) else: res = d.verify(idx=y) + algo = codecs.decode(d.signature_fields.get(b'a'), 'ascii') if res: - if d.signature_fields.get(b'a') == 'ed25519-sha256': + if algo == 'ed25519-sha256': self.dkim_comment = ('Good {0} signature' - .format(d.signature_fields - .get(b'a'))) + .format(algo)) else: self.dkim_comment = ('Good {0} bit {1} signature' - .format(d.keysize, - d.signature_fields - .get(b'a'))) + .format(d.keysize, algo)) else: self.dkim_comment = ('Bad {0} bit {1} signature.' - .format(d.keysize, - d.signature_fields.get(b'a'))) + .format(d.keysize, algo)) except dkim.DKIMException as x: self.dkim_comment = str(x) if milterconfig.get('Syslog'): @@ -288,9 +287,9 @@ class dkimMilter(Milter.Base): self.dkim_comment = str(x) if milterconfig.get('Syslog'): syslog.syslog("check_dkim: {0}".format(x)) - self.header_i = d.signature_fields.get(b'i') - self.header_d = d.signature_fields.get(b'd') - self.header_a = d.signature_fields.get(b'a') + self.header_i = codecs.decode(d.signature_fields.get(b'i'), 'ascii') + self.header_d = codecs.decode(d.signature_fields.get(b'd'), 'ascii') + self.header_a = codecs.decode(d.signature_fields.get(b'a'), 'ascii') if res: if (milterconfig.get('Syslog') and (milterconfig.get('SyslogSuccess') or From 7092874729417f7b8032520af21f97375d570d3a Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 17:12:59 -0500 Subject: [PATCH 30/32] Enable sd_listen_fds(3)-style socket-activation support I've added straightforward systemd unit files in system/socket-activation/ that make use of this approach, and a README.md in the same location that describes the tradeoffs. --- dkimpy_milter/__init__.py | 11 ++++++- system/socket-activation/README.md | 32 +++++++++++++++++++ .../socket-activation/dkimpy-milter.service | 11 +++++++ system/socket-activation/dkimpy-milter.socket | 12 +++++++ 4 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 system/socket-activation/README.md create mode 100644 system/socket-activation/dkimpy-milter.service create mode 100644 system/socket-activation/dkimpy-milter.socket diff --git a/dkimpy_milter/__init__.py b/dkimpy_milter/__init__.py index 3f9eef9..4c64891 100644 --- a/dkimpy_milter/__init__.py +++ b/dkimpy_milter/__init__.py @@ -355,7 +355,16 @@ def main(): miltername = 'dkimpy-filter' socketname = milterconfig.get('Socket') if socketname is None: - socketname = 'local:/var/run/dkimpy-milter/dkimpy-milter.sock' + if int(os.environ.get('LISTEN_PID', '0')) == os.getpid(): + lfds = os.environ.get('LISTEN_FDS') + if lfds is not None: + if lfds != '1': + syslog.syslog('LISTEN_FDS is set to "{0}", but we only know how to deal with "1", ignoring it'. + format(lfds)) + else: + socketname = 'fd:3' + if socketname is None: + socketname = 'local:/var/run/dkimpy-milter/dkimpy-milter.sock' own_socketfile(milterconfig, socketname) drop_privileges(milterconfig) sys.stdout.flush() diff --git a/system/socket-activation/README.md b/system/socket-activation/README.md new file mode 100644 index 0000000..b4410ac --- /dev/null +++ b/system/socket-activation/README.md @@ -0,0 +1,32 @@ +This directory contains example systemd unit files for running a +supervised, socket-activated instance of dkimpy-milter. + +There are several advantages of using socket activation: + +- dkimpy-milter never runs with elevated privileges, they are dropped + before any dkimpy-milter code is executed. + +- The socket is opened before dkimpy-milter runs. This means that + clients can connect() to the socket immediately. So even if there + is a delay in dkimpy-milter startup, or in libmilter itself, the + connection will not fail. + +- You can set the privileges of a listening Unix-domain socket by an + override of ListenGroup= in dkimpy-milter.socket (see + systemd.unit(5) for how to override). This lets you control who has + access to the daemon with finer granularity than is available with + dkimpy-milter on its own. + +- dkimpy-milter will not consume system resources if it is not used. + +- A fully-supervised dkimpy-milter needs no PIDFile, UMask, UserID, or + Socket configuation. This eliminates common race conditions and + startup failures, and simplifies the resulting configuration file. + +There is one downside to using socket activation: + +- it will only work on systems where libmilter can support connection + strings like "fd:3". This has been supported on Debian and derived + systems since sendmail 8.14.4-6 (before Debian Jessie, in early + 2014), see for example: + https://sources.debian.org/src/sendmail/8.15.2-8/debian/patches/socket_activation.patch/ diff --git a/system/socket-activation/dkimpy-milter.service b/system/socket-activation/dkimpy-milter.service new file mode 100644 index 0000000..2116bb7 --- /dev/null +++ b/system/socket-activation/dkimpy-milter.service @@ -0,0 +1,11 @@ +[Unit] +Description=DKIMpy Milter +Documentation=man:dkimpy-milter(8) man:dkimpy-milter.conf(5) +Requires=dkimpy-milter.socket + +[Service] +ExecStart=/usr/bin/dkimpy-milter /etc/dkimpy-milter.conf +User=dkimpy-milter + +[Install] +Also=dkimpy-milter.socket diff --git a/system/socket-activation/dkimpy-milter.socket b/system/socket-activation/dkimpy-milter.socket new file mode 100644 index 0000000..d4338a3 --- /dev/null +++ b/system/socket-activation/dkimpy-milter.socket @@ -0,0 +1,12 @@ +[Unit] +Description=DKIMpy Milter socket +Documentation=man:dkimpy-milter(8) man:dkimpy-milter.conf(5) + +[Socket] +ListenStream=/run/dkimpy-milter/dkimpy-milter.sock +SocketMode=0660 +# override SocketGroup to grant access to members of another system group: +SocketGroup=dkimpy-milter + +[Install] +WantedBy=sockets.target From 4b0c39b0c7460d15ae0b9b24127d9048a0de11b4 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Sun, 24 Feb 2019 06:57:47 -0500 Subject: [PATCH 31/32] Start changes for python3 update --- CHANGES | 5 ++++- README | 2 +- setup.py | 8 ++++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGES b/CHANGES index 4c11ec8..6fc2562 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,8 @@ +1.1.0 UNRELEASED + - Port to python3 + 1.0.1 2019-02-11 - * Reorder milter start and dropping privileges so permissions on Unix socket + - Reorder milter start and dropping privileges so permissions on Unix socket are correct (LP: 1797720) - Make domain checks case insensitive for determining if signing should be done (LP: #1815311) diff --git a/README b/README index f7de656..535f178 100644 --- a/README +++ b/README @@ -17,7 +17,7 @@ to be installed when installing using setup.py, the following incantation is required because setuptools developers decided not being able to do this by default is a feature: -python setup.py install --single-version-externally-managed --record=/dev/null +python3 setup.py install --single-version-externally-managed --record=/dev/null For users of Debian Stable (Debian 9, Codename Squeeze), all dependencies are available in either the main or backports repositories: diff --git a/setup.py b/setup.py index 41a11a5..f3c9f26 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,7 @@ -#! /usr/bin/python +#! /usr/bin/python3 # dkimpy-milter: A DKIM signing/verification Milter application # Author: Scott Kitterman -# Copyright 2018 Scott Kitterman +# Copyright 2018,2019 Scott Kitterman """ This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2 of the License, or @@ -30,7 +30,7 @@ except ImportError: # If PyDNS is not installed, prefer dnspython setup( name='dkimpy-milter', - version='1.0.1', + version='1.1.0', author='Scott Kitterman', author_email='scott@kitterman.com', url='https://launchpad.net/dkimpy-milter', @@ -43,7 +43,7 @@ setup( 'License :: OSI Approved :: GNU General Public License (GPL)', 'Natural Language :: English', 'Operating System :: POSIX', - 'Programming Language :: Python :: 2 :: Only', + 'Programming Language :: Python :: 3', 'Topic :: Communications :: Email :: Mail Transport Agents', 'Topic :: Communications :: Email :: Filters', 'Topic :: Security', From 23d91b2b50021ae7001febf869a170e70acc9966 Mon Sep 17 00:00:00 2001 From: Scott Kitterman Date: Sun, 24 Feb 2019 07:19:18 -0500 Subject: [PATCH 32/32] Update CHANGES for merge of dkg/test-suite --- CHANGES | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES b/CHANGES index 6fc2562..8899b06 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,10 @@ 1.1.0 UNRELEASED - Port to python3 + - Add test suite using opendkim miltertest + - When Socket is absolute path, do not strip leading / + - Handle unix: socket prefix the same as local: + - Set up correct AuthservID defaults + - config: Reassemble strings sensibly 1.0.1 2019-02-11 - Reorder milter start and dropping privileges so permissions on Unix socket