* Wed May 03 2023 Jon Maloy <jmaloy@redhat.com> - 23.1.1-3

- ci-Don-t-change-permissions-of-netrules-target-2076.patch [bz#2182947]
- ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch [bz#2190081]
- Resolves: bz#2182947
  (Request to backport "Don't change permissions of netrules target (#2076)")
- Resolves: bz#2190081
  (CVE-2023-1786 cloud-init: sensitive data could be exposed in logs [rhel-8])
This commit is contained in:
Jon Maloy 2023-05-03 18:04:26 +00:00
parent d4dc8ce547
commit 85a4c86455
3 changed files with 442 additions and 1 deletions

View File

@ -0,0 +1,120 @@
From 285d8d8005db06ea86afc042bc2eec07bf3c6fab Mon Sep 17 00:00:00 2001
From: James Falcon <james.falcon@canonical.com>
Date: Thu, 23 Mar 2023 10:21:56 -0500
Subject: [PATCH 1/2] Don't change permissions of netrules target (#2076)
RH-Author: Ani Sinha <None>
RH-MergeRequest: 98: Don't change permissions of netrules target (#2076)
RH-Bugzilla: 2182947
RH-Acked-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>
RH-Commit: [1/1] 37fa74519da67b383de87b41108561b09d7b9210 (anisinha/rhel-cloud-init)
Set permissions if file doesn't exist. Leave them if it does.
LP: #2011783
Co-authored-by: Chad Smith <chad.smith@canonical.com>
(cherry picked from commit 56c88cafd1b3606e814069a79f4ec265fc427c87)
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
cloudinit/net/eni.py | 4 +++-
cloudinit/net/sysconfig.py | 7 ++++++-
tests/unittests/distros/test_netconfig.py | 20 ++++++++++++++++++--
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py
index 53bd35ca..1de3bec2 100644
--- a/cloudinit/net/eni.py
+++ b/cloudinit/net/eni.py
@@ -576,7 +576,9 @@ class Renderer(renderer.Renderer):
netrules = subp.target_path(target, self.netrules_path)
util.ensure_dir(os.path.dirname(netrules))
util.write_file(
- netrules, self._render_persistent_net(network_state)
+ netrules,
+ content=self._render_persistent_net(network_state),
+ preserve_mode=True,
)
diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index db084e07..da6d11b3 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -1033,7 +1033,12 @@ class Renderer(renderer.Renderer):
if self.netrules_path:
netrules_content = self._render_persistent_net(network_state)
netrules_path = subp.target_path(target, self.netrules_path)
- util.write_file(netrules_path, netrules_content, file_mode)
+ util.write_file(
+ netrules_path,
+ content=netrules_content,
+ mode=file_mode,
+ preserve_mode=True,
+ )
if available_nm(target=target):
enable_ifcfg_rh(subp.target_path(target, path=NM_CFG_FILE))
diff --git a/tests/unittests/distros/test_netconfig.py b/tests/unittests/distros/test_netconfig.py
index e9fb0591..b1c89ce3 100644
--- a/tests/unittests/distros/test_netconfig.py
+++ b/tests/unittests/distros/test_netconfig.py
@@ -458,8 +458,16 @@ class TestNetCfgDistroUbuntuEni(TestNetCfgDistroBase):
def eni_path(self):
return "/etc/network/interfaces.d/50-cloud-init.cfg"
+ def rules_path(self):
+ return "/etc/udev/rules.d/70-persistent-net.rules"
+
def _apply_and_verify_eni(
- self, apply_fn, config, expected_cfgs=None, bringup=False
+ self,
+ apply_fn,
+ config,
+ expected_cfgs=None,
+ bringup=False,
+ previous_files=(),
):
if not expected_cfgs:
raise ValueError("expected_cfg must not be None")
@@ -467,7 +475,11 @@ class TestNetCfgDistroUbuntuEni(TestNetCfgDistroBase):
tmpd = None
with mock.patch("cloudinit.net.eni.available") as m_avail:
m_avail.return_value = True
+ path_modes = {}
with self.reRooted(tmpd) as tmpd:
+ for previous_path, content, mode in previous_files:
+ util.write_file(previous_path, content, mode=mode)
+ path_modes[previous_path] = mode
apply_fn(config, bringup)
results = dir2dict(tmpd)
@@ -478,7 +490,9 @@ class TestNetCfgDistroUbuntuEni(TestNetCfgDistroBase):
print(results[cfgpath])
print("----------")
self.assertEqual(expected, results[cfgpath])
- self.assertEqual(0o644, get_mode(cfgpath, tmpd))
+ self.assertEqual(
+ path_modes.get(cfgpath, 0o644), get_mode(cfgpath, tmpd)
+ )
def test_apply_network_config_and_bringup_filters_priority_eni_ub(self):
"""Network activator search priority can be overridden from config."""
@@ -527,11 +541,13 @@ class TestNetCfgDistroUbuntuEni(TestNetCfgDistroBase):
def test_apply_network_config_eni_ub(self):
expected_cfgs = {
self.eni_path(): V1_NET_CFG_OUTPUT,
+ self.rules_path(): "",
}
self._apply_and_verify_eni(
self.distro.apply_network_config,
V1_NET_CFG,
expected_cfgs=expected_cfgs.copy(),
+ previous_files=((self.rules_path(), "something", 0o660),),
)
def test_apply_network_config_ipv6_ub(self):
--
2.37.3

View File

@ -0,0 +1,309 @@
From dd1a79fc5c0b5f486ca2e66ed3a45c8f4f7b1f15 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 2/2] Make user/vendor data sensitive and remove log
permissions (#2144)
RH-Author: Ani Sinha <None>
RH-MergeRequest: 99: Make user/vendor data sensitive and remove log permissions (#2144)
RH-Bugzilla: 2190081
RH-Acked-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>
RH-Commit: [1/1] 1b34e2c9c61a90abb88f2df87d41f96b54e79ff7 (anisinha/rhel-cloud-init)
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.
Conflicts:
cloudinit/sources/DataSourceVultr.py
- editor directives missing in file on upstream version.
LP: #2013967
CVE: CVE-2023-1786
(cherry picked from commit a378b7e4f47375458651c0972e7cd813f6fe0a6b)
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
cloudinit/sources/DataSourceLXD.py | 9 ++++++---
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(+), 23 deletions(-)
diff --git a/cloudinit/sources/DataSourceLXD.py b/cloudinit/sources/DataSourceLXD.py
index ab440cc8..e4cae91a 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 9d7c84fb..660e9f14 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(
@@ -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
@@ -146,7 +147,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 565e1754..5c6ae8b1 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -110,7 +110,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)
@@ -132,6 +135,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)
@@ -139,9 +148,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
@@ -249,6 +263,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 a624a6fb..1326d205 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -204,7 +204,9 @@ class Init:
log_file = util.get_cfg_option_str(self.cfg, "def_log_file")
log_file_mode = util.get_cfg_option_int(self.cfg, "def_log_file_mode")
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 0447e02c..eb27198f 100644
--- a/tests/unittests/sources/test_init.py
+++ b/tests/unittests/sources/test_init.py
@@ -458,12 +458,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,
)
@@ -490,7 +502,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,
@@ -500,6 +514,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",
@@ -522,14 +537,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))
@@ -574,6 +593,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 15a7e973..a61f9df9 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.37.3

View File

@ -6,7 +6,7 @@
Name: cloud-init
Version: 23.1.1
Release: 2%{?dist}
Release: 3%{?dist}
Summary: Cloud instance init scripts
Group: System Environment/Base
@ -23,6 +23,10 @@ Patch0006: 0006-Revert-Add-native-NetworkManager-support-1224.patch
Patch0007: 0007-settings.py-update-settings-for-rhel.patch
# For bz#2182407 - cloud-init strips new line from "/etc/hostname" when processing "/var/lib/cloud/data/previous-hostname"
Patch8: ci-rhel-make-sure-previous-hostname-file-ends-with-a-ne.patch
# For bz#2182947 - Request to backport "Don't change permissions of netrules target (#2076)"
Patch9: ci-Don-t-change-permissions-of-netrules-target-2076.patch
# For bz#2190081 - CVE-2023-1786 cloud-init: sensitive data could be exposed in logs [rhel-8]
Patch10: ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch
BuildArch: noarch
@ -209,6 +213,14 @@ fi
%config(noreplace) %{_sysconfdir}/rsyslog.d/21-cloudinit.conf
%changelog
* Wed May 03 2023 Jon Maloy <jmaloy@redhat.com> - 23.1.1-3
- ci-Don-t-change-permissions-of-netrules-target-2076.patch [bz#2182947]
- ci-Make-user-vendor-data-sensitive-and-remove-log-permi.patch [bz#2190081]
- Resolves: bz#2182947
(Request to backport "Don't change permissions of netrules target (#2076)")
- Resolves: bz#2190081
(CVE-2023-1786 cloud-init: sensitive data could be exposed in logs [rhel-8])
* Tue Apr 25 2023 Jon Maloy <jmaloy@redhat.com> - 23.1.1-2
- ci-rhel-make-sure-previous-hostname-file-ends-with-a-ne.patch [bz#2182407]
- Resolves: bz#2182407