feat: Set RH ssh key permissions when no 'ssh_keys' group (#5296)

Fedora core 38 and above, centos 10 stream and all distributions derived from
them do not have the group 'ssh_keys'. Please see the fedora rawhide change
https://src.fedoraproject.org/rpms/openssh/c/7a21555354a2c5e724aa4c287b640c24bf108780?branch=rawhide

In those distributions, openssh versions are 9 and above. The private
key permissions are set as 0o600 and the public key permissions are set as
0o644 from sshd-keygen utility. The 'root' group owns the keys.
Please see
https://src.fedoraproject.org/rpms/openssh/c/b615362fd0b4da657d624571441cb74983de6e3f?branch=rawhide

In older releases where 'ssh_keys' group is present, the private key
permissions are set as 0o640. Public key permissions are 0o644. These
releases have openssh version less than 9.

Since cloud-init generates the keys and not the sshd-genkey utility,
permissions must be set accordingly for cloud-init generated public and
private keys for all cases. This includes cases where 'ssh_keys' group is
absent. This change fixes this. The code has been reworked a little
bit so as to simplify things. Unit tests have been adjusted accordingly.

Signed-off-by: Ani Sinha <anisinha@redhat.com>

cloudinit/config/cc_ssh.py

@@ -184,6 +184,42 @@
 KEY_GEN_TPL = 'o=$(ssh-keygen -yf "%s") && echo "$o" root@localhost > "%s"'
 
 
+def set_redhat_keyfile_perms(keyfile: str) -> None:
+    """
+    For fedora 37, centos 9 stream and below:
+     - sshd version is earlier than version 9.
+     - 'ssh_keys' group is present and owns the private keys.
+     - private keys have permission 0o640.
+    For fedora 38, centos 10 stream and above:
+     - ssh version is atleast version 9.
+     - 'ssh_keys' group is absent. 'root' group owns the keys.
+     - private keys have permission 0o600, same as upstream.
+    Public keys in all cases have permission 0o644.
+    """
+    permissions_public = 0o644
+    ssh_version = ssh_util.get_opensshd_upstream_version()
+    if ssh_version and ssh_version < util.Version(9, 0):
+        # fedora 37, centos 9 stream and below has sshd
+        # versions less than 9 and private key permissions are
+        # set to 0o640 from sshd-keygen.
+        # See sanitize permissions" section in sshd-keygen.
+        permissions_private = 0o640
+    else:
+        # fedora 38, centos 10 stream and above. sshd-keygen sets
+        # private key persmissions to 0o600.
+        permissions_private = 0o600
+
+    gid = util.get_group_id("ssh_keys")
+    if gid != -1:
+        # 'ssh_keys' group exists for fedora 37, centos 9 stream
+        # and below. On these distros, 'ssh_keys' group own the private
+        # keys. When 'ssh_keys' group is absent for newer distros,
+        # 'root' group owns the private keys which is the default.
+        os.chown(keyfile, -1, gid)
+    os.chmod(keyfile, permissions_private)
+    os.chmod(f"{keyfile}.pub", permissions_public)
+
+
 def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:
 
     # remove the static keys from the pristine image
@@ -280,16 +316,8 @@ def handle(name: str, cfg: Config, cloud: Cloud, args: list) -> None:
                     ):
                         sys.stdout.write(util.decode_binary(out))
 
-                    gid = util.get_group_id("ssh_keys")
-                    if gid != -1:
-                        # perform same "sanitize permissions" as sshd-keygen
-                        permissions_private = 0o600
-                        ssh_version = ssh_util.get_opensshd_upstream_version()
-                        if ssh_version and ssh_version < util.Version(9, 0):
-                            permissions_private = 0o640
-                        os.chown(keyfile, -1, gid)
-                        os.chmod(keyfile, permissions_private)
-                        os.chmod(f"{keyfile}.pub", 0o644)
+                    if cloud.distro.osfamily == "redhat":
+                        set_redhat_keyfile_perms(keyfile)
                 except subp.ProcessExecutionError as e:
                     err = util.decode_binary(e.stderr).lower()
                     if e.exit_code == 1 and err.lower().startswith(

tests/unittests/config/test_cc_ssh.py

@@ -307,7 +307,7 @@ def test_handle_publish_hostkeys(
 
     @pytest.mark.parametrize(
         "ssh_keys_group_exists,sshd_version,expected_private_permissions",
-        [(False, 0, 0), (True, 8, 0o640), (True, 10, 0o600)],
+        [(False, 9, 0o600), (True, 8, 0o640), (True, 10, 0o600)],
     )
     @mock.patch(MODPATH + "subp.subp", return_value=("", ""))
     @mock.patch(MODPATH + "util.get_group_id", return_value=10)
@@ -336,18 +336,17 @@ def test_ssh_hostkey_permissions(
         m_gid.return_value = 10 if ssh_keys_group_exists else -1
         m_sshd_version.return_value = util.Version(sshd_version, 0)
         key_path = cc_ssh.KEY_FILE_TPL % "rsa"
-        cloud = get_cloud(distro="ubuntu")
+        cloud = get_cloud(distro="centos")
         cc_ssh.handle("name", {"ssh_genkeytypes": ["rsa"]}, cloud, [])
         if ssh_keys_group_exists:
             m_chown.assert_called_once_with(key_path, -1, 10)
-            assert m_chmod.call_args_list == [
-                mock.call(key_path, expected_private_permissions),
-                mock.call(f"{key_path}.pub", 0o644),
-            ]
         else:
-            m_sshd_version.assert_not_called()
             m_chown.assert_not_called()
-            m_chmod.assert_not_called()
+
+        assert m_chmod.call_args_list == [
+            mock.call(key_path, expected_private_permissions),
+            mock.call(f"{key_path}.pub", 0o644),
+        ]
 
     @pytest.mark.parametrize("with_sshd_dconf", [False, True])
     @mock.patch(MODPATH + "util.ensure_dir")