rpms/cloud-init
8
1
Fork 1
Code Issues Pull Requests Releases Activity

import cloud-init-20.3-10.el8_4.5

Browse Source
This commit is contained in:
CentOS Sources 2021-08-10 08:03:26 -04:00 committed by Andrew Lukoshko
parent f0510e213e
commit 5918eb684a
3 changed files with 461 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
SOURCES/ci-rhel-cloud.cfg-remove-ssh_genkeytypes-in-settings.py.patch Normal file
View File

@ -0,0 +1,64 @@
From 569a19866bba846bbea8c99b9998336299570783 Mon Sep 17 00:00:00 2001
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
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 <eesposit@redhat.com>
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 <cavery@redhat.com>
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>
RH-Acked-by: Mohamed Gamal Morsy <mmorsy@redhat.com>
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 <eesposit@redhat.com>
---
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

378
SOURCES/ci-write-passwords-only-to-serial-console-lock-down-clo.patch Normal file
View File

@ -0,0 +1,378 @@
From 4c38c004b016a4f0f255af8a779a58e549068967 Mon Sep 17 00:00:00 2001
From: Eduardo Otubo <otubo@redhat.com>
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 <otubo@redhat.com>
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 <eesposit@redhat.com>
RH-Acked-by: Mohamed Gamal Morsy <mmorsy@redhat.com>
commit b794d426b9ab43ea9d6371477466070d86e10668
Author: Daniel Watkins <oddbloke@ubuntu.com>
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 <otubo@redhat.com>
---
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

22
SPECS/cloud-init.spec
View File

@ -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 <mrezanin@redhat.com> - 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 <mrezanin@redhat.com> - 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 <mrezanin@redhat.com> - 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]