rpms / openssh

Clone
Source Code
GIT

#39 Use a service unit to strip ssh_keys group from host keys (rhbz#2172956)
Closed a year ago by siosm. Opened a year ago by siosm.
rpms/ siosm/openssh ssh-host-key-unit  into  rawhide

file modified
+13 -6 
@@ -47,7 +47,7 @@ 
 

  

  # Do not forget to bump pam_ssh_agent_auth release if you rewind the main package release to 1
 

  %global openssh_ver 9.0p1
 

- %global openssh_rel 10
 

+ %global openssh_rel 11
 

  %global pam_ssh_agent_ver 0.10.4
 

  %global pam_ssh_agent_rel 7
 

  
@@ -74,6 +74,7 @@ 
 

  Source16: ssh-agent.service
 

  Source17: ssh-agent.socket
 

  Source19: openssh-server-systemd-sysusers.conf
 

+ Source20: ssh-host-keys-perms.service
 

  

  #https://bugzilla.mindrot.org/show_bug.cgi?id=2581
 

  Patch100: openssh-6.7p1-coverity.patch
 
@@ -582,6 +583,12 @@ 
 

  install contrib/ssh-copy-id.1 $RPM_BUILD_ROOT%{_mandir}/man1/
 

  install -d -m711 ${RPM_BUILD_ROOT}/%{_datadir}/empty.sshd
 

  install -p -D -m 0644 %{SOURCE19} %{buildroot}%{_sysusersdir}/openssh-server.conf
 

+ # Migration for Fedora 38 change to remove group ownership for standard host keys
 

+ # See https://fedoraproject.org/wiki/Changes/SSHKeySignSuidBit
 

+ # To be removed in Fedora 40
 

+ install -m644 %{SOURCE20} $RPM_BUILD_ROOT/%{_unitdir}/ssh-host-keys-perms.service
 

+ install -d -m755 $RPM_BUILD_ROOT/%{_unitdir}/sshd.service.wants.d/
 

+ ln -s /%{_unitdir}/ssh-host-keys-perms.service $RPM_BUILD_ROOT/%{_unitdir}/sshd.service.wants.d/ssh-host-keys-perms.service
 

  

  %if ! %{no_gnome_askpass}
 

  install contrib/gnome-ssh-askpass $RPM_BUILD_ROOT%{_libexecdir}/openssh/gnome-ssh-askpass
 
@@ -608,11 +615,6 @@ 
 

  

  %pre server
 

  %sysusers_create_compat %{SOURCE19}
 

- # Migration scriptlet for Fedora 38/39
jlebon commented a year ago

On traditional systems, don't we still need to chmod/chown the keys at update time though? Otherwise, sshd.service won't be able to restart after it's updated. Probably would be cleaner at this point to split the logic into a separate script and have both the systemd service and scriptlet call it.

 
- # We want to remove group ownership for standard host keys if they exist
 

- test -f /etc/ssh/ssh_host_rsa_key     && /usr/bin/chmod g-r /etc/ssh/ssh_host_rsa_key     || :
 

- test -f /etc/ssh/ssh_host_ecdsa_key   && /usr/bin/chmod g-r /etc/ssh/ssh_host_ecdsa_key   || :
 

- test -f /etc/ssh/ssh_host_ed25519_key && /usr/bin/chmod g-r /etc/ssh/ssh_host_ed25519_key || :
 

  

  %post server
 

  %systemd_post sshd.service sshd.socket
 
@@ -699,6 +701,8 @@ 
 

  %attr(0644,root,root) %{_unitdir}/sshd-keygen@.service
 

  %attr(0644,root,root) %{_unitdir}/sshd-keygen.target
 

  %attr(0644,root,root) %{_sysusersdir}/openssh-server.conf
 

+ %attr(0644,root,root) %{_unitdir}/ssh-host-keys-perms.service
 

+ %attr(0755,root,root) %{_unitdir}/sshd.service.wants.d/ssh-host-keys-perms.service
 

  

  %files keycat
 

  %doc HOWTO.ssh-keycat
 
@@ -720,6 +724,9 @@ 
 

  %endif
 

  

  %changelog
 

+ * Wed Feb 22 2023 Timothée Ravier <tim@siosm.fr> - 9.0p1-11
 

+ - Use a systemd unit to restore default host key permissions (rhbz#2172956)
 

+ 

  * Fri Dec 02 2022 Dmitry Belyavskiy <dbelyavs@redhat.com> - 9.0p1-10
 

  - Restore upstream behaviour and default host key permissions (rhbz#2141272)

file added
+14 
@@ -0,0 +1,14 @@ 
 

+ [Unit]
 

+ Description=Update OpenSSH host key permissions
 

+ Documentation=https://fedoraproject.org/wiki/Changes/SSHKeySignSuidBit
 

+ Before=sshd.service
 

+ After=ssh-keygen.target
jlebon commented a year ago

sshd-keygen.target

 
+ ConditionPathExists=!/var/lib/.fedora_38_change_openssh_host_key_perms
 

+ 

+ [Service]
 

+ # Remove group ownership for standard host keys if they exist
 

+ ExecStart=-/bin/bash -c "for key in $(sshd -T | grep "^hostkey " | cut -d' ' -f2); do test -f $key && (chmod g-r $key ; chown root:root $key); done"

I can't remember - do $ in these unit files need to be escaped. IIRC it's something like $$. 

+ ExecStart=touch "/var/lib/.fedora_38_change_openssh_host_key_perms"
 

+ 

+ [Install]
 

+ WantedBy=sshd.service

Use a systemd service unit to strip the ssh_keys group and change the
mode for host keys. This ensure that this migration is done right before
the openssh server startup on all kind of systems, either RPM or
rpm-ostree based.

Use a marker file to only do this once.

See: https://fedoraproject.org/wiki/Changes/SSHKeySignSuidBit
Fixes: 7a21555 Get rid of ssh_keys group for new installations

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

I wonder if instead of || : here we want ExecStart-=/bin/bash.... The errors get ignored but I think the systemctl status ssh-host-keys-perms.service will show ones that failed.

Maybe we should include a hint somewhere in a comment in these files about when we think it would be safe to drop this service.

Do I understand correctly that if for whatever reason this service fails or doesn't run, the failure mode is that users can no longer SSH into their machines (if using host-based auth)?

If so, given that SSH is so crucial, I'd suggest carrying the permissive mode bits patch for a bit longer to provide a migration window rather than a single migration event.

Could you please explain what's wrong with changing the permissions on installation?

Could you please explain what's wrong with changing the permissions on installation?

In OSTree-based variants, updates are created server-side and shipped to users in one unit. So on variants which ship openssh-server by default (which is all of them), this %pre script will run in Koji where it does not have access to the user data. Having it be a systemd unit allows the migration to happen on the users' machines.

Can you confirm if my assessment in https://src.fedoraproject.org/rpms/openssh/pull-request/39#comment-129094 is correct?

@jlebon no, the sshd will not start. It's much worse than just HostBasedAuth

@jlebon no, the sshd will not start. It's much worse than just HostBasedAuth

Right, makes sense. WDYT about instead of carrying the patch as is, we tweak it so that sshd still emits the message but doesn't error out? Ideally, we'd have some better way to notify the user of the error condition we're in, but the best way to do that might be variant-specific. There's /etc/motd.d I guess? It's awkward though because all SSH users would see it instead of just those who should know about it (and can actually do something about it). Another approach is making the service sleep for some time after emitting the warning before continuing.

The current approach (both rpm scriptlet and systemd unit) operate on:

/etc/ssh/ssh_host_rsa_key
/etc/ssh/ssh_host_ecdsa_key
/etc/ssh/ssh_host_ed25519_key

but those are just the default HostKey values. What if someone changed the default on their system. I think we need to give them some time to absorb this change. Unfortunately this is a hard problem to solve. Sleeping on startup if we detect the non-compliant perms may be enough to get the admins to look at the system and fix things. We could ramp up the sleep over time. 60s, 300s, 600s, etc.. to hopefully give them a clue.

Obviously the failure scenario here is much worse if users have no other way to get into the system other than SSH. If they do have a serial or VGA console they can fix the permissions directly, though that does require them to either know the root password (or have a privileged account) or drop into single user mode.

What if someone changed the default on their system. I think we need to give them some time to absorb this change.

Yes. 100% agreed. I think we should be patching the openssh server binary itself to do this sort of "warn and sleep" and not hack it in separate systemd units or %post scripts.

Short of re-implementing a parser for the OpenSSH config format outside of the project, patching the binary is going to work most reliably.

Evil idea,
append the issue to motd (or print it directly from sshd) in addition to delays, so that when they log in they get that error directly in their face that tells them this is going away and if they do nothing eventually a future upgrade will break sshd and they will be locked out.

It won't help people that use exclusively automation to handle machines (like ansible) for that there is the delay to draw attention and hopefully cause an admin to actually try to manually log in via ssh.

However motd would also be shown on interactive login if they have a remote console, which is why I propose it.

I think we should be patching the openssh server binary itself to do this sort of "warn and sleep" and not hack it in separate systemd units or %post scripts.

100% - I realize I wasn't clear on that front, but yes. The part now that fails (on startup) in the ssh binary when the perms aren't correct is the part that would do the warn+sleep.

rebased onto 6d350edd85775416d05522bff21f3696601504ff
a year ago

rebased onto bf9aa35838195c93fbf05cf128a201d799613d64
a year ago

I've update my PR to reflect our recent discussion. The service should now update all configured keys which should make this much safer (even safer that the current code).

This still needs testing.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/09706f889ad74cb49867ac70c7424c45

rebased onto 8da5856fe032a8d0a96fdea6abc5280c2cf4e1c9
a year ago

maybe let's do grep "^hostkey " just so we make sure we only capture lines that start with hostkey

rebased onto 631a2716e8bf1f90ff15a28c02c31dd86005cdd7
a year ago

rebased onto 397dbe2f2c6653b1afeb057ac5e0ee4a85699f10
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/14d5831cfc934d07999fc56a104f27be

rebased onto 3cacb223d898c476da7608e8acb0163ccaf5aad6
a year ago

I've fixed this PR so that it builds. We can now start testing it.

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/4de8232611614052951f7ef2a3f78ee8

rebased onto 31834d2add0c213c6bbfd88d9d1b0f0c75e89470
a year ago

I've only set the full path for Bash as that's started by systemd. Everything else is started from Bash so should be fine and in $PATH.

Shouldn't this be sshd.service? Also, shouldn't it be WantedBy= instead so that if we somehow fail, sshd.service is still started (it might fail anyway, but we might as well try it).

Note this service will also run on freshly provisioned systems where the SSH host keys were just generated. I think the code here will run concurrently with sshd-keygen@.service but should still work fine as written here. But maybe we could at least include ConditionFirstBoot=false for the benefit of distros where that directive works (like FCOS).

But maybe we could at least include ConditionFirstBoot=false for the benefit of distros where that directive works (like FCOS).

But I guess anyway the service will run on the next boot since the stamp file won't be written. So maybe it's simpler to just not do this and keep it uniform across Fedora.

rebased onto 98decfaef9a3d36557a137cb40052b0357d05047
a year ago

rebased onto 4e3a8d1
a year ago

Thanks, fixed the sshd typo and used wants instead of requires.

If we want to use ConditionFirstBoot=false then we need to double check how conditions compose.

sshd-keygen.target

If we want to use ConditionFirstBoot=false then we need to double check how conditions compose.

It would be a logical AND. But anyway, I think I've convinced myself out of it.

I can't remember - do $ in these unit files need to be escaped. IIRC it's something like $$.

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/23c0f513fab94245a031ba276d961820

On traditional systems, don't we still need to chmod/chown the keys at update time though? Otherwise, sshd.service won't be able to restart after it's updated. Probably would be cleaner at this point to split the logic into a separate script and have both the systemd service and scriptlet call it.

I was trying to take this and get it into a usable form and test it but it turns out the sshd -T trick doesn't work because it also gets tripped up on the non-conforming host keys:

$ sudo sshd -T
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Permissions 0640 for '/etc/ssh/ssh_host_rsa_key' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Permissions 0640 for '/etc/ssh/ssh_host_ecdsa_key' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Permissions 0640 for '/etc/ssh/ssh_host_ed25519_key' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
sshd: no hostkeys available -- exiting.

Unless anyone has any other ideas we'll be back to only updating the default host keys and anyone has any other host keys in other locations will be screwed.

I guess we could parse that output and pull out the paths, but that's pretty yucky.

Could sshd be changed to not fail on the permission issue in -T mode since that's the thing we're trying to fix in the first place? Or are there security concerns with that?
Edited a year ago by jlebon

I posted an updated and tested version of this over in https://src.fedoraproject.org/rpms/openssh/pull-request/40#

Pull-Request has been closed by siosm
a year ago
Metadata
None
No Tags
Changes Summary 2
+13 -6
file changed
openssh.spec
+14
file added
ssh-host-keys-perms.service
Powered by Pagure 5.13.3
Documentation • File an Issue • About this Instance • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.