diff --git a/backport-CVE-2023-1786.patch b/backport-CVE-2023-1786.patch new file mode 100644 index 0000000000000000000000000000000000000000..e8dcf762c631273d615bd526a0df2858883b38e4 --- /dev/null +++ b/backport-CVE-2023-1786.patch @@ -0,0 +1,332 @@ +From a378b7e4f47375458651c0972e7cd813f6fe0a6b Mon Sep 17 00:00:00 2001 +From: James Falcon +Date: Wed, 26 Apr 2023 15:11:55 -0500 +Subject: [PATCH] Make user/vendor data sensitive and remove log permissions + (#2144) + +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. + +LP: #2013967 +CVE: CVE-2023-1786 +--- + cloudinit/sources/DataSourceLXD.py | 11 +++++- + cloudinit/sources/DataSourceVultr.py | 14 +++---- + cloudinit/sources/__init__.py | 35 ++++++++++++++--- + cloudinit/sources/tests/test_init.py | 58 ++++++++++++++++++++++++---- + cloudinit/stages.py | 4 +- + cloudinit/tests/test_stages.py | 18 +++++---- + 6 files changed, 109 insertions(+), 31 deletions(-) + +diff --git a/cloudinit/sources/DataSourceLXD.py b/cloudinit/sources/DataSourceLXD.py +index 732b32f..1e1e9e2 100644 +--- a/cloudinit/sources/DataSourceLXD.py ++++ b/cloudinit/sources/DataSourceLXD.py +@@ -14,6 +14,7 @@ import os + + import requests + from requests.adapters import HTTPAdapter ++from typing import Tuple + + # pylint fails to import the two modules below. + # These are imported via requests.packages rather than urllib3 because: +@@ -173,8 +174,14 @@ class DataSourceLXD(sources.DataSource): + _network_config = sources.UNSET + _crawled_metadata = sources.UNSET + +- sensitive_metadata_keys = ( +- 'merged_cfg', 'user.meta-data', 'user.vendor-data', 'user.user-data', ++ 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", + ) + + def _is_platform_viable(self) -> bool: +diff --git a/cloudinit/sources/DataSourceVultr.py b/cloudinit/sources/DataSourceVultr.py +index 68e1ff0..4d41d4e 100644 +--- a/cloudinit/sources/DataSourceVultr.py ++++ b/cloudinit/sources/DataSourceVultr.py +@@ -10,6 +10,8 @@ from cloudinit import sources + from cloudinit import util + from cloudinit import version + ++from typing import Tuple ++ + import cloudinit.sources.helpers.vultr as vultr + + LOG = log.getLogger(__name__) +@@ -29,6 +31,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 + +@@ -141,7 +142,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 f2f2343..20cc397 100644 +--- a/cloudinit/sources/__init__.py ++++ b/cloudinit/sources/__init__.py +@@ -13,7 +13,7 @@ import copy + import json + import os + from collections import namedtuple +-from typing import Dict, List # noqa: F401 ++from typing import Dict, List, Tuple # noqa: F401 + + from cloudinit import dmi + from cloudinit import importer +@@ -103,7 +103,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) +@@ -124,6 +127,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) +@@ -131,9 +140,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 + + +@@ -215,7 +229,18 @@ class DataSource(CloudInitPickleMixin, metaclass=abc.ABCMeta): + + # N-tuple of keypaths or keynames redact from instance-data.json for + # non-root users +- sensitive_metadata_keys = ('merged_cfg', 'security-credentials',) ++ 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", ++ ) + + _ci_pkl_version = 1 + +diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py +index ae09cb1..bce2ab5 100644 +--- a/cloudinit/sources/tests/test_init.py ++++ b/cloudinit/sources/tests/test_init.py +@@ -353,11 +353,32 @@ class TestDataSource(CiTestCase): + 'availability_zone': 'myaz', + 'local-hostname': 'test-subclass-hostname', + 'region': 'myregion', +- 'some': {'security-credentials': { +- 'cred1': 'sekret', 'cred2': 'othersekret'}}}) ++ 'some': { ++ 'security-credentials': { ++ 'cred1': 'sekret', 'cred2': 'othersekret' ++ } ++ }, ++ "someother": { ++ "nested": { ++ "userData": "HIDE ME", ++ } ++ }, ++ "VENDOR-DAta": "HIDE ME TOO", ++ }, ++ ) + self.assertCountEqual( +- ('merged_cfg', 'security-credentials',), +- datasource.sensitive_metadata_keys) ++ ( ++ "merged_cfg", ++ "security-credentials", ++ "userdata", ++ "user-data", ++ "user_data", ++ "vendordata", ++ "vendor-data", ++ "ds/vendor_data", ++ ), ++ datasource.sensitive_metadata_keys, ++ ) + sys_info = { + "python": "3.7", + "platform": +@@ -373,7 +394,11 @@ class TestDataSource(CiTestCase): + 'base64_encoded_keys': [], + 'merged_cfg': REDACT_SENSITIVE_VALUE, + 'sensitive_keys': [ +- 'ds/meta_data/some/security-credentials', 'merged_cfg'], ++ "ds/meta_data/VENDOR-DAta", ++ "ds/meta_data/some/security-credentials", ++ "ds/meta_data/someother/nested/userData", ++ "merged_cfg", ++ ], + 'sys_info': sys_info, + 'v1': { + '_beta_keys': ['subplatform'], +@@ -381,6 +406,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', +@@ -401,10 +427,16 @@ 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}}} ++ 'some': {'security-credentials': REDACT_SENSITIVE_VALUE}, ++ "someother": { ++ "nested": {"userData": REDACT_SENSITIVE_VALUE} ++ }, ++ }, ++ }, + } + self.assertCountEqual(expected, redacted) + file_stat = os.stat(json_file) +@@ -432,8 +464,18 @@ class TestDataSource(CiTestCase): + "variant": "ubuntu", "dist": ["ubuntu", "20.04", "focal"]} + + self.assertCountEqual( +- ('merged_cfg', 'security-credentials',), +- datasource.sensitive_metadata_keys) ++ ( ++ "merged_cfg", ++ "security-credentials", ++ "userdata", ++ "user-data", ++ "user_data", ++ "vendordata", ++ "vendor-data", ++ "ds/vendor_data", ++ ), ++ datasource.sensitive_metadata_keys, ++ ) + with mock.patch("cloudinit.util.system_info", return_value=sys_info): + datasource.get_data() + sensitive_json_file = self.tmp_path(INSTANCE_JSON_SENSITIVE_FILE, tmp) +diff --git a/cloudinit/stages.py b/cloudinit/stages.py +index 731b298..59b0925 100644 +--- a/cloudinit/stages.py ++++ b/cloudinit/stages.py +@@ -204,7 +204,9 @@ class Init(object): + util.ensure_dirs(self._initial_subdirs()) + log_file = util.get_cfg_option_str(self.cfg, 'def_log_file') + 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/cloudinit/tests/test_stages.py b/cloudinit/tests/test_stages.py +index a50836a..aeab17a 100644 +--- a/cloudinit/tests/test_stages.py ++++ b/cloudinit/tests/test_stages.py +@@ -458,21 +458,25 @@ 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) + + # vi: ts=4 expandtab +-- +2.33.0 + diff --git a/cloud-init.spec b/cloud-init.spec index 9a95d3c79c7f1c6cf638fb182c2819883bf89499..aae68ef6eb525e6c5d9946ec133c148ba3f2ce4c 100644 --- a/cloud-init.spec +++ b/cloud-init.spec @@ -1,6 +1,6 @@ Name: cloud-init Version: 21.4 -Release: 12 +Release: 13 Summary: the defacto multi-distribution package that handles early initialization of a cloud instance. License: ASL 2.0 or GPLv3 URL: http://launchpad.net/cloud-init @@ -25,6 +25,7 @@ Patch13: backport-CVE-2022-2084.patch Patch14: remove-schema-errors-from-log-for-cloudinit-config-cc_.patch Patch15: backport-cloudinit-net-handle-two-different-routes-for-the-sa.patch Patch16: backport-Cleanup-ephemeral-IP-routes-on-exception-2100.patch +#Patch17: backport-CVE-2023-1786.patch Patch9000: Fix-the-error-level-logs-displayed-for-the-cloud-init-local-service.patch @@ -136,6 +137,9 @@ fi %exclude /usr/share/doc/* %changelog +* Wed May 24 2023 shixuantong - 21.4-13 +- fix CVE-2023-1786 + * Fri May 19 2023 shixuantong - 21.4-12 - Cleanup ephemeral IP routes on exception and handle two different routes for the same ip