diff --git a/.gitignore b/.gitignore index 9e59230..8bd7e59 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ dist dkimpy_milter.egg-info *.pyc +*~ diff --git a/CHANGES b/CHANGES index 06cd982..8899b06 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,20 @@ +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 + 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 + 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 - Deleted reference to obsolete syslog target in unit file diff --git a/README b/README index 2d7dca5..535f178 100644 --- a/README +++ b/README @@ -1,16 +1,23 @@ -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 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: @@ -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 @@ -77,7 +127,59 @@ 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 -For Postfix: +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. + +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 @@ -127,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 have finished development. The specification is -complete. Version 1.0.0 and later support RFC 8463. +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. diff --git a/TODO b/TODO index 58b7838..0314aaa 100644 --- a/TODO +++ b/TODO @@ -39,16 +39,20 @@ MacroListVerify implemented verified SyslogSuccess implemented verified 1.0.0 -No additional features planned +No additional features + +1.0.1 +Bug fix only, improved documentation 1.1.0 +Port to Python 3 +Subdomain support KeyTable KeytableEd25519 SigningTable SigningTableEd25519 -Plannedataset type support (if needed): -db:/.db +Planned dataset type support (if needed): mdb: Considered for near-term feature release @@ -56,7 +60,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 61bb1dd..4c64891 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. @@ -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 @@ -39,7 +40,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]+') @@ -61,7 +62,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: @@ -108,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: @@ -131,7 +134,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 @@ -140,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 @@ -193,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) @@ -216,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)): @@ -228,15 +232,15 @@ 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, + 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)): @@ -244,7 +248,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)) @@ -258,21 +262,23 @@ 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) + 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'): @@ -281,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 @@ -292,8 +298,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 +309,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: @@ -348,13 +354,24 @@ def main(): Milter.set_flags(Milter.CHGHDRS + Milter.ADDHDRS) miltername = 'dkimpy-filter' socketname = milterconfig.get('Socket') + if socketname is None: + 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() + 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() diff --git a/dkimpy_milter/__main__.py b/dkimpy_milter/__main__.py new file mode 100644 index 0000000..f95075f --- /dev/null +++ b/dkimpy_milter/__main__.py @@ -0,0 +1,6 @@ +#!/usr/bin/python3 + +from dkimpy_milter import main + +if __name__ == "__main__": + main() diff --git a/dkimpy_milter/config.py b/dkimpy_milter/config.py index 6086e96..3e2c736 100644 --- a/dkimpy_milter/config.py +++ b/dkimpy_milter/config.py @@ -31,16 +31,16 @@ 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', + 'Socket': None, + 'PidFile': None, 'UserID': 'dkimpy-milter', 'Canonicalization': 'relaxed/simple', 'InternalHosts': '127.0.0.1', @@ -48,6 +48,7 @@ defaultConfigData = { 'DiagnosticDirectory': '', 'MacroList': '', 'MacroListVerify': '', + 'DNSOverride': None, 'debugLevel': 0 # Undocumented config item for developer use } @@ -84,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): @@ -109,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 @@ -159,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 @@ -224,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]) @@ -294,19 +295,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))) @@ -346,13 +335,14 @@ def _readConfigFile(path, configData=None, configGlobal={}): 'DiagnosticDirectory': 'str', 'MacroList': 'dataset', 'MacroListVerify': 'dataset', + 'DNSOverride': 'str', 'debugLevel': 'int' } # 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) @@ -400,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': @@ -411,7 +404,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 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/dkimpy_milter/util.py b/dkimpy_milter/util.py index 67dd596..bcdd11b 100644 --- a/dkimpy_milter/util.py +++ b/dkimpy_milter/util.py @@ -115,45 +115,59 @@ 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 -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')) - if milterconfig.get('Socket')[:1] == '/': - os.chown(milterconfig.get('Socket')[1:], user, group) - if milterconfig.get('Socket')[:6] == "local:": - os.chown(milterconfig.get('Socket')[6:], user, group) + offset = None + if sockname is None: + sockname = milterconfig.get('Socket') + if sockname is None: + return + if sockname[:1] == '/': + offset = 0 + elif sockname[:6] == "local:": + offset = 6 + elif sockname[:5] == "unix:": + offset = 5 + + if offset is not None: + if os.path.exists(sockname[offset:]): + os.chown(sockname[offset:], user, group) def read_keyfile(milterconfig, keytype): diff --git a/man/dkimpy-milter.conf.5 b/man/dkimpy-milter.conf.5 index fd05a93..f420e66 100644 --- a/man/dkimpy-milter.conf.5 +++ b/man/dkimpy-milter.conf.5 @@ -324,6 +324,13 @@ be set: (a) Domain, KeyFile, Selector, no KeyTable, no SigningTable; (b) KeyTable, SigningTable, no Domain, no KeyFile, no Selector; +.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 @@ -344,7 +351,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) diff --git a/setup.py b/setup.py index d5180af..b727841 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 @@ -23,14 +23,14 @@ 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'] + import dns + 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', 'dnspython'] + kw['install_requires'] = ['dkimpy>=0.7', 'pymilter', 'authres>=1.1.0', 'PyNaCl', 'PyDNS'] setup( name='dkimpy-milter', - version='1.0.0', + 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 :: Only', 'Topic :: Communications :: Email :: Mail Transport Agents', 'Topic :: Communications :: Email :: Filters', 'Topic :: Security', 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 diff --git a/tests/00_minimal.miltertest b/tests/00_minimal.miltertest new file mode 100644 index 0000000..fbe0849 --- /dev/null +++ b/tests/00_minimal.miltertest @@ -0,0 +1,12 @@ +-- -*- lua -*- +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 diff --git a/tests/01_connect.miltertest b/tests/01_connect.miltertest new file mode 100755 index 0000000..2f43eff --- /dev/null +++ b/tests/01_connect.miltertest @@ -0,0 +1,40 @@ +-- -*- lua -*- +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 "..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 "..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 "..keytype.." "..func) +-- else +-- print("could not change body "..keytype.." "..func) +-- end + + 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 new file mode 100644 index 0000000..cb5e7ff --- /dev/null +++ b/tests/02_sign_message.miltertest @@ -0,0 +1,100 @@ +-- -*- lua -*- + +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 + +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 diff --git a/tests/dkimpy-milter b/tests/dkimpy-milter new file mode 100755 index 0000000..9ee02e9 --- /dev/null +++ b/tests/dkimpy-milter @@ -0,0 +1,2 @@ +#!/bin/sh +python3 -m dkimpy_milter "$@" diff --git a/tests/runtests b/tests/runtests new file mode 100755 index 0000000..7878f17 --- /dev/null +++ b/tests/runtests @@ -0,0 +1,84 @@ +#!/bin/bash + +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" + +for keytype in "${KEY_TYPES[@]}"; do + dknewkey --ktype "$keytype" "testkey.$keytype" + if [ "$keytype" = ed25519 ]; then + keyfile=KeyFileEd25519 + selector=SelectorEd25519 + else + keyfile=KeyFile + selector=Selector + fi + cat > "$keytype.signing.conf" < "$keytype.verify.conf" < %s:\n" "$errdata" + cat "$errdata" + printf -- "-> end %s\n" "$errdata" + fi + done + done + rm -rf "$WORKDIR" +} + +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 milters are all 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