restart doesn't test for syntax errors

Bug #1913810 reported by Andreas Hasenack
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
openssh (Ubuntu)
Confirmed
Undecided
Unassigned

Bug Description

Tested openssh on bionic and groovy, same issue.

The switch to systemd lost the ability to do a sanity check on the config file (via sshd -t) before attempting to restart sshd. This was originally bug #624361 in the SySV days, fixed in the initscript back then.

The sysv script still does it, but it's not used anymore:
 restart)
        check_privsep_dir
        check_config
        log_daemon_msg "Restarting OpenBSD Secure Shell server" "sshd" || true

And:
check_config() {
    if [ ! -e /etc/ssh/sshd_not_to_be_run ]; then
        /usr/sbin/sshd $SSHD_OPTS -t || exit 1
    fi
}

The systemd service file has only ExecStartPre, which doesn't let it start if there is an error, but will happily stop it:
[Unit]
Description=OpenBSD Secure Shell server
After=network.target auditd.service
ConditionPathExists=!/etc/ssh/sshd_not_to_be_run

[Service]
EnvironmentFile=-/etc/default/ssh
ExecStartPre=/usr/sbin/sshd -t
ExecStart=/usr/sbin/sshd -D $SSHD_OPTS
ExecReload=/usr/sbin/sshd -t
ExecReload=/bin/kill -HUP $MAINPID
...

Example:
# sshd -t
# systemctl restart sshd
# telnet localhost 22
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
SSH-2.0-OpenSSH_7.6p1 Ubuntu-4ubuntu0.3
^]
telnet> quit
Connection closed.

# echo "syntax error" >> /etc/ssh/sshd_config
# sshd -t
/etc/ssh/sshd_config: line 123: Bad configuration option: syntax
/etc/ssh/sshd_config: terminating, 1 bad configuration options

# systemctl restart sshd
Job for ssh.service failed because the control process exited with error code.
See "systemctl status ssh.service" and "journalctl -xe" for details.

# telnet localhost 22
Trying 127.0.0.1...
telnet: Unable to connect to remote host: Connection refused
#

tags: added: server-next
Changed in openssh (Ubuntu):
status: New → Confirmed
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Ideally, this should be supported by systemd somehow. There is this (old) discussion upstream, which is relevant here: https://github.com/systemd/systemd/issues/2175

If we introduced the desired behavior by including an ExecStop script to the systemd unit configuration file, we would introduce a regression since stopping the service for erroneous configuration files would not be allowed (this was not the behavior for sysV).

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

yeah, it's specifically restart that we want to check

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi,
since the obvious solution (ExecStop) was found to be bad as it introduces a regression and it is no more even clear on which level it has to be solved (systemd, openssh in Debian, openssh in Ubuntu) I think this is no more as-actionable as before.

Sure if someone has the time (or just enjoys it) please work out a solution that works out fine and suggest it here or to Debian.
The most obvious one we see in a few other packages is a wrapper script instead of calling the binary. But that often enough brings other issues and some people just hate the approach.
Maybe worth having a discussion with cjwatson upfront what he thinks would be acceptible for packaging in Debian.

untagging server-next for the reasons outlined above.

tags: removed: server-next
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.