sailesh1993 / rpms / cloud-init

Forked from rpms/cloud-init a year ago
Clone
f25da3
From 5bfe2ee2b063d87e6fd255d6c5e63123aa3f6de0 Mon Sep 17 00:00:00 2001
f25da3
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
f25da3
Date: Sat, 21 Aug 2021 13:55:53 +0200
f25da3
Subject: [PATCH] Fix home permissions modified by ssh module (SC-338) (#984)
f25da3
f25da3
RH-Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>
f25da3
RH-MergeRequest: 9: Fix home permissions modified by ssh module
f25da3
RH-Commit: [1/1] ab55db88aa1bf2f77acaca5e76ffabbab72b1fb2 (eesposit/cloud-init-centos-)
f25da3
RH-Bugzilla: 1995843
f25da3
RH-Acked-by: Mohamed Gamal Morsy <mmorsy@redhat.com>
f25da3
RH-Acked-by: Eduardo Otubo <otubo@redhat.com>
f25da3
f25da3
TESTED: By me and QA
f25da3
BREW: 39178085
f25da3
f25da3
Fix home permissions modified by ssh module (SC-338) (#984)
f25da3
f25da3
commit 7d3f5d750f6111c2716143364ea33486df67c927
f25da3
Author: James Falcon <therealfalcon@gmail.com>
f25da3
Date:   Fri Aug 20 17:09:49 2021 -0500
f25da3
f25da3
    Fix home permissions modified by ssh module (SC-338) (#984)
f25da3
f25da3
    Fix home permissions modified by ssh module
f25da3
f25da3
    In #956, we updated the file and directory permissions for keys not in
f25da3
    the user's home directory. We also unintentionally modified the
f25da3
    permissions within the home directory as well. These should not change,
f25da3
    and this commit changes that back.
f25da3
f25da3
    LP: #1940233
f25da3
f25da3
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
f25da3
---
f25da3
 cloudinit/ssh_util.py                         |  35 ++++-
f25da3
 .../modules/test_ssh_keysfile.py              | 132 +++++++++++++++---
f25da3
 2 files changed, 146 insertions(+), 21 deletions(-)
f25da3
f25da3
diff --git a/cloudinit/ssh_util.py b/cloudinit/ssh_util.py
f25da3
index b8a3c8f7..9ccadf09 100644
f25da3
--- a/cloudinit/ssh_util.py
f25da3
+++ b/cloudinit/ssh_util.py
f25da3
@@ -321,23 +321,48 @@ def check_create_path(username, filename, strictmodes):
f25da3
         home_folder = os.path.dirname(user_pwent.pw_dir)
f25da3
         for directory in directories:
f25da3
             parent_folder += "/" + directory
f25da3
-            if home_folder.startswith(parent_folder):
f25da3
+
f25da3
+            # security check, disallow symlinks in the AuthorizedKeysFile path.
f25da3
+            if os.path.islink(parent_folder):
f25da3
+                LOG.debug(
f25da3
+                    "Invalid directory. Symlink exists in path: %s",
f25da3
+                    parent_folder)
f25da3
+                return False
f25da3
+
f25da3
+            if os.path.isfile(parent_folder):
f25da3
+                LOG.debug(
f25da3
+                    "Invalid directory. File exists in path: %s",
f25da3
+                    parent_folder)
f25da3
+                return False
f25da3
+
f25da3
+            if (home_folder.startswith(parent_folder) or
f25da3
+                    parent_folder == user_pwent.pw_dir):
f25da3
                 continue
f25da3
 
f25da3
-            if not os.path.isdir(parent_folder):
f25da3
+            if not os.path.exists(parent_folder):
f25da3
                 # directory does not exist, and permission so far are good:
f25da3
                 # create the directory, and make it accessible by everyone
f25da3
                 # but owned by root, as it might be used by many users.
f25da3
                 with util.SeLinuxGuard(parent_folder):
f25da3
-                    os.makedirs(parent_folder, mode=0o755, exist_ok=True)
f25da3
-                    util.chownbyid(parent_folder, root_pwent.pw_uid,
f25da3
-                                   root_pwent.pw_gid)
f25da3
+                    mode = 0o755
f25da3
+                    uid = root_pwent.pw_uid
f25da3
+                    gid = root_pwent.pw_gid
f25da3
+                    if parent_folder.startswith(user_pwent.pw_dir):
f25da3
+                        mode = 0o700
f25da3
+                        uid = user_pwent.pw_uid
f25da3
+                        gid = user_pwent.pw_gid
f25da3
+                    os.makedirs(parent_folder, mode=mode, exist_ok=True)
f25da3
+                    util.chownbyid(parent_folder, uid, gid)
f25da3
 
f25da3
             permissions = check_permissions(username, parent_folder,
f25da3
                                             filename, False, strictmodes)
f25da3
             if not permissions:
f25da3
                 return False
f25da3
 
f25da3
+        if os.path.islink(filename) or os.path.isdir(filename):
f25da3
+            LOG.debug("%s is not a file!", filename)
f25da3
+            return False
f25da3
+
f25da3
         # check the file
f25da3
         if not os.path.exists(filename):
f25da3
             # if file does not exist: we need to create it, since the
f25da3
diff --git a/tests/integration_tests/modules/test_ssh_keysfile.py b/tests/integration_tests/modules/test_ssh_keysfile.py
f25da3
index f82d7649..3159feb9 100644
f25da3
--- a/tests/integration_tests/modules/test_ssh_keysfile.py
f25da3
+++ b/tests/integration_tests/modules/test_ssh_keysfile.py
f25da3
@@ -10,10 +10,10 @@ TEST_USER1_KEYS = get_test_rsa_keypair('test1')
f25da3
 TEST_USER2_KEYS = get_test_rsa_keypair('test2')
f25da3
 TEST_DEFAULT_KEYS = get_test_rsa_keypair('test3')
f25da3
 
f25da3
-USERDATA = """\
f25da3
+_USERDATA = """\
f25da3
 #cloud-config
f25da3
 bootcmd:
f25da3
- - sed -i 's;#AuthorizedKeysFile.*;AuthorizedKeysFile /etc/ssh/authorized_keys %h/.ssh/authorized_keys2;' /etc/ssh/sshd_config
f25da3
+ - {bootcmd}
f25da3
 ssh_authorized_keys:
f25da3
  - {default}
f25da3
 users:
f25da3
@@ -24,27 +24,17 @@ users:
f25da3
 - name: test_user2
f25da3
   ssh_authorized_keys:
f25da3
     - {user2}
f25da3
-""".format(  # noqa: E501
f25da3
+""".format(
f25da3
+    bootcmd='{bootcmd}',
f25da3
     default=TEST_DEFAULT_KEYS.public_key,
f25da3
     user1=TEST_USER1_KEYS.public_key,
f25da3
     user2=TEST_USER2_KEYS.public_key,
f25da3
 )
f25da3
 
f25da3
 
f25da3
-@pytest.mark.ubuntu
f25da3
-@pytest.mark.user_data(USERDATA)
f25da3
-def test_authorized_keys(client: IntegrationInstance):
f25da3
-    expected_keys = [
f25da3
-        ('test_user1', '/home/test_user1/.ssh/authorized_keys2',
f25da3
-         TEST_USER1_KEYS),
f25da3
-        ('test_user2', '/home/test_user2/.ssh/authorized_keys2',
f25da3
-         TEST_USER2_KEYS),
f25da3
-        ('ubuntu', '/home/ubuntu/.ssh/authorized_keys2',
f25da3
-         TEST_DEFAULT_KEYS),
f25da3
-        ('root', '/root/.ssh/authorized_keys2', TEST_DEFAULT_KEYS),
f25da3
-    ]
f25da3
-
f25da3
+def common_verify(client, expected_keys):
f25da3
     for user, filename, keys in expected_keys:
f25da3
+        # Ensure key is in the key file
f25da3
         contents = client.read_from_file(filename)
f25da3
         if user in ['ubuntu', 'root']:
f25da3
             # Our personal public key gets added by pycloudlib
f25da3
@@ -83,3 +73,113 @@ def test_authorized_keys(client: IntegrationInstance):
f25da3
                     look_for_keys=False,
f25da3
                     allow_agent=False,
f25da3
                 )
f25da3
+
f25da3
+        # Ensure we haven't messed with any /home permissions
f25da3
+        # See LP: #1940233
f25da3
+        home_dir = '/home/{}'.format(user)
f25da3
+        home_perms = '755'
f25da3
+        if user == 'root':
f25da3
+            home_dir = '/root'
f25da3
+            home_perms = '700'
f25da3
+        assert '{} {}'.format(user, home_perms) == client.execute(
f25da3
+            'stat -c "%U %a" {}'.format(home_dir)
f25da3
+        )
f25da3
+        if client.execute("test -d {}/.ssh".format(home_dir)).ok:
f25da3
+            assert '{} 700'.format(user) == client.execute(
f25da3
+                'stat -c "%U %a" {}/.ssh'.format(home_dir)
f25da3
+            )
f25da3
+        assert '{} 600'.format(user) == client.execute(
f25da3
+            'stat -c "%U %a" {}'.format(filename)
f25da3
+        )
f25da3
+
f25da3
+        # Also ensure ssh-keygen works as expected
f25da3
+        client.execute('mkdir {}/.ssh'.format(home_dir))
f25da3
+        assert client.execute(
f25da3
+            "ssh-keygen -b 2048 -t rsa -f {}/.ssh/id_rsa -q -N ''".format(
f25da3
+                home_dir)
f25da3
+        ).ok
f25da3
+        assert client.execute('test -f {}/.ssh/id_rsa'.format(home_dir))
f25da3
+        assert client.execute('test -f {}/.ssh/id_rsa.pub'.format(home_dir))
f25da3
+
f25da3
+    assert 'root 755' == client.execute('stat -c "%U %a" /home')
f25da3
+
f25da3
+
f25da3
+DEFAULT_KEYS_USERDATA = _USERDATA.format(bootcmd='""')
f25da3
+
f25da3
+
f25da3
+@pytest.mark.ubuntu
f25da3
+@pytest.mark.user_data(DEFAULT_KEYS_USERDATA)
f25da3
+def test_authorized_keys_default(client: IntegrationInstance):
f25da3
+    expected_keys = [
f25da3
+        ('test_user1', '/home/test_user1/.ssh/authorized_keys',
f25da3
+         TEST_USER1_KEYS),
f25da3
+        ('test_user2', '/home/test_user2/.ssh/authorized_keys',
f25da3
+         TEST_USER2_KEYS),
f25da3
+        ('ubuntu', '/home/ubuntu/.ssh/authorized_keys',
f25da3
+         TEST_DEFAULT_KEYS),
f25da3
+        ('root', '/root/.ssh/authorized_keys', TEST_DEFAULT_KEYS),
f25da3
+    ]
f25da3
+    common_verify(client, expected_keys)
f25da3
+
f25da3
+
f25da3
+AUTHORIZED_KEYS2_USERDATA = _USERDATA.format(bootcmd=(
f25da3
+    "sed -i 's;#AuthorizedKeysFile.*;AuthorizedKeysFile "
f25da3
+    "/etc/ssh/authorized_keys %h/.ssh/authorized_keys2;' "
f25da3
+    "/etc/ssh/sshd_config"))
f25da3
+
f25da3
+
f25da3
+@pytest.mark.ubuntu
f25da3
+@pytest.mark.user_data(AUTHORIZED_KEYS2_USERDATA)
f25da3
+def test_authorized_keys2(client: IntegrationInstance):
f25da3
+    expected_keys = [
f25da3
+        ('test_user1', '/home/test_user1/.ssh/authorized_keys2',
f25da3
+         TEST_USER1_KEYS),
f25da3
+        ('test_user2', '/home/test_user2/.ssh/authorized_keys2',
f25da3
+         TEST_USER2_KEYS),
f25da3
+        ('ubuntu', '/home/ubuntu/.ssh/authorized_keys2',
f25da3
+         TEST_DEFAULT_KEYS),
f25da3
+        ('root', '/root/.ssh/authorized_keys2', TEST_DEFAULT_KEYS),
f25da3
+    ]
f25da3
+    common_verify(client, expected_keys)
f25da3
+
f25da3
+
f25da3
+NESTED_KEYS_USERDATA = _USERDATA.format(bootcmd=(
f25da3
+    "sed -i 's;#AuthorizedKeysFile.*;AuthorizedKeysFile "
f25da3
+    "/etc/ssh/authorized_keys %h/foo/bar/ssh/keys;' "
f25da3
+    "/etc/ssh/sshd_config"))
f25da3
+
f25da3
+
f25da3
+@pytest.mark.ubuntu
f25da3
+@pytest.mark.user_data(NESTED_KEYS_USERDATA)
f25da3
+def test_nested_keys(client: IntegrationInstance):
f25da3
+    expected_keys = [
f25da3
+        ('test_user1', '/home/test_user1/foo/bar/ssh/keys',
f25da3
+         TEST_USER1_KEYS),
f25da3
+        ('test_user2', '/home/test_user2/foo/bar/ssh/keys',
f25da3
+         TEST_USER2_KEYS),
f25da3
+        ('ubuntu', '/home/ubuntu/foo/bar/ssh/keys',
f25da3
+         TEST_DEFAULT_KEYS),
f25da3
+        ('root', '/root/foo/bar/ssh/keys', TEST_DEFAULT_KEYS),
f25da3
+    ]
f25da3
+    common_verify(client, expected_keys)
f25da3
+
f25da3
+
f25da3
+EXTERNAL_KEYS_USERDATA = _USERDATA.format(bootcmd=(
f25da3
+    "sed -i 's;#AuthorizedKeysFile.*;AuthorizedKeysFile "
f25da3
+    "/etc/ssh/authorized_keys /etc/ssh/authorized_keys/%u/keys;' "
f25da3
+    "/etc/ssh/sshd_config"))
f25da3
+
f25da3
+
f25da3
+@pytest.mark.ubuntu
f25da3
+@pytest.mark.user_data(EXTERNAL_KEYS_USERDATA)
f25da3
+def test_external_keys(client: IntegrationInstance):
f25da3
+    expected_keys = [
f25da3
+        ('test_user1', '/etc/ssh/authorized_keys/test_user1/keys',
f25da3
+         TEST_USER1_KEYS),
f25da3
+        ('test_user2', '/etc/ssh/authorized_keys/test_user2/keys',
f25da3
+         TEST_USER2_KEYS),
f25da3
+        ('ubuntu', '/etc/ssh/authorized_keys/ubuntu/keys',
f25da3
+         TEST_DEFAULT_KEYS),
f25da3
+        ('root', '/etc/ssh/authorized_keys/root/keys', TEST_DEFAULT_KEYS),
f25da3
+    ]
f25da3
+    common_verify(client, expected_keys)
f25da3
-- 
f25da3
2.27.0
f25da3