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

pam_gnupg: init at fbd75b7 #77002

Closed
wants to merge 3 commits into from
Closed

pam_gnupg: init at fbd75b7 #77002

wants to merge 3 commits into from

Conversation

@ahesford
Copy link

@ahesford ahesford commented Jan 6, 2020

Motivation for this change

The pam_gnupg module allows GPG keys to be unlocked during user authentication and session creation in PAM. This request adds a new package for pam_gnupg to nix, and modifies the PAM module in NixOS to add optional support for pam_gnupg to the system PAM configuration.

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.
Notify maintainers

cc @prusnak

{ stdenv, fetchgit, autoreconfHook, gnupg, pam } :

stdenv.mkDerivation rec {
name = "pam_gnupg-fbd75b7";

This comment has been minimized.

@prusnak

prusnak Jan 9, 2020
Member

Use this instead of name

pname = "pam_gnupg";
version = "unstable-2019-12-06";
@@ -438,6 +441,8 @@ let
"session optional ${pkgs.otpw}/lib/security/pam_otpw.so"}
${optionalString cfg.startSession
"session optional ${pkgs.systemd}/lib/security/pam_systemd.so"}
${optionalString config.security.pam.enableGnupg

This comment has been minimized.

@prusnak

prusnak Jan 9, 2020
Member

It seems you are mixing tabs and spaces here because the columns do not align.

|| cfg.pamMount
|| cfg.enableKwallet
|| cfg.enableGnomeKeyring
|| cfg.googleAuthenticator.enable
|| cfg.duoSecurity.enable)) ''
auth required pam_unix.so ${optionalString cfg.allowNullPassword "nullok"} likeauth
${optionalString config.security.pam.enableGnupg

This comment has been minimized.

@prusnak

prusnak Jan 9, 2020
Member

It seems you are mixing tabs and spaces here because the columns do not align.

@@ -361,12 +361,15 @@ let
# We use try_first_pass the second time to avoid prompting password twice
(optionalString (cfg.unixAuth &&
(config.security.pam.enableEcryptfs
|| config.security.pam.enableGnupg

This comment has been minimized.

@prusnak

prusnak Jan 9, 2020
Member

It seems you are mixing tabs and spaces here because the columns do not align.

ahesford added 2 commits Jan 6, 2020
nixos/pam: add support for new pam_gnupg package in pam configs
Replace tabs with spaces and change unstable version numbering
@ahesford
Copy link
Author

@ahesford ahesford commented Jan 9, 2020

@prusnak Thanks for the feedback; the requested changes have been made.

configurePhase = ''
./configure --prefix=$out --with-moduledir=$out/lib/security
'';
Comment on lines 20 to 22

This comment has been minimized.

@prusnak

prusnak Jan 9, 2020
Member

replace with:

configureFlags = [ "--with-moduledir=$out/lib/security" ];

This comment has been minimized.

@ahesford

ahesford Jan 9, 2020
Author

I tried this originally, but the build failed because the shell somehow expanded "$o" to an empty string and made moduledir "ut/lib/security". The configurePhase redefinition didn't cause the same problem. It turns out that using quoted braces around "out" when using configureFlags also solves the problem, so the latest commit adopts the correct form.

preAutoreconf = ''
mkdir m4
'';
Comment on lines 16 to 18

This comment has been minimized.

@prusnak

prusnak Jan 9, 2020
Member

Is this really necessary?

This comment has been minimized.

@ahesford

ahesford Jan 9, 2020
Author

It silenced a warning during the build phase, but I removed it because it doesn't cause the build to fail.

@ahesford
Copy link
Author

@ahesford ahesford commented Feb 10, 2020

I no longer have a NixOS installation and will be unable to respond meaningfully to further requests for changes.

@ahesford ahesford closed this Feb 10, 2020
@ahesford ahesford deleted the ahesford:pam_gnupg branch Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.