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.

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

nixos/security/misc: init #48439

Merged
merged 1 commit into from Oct 15, 2018
Merged

nixos/security/misc: init #48439

merged 1 commit into from Oct 15, 2018

Conversation

joachifm
Copy link
Contributor

A module for security options that are too small to warrant their own module.

Motivation for this change

The impetus for adding this module is to make it more convenient to override
the behavior of the hardened profile wrt user namespaces.
Without a dedicated option for user namespaces, the user needs to

  1. know which sysctl knob controls userns
  2. know how large a value the sysctl knob needs to allow e.g.,
    Nix sandbox builds to work
    Because Nix sandbox support is a likely incompatibility for hardened profile
    users, it makes sense to create a dedicated option.

Supercedes #48207, resolving the
immediate issue addressed by that PR.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

@peterhoeg
Copy link
Member

This is nice! Nitpick comment - don't we normally "enable" stuff instead of "disable" like it's done here?

@joachifm
Copy link
Contributor Author

In this case, "disable" sounds better to me because the module doesn't provide user namespaces, it only takes them away.

@joachifm
Copy link
Contributor Author

If disable is too unclear, maybe "remove" or some other wording could be used. I don't want to say enableUserNamespaces = false in my config, however, as long as the default is to enable them, if that makes sense.

@lheckemann
Copy link
Member

I don't like "disable" options, as they result in more layers of negation, which can get pretty confusing. I think an "enable" or maybe better "allow" (since it's less suggestive that the default is not to) option would be better.

@joachifm
Copy link
Contributor Author

Okay, I'll change it to allowUserNamespaces

Apart from that, any thoughts on having a module for this compared to just adding the assert to the profile? I'm not sure at present what else could go into this module, but I'm thinking some of the other hardened profile things could be reified as options here, to aid discovery and overriding for users of the profile. But unless that happens, I'm not sure the module is justifiable?

@DIzFer
Copy link
Contributor

DIzFer commented Oct 15, 2018

Ha! I was too slow. I was stuck trying to find the assertions magic. WIll test tomorrow probably :)

While I'm here, have I been living a lie, or is ENOSPEC a typo?

@joachifm
Copy link
Contributor Author

@DIzFer It is a typo, indeed :)

@joachifm
Copy link
Contributor Author

Barring objections I'll push a squashed version of this soonish.

A module for security options that are too small to warrant their own module.

The impetus for adding this module is to make it more convenient to override
the behavior of the hardened profile wrt user namespaces.
Without a dedicated option for user namespaces, the user needs to
1) know which sysctl knob controls userns
2) know how large a value the sysctl knob needs to allow e.g.,
   Nix sandbox builds to work

In the future, other mitigations currently enabled by the hardened profile may
be promoted to options in this module.
@joachifm joachifm merged commit 205aff5 into NixOS:master Oct 15, 2018
@joachifm
Copy link
Contributor Author

Thanks for the feedback all :)

@joachifm joachifm deleted the hardened-misc branch October 16, 2018 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants