diff options
author | Daniel Watkins <oddbloke@ubuntu.com> | 2021-03-19 10:06:42 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-19 10:06:42 -0400 |
commit | b794d426b9ab43ea9d6371477466070d86e10668 (patch) | |
tree | 11e19cd3e8db36dee151da4933e5429b18660268 /cloudinit/config | |
parent | c6726c2bbe82b738bd0a7fb308496a497c797d5f (diff) | |
download | vyos-cloud-init-b794d426b9ab43ea9d6371477466070d86e10668.tar.gz vyos-cloud-init-b794d426b9ab43ea9d6371477466070d86e10668.zip |
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
Diffstat (limited to 'cloudinit/config')
-rwxr-xr-x | cloudinit/config/cc_set_passwords.py | 5 | ||||
-rw-r--r-- | cloudinit/config/tests/test_set_passwords.py | 40 |
2 files changed, 33 insertions, 12 deletions
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 |