Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

audit: add patch to allow 0555 permissions #96626

Closed
wants to merge 1 commit into from
Closed

audit: add patch to allow 0555 permissions #96626

wants to merge 1 commit into from

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Aug 29, 2020 •
edited

Motivation for this change

When configuring the dispatcher like this in /etc/audit/auditd.conf:

dispatcher = ${audit}/bin/audispd

Then it would complain that the permissions are not valid for the binary. This is now fixed with the added patch.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@saschagrunert
Copy link
Member Author

cc @copumpkin @lihop

@saschagrunert
Copy link
Member Author

@GrahamcOfBorg build audit

@saschagrunert
Copy link
Member Author

@zowoq partially related to our container work. Auditd seems the only way to log security/permission issues together with containers.

@saschagrunert saschagrunert mentioned this pull request Aug 29, 2020
Open
jtojnar
jtojnar reviewed Aug 30, 2020
View reviewed changes
pkgs/os-specific/linux/audit/default.nix Outdated
@@ -36,7 +36,7 @@ stdenv.mkDerivation rec {
# TODO: Remove the musl patches when
# https://github.com/linux-audit/audit-userspace/pull/25
# is available with the next release.
patches = stdenv.lib.optional stdenv.hostPlatform.isMusl [
patches = [ ./allowed-permissions.patch ] ++ stdenv.lib.optional stdenv.hostPlatform.isMusl [
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment explaining the patch, ideally linking to a relevant upstream issue?

It would also be nice to move the musl comment inside the musl conditional to make it easier to see what it refers to.

Copy link
Member Author

Choose a reason for hiding this comment

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

Did add a comment and reformatted the section.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also clarify whether there is an upstream issue or pull request?

Copy link
Contributor

Choose a reason for hiding this comment

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

This still doesn't seem resolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that there is any PR for linux-audit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an issue at least? How will upstream learn we have this problem?

Copy link
Member Author

Choose a reason for hiding this comment

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

I created a PR for audit there: linux-audit/audit-userspace#138

I'm not sure how open they're about this contributions, so let's see.

@saschagrunert
audit: add patch to allow 0555 permissions
b52dd51 
Signed-off-by: Sascha Grunert <sgrunert@suse.com>
@vcunat vcunat requested a review from jtojnar October 18, 2020 09:48
@SuperSandro2000
Copy link
Member

Can you please target staging with this mass rebuild?

@saschagrunert saschagrunert changed the base branch from master to staging November 27, 2020 11:02
@saschagrunert
Copy link
Member Author

Can you please target staging with this mass rebuild?

Yep, sure changed it to target staging.

@stale
Copy link

stale bot commented Jul 21, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 21, 2021
@saschagrunert saschagrunert deleted the audit branch October 8, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar Awaiting requested review from jtojnar

Assignees
No one assigned
Labels
2.status: merge conflict 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@saschagrunert @SuperSandro2000 @jtojnar