Implement OWASP security logging. by blackboxsw · Pull Request #6689 · canonical/cloud-init

blackboxsw · 2026-01-26T21:48:31Z

Provide an initial spike for logging OWASP formatted events in cloud-init for discussion. Integration tests will be added upon agreement for security logging procedures.

Proposed Commit Message

    feat: add OSWAP security event logging for user creation and system restart
    
    Implement OWASP structured logs in /var/log/cloud-init-security.log.
    
    Add security-related operations performed by cloud-init on behalf
    of user-data or platform meta-data:
    - user creation
    - user password change
    - system restart
    - system shutdown
  
    Default security log file can be changed by setting an alternative value
    for security_log_file in /etc/cloud/cloud.cfg(.d/*.cfg).

Additional Context

A followup PR will provide USER update functionality due to groups: { admingrp: [root, sys]} because this will require a refactor of Distro.create_group for multiple distros

Test Steps

CLOUD_INIT_CLOUD_INIT_OS_IMAGE=resolute tox -e integration-tests -- tests/integration_tests/modules/test_combined.py::TestCombined::test_security_logs

## Merge type

- [ ] Squash merge using "Proposed Commit Message"
- [x] Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

…estart Implement OWASP structured logs in /var/log/cloud-init-security.log. Add security-related operations performed by cloud-init on behalf of user-data or platform meta-data: - user creation - user password change - system restart - system shutdown Default security log file can be changed by setting an alternative value for security_log_file in /etc/cloud/cloud.cfg(.d/*.cfg).

blackboxsw · 2026-01-27T16:03:59Z

cc: @dbungert

holmanb

Thanks for this @blackboxsw. First pass.

If the goal is for this to be cross-distro, then implementing this in the distro class doesn't seem optimal because distro methods are expected to be (and in this case are) overridden.

Also, is there a reason that the logger package cannot be used? Manually writing to files seems undesirable - I would have expected this to define a custom logger for this purpose.

Thank you for the type annotations. Besides just annotating the types, I'd like to see if we can also avoid unnecessary complexity - something that annotations can assist with. For example rather than checking an Optional parameter for trutheyness, we could instead avoid the possibility of None and pass a falsey value instead.

holmanb · 2026-01-26T23:36:44Z

cloudinit/log/security_event_log.py

+        # Create file with restricted permissions if it doesn't exist
+        if not os.path.exists(log_file):
+            util.ensure_file(log_file, mode=0o600, preserve_mode=False)
+
+        util.append_file(log_file, json_line, disable_logging=True)


This doesn't seem ideal. Was Python's builtin logger considered?

This complicates code in util and makes for a bunch of otherwise unnecessary logger file parameters.

I've adapted this to use python logging and a new SECURITY level log with custom handler which writes to
/var/log/cloud-init-security.log

holmanb · 2026-01-27T00:11:14Z

cloudinit/distros/__init__.py

+            sec_log_user_created(
+                userid="cloud-init",
+                new_userid=name,
+                attributes={"snapuser": True, "sudo": True},
+            )


Intermixing code of different different purposes like this will lead to difficulty maintaining and auditing the code. I would prefer if we could find a cleaner design.

good point, let's go decorator route instead,

holmanb · 2026-01-27T00:13:19Z

cloudinit/log/security_event_log.py

+LOG = logging.getLogger(__name__)
+
+# Hard-coded application identifier per spec
+APP_ID = "canonical.cloud_init"


Why the underscore? If no hyphen is allowed then I would prefer "canonical.cloudinit" over this.

Adapted to prefer hyphenated like our product name.

holmanb · 2026-01-27T00:18:02Z

cloudinit/util.py

    rotated_logs = []
    if not cfg or not isinstance(cfg, dict):
        return logs
-    default_log = cfg.get("def_log_file")


The changes in this file seem unnecessary.

cloudinit/log/security_event_log.py

holmanb · 2026-01-27T00:22:16Z

cloudinit/log/security_event_log.py

+    userid: str,
+    new_userid: str,
+    attributes: Optional[Dict[str, Any]] = None,
+    log_file: Optional[str] = DEFAULT_SECURITY_LOG,


Why the log file parameters? This seems unnecessary.

holmanb · 2026-01-27T00:25:03Z

cloudinit/log/security_event_log.py

+def sec_log_user_created(
+    userid: str,
+    new_userid: str,
+    attributes: Optional[Dict[str, Any]] = None,


Optional parameters often complicate type annotations unnecessarily. Can we restructure this to be cleaner? Same comment repeated throughout this code.

holmanb · 2026-01-27T16:11:19Z

cloudinit/distros/__init__.py

            util.logexc(LOG, "Failed to set password for %s", user)
            raise e

+        # Log security event for password change


This comment seems unnecessary.

holmanb · 2026-01-27T16:11:26Z

cloudinit/distros/__init__.py

        cmd = ["chpasswd"] + (["-e"] if hashed else [])
        subp.subp(cmd, data=payload)

+        # Log security event for each password change


This comment seems unnecessary.

holmanb · 2026-01-27T16:30:29Z

cloudinit/log/security_event_log.py

+    if params:
+        # Filter out None values and convert to strings
+        filtered_params = [str(p) for p in params if p is not None]
+        if filtered_params:


This conditional is unnecessary.

github-actions · 2026-02-25T00:39:17Z

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging blackboxsw, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag blackboxsw to reopen it.)

Replace LevelBasedFormatter with handler-level filters. SecurityOnlyFilter and NoSecurityFilter are applied at the handler level so SECURITY records flow only to a dedicated FileHandler on cloud-init-output.log using SECURITY_LOG_FORMAT, and are blocked from stderr and LogExporter. setup_security_logging() silently no-ops on OSError so unprivileged test environments are unaffected. Update test_logger_prints_security_as_ json_lines to assert file output and absence from stderr.

Use netdev_info to discover any interface with a global scope IP address. If no IPv4 address is present, return IPv6. If no network devices are up or no global addresses are present, avoid adding host_ip to event logs. Additionally, add "type": "security" to all events.

…ogger

…down Correct type hint for netdev_info

blackboxsw · 2026-03-05T05:19:33Z

I believe I have addressed all comments and this is ready for re-review.

blackboxsw added 2 commits January 26, 2026 14:40

doc: document security_log_file base config setting

a4bce71

github-actions bot added the documentation This Pull Request changes documentation label Jan 26, 2026

blackboxsw requested a review from holmanb January 26, 2026 21:48

blackboxsw assigned blackboxsw and holmanb and unassigned blackboxsw Jan 26, 2026

holmanb reviewed Jan 27, 2026

View reviewed changes

holmanb added the incomplete Action required by submitter label Feb 10, 2026

github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Feb 25, 2026

blackboxsw added 3 commits February 25, 2026 10:02

feat: define decorators for sec_log_* and apply to distro methods

71eef7f

chore: revert unnecessary changes to cloudinit.util

6547f4a

github-actions bot removed the stale-pr Pull request is stale; will be auto-closed soon label Feb 26, 2026

blackboxsw force-pushed the owasp-logs-poc branch from 753f0fc to 046db76 Compare March 2, 2026 15:52

blackboxsw force-pushed the owasp-logs-poc branch from 046db76 to e7168c0 Compare March 2, 2026 16:44

blackboxsw added 3 commits March 2, 2026 10:27

fix: security_event_log setup should only be added when absent from l…

b87ffe3

…ogger

feat: sec log groups, sudo and doas

e0af8b5

feat: sec log groups, sudo and doas

eb6a926

blackboxsw removed the incomplete Action required by submitter label Mar 2, 2026

blackboxsw added 2 commits March 4, 2026 11:47

chore: mypy warnings and prevent subclassing for create_user and shut…

30207f1

…down Correct type hint for netdev_info

fix: ignore down nics, support chpasswd logging

df1d410

blackboxsw changed the title ~~POC: Implement OWASP security logging.~~ Implement OWASP security logging. Mar 5, 2026

blackboxsw requested a review from holmanb March 5, 2026 05:19

Conversation

blackboxsw commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Commit Message

Additional Context

Test Steps

Uh oh!

blackboxsw commented Jan 27, 2026

Uh oh!

holmanb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 25, 2026

Uh oh!

blackboxsw commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

blackboxsw commented Jan 26, 2026 •

edited

Loading