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

nixos/gnupg: add pinentry to systemPackages #90214

Closed
wants to merge 1 commit into from
Closed

nixos/gnupg: add pinentry to systemPackages #90214

wants to merge 1 commit into from

Conversation

jonringer
Copy link
Contributor

@jonringer jonringer commented Jun 13, 2020 •
edited

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.

@jonringer jonringer requested a review from skykanin June 13, 2020 00:53
@doronbehar
Copy link
Contributor

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
    '';

Ma27
Ma27 reviewed Jun 14, 2020
View reviewed changes
nixos/modules/programs/gnupg.nix Outdated
@@ -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 ];
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really sure how to implement that logic

Copy link
Member

Choose a reason for hiding this comment

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

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

nixpkgs/nixos/modules/programs/gnupg.nix

Lines 97 to 102 in 7e9b156

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
'' ];
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@jtojnar
Copy link
Contributor

jtojnar commented Jun 17, 2020 •
edited

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

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 gnupg-progpagate-pinentry branch July 2, 2020 19:06
@flokli flokli mentioned this pull request Jul 12, 2020
Merged
10 tasks
@flokli flokli mentioned this pull request Nov 22, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ma27 Ma27 Ma27 left review comments

@skykanin skykanin Awaiting requested review from skykanin

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@jonringer @doronbehar @jtojnar @flokli @Ma27