From 85a4c86455edcd6f4aa5969636b100504a99e86a Mon Sep 17 00:00:00 2001 From: Jon Maloy Date: Wed, 3 May 2023 18:04:26 +0000 Subject: [PATCH] * Wed May 03 2023 Jon Maloy - 23.1.1-3 - ci-Don-t-change-permissions-of-netrules-target-2076.patch [bz#2182947] - ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch [bz#2190081] - Resolves: bz#2182947 (Request to backport "Don't change permissions of netrules target (#2076)") - Resolves: bz#2190081 (CVE-2023-1786 cloud-init: sensitive data could be exposed in logs [rhel-8]) --- ...-permissions-of-netrules-target-2076.patch | 120 +++++++ ...-data-sensitive-and-remove-log-permi.patch | 309 ++++++++++++++++++ cloud-init.spec | 14 +- 3 files changed, 442 insertions(+), 1 deletion(-) create mode 100644 ci-Don-t-change-permissions-of-netrules-target-2076.patch create mode 100644 ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch diff --git a/ci-Don-t-change-permissions-of-netrules-target-2076.patch b/ci-Don-t-change-permissions-of-netrules-target-2076.patch new file mode 100644 index 0000000..f03aff0 --- /dev/null +++ b/ci-Don-t-change-permissions-of-netrules-target-2076.patch @@ -0,0 +1,120 @@ +From 285d8d8005db06ea86afc042bc2eec07bf3c6fab Mon Sep 17 00:00:00 2001 +From: James Falcon +Date: Thu, 23 Mar 2023 10:21:56 -0500 +Subject: [PATCH 1/2] Don't change permissions of netrules target (#2076) + +RH-Author: Ani Sinha +RH-MergeRequest: 98: Don't change permissions of netrules target (#2076) +RH-Bugzilla: 2182947 +RH-Acked-by: Emanuele Giuseppe Esposito +RH-Acked-by: Vitaly Kuznetsov +RH-Commit: [1/1] 37fa74519da67b383de87b41108561b09d7b9210 (anisinha/rhel-cloud-init) + +Set permissions if file doesn't exist. Leave them if it does. + +LP: #2011783 + +Co-authored-by: Chad Smith +(cherry picked from commit 56c88cafd1b3606e814069a79f4ec265fc427c87) +Signed-off-by: Ani Sinha +--- + cloudinit/net/eni.py | 4 +++- + cloudinit/net/sysconfig.py | 7 ++++++- + tests/unittests/distros/test_netconfig.py | 20 ++++++++++++++++++-- + 3 files changed, 27 insertions(+), 4 deletions(-) + +diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py +index 53bd35ca..1de3bec2 100644 +--- a/cloudinit/net/eni.py ++++ b/cloudinit/net/eni.py +@@ -576,7 +576,9 @@ class Renderer(renderer.Renderer): + netrules = subp.target_path(target, self.netrules_path) + util.ensure_dir(os.path.dirname(netrules)) + util.write_file( +- netrules, self._render_persistent_net(network_state) ++ netrules, ++ content=self._render_persistent_net(network_state), ++ preserve_mode=True, + ) + + +diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py +index db084e07..da6d11b3 100644 +--- a/cloudinit/net/sysconfig.py ++++ b/cloudinit/net/sysconfig.py +@@ -1033,7 +1033,12 @@ class Renderer(renderer.Renderer): + if self.netrules_path: + netrules_content = self._render_persistent_net(network_state) + netrules_path = subp.target_path(target, self.netrules_path) +- util.write_file(netrules_path, netrules_content, file_mode) ++ util.write_file( ++ netrules_path, ++ content=netrules_content, ++ mode=file_mode, ++ preserve_mode=True, ++ ) + if available_nm(target=target): + enable_ifcfg_rh(subp.target_path(target, path=NM_CFG_FILE)) + +diff --git a/tests/unittests/distros/test_netconfig.py b/tests/unittests/distros/test_netconfig.py +index e9fb0591..b1c89ce3 100644 +--- a/tests/unittests/distros/test_netconfig.py ++++ b/tests/unittests/distros/test_netconfig.py +@@ -458,8 +458,16 @@ class TestNetCfgDistroUbuntuEni(TestNetCfgDistroBase): + def eni_path(self): + return "/etc/network/interfaces.d/50-cloud-init.cfg" + ++ def rules_path(self): ++ return "/etc/udev/rules.d/70-persistent-net.rules" ++ + def _apply_and_verify_eni( +- self, apply_fn, config, expected_cfgs=None, bringup=False ++ self, ++ apply_fn, ++ config, ++ expected_cfgs=None, ++ bringup=False, ++ previous_files=(), + ): + if not expected_cfgs: + raise ValueError("expected_cfg must not be None") +@@ -467,7 +475,11 @@ class TestNetCfgDistroUbuntuEni(TestNetCfgDistroBase): + tmpd = None + with mock.patch("cloudinit.net.eni.available") as m_avail: + m_avail.return_value = True ++ path_modes = {} + with self.reRooted(tmpd) as tmpd: ++ for previous_path, content, mode in previous_files: ++ util.write_file(previous_path, content, mode=mode) ++ path_modes[previous_path] = mode + apply_fn(config, bringup) + + results = dir2dict(tmpd) +@@ -478,7 +490,9 @@ class TestNetCfgDistroUbuntuEni(TestNetCfgDistroBase): + print(results[cfgpath]) + print("----------") + self.assertEqual(expected, results[cfgpath]) +- self.assertEqual(0o644, get_mode(cfgpath, tmpd)) ++ self.assertEqual( ++ path_modes.get(cfgpath, 0o644), get_mode(cfgpath, tmpd) ++ ) + + def test_apply_network_config_and_bringup_filters_priority_eni_ub(self): + """Network activator search priority can be overridden from config.""" +@@ -527,11 +541,13 @@ class TestNetCfgDistroUbuntuEni(TestNetCfgDistroBase): + def test_apply_network_config_eni_ub(self): + expected_cfgs = { + self.eni_path(): V1_NET_CFG_OUTPUT, ++ self.rules_path(): "", + } + self._apply_and_verify_eni( + self.distro.apply_network_config, + V1_NET_CFG, + expected_cfgs=expected_cfgs.copy(), ++ previous_files=((self.rules_path(), "something", 0o660),), + ) + + def test_apply_network_config_ipv6_ub(self): +-- +2.37.3 + diff --git a/ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch b/ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch new file mode 100644 index 0000000..36b379b --- /dev/null +++ b/ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch @@ -0,0 +1,309 @@ +From dd1a79fc5c0b5f486ca2e66ed3a45c8f4f7b1f15 Mon Sep 17 00:00:00 2001 +From: James Falcon +Date: Wed, 26 Apr 2023 15:11:55 -0500 +Subject: [PATCH 2/2] Make user/vendor data sensitive and remove log + permissions (#2144) + +RH-Author: Ani Sinha +RH-MergeRequest: 99: Make user/vendor data sensitive and remove log permissions (#2144) +RH-Bugzilla: 2190081 +RH-Acked-by: Emanuele Giuseppe Esposito +RH-Acked-by: Vitaly Kuznetsov +RH-Commit: [1/1] 1b34e2c9c61a90abb88f2df87d41f96b54e79ff7 (anisinha/rhel-cloud-init) + +Because user data and vendor data may contain sensitive information, +this commit ensures that any user data or vendor data written to +instance-data.json gets redacted and is only available to root user. + +Also, modify the permissions of cloud-init.log to be 640, so that +sensitive data leaked to the log isn't world readable. +Additionally, remove the logging of user data and vendor data to +cloud-init.log from the Vultr datasource. + +Conflicts: + cloudinit/sources/DataSourceVultr.py + - editor directives missing in file on upstream version. + +LP: #2013967 +CVE: CVE-2023-1786 +(cherry picked from commit a378b7e4f47375458651c0972e7cd813f6fe0a6b) +Signed-off-by: Ani Sinha +--- + cloudinit/sources/DataSourceLXD.py | 9 ++++++--- + cloudinit/sources/DataSourceVultr.py | 14 ++++++-------- + cloudinit/sources/__init__.py | 28 +++++++++++++++++++++++++--- + cloudinit/stages.py | 4 +++- + tests/unittests/sources/test_init.py | 27 ++++++++++++++++++++++++++- + tests/unittests/test_stages.py | 18 +++++++++++------- + 6 files changed, 77 insertions(+), 23 deletions(-) + +diff --git a/cloudinit/sources/DataSourceLXD.py b/cloudinit/sources/DataSourceLXD.py +index ab440cc8..e4cae91a 100644 +--- a/cloudinit/sources/DataSourceLXD.py ++++ b/cloudinit/sources/DataSourceLXD.py +@@ -14,7 +14,7 @@ import stat + import time + from enum import Flag, auto + from json.decoder import JSONDecodeError +-from typing import Any, Dict, List, Optional, Union, cast ++from typing import Any, Dict, List, Optional, Tuple, Union, cast + + import requests + from requests.adapters import HTTPAdapter +@@ -168,11 +168,14 @@ class DataSourceLXD(sources.DataSource): + _network_config: Union[Dict, str] = sources.UNSET + _crawled_metadata: Union[Dict, str] = sources.UNSET + +- sensitive_metadata_keys = ( +- "merged_cfg", ++ sensitive_metadata_keys: Tuple[ ++ str, ... ++ ] = sources.DataSource.sensitive_metadata_keys + ( + "user.meta-data", + "user.vendor-data", + "user.user-data", ++ "cloud-init.user-data", ++ "cloud-init.vendor-data", + ) + + skip_hotplug_detect = True +diff --git a/cloudinit/sources/DataSourceVultr.py b/cloudinit/sources/DataSourceVultr.py +index 9d7c84fb..660e9f14 100644 +--- a/cloudinit/sources/DataSourceVultr.py ++++ b/cloudinit/sources/DataSourceVultr.py +@@ -5,6 +5,8 @@ + # Vultr Metadata API: + # https://www.vultr.com/metadata/ + ++from typing import Tuple ++ + import cloudinit.sources.helpers.vultr as vultr + from cloudinit import log as log + from cloudinit import sources, util, version +@@ -28,6 +30,10 @@ class DataSourceVultr(sources.DataSource): + + dsname = "Vultr" + ++ sensitive_metadata_keys: Tuple[ ++ str, ... ++ ] = sources.DataSource.sensitive_metadata_keys + ("startup-script",) ++ + def __init__(self, sys_cfg, distro, paths): + super(DataSourceVultr, self).__init__(sys_cfg, distro, paths) + self.ds_cfg = util.mergemanydict( +@@ -54,13 +60,8 @@ class DataSourceVultr(sources.DataSource): + self.get_datasource_data(self.metadata) + + # Dump some data so diagnosing failures is manageable +- LOG.debug("Vultr Vendor Config:") +- LOG.debug(util.json_dumps(self.metadata["vendor-data"])) + LOG.debug("SUBID: %s", self.metadata["instance-id"]) + LOG.debug("Hostname: %s", self.metadata["local-hostname"]) +- if self.userdata_raw is not None: +- LOG.debug("User-Data:") +- LOG.debug(self.userdata_raw) + + return True + +@@ -146,7 +147,4 @@ if __name__ == "__main__": + config = md["vendor-data"] + sysinfo = vultr.get_sysinfo() + +- print(util.json_dumps(sysinfo)) +- print(util.json_dumps(config)) +- + # vi: ts=4 expandtab +diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py +index 565e1754..5c6ae8b1 100644 +--- a/cloudinit/sources/__init__.py ++++ b/cloudinit/sources/__init__.py +@@ -110,7 +110,10 @@ def process_instance_metadata(metadata, key_path="", sensitive_keys=()): + sub_key_path = key_path + "/" + key + else: + sub_key_path = key +- if key in sensitive_keys or sub_key_path in sensitive_keys: ++ if ( ++ key.lower() in sensitive_keys ++ or sub_key_path.lower() in sensitive_keys ++ ): + sens_keys.append(sub_key_path) + if isinstance(val, str) and val.startswith("ci-b64:"): + base64_encoded_keys.append(sub_key_path) +@@ -132,6 +135,12 @@ def redact_sensitive_keys(metadata, redact_value=REDACT_SENSITIVE_VALUE): + + Replace any keys values listed in 'sensitive_keys' with redact_value. + """ ++ # While 'sensitive_keys' should already sanitized to only include what ++ # is in metadata, it is possible keys will overlap. For example, if ++ # "merged_cfg" and "merged_cfg/ds/userdata" both match, it's possible that ++ # "merged_cfg" will get replaced first, meaning "merged_cfg/ds/userdata" ++ # no longer represents a valid key. ++ # Thus, we still need to do membership checks in this function. + if not metadata.get("sensitive_keys", []): + return metadata + md_copy = copy.deepcopy(metadata) +@@ -139,9 +148,14 @@ def redact_sensitive_keys(metadata, redact_value=REDACT_SENSITIVE_VALUE): + path_parts = key_path.split("/") + obj = md_copy + for path in path_parts: +- if isinstance(obj[path], dict) and path != path_parts[-1]: ++ if ( ++ path in obj ++ and isinstance(obj[path], dict) ++ and path != path_parts[-1] ++ ): + obj = obj[path] +- obj[path] = redact_value ++ if path in obj: ++ obj[path] = redact_value + return md_copy + + +@@ -249,6 +263,14 @@ class DataSource(CloudInitPickleMixin, metaclass=abc.ABCMeta): + sensitive_metadata_keys: Tuple[str, ...] = ( + "merged_cfg", + "security-credentials", ++ "userdata", ++ "user-data", ++ "user_data", ++ "vendordata", ++ "vendor-data", ++ # Provide ds/vendor_data to avoid redacting top-level ++ # "vendor_data": {enabled: True} ++ "ds/vendor_data", + ) + + # True on datasources that may not see hotplugged devices reflected +diff --git a/cloudinit/stages.py b/cloudinit/stages.py +index a624a6fb..1326d205 100644 +--- a/cloudinit/stages.py ++++ b/cloudinit/stages.py +@@ -204,7 +204,9 @@ class Init: + log_file = util.get_cfg_option_str(self.cfg, "def_log_file") + log_file_mode = util.get_cfg_option_int(self.cfg, "def_log_file_mode") + if log_file: +- util.ensure_file(log_file, mode=0o640, preserve_mode=True) ++ # At this point the log file should have already been created ++ # in the setupLogging function of log.py ++ util.ensure_file(log_file, mode=0o640, preserve_mode=False) + perms = self.cfg.get("syslog_fix_perms") + if not perms: + perms = {} +diff --git a/tests/unittests/sources/test_init.py b/tests/unittests/sources/test_init.py +index 0447e02c..eb27198f 100644 +--- a/tests/unittests/sources/test_init.py ++++ b/tests/unittests/sources/test_init.py +@@ -458,12 +458,24 @@ class TestDataSource(CiTestCase): + "cred2": "othersekret", + } + }, ++ "someother": { ++ "nested": { ++ "userData": "HIDE ME", ++ } ++ }, ++ "VENDOR-DAta": "HIDE ME TOO", + }, + ) + self.assertCountEqual( + ( + "merged_cfg", + "security-credentials", ++ "userdata", ++ "user-data", ++ "user_data", ++ "vendordata", ++ "vendor-data", ++ "ds/vendor_data", + ), + datasource.sensitive_metadata_keys, + ) +@@ -490,7 +502,9 @@ class TestDataSource(CiTestCase): + "base64_encoded_keys": [], + "merged_cfg": REDACT_SENSITIVE_VALUE, + "sensitive_keys": [ ++ "ds/meta_data/VENDOR-DAta", + "ds/meta_data/some/security-credentials", ++ "ds/meta_data/someother/nested/userData", + "merged_cfg", + ], + "sys_info": sys_info, +@@ -500,6 +514,7 @@ class TestDataSource(CiTestCase): + "availability_zone": "myaz", + "cloud-name": "subclasscloudname", + "cloud_name": "subclasscloudname", ++ "cloud_id": "subclasscloudname", + "distro": "ubuntu", + "distro_release": "focal", + "distro_version": "20.04", +@@ -522,14 +537,18 @@ class TestDataSource(CiTestCase): + "ds": { + "_doc": EXPERIMENTAL_TEXT, + "meta_data": { ++ "VENDOR-DAta": REDACT_SENSITIVE_VALUE, + "availability_zone": "myaz", + "local-hostname": "test-subclass-hostname", + "region": "myregion", + "some": {"security-credentials": REDACT_SENSITIVE_VALUE}, ++ "someother": { ++ "nested": {"userData": REDACT_SENSITIVE_VALUE} ++ }, + }, + }, + } +- self.assertCountEqual(expected, redacted) ++ self.assertEqual(expected, redacted) + file_stat = os.stat(json_file) + self.assertEqual(0o644, stat.S_IMODE(file_stat.st_mode)) + +@@ -574,6 +593,12 @@ class TestDataSource(CiTestCase): + ( + "merged_cfg", + "security-credentials", ++ "userdata", ++ "user-data", ++ "user_data", ++ "vendordata", ++ "vendor-data", ++ "ds/vendor_data", + ), + datasource.sensitive_metadata_keys, + ) +diff --git a/tests/unittests/test_stages.py b/tests/unittests/test_stages.py +index 15a7e973..a61f9df9 100644 +--- a/tests/unittests/test_stages.py ++++ b/tests/unittests/test_stages.py +@@ -606,19 +606,23 @@ class TestInit_InitializeFilesystem: + # Assert we create it 0o640 by default if it doesn't already exist + assert 0o640 == stat.S_IMODE(log_file.stat().mode) + +- def test_existing_file_permissions_are_not_modified(self, init, tmpdir): +- """If the log file already exists, we should not modify its permissions ++ def test_existing_file_permissions(self, init, tmpdir): ++ """Test file permissions are set as expected. ++ ++ CIS Hardening requires 640 permissions. These permissions are ++ currently hardcoded on every boot, but if there's ever a reason ++ to change this, we need to then ensure that they ++ are *not* set every boot. + + See https://bugs.launchpad.net/cloud-init/+bug/1900837. + """ +- # Use a mode that will never be made the default so this test will +- # always be valid +- mode = 0o606 + log_file = tmpdir.join("cloud-init.log") + log_file.ensure() +- log_file.chmod(mode) ++ # Use a mode that will never be made the default so this test will ++ # always be valid ++ log_file.chmod(0o606) + init._cfg = {"def_log_file": str(log_file)} + + init._initialize_filesystem() + +- assert mode == stat.S_IMODE(log_file.stat().mode) ++ assert 0o640 == stat.S_IMODE(log_file.stat().mode) +-- +2.37.3 + diff --git a/cloud-init.spec b/cloud-init.spec index c4c7d51..f0f2beb 100644 --- a/cloud-init.spec +++ b/cloud-init.spec @@ -6,7 +6,7 @@ Name: cloud-init Version: 23.1.1 -Release: 2%{?dist} +Release: 3%{?dist} Summary: Cloud instance init scripts Group: System Environment/Base @@ -23,6 +23,10 @@ Patch0006: 0006-Revert-Add-native-NetworkManager-support-1224.patch Patch0007: 0007-settings.py-update-settings-for-rhel.patch # For bz#2182407 - cloud-init strips new line from "/etc/hostname" when processing "/var/lib/cloud/data/previous-hostname" Patch8: ci-rhel-make-sure-previous-hostname-file-ends-with-a-ne.patch +# For bz#2182947 - Request to backport "Don't change permissions of netrules target (#2076)" +Patch9: ci-Don-t-change-permissions-of-netrules-target-2076.patch +# For bz#2190081 - CVE-2023-1786 cloud-init: sensitive data could be exposed in logs [rhel-8] +Patch10: ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch BuildArch: noarch @@ -209,6 +213,14 @@ fi %config(noreplace) %{_sysconfdir}/rsyslog.d/21-cloudinit.conf %changelog +* Wed May 03 2023 Jon Maloy - 23.1.1-3 +- ci-Don-t-change-permissions-of-netrules-target-2076.patch [bz#2182947] +- ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch [bz#2190081] +- Resolves: bz#2182947 + (Request to backport "Don't change permissions of netrules target (#2076)") +- Resolves: bz#2190081 + (CVE-2023-1786 cloud-init: sensitive data could be exposed in logs [rhel-8]) + * Tue Apr 25 2023 Jon Maloy - 23.1.1-2 - ci-rhel-make-sure-previous-hostname-file-ends-with-a-ne.patch [bz#2182407] - Resolves: bz#2182407