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

security.pam: make pam_unix.so required, not sufficient #52488

Merged
merged 1 commit into from Dec 21, 2018
Merged

security.pam: make pam_unix.so required, not sufficient #52488

merged 1 commit into from Dec 21, 2018

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Dec 18, 2018

Having pam_unix set to "sufficient" means early-succeeding account
management group, as soon as pam_unix.so is succeeding.

This is not sufficient. For example, nixos modules might install nss
modules for user lookup, so pam_unix.so succeeds, and we end the stack
successfully, even though other pam account modules might want to do
more extensive checks.

Other distros seem to set pam_unix.so to 'required', so if there are
other pam modules in that management group, they get a chance to do some
validation too.

For SSSD, @PsyanticY already added a workaround knob in
#31969, while stating this should
be the default anyway.

I did some thinking in what could break - after this commit, we require
pam_unix to succeed, means we require getent passwd $username to
return something.
This is the case for all local users due to the passwd nss module, and
also the case for all modules installing their nss module to
nsswitch.conf - true for ldap (if not explicitly disabled) and sssd.

I'm not so sure about krb5, cc @eqyiel for opinions. Is there some nss
module loaded? Should the pam account module be placed before pam_unix?

We don't drop the security.pam.services.<name?>.sssdStrictAccess
option, as it's also used some lines below to tweak error behaviour
inside the pam sssd module itself (by changing it's 'control' field).

This is also required to get admin login for Google OS Login working
(#51566), as their pam_oslogin_admin accounts module takes care of sudo
configuration.

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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@flokli
Copy link
Contributor Author

flokli commented Dec 18, 2018

I couldn't test the ldap test for example, as nss-pam-ldapd is currently broken.

@PsyanticY
Copy link
Contributor

@flokli Not sure of this is the right place to bring this up but when installing sssd nixos also keeps nscd which is basically doing the same job as sssd. Having 2 caching daemons can cause various issues.
Here Redhat opinion on this. So what's your opinion on this and how we can address that.

@flokli
Copy link
Contributor Author

flokli commented Dec 18, 2018

@PsyanticY we don't use nscd's caching features - we only abuse it to use enabled nss modules: https://github.com/NixOS/nixpkgs/pull/50316/files#diff-09da2f18ff6731224a67af7f0081d111R250

So basically nscd never caches, and sssd's cache is used if configured properly.

@flokli
security.pam: make pam_unix.so required, not sufficient
d180bf3 
Having pam_unix set to "sufficient" means early-succeeding account
management group, as soon as pam_unix.so is succeeding.

This is not sufficient. For example, nixos modules might install nss
modules for user lookup, so pam_unix.so succeeds, and we end the stack
successfully, even though other pam account modules might want to do
more extensive checks.

Other distros seem to set pam_unix.so to 'required', so if there are
other pam modules in that management group, they get a chance to do some
validation too.

For SSSD, @PsyanticY already added a workaround knob in
NixOS#31969, while stating this should
be the default anyway.

I did some thinking in what could break - after this commit, we require
pam_unix to succeed, means we require `getent passwd $username` to
return something.
This is the case for all local users due to the passwd nss module, and
also the case for all modules installing their nss module to
nsswitch.conf - true for ldap (if not explicitly disabled) and sssd.

I'm not so sure about krb5, cc @eqyiel for opinions. Is there some nss
module loaded? Should the pam account module be placed before pam_unix?

We don't drop the `security.pam.services.<name?>.sssdStrictAccess`
option, as it's also used some lines below to tweak error behaviour
inside the pam sssd module itself (by changing it's 'control' field).

This is also required to get admin login for Google OS Login working
(NixOS#51566), as their pam_oslogin_admin accounts module takes care of sudo
configuration.
@flokli
Copy link
Contributor Author

flokli commented Dec 21, 2018

Rebased on master.

Now that nss-pam-ldapd was fixed in unstable (thanks @Mic92 ❤️ ), I gave nixosTests.ldap a go, and it also did went through. So I'd say this is good to merge.

@flokli
Copy link
Contributor Author

flokli commented Dec 21, 2018

@GrahamcOfBorg build nixosTests.ldap

@flokli flokli merged commit 9c86e8f into NixOS:master Dec 21, 2018
@flokli flokli deleted the pam_account_unix_required branch December 21, 2018 16:49
grahamc
grahamc reviewed Dec 21, 2018
View reviewed changes
nixos/doc/manual/release-notes/rl-1903.xml
enabled. In case your setup breaks due to some later pam account module
previosuly shadowed, or failing NSS lookups, please file a bug. You can
get back the old behaviour by manually setting
<literal><![CDATA[security.pam.services.<name?>.text]]></literal>.
Copy link
Member

Choose a reason for hiding this comment

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

manually setting this to what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

depends on the module configuration - basically looking at /etc/pam.d/<name>, and replacing required with sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As explained in the PR and commit messages, I don't expect this to break things , and if it does, we should fix it.

@flokli flokli mentioned this pull request Dec 21, 2018
Merged
14 tasks
@flokli flokli mentioned this pull request Dec 30, 2018
Closed
9 tasks
@tommy4st
Copy link

For Kerberos, it's not fully working. Right now I have to choose between unlocking my gnome-keyring or using Kerberos. There is probably a problem with the following lines:

Here:

nixpkgs/nixos/modules/security/pam.nix

Line 390 in f9bb39e

auth required pam_unix.so ${optionalString cfg.allowNullPassword "nullok"} ${optionalString cfg.nodelay "nodelay"} likeauth

pam_auth is required and here:

nixpkgs/nixos/modules/security/pam.nix

Line 406 in f9bb39e

"auth sufficient pam_unix.so ${optionalString cfg.allowNullPassword "nullok"} ${optionalString cfg.nodelay "nodelay"} likeauth try_first_pass"}

it is sufficient, that means - as stated in a comment - nothing runs afterwards, but there follow necessary the pam_krb5 lines

nixpkgs/nixos/modules/security/pam.nix

Lines 413 to 415 in f9bb39e

${optionalString config.krb5.enable ''
auth [default=ignore success=1 service_err=reset] ${pam_krb5}/lib/security/pam_krb5.so use_first_pass
auth [default=die success=done] ${pam_ccreds}/lib/security/pam_ccreds.so action=validate use_first_pass

When I can disable unixAuth, Kerberos works as expected, but the Gnome Keyring here:

nixpkgs/nixos/modules/security/pam.nix

Lines 398 to 399 in f9bb39e

${optionalString cfg.enableGnomeKeyring
"auth optional ${pkgs.gnome3.gnome-keyring}/lib/security/pam_gnome_keyring.so"}

will not be unlocked. But when I enable unixAuth, the pam_krb5 cannot be reached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grahamc grahamc grahamc left review comments

@Mic92 Mic92 Mic92 approved these changes

@infinisil infinisil Awaiting requested review from infinisil

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@flokli @PsyanticY @tommy4st @grahamc @Mic92 @GrahamcOfBorg