fix CVE-2023-1786
This commit is contained in:
parent
cbe4062c01
commit
e94c8e2d0b
295
backport-CVE-2023-1786.patch
Normal file
295
backport-CVE-2023-1786.patch
Normal file
@ -0,0 +1,295 @@
|
|||||||
|
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/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 | 18 +++++++++++-------
|
||||||
|
6 files changed, 77 insertions(+), 22 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/cloudinit/sources/DataSourceLXD.py b/cloudinit/sources/DataSourceLXD.py
|
||||||
|
index 640348f..8f21a1b 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 Tuple
|
||||||
|
|
||||||
|
import requests
|
||||||
|
from requests.adapters import HTTPAdapter
|
||||||
|
@@ -145,11 +146,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 8c2e82c..8e5253f 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(
|
||||||
|
@@ -60,13 +66,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
|
||||||
|
|
||||||
|
@@ -151,7 +152,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 fff760f..b678cc8 100644
|
||||||
|
--- a/cloudinit/sources/__init__.py
|
||||||
|
+++ b/cloudinit/sources/__init__.py
|
||||||
|
@@ -113,7 +113,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)
|
||||||
|
@@ -135,6 +138,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)
|
||||||
|
@@ -142,9 +151,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
|
||||||
|
|
||||||
|
|
||||||
|
@@ -247,6 +261,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 27af605..88c8e68 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/tests/unittests/sources/test_init.py b/tests/unittests/sources/test_init.py
|
||||||
|
index ce8fc97..e21c9d6 100644
|
||||||
|
--- a/tests/unittests/sources/test_init.py
|
||||||
|
+++ b/tests/unittests/sources/test_init.py
|
||||||
|
@@ -447,12 +447,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,
|
||||||
|
)
|
||||||
|
@@ -479,7 +491,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,
|
||||||
|
@@ -489,6 +503,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",
|
||||||
|
@@ -511,14 +526,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))
|
||||||
|
|
||||||
|
@@ -563,6 +582,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 9fa2e62..138d79b 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.33.0
|
||||||
|
|
||||||
@ -1,6 +1,6 @@
|
|||||||
Name: cloud-init
|
Name: cloud-init
|
||||||
Version: 22.2
|
Version: 22.2
|
||||||
Release: 9
|
Release: 10
|
||||||
Summary: the defacto multi-distribution package that handles early initialization of a cloud instance.
|
Summary: the defacto multi-distribution package that handles early initialization of a cloud instance.
|
||||||
License: ASL 2.0 or GPLv3
|
License: ASL 2.0 or GPLv3
|
||||||
URL: http://launchpad.net/cloud-init
|
URL: http://launchpad.net/cloud-init
|
||||||
@ -17,6 +17,7 @@ Patch5: backport-Fix-permission-of-SSH-host-keys-1971.patch
|
|||||||
Patch6: backport-Do-not-change-permissions-of-netrules-target.patch
|
Patch6: backport-Do-not-change-permissions-of-netrules-target.patch
|
||||||
Patch7: backport-CVE-2022-2084.patch
|
Patch7: backport-CVE-2022-2084.patch
|
||||||
Patch8: backport-Cleanup-ephemeral-IP-routes-on-exception.patch
|
Patch8: backport-Cleanup-ephemeral-IP-routes-on-exception.patch
|
||||||
|
Patch9: backport-CVE-2023-1786.patch
|
||||||
|
|
||||||
Patch9000: fix-permission-of-the-private-key.patch
|
Patch9000: fix-permission-of-the-private-key.patch
|
||||||
|
|
||||||
@ -133,6 +134,12 @@ fi
|
|||||||
%exclude /usr/share/doc/*
|
%exclude /usr/share/doc/*
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Wed May 24 2023 shixuantong <shixuantong1@huawei.com> - 22.2-10
|
||||||
|
- Type:CVE
|
||||||
|
- ID:CVE-2023-1786
|
||||||
|
- SUG:NA
|
||||||
|
- DESC:fix CVE-2023-1786
|
||||||
|
|
||||||
* Thu May 18 2023 shixuantong <shixuantong1@huawei.com> - 22.2-9
|
* Thu May 18 2023 shixuantong <shixuantong1@huawei.com> - 22.2-9
|
||||||
- Type:bugfix
|
- Type:bugfix
|
||||||
- ID:NA
|
- ID:NA
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user