263 lines
9.8 KiB
Diff
263 lines
9.8 KiB
Diff
From 5bfe2ee2b063d87e6fd255d6c5e63123aa3f6de0 Mon Sep 17 00:00:00 2001
|
|
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
|
|
Date: Sat, 21 Aug 2021 13:55:53 +0200
|
|
Subject: [PATCH] Fix home permissions modified by ssh module (SC-338) (#984)
|
|
|
|
RH-Author: Emanuele Giuseppe Esposito <eesposit@redhat.com>
|
|
RH-MergeRequest: 9: Fix home permissions modified by ssh module
|
|
RH-Commit: [1/1] ab55db88aa1bf2f77acaca5e76ffabbab72b1fb2 (eesposit/cloud-init-centos-)
|
|
RH-Bugzilla: 1995843
|
|
RH-Acked-by: Mohamed Gamal Morsy <mmorsy@redhat.com>
|
|
RH-Acked-by: Eduardo Otubo <otubo@redhat.com>
|
|
|
|
TESTED: By me and QA
|
|
BREW: 39178085
|
|
|
|
Fix home permissions modified by ssh module (SC-338) (#984)
|
|
|
|
commit 7d3f5d750f6111c2716143364ea33486df67c927
|
|
Author: James Falcon <therealfalcon@gmail.com>
|
|
Date: Fri Aug 20 17:09:49 2021 -0500
|
|
|
|
Fix home permissions modified by ssh module (SC-338) (#984)
|
|
|
|
Fix home permissions modified by ssh module
|
|
|
|
In #956, we updated the file and directory permissions for keys not in
|
|
the user's home directory. We also unintentionally modified the
|
|
permissions within the home directory as well. These should not change,
|
|
and this commit changes that back.
|
|
|
|
LP: #1940233
|
|
|
|
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
|
|
---
|
|
cloudinit/ssh_util.py | 35 ++++-
|
|
.../modules/test_ssh_keysfile.py | 132 +++++++++++++++---
|
|
2 files changed, 146 insertions(+), 21 deletions(-)
|
|
|
|
diff --git a/cloudinit/ssh_util.py b/cloudinit/ssh_util.py
|
|
index b8a3c8f7..9ccadf09 100644
|
|
--- a/cloudinit/ssh_util.py
|
|
+++ b/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
|
|
diff --git a/tests/integration_tests/modules/test_ssh_keysfile.py b/tests/integration_tests/modules/test_ssh_keysfile.py
|
|
index f82d7649..3159feb9 100644
|
|
--- a/tests/integration_tests/modules/test_ssh_keysfile.py
|
|
+++ b/tests/integration_tests/modules/test_ssh_keysfile.py
|
|
@@ -10,10 +10,10 @@ TEST_USER1_KEYS = get_test_rsa_keypair('test1')
|
|
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 @@ users:
|
|
- 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)
|
|
--
|
|
2.27.0
|
|
|