diff --git a/SOURCES/ci-rhel-cloud.cfg-remove-ssh_genkeytypes-in-settings.py.patch b/SOURCES/ci-rhel-cloud.cfg-remove-ssh_genkeytypes-in-settings.py.patch new file mode 100644 index 0000000..db5eac3 --- /dev/null +++ b/SOURCES/ci-rhel-cloud.cfg-remove-ssh_genkeytypes-in-settings.py.patch @@ -0,0 +1,64 @@ +From 569a19866bba846bbea8c99b9998336299570783 Mon Sep 17 00:00:00 2001 +From: Emanuele Giuseppe Esposito +Date: Thu, 20 May 2021 08:53:55 +0200 +Subject: [PATCH 1/2] rhel/cloud.cfg: remove ssh_genkeytypes in settings.py and + set in cloud.cfg + +RH-Author: Emanuele Giuseppe Esposito +RH-MergeRequest: 8: rhel/cloud.cfg: remove ssh_genkeytypes in settings.py and set in cloud.cfg +RH-Commit: [1/1] 9c39347a790360bc23c5ea3d8a34d0722d0cd1ac +RH-Bugzilla: 1963981 +RH-Acked-by: Cathy Avery +RH-Acked-by: Vitaly Kuznetsov +RH-Acked-by: Mohamed Gamal Morsy + +Currently genkeytypes in cloud.cfg is set to None, so together with +ssh_deletekeys=1 cloudinit on first boot it will just delete the existing +keys and not generate new ones. + +Just removing that property in cloud.cfg is not enough, because +settings.py provides another empty default value that will be used +instead, resulting to no key generated even when the property is not defined. + +Removing genkeytypes also in settings.py will default to GENERATE_KEY_NAMES, +but since we want only 'rsa', 'ecdsa' and 'ed25519', add back genkeytypes in +cloud.cfg with the above defaults. + +Also remove ssh_deletekeys in settings.py as we always need +to 1 (and it also defaults to 1). + +Signed-off-by: Emanuele Giuseppe Esposito +--- + cloudinit/settings.py | 2 -- + rhel/cloud.cfg | 2 +- + 2 files changed, 1 insertion(+), 3 deletions(-) + +diff --git a/cloudinit/settings.py b/cloudinit/settings.py +index 439eee02..87398eec 100644 +--- a/cloudinit/settings.py ++++ b/cloudinit/settings.py +@@ -48,8 +48,6 @@ CFG_BUILTIN = { + 'def_log_file_mode': 0o600, + 'log_cfgs': [], + 'mount_default_fields': [None, None, 'auto', 'defaults,nofail', '0', '2'], +- 'ssh_deletekeys': False, +- 'ssh_genkeytypes': [], + 'syslog_fix_perms': [], + 'system_info': { + 'paths': { +diff --git a/rhel/cloud.cfg b/rhel/cloud.cfg +index 9ecba215..cbee197a 100644 +--- a/rhel/cloud.cfg ++++ b/rhel/cloud.cfg +@@ -7,7 +7,7 @@ ssh_pwauth: 0 + mount_default_fields: [~, ~, 'auto', 'defaults,nofail,x-systemd.requires=cloud-init.service', '0', '2'] + resize_rootfs_tmp: /dev + ssh_deletekeys: 1 +-ssh_genkeytypes: ~ ++ssh_genkeytypes: ['rsa', 'ecdsa', 'ed25519'] + syslog_fix_perms: ~ + disable_vmware_customization: false + +-- +2.27.0 + diff --git a/SOURCES/ci-write-passwords-only-to-serial-console-lock-down-clo.patch b/SOURCES/ci-write-passwords-only-to-serial-console-lock-down-clo.patch new file mode 100644 index 0000000..4eb8117 --- /dev/null +++ b/SOURCES/ci-write-passwords-only-to-serial-console-lock-down-clo.patch @@ -0,0 +1,378 @@ +From 4c38c004b016a4f0f255af8a779a58e549068967 Mon Sep 17 00:00:00 2001 +From: Eduardo Otubo +Date: Mon, 5 Jul 2021 14:19:06 +0200 +Subject: [PATCH] write passwords only to serial console, lock down + cloud-init-output.log (#847) + +RH-Author: Eduardo Otubo +RH-MergeRequest: 22: write passwords only to serial console, lock down cloud-init-output.log (#847) +RH-Commit: [1/1] db5da651f305f65f518a548bb57c4977b17b4070 (otubo/cloud-init) +RH-Bugzilla: 1979252 +RH-Acked-by: Emanuele Giuseppe Esposito +RH-Acked-by: Mohamed Gamal Morsy + +commit b794d426b9ab43ea9d6371477466070d86e10668 +Author: Daniel Watkins +Date: Fri Mar 19 10:06:42 2021 -0400 + + write passwords only to serial console, lock down cloud-init-output.log (#847) + + Prior to this commit, when a user specified configuration which would + generate random passwords for users, cloud-init would cause those + passwords to be written to the serial console by emitting them on + stderr. In the default configuration, any stdout or stderr emitted by + cloud-init is also written to `/var/log/cloud-init-output.log`. This + file is world-readable, meaning that those randomly-generated passwords + were available to be read by any user with access to the system. This + presents an obvious security issue. + + This commit responds to this issue in two ways: + + * We address the direct issue by moving from writing the passwords to + sys.stderr to writing them directly to /dev/console (via + util.multi_log); this means that the passwords will never end up in + cloud-init-output.log + * To avoid future issues like this, we also modify the logging code so + that any files created in a log sink subprocess will only be + owner/group readable and, if it exists, will be owned by the adm + group. This results in `/var/log/cloud-init-output.log` no longer + being world-readable, meaning that if there are other parts of the + codebase that are emitting sensitive data intended for the serial + console, that data is no longer available to all users of the system. + + LP: #1918303 + +Signed-off-by: Eduardo Otubo +--- + cloudinit/config/cc_set_passwords.py | 5 +- + cloudinit/config/tests/test_set_passwords.py | 40 ++++++-- + cloudinit/tests/test_util.py | 101 +++++++++++++++++++ + cloudinit/util.py | 38 ++++++- + tests/integration_tests/test_logging.py | 22 ++++ + tests/unittests/test_util.py | 4 + + 6 files changed, 194 insertions(+), 16 deletions(-) + create mode 100644 tests/integration_tests/test_logging.py + +diff --git a/cloudinit/config/cc_set_passwords.py b/cloudinit/config/cc_set_passwords.py +index d6b5682d..433de751 100755 +--- a/cloudinit/config/cc_set_passwords.py ++++ b/cloudinit/config/cc_set_passwords.py +@@ -78,7 +78,6 @@ password. + """ + + import re +-import sys + + from cloudinit.distros import ug_util + from cloudinit import log as logging +@@ -214,7 +213,9 @@ def handle(_name, cfg, cloud, log, args): + if len(randlist): + blurb = ("Set the following 'random' passwords\n", + '\n'.join(randlist)) +- sys.stderr.write("%s\n%s\n" % blurb) ++ util.multi_log( ++ "%s\n%s\n" % blurb, stderr=False, fallback_to_stdout=False ++ ) + + if expire: + expired_users = [] +diff --git a/cloudinit/config/tests/test_set_passwords.py b/cloudinit/config/tests/test_set_passwords.py +index daa1ef51..bbe2ee8f 100644 +--- a/cloudinit/config/tests/test_set_passwords.py ++++ b/cloudinit/config/tests/test_set_passwords.py +@@ -74,10 +74,6 @@ class TestSetPasswordsHandle(CiTestCase): + + with_logs = True + +- def setUp(self): +- super(TestSetPasswordsHandle, self).setUp() +- self.add_patch('cloudinit.config.cc_set_passwords.sys.stderr', 'm_err') +- + def test_handle_on_empty_config(self, *args): + """handle logs that no password has changed when config is empty.""" + cloud = self.tmp_cloud(distro='ubuntu') +@@ -129,10 +125,12 @@ class TestSetPasswordsHandle(CiTestCase): + mock.call(['pw', 'usermod', 'ubuntu', '-p', '01-Jan-1970'])], + m_subp.call_args_list) + ++ @mock.patch(MODPATH + "util.multi_log") + @mock.patch(MODPATH + "util.is_BSD") + @mock.patch(MODPATH + "subp.subp") +- def test_handle_on_chpasswd_list_creates_random_passwords(self, m_subp, +- m_is_bsd): ++ def test_handle_on_chpasswd_list_creates_random_passwords( ++ self, m_subp, m_is_bsd, m_multi_log ++ ): + """handle parses command set random passwords.""" + m_is_bsd.return_value = False + cloud = self.tmp_cloud(distro='ubuntu') +@@ -146,10 +144,32 @@ class TestSetPasswordsHandle(CiTestCase): + self.assertIn( + 'DEBUG: Handling input for chpasswd as list.', + self.logs.getvalue()) +- self.assertNotEqual( +- [mock.call(['chpasswd'], +- '\n'.join(valid_random_pwds) + '\n')], +- m_subp.call_args_list) ++ ++ self.assertEqual(1, m_subp.call_count) ++ args, _kwargs = m_subp.call_args ++ self.assertEqual(["chpasswd"], args[0]) ++ ++ stdin = args[1] ++ user_pass = { ++ user: password ++ for user, password ++ in (line.split(":") for line in stdin.splitlines()) ++ } ++ ++ self.assertEqual(1, m_multi_log.call_count) ++ self.assertEqual( ++ mock.call(mock.ANY, stderr=False, fallback_to_stdout=False), ++ m_multi_log.call_args ++ ) ++ ++ self.assertEqual(set(["root", "ubuntu"]), set(user_pass.keys())) ++ written_lines = m_multi_log.call_args[0][0].splitlines() ++ for password in user_pass.values(): ++ for line in written_lines: ++ if password in line: ++ break ++ else: ++ self.fail("Password not emitted to console") + + + # vi: ts=4 expandtab +diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py +index 096a3037..b8dfbf51 100644 +--- a/cloudinit/tests/test_util.py ++++ b/cloudinit/tests/test_util.py +@@ -771,4 +771,105 @@ class TestMountCb: + ] == callback.call_args_list + + ++@mock.patch("cloudinit.util.write_file") ++class TestEnsureFile: ++ """Tests for ``cloudinit.util.ensure_file``.""" ++ ++ def test_parameters_passed_through(self, m_write_file): ++ """Test the parameters in the signature are passed to write_file.""" ++ util.ensure_file( ++ mock.sentinel.path, ++ mode=mock.sentinel.mode, ++ preserve_mode=mock.sentinel.preserve_mode, ++ ) ++ ++ assert 1 == m_write_file.call_count ++ args, kwargs = m_write_file.call_args ++ assert (mock.sentinel.path,) == args ++ assert mock.sentinel.mode == kwargs["mode"] ++ assert mock.sentinel.preserve_mode == kwargs["preserve_mode"] ++ ++ @pytest.mark.parametrize( ++ "kwarg,expected", ++ [ ++ # Files should be world-readable by default ++ ("mode", 0o644), ++ # The previous behaviour of not preserving mode should be retained ++ ("preserve_mode", False), ++ ], ++ ) ++ def test_defaults(self, m_write_file, kwarg, expected): ++ """Test that ensure_file defaults appropriately.""" ++ util.ensure_file(mock.sentinel.path) ++ ++ assert 1 == m_write_file.call_count ++ _args, kwargs = m_write_file.call_args ++ assert expected == kwargs[kwarg] ++ ++ def test_static_parameters_are_passed(self, m_write_file): ++ """Test that the static write_files parameters are passed correctly.""" ++ util.ensure_file(mock.sentinel.path) ++ ++ assert 1 == m_write_file.call_count ++ _args, kwargs = m_write_file.call_args ++ assert "" == kwargs["content"] ++ assert "ab" == kwargs["omode"] ++ ++ ++@mock.patch("cloudinit.util.grp.getgrnam") ++@mock.patch("cloudinit.util.os.setgid") ++@mock.patch("cloudinit.util.os.umask") ++class TestRedirectOutputPreexecFn: ++ """This tests specifically the preexec_fn used in redirect_output.""" ++ ++ @pytest.fixture(params=["outfmt", "errfmt"]) ++ def preexec_fn(self, request): ++ """A fixture to gather the preexec_fn used by redirect_output. ++ ++ This enables simpler direct testing of it, and parameterises any tests ++ using it to cover both the stdout and stderr code paths. ++ """ ++ test_string = "| piped output to invoke subprocess" ++ if request.param == "outfmt": ++ args = (test_string, None) ++ elif request.param == "errfmt": ++ args = (None, test_string) ++ with mock.patch("cloudinit.util.subprocess.Popen") as m_popen: ++ util.redirect_output(*args) ++ ++ assert 1 == m_popen.call_count ++ _args, kwargs = m_popen.call_args ++ assert "preexec_fn" in kwargs, "preexec_fn not passed to Popen" ++ return kwargs["preexec_fn"] ++ ++ def test_preexec_fn_sets_umask( ++ self, m_os_umask, _m_setgid, _m_getgrnam, preexec_fn ++ ): ++ """preexec_fn should set a mask that avoids world-readable files.""" ++ preexec_fn() ++ ++ assert [mock.call(0o037)] == m_os_umask.call_args_list ++ ++ def test_preexec_fn_sets_group_id_if_adm_group_present( ++ self, _m_os_umask, m_setgid, m_getgrnam, preexec_fn ++ ): ++ """We should setgrp to adm if present, so files are owned by them.""" ++ fake_group = mock.Mock(gr_gid=mock.sentinel.gr_gid) ++ m_getgrnam.return_value = fake_group ++ ++ preexec_fn() ++ ++ assert [mock.call("adm")] == m_getgrnam.call_args_list ++ assert [mock.call(mock.sentinel.gr_gid)] == m_setgid.call_args_list ++ ++ def test_preexec_fn_handles_absent_adm_group_gracefully( ++ self, _m_os_umask, m_setgid, m_getgrnam, preexec_fn ++ ): ++ """We should handle an absent adm group gracefully.""" ++ m_getgrnam.side_effect = KeyError("getgrnam(): name not found: 'adm'") ++ ++ preexec_fn() ++ ++ assert 0 == m_setgid.call_count ++ + # vi: ts=4 expandtab +diff --git a/cloudinit/util.py b/cloudinit/util.py +index cf9e349f..94cec6ee 100644 +--- a/cloudinit/util.py ++++ b/cloudinit/util.py +@@ -391,7 +391,7 @@ def find_modules(root_dir): + + + def multi_log(text, console=True, stderr=True, +- log=None, log_level=logging.DEBUG): ++ log=None, log_level=logging.DEBUG, fallback_to_stdout=True): + if stderr: + sys.stderr.write(text) + if console: +@@ -400,7 +400,7 @@ def multi_log(text, console=True, stderr=True, + with open(conpath, 'w') as wfh: + wfh.write(text) + wfh.flush() +- else: ++ elif fallback_to_stdout: + # A container may lack /dev/console (arguably a container bug). If + # it does not exist, then write output to stdout. this will result + # in duplicate stderr and stdout messages if stderr was True. +@@ -650,6 +650,26 @@ def redirect_output(outfmt, errfmt, o_out=None, o_err=None): + if not o_err: + o_err = sys.stderr + ++ # pylint: disable=subprocess-popen-preexec-fn ++ def set_subprocess_umask_and_gid(): ++ """Reconfigure umask and group ID to create output files securely. ++ ++ This is passed to subprocess.Popen as preexec_fn, so it is executed in ++ the context of the newly-created process. It: ++ ++ * sets the umask of the process so created files aren't world-readable ++ * if an adm group exists in the system, sets that as the process' GID ++ (so that the created file(s) are owned by root:adm) ++ """ ++ os.umask(0o037) ++ try: ++ group_id = grp.getgrnam("adm").gr_gid ++ except KeyError: ++ # No adm group, don't set a group ++ pass ++ else: ++ os.setgid(group_id) ++ + if outfmt: + LOG.debug("Redirecting %s to %s", o_out, outfmt) + (mode, arg) = outfmt.split(" ", 1) +@@ -659,7 +679,12 @@ def redirect_output(outfmt, errfmt, o_out=None, o_err=None): + owith = "wb" + new_fp = open(arg, owith) + elif mode == "|": +- proc = subprocess.Popen(arg, shell=True, stdin=subprocess.PIPE) ++ proc = subprocess.Popen( ++ arg, ++ shell=True, ++ stdin=subprocess.PIPE, ++ preexec_fn=set_subprocess_umask_and_gid, ++ ) + new_fp = proc.stdin + else: + raise TypeError("Invalid type for output format: %s" % outfmt) +@@ -681,7 +706,12 @@ def redirect_output(outfmt, errfmt, o_out=None, o_err=None): + owith = "wb" + new_fp = open(arg, owith) + elif mode == "|": +- proc = subprocess.Popen(arg, shell=True, stdin=subprocess.PIPE) ++ proc = subprocess.Popen( ++ arg, ++ shell=True, ++ stdin=subprocess.PIPE, ++ preexec_fn=set_subprocess_umask_and_gid, ++ ) + new_fp = proc.stdin + else: + raise TypeError("Invalid type for error format: %s" % errfmt) +diff --git a/tests/integration_tests/test_logging.py b/tests/integration_tests/test_logging.py +new file mode 100644 +index 00000000..b31a0434 +--- /dev/null ++++ b/tests/integration_tests/test_logging.py +@@ -0,0 +1,22 @@ ++"""Integration tests relating to cloud-init's logging.""" ++ ++ ++class TestVarLogCloudInitOutput: ++ """Integration tests relating to /var/log/cloud-init-output.log.""" ++ ++ def test_var_log_cloud_init_output_not_world_readable(self, client): ++ """ ++ The log can contain sensitive data, it shouldn't be world-readable. ++ ++ LP: #1918303 ++ """ ++ # Check the file exists ++ assert client.execute("test -f /var/log/cloud-init-output.log").ok ++ ++ # Check its permissions are as we expect ++ perms, user, group = client.execute( ++ "stat -c %a:%U:%G /var/log/cloud-init-output.log" ++ ).split(":") ++ assert "640" == perms ++ assert "root" == user ++ assert "adm" == group +diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py +index fc557469..9c51768d 100644 +--- a/tests/unittests/test_util.py ++++ b/tests/unittests/test_util.py +@@ -695,6 +695,10 @@ class TestMultiLog(helpers.FilesystemMockingTestCase): + util.multi_log(logged_string) + self.assertEqual(logged_string, self.stdout.getvalue()) + ++ def test_logs_dont_go_to_stdout_if_fallback_to_stdout_is_false(self): ++ util.multi_log('something', fallback_to_stdout=False) ++ self.assertEqual('', self.stdout.getvalue()) ++ + def test_logs_go_to_log_if_given(self): + log = mock.MagicMock() + logged_string = 'something very important' +-- +2.27.0 + diff --git a/SPECS/cloud-init.spec b/SPECS/cloud-init.spec index da84b6e..f4ac45e 100644 --- a/SPECS/cloud-init.spec +++ b/SPECS/cloud-init.spec @@ -6,7 +6,7 @@ Name: cloud-init Version: 20.3 -Release: 10%{?dist}.3 +Release: 10%{?dist}.5 Summary: Cloud instance init scripts Group: System Environment/Base @@ -46,6 +46,10 @@ Patch18: ci-Fix-requiring-device-number-on-EC2-derivatives-836.patch Patch19: ci-get_interfaces-don-t-exclude-Open-vSwitch-bridge-bon.patch # For bz#1957135 - Intermittent failure to start cloud-init due to failure to detect macs [rhel-8.4.0.z] Patch20: ci-net-exclude-OVS-internal-interfaces-in-get_interface.patch +# For bz#1963981 - [cloud-init] From RHEL 82+ cloud-init no longer displays sshd keys fingerprints from instance launched from a backup image [rhel-8.4.0.z] +Patch21: ci-rhel-cloud.cfg-remove-ssh_genkeytypes-in-settings.py.patch +# For bz#1979252 - CVE-2021-3429 cloud-init: randomly generated passwords logged in clear-text to world-readable file [rhel-8] [rhel-8.4.0.z] +Patch22: ci-write-passwords-only-to-serial-console-lock-down-clo.patch BuildArch: noarch @@ -199,8 +203,7 @@ if [ $1 -eq 0 ] ; then fi %postun -%systemd_postun - +%systemd_postun cloud-config.service cloud-config.target cloud-final.service cloud-init.service cloud-init.target cloud-init-local.service %files %license LICENSE @@ -237,6 +240,19 @@ fi %config(noreplace) %{_sysconfdir}/rsyslog.d/21-cloudinit.conf %changelog +* Thu Jul 15 2021 Miroslav Rezanina - 20.3-10.el8_4.5 +- ci-write-passwords-only-to-serial-console-lock-down-clo.patch [bz#1979252] +- Resolves: bz#1979252 + (CVE-2021-3429 cloud-init: randomly generated passwords logged in clear-text to world-readable file [rhel-8] [rhel-8.4.0.z]) + +* Mon Jun 07 2021 Miroslav Rezanina - 20.3-10.el8_4.4 +- ci-rhel-cloud.cfg-remove-ssh_genkeytypes-in-settings.py.patch [bz#1963981] +- ci-cloud-init.spec.template-update-systemd_postun-param.patch [bz#1967600] +- Resolves: bz#1963981 + ([cloud-init] From RHEL 82+ cloud-init no longer displays sshd keys fingerprints from instance launched from a backup image [rhel-8.4.0.z]) +- Resolves: bz#1967600 + (cloud-init brew build fails on Fedora 33 [rhel-8.4.0.z]) + * Thu May 13 2021 Miroslav Rezanina - 20.3-10.el8_4.3 - ci-get_interfaces-don-t-exclude-Open-vSwitch-bridge-bon.patch [bz#1957135] - ci-net-exclude-OVS-internal-interfaces-in-get_interface.patch [bz#1957135]