Don't loosen the permissions of the log file

This commit is contained in:
shixuantong 2023-12-14 10:40:25 +08:00
parent b33eeb1daa
commit 6b5b87737f
2 changed files with 125 additions and 1 deletions

View File

@ -0,0 +1,116 @@
From 2fb656fd991d788ed54e098815d93458e46f069e Mon Sep 17 00:00:00 2001
From: Brett Holman <brett.holman@canonical.com>
Date: Fri, 24 Nov 2023 15:54:09 +0000
Subject: [PATCH] fix: Don't loosen the permissions of the log file (#4628)
Previous implementations loosened permissions in non-default scenarios.
Fixes GH-4243
---
cloudinit/stages.py | 15 +++++++++++-
tests/unittests/test_stages.py | 45 +++++++++++++++++++++++++---------
2 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/cloudinit/stages.py b/cloudinit/stages.py
index 509d8f7..79072e7 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -8,6 +8,7 @@ import copy
import os
import sys
from collections import namedtuple
+from contextlib import suppress
from typing import Dict, Iterable, List, Optional, Set
from cloudinit import cloud, distros, handlers, helpers, importer
@@ -199,13 +200,25 @@ class Init:
def initialize(self):
self._initialize_filesystem()
+ @staticmethod
+ def _get_strictest_mode(mode_1: int, mode_2: int) -> int:
+ return mode_1 & mode_2
+
def _initialize_filesystem(self):
+ mode = 0o640
+
util.ensure_dirs(self._initial_subdirs())
log_file = util.get_cfg_option_str(self.cfg, "def_log_file")
if log_file:
# 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)
+ with suppress(OSError):
+ mode = self._get_strictest_mode(
+ 0o640, util.get_permissions(log_file)
+ )
+
+ # set file mode to the strictest of 0o640 and the current mode
+ util.ensure_file(log_file, mode, preserve_mode=False)
perms = self.cfg.get("syslog_fix_perms")
if not perms:
perms = {}
diff --git a/tests/unittests/test_stages.py b/tests/unittests/test_stages.py
index a61f9df..2f62eb7 100644
--- a/tests/unittests/test_stages.py
+++ b/tests/unittests/test_stages.py
@@ -606,23 +606,44 @@ 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(self, init, tmpdir):
+ @pytest.mark.parametrize(
+ "input, expected",
+ [
+ (0o777, 0o640),
+ (0o640, 0o640),
+ (0o606, 0o600),
+ (0o501, 0o400),
+ ],
+ )
+ def test_existing_file_permissions(self, init, tmpdir, input, expected):
"""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.
+ CIS Hardening requires file mode 0o640 or stricter. Set the
+ permissions to the subset of 0o640 and the current
+ mode.
See https://bugs.launchpad.net/cloud-init/+bug/1900837.
"""
log_file = tmpdir.join("cloud-init.log")
log_file.ensure()
- # Use a mode that will never be made the default so this test will
- # always be valid
- log_file.chmod(0o606)
+ log_file.chmod(input)
init._cfg = {"def_log_file": str(log_file)}
-
- init._initialize_filesystem()
-
- assert 0o640 == stat.S_IMODE(log_file.stat().mode)
+ with mock.patch.object(stages.util, "ensure_file") as ensure:
+ init._initialize_filesystem()
+ assert expected == ensure.call_args[0][1]
+
+
+@pytest.mark.parametrize(
+ "mode_1, mode_2, expected",
+ [
+ (0o777, 0o640, 0o640),
+ (0o640, 0o777, 0o640),
+ (0o640, 0o541, 0o440),
+ (0o111, 0o050, 0o010),
+ (0o631, 0o640, 0o600),
+ (0o661, 0o640, 0o640),
+ (0o453, 0o611, 0o411),
+ ],
+)
+def test_strictest_permissions(mode_1, mode_2, expected):
+ assert expected == stages.Init._get_strictest_mode(mode_1, mode_2)
--
2.27.0

View File

@ -1,6 +1,6 @@
Name: cloud-init Name: cloud-init
Version: 23.2.2 Version: 23.2.2
Release: 4 Release: 5
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
@ -15,6 +15,8 @@ Patch3: add-variable-to-forbid-tmp-dir.patch
Patch4: Fix-the-error-level-logs-displayed-for-the-cloud-init-local-service.patch Patch4: Fix-the-error-level-logs-displayed-for-the-cloud-init-local-service.patch
Patch5: cloud-init-23.2.2-nm-controlled.patch Patch5: cloud-init-23.2.2-nm-controlled.patch
Patch6000: backport-fix-Don-t-loosen-the-permissions-of-the-log-file-462.patch
Patch9000: do-not-generate-dsa.patch Patch9000: do-not-generate-dsa.patch
BuildRequires: pkgconfig(systemd) python3-devel python3-setuptools systemd BuildRequires: pkgconfig(systemd) python3-devel python3-setuptools systemd
@ -122,6 +124,12 @@ fi
%exclude /usr/share/doc/* %exclude /usr/share/doc/*
%changelog %changelog
* Thu Dec 14 2023 shixuantong <shixuantong1@huawei.com> - 23.2.2-5
- Type:bugfix
- ID:NA
- SUG:NA
- DESC:Don't loosen the permissions of the log file
* Wed Nov 01 2023 shixuantong <shixuantong1@huawei.com> - 23.2.2-4 * Wed Nov 01 2023 shixuantong <shixuantong1@huawei.com> - 23.2.2-4
- Type:bugfix - Type:bugfix
- ID:NA - ID:NA