139 lines
5.5 KiB
Diff
139 lines
5.5 KiB
Diff
From a378b7e4f47375458651c0972e7cd813f6fe0a6b Mon Sep 17 00:00:00 2001
|
|
From: James Falcon <james.falcon@canonical.com>
|
|
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/__init__.py | 28 +++++++++++++++++++++++++---
|
|
cloudinit/stages.py | 4 +++-
|
|
cloudinit/sources/tests/test_init.py | 27 ++++++++++++++++++++++++++-
|
|
3 files changed, 49 insertions(+), 6 deletions(-)
|
|
|
|
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
|
|
+ ):
|
|
md_copy['sensitive_keys'].append(sub_key_path)
|
|
if isinstance(val, str) and val.startswith('ci-b64:'):
|
|
md_copy['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
|
|
|
|
|
|
@@ -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 = ('security-credentials',)
|
|
+ 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",
|
|
+ )
|
|
|
|
def __init__(self, sys_cfg, distro, paths, ud_proc=None):
|
|
self.sys_cfg = sys_cfg
|
|
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(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/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py
|
|
index 96e4dd90..005a571b 100644
|
|
--- a/cloudinit/sources/tests/test_init.py
|
|
+++ b/cloudinit/sources/tests/test_init.py
|
|
@@ -329,9 +329,24 @@ class TestDataSource(CiTestCase):
|
|
'local-hostname': 'test-subclass-hostname',
|
|
'region': 'myregion',
|
|
'some': {'security-credentials': {
|
|
- 'cred1': 'sekret', 'cred2': 'othersekret'}}})
|
|
+ 'cred1': 'sekret', 'cred2': 'othersekret'
|
|
+ }
|
|
+ },
|
|
+ },
|
|
+ )
|
|
self.assertEqual(
|
|
- ('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,
|
|
+ )
|
|
datasource.get_data()
|
|
json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp)
|
|
sensitive_json_file = self.tmp_path(INSTANCE_JSON_SENSITIVE_FILE, tmp)
|
|
--
|
|
2.27.0
|
|
|