Hi Vivek, Thanks for the v2, I see the branch specification this time.
My only outstanding concern is .. are newer branches impacted by this CVE ? In particular Nanbield. I assume my staged update in master-next covers master, so it shouldn't be a concern. That's the type of indication I was suggesting for a CVE patch on a maintained branch. Just a quick statement that lists the other active branches (which are the same as the OE core maintained branches at any given time) and whether or not they are vulnerable to the issue. Bruce On Tue, Dec 19, 2023 at 12:02 AM Vivek Kumbhar via lists.yoctoproject.org <[email protected]> wrote: > > Upstream-Status: Backport from > https://github.com/canonical/cloud-init/commit/a378b7e4f47375458651c0972e7cd813f6fe0a6b > > Signed-off-by: Vivek Kumbhar <[email protected]> > --- > .../cloud-init/cloud-init/CVE-2023-1786.patch | 294 ++++++++++++++++++ > .../cloud-init/cloud-init_21.4.bb | 1 + > 2 files changed, 295 insertions(+) > create mode 100644 recipes-extended/cloud-init/cloud-init/CVE-2023-1786.patch > > diff --git a/recipes-extended/cloud-init/cloud-init/CVE-2023-1786.patch > b/recipes-extended/cloud-init/cloud-init/CVE-2023-1786.patch > new file mode 100644 > index 00000000..425edd97 > --- /dev/null > +++ b/recipes-extended/cloud-init/cloud-init/CVE-2023-1786.patch > @@ -0,0 +1,294 @@ > +From a378b7e4f47375458651c0972e7cd813f6fe0a6b Mon Sep 17 00:00:00 2001 > +From: James Falcon <[email protected]> > +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 > + > +Upstream-Status: Backport > [https://github.com/canonical/cloud-init/commit/a378b7e4f47375458651c0972e7cd813f6fe0a6b] > +CVE: CVE-2023-1786 > +Signed-off-by: Vivek Kumbhar <[email protected]> > +--- > + cloudinit/sources/DataSourceLXD.py | 8 ++++++-- > + 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 | 16 +++++++++------- > + 6 files changed, 75 insertions(+), 22 deletions(-) > + > +diff --git a/cloudinit/sources/DataSourceLXD.py > b/cloudinit/sources/DataSourceLXD.py > +index 071ea87cf..ea81ccb42 100644 > +--- a/cloudinit/sources/DataSourceLXD.py > ++++ b/cloudinit/sources/DataSourceLXD.py > +@@ -13,6 +13,7 @@ import os > + import socket > + import stat > + from json.decoder import JSONDecodeError > ++from typing import Any, Dict, List, Optional, Tuple, Union, cas > + > + import requests > + from requests.adapters import HTTPAdapter > +@@ -170,11 +171,14 @@ class DataSourceLXD(sources.DataSource): > + _network_config = sources.UNSET > + _crawled_metadata = 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", > + ) > + > + def _is_platform_viable(self) -> bool: > +diff --git a/cloudinit/sources/DataSourceVultr.py > b/cloudinit/sources/DataSourceVultr.py > +index 13f7c24d6..debebc0d7 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( > +@@ -56,13 +62,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 > + > +@@ -147,7 +148,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 9083f3995..8fb9d4cba 100644 > +--- a/cloudinit/sources/__init__.py > ++++ b/cloudinit/sources/__init__.py > +@@ -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) > +@@ -125,6 +128,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) > +@@ -132,9 +141,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 > + > + > +@@ -237,6 +251,14 @@ class DataSource(CloudInitPickleMixin, > metaclass=abc.ABCMeta): > + sensitive_metadata_keys = ( > + "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/stages.py b/cloudinit/stages.py > +index b1a6bc495..85db6bec8 100644 > +--- a/cloudinit/stages.py > ++++ b/cloudinit/stages.py > +@@ -202,7 +202,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/tests/unittests/sources/test_init.py > b/tests/unittests/sources/test_init.py > +index 745a7fa6b..6eebb512d 100644 > +--- a/tests/unittests/sources/test_init.py > ++++ b/tests/unittests/sources/test_init.py > +@@ -442,12 +442,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, > + ) > +@@ -474,7 +486,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, > +@@ -484,6 +498,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", > +@@ -506,14 +521,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)) > + > +@@ -558,6 +577,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 be1a07870..2a9593f61 100644 > +--- a/tests/unittests/test_stages.py > ++++ b/tests/unittests/test_stages.py > +@@ -547,17 +547,19 @@ 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() > +-- > +2.35.7 > diff --git a/recipes-extended/cloud-init/cloud-init_21.4.bb > b/recipes-extended/cloud-init/cloud-init_21.4.bb > index 5cb62272..b9573420 100644 > --- a/recipes-extended/cloud-init/cloud-init_21.4.bb > +++ b/recipes-extended/cloud-init/cloud-init_21.4.bb > @@ -9,6 +9,7 @@ SRC_URI = > "git://github.com/canonical/cloud-init;branch=main;protocol=https \ > file://cloud-init-source-local-lsb-functions.patch \ > file://0001-setup.py-check-for-install-anywhere-in-args.patch \ > file://0001-setup.py-respect-udevdir-variable.patch \ > + file://CVE-2023-1786.patch \ > " > > S = "${WORKDIR}/git" > -- > 2.39.3 > > > > -- - Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end - "Use the force Harry" - Gandalf, Star Trek II
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#8522): https://lists.yoctoproject.org/g/meta-virtualization/message/8522 Mute This Topic: https://lists.yoctoproject.org/mt/103258087/21656 Group Owner: [email protected] Unsubscribe: https://lists.yoctoproject.org/g/meta-virtualization/leave/6693005/21656/1014668956/xyzzy [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
