Package: release.debian.org Severity: normal User: release.debian....@packages.debian.org Usertags: unblock
Please unblock package cloud-init cloud-init 20.4.1-2 includes a targeted fix for bug #985540 (CVE-2021-3429). The fix was cherry-picked with minimal modifications from upstream's git repository and has been tested to validate the expected change in behavior (passwords are no longer logged to world-readable files). debdiff against the current bullseye version is attached. unblock cloud-init/20.4.1-2
diff -Nru cloud-init-20.4.1/debian/changelog cloud-init-20.4.1/debian/changelog --- cloud-init-20.4.1/debian/changelog 2021-01-19 10:27:39.000000000 -0800 +++ cloud-init-20.4.1/debian/changelog 2021-03-19 09:18:59.000000000 -0700 @@ -1,3 +1,10 @@ +cloud-init (20.4.1-2) unstable; urgency=high + + * Avoid logging generated passwords to world-readable log files. + CVE-2021-3429. (Closes: #985540) + + -- Noah Meyerhans <no...@debian.org> Fri, 19 Mar 2021 09:18:59 -0700 + cloud-init (20.4.1-1) unstable; urgency=medium * d/watch: switch upstream to github diff -Nru cloud-init-20.4.1/debian/patches/dont_log_generated_passwords.patch cloud-init-20.4.1/debian/patches/dont_log_generated_passwords.patch --- cloud-init-20.4.1/debian/patches/dont_log_generated_passwords.patch 1969-12-31 16:00:00.000000000 -0800 +++ cloud-init-20.4.1/debian/patches/dont_log_generated_passwords.patch 2021-03-19 09:18:59.000000000 -0700 @@ -0,0 +1,316 @@ +Description: Don't log generated passwords +Origin: upstream +Bug-Debian: https://bugs.debian.org/985540 +Applied-Upstream: https://github.com/canonical/cloud-init/commit/b794d426b9ab43ea9d6371477466070d86e10668 +--- +This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ +diff --git a/cloudinit/config/cc_set_passwords.py b/cloudinit/config/cc_set_passwords.py +index d6b5682db..433de751f 100755 +--- a/cloudinit/config/cc_set_passwords.py ++++ b/cloudinit/config/cc_set_passwords.py +@@ -78,7 +78,6 @@ + """ + + 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 daa1ef518..bbe2ee8fa 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 @@ def test_bsd_calls_custom_pw_cmds_to_set_and_expire_passwords( + 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 @@ def test_handle_on_chpasswd_list_creates_random_passwords(self, m_subp, + 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 b7a302f1c..e811917e0 100644 +--- a/cloudinit/tests/test_util.py ++++ b/cloudinit/tests/test_util.py +@@ -851,4 +851,60 @@ def test_static_parameters_are_passed(self, m_write_file): + 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 769f3425e..4e0a72db8 100644 +--- a/cloudinit/util.py ++++ b/cloudinit/util.py +@@ -359,7 +359,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: +@@ -368,7 +368,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. +@@ -623,6 +623,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) +@@ -632,7 +652,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) +@@ -654,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 error format: %s" % errfmt) +diff --git a/tests/integration_tests/modules/test_set_password.py b/tests/integration_tests/modules/test_set_password.py +index b13f76fbf..d7cf91a57 100644 +--- a/tests/integration_tests/modules/test_set_password.py ++++ b/tests/integration_tests/modules/test_set_password.py +@@ -116,6 +116,30 @@ def test_random_passwords_set_correctly(self, class_client): + # Which are not the same + assert shadow_users["harry"] != shadow_users["dick"] + ++ def test_random_passwords_not_stored_in_cloud_init_output_log( ++ self, class_client ++ ): ++ """We should not emit passwords to the in-instance log file. ++ ++ LP: #1918303 ++ """ ++ cloud_init_output = class_client.read_from_file( ++ "/var/log/cloud-init-output.log" ++ ) ++ assert "dick:" not in cloud_init_output ++ assert "harry:" not in cloud_init_output ++ ++ def test_random_passwords_emitted_to_serial_console(self, class_client): ++ """We should emit passwords to the serial console. (LP: #1918303)""" ++ try: ++ console_log = class_client.instance.console_log() ++ except NotImplementedError: ++ # Assume that an exception here means that we can't use the console ++ # log ++ pytest.skip("NotImplementedError when requesting console log") ++ assert "dick:" in console_log ++ assert "harry:" in console_log ++ + def test_explicit_password_set_correctly(self, class_client): + """Test that an explicitly-specified password is set correctly.""" + shadow_users, _ = self._fetch_and_parse_etc_shadow(class_client) +diff --git a/tests/integration_tests/test_logging.py b/tests/integration_tests/test_logging.py +new file mode 100644 +index 000000000..b31a04348 +--- /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 857629f1c..e52920010 100644 +--- a/tests/unittests/test_util.py ++++ b/tests/unittests/test_util.py +@@ -572,6 +572,10 @@ def test_logs_go_to_stdout_if_console_does_not_exist(self): + 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' diff -Nru cloud-init-20.4.1/debian/patches/series cloud-init-20.4.1/debian/patches/series --- cloud-init-20.4.1/debian/patches/series 2020-11-25 16:07:55.000000000 -0800 +++ cloud-init-20.4.1/debian/patches/series 2021-03-19 09:02:44.000000000 -0700 @@ -5,3 +5,4 @@ cloud-init-before-chronyd.patch 0009-Drop-all-unused-extended-version-handling.patch 0012-Fix-message-when-a-local-is-missing.patch +dont_log_generated_passwords.patch
