Fix home permissions modified by ssh module (SC-338)

cloudinit/ssh_util.py

@@ -321,23 +321,48 @@ def check_create_path(username, filename, strictmodes):
         home_folder = os.path.dirname(user_pwent.pw_dir)
         for directory in directories:
             parent_folder += "/" + directory
-            if home_folder.startswith(parent_folder):
+
+            # security check, disallow symlinks in the AuthorizedKeysFile path.
+            if os.path.islink(parent_folder):
+                LOG.debug(
+                    "Invalid directory. Symlink exists in path: %s",
+                    parent_folder)
+                return False
+
+            if os.path.isfile(parent_folder):
+                LOG.debug(
+                    "Invalid directory. File exists in path: %s",
+                    parent_folder)
+                return False
+
+            if (home_folder.startswith(parent_folder) or
+                    parent_folder == user_pwent.pw_dir):
                 continue
 
-            if not os.path.isdir(parent_folder):
+            if not os.path.exists(parent_folder):
                 # directory does not exist, and permission so far are good:
                 # create the directory, and make it accessible by everyone
                 # but owned by root, as it might be used by many users.
                 with util.SeLinuxGuard(parent_folder):
-                    os.makedirs(parent_folder, mode=0o755, exist_ok=True)
-                    util.chownbyid(parent_folder, root_pwent.pw_uid,
-                                   root_pwent.pw_gid)
+                    mode = 0o755
+                    uid = root_pwent.pw_uid
+                    gid = root_pwent.pw_gid
+                    if parent_folder.startswith(user_pwent.pw_dir):
+                        mode = 0o700
+                        uid = user_pwent.pw_uid
+                        gid = user_pwent.pw_gid
+                    os.makedirs(parent_folder, mode=mode, exist_ok=True)
+                    util.chownbyid(parent_folder, uid, gid)
 
             permissions = check_permissions(username, parent_folder,
                                             filename, False, strictmodes)
             if not permissions:
                 return False
 
+        if os.path.islink(filename) or os.path.isdir(filename):
+            LOG.debug("%s is not a file!", filename)
+            return False
+
         # check the file
         if not os.path.exists(filename):
             # if file does not exist: we need to create it, since the

tests/integration_tests/modules/test_ssh_keysfile.py

@@ -10,10 +10,10 @@
 TEST_USER2_KEYS = get_test_rsa_keypair('test2')
 TEST_DEFAULT_KEYS = get_test_rsa_keypair('test3')
 
-USERDATA = """\
+_USERDATA = """\
 #cloud-config
 bootcmd:
- - sed -i 's;#AuthorizedKeysFile.*;AuthorizedKeysFile /etc/ssh/authorized_keys %h/.ssh/authorized_keys2;' /etc/ssh/sshd_config
+ - {bootcmd}
 ssh_authorized_keys:
  - {default}
 users:
@@ -24,27 +24,17 @@
 - name: test_user2
   ssh_authorized_keys:
     - {user2}
-""".format(  # noqa: E501
+""".format(
+    bootcmd='{bootcmd}',
     default=TEST_DEFAULT_KEYS.public_key,
     user1=TEST_USER1_KEYS.public_key,
     user2=TEST_USER2_KEYS.public_key,
 )
 
 
-@pytest.mark.ubuntu
-@pytest.mark.user_data(USERDATA)
-def test_authorized_keys(client: IntegrationInstance):
-    expected_keys = [
-        ('test_user1', '/home/test_user1/.ssh/authorized_keys2',
-         TEST_USER1_KEYS),
-        ('test_user2', '/home/test_user2/.ssh/authorized_keys2',
-         TEST_USER2_KEYS),
-        ('ubuntu', '/home/ubuntu/.ssh/authorized_keys2',
-         TEST_DEFAULT_KEYS),
-        ('root', '/root/.ssh/authorized_keys2', TEST_DEFAULT_KEYS),
-    ]
-
+def common_verify(client, expected_keys):
     for user, filename, keys in expected_keys:
+        # Ensure key is in the key file
         contents = client.read_from_file(filename)
         if user in ['ubuntu', 'root']:
             # Our personal public key gets added by pycloudlib
@@ -83,3 +73,113 @@ def test_authorized_keys(client: IntegrationInstance):
                     look_for_keys=False,
                     allow_agent=False,
                 )
+
+        # Ensure we haven't messed with any /home permissions
+        # See LP: #1940233
+        home_dir = '/home/{}'.format(user)
+        home_perms = '755'
+        if user == 'root':
+            home_dir = '/root'
+            home_perms = '700'
+        assert '{} {}'.format(user, home_perms) == client.execute(
+            'stat -c "%U %a" {}'.format(home_dir)
+        )
+        if client.execute("test -d {}/.ssh".format(home_dir)).ok:
+            assert '{} 700'.format(user) == client.execute(
+                'stat -c "%U %a" {}/.ssh'.format(home_dir)
+            )
+        assert '{} 600'.format(user) == client.execute(
+            'stat -c "%U %a" {}'.format(filename)
+        )
+
+        # Also ensure ssh-keygen works as expected
+        client.execute('mkdir {}/.ssh'.format(home_dir))
+        assert client.execute(
+            "ssh-keygen -b 2048 -t rsa -f {}/.ssh/id_rsa -q -N ''".format(
+                home_dir)
+        ).ok
+        assert client.execute('test -f {}/.ssh/id_rsa'.format(home_dir))
+        assert client.execute('test -f {}/.ssh/id_rsa.pub'.format(home_dir))
+
+    assert 'root 755' == client.execute('stat -c "%U %a" /home')
+
+
+DEFAULT_KEYS_USERDATA = _USERDATA.format(bootcmd='""')
+
+
+@pytest.mark.ubuntu
+@pytest.mark.user_data(DEFAULT_KEYS_USERDATA)
+def test_authorized_keys_default(client: IntegrationInstance):
+    expected_keys = [
+        ('test_user1', '/home/test_user1/.ssh/authorized_keys',
+         TEST_USER1_KEYS),
+        ('test_user2', '/home/test_user2/.ssh/authorized_keys',
+         TEST_USER2_KEYS),
+        ('ubuntu', '/home/ubuntu/.ssh/authorized_keys',
+         TEST_DEFAULT_KEYS),
+        ('root', '/root/.ssh/authorized_keys', TEST_DEFAULT_KEYS),
+    ]
+    common_verify(client, expected_keys)
+
+
+AUTHORIZED_KEYS2_USERDATA = _USERDATA.format(bootcmd=(
+    "sed -i 's;#AuthorizedKeysFile.*;AuthorizedKeysFile "
+    "/etc/ssh/authorized_keys %h/.ssh/authorized_keys2;' "
+    "/etc/ssh/sshd_config"))
+
+
+@pytest.mark.ubuntu
+@pytest.mark.user_data(AUTHORIZED_KEYS2_USERDATA)
+def test_authorized_keys2(client: IntegrationInstance):
+    expected_keys = [
+        ('test_user1', '/home/test_user1/.ssh/authorized_keys2',
+         TEST_USER1_KEYS),
+        ('test_user2', '/home/test_user2/.ssh/authorized_keys2',
+         TEST_USER2_KEYS),
+        ('ubuntu', '/home/ubuntu/.ssh/authorized_keys2',
+         TEST_DEFAULT_KEYS),
+        ('root', '/root/.ssh/authorized_keys2', TEST_DEFAULT_KEYS),
+    ]
+    common_verify(client, expected_keys)
+
+
+NESTED_KEYS_USERDATA = _USERDATA.format(bootcmd=(
+    "sed -i 's;#AuthorizedKeysFile.*;AuthorizedKeysFile "
+    "/etc/ssh/authorized_keys %h/foo/bar/ssh/keys;' "
+    "/etc/ssh/sshd_config"))
+
+
+@pytest.mark.ubuntu
+@pytest.mark.user_data(NESTED_KEYS_USERDATA)
+def test_nested_keys(client: IntegrationInstance):
+    expected_keys = [
+        ('test_user1', '/home/test_user1/foo/bar/ssh/keys',
+         TEST_USER1_KEYS),
+        ('test_user2', '/home/test_user2/foo/bar/ssh/keys',
+         TEST_USER2_KEYS),
+        ('ubuntu', '/home/ubuntu/foo/bar/ssh/keys',
+         TEST_DEFAULT_KEYS),
+        ('root', '/root/foo/bar/ssh/keys', TEST_DEFAULT_KEYS),
+    ]
+    common_verify(client, expected_keys)
+
+
+EXTERNAL_KEYS_USERDATA = _USERDATA.format(bootcmd=(
+    "sed -i 's;#AuthorizedKeysFile.*;AuthorizedKeysFile "
+    "/etc/ssh/authorized_keys /etc/ssh/authorized_keys/%u/keys;' "
+    "/etc/ssh/sshd_config"))
+
+
+@pytest.mark.ubuntu
+@pytest.mark.user_data(EXTERNAL_KEYS_USERDATA)
+def test_external_keys(client: IntegrationInstance):
+    expected_keys = [
+        ('test_user1', '/etc/ssh/authorized_keys/test_user1/keys',
+         TEST_USER1_KEYS),
+        ('test_user2', '/etc/ssh/authorized_keys/test_user2/keys',
+         TEST_USER2_KEYS),
+        ('ubuntu', '/etc/ssh/authorized_keys/ubuntu/keys',
+         TEST_DEFAULT_KEYS),
+        ('root', '/etc/ssh/authorized_keys/root/keys', TEST_DEFAULT_KEYS),
+    ]
+    common_verify(client, expected_keys)