From ae5df04016abf22e5733337e0f120b0bdaef6aa6 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Sat, 21 Mar 2026 22:39:48 +0000 Subject: [PATCH 1/3] refactor: generalize Distro add_user and shutdown_command Perform preliminary refactor to be used by securirity event logging. Split add_user method into separate methods: - _add_user_preprocess_kwargs: filter distro-specific args before cmd - _build_add_user_cmd: return tuple of cmd and log_command for the useradd - _post_add_user: distro-specific post-creation steps for Alpine - _user_groups_to_list: normalize group input to a list Move util.is_user check into create_user and make add_user raise on failure instead of returning bool. Subclasse now only override the separate methods instead of duplicating add_user. Refactor shutdown_command introducing a new _build_shutdown_command which is overridden in subclasses. --- cloudinit/config/cc_set_passwords.py | 4 +- cloudinit/distros/__init__.py | 163 ++++++++++-------- cloudinit/distros/alpine.py | 153 +++++----------- cloudinit/distros/bsd.py | 4 +- cloudinit/distros/freebsd.py | 35 ++-- cloudinit/distros/netbsd.py | 36 ++-- cloudinit/distros/raspberry_pi_os.py | 20 +-- cloudinit/netinfo.py | 2 +- tests/unittests/distros/test_alpine.py | 4 +- tests/unittests/distros/test_create_users.py | 34 ++-- tests/unittests/distros/test_dragonflybsd.py | 2 +- tests/unittests/distros/test_freebsd.py | 2 +- tests/unittests/distros/test_netbsd.py | 2 +- tests/unittests/distros/test_openbsd.py | 2 +- .../unittests/distros/test_raspberry_pi_os.py | 48 +++--- .../distros/test_user_data_normalize.py | 8 +- 16 files changed, 216 insertions(+), 303 deletions(-) diff --git a/cloudinit/config/cc_set_passwords.py b/cloudinit/config/cc_set_passwords.py index 22547d0fd42..4fe5668d3d4 100644 --- a/cloudinit/config/cc_set_passwords.py +++ b/cloudinit/config/cc_set_passwords.py @@ -11,7 +11,7 @@ import random import re import string -from typing import List +from typing import List, Tuple from cloudinit import features, lifecycle, subp, util from cloudinit.cloud import Cloud @@ -32,7 +32,7 @@ LOG = logging.getLogger(__name__) -def get_users_by_type(users_list: list, pw_type: str) -> list: +def get_users_by_type(users_list: list, pw_type: str) -> List[Tuple[str, str]]: """either password or type: RANDOM is required, user is always required""" return ( [] diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 9ed5540d7d7..bffef90dab5 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -659,27 +659,78 @@ def preferred_ntp_clients(self): def get_default_user(self): return self.get_option("default_user") - def add_user(self, name, **kwargs) -> bool: - """ - Add a user to the system using standard GNU tools + def _user_groups_to_list(self, groups) -> List[str]: + """Return a list of designation groups with whitespace removed.""" + if not groups: + return [] + if isinstance(groups, str): + groups = groups.split(",") + return [g.strip() for g in groups] + + def _get_elevated_roles(self, **kwargs) -> List[str]: + elevated_roles = [] + if kwargs.get("sudo"): + elevated_roles.append("sudo") + if kwargs.get("doas"): + elevated_roles.append("doas") + return elevated_roles + + def add_user(self, name, **kwargs) -> None: + """Add a user to the system.""" + + self._add_user_preprocess_kwargs(name, kwargs) + + create_groups = kwargs.pop("create_groups", True) + + if isinstance(kwargs.get("groups", None), dict): + lifecycle.deprecate( + deprecated=f"The user {name} has a 'groups' config value " + "of type dict", + deprecated_version="22.3", + extra_message="Use a comma-delimited string or " + "array instead: group1,group2.", + ) + groups = self._user_groups_to_list(kwargs.pop("groups", None)) + if groups: + primary_group = kwargs.get("primary_group") + if primary_group: + groups.append(primary_group) + + if create_groups and groups: + for group in groups: + if not util.is_group(group): + self.create_group(group) + LOG.debug("created group '%s' for user '%s'", group, name) - This should be overridden on distros where useradd is not desirable or - not available. + if "uid" in kwargs: + kwargs["uid"] = str(kwargs["uid"]) - Returns False if user already exists, otherwise True. + LOG.debug("Adding user %s", name) + cmd, log_cmd = self._build_add_user_cmd(name, groups, **kwargs) + try: + subp.subp(cmd, logstring=log_cmd) + except Exception as e: + util.logexc(LOG, "Failed to create user %s", name) + raise e + + self._post_add_user(name, groups, **kwargs) + + def _add_user_preprocess_kwargs(self, name: str, kwargs: dict) -> None: + """Preprocess kwargs in-place before building the add-user command. + + Overridden to filter for distro-specific user creation tools. """ - # XXX need to make add_user idempotent somehow as we - # still want to add groups or modify SSH keys on pre-existing - # users in the image. - if util.is_user(name): - LOG.info("User %s already exists, skipping.", name) - return False - if "create_groups" in kwargs: - create_groups = kwargs.pop("create_groups") - else: - create_groups = True + def _build_add_user_cmd( + self, name: str, groups: List[str], **kwargs + ) -> Tuple[List[str], List[str]]: + """Build the useradd command for GNU/Linux systems. + Overridden for distro-specific user-creation tools. + + Returns a (cmd, log_cmd) tuple where log_cmd has sensitive values + redacted. + """ useradd_cmd = ["useradd", name] log_useradd_cmd = ["useradd", name] if util.system_is_snappy(): @@ -694,7 +745,6 @@ def add_user(self, name, **kwargs) -> bool: "homedir": "--home", "primary_group": "--gid", "uid": "--uid", - "groups": "--groups", "passwd": "--password", "shell": "--shell", "expiredate": "--expiredate", @@ -710,42 +760,10 @@ def add_user(self, name, **kwargs) -> bool: redact_opts = ["passwd"] - # support kwargs having groups=[list] or groups="g1,g2" - groups = kwargs.get("groups") - if groups: - if isinstance(groups, str): - groups = groups.split(",") - - if isinstance(groups, dict): - lifecycle.deprecate( - deprecated=f"The user {name} has a 'groups' config value " - "of type dict", - deprecated_version="22.3", - extra_message="Use a comma-delimited string or " - "array instead: group1,group2.", - ) - - # remove any white spaces in group names, most likely - # that came in as a string like: groups: group1, group2 - groups = [g.strip() for g in groups] - - # kwargs.items loop below wants a comma delimited string - # that can go right through to the command. - kwargs["groups"] = ",".join(groups) - - primary_group = kwargs.get("primary_group") - if primary_group: - groups.append(primary_group) - - if create_groups and groups: - for group in groups: - if not util.is_group(group): - self.create_group(group) - LOG.debug("created group '%s' for user '%s'", group, name) - if "uid" in kwargs.keys(): - kwargs["uid"] = str(kwargs["uid"]) - # Check the values and create the command + if groups: + useradd_cmd.extend(["--groups", ",".join(groups)]) + log_useradd_cmd.extend(["--groups", ",".join(groups)]) for key, val in sorted(kwargs.items()): if key in useradd_opts and val and isinstance(val, str): useradd_cmd.extend([useradd_opts[key], val]) @@ -769,16 +787,13 @@ def add_user(self, name, **kwargs) -> bool: useradd_cmd.append("-m") log_useradd_cmd.append("-m") - # Run the command - LOG.debug("Adding user %s", name) - try: - subp.subp(useradd_cmd, logstring=log_useradd_cmd) - except Exception as e: - util.logexc(LOG, "Failed to create user %s", name) - raise e + return useradd_cmd, log_useradd_cmd - # Indicate that a new user was created - return True + def _post_add_user(self, name: str, groups: List[str], **kwargs) -> None: + """Hook called after the user-creation command succeeds. + + Overridden to perform distro-specific post-creation steps. + """ def add_snap_user(self, name, **kwargs): """ @@ -802,14 +817,10 @@ def add_snap_user(self, name, **kwargs): create_user_cmd, logstring=create_user_cmd, capture=True ) LOG.debug("snap create-user returned: %s:%s", out, err) - jobj = util.load_json(out) - username = jobj.get("username", None) except Exception as e: util.logexc(LOG, "Failed to create snap user %s", name) raise e - return username - def _shadow_file_has_empty_user_password(self, username) -> bool: """ Check whether username exists in shadow files with empty password. @@ -869,8 +880,11 @@ def create_user(self, name, **kwargs): if "snapuser" in kwargs: return self.add_snap_user(name, **kwargs) - # Add the user - pre_existing_user = not self.add_user(name, **kwargs) + pre_existing_user = util.is_user(name) + if pre_existing_user: + LOG.info("User %s already exists, skipping.", name) + else: + self.add_user(name, **kwargs) has_existing_password = False ud_blank_password_specified = False @@ -1021,7 +1035,6 @@ def create_user(self, name, **kwargs): ssh_util.setup_user_keys( set(cloud_keys), name, options=disable_option ) - return True def lock_passwd(self, name): """ @@ -1113,7 +1126,7 @@ def set_passwd(self, user, passwd, hashed=False): return True - def chpasswd(self, plist_in: list, hashed: bool): + def chpasswd(self, plist_in: List[Tuple[str, str]], hashed: bool): payload = ( "\n".join( (":".join([name, password]) for name, password in plist_in) @@ -1323,8 +1336,6 @@ def create_group(self, name, members=None): @classmethod def shutdown_command(cls, *, mode, delay, message): - # called from cc_power_state_change.load_power_state - command = ["shutdown", cls.shutdown_options_map[mode]] try: if delay != "now": delay = "+%d" % int(delay) @@ -1333,10 +1344,14 @@ def shutdown_command(cls, *, mode, delay, message): "power_state[delay] must be 'now' or '+m' (minutes)." " found '%s'." % (delay,) ) from e - args = command + [delay] + return cls._build_shutdown_command(mode, delay, message) + + @classmethod + def _build_shutdown_command(cls, mode, delay, message): + command = ["shutdown", cls.shutdown_options_map[mode], delay] if message: - args.append(message) - return args + command.append(message) + return command @classmethod def reload_init(cls, rcs=None): diff --git a/cloudinit/distros/alpine.py b/cloudinit/distros/alpine.py index 19912d3724f..63f15f88557 100644 --- a/cloudinit/distros/alpine.py +++ b/cloudinit/distros/alpine.py @@ -11,9 +11,9 @@ import re import stat from datetime import datetime -from typing import Any, Dict, Optional +from typing import List, Optional, Tuple -from cloudinit import distros, helpers, lifecycle, subp, util +from cloudinit import distros, helpers, subp, util from cloudinit.distros.parsers.hostname import HostnameConf from cloudinit.settings import PER_ALWAYS, PER_INSTANCE @@ -205,29 +205,16 @@ def preferred_ntp_clients(self): return self._preferred_ntp_clients - def add_user(self, name, **kwargs) -> bool: - """ - Add a user to the system using standard tools - - On Alpine this may use either 'useradd' or 'adduser' depending - on whether the 'shadow' package is installed. - - Returns False if user already exists, otherwise True. - """ - if util.is_user(name): - LOG.info("User %s already exists, skipping.", name) - return False - - if "selinux_user" in kwargs: + def _add_user_preprocess_kwargs(self, name: str, kwargs: dict) -> None: + if kwargs.pop("selinux_user", None): LOG.warning("Ignoring selinux_user parameter for Alpine Linux") - del kwargs["selinux_user"] - # If 'useradd' is available then use the generic - # add_user function from __init__.py instead. + def _build_add_user_cmd( + self, name: str, groups: List[str], **kwargs + ) -> Tuple[List[str], List[str]]: + # If 'useradd' is available then use the generic GNU implementation. if subp.which("useradd"): - return super().add_user(name, **kwargs) - - create_groups = kwargs.pop("create_groups", True) + return super()._build_add_user_cmd(name, groups, **kwargs) adduser_cmd = ["adduser", "-D"] @@ -244,102 +231,62 @@ def add_user(self, name, **kwargs) -> bool: adduser_flags = {"system": "-S"} - # support kwargs having groups=[list] or groups="g1,g2" - groups = kwargs.get("groups") - if groups: - if isinstance(groups, str): - groups = groups.split(",") - elif isinstance(groups, dict): - lifecycle.deprecate( - deprecated=f"The user {name} has a 'groups' config value " - "of type dict", - deprecated_version="22.3", - extra_message="Use a comma-delimited string or " - "array instead: group1,group2.", - ) - - # remove any white spaces in group names, most likely - # that came in as a string like: groups: group1, group2 - groups = [g.strip() for g in groups] - - # kwargs.items loop below wants a comma delimited string - # that can go right through to the command. - kwargs["groups"] = ",".join(groups) - - if kwargs.get("primary_group"): - groups.append(kwargs["primary_group"]) - - if create_groups and groups: - for group in groups: - if not util.is_group(group): - self.create_group(group) - LOG.debug("created group '%s' for user '%s'", group, name) - if "uid" in kwargs: - kwargs["uid"] = str(kwargs["uid"]) - - unsupported_busybox_values: Dict[str, Any] = { - "groups": [], - "expiredate": None, - "inactive": None, - "passwd": None, + # Build the command, skipping options unsupported by busybox adduser + # (groups, passwd, expiredate, inactive — handled in _post_add_user). + unsupported_busybox_keys = { + "groups", + "expiredate", + "inactive", + "passwd", } - - # Check the values and create the command for key, val in sorted(kwargs.items()): if key in adduser_opts and val and isinstance(val, str): adduser_cmd.extend([adduser_opts[key], val]) - elif ( - key in unsupported_busybox_values - and val - and isinstance(val, str) - ): - # Busybox's 'adduser' does not support specifying these - # options so store them for use via alternative means. - if key == "groups": - unsupported_busybox_values[key] = val.split(",") - else: - unsupported_busybox_values[key] = val elif key in adduser_flags and val: adduser_cmd.append(adduser_flags[key]) + elif key in unsupported_busybox_keys: + pass # handled in _post_add_user - # Don't create the home directory if directed so - # or if the user is a system user + # Don't create the home directory if directed so or if the user is a + # system user if kwargs.get("no_create_home") or kwargs.get("system"): adduser_cmd.append("-H") # Busybox's 'adduser' puts username at end of command adduser_cmd.append(name) - # Run the command - LOG.debug("Adding user %s", name) - try: - subp.subp(adduser_cmd) - except subp.ProcessExecutionError as e: - LOG.warning("Failed to create user %s", name) - raise e + return adduser_cmd, adduser_cmd + + def _post_add_user(self, name: str, groups: List[str], **kwargs) -> None: + # When useradd is available, the GNU implementation handles everything. + if subp.which("useradd"): + return - # Process remaining options that Busybox's 'adduser' does not support + # Busybox post-creation steps. # Separately add user to each additional group as Busybox's # 'adduser' does not support specifying additional groups. - for addn_group in unsupported_busybox_values[ - "groups" - ]: # pylint: disable=E1133 + for addn_group in groups: + addn_group = addn_group.strip() + if not addn_group: + continue LOG.debug("Adding user to group %s", addn_group) try: subp.subp(["addgroup", name, addn_group]) except subp.ProcessExecutionError as e: util.logexc( - LOG, "Failed to add user %s to group %s", name, addn_group + LOG, + "Failed to add user %s to group %s", + name, + addn_group, ) raise e - if unsupported_busybox_values["passwd"]: + passwd_val = kwargs.get("passwd") + if passwd_val: # Separately set password as Busybox's 'adduser' does # not support passing password as CLI option. - super().set_passwd( - name, unsupported_busybox_values["passwd"], hashed=True - ) + super().set_passwd(name, passwd_val, hashed=True) # Busybox's 'adduser' is hardcoded to always set the following field # values (numbered from "0") in /etc/shadow unlike 'useradd': @@ -356,10 +303,9 @@ def add_user(self, name, **kwargs) -> bool: # values directly in /etc/shadow file as Busybox's 'adduser' # does not support passing these as CLI options. - expiredate = unsupported_busybox_values["expiredate"] - inactive = unsupported_busybox_values["inactive"] + expiredate = kwargs.get("expiredate") + inactive = kwargs.get("inactive") - shadow_contents = None shadow_file = self.shadow_fn try: shadow_contents = util.load_text_file(shadow_file) @@ -420,9 +366,6 @@ def add_user(self, name, **kwargs) -> bool: LOG, "Failed to update %s for user %s", shadow_file, name ) - # Indicate that a new user was created - return True - def lock_passwd(self, name): """ Lock the password of a user, i.e., disable password logins @@ -573,14 +516,13 @@ def create_group(self, name, members=None): subp.subp(["addgroup", member, name]) LOG.info("Added user '%s' to group '%s'", member, name) - def shutdown_command(self, mode="poweroff", delay="now", message=None): - # called from cc_power_state_change.load_power_state + @classmethod + def _build_shutdown_command(cls, mode, delay, message): # Alpine has halt/poweroff/reboot, with the following specifics: # - we use them rather than the generic "shutdown" # - delay is given with "-d [integer]" # - the integer is in seconds, cannot be "now", and takes no "+" # - no message is supported (argument ignored, here) - command = [mode, "-d"] # Convert delay from minutes to seconds, as Alpine's @@ -589,14 +531,7 @@ def shutdown_command(self, mode="poweroff", delay="now", message=None): # Alpine's commands do not understand "now". command += ["0"] else: - try: - command.append(str(int(delay) * 60)) - except ValueError as e: - raise TypeError( - "power_state[delay] must be 'now' or '+m' (minutes)." - " found '%s'." % (delay,) - ) from e - + command.append(str(int(delay) * 60)) return command @staticmethod @@ -608,7 +543,7 @@ def uses_systemd(): @classmethod def manage_service( - self, action: str, service: str, *extra_args: str, rcs=None + cls, action: str, service: str, *extra_args: str, rcs=None ): """ Perform the requested action on a service. This handles OpenRC diff --git a/cloudinit/distros/bsd.py b/cloudinit/distros/bsd.py index 8433aac2aa8..ac489a8936b 100644 --- a/cloudinit/distros/bsd.py +++ b/cloudinit/distros/bsd.py @@ -1,7 +1,7 @@ import logging import platform import re -from typing import List, Optional +from typing import List, Optional, Tuple import cloudinit.net.netops.bsd_netops as bsd_netops from cloudinit import distros, helpers, net, subp, util @@ -149,7 +149,7 @@ def set_timezone(self, tz): def apply_locale(self, locale, out_fn=None): LOG.debug("Cannot set the locale.") - def chpasswd(self, plist_in: list, hashed: bool): + def chpasswd(self, plist_in: List[Tuple[str, str]], hashed: bool): for name, password in plist_in: self.set_passwd(name, password, hashed=hashed) diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py index 3eb4fbbf214..1880d6931f0 100644 --- a/cloudinit/distros/freebsd.py +++ b/cloudinit/distros/freebsd.py @@ -8,6 +8,7 @@ import os import re from io import StringIO +from typing import List, Tuple import cloudinit.distros.bsd from cloudinit import subp, util @@ -97,24 +98,15 @@ def manage_service( def _get_add_member_to_group_cmd(self, member_name, group_name): return ["pw", "usermod", "-n", member_name, "-G", group_name] - def add_user(self, name, **kwargs) -> bool: - """ - Add a user to the system using standard tools - - Returns False if user already exists, otherwise True. - """ - if util.is_user(name): - LOG.info("User %s already exists, skipping.", name) - return False - + def _build_add_user_cmd( + self, name: str, groups: List[str], **kwargs + ) -> Tuple[List[str], List[str]]: pw_useradd_cmd = ["pw", "useradd", "-n", name] log_pw_useradd_cmd = ["pw", "useradd", "-n", name] - pw_useradd_opts = { "homedir": "-d", "gecos": "-c", "primary_group": "-g", - "groups": "-G", "shell": "-s", "inactive": "-E", "uid": "-u", @@ -125,6 +117,8 @@ def add_user(self, name, **kwargs) -> bool: "no_log_init": "--no-log-init", } + if groups: + pw_useradd_cmd.extend(["-G", ",".join(groups)]) for key, val in kwargs.items(): if key in pw_useradd_opts and val and isinstance(val, (str, int)): pw_useradd_cmd.extend([pw_useradd_opts[key], str(val)]) @@ -143,22 +137,15 @@ def add_user(self, name, **kwargs) -> bool: log_pw_useradd_cmd.append("-d" + homedir) log_pw_useradd_cmd.append("-m") - # Run the command - LOG.info("Adding user %s", name) - try: - subp.subp(pw_useradd_cmd, logstring=log_pw_useradd_cmd) - except Exception: - util.logexc(LOG, "Failed to create user %s", name) - raise - # Set the password if it is provided - # For security consideration, only hashed passwd is assumed + return pw_useradd_cmd, log_pw_useradd_cmd + + def _post_add_user(self, name: str, groups: List[str], **kwargs) -> None: + # Set the password if it is provided. + # For security consideration, only hashed passwd is assumed. passwd_val = kwargs.get("passwd", None) if passwd_val is not None: self.set_passwd(name, passwd_val, hashed=True) - # Indicate that a new user was created - return True - def expire_passwd(self, user): try: subp.subp(["pw", "usermod", user, "-p", "01-Jan-1970"]) diff --git a/cloudinit/distros/netbsd.py b/cloudinit/distros/netbsd.py index 157aba06924..13f82c6dad8 100644 --- a/cloudinit/distros/netbsd.py +++ b/cloudinit/distros/netbsd.py @@ -6,7 +6,7 @@ import logging import os import platform -from typing import Any +from typing import Any, List, Tuple import cloudinit.distros.bsd from cloudinit import subp, util @@ -74,16 +74,9 @@ def __init__(self, name, cfg, paths): def _get_add_member_to_group_cmd(self, member_name, group_name): return ["usermod", "-G", group_name, member_name] - def add_user(self, name, **kwargs) -> bool: - """ - Add a user to the system using standard tools - - Returns False if user already exists, otherwise True. - """ - if util.is_user(name): - LOG.info("User %s already exists, skipping.", name) - return False - + def _build_add_user_cmd( + self, name: str, groups: List[str], **kwargs + ) -> Tuple[List[str], List[str]]: adduser_cmd = ["useradd"] log_adduser_cmd = ["useradd"] @@ -91,7 +84,6 @@ def add_user(self, name, **kwargs) -> bool: "homedir": "-d", "gecos": "-c", "primary_group": "-g", - "groups": "-G", "shell": "-s", } adduser_flags = { @@ -100,10 +92,11 @@ def add_user(self, name, **kwargs) -> bool: "no_log_init": "--no-log-init", } + if groups: + adduser_cmd.extend(["-G", ",".join(groups)]) for key, val in kwargs.items(): if key in adduser_opts and val and isinstance(val, str): adduser_cmd.extend([adduser_opts[key], val]) - elif key in adduser_flags and val: adduser_cmd.append(adduser_flags[key]) log_adduser_cmd.append(adduser_flags[key]) @@ -115,22 +108,15 @@ def add_user(self, name, **kwargs) -> bool: adduser_cmd += [name] log_adduser_cmd += [name] - # Run the command - LOG.info("Adding user %s", name) - try: - subp.subp(adduser_cmd, logstring=log_adduser_cmd) - except Exception: - util.logexc(LOG, "Failed to create user %s", name) - raise - # Set the password if it is provided - # For security consideration, only hashed passwd is assumed + return adduser_cmd, log_adduser_cmd + + def _post_add_user(self, name: str, groups: List[str], **kwargs) -> None: + # Set the password if it is provided. + # For security consideration, only hashed passwd is assumed. passwd_val = kwargs.get("passwd", None) if passwd_val is not None: self.set_passwd(name, passwd_val, hashed=True) - # Indicate that a new user was created - return True - def set_passwd(self, user, passwd, hashed=False): if hashed: hashed_pw = passwd diff --git a/cloudinit/distros/raspberry_pi_os.py b/cloudinit/distros/raspberry_pi_os.py index a6cf0f98585..a78dcb909b6 100644 --- a/cloudinit/distros/raspberry_pi_os.py +++ b/cloudinit/distros/raspberry_pi_os.py @@ -5,6 +5,7 @@ # This file is part of cloud-init. See LICENSE file for license information. import logging +from typing import List from cloudinit import net, subp from cloudinit.distros import debian @@ -67,20 +68,7 @@ def apply_locale(self, locale, out_fn=None, keyname="LANG"): else: LOG.error("Failed to set locale %s", locale) - def add_user(self, name, **kwargs) -> bool: - """ - Add a user to the system using standard GNU tools - - This should be overridden on distros where useradd is not desirable or - not available. - - Returns False if user already exists, otherwise True. - """ - result = super().add_user(name, **kwargs) - - if not result: - return result - + def _post_add_user(self, name: str, groups: List[str], **kwargs) -> None: try: subp.subp( [ @@ -90,12 +78,8 @@ def add_user(self, name, **kwargs) -> bool: ], update_env={"SUDO_USER": name}, ) - except subp.ProcessExecutionError as e: LOG.error("Failed to setup user: %s", e) - return False - - return True def generate_fallback_config(self): # Based on Photon OS implementation diff --git a/cloudinit/netinfo.py b/cloudinit/netinfo.py index 69cc56f8926..7e79db0576f 100644 --- a/cloudinit/netinfo.py +++ b/cloudinit/netinfo.py @@ -292,7 +292,7 @@ def _netdev_info_ifconfig(ifconfig_data): def netdev_info( empty="", -) -> Dict[str, Dict[str, Interface]]: +) -> Dict[str, Interface]: """return the instance's interfaces and interface data includes, interface name, link state, hardware address, and lists of ipv4 diff --git a/tests/unittests/distros/test_alpine.py b/tests/unittests/distros/test_alpine.py index 6e301d404af..966f4e7ef07 100644 --- a/tests/unittests/distros/test_alpine.py +++ b/tests/unittests/distros/test_alpine.py @@ -41,7 +41,9 @@ def test_busybox_add_user(self, m_which, m_subp, tmpdir): distro.add_user(user, lock_passwd=True) - m_subp.assert_called_with(["adduser", "-D", user]) + m_subp.assert_called_with( + ["adduser", "-D", user], logstring=["adduser", "-D", user] + ) contents = util.load_text_file(shadow_file) expected = root_entry + "\n" + user + ":!:19848::::::" + "\n" diff --git a/tests/unittests/distros/test_create_users.py b/tests/unittests/distros/test_create_users.py index 095924b106b..90b2f60bd36 100644 --- a/tests/unittests/distros/test_create_users.py +++ b/tests/unittests/distros/test_create_users.py @@ -494,7 +494,7 @@ def test_only_new_group_added(self, m_is_group, m_subp, dist, mocker): @mock.patch("cloudinit.distros.util.is_group") def test_snappy_only_new_group_added( - self, m_is_group, m_subp, dist, mocker + self, m_is_group, m_subp, dist, mocker, caplog ): mocker.patch( "cloudinit.distros.util.system_is_snappy", return_value=True @@ -638,12 +638,17 @@ def test_explicit_sudo_false(self, m_subp, dist, caplog, mocker): ) else ["INFO"] ) - assert caplog.records[1].levelname in expected_levels - assert ( + deprecation_msg = ( "The value of 'false' in user foo_user's 'sudo' " "config is deprecated in 22.2 and scheduled to be removed" " in 27.2. Use 'null' instead." - ) in caplog.text + ) + deprecation_record = None + for record in caplog.records: + if deprecation_msg in record.msg: + deprecation_record = record + assert deprecation_record, "Missing deprecation log" + assert deprecation_record.levelname in expected_levels def test_explicit_sudo_none(self, m_subp, dist, caplog, mocker): mocker.patch( @@ -736,11 +741,13 @@ def test_setup_ssh_authorized_keys_with_integer( """ssh_authorized_keys warns on non-iterable/string type.""" dist.create_user(USER, ssh_authorized_keys=-1) m_setup_user_keys.assert_called_once_with(set([]), USER) - assert caplog.records[1].levelname in ["WARNING", "DEPRECATED"] - assert ( + deprecation_msg = ( "Invalid type '' detected for 'ssh_authorized_keys'" - in caplog.text ) + deprecation_record = [ + r for r in caplog.records if deprecation_msg in r.message + ][0] + assert deprecation_record.levelname in ["WARNING", "DEPRECATED"] @mock.patch("cloudinit.ssh_util.setup_user_keys") def test_create_user_with_ssh_redirect_user_no_cloud_keys( @@ -748,11 +755,16 @@ def test_create_user_with_ssh_redirect_user_no_cloud_keys( ): """Log a warning when trying to redirect a user no cloud ssh keys.""" dist.create_user(USER, ssh_redirect_user="someuser") - assert caplog.records[1].levelname in ["WARNING", "DEPRECATED"] - assert ( + deprecation_msg = ( "Unable to disable SSH logins for foo_user given " - "ssh_redirect_user: someuser. No cloud public-keys present.\n" - ) in caplog.text + "ssh_redirect_user: someuser. No cloud public-keys present." + ) + deprecation_records = [ + record + for record in caplog.records + if deprecation_msg in record.message + ] + assert deprecation_records[0].levelname in ["WARNING", "DEPRECATED"] m_setup_user_keys.assert_not_called() @mock.patch("cloudinit.ssh_util.setup_user_keys") diff --git a/tests/unittests/distros/test_dragonflybsd.py b/tests/unittests/distros/test_dragonflybsd.py index 7222aef792b..51ea4b9f191 100644 --- a/tests/unittests/distros/test_dragonflybsd.py +++ b/tests/unittests/distros/test_dragonflybsd.py @@ -10,7 +10,7 @@ class TestDragonFlyBSD: @mock.patch(M_PATH + "subp.subp") def test_add_user(self, m_subp): distro = get_distro("dragonflybsd") - assert True is distro.add_user("me2", uid=1234, default=False) + distro.add_user("me2", uid=1234, default=False) assert [ mock.call( [ diff --git a/tests/unittests/distros/test_freebsd.py b/tests/unittests/distros/test_freebsd.py index ce844f10d9c..20b2788880a 100644 --- a/tests/unittests/distros/test_freebsd.py +++ b/tests/unittests/distros/test_freebsd.py @@ -12,7 +12,7 @@ class TestFreeBSD: @mock.patch(M_PATH + "subp.subp") def test_add_user(self, m_subp): distro = get_distro("freebsd") - assert True is distro.add_user("me2", uid=1234, default=False) + distro.add_user("me2", uid=1234, default=False) assert [ mock.call( [ diff --git a/tests/unittests/distros/test_netbsd.py b/tests/unittests/distros/test_netbsd.py index e5df07d53da..aa71510ea8d 100644 --- a/tests/unittests/distros/test_netbsd.py +++ b/tests/unittests/distros/test_netbsd.py @@ -19,7 +19,7 @@ class TestNetBSD: @mock.patch(M_PATH + "subp.subp") def test_add_user(self, m_subp): distro = get_distro("netbsd") - assert True is distro.add_user("me2", uid=1234, default=False) + distro.add_user("me2", uid=1234, default=False) assert [ mock.call( ["useradd", "-m", "me2"], logstring=["useradd", "-m", "me2"] diff --git a/tests/unittests/distros/test_openbsd.py b/tests/unittests/distros/test_openbsd.py index 78342c176c0..73eb435fab0 100644 --- a/tests/unittests/distros/test_openbsd.py +++ b/tests/unittests/distros/test_openbsd.py @@ -9,7 +9,7 @@ class TestOpenBSD: @mock.patch(M_PATH + "subp.subp") def test_add_user(self, m_subp): distro = get_distro("openbsd") - assert True is distro.add_user("me2", uid=1234, default=False) + distro.add_user("me2", uid=1234, default=False) assert [ mock.call( ["useradd", "-m", "me2"], logstring=["useradd", "-m", "me2"] diff --git a/tests/unittests/distros/test_raspberry_pi_os.py b/tests/unittests/distros/test_raspberry_pi_os.py index 2ef36acf117..8d30f45709d 100644 --- a/tests/unittests/distros/test_raspberry_pi_os.py +++ b/tests/unittests/distros/test_raspberry_pi_os.py @@ -118,43 +118,39 @@ def test_apply_locale_fallback_to_utf8(self, m_subp): ) @mock.patch(M_PATH + "subp.subp") - def test_add_user_happy_path(self, m_subp): + @mock.patch("cloudinit.distros.util.is_user", return_value=False) + def test_add_user_happy_path(self, m_is_user, m_subp): cls = fetch("raspberry_pi_os") distro = cls("raspberry-pi-os", {}, None) - # Mock the superclass add_user to return True - with mock.patch( - "cloudinit.distros.debian.Distro.add_user", return_value=True + with mock.patch.object( + distro, + "_build_add_user_cmd", + return_value=(["useradd", "pi", "-m"], ["useradd", "pi", "-m"]), ): - assert distro.add_user("pi") is True - m_subp.assert_called_once_with( + distro.add_user("pi") + m_subp.assert_any_call( ["/usr/bin/rename-user", "-f", "-s"], update_env={"SUDO_USER": "pi"}, ) @mock.patch(M_PATH + "subp.subp") - def test_add_user_existing_user(self, m_subp): + @mock.patch("cloudinit.distros.util.is_user", return_value=False) + def test_add_user_rename_fails_logs_error(self, m_is_user, m_subp, caplog): cls = fetch("raspberry_pi_os") distro = cls("raspberry-pi-os", {}, None) - with mock.patch( - "cloudinit.distros.debian.Distro.add_user", return_value=False + # First subp call (useradd) succeeds; second (rename-user) fails. + m_subp.side_effect = [ + None, + ProcessExecutionError("rename-user failed"), + ] + with mock.patch.object( + distro, + "_build_add_user_cmd", + return_value=(["useradd", "pi", "-m"], ["useradd", "pi", "-m"]), ): - assert distro.add_user("pi") is False - m_subp.assert_not_called() - - @mock.patch( - M_PATH + "subp.subp", - side_effect=ProcessExecutionError("rename-user failed"), - ) - @mock.patch("cloudinit.distros.debian.Distro.add_user", return_value=True) - def test_add_user_rename_fails_logs_error( - self, m_super_add_user, m_subp, caplog - ): - cls = fetch("raspberry_pi_os") - distro = cls("raspberry-pi-os", {}, None) - - with caplog.at_level(logging.ERROR): - assert distro.add_user("pi") is False - assert "Failed to setup user" in caplog.text + with caplog.at_level(logging.ERROR): + distro.add_user("pi") + assert "Failed to setup user" in caplog.text @mock.patch( "cloudinit.net.generate_fallback_config", diff --git a/tests/unittests/distros/test_user_data_normalize.py b/tests/unittests/distros/test_user_data_normalize.py index a1a77d1a0e9..e22741c627f 100644 --- a/tests/unittests/distros/test_user_data_normalize.py +++ b/tests/unittests/distros/test_user_data_normalize.py @@ -279,12 +279,10 @@ def test_create_snap_user(self, mock_subp): } users, _groups = self._norm(ug_cfg, distro) for user, config in users.items(): - print("user=%s config=%s" % (user, config)) - username = distro.create_user(user, **config) + distro.create_user(user, **config) snapcmd = ["snap", "create-user", "--sudoer", "--json", "joe@joe.com"] mock_subp.assert_called_with(snapcmd, capture=True, logstring=snapcmd) - assert username == "joe" @mock.patch("cloudinit.subp.subp") def test_create_snap_user_known(self, mock_subp): @@ -299,8 +297,7 @@ def test_create_snap_user_known(self, mock_subp): } users, _groups = self._norm(ug_cfg, distro) for user, config in users.items(): - print("user=%s config=%s" % (user, config)) - username = distro.create_user(user, **config) + distro.create_user(user, **config) snapcmd = [ "snap", @@ -311,7 +308,6 @@ def test_create_snap_user_known(self, mock_subp): "joe@joe.com", ] mock_subp.assert_called_with(snapcmd, capture=True, logstring=snapcmd) - assert username == "joe" @mock.patch("cloudinit.util.system_is_snappy") @mock.patch("cloudinit.util.is_group") From b7b0077e8b2749e87b456c3d8a7d468c61b6d5ea Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Sat, 21 Mar 2026 22:41:20 +0000 Subject: [PATCH 2/3] feat: owasp security event logging for user, password, and shutdown events Add a security event logging subsystem following the OWASP Logging Vocabulary Cheat Sheet. Events are emitted as JSON Lines on a new SECURITY log level which is routed to a separate log file (default: /var/log/cloud-init-security.log). Add cloudinit/log/security_event_log.py which provides: - OWASPEventType / OWASPEventLevel enums for standardised event strings - Four decorators consumed by Distro methods: sec_log_user_created, sec_log_password_changed, sec_log_password_changed_batch, sec_log_system_shutdown cloudinit/log/loggers.py now has a custom SecurityFormatter that injects an ISO-8601 timestamp into log records. Apply decorators to the Distro class, and prevent subclassing of decorated methods. Use _get_elevated_roles helper exposes sudo/doas membership to the decorator. --- cloudinit/distros/__init__.py | 16 + cloudinit/log/loggers.py | 69 ++- cloudinit/log/security_event_log.py | 186 +++++++++ doc/rtd/reference/cli.rst | 1 + doc/rtd/reference/user_files.rst | 3 + .../modules/test_combined.py | 29 +- .../modules/test_security_event_log.py | 236 +++++++++++ tests/unittests/distros/test_create_users.py | 17 +- tests/unittests/distros/test_passwords.py | 47 +++ .../distros/test_user_data_normalize.py | 8 +- tests/unittests/log/__init__.py | 1 + .../unittests/log/test_security_event_log.py | 393 ++++++++++++++++++ tests/unittests/test_log.py | 25 +- tests/unittests/util.py | 20 +- 14 files changed, 1040 insertions(+), 11 deletions(-) create mode 100644 cloudinit/log/security_event_log.py create mode 100644 tests/integration_tests/modules/test_security_event_log.py create mode 100644 tests/unittests/distros/test_passwords.py create mode 100644 tests/unittests/log/__init__.py create mode 100644 tests/unittests/log/test_security_event_log.py diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index bffef90dab5..f7d6fa371a7 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -31,6 +31,7 @@ Tuple, Type, Union, + final, ) import cloudinit.net.netops.iproute2 as iproute2 @@ -52,6 +53,12 @@ from cloudinit.distros.parsers import hosts from cloudinit.features import ALLOW_EC2_MIRRORS_ON_NON_AWS_INSTANCE_TYPES from cloudinit.lifecycle import log_with_downgradable_level +from cloudinit.log.security_event_log import ( + sec_log_password_changed, + sec_log_password_changed_batch, + sec_log_system_shutdown, + sec_log_user_created, +) from cloudinit.net import activators, dhcp, renderers from cloudinit.net.netops import NetOps from cloudinit.net.network_state import parse_net_config_data @@ -675,6 +682,8 @@ def _get_elevated_roles(self, **kwargs) -> List[str]: elevated_roles.append("doas") return elevated_roles + @final + @sec_log_user_created def add_user(self, name, **kwargs) -> None: """Add a user to the system.""" @@ -795,6 +804,8 @@ def _post_add_user(self, name: str, groups: List[str], **kwargs) -> None: Overridden to perform distro-specific post-creation steps. """ + @final + @sec_log_user_created def add_snap_user(self, name, **kwargs): """ Add a snappy user to the system using snappy tools @@ -855,6 +866,7 @@ def _shadow_file_has_empty_user_password(self, username) -> bool: return True return False + @final def create_user(self, name, **kwargs): """ Creates or partially updates the ``name`` user in the system. @@ -1106,6 +1118,7 @@ def expire_passwd(self, user): util.logexc(LOG, "Failed to set 'expire' for %s", user) raise e + @sec_log_password_changed def set_passwd(self, user, passwd, hashed=False): pass_string = "%s:%s" % (user, passwd) cmd = ["chpasswd"] @@ -1126,6 +1139,7 @@ def set_passwd(self, user, passwd, hashed=False): return True + @sec_log_password_changed_batch def chpasswd(self, plist_in: List[Tuple[str, str]], hashed: bool): payload = ( "\n".join( @@ -1335,6 +1349,8 @@ def create_group(self, name, members=None): LOG.info("Added user '%s' to group '%s'", member, name) @classmethod + @final + @sec_log_system_shutdown def shutdown_command(cls, *, mode, delay, message): try: if delay != "now": diff --git a/cloudinit/log/loggers.py b/cloudinit/log/loggers.py index a3e5a78c7f5..ae6d3c90290 100644 --- a/cloudinit/log/loggers.py +++ b/cloudinit/log/loggers.py @@ -10,7 +10,9 @@ import collections.abc # pylint: disable=import-error import copy +import datetime import io +import json import logging import logging.config import logging.handlers @@ -22,6 +24,8 @@ from typing import DefaultDict DEFAULT_LOG_FORMAT = "%(asctime)s - %(filename)s[%(levelname)s]: %(message)s" +SECURITY_LOG_FORMAT = "%(message)s" +SECURITY = logging.WARNING - 5 DEPRECATED = 35 TRACE = logging.DEBUG - 5 @@ -41,6 +45,42 @@ def trace(self, *args, **kwargs): def deprecated(self, *args, **kwargs): pass + def security(self, *args, **kwargs): + pass + + +SECURITY_LOG_FILE = "/var/log/cloud-init-security.log" + + +class SecurityOnlyFilter(logging.Filter): + """Pass only SECURITY level records.""" + + def filter(self, record): + return record.levelno == SECURITY + + +class NoSecurityFilter(logging.Filter): + """Block SECURITY level records from non-security handlers.""" + + def filter(self, record): + return record.levelno != SECURITY + + +class SecurityFormatter(logging.Formatter): + """Inject a 'datetime' field (UTC ISO-8601) into SECURITY JSON messages.""" + + def format(self, record: logging.LogRecord) -> str: + # Use record.msg instead of getMessage which formats dict to JSON + event = copy.deepcopy(record.msg) + if not isinstance(event, dict): + raise TypeError( + f"SECURITY logs expected dict but found {type(event)}: {event}" + ) + event["datetime"] = datetime.datetime.fromtimestamp( + record.created, tz=datetime.timezone.utc + ).isoformat() + return json.dumps(event, separators=(",", ":")) + def setup_basic_logging(level=logging.DEBUG, formatter=None): formatter = formatter or logging.Formatter(DEFAULT_LOG_FORMAT) @@ -48,10 +88,29 @@ def setup_basic_logging(level=logging.DEBUG, formatter=None): console = logging.StreamHandler(sys.stderr) console.setFormatter(formatter) console.setLevel(level) + console.addFilter(NoSecurityFilter()) root.addHandler(console) root.setLevel(level) +def setup_security_logging( + root: logging.Logger, log_file: str = SECURITY_LOG_FILE +) -> None: + """Attach a FileHandler routing SECURITY records to log_file if absent.""" + for h in root.handlers: + if isinstance(h, logging.FileHandler) and h.baseFilename == log_file: + return # handler already attached + + try: + handler = logging.FileHandler(log_file) + except OSError: + return + handler.setFormatter(SecurityFormatter()) + handler.addFilter(SecurityOnlyFilter()) + handler.setLevel(SECURITY) + logging.getLogger().addHandler(handler) + + def flush_loggers(root): if not root: return @@ -63,7 +122,7 @@ def flush_loggers(root): def define_extra_loggers() -> None: - """Add DEPRECATED and TRACE log levels to the logging module.""" + """Add SECURITY, DEPRECATED and TRACE log levels to the logging module.""" def new_logger(level): def log_at_level(self, message, *args, **kwargs): @@ -72,8 +131,10 @@ def log_at_level(self, message, *args, **kwargs): return log_at_level + logging.addLevelName(SECURITY, "SECURITY") logging.addLevelName(DEPRECATED, "DEPRECATED") logging.addLevelName(TRACE, "TRACE") + setattr(logging.Logger, "security", new_logger(SECURITY)) setattr(logging.Logger, "deprecated", new_logger(DEPRECATED)) setattr(logging.Logger, "trace", new_logger(TRACE)) @@ -122,6 +183,7 @@ def setup_logging(cfg=None): # Attempt to load its config. logging.config.fileConfig(log_cfg) + setup_security_logging(root_logger) # Configure warning exporter after loading logging configuration root_logger.addHandler(exporter) @@ -216,7 +278,10 @@ def configure_root_logger(): # add handler only to the root logger handler = LogExporter() handler.setLevel(logging.WARN) - logging.getLogger().addHandler(handler) + handler.addFilter(NoSecurityFilter()) + root_logger = logging.getLogger() + root_logger.addHandler(handler) + setup_security_logging(root_logger) # LogRecord allows us to report more useful information than __init__.py logging.setLogRecordFactory(CloudInitLogRecord) diff --git a/cloudinit/log/security_event_log.py b/cloudinit/log/security_event_log.py new file mode 100644 index 00000000000..f93273b4dd6 --- /dev/null +++ b/cloudinit/log/security_event_log.py @@ -0,0 +1,186 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +""" +OWASP-formatted Security Event Logging for cloud-init. + +This module provides security event logging following the OWASP Logging +Vocabulary Cheat Sheet: +https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Logging_Vocabulary_Cheat_Sheet.md + +Security events are logged in JSON Lines format with standardized fields: +- datetime: ISO 8601 timestamp with UTC offset +- appid: Application identifier (canonical.cloud-init) +- type: "security" +- event: Event type with optional parameters (e.g., user_created:root,ubuntu) +- level: INFO, WARN, or CRITICAL +- description: Human-readable summary +- host_ip: Optional IP address, included when network information is available. +""" + +import functools +import logging +from enum import Enum +from typing import Any, Dict, List, Optional, Tuple + +from cloudinit import util +from cloudinit.log import loggers + +LOG = logging.getLogger(__name__) + +# Hard-coded application identifier +APP_ID = "canonical.cloud-init" + + +class OWASPEventLevel(Enum): + """OWASP log levels.""" + + INFO = "INFO" + WARN = "WARN" + CRITICAL = "CRITICAL" + + +class OWASPEventType(Enum): + """OWASP security event types.""" + + # Authentication events [AUTHN] + AUTHN_PASSWORD_CHANGE = "authn_password_change" + + # System events [SYS] + SYS_SHUTDOWN = "sys_shutdown" + SYS_RESTART = "sys_restart" + + # User management events [USER] + USER_CREATED = "user_created" + # TODO(USER_UPDATED = "user_updated") + + +def _log_security_event( + event_type: OWASPEventType, + level: OWASPEventLevel, + description: str, + event_params: Optional[List[str]] = None, + additional_data: Optional[Dict[str, Any]] = None, +) -> None: + """ + Log a security event in OWASP format. + + :param event_type: Type of security event. + :param level: OWASP Log level (INFO, WARN, CRITICAL). + :param description: Human-readable description of the event. + :param event_params: Parameters to include in the event string. + :param additional_data: Additional context-specific data. + """ + # cloud-init is the default primary 'actor' for any system change operation + if event_params: + event_params.insert(0, "cloud-init") + else: + event_params = ["cloud-init"] + + event_str = event_type.value + if event_params: + event_str += ":" + ",".join(event_params) + event = { + "appid": APP_ID, + "type": "security", + "event": event_str, + "level": str(level.value), + "description": description, + "hostname": util.get_hostname(), + } + if additional_data: + # Merge additional non-empty data but don't overwrite core fields + for key, value in additional_data.items(): + if key not in event and value: + event[key] = value + + LOG.log(loggers.SECURITY, event) + + +def sec_log_user_created(func): + """A decorator to log a user creation event and group attributes.""" + + @functools.wraps(func) + def decorator(self, name: str, *args, **kwargs): + if not name: + raise RuntimeError( + "sec_log_user_created requires positional param name or kwarg" + ) + params = [name] + groups_msg = "" + groups = self._user_groups_to_list(kwargs.get("groups", None)) + all_groups = groups + self._get_elevated_roles(**kwargs) + if all_groups: + groups_suffix = ",".join(all_groups) + groups_msg = f" in groups: {groups_suffix}" + params.append(f"groups:{groups_suffix}") + + response = func(self, name, *args, **kwargs) + # User creation operation did not raise an Exception + _log_security_event( + event_type=OWASPEventType.USER_CREATED, + # Treat INFO level as this is prescribed provisioning at launch + level=OWASPEventLevel.INFO, + description=f"User '{name}' was created{groups_msg}", + event_params=params, + ) + return response + + return decorator + + +def sec_log_password_changed_batch(func): + @functools.wraps(func) + def decorator(self, plist_in: List[Tuple[str, str]], *args, **kwargs): + response = func(self, plist_in, *args, **kwargs) + for userid, _ in plist_in: + _log_security_event( + event_type=OWASPEventType.AUTHN_PASSWORD_CHANGE, + level=OWASPEventLevel.INFO, + description=f"Password changed for user '{userid}'", + event_params=[userid], + ) + return response + + return decorator + + +def sec_log_password_changed(func): + """A decorator logging a password change event.""" + + @functools.wraps(func) + def decorator(self, user: str, *args, **kwargs): + response = func(self, user, *args, **kwargs) + _log_security_event( + event_type=OWASPEventType.AUTHN_PASSWORD_CHANGE, + level=OWASPEventLevel.INFO, + description=f"Password changed for user '{user}'", + event_params=[user], + ) + return response + + return decorator + + +def sec_log_system_shutdown(func): + """A decorator logging a system shutdown event.""" + + @functools.wraps(func) + def decorator(cls, mode: str, delay: str, message): + if mode == "reboot": + event_type = OWASPEventType.SYS_RESTART + description = "System restart initiated" + else: + event_type = OWASPEventType.SYS_SHUTDOWN + description = "System shutdown initiated" + if message: + description += f": {message}" + + _log_security_event( + event_type=event_type, + level=OWASPEventLevel.INFO, + description=description, + additional_data={"delay": delay, "mode": mode}, + ) + return func(cls, mode=mode, delay=delay, message=message) + + return decorator diff --git a/doc/rtd/reference/cli.rst b/doc/rtd/reference/cli.rst index cefb89ab2e7..106acd4d36f 100644 --- a/doc/rtd/reference/cli.rst +++ b/doc/rtd/reference/cli.rst @@ -85,6 +85,7 @@ Logs collected include: * :file:`/var/log/cloud-init.log` * :file:`/var/log/cloud-init-output.log` +* :file:`/var/log/cloud-init-security.log` * :file:`/run/cloud-init` * :file:`/var/lib/cloud/instance/user-data.txt` * ``cloud-init`` package version diff --git a/doc/rtd/reference/user_files.rst b/doc/rtd/reference/user_files.rst index ab1b951f9d4..948b235020a 100644 --- a/doc/rtd/reference/user_files.rst +++ b/doc/rtd/reference/user_files.rst @@ -14,6 +14,9 @@ Log files - :file:`/var/log/cloud-init.log`: The primary log file. Verbose, but useful. - :file:`/var/log/cloud-init-output.log`: Captures the output from each stage. Output from user scripts goes here. +- :file:`/var/log/cloud-init-security.log`: This file contains OWASP security + events as JSON lines format implementing OWASP events including user creation + and update, password modification and system power events. Logs are appended to the files in :file:`/var/log`: files may contain logs from multiple boots. diff --git a/tests/integration_tests/modules/test_combined.py b/tests/integration_tests/modules/test_combined.py index 857023780d1..a9a11e78d13 100644 --- a/tests/integration_tests/modules/test_combined.py +++ b/tests/integration_tests/modules/test_combined.py @@ -10,6 +10,7 @@ import json import re import uuid +from datetime import datetime from pathlib import Path import pytest @@ -26,7 +27,11 @@ OS_IMAGE_TYPE, PLATFORM, ) -from tests.integration_tests.releases import CURRENT_RELEASE, IS_UBUNTU, NOBLE +from tests.integration_tests.releases import ( + CURRENT_RELEASE, + IS_UBUNTU, + NOBLE, +) from tests.integration_tests.util import ( get_feature_flag_value, get_inactive_modules, @@ -219,6 +224,28 @@ def test_rsyslog(self, class_client: IntegrationInstance): "/var/spool/rsyslog/cloudinit.log" ) + def test_security_logs(self, class_client: IntegrationInstance): + """Test security logs are set in /var/log/cloud-init-security.log.""" + client = class_client + security_logs = [] + for line in client.read_from_file( + "/var/log/cloud-init-security.log" + ).splitlines(): + sec_log = json.loads(line) + assert datetime.strptime( + sec_log.pop("datetime"), "%Y-%m-%dT%H:%M:%S.%f+00:00" + ) + security_logs.append(sec_log) + assert len(security_logs) >= 1 + assert { + "appid": "canonical.cloud-init", + "description": "User 'craig' was created", + "event": "user_created:cloud-init,craig", + "type": "security", + "level": "INFO", + "hostname": client.instance.name, + } in security_logs + def test_runcmd(self, class_client: IntegrationInstance): """Test runcmd works as expected""" client = class_client diff --git a/tests/integration_tests/modules/test_security_event_log.py b/tests/integration_tests/modules/test_security_event_log.py new file mode 100644 index 00000000000..c60efba1938 --- /dev/null +++ b/tests/integration_tests/modules/test_security_event_log.py @@ -0,0 +1,236 @@ +# This file is part of cloud-init. See LICENSE file for license information. +"""Integration tests for cloudinit.log.security_event_log. + +Validates that OWASP-formatted security event entries are written to +/var/log/cloud-init-security.log for: + - user creation (USER_CREATED / user_created) + - password change (AUTHN_PASSWORD_CHANGE / authn_password_change) + - system reboot (SYS_RESTART / sys_restart) +""" +import json +import time + +import pytest + +from tests.integration_tests.instances import IntegrationInstance +from tests.integration_tests.integration_settings import PLATFORM +from tests.integration_tests.releases import IS_UBUNTU + +SECURITY_LOG_FILE = "/var/log/cloud-init-security.log" + +# --------------------------------------------------------------------------- +# Cloud-config snippets +# --------------------------------------------------------------------------- + +USER_CREATION_USER_DATA = """\ +#cloud-config +users: + - name: sectest1 + gecos: Security Test User 1 + lock_passwd: true + - name: sectest2 + gecos: Security Test User 2 + lock_passwd: true +""" + +PASSWORD_CHANGE_USER_DATA = """\ +#cloud-config +users: + - name: pwuser + gecos: Password Change Test User + lock_passwd: false + hashed_passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/ +""" # noqa: E501 + +REBOOT_USER_DATA = """\ +#cloud-config +power_state: + delay: now + mode: reboot + message: cloud-init-security-test-reboot + timeout: 30 + condition: true +""" + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _parse_security_log(log_content: str) -> list: + """Return a list of dicts, one per non-empty JSON line in *log_content*.""" + events = [] + for line in log_content.splitlines(): + line = line.strip() + if line: + events.append(json.loads(line)) + return events + + +def _filter_events(events: list, event_prefix: str) -> list: + """Return events whose ``event`` field starts with *event_prefix*.""" + return [e for e in events if e.get("event", "").startswith(event_prefix)] + + +def _assert_owasp_fields(event: dict) -> None: + """Assert all mandatory OWASP envelope fields are present and valid.""" + assert ( + event.get("appid") == "canonical.cloud_init" + ), f"appid mismatch in event: {event}" + assert "datetime" in event, f"Missing 'datetime' in event: {event}" + # ISO 8601 requires a 'T' separator and a UTC offset + assert ( + "T" in event["datetime"] + ), f"datetime '{event['datetime']}' not ISO 8601" + assert ( + "+" in event["datetime"] or "Z" in event["datetime"] + ), f"datetime '{event['datetime']}' has no timezone offset" + assert "hostname" in event, f"Missing 'hostname' in event: {event}" + assert event.get("level") in ( + "INFO", + "WARN", + "CRITICAL", + ), f"Unexpected level '{event.get('level')}' in event: {event}" + + +def _detect_reboot(instance: IntegrationInstance) -> None: + """Block until cloud-init.log records at least two 'init-local' runs.""" + instance.instance.wait() + for _ in range(600): + try: + log = instance.read_from_file("/var/log/cloud-init.log") + if log.count("running 'init-local'") >= 2: + return + except Exception: + pass + time.sleep(1) + raise AssertionError("Instance did not reboot within the expected window") + + +# --------------------------------------------------------------------------- +# Test: user creation +# --------------------------------------------------------------------------- + + +@pytest.mark.ci +@pytest.mark.user_data(USER_CREATION_USER_DATA) +def test_user_created_security_events(client: IntegrationInstance): + """A user_created event is written for each new user created by cloud-init. + + The decorator ``sec_log_user_created`` on ``Distro.create_user`` emits a + WARN-level JSON entry per successfully created user. + """ + log_content = client.read_from_file(SECURITY_LOG_FILE) + events = _parse_security_log(log_content) + user_events = _filter_events(events, "user_created:") + + # Collect the usernames referenced in the log + logged_users = { + e["event"].split(":", 1)[1].split(",", 1)[-1] for e in user_events + } + + assert "sectest1" in logged_users, ( + f"No user_created entry for 'sectest1'. " + f"user_created events found: {user_events}" + ) + assert "sectest2" in logged_users, ( + f"No user_created entry for 'sectest2'. " + f"user_created events found: {user_events}" + ) + + # Validate every sectest* entry in detail + for username in ("sectest1", "sectest2"): + matching = [ + e + for e in user_events + if e["event"] == f"user_created:cloud-init,{username}" + ] + assert matching, f"Exact user_created event not found for '{username}'" + event = matching[0] + _assert_owasp_fields(event) + assert event["level"] == "WARN", ( + f"user_created event for '{username}' should be WARN, " + f"got '{event['level']}'" + ) + assert ( + event["description"] == f"User '{username}' was created" + ), f"Unexpected description in event: {event}" + + +# --------------------------------------------------------------------------- +# Test: password change +# --------------------------------------------------------------------------- + + +@pytest.mark.ci +@pytest.mark.user_data(PASSWORD_CHANGE_USER_DATA) +def test_password_change_security_events(client: IntegrationInstance): + """An authn_password_change event is written when Distro.set_passwd runs. + + The decorator ``sec_log_password_changed`` on ``Distro.set_passwd`` emits + an INFO-level JSON entry whenever a user password is set via + ``create_user`` (triggered by ``plain_text_passwd`` or ``hashed_passwd`` + in user-data). + """ + log_content = client.read_from_file(SECURITY_LOG_FILE) + events = _parse_security_log(log_content) + pw_events = _filter_events(events, "authn_password_change:") + + assert pw_events, ( + "Expected at least one authn_password_change event. " + f"Security log:\n{log_content}" + ) + + event = pw_events[0] + _assert_owasp_fields(event) + assert ( + event["level"] == "INFO" + ), f"authn_password_change should be INFO, got '{event['level']}'" + # The event string must start with the cloud-init actor prefix + assert event["event"].startswith( + "authn_password_change:cloud-init," + ), f"Unexpected event string format: {event['event']}" + + +# --------------------------------------------------------------------------- +# Test: system reboot +# --------------------------------------------------------------------------- + + +@pytest.mark.skipif(not IS_UBUNTU, reason="Only tested on Ubuntu") +@pytest.mark.skipif( + PLATFORM != "lxd_container", + reason="Reboot detection is most reliable on lxd_container", +) +def test_reboot_security_event(session_cloud): + """A sys_restart event is written when power_state triggers a reboot. + + The decorator ``sec_log_system_shutdown`` on ``Distro.shutdown_command`` + emits an INFO-level JSON entry before the shutdown command runs. + """ + with session_cloud.launch( + user_data=REBOOT_USER_DATA, + wait=False, + ) as instance: + _detect_reboot(instance) + log_content = instance.read_from_file(SECURITY_LOG_FILE) + + events = _parse_security_log(log_content) + restart_events = _filter_events(events, "sys_restart:") + + assert restart_events, ( + "Expected at least one sys_restart event in security log. " + f"Log content:\n{log_content}" + ) + + event = restart_events[0] + _assert_owasp_fields(event) + assert ( + event["event"] == "sys_restart:cloud-init" + ), f"Unexpected event string: {event['event']}" + assert ( + event["level"] == "INFO" + ), f"sys_restart event should be INFO, got '{event['level']}'" + assert ( + event["description"] == "System restart initiated" + ), f"Unexpected description: {event['description']}" diff --git a/tests/unittests/distros/test_create_users.py b/tests/unittests/distros/test_create_users.py index 90b2f60bd36..580e9c3e88c 100644 --- a/tests/unittests/distros/test_create_users.py +++ b/tests/unittests/distros/test_create_users.py @@ -6,6 +6,7 @@ import pytest from cloudinit import distros, features, lifecycle, ssh_util +from cloudinit.log import loggers from tests.unittests.helpers import get_distro, mock from tests.unittests.util import abstract_to_concrete @@ -447,6 +448,11 @@ def test_create_passwd_existing_user( for log in expected_logs: assert log in caplog.text assert m_subp.call_args_list == expected + if "passwd" not in create_kwargs: + assert ( + "authn_password_change:cloud-init,foo_user" + in caplog.records[-1].msg["event"] + ) @mock.patch("cloudinit.distros.util.is_group") def test_group_added(self, m_is_group, m_subp, dist, mocker): @@ -511,6 +517,10 @@ def test_snappy_only_new_group_added( mock.call(["passwd", "-l", USER]), ] assert m_subp.call_args_list == expected + assert ( + caplog.records[-1].msg["event"] + == "user_created:cloud-init,foo_user,groups:group1,existing_group" + ) @mock.patch("cloudinit.distros.util.is_group") def test_create_groups_with_whitespace_string( @@ -643,12 +653,17 @@ def test_explicit_sudo_false(self, m_subp, dist, caplog, mocker): "config is deprecated in 22.2 and scheduled to be removed" " in 27.2. Use 'null' instead." ) - deprecation_record = None + deprecation_record = security_record = None for record in caplog.records: if deprecation_msg in record.msg: deprecation_record = record + if record.levelno == loggers.SECURITY: + security_record = record assert deprecation_record, "Missing deprecation log" assert deprecation_record.levelname in expected_levels + assert security_record, "Missing security log" + security_event = security_record.msg + assert security_event["event"] == "user_created:cloud-init,foo_user" def test_explicit_sudo_none(self, m_subp, dist, caplog, mocker): mocker.patch( diff --git a/tests/unittests/distros/test_passwords.py b/tests/unittests/distros/test_passwords.py new file mode 100644 index 00000000000..0da18bb90bf --- /dev/null +++ b/tests/unittests/distros/test_passwords.py @@ -0,0 +1,47 @@ +# This file is part of cloud-init. See LICENSE file for license information. + + +import pytest + +from tests.unittests.helpers import get_distro, mock + +USER = "foo_user" + + +@mock.patch("cloudinit.distros.subp.subp") +class TestChpasswd: + + @pytest.mark.parametrize( + "plist_in,hashed,expected", + [ + pytest.param( + (("u1", "pw1"), ("u2", "pw2")), + False, + [ + mock.call(["chpasswd"], data="u1:pw1\nu2:pw2\n"), + ], + id="clear_text_passwords", + ), + pytest.param( + (("u1", "hash1"), ("u2", "hash2")), + True, + [ + mock.call(["chpasswd", "-e"], data="u1:hash1\nu2:hash2\n"), + ], + id="hashed_passwords", + ), + ], + ) + def test_create_options( + self, + m_subp, + plist_in, + hashed, + expected, + caplog, + ): + dist = get_distro("ubuntu") + dist.chpasswd(plist_in=plist_in, hashed=hashed) + assert m_subp.call_args_list == expected + for user in ("u1", "u2"): + assert f"authn_password_change:cloud-init,{user}" in caplog.text diff --git a/tests/unittests/distros/test_user_data_normalize.py b/tests/unittests/distros/test_user_data_normalize.py index e22741c627f..2c483c9d055 100644 --- a/tests/unittests/distros/test_user_data_normalize.py +++ b/tests/unittests/distros/test_user_data_normalize.py @@ -267,7 +267,7 @@ def test_users_dict(self): assert {"default": False} == users["bob"] @mock.patch("cloudinit.subp.subp") - def test_create_snap_user(self, mock_subp): + def test_create_snap_user(self, mock_subp, caplog): mock_subp.side_effect = [ ('{"username": "joe", "ssh-key-count": 1}\n', "") ] @@ -283,9 +283,11 @@ def test_create_snap_user(self, mock_subp): snapcmd = ["snap", "create-user", "--sudoer", "--json", "joe@joe.com"] mock_subp.assert_called_with(snapcmd, capture=True, logstring=snapcmd) + event = caplog.records[-1].msg + assert "user_created:cloud-init,joe" == event["event"] @mock.patch("cloudinit.subp.subp") - def test_create_snap_user_known(self, mock_subp): + def test_create_snap_user_known(self, mock_subp, caplog): mock_subp.side_effect = [ ('{"username": "joe", "ssh-key-count": 1}\n', "") ] @@ -308,6 +310,8 @@ def test_create_snap_user_known(self, mock_subp): "joe@joe.com", ] mock_subp.assert_called_with(snapcmd, capture=True, logstring=snapcmd) + event = caplog.records[-1].msg + assert "user_created:cloud-init,joe" == event["event"] @mock.patch("cloudinit.util.system_is_snappy") @mock.patch("cloudinit.util.is_group") diff --git a/tests/unittests/log/__init__.py b/tests/unittests/log/__init__.py new file mode 100644 index 00000000000..da6365a5941 --- /dev/null +++ b/tests/unittests/log/__init__.py @@ -0,0 +1 @@ +# This file is part of cloud-init. See LICENSE file for license information. diff --git a/tests/unittests/log/test_security_event_log.py b/tests/unittests/log/test_security_event_log.py new file mode 100644 index 00000000000..8ba113bde2c --- /dev/null +++ b/tests/unittests/log/test_security_event_log.py @@ -0,0 +1,393 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +"""Tests for cloudinit.log.security_event_log""" + +import json +import logging + +import pytest + +from cloudinit.log import loggers, security_event_log +from cloudinit.log.loggers import SecurityFormatter +from cloudinit.log.security_event_log import ( + OWASPEventLevel, + OWASPEventType, + sec_log_password_changed, + sec_log_password_changed_batch, + sec_log_system_shutdown, + sec_log_user_created, +) +from cloudinit.util import get_hostname +from tests.unittests.util import MockDistro + +MPATH = "cloudinit.log.security_event_log." + + +class TestBuildEventString: + """Tests for _build_event_string function.""" + + @pytest.mark.parametrize( + "event_type,params,expected", + [ + pytest.param( + OWASPEventType.SYS_SHUTDOWN, + None, + "sys_shutdown:cloud-init", + id="no_params", + ), + pytest.param( + OWASPEventType.AUTHN_PASSWORD_CHANGE, + ["testuser"], + "authn_password_change:cloud-init,testuser", + id="single_param", + ), + pytest.param( + OWASPEventType.USER_CREATED, + ["newuser", "groups:wheel"], + "user_created:cloud-init,newuser,groups:wheel", + id="multiple_params", + ), + ], + ) + def test_event_string_formatting( + self, event_type, params, expected, caplog + ): + """Test event string formatting with various parameter combinations.""" + security_event_log._log_security_event( + event_type=event_type, + level=OWASPEventLevel.WARN, + description="Test Descr", + event_params=params, + ) + event = caplog.records[0].msg + assert event["appid"] == "canonical.cloud-init" + assert event["level"] == "WARN" + assert event["event"] == expected + + def test_additional_data_does_not_overwrite_core_fields(self, caplog): + """Test that additional data cannot overwrite core fields.""" + security_event_log._log_security_event( + event_type=OWASPEventType.USER_CREATED, + level=OWASPEventLevel.INFO, + description="Test event", + additional_data={"appid": "malicious.app", "level": "CRITICAL"}, + ) + event = caplog.records[0].msg + assert event["appid"] == "canonical.cloud-init" + assert event["level"] == "INFO" + + +class TestLogSecurityEvent: + """Tests for _log_security_event function.""" + + def test_event_with_additional_data(self, caplog): + """Test event includes additional data when provided.""" + security_event_log._log_security_event( + event_type=OWASPEventType.USER_CREATED, + level=OWASPEventLevel.INFO, + description="Test event", + additional_data={"groups": "wheel", "shell": "/bin/bash"}, + ) + event = caplog.records[0].msg + assert event["groups"] == "wheel" + assert event["shell"] == "/bin/bash" + + def test_writes_json_to_file(self, caplog): + """Test that event is written to log file with OWASP fields.""" + with caplog.at_level(loggers.SECURITY): + security_event_log._log_security_event( + event_type=OWASPEventType.USER_CREATED, + level=OWASPEventLevel.INFO, + description="User created successfully", + event_params=["testuser"], + ) + event = caplog.records[0].msg + + assert event["event"] == "user_created:cloud-init,testuser" + assert event["level"] == "INFO" + assert event["appid"] == "canonical.cloud-init" + assert event["description"] == "User created successfully" + assert "hostname" in event + + def test_appends_multiple_events(self, caplog): + """Test that multiple events are appended to the log file.""" + with caplog.at_level(loggers.SECURITY): + security_event_log._log_security_event( + event_type=OWASPEventType.USER_CREATED, + level=OWASPEventLevel.INFO, + description="First user", + event_params=["cloud-init", "user1"], + ) + + security_event_log._log_security_event( + event_type=OWASPEventType.USER_CREATED, + level=OWASPEventLevel.INFO, + description="Second user", + event_params=["cloud-init", "user2"], + ) + + assert len(caplog.records) == 2 + event1 = caplog.records[0].msg + event2 = caplog.records[1].msg + assert "user1" in event1["event"] + assert "user2" in event2["event"] + + +class TestUserCreatedEvent: + """Tests for sec_log_user_created function.""" + + @pytest.mark.parametrize( + "uc_kwargs,event_id,description", + [ + pytest.param( + {}, + "user_created:cloud-init,testuser", + "User 'testuser' was created", + id="user_created_logs_event", + ), + pytest.param( + {"groups": ["grp1", "grp2"]}, + "user_created:cloud-init,testuser,groups:grp1,grp2", + "User 'testuser' was created in groups: grp1,grp2", + id="user_created_with_groups_logs_event", + ), + pytest.param( + {"sudo": True, "groups": ["grp1"]}, + "user_created:cloud-init,testuser,groups:grp1,sudo", + "User 'testuser' was created in groups: grp1,sudo", + id="user_created_with_sudo_and_groups_logs_event", + ), + pytest.param( + {"doas": True, "groups": ["grp2"]}, + "user_created:cloud-init,testuser,groups:grp2,doas", + "User 'testuser' was created in groups: grp2,doas", + id="user_created_with_doas_and_groups_logs_event", + ), + ], + ) + def test_logs_user_created_event( + self, uc_kwargs, event_id, description, caplog + ): + """Test logging a user creation event.""" + + class DecoratedSetPasswordTest(MockDistro): + + @sec_log_user_created + def user_created_decorator_test(self, name, **kwargs): + return + + with caplog.at_level(loggers.SECURITY): + DecoratedSetPasswordTest().user_created_decorator_test( + name="testuser", + **uc_kwargs, + ) + + assert { + "appid": "canonical.cloud-init", + "event": event_id, + "description": description, + "hostname": get_hostname(), + "level": "INFO", + "type": "security", + } == caplog.records[0].msg + + +class TestPasswordChangedEvent: + """Tests for sec_log_password_changed function.""" + + def test_logs_password_changed_event(self, caplog): + """Test logging a password change event.""" + + class DecoratedSetPasswordTest: + @sec_log_password_changed + def set_passwd(self, user): + pass + + method_test = DecoratedSetPasswordTest() + + with caplog.at_level(loggers.SECURITY): + method_test.set_passwd(user="testuser") + method_test.set_passwd("testuser") # Test positional params + + expected_value = { + "appid": "canonical.cloud-init", + "event": "authn_password_change:cloud-init,testuser", + "description": "Password changed for user 'testuser'", + "hostname": get_hostname(), + "level": "INFO", + "type": "security", + } + + for record in caplog.records: + assert expected_value == record.msg + + +class TestPasswordChangedBatchEvent: + """Tests for sec_log_password_changed_batch function.""" + + def test_logs_password_changed_event_for_each_user(self, caplog): + """Test logging a password change event.""" + + class DecoratedChpasswdTest: + @sec_log_password_changed_batch + def chpasswd(self, plist_in): + pass + + method_test = DecoratedChpasswdTest() + + with caplog.at_level(loggers.SECURITY): + method_test.chpasswd(plist_in=(("testuser", "pw1"),)) + + expected_value = { + "appid": "canonical.cloud-init", + "event": "authn_password_change:cloud-init,testuser", + "description": "Password changed for user 'testuser'", + "hostname": get_hostname(), + "level": "INFO", + "type": "security", + } + + for record in caplog.records: + assert expected_value == record.msg + + +class TestSystemShutdownEvent: + """Tests for sec_log_system_shutdown function.""" + + @pytest.mark.parametrize( + "mode,delay,message,expected_event,expected_descr", + [ + pytest.param( + "poweroff", + "+5", + "", + "sys_shutdown:cloud-init", + "System shutdown initiated", + id="poweroff_with_delay", + ), + pytest.param( + "reboot", + "now", + None, + "sys_restart:cloud-init", + "System restart initiated", + id="reboot_immediate", + ), + pytest.param( + "reboot", + "now", + "Restart FTW", + "sys_restart:cloud-init", + "System restart initiated: Restart FTW", + id="reboot_immediate", + ), + ], + ) + def test_logs_system_shutdown_event( + self, + mode, + delay, + message, + expected_event, + expected_descr, + caplog, + ): + """Test logging a system shutdown event.""" + + class DecoratedShutDownTest: + @classmethod + @sec_log_system_shutdown + def shutdown_test(cls, mode, delay, message): + pass + + method_test = DecoratedShutDownTest() + + with caplog.at_level(loggers.SECURITY): + method_test.shutdown_test( + mode=mode, + delay=delay, + message=message, + ) + + expected = { + "appid": "canonical.cloud-init", + "delay": delay, + "description": expected_descr, + "event": expected_event, + "hostname": get_hostname(), + "level": "INFO", + "mode": "reboot", + "type": "security", + } + if mode != "reboot": + expected["mode"] = mode + assert expected == caplog.records[0].msg + + +class TestSecurityFormatter: + """Tests for SecurityFormatter.""" + + def _make_record(self, msg) -> logging.LogRecord: + record = logging.LogRecord( + name="test", + level=loggers.SECURITY, + pathname="", + lineno=0, + msg=msg, + args=(), + exc_info=None, + ) + return record + + def test_injects_datetime_into_json_message(self): + """Formatter adds 'datetime' and formats valid JSON messages.""" + record = self._make_record({"appid": "canonical.cloud-init"}) + result = json.loads(SecurityFormatter().format(record)) + assert "datetime" in result + # ISO 8601: contains 'T' separator and UTC offset + assert "T" in result["datetime"] + assert "+" in result["datetime"] or "Z" in result["datetime"] + + def test_errors_on_non_dict(self): + """Non-Dict messages are returned unchanged.""" + record = self._make_record("not dict") + with pytest.raises(TypeError, match="SECURITY logs expected dict but"): + SecurityFormatter().format(record) + + def test_datetime_uses_record_created_timestamp(self): + """The injected datetime reflects the log record's creation time.""" + import datetime as dt + + record = self._make_record({}) + result = json.loads(SecurityFormatter().format(record)) + expected = dt.datetime.fromtimestamp( + record.created, tz=dt.timezone.utc + ).isoformat() + assert result["datetime"] == expected + + +class TestEventTypeEnums: + """Tests for event type enum values.""" + + @pytest.mark.parametrize( + "event_type,expected_value", + [ + pytest.param( + OWASPEventType.AUTHN_PASSWORD_CHANGE, + "authn_password_change", + id="authn_password_change", + ), + pytest.param( + OWASPEventType.SYS_SHUTDOWN, "sys_shutdown", id="sys_shutdown" + ), + pytest.param( + OWASPEventType.SYS_RESTART, "sys_restart", id="sys_restart" + ), + pytest.param( + OWASPEventType.USER_CREATED, "user_created", id="user_created" + ), + ], + ) + def test_event_type_values(self, event_type, expected_value): + """Test event type enum values.""" + assert event_type.value == expected_value diff --git a/tests/unittests/test_log.py b/tests/unittests/test_log.py index 4b1998fb4a0..94abea3bad9 100644 --- a/tests/unittests/test_log.py +++ b/tests/unittests/test_log.py @@ -4,6 +4,7 @@ import datetime import io +import json import logging import time from typing import cast @@ -97,6 +98,13 @@ def test_trace_log_level(self, caplog): assert "TRACE" == caplog.records[0].levelname assert "trace message" in caplog.text + def test_security_log_level(self, caplog): + logger = cast(loggers.CustomLoggerType, logging.getLogger()) + logger.setLevel(logging.NOTSET) + logger.security("security message") + assert "SECURITY" == caplog.records[0].levelname + assert "security message" in caplog.text + @pytest.mark.parametrize( "expected_log_level, deprecation_info_boundary", ( @@ -165,8 +173,23 @@ def test_log_deduplication(self, caplog): assert 2 == len(caplog.records) -def test_logger_prints_to_stderr(capsys): +def test_logger_prints_to_stderr(capsys, caplog): message = "to stdout" loggers.setup_basic_logging() logging.getLogger().warning(message) assert message in capsys.readouterr().err + + +def test_logger_prints_security_as_json_lines(tmp_path, capsys, caplog): + """Security logs accepts dict as payload logs JSON lines.""" + log_file = tmp_path / "cloud-init-output.log" + message = {"key": "value"} # Security logs expect python dict + loggers.setup_basic_logging() + root = cast(loggers.CustomLoggerType, logging.getLogger()) + loggers.setup_security_logging(root=root, log_file=str(log_file)) + root.security(message) + message_json = json.dumps(message, separators=(",", ":")) + logged_event = json.loads(log_file.read_text()) + assert logged_event.pop("datetime") + assert logged_event == message + assert message_json not in capsys.readouterr().err diff --git a/tests/unittests/util.py b/tests/unittests/util.py index 72573075b21..689fb9ef5d7 100644 --- a/tests/unittests/util.py +++ b/tests/unittests/util.py @@ -4,6 +4,7 @@ from cloudinit import cloud, distros, helpers from cloudinit.config import Config +from cloudinit.log import security_event_log from cloudinit.net.dhcp import IscDhclient from cloudinit.sources import DataSource, DataSourceHostname from cloudinit.sources.DataSourceNone import DataSourceNone @@ -143,13 +144,21 @@ def update_hostname(self, hostname, fqdn, prev_hostname_fn): def update_etc_hosts(self, hostname, fqdn): pass - def add_user(self, name, **kwargs): + @security_event_log.sec_log_user_created # type: ignore[misc] + def add_user( + self, name, **kwargs + ): # pylint: disable=overridden-final-method pass - def add_snap_user(self, name, **kwargs): + @security_event_log.sec_log_user_created # type: ignore[misc] + def add_snap_user( + self, name, **kwargs + ): # pylint: disable=overridden-final-method return "snap_user" - def create_user(self, name, **kwargs): + def create_user( # type: ignore[misc] + self, name, **kwargs + ): # pylint: disable=overridden-final-method return True def lock_passwd(self, name): @@ -170,7 +179,10 @@ def write_sudo_rules(self, user, rules, sudo_file=None): def create_group(self, name, members=None): pass - def shutdown_command(self, *, mode, delay, message): + @security_event_log.sec_log_system_shutdown # type: ignore[misc] + def shutdown_command( + self, *, mode, delay, message + ): # pylint: disable=overridden-final-method pass def package_command(self, command, args=None, pkgs=None): From 86f1ec745f5d860383f762b097327eea18265c7b Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 24 Mar 2026 13:44:19 -0600 Subject: [PATCH 3/3] ci: create debs based on current PR number --- .github/workflows/24-pr-integration.yml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/24-pr-integration.yml b/.github/workflows/24-pr-integration.yml index f25849eed3f..ef838a01054 100644 --- a/.github/workflows/24-pr-integration.yml +++ b/.github/workflows/24-pr-integration.yml @@ -1,6 +1,7 @@ name: "PR: Integration" on: + workflow_dispatch: pull_request: branches: - main @@ -45,12 +46,14 @@ jobs: - name: Build package run: | DEB_BUILD_OPTIONS=nocheck ./packages/bddeb -d --release ${{ env.RELEASE }} - cp cloud-init-base*.deb ${{ runner.temp }} + DEB_PKG=cloud-init-base-PR-${GITHUB_REF_NAME%/*}-${{ env.RELEASE }}.deb + cp cloud-init-base*.deb ${{ runner.temp }}/$DEB_PKG + echo "DEB_PKG=${DEB_PKG}" >> $GITHUB_ENV - name: Archive debs as artifacts uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: 'cloud-init-${{ env.RELEASE }}-deb' - path: '${{ runner.temp }}/cloud-init-base*.deb' + path: '${{ runner.temp }}/${{ env.DEB_PKG }}' retention-days: 3 - name: Setup LXD uses: canonical/setup-lxd@8c6a87bfb56aa48f3fb9b830baa18562d8bfd4ee # v1 @@ -59,12 +62,12 @@ jobs: - name: Verify deb package run: | ls -hal '${{ runner.temp }}' - echo ${{ runner.temp }}/cloud-init-base*.deb || true - ls -hal ${{ runner.temp }}/cloud-init-base*.deb || true + echo ${{ runner.temp }}/${DEB_PKG} || true + ls -hal ${{ runner.temp }}/${DEB_PKG} || true - name: Set up Pycloudlib run: | ssh-keygen -P "" -q -f ~/.ssh/id_rsa echo "[lxd]" > /home/$USER/.config/pycloudlib.toml - name: Run integration Tests run: | - CLOUD_INIT_CLOUD_INIT_SOURCE="$(ls ${{ runner.temp }}/cloud-init-base*.deb)" CLOUD_INIT_OS_IMAGE=${{ env.RELEASE }} tox -e integration-tests-ci -- --color=yes tests/integration_tests/ + CLOUD_INIT_CLOUD_INIT_SOURCE="$(ls ${{ runner.temp }}/${{ env.DEB_PKG }})" CLOUD_INIT_OS_IMAGE=${{ env.RELEASE }} tox -e integration-tests-ci -- --color=yes tests/integration_tests/