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

Jump to bottom

GCE OSLogin module: init #51566

Merged
merged 7 commits into from Dec 24, 2018
Merged

GCE OSLogin module: init #51566

merged 7 commits into from Dec 24, 2018

Conversation

adisbladis
Copy link
Member

@adisbladis adisbladis commented Dec 5, 2018 •
edited by flokli

Motivation for this change
TODO
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

flokli commented Dec 6, 2018

cc @arianvp I cherry-picked commits from #50316 in here as well.

@flokli
Copy link
Contributor

flokli commented Dec 6, 2018 •
edited

so far, logging in using the snakeoil ssh keys works, but there is some pam weirdness going on:

server# [   11.598934] systemd[1]: Starting User Manager for UID 1009719691...
server# [   11.626640] systemd[851]: PAM unable to resolve symbol: pam_sm_authenticate
server# [   11.628612] systemd[851]: PAM unable to resolve symbol: pam_sm_setcred
server# [   11.632156] systemd[851]: PAM unable to resolve symbol: pam_sm_open_session
server# [   11.634596] systemd[851]: PAM unable to resolve symbol: pam_sm_close_session

I guess, that's the reason fro why pam_oslogin_admin does not create the sudoers config file in /run/google-sudoers.d/mockadmin to allow mockadmin to sudo, which is why the test currently is failing.

@flokli flokli force-pushed the google-oslogin branch 2 times, most recently from 8bcdab3 to e8dff53 Compare December 6, 2018 15:38
flokli
flokli reviewed Dec 6, 2018
View reviewed changes
nixos/modules/security/google_oslogin.nix Outdated
#includedir /run/google-sudoers.d
'';
systemd.tmpfiles.rules = [
"d /run/google-sudoers.d 660 root root -"
Copy link
Contributor

Choose a reason for hiding this comment

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

The - for age means systemd will create the directory with mentioned permissions and ownership, but not automatically clean up anything.

I moved that from /var/google-sudoers.d (and patched pam_module/pam_oslogin_admin.cc), as that location seemed much more suitable for runtime files.

pam_oslogin_admin.cc has code to clean up existing files if admin status is revoked, it's just a more meaningful location (and cleans up expired users on a reboot)

@flokli
Copy link
Contributor

flokli commented Dec 6, 2018

cc @Assassinkin @matthewbauer for other recent pam contributions

@adisbladis
Copy link
Member Author

@flokli Could you squash the fixup commits? I don't mind you force-pushing to my fork.

@flokli
Copy link
Contributor

flokli commented Dec 6, 2018

I'd still like to get the TODOs fixed and #50316 merged before rebasing again.

Any ideas about the " PAM unable to resolve symbol:" errors?

This was referenced Dec 6, 2018
Closed
Closed
Closed
@flokli flokli force-pushed the google-oslogin branch 2 times, most recently from 7ca5056 to df7b1eb Compare December 12, 2018 14:06
@flokli
Copy link
Contributor

flokli commented Dec 12, 2018 •
edited

Finally got sudo working - culprit was the pam_unix.so module configured wrongly in NixOS (see 04bf65e5ab26634aa5981a4762f70635d129daa0) for a detailed explanation.

This was fixed for sssd only in #31969, @dezgeg, @PsyanticY, can you have a look at 04bf65e5ab26634aa5981a4762f70635d129daa0, too?

@adisbladis adisbladis changed the title WIP: GCE OSLogin module: init GCE OSLogin module: init Dec 12, 2018
@adisbladis
Copy link
Member Author

Things seems to be working great now! Thanks @flokli for all the good work.

@PsyanticY
Copy link
Contributor

@flokli I think it should be like that for all module not just sssd. account required pam_unix.so. when i fixed that for SSSD I mentioned that it should be like this for all modules but i thought maybe there was some use case for it being sufficient in nixos even thought all the other disto use required

@PsyanticY
Copy link
Contributor

Even this part

          ${optionalString (config.services.sssd.enable && cfg.sssdStrictAccess==false)
               "account sufficient ${pkgs.sssd}/lib/security/pam_sss.so"}
           ${optionalString (config.services.sssd.enable && cfg.sssdStrictAccess)
              "account [default=bad success=ok user_unknown=ignore] ${pkgs.sssd}/lib/security/pam_sss.so"}

it should not be sufficient. I would suggest removing the sssdStrictAccess option and swapping sufficient with this [default=bad success=ok user_unknown=ignore] .

@flokli
Copy link
Contributor

flokli commented Dec 12, 2018

Reading up on https://wiki.debian.org/LDAP/PAM, it seems this should at least work if all these 'external' pam modules provide an nss module, too. There are other examples on what can/should be done instead - but it seems to be very a combinatory hell.

Maybe we should limit the pam module to only allow one external pam module (sssd/ldap/oslogin/kerberos) to be active, and check configuration for them?

@flokli flokli mentioned this pull request Dec 18, 2018
Merged
10 tasks
flokli added a commit to flokli/nixpkgs that referenced this pull request Dec 21, 2018
@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
google-compute-engine-oslogin: init at 1.4.3
fb41136
@flokli
security.pam.services.<name?>.: add googleOsLogin(AccountVerification…
be5ad77 
…|Authentication)
@flokli
config.security.googleOsLogin: add module
c6de45c 
The OS Login package enables the following components:
AuthorizedKeysCommand to query valid SSH keys from the user's OS Login
profile during ssh authentication phase.
NSS Module to provide user and group information
PAM Module for the sshd service, providing authorization and
authentication support, allowing the system to use data stored in
Google Cloud IAM permissions to control both, the ability to log into
an instance, and to perform operations as root (sudo).
@flokli
config.nsswitch: load cache_oslogin and oslogin nss modules if config…
04f3562 
….security.googleOsLogin.enable is set
@flokli
nixos/tests: add google-oslogin test
0f46188
@flokli
nixos/modules/virtualisation/google-compute-config.nix: remove google…
706efad 
…-accounts-daemon

Use googleOsLogin for login instead.
This allows setting users.mutableUsers back to false, and to strip the
security.sudo.extraConfig.

security.sudo.enable is default anyhow, so we can remove that as well.
@flokli
Copy link
Contributor

flokli commented Dec 21, 2018

With #52488 being merged, this should be good to go, too.

@flokli flokli requested a review from grahamc December 21, 2018 17:19
@zimbatm zimbatm merged commit d06f798 into NixOS:master Dec 24, 2018
@colemickens colemickens mentioned this pull request Apr 24, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flokli flokli flokli left review comments

@zimbatm zimbatm zimbatm approved these changes

@infinisil infinisil Awaiting requested review from infinisil

@grahamc grahamc Awaiting requested review from grahamc

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 8.has: package (new) 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
5 participants
@adisbladis @flokli @PsyanticY @zimbatm @GrahamcOfBorg