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/gnupg: add pinentry to systemPackages #90214

Closed
wants to merge 1 commit into from

Conversation

@jonringer
Copy link
Contributor

jonringer commented Jun 13, 2020

Motivation for this change

This is needed at runtime:

gpg: agent_genkey failed: No pinentry
Key generation failed: No pinentry
gpgconf --check-programs
gpgconf: error running '/nix/store/dxf4w1gdp2x87d2q326a0064qnbvcl19-gnupg-2.2.20/bin/pinentry': probably not installed
gpg:OpenPGP:/nix/store/dxf4w1gdp2x87d2q326a0064qnbvcl19-gnupg-2.2.20/bin/gpg:1:1:
gpg-agent:Private Keys:/nix/store/dxf4w1gdp2x87d2q326a0064qnbvcl19-gnupg-2.2.20/bin/gpg-agent:1:1:
scdaemon:Smartcards:/nix/store/dxf4w1gdp2x87d2q326a0064qnbvcl19-gnupg-2.2.20/libexec/scdaemon:1:1:
gpgsm:S/MIME:/nix/store/dxf4w1gdp2x87d2q326a0064qnbvcl19-gnupg-2.2.20/bin/gpgsm:1:1:
dirmngr:Network:/nix/store/dxf4w1gdp2x87d2q326a0064qnbvcl19-gnupg-2.2.20/bin/dirmngr:1:1:
pinentry:Passphrase Entry:/nix/store/dxf4w1gdp2x87d2q326a0064qnbvcl19-gnupg-2.2.20/bin/pinentry:0:0:
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.
@doronbehar
Copy link
Contributor

doronbehar commented Jun 13, 2020

I've encountered a related issue once where I wanted to be able to have 3 different types of pinentry programs - the attributes: pinentry-gtk2, pinentry-curses & pinentry but the workaround I've found necessary for that was:

    environment.extraSetup = ''
      ln -s ${pkgs.pinentry-gtk2}/bin/pinentry $out/bin/pinentry-gtk-2
      ln -s ${pkgs.pinentry-curses}/bin/pinentry $out/bin/pinentry-curses
      ln -s ${pkgs.pinentry}/bin/pinentry $out/bin/pinentry-tty
      ln -s $out/bin/pinentry-tty $out/bin/pinentry
    '';
@@ -123,7 +123,7 @@ in

services.dbus.packages = mkIf (cfg.agent.pinentryFlavor == "gnome3") [ pkgs.gcr ];

environment.systemPackages = with pkgs; [ cfg.package ];
environment.systemPackages = with pkgs; [ cfg.package pinentry ];

This comment has been minimized.

@Ma27

Ma27 Jun 14, 2020 Member

Shouldn't we install the proper pinentry package (i.e. qt, gtk or curses flavour)?

This comment has been minimized.

@jonringer

jonringer Jun 16, 2020 Author Contributor

Not really sure how to implement that logic

This comment has been minimized.

@Ma27

Ma27 Jun 16, 2020 Member

How about solving it just like it's done here:

systemd.user.services.gpg-agent = mkIf (cfg.agent.pinentryFlavor != null) {
serviceConfig.ExecStart = [ "" ''
${cfg.package}/bin/gpg-agent --supervised \
--pinentry-program ${pkgs.pinentry.${cfg.agent.pinentryFlavor}}/bin/pinentry
'' ];
};

This comment has been minimized.

@jonringer

jonringer Jun 16, 2020 Author Contributor

that's constructing a path, I don't think I can put just a path as an item to: environment.systemPackages

This comment has been minimized.

@Ma27

Ma27 Jun 16, 2020 Member

I'm talking about adding pinentry.${cfg.agent.pinentryFlavor} to systemPackages. The pinentry-package as an output for each flavor.

This comment has been minimized.

@jonringer

jonringer Jun 17, 2020 Author Contributor

ah, interesting. I misread that line, though it was "${pkgs.pinentry}.${cfg.agent.pinentryFlavor}", as that's more common

@jonringer jonringer force-pushed the jonringer:gnupg-progpagate-pinentry branch from d9e5558 to e9e7886 Jun 17, 2020
@jtojnar
Copy link
Contributor

jtojnar commented Jun 17, 2020

Would just adding a pinentry to PATH work? IIRC the whole reason we had to do the gpg-agent service override was because it did not work.

There were suggestions to add a script to ${gnupg}/bin/pinentry that would run pinentry from PATH when available, for cases when gpg-agent is not used.

Edit: See #73332 (comment)

@flokli
Copy link
Contributor

flokli commented Jun 28, 2020

I haven't yet tried out, but I could imagine the following might work in more environments and usecases:

  • removing the --pinentry-program argument in gpg-agent.service
  • allow the co-existence of multiple pinentries (by removing the bin/pinentry -> bin/pinentry-${flavourName} symlink in each output)
  • provide a pinentry symlink in $PATH pointing to the selected flavour via environment.systemPackages
  • provide the selected pinentry.${flavour} package in environment.systemPackages

That way, it should again become possible to configure the pinentry program via gpg-agent.conf, it shouldn't depend on the socket-activated gpg-agent.{socket,service} anymore (so gpg could spin up gpg-agent by itself, if it wants to).

It might also fix some nix-on-non-NixOS usecases, once changes have trickled into the home-manager module.

I'll try things out and will send a PR.

@flokli
Copy link
Contributor

flokli commented Jul 1, 2020

So, I did do some digging, and it seems gnupg defaults to ${gnupg}/bin/pinentry, if we don't explicitly pass it another pinentry program during configure. it doesn't try to discover from $PATH, at least not without more patching (but I'm also not sure anymore if it's a good idea to discover from $PATH, or a potential security hole). So I'm not sure this PR here will solve anything. I assume people who don't want to use the pinentry flavour selected by the module system should set it to null, and point to their pinentry of choice inside gpg-agent.conf (like in #73332).

On the topic of falling back to ncurses if you're connecting via ssh (as #73332 (comment) seems to suggest):

I see there's some ncurses fallback code on some graphical pinentries.

However, this seems to not be working in all cases (for example services.screen-locker uses xss-lock with i3lock, which only seems to set SetIdleHint from logind, but the gnome3 pinentry checks for the gnome-screensaver status over its specific DBUS endpoint).

I assume adding more generic support for these things into the graphical pinentries should be a good thing, and could improve the situation a lot.

There's also PINENTRY_USER_DATA, but this seems to be more of a loophole for people to write their own wrappers than a standardized solution, so not sure if we should do anything there.

@jonringer
Copy link
Contributor Author

jonringer commented Jul 2, 2020

I made the PR because someone on discord had trouble with the service, and thought it would be an easy enough fix to just include the command on the system PATH.

I'm going to transfer this to an issue, and close the PR. I don't have enough familiarity to know a good path forward, if someone else would like to solve the usability issue, then they can do so in a different PR.

@jonringer jonringer closed this Jul 2, 2020
@jonringer jonringer deleted the jonringer:gnupg-progpagate-pinentry branch Jul 2, 2020
@flokli flokli mentioned this pull request Jul 12, 2020
6 of 10 tasks complete
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

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