Skip to content

nixos: Disable audit by default#17916

Closed
dezgeg wants to merge 4 commits intoNixOS:masterfrom
dezgeg:pr-audit
Closed

nixos: Disable audit by default#17916
dezgeg wants to merge 4 commits intoNixOS:masterfrom
dezgeg:pr-audit

Conversation

@dezgeg
Copy link
Copy Markdown
Contributor

@dezgeg dezgeg commented Aug 22, 2016

Because in its default enabled state it it causes a global performance
hit on all system calls (https://fedorahosted.org/fesco/ticket/1311) and
unwanted spam in dmesg, in particular when using Chromium (#13710).

To actually make audit fully disabled, use the big hammer by putting audit=0 to the kernel parameters.
That is actually needed because otherwise journald starts it anyway. For some discussion on the usefulness of that:
- https://fedorahosted.org/fesco/ticket/1311
- systemd/systemd#959
- openSUSE/systemd@64f83d3

Finally, split audit to multiple outputs - that prevents the library part of audit (used by systemd for instance) from depending on openldap, which currently brings in 180+ MB of crud
into the closure due to it depending on glibc headers and other stuff. (That one could
really be fixed by NixOS/patchelf#98, FWIW).

cc @edolstra @copumpkin

@mention-bot
Copy link
Copy Markdown

mention-bot commented Aug 22, 2016

@dezgeg, thanks for your PR! By analyzing the annotation information on this pull request, we identified @copumpkin, @edolstra and @domenkozar to be potential reviewers

@edolstra
Copy link
Copy Markdown
Member

edolstra commented Aug 23, 2016

Seems fine with me, but maybe it's nicer to use the -a task,never audit rule suggested by https://fedorahosted.org/fesco/ticket/1311? That simplifies dealing with the case where we switch from a configuration where auditing is disabled to one where it's enabled, which might not be the case if audit=0.

@copumpkin
Copy link
Copy Markdown
Member

copumpkin commented Aug 23, 2016

Agree with @edolstra. Also wanted to point out that although I added a module for audit, it was already enabled by default before I added it, based on kernel settings.

@dezgeg
Copy link
Copy Markdown
Contributor Author

dezgeg commented Aug 25, 2016

Yes, I think that should work, but then we depend on the whole audit package again, and it feels daft to have some package installed to fix some other's braindamage.

Hilariously enough, they actually recommend auditing from kernel completely when using containers: https://github.com/systemd/systemd/blob/dadd6ecfa5eaf842763dca545b4c04f33831789e/README#L104 :D. I wonder if that's actually relevant there and not bitrotten...

dezgeg added 4 commits August 28, 2016 20:13
@dezgeg
libaudit: Split into multiple outputs
27bd2bc
@dezgeg
audit service: Explicitly call auditctl to disable everything
a873308 
Otherwise, journald might be starting auditing.
Some reading:
    - https://fedorahosted.org/fesco/ticket/1311
    - systemd/systemd#959
    - openSUSE/systemd@64f83d3
@dezgeg
audit: Disable by default
457e3ad 
Because in its default enabled state it it causes a global performance
hit on all system calls (https://fedorahosted.org/fesco/ticket/1311) and
unwanted spam in dmesg, in particular when using Chromium
(NixOS#13710).
@dezgeg
release notes: Add note about audit being disabled by default
a4fbebc
@dezgeg
Copy link
Copy Markdown
Contributor Author

dezgeg commented Aug 28, 2016

Ok, now we call auditctl -e 0 -a task,never if the service is not enabled.

@dezgeg dezgeg added this to the 16.09 milestone Aug 28, 2016
@FRidh
Copy link
Copy Markdown
Member

FRidh commented Aug 30, 2016

Can this be considered before the branch off? I'm not familiar with the details but shutting up chromium does make me happy.

@dezgeg dezgeg mentioned this pull request Aug 31, 2016
Closed
1 task
@dezgeg
Copy link
Copy Markdown
Contributor Author

dezgeg commented Aug 31, 2016

Unless someone objects, I plan to take this in today along with #18132 to avoid extra building (as both are systemd dependencies).

@dezgeg dezgeg self-assigned this Aug 31, 2016
@dezgeg
Copy link
Copy Markdown
Contributor Author

dezgeg commented Sep 1, 2016

In staging now (to fix one conflict) which is almost build, merging to master soon as trunk-combined first evaluates successfully (there was eval error which I hopefully fixed this morning) to get a reference point.

@rasendubi rasendubi mentioned this pull request Sep 1, 2016
Closed
dezgeg added a commit that referenced this pull request Sep 1, 2016
@dezgeg
Merge staging into master
8c4aeb1 
Brings in:
    - changed output order for multiple outputs:
      #14766
    - audit disabled by default
      #17916

 Conflicts:
	pkgs/development/libraries/openldap/default.nix
@dezgeg
Copy link
Copy Markdown
Contributor Author

dezgeg commented Sep 1, 2016

Merged.

@dezgeg dezgeg closed this Sep 1, 2016
@domenkozar domenkozar added the 9.needs: changelog This PR needs a changelog entry label Sep 17, 2016
@domenkozar
Copy link
Copy Markdown
Member

domenkozar commented Sep 30, 2016

@dezgeg could you add a changelog entry for this?

@dezgeg
Copy link
Copy Markdown
Contributor Author

dezgeg commented Sep 30, 2016

@domenkozar it already has one: 5ad122b

@domenkozar domenkozar added 8.has: changelog This PR adds or changes release notes and removed 9.needs: changelog This PR needs a changelog entry labels Sep 30, 2016
@Profpatsch Profpatsch mentioned this pull request Jun 23, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@dezgeg dezgeg

Labels

8.has: changelog This PR adds or changes release notes

Projects

None yet

Milestone

16.09

Development

Successfully merging this pull request may close these issues.

6 participants

@dezgeg @mention-bot @edolstra @copumpkin @FRidh @domenkozar