From a378b7e4f47375458651c0972e7cd813f6fe0a6b Mon Sep 17 00:00:00 2001 From: James Falcon Date: Wed, 26 Apr 2023 15:11:55 -0500 Subject: 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 | 9 ++++++--- cloudinit/sources/DataSourceVultr.py | 14 ++++++-------- cloudinit/sources/__init__.py | 28 +++++++++++++++++++++++++--- cloudinit/stages.py | 4 +++- 4 files changed, 40 insertions(+), 15 deletions(-) (limited to 'cloudinit') diff --git a/cloudinit/sources/DataSourceLXD.py b/cloudinit/sources/DataSourceLXD.py index 2643149b..6a41fbcc 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 7b7cc696..4141be90 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, stages, 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 @@ -155,6 +156,3 @@ if __name__ == "__main__": ) config = md["vendor-data"] sysinfo = vultr.get_sysinfo() - - print(util.json_dumps(sysinfo)) - print(util.json_dumps(config)) diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 90521ba2..9d91512f 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -111,7 +111,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) @@ -133,6 +136,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) @@ -140,9 +149,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 @@ -250,6 +264,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 65f952e7..509d8f7f 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -203,7 +203,9 @@ class Init: 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 = {} -- cgit v1.2.1