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

GCE OSLogin module: init #51566

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@adisbladis
Member

adisbladis commented Dec 5, 2018

Motivation for this change
TODO
  • get #50316 merged, rebase on master
  • fix sudo
  • integrate with gke profiles
  • add to release notes
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.

Show resolved Hide resolved nixos/modules/security/pam.nix Outdated

@flokli flokli force-pushed the adisbladis:google-oslogin branch from e559972 to 7133576 Dec 6, 2018

@flokli

This comment has been minimized.

Contributor

flokli commented Dec 6, 2018

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

@flokli

This comment has been minimized.

Contributor

flokli commented Dec 6, 2018

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 adisbladis:google-oslogin branch 2 times, most recently from 8bcdab3 to e8dff53 Dec 6, 2018

#includedir /run/google-sudoers.d
'';
systemd.tmpfiles.rules = [
"d /run/google-sudoers.d 660 root root -"

This comment has been minimized.

@flokli

flokli Dec 6, 2018

Contributor

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)

Show resolved Hide resolved nixos/modules/security/pam.nix Outdated
Show resolved Hide resolved nixos/modules/security/pam.nix Outdated
@flokli

This comment has been minimized.

Contributor

flokli commented Dec 6, 2018

cc @Assassinkin @matthewbauer for other recent pam contributions

@zimbatm

zimbatm approved these changes Dec 6, 2018

@adisbladis

This comment has been minimized.

Member

adisbladis commented Dec 6, 2018

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

@flokli

This comment has been minimized.

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?

@flokli

This comment has been minimized.

Contributor

flokli commented Dec 7, 2018

Ok, after some deeper look into pam, this is due to us misusing some oslogin modules in wrong management groups. I'll update this accordingly.

Show resolved Hide resolved nixos/modules/security/pam.nix Outdated

@flokli flokli force-pushed the adisbladis:google-oslogin branch 3 times, most recently from 7ca5056 to df7b1eb Dec 12, 2018

@flokli

This comment has been minimized.

Contributor

flokli commented Dec 12, 2018

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

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

@flokli flokli force-pushed the adisbladis:google-oslogin branch from df7b1eb to 04bf65e Dec 12, 2018

@adisbladis adisbladis changed the title from WIP: GCE OSLogin module: init to GCE OSLogin module: init Dec 12, 2018

@adisbladis

This comment has been minimized.

Member

adisbladis commented Dec 12, 2018

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

@PsyanticY

This comment has been minimized.

Contributor

PsyanticY commented Dec 12, 2018

@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

This comment has been minimized.

Contributor

PsyanticY commented Dec 12, 2018

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

This comment has been minimized.

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 added some commits Dec 12, 2018

config.security.googleOsLogin: add module
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).
security.pam: make pam_unix.so required, not sufficient
Setting 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, modules might install nss modules
for user lookup, so pam_unix.so succeeds, and we end the stack
successfully, even though other pam modules might want to do more
extensive checks.

Other distros 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.

This broke SSSD, for which @PsyanticY added a workaround knob
in #31969.

This also breaks parts of Google OS Login, as the pam_oslogin_admin.so
module doesn't get a chance to add a sudoers file for admins (while the
NSS module fixes the lookup done in pam_unix.so)

This changes the default of pam_unix.so to 'required' We don't drop the
`security.pam.services.<name?>.sssdStrictAccess` option, as it's used
some lines below to tweak error behaviour inside the pam sssd module.
nixos/modules/virtualisation/google-compute-config.nix: remove google…
…-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 flokli force-pushed the adisbladis:google-oslogin branch from 056da77 to 88d2f6f Dec 13, 2018

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