sshd overrides from /etc/ssh/sshd_config.d/*conf apply in reverse lexographic order

[Robert C Jennings]

I am looking at the addition of 'Include /etc/ssh/sshd_config.d/*conf' for use in Ubuntu cloud images. I wanted to add a config file and see if I had done things correctly. I assumed that the files were sourced lexographically (based on use of glob() in readconf.h) so that I could document how users could override our tuning. But it appears from 'sshd -T' output and observed behavior that the first file in /etc/sshd_config.d/ to define a parameter wins. I see in 'sshd -ddd' output that they are parsed lexographically but it seems that their settings apply in reverse (or whichever comes first) if that makes sense. I'd like to understand if this is correct behavior and get it documented.

Steps to reproduce on focal with openssh-server 1:8.2p1-4:

1. Create the following files in /etc/ssh/sshd_config.d/ with the content shown below:
40-cloudimg-settings.conf:
  ClientAliveInterval 110
  PasswordAuthentication yes
  PermitRootLogin no

50-cloudimg-settings.conf:
  ClientAliveInterval 120
  PermitRootLogin yes

60-cloudimg-settings.conf:
  ClientAliveInterval 180

2. Check what sshd thinks the values will be with 'sshd -T|grep -i clientaliveinterval' and 'sshd -T|grep permitrootlogin'
clientaliveinterval 110
permitrootlogin no

(The tuning I cared about was ClientAliveInterval for my work but PermitRootLogin is easier to demonstrate)

3. Run '/usr/sbin/sshd -ddd' to check debug output for config file parsing behavior:
debug2: load_server_config: filename /etc/ssh/sshd_config
debug2: load_server_config: done config len = 296
debug2: parse_server_config_depth: config /etc/ssh/sshd_config len 296
debug2: /etc/ssh/sshd_config line 13: new include /etc/ssh/sshd_config.d/*.conf
debug2: /etc/ssh/sshd_config line 13: including /etc/ssh/sshd_config.d/40-cloudimg-settings.conf
debug2: load_server_config: filename /etc/ssh/sshd_config.d/40-cloudimg-settings.conf
debug2: load_server_config: done config len = 71
debug2: parse_server_config_depth: config /etc/ssh/sshd_config.d/40-cloudimg-settings.conf len 71
debug3: /etc/ssh/sshd_config.d/40-cloudimg-settings.conf:1 setting ClientAliveInterval 110
debug3: /etc/ssh/sshd_config.d/40-cloudimg-settings.conf:2 setting PasswordAuthentication yes
debug3: /etc/ssh/sshd_config.d/40-cloudimg-settings.conf:3 setting PermitRootLogin no
debug2: /etc/ssh/sshd_config line 13: including /etc/ssh/sshd_config.d/50-cloudimg-settings.conf
debug2: load_server_config: filename /etc/ssh/sshd_config.d/50-cloudimg-settings.conf
debug2: load_server_config: done config len = 46
debug2: parse_server_config_depth: config /etc/ssh/sshd_config.d/50-cloudimg-settings.conf len 46
debug3: /etc/ssh/sshd_config.d/50-cloudimg-settings.conf:1 setting ClientAliveInterval 120
debug3: /etc/ssh/sshd_config.d/50-cloudimg-settings.conf:2 setting PermitRootLogin yes
debug2: /etc/ssh/sshd_config line 13: including /etc/ssh/sshd_config.d/60-cloudimg-settings.conf
debug2: load_server_config: filename /etc/ssh/sshd_config.d/60-cloudimg-settings.conf
debug2: load_server_config: done config len = 25
debug2: parse_server_config_depth: config /etc/ssh/sshd_config.d/60-cloudimg-settings.conf len 25
debug3: /etc/ssh/sshd_config.d/60-cloudimg-settings.conf:1 setting ClientAliveInterval 180

4. Set a root password and unlock the account.

5. Attempt to ssh as root to the instance with a password.

Observation:
 * Root password login is denied if PermitRootLogin is 'no' in 40-foo.conf and 'yes' in 50-foo.conf
 * Root password login is allowed if PermitRootLogin is 'yes' in 40-foo.conf and 'no' in 50-foo.conf

It appears in 'sshd -ddd' output that files are parsed in lexographic order (40-foo.conf before 50-foo.conf) but the behavior observed indicates that the value set in 40-foo.conf overrides 50-foo.conf which is counter to expectations.

[Colin Watson (cjwatson)]

This is intentional and documented on the part of upstream, and this behaviour is why I include sshd_config.d at the start of sshd_config rather than the end. Yes, it's arguably counter-intuitive; no, there isn't much I can do about it. sshd_config(5) says in the first paragraph of its description:

  "For each keyword, the first obtained value will be used."

[Robert C Jennings (rcj)]

Drat, I see that now in sshd_config. I think I tried searching for "first" and maybe by that point I was just looking at sshd's man page. Okay, well then this is fine for me.... It's documented and stable. We can comfortably make this change for cloud-images. Thanks Colin!

[Dimitri John Ledkov (xnox)]

Can we please change:

Include /etc/ssh/sshd_config.d/*conf

To:

Include /run/ssh/sshd_config.d/*conf
Include /etc/ssh/sshd_config.d/*conf
Include /lib/ssh/sshd_config.d/*conf

?

This will help us achieving the goal of emptier /etc, allow baking "image" configs in /lib, have user overrides in /etc, and allow initrd/runtime configs in /run. This follows the principle of separating different configs (transient, user-overrides, persistent/defaults) as is becoming popular in many projects.

Also, does it mean we could potentially move all of the package default /etc/ssh/sshd_config to /usr/lib ? which includes /etc/ssh/sshd_config ? Something like:

/lib/ssh/sshd_config would then have
Include /run/ssh/sshd_config.d/*
Include /etc/ssh/sshd_config.d/*
Include /etc/ssh/sshd_config
Include /lib/ssh/sshd_config.d/*
<all other defaults>

It would be nice if /etc/ssh only had the host keys, and no other default options.

[Dimitri John Ledkov (xnox)]

I wonder, if i can try doing that with core22

[Seth Arnold (seth-arnold)]

On Fri, Apr 24, 2020 at 01:16:31PM -0000, Dimitri John Ledkov wrote:
> Include /run/ssh/sshd_config.d/*conf
> Include /etc/ssh/sshd_config.d/*conf
> Include /lib/ssh/sshd_config.d/*conf

> It would be nice if /etc/ssh only had the host keys, and no other
> default options.

This feels like it'd also need systemd-style config options to allow
admins to say they don't want specific packaged configs, too.

This mechanism could be ideal for eg ec2-instance-connect, except the
current implementation, via:
/lib/systemd/system/ssh.service.d/ec2-instance-connect.conf
can be ignored via a symlink to /dev/null in
/etc/systemd/system/ssh.service.d/ec2-instance-connect.conf

Changing to sshd config snippets in /lib/ssh/sshd_config.d/ would now
require uninstalling the package entirely, which might also require
uninstalling meta-packages.

A simple 'include' mechanism without allowances for nulling out unwanted
configs is useful but probably not alone sufficient.

Thanks