[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#778913: openssh-server: init (at least systemd) doesn't notice when sshd fails to start and reports success



Control: tags -1 + patch

Am 12.05.2015 um 13:45 schrieb Michael Biebl:
> On Mon, 30 Mar 2015 04:02:01 +0200 Christoph Anton Mitterer
>> As for sd_notify,... a simply google query didn't turn up any existing
>> patches for that and it may be hard to convince upstream to do it ;)
> 
> A patch for that should be not that complicated and might even be worth
> shipping downstream if upstream doesn't want to apply it.

Attached is a patch which adds support for sd_notify.
The configure.ac changes are a bit more convoluted then I hoped since 
openssh doesn't use the pkg-config provided macros.


A quick test (with a broken configuration file) at least seems to 
properly error out:


root@pluto:~# systemctl status ssh.service 
● ssh.service - OpenBSD Secure Shell server
   Loaded: loaded (/etc/systemd/system/ssh.service; enabled)
   Active: active (running) since Di 2015-05-12 17:03:28 CEST; 4s ago
 Main PID: 13021 (sshd)
   CGroup: /system.slice/ssh.service
           └─13021 /usr/sbin/sshd -D

Mai 12 17:03:28 pluto sshd[13021]: Server listening on 0.0.0.0 port 22.
Mai 12 17:03:28 pluto sshd[13021]: Server listening on :: port 22.

root@pluto:~# echo foobar >> /etc/ssh/sshd_config 

root@pluto:~# systemctl restart ssh.service 
Job for ssh.service failed. See 'systemctl status ssh.service' and 'journalctl -xn' for details.

root@pluto:~# systemctl status ssh.service 
● ssh.service - OpenBSD Secure Shell server
   Loaded: loaded (/etc/systemd/system/ssh.service; enabled)
   Active: failed (Result: start-limit) since Di 2015-05-12 17:03:51 CEST; 5s ago
  Process: 13053 ExecStart=/usr/sbin/sshd -D $SSHD_OPTS (code=exited, status=255)
 Main PID: 13053 (code=exited, status=255)

Mai 12 17:03:51 pluto sshd[13053]: /etc/ssh/sshd_config: terminating, 1 bad configuration options
Mai 12 17:03:51 pluto systemd[1]: ssh.service: main process exited, code=exited, status=255/n/a
Mai 12 17:03:51 pluto systemd[1]: Failed to start OpenBSD Secure Shell server.
Mai 12 17:03:51 pluto systemd[1]: Unit ssh.service entered failed state.
Mai 12 17:03:51 pluto systemd[1]: ssh.service start request repeated too quickly, refusing to start.
Mai 12 17:03:51 pluto systemd[1]: Failed to start OpenBSD Secure Shell server.
Mai 12 17:03:51 pluto systemd[1]: Unit ssh.service entered failed state.


As you can see, systemd tries to repeatedly start the service until it hits
start-limit.
We should use sd_notify in that case to pass a correct error code to systemd.

The patch is not complete yet, more a PoC.

That said, would be glad if Colin could give it some proper review.
Don't want to spend time on it, if it's unlikely to get merged.


Michael
-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
diff --git a/configure.ac b/configure.ac
index f5c65c5..ef154ba 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4137,6 +4137,29 @@ AC_ARG_WITH(consolekit,
 	fi ]
 )
 
+# Check whether user wants systemd support
+SYSTEMD_MSG="no"
+AC_ARG_WITH(systemd,
+	[  --with-systemd          Enable systemd support],
+	[ if test "x$withval" != "xno" ; then
+		AC_PATH_TOOL([PKGCONFIG], [pkg-config], [no])
+		if test "$PKGCONFIG" != "no"; then
+			AC_MSG_CHECKING([for libsystemd])
+			if $PKGCONFIG --exists libsystemd; then
+				SYSTEMD_CFLAGS=`$PKGCONFIG --cflags libsystemd`
+				SYSTEMD_LIBS=`$PKGCONFIG --libs libsystemd`
+				CPPFLAGS="$CPPFLAGS $SYSTEMD_CFLAGS"
+				SSHDLIBS="$SSHDLIBS $SYSTEMD_LIBS"
+				AC_MSG_RESULT([yes])
+				AC_DEFINE(HAVE_SYSTEMD, 1, [Define if you want systemd support.])
+				SYSTEMD_MSG="yes"
+			else
+				AC_MSG_RESULT([no])
+			fi
+		fi
+	fi ]
+)
+
 # Looking for programs, paths and files
 
 PRIVSEP_PATH=/var/empty
@@ -4939,6 +4962,7 @@ echo "                   libedit support: $LIBEDIT_MSG"
 echo "  Solaris process contract support: $SPC_MSG"
 echo "           Solaris project support: $SP_MSG"
 echo "                ConsoleKit support: $CONSOLEKIT_MSG"
+echo "                   systemd support: $SYSTEMD_MSG"
 echo "       IP address in \$DISPLAY hack: $DISPLAY_HACK_MSG"
 echo "           Translate v4 in v6 hack: $IPV4_IN6_HACK_MSG"
 echo "                  BSD Auth support: $BSD_AUTH_MSG"
diff --git a/debian/control b/debian/control
index c513f4e..6144cf3 100644
--- a/debian/control
+++ b/debian/control
@@ -2,7 +2,7 @@ Source: openssh
 Section: net
 Priority: standard
 Maintainer: Debian OpenSSH Maintainers <debian-ssh@lists.debian.org>
-Build-Depends: libwrap0-dev | libwrap-dev, zlib1g-dev (>= 1:1.2.3), libssl-dev (>= 0.9.8g), libpam0g-dev | libpam-dev, libgtk2.0-dev, libedit-dev, debhelper (>= 9~), dh-exec, libselinux1-dev [linux-any], libkrb5-dev | heimdal-dev, dpkg-dev (>= 1.16.1~), libck-connector-dev, dh-autoreconf, autotools-dev, dh-systemd (>= 1.4)
+Build-Depends: libwrap0-dev | libwrap-dev, zlib1g-dev (>= 1:1.2.3), libssl-dev (>= 0.9.8g), libpam0g-dev | libpam-dev, libgtk2.0-dev, libedit-dev, debhelper (>= 9~), dh-exec, libselinux1-dev [linux-any], libkrb5-dev | heimdal-dev, dpkg-dev (>= 1.16.1~), libck-connector-dev, dh-autoreconf, autotools-dev, dh-systemd (>= 1.4), libsystemd-dev [linux-any]
 XS-Testsuite: autopkgtest
 Standards-Version: 3.9.6
 Uploaders: Colin Watson <cjwatson@debian.org>, Matthew Vernon <matthew@debian.org>
diff --git a/debian/rules b/debian/rules
index 570e651..8429054 100755
--- a/debian/rules
+++ b/debian/rules
@@ -91,6 +91,7 @@ confflags += --with-kerberos5=/usr
 confflags += --with-ssl-engine
 ifeq ($(DEB_HOST_ARCH_OS),linux)
 confflags += --with-selinux
+confflags += --with-systemd
 endif
 ifeq ($(DISTRIBUTOR),Ubuntu)
 confflags += --with-consolekit
diff --git a/debian/systemd/ssh.service b/debian/systemd/ssh.service
index ff28d39..51b3566 100644
--- a/debian/systemd/ssh.service
+++ b/debian/systemd/ssh.service
@@ -9,6 +9,7 @@ ExecStart=/usr/sbin/sshd -D $SSHD_OPTS
 ExecReload=/bin/kill -HUP $MAINPID
 KillMode=process
 Restart=on-failure
+Type=notify
 
 [Install]
 WantedBy=multi-user.target
diff --git a/sshd.c b/sshd.c
index 23d5a64..180e9eb 100644
--- a/sshd.c
+++ b/sshd.c
@@ -84,6 +84,10 @@
 #include <prot.h>
 #endif
 
+#ifdef HAVE_SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
+
 #include "xmalloc.h"
 #include "ssh.h"
 #include "ssh1.h"
@@ -1927,6 +1931,12 @@ main(int ac, char **av)
 	/* ignore SIGPIPE */
 	signal(SIGPIPE, SIG_IGN);
 
+
+#ifdef HAVE_SYSTEMD
+	/* Signal systemd that we are ready to accept connections */
+	sd_notify(0, "READY=1");
+#endif
+
 	/* Get a connection, either from inetd or a listening TCP socket */
 	if (inetd_flag) {
 		server_accept_inetd(&sock_in, &sock_out);

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: