Merge dkimpy-milter:dkg/socket-activation into dkimpy-milter:master

Proposed by dkg
Status: Superseded
Proposed branch: dkimpy-milter:dkg/socket-activation
Merge into: dkimpy-milter:master
Prerequisite: dkimpy-milter:dkg/python3
Diff against target: 189 lines (+86/-14) (has conflicts)
7 files modified
dkimpy_milter/ (+12/-1)
dkimpy_milter/ (+2/-2)
dkimpy_milter/ (+16/-10)
man/dkimpy-milter.conf.5 (+1/-1)
system/socket-activation/ (+32/-0)
system/socket-activation/dkimpy-milter.service (+11/-0)
system/socket-activation/dkimpy-milter.socket (+12/-0)
Conflict in README
Conflict in TODO
Conflict in man/dkimpy-milter.conf.5
Reviewer Review Type Date Requested Status
Scott Kitterman Pending
Review via email:

This proposal has been superseded by a proposal from 2019-02-25.

Commit message

This series makes it possible to do socket activation. It is written on top of the earlier patch series to make it easier to consider merging in sequence (it is also quite close to several bugfixes)

To post a comment you must log in.

Unmerged commits

7092874... by dkg

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 in the same location that describes the tradeoffs.

25fdd3b... by dkg

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.

9d5316c... by dkg

Handle defaults for Socket differently

We want to be able to select the default for Socket differently in the

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.

ea09bab... by dkg

Convert to python3

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/ 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.

391b535... by dkg

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, 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

ad8f396... by dkg

Expand test suite to cover RSA as well as ed25519

479820a... by dkg

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.

7bfb87f... by dkg

Set up, 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.

5c1d5d6... by dkg

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.

ae31730... by dkg

check for actions claimed by the filter

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/dkimpy_milter/ b/dkimpy_milter/
index bcdf2a7..4c64891 100644
--- a/dkimpy_milter/
+++ b/dkimpy_milter/
@@ -354,7 +354,18 @@ def main():
354 Milter.set_flags(Milter.CHGHDRS + Milter.ADDHDRS)354 Milter.set_flags(Milter.CHGHDRS + Milter.ADDHDRS)
355 miltername = 'dkimpy-filter'355 miltername = 'dkimpy-filter'
356 socketname = milterconfig.get('Socket')356 socketname = milterconfig.get('Socket')
357 own_socketfile(milterconfig)357 if socketname is None:
358 if int(os.environ.get('LISTEN_PID', '0')) == os.getpid():
359 lfds = os.environ.get('LISTEN_FDS')
360 if lfds is not None:
361 if lfds != '1':
362 syslog.syslog('LISTEN_FDS is set to "{0}", but we only know how to deal with "1", ignoring it'.
363 format(lfds))
364 else:
365 socketname = 'fd:3'
366 if socketname is None:
367 socketname = 'local:/var/run/dkimpy-milter/dkimpy-milter.sock'
368 own_socketfile(milterconfig, socketname)
358 drop_privileges(milterconfig)369 drop_privileges(milterconfig)
359 sys.stdout.flush()370 sys.stdout.flush()
360 Milter.runmilter(miltername, socketname, 240)371 Milter.runmilter(miltername, socketname, 240)
diff --git a/dkimpy_milter/ b/dkimpy_milter/
index bf6551a..3e2c736 100644
--- a/dkimpy_milter/
+++ b/dkimpy_milter/
@@ -39,8 +39,8 @@ defaultConfigData = {
39 'SyslogFacility': 'mail',39 'SyslogFacility': 'mail',
40 'UMask': 0o07,40 'UMask': 0o07,
41 'Mode': 'sv',41 'Mode': 'sv',
42 'Socket': 'local:/var/run/dkimpy-milter/dkimpy-milter.sock',42 'Socket': None,
43 'PidFile': '/var/run/dkimpy-milter/',43 'PidFile': None,
44 'UserID': 'dkimpy-milter',44 'UserID': 'dkimpy-milter',
45 'Canonicalization': 'relaxed/simple',45 'Canonicalization': 'relaxed/simple',
46 'InternalHosts': '',46 'InternalHosts': '',
diff --git a/dkimpy_milter/ b/dkimpy_milter/
index 53d4c0d..bcdd11b 100644
--- a/dkimpy_milter/
+++ b/dkimpy_milter/
@@ -115,43 +115,49 @@ def write_pid(milterconfig):
115 """Write PID in pidfile. Will not overwrite an existing file."""115 """Write PID in pidfile. Will not overwrite an existing file."""
116 import os116 import os
117 import syslog117 import syslog
118 if not os.path.isfile(milterconfig.get('PidFile')):118 pidfile = milterconfig.get('PidFile')
119 if pidfile is None:
120 return
121 if not os.path.isfile(pidfile):
119 pid = str(os.getpid())122 pid = str(os.getpid())
120 try:123 try:
121 f = open(milterconfig.get('PidFile'), 'w')124 f = open(pidfile, 'w')
122 except IOError as e:125 except IOError as e:
123 if str(e)[:35] == '[Errno 2] No such file or directory':126 if str(e)[:35] == '[Errno 2] No such file or directory':
124 piddir = milterconfig.get('PidFile').rsplit('/', 1)[0]127 piddir = pidfile.rsplit('/', 1)[0]
125 os.mkdir(piddir)128 os.mkdir(piddir)
126 user, group = user_group(milterconfig.get('UserID'))129 user, group = user_group(milterconfig.get('UserID'))
127 os.chown(piddir, user, group)130 os.chown(piddir, user, group)
128 f = open(milterconfig.get('PidFile'), 'w')131 f = open(pidfile, 'w')
129 if milterconfig.get('Syslog'):132 if milterconfig.get('Syslog'):
130 syslog.syslog('PID dir created: {0}'.format(piddir))133 syslog.syslog('PID dir created: {0}'.format(piddir))
131 else:134 else:
132 if milterconfig.get('Syslog'):135 if milterconfig.get('Syslog'):
133 syslog.syslog('Unable to write pidfle {0}. IOError: {1}'136 syslog.syslog('Unable to write pidfle {0}. IOError: {1}'
134 .format(milterconfig.get('PidFile'), e))137 .format(pidfile, e))
135 raise138 raise
136 f.write(pid)139 f.write(pid)
137 f.close()140 f.close()
138 user, group = user_group(milterconfig.get('UserID'))141 user, group = user_group(milterconfig.get('UserID'))
139 os.chown(milterconfig.get('PidFile'), user, group)142 os.chown(pidfile, user, group)
140 else:143 else:
141 if milterconfig.get('Syslog'):144 if milterconfig.get('Syslog'):
142 syslog.syslog('Unable to write pidfle {0}. File exists.'145 syslog.syslog('Unable to write pidfle {0}. File exists.'
143 .format(milterconfig.get('PidFile')))146 .format(pidfile))
144 raise RuntimeError('Unable to write pidfle {0}. File exists.'147 raise RuntimeError('Unable to write pidfle {0}. File exists.'
145 .format(milterconfig.get('PidFile')))148 .format(pidfile))
146 return pid149 return pid
149def own_socketfile(milterconfig):152def own_socketfile(milterconfig, sockname=None):
150 """If socket is Unix socket, chown to UserID before dropping privileges"""153 """If socket is Unix socket, chown to UserID before dropping privileges"""
151 import os154 import os
152 user, group = user_group(milterconfig.get('UserID'))155 user, group = user_group(milterconfig.get('UserID'))
153 offset = None156 offset = None
154 sockname = milterconfig.get('Socket')157 if sockname is None:
158 sockname = milterconfig.get('Socket')
159 if sockname is None:
160 return
155 if sockname[:1] == '/':161 if sockname[:1] == '/':
156 offset = 0162 offset = 0
157 elif sockname[:6] == "local:":163 elif sockname[:6] == "local:":
diff --git a/man/dkimpy-milter.conf.5 b/man/dkimpy-milter.conf.5
index d0662c4..ea07593 100644
--- a/man/dkimpy-milter.conf.5
+++ b/man/dkimpy-milter.conf.5
@@ -355,7 +355,7 @@ will be checked. [PeerList NOT IMPLEMENTED - included for reference only]
356.I PidFile (string)356.I PidFile (string)
357Specifies the path to a file that should be created at process start357Specifies the path to a file that should be created at process start
358containing the process ID.358containing the process ID. If not specified, no such file will be created.
361.I Selector (string)361.I Selector (string)
diff --git a/system/socket-activation/ b/system/socket-activation/
362new file mode 100644362new file mode 100644
index 0000000..b4410ac
--- /dev/null
+++ b/system/socket-activation/
@@ -0,0 +1,32 @@
1This directory contains example systemd unit files for running a
2supervised, socket-activated instance of dkimpy-milter.
4There are several advantages of using socket activation:
6- dkimpy-milter never runs with elevated privileges, they are dropped
7 before any dkimpy-milter code is executed.
9- The socket is opened before dkimpy-milter runs. This means that
10 clients can connect() to the socket immediately. So even if there
11 is a delay in dkimpy-milter startup, or in libmilter itself, the
12 connection will not fail.
14- You can set the privileges of a listening Unix-domain socket by an
15 override of ListenGroup= in dkimpy-milter.socket (see
16 systemd.unit(5) for how to override). This lets you control who has
17 access to the daemon with finer granularity than is available with
18 dkimpy-milter on its own.
20- dkimpy-milter will not consume system resources if it is not used.
22- A fully-supervised dkimpy-milter needs no PIDFile, UMask, UserID, or
23 Socket configuation. This eliminates common race conditions and
24 startup failures, and simplifies the resulting configuration file.
26There is one downside to using socket activation:
28- it will only work on systems where libmilter can support connection
29 strings like "fd:3". This has been supported on Debian and derived
30 systems since sendmail 8.14.4-6 (before Debian Jessie, in early
31 2014), see for example:
diff --git a/system/socket-activation/dkimpy-milter.service b/system/socket-activation/dkimpy-milter.service
0new file mode 10064433new file mode 100644
index 0000000..2116bb7
--- /dev/null
+++ b/system/socket-activation/dkimpy-milter.service
@@ -0,0 +1,11 @@
2Description=DKIMpy Milter
3Documentation=man:dkimpy-milter(8) man:dkimpy-milter.conf(5)
7ExecStart=/usr/bin/dkimpy-milter /etc/dkimpy-milter.conf
diff --git a/system/socket-activation/dkimpy-milter.socket b/system/socket-activation/dkimpy-milter.socket
0new file mode 10064412new file mode 100644
index 0000000..d4338a3
--- /dev/null
+++ b/system/socket-activation/dkimpy-milter.socket
@@ -0,0 +1,12 @@
2Description=DKIMpy Milter socket
3Documentation=man:dkimpy-milter(8) man:dkimpy-milter.conf(5)
8# override SocketGroup to grant access to members of another system group:


People subscribed via source and target branches

to all changes: