kevin martin wrote:
> not sure why having the systemd notify code in openssh as a
> configure time option would be such a bad thing.
At the very least it introduces a dependency on libsystemd into sshd,
which is undesirable for reasons of security and convenience. The
principle of "you are done when you can not remove any more" confirms
that it is unwise to add dependencies without very careful consideration.
I've read through the debian and Red Hat bug reports.
There are two different but related problems here:
1. For systemctl [re]start, when a .service file has Type=simple,
systemd assumes that service startup can never fail, and immediately
considers this service successfully started when the exec() of sshd
has succeeded.
That's debatable design within systemd, but it's hard for systemd to
know when a given service has actually started successfully, and
services which fit that assumption do exist.
So when sshd detects an error on startup and exits with an error code
shortly after being started, systemd considers the service to first
have started successfully and then to have exited with an error, so
it then restarts the service. Repeat.
When service limits are exhausted the service ends up in a failed state.
Meanwhile, the systemctl [re]start command doesn't report any error
to the administrator, because systemd considers the service to have
[re]started successfully once. This is "error messages are lost".
2. For systemctl reload, systemd can and arguably should send SIGHUP
to sshd. More uncertainty and assumptions within systemd follows;
sshd re-exec:s, meaning that the PID stays the same, so systemd
doesn't receive SIGCHLD and so even if 1. is fixed, here systemd will
not understand that there an error during startup of the new sshd is
to be considered a failed reload. Ie. the above problems apply here
again. The systemctl reload sshd command is always immediately
successful, even if re-exec:ed sshd detects an error in the config
file.
In both these cases, systemctl reports no error, while sshd isn't running.
So what to do?
A workaround for [re]start is to add sshd -t ExecStartPre linting,
but that doesn't help at all with reload.
It would be good to have sshd integrate with systemd here, but we
need to avoid the libsystemd dependency.
Fortunately, sd_notify() doesn't need to do all too much; almost
everything is used before in the OpenSSH codebase, so it's easy
enough to add local code for it. It's a sendmsg() with SCM_CREDENTIALS
to the AF_UNIX SOCK_DGRAM named in $NOTIFY_SOCKET.
The file descriptor passing code in monitor_fdpass.c sends other
messages with ancillary data.
Damien, how do you feel about adding the notification without the
dependency, maybe conditioned on a configure.ac check for (Linux-only)
SCM_CREDENTIALS?
I think the minimum viable product would be to emit READY=1 once
startup is complete and RELOADING=1 on SIGHUP receipt.
STOPPING=1 would also make sense in sshd exit paths if something
could end up blocking along the way, but at least the SIGTERM case
in server_accept_loop() doesn't seem to need that.
STATUS= and ERRNO= could be nice-to-haves for error messages.
So I wrote a simple sd_notify() and am attaching it here, but the
address part and a connect() may need to be outside the function
with privilege separation. Thoughts on this idea?
//Peter
/*
* Copyright (c) 2018 Peter Stuge <peter@xxxxxxxx>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
* copyright notice and this permission notice appear in all copies.
*
* THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
#define _GNU_SOURCE
#include <stdio.h>
#include <stddef.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#include <errno.h>
int sd_notify(int unset_environment, const char *state) {
char *path;
int pathlen, ret = 0, sock = -1;
struct sockaddr_un sa = { AF_UNIX };
struct iovec vec;
struct msghdr msg;
struct ucred creds;
char cmsgbuf[CMSG_SPACE(sizeof creds)];
struct cmsghdr *cmsg;
path = getenv("NOTIFY_SOCKET");
if (NULL == path)
return 0;
pathlen = snprintf(sa.sun_path, sizeof sa.sun_path, "%s", path);
if (pathlen >= sizeof sa.sun_path) {
ret = -ENOMEM;
goto done;
}
if ('@' == sa.sun_path[0])
sa.sun_path[0] = '\0';
else
pathlen++;
sock = socket(sa.sun_family, SOCK_DGRAM, 0);
if (-1 == sock)
goto done;
memset(&msg, 0, sizeof msg);
msg.msg_name = &sa;
msg.msg_namelen = offsetof(struct sockaddr_un, sun_path) + pathlen;
vec.iov_base = (char *)state;
vec.iov_len = strlen(state);
msg.msg_iov = &vec;
msg.msg_iovlen = 1;
memset(&creds, 0, sizeof creds);
creds.pid = getpid();
creds.uid = getuid();
creds.gid = getgid();
memset(cmsgbuf, 0, sizeof cmsgbuf);
msg.msg_control = cmsgbuf;
msg.msg_controllen = sizeof cmsgbuf;
cmsg = CMSG_FIRSTHDR(&msg);
cmsg->cmsg_level = SOL_SOCKET;
cmsg->cmsg_type = SCM_CREDENTIALS;
cmsg->cmsg_len = CMSG_LEN(sizeof creds);
memcpy(CMSG_DATA(cmsg), &creds, sizeof creds);
msg.msg_controllen = cmsg->cmsg_len;
if (-1 == sendmsg(sock, &msg, 0))
goto done;
ret = 1;
done:
if (0 == ret)
ret = -errno;
if (sock != -1 && -1 == close(sock) && 1 == ret)
ret = -errno;
if (unset_environment && -1 == unsetenv("NOTIFY_SOCKET") && 1 == ret)
ret = -errno;
return ret;
}
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
