From 9d5316ca0e6c573db6a0d3aeaad4e23815e8e301 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 15:41:35 -0500 Subject: [PATCH 1/3] 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 2/3] 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 7092874729417f7b8032520af21f97375d570d3a Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 21 Feb 2019 17:12:59 -0500 Subject: [PATCH 3/3] 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