Skip to content

nixos/audit: fix journald test#432238

Merged
nikstur merged 1 commit intoNixOS:masterfrom
nikstur:fix-journald-audit
Aug 9, 2025
Merged

nixos/audit: fix journald test#432238
nikstur merged 1 commit intoNixOS:masterfrom
nikstur:fix-journald-audit

Conversation

@nikstur
Copy link
Contributor

@nikstur nikstur commented Aug 9, 2025

Follow-up on #429553

Fixes nixosTests.systemd-journal which is currently broken on master.

Makes the audit module responsible for setting up the audit subsystem of the kernel. The auditd module is now only responsible for setting up the daemon.

Enable the audit subsystem early via kernelParams.

Increase the default audit backlog limit so that it works out of the box for a normal system.

Remove a superfluous and pointless test case.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Aug 9, 2025
@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented Aug 9, 2025

Should the auditd module enable the audit module? The userspace daemon would be mighty pointless if the system audit isn't enabled.
Nvm, it already does, sorry!

Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of nits, the overall changes look very sensible.

diff --git a/nixos/modules/security/auditd.nix b/nixos/modules/security/auditd.nix
index 2dea938acc7b..036ce9c01cae 100644
--- a/nixos/modules/security/auditd.nix
+++ b/nixos/modules/security/auditd.nix
@@ -202,7 +202,7 @@ in
       }
     ];
 
-    # Starting auditd should also enable loading the audit rules..
+    # Starting the userspace daemon should also enable audit in the kernel
     security.audit.enable = lib.mkDefault true;
 
     # setting this to anything other than /etc/audit/plugins.d will break, so we pin it here

This comment is now inaccurate and should reflect the auditd module is only the user space daemon.

diff --git a/nixos/tests/audit.nix b/nixos/tests/audit.nix
index 7f1280060824..074af12b4c5f 100644
--- a/nixos/tests/audit.nix
+++ b/nixos/tests/audit.nix
@@ -33,8 +33,10 @@
     machine.wait_for_unit("audit-rules.service")
     machine.wait_for_unit("auditd.service")
 
-    with subtest("Audit subsystem gets enabled"):
-      assert "enabled 1" in machine.succeed("auditctl -s")
+    with subtest("Audit subsystem gets enabled with correct backlog limits"):
+      audit_status = machine.succeed("auditctl -s")
+      assert "enabled 1" in audit_status, "expected audit to be enabled"
+      assert "backlog_limit 1024" in audit_status, "expected backlog limit 1024"
 
     with subtest("unix socket plugin activated"):
       machine.succeed("stat /var/run/audispd_events")

the backlog_limit being different from kernel default might warrant a test to make sure its picked up correctly. Otherwise we might miss if e.g. the kernel flag is renamed or something like that.

Makes the audit module responsible for setting up the audit subsystem of
the kernel. The auditd module is now only responsible for setting up the
daemon.

Enable the audit subsystem early via kernelParams.

Increase the default audit backlog limit so that it works out of the box
for a normal system.

Remove a superfluous and pointless test case.
@nikstur nikstur force-pushed the fix-journald-audit branch from dbfff11 to 439d68b Compare August 9, 2025 13:00
Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this looks very sensible now, thank you

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 9, 2025
@nikstur nikstur merged commit 4dcfd5b into NixOS:master Aug 9, 2025
26 of 28 checks passed
@nikstur nikstur deleted the fix-journald-audit branch August 9, 2025 14:43
@LordGrimmauld LordGrimmauld mentioned this pull request Aug 12, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants