yubikey-agent: init at 0.1.3 #92936

philandstuff · 2020-07-11T15:37:51Z

Motivation for this change

This adds yubikey-agent as a package and a nixos module.

The yubikey-agent is impure in how it calls pinentry; I tried patching
gopass to point at a particular pinentry but this changes the value of
vendorSha256 depending on the particular choice of pinentry program,
which felt frustratingly difficult. Instead, we use wrapProgram to
set a default pinentry in PATH, and use a systemd override to prefix
PATH to select a chosen pinentry program if specified.

On linux, we need libnotify to provide the notify-send utility for
desktop notifications (such as "Waiting for Yubikey touch...").

The systemd unit a copy of the one in the upstream repo, in systemd.md:
https://github.com/FiloSottile/yubikey-agent/blob/master/systemd.md

This might work on other flavors of unix, but I haven't tested.

Things done

flokli

Thanks for the code! I don't think we should reinvent yet another pinentry selection, so we should just reuse the one from the gnupg module for the time being.

nixos/modules/services/security/yubikey-agent.nix

pkgs/tools/security/yubikey-agent/yubikey-agent.service

pkgs/top-level/all-packages.nix

pkgs/tools/security/yubikey-agent/default.nix

flokli · 2020-07-12T12:07:26Z

Is there any context/docs on why yubikey-agent is fine to discover pinentry from $PATH, whereas gpg-agent.conf insists on absoulte paths?

I did some digging in #90214 (comment), so would be nice to hear @FiloSottile's opinion on that as well :-)

pkgs/tools/security/yubikey-agent/yubikey-agent.service

philandstuff · 2020-07-12T14:39:31Z

@flokli thank you for your thorough review! I agree with most of your points and will update the PR.

Yes, the way pinentry is configured is weird and idiosyncratic.

The gnupg nix package sets a hardcoded absolute path to pinentry. This works ok because programs like gpg-agent have a --pinentry-program command-line option to choose a different pinentry at runtime, as well as config file options and suchlike. This means that the gnupg nixos module can allow the user to configure their choice of pinentry program easily, even if the underlying gnupg package has a hardcoded pinentry path as a default value.

yubikey-agent pulls pinentry from PATH, and it does so in a 3rd party module (it's actually within gopass), which makes it incredibly awkward to patch an absolute path to pinentry instead. yubikey-agent also offers no way to choose a pinentry program at runtime other than by putting the desired one first in PATH. It has some code to dispatch to a different binary name on different OSes (pinentry.exe on windows, pinentry-mac on macOS, pinentry otherwise) but on non-macOS unixes it basically assumes the underlying distribution will provide an appropriate pinentry, in the PATH. In fairness this is the case with most distributions, but with nix it's completely against the philosophy.

I'm not sure I can think of a better alternative.

flokli · 2020-07-12T15:55:11Z

Doing the dance via the socket-activated gpg-agent.service in each users' systemd session was mostly done to avoid shipping the whole runtime closure of all (also graphical) pinentry flavours, and to provide a way to select the flavour (#71095), but as written in #90214 (comment), discovering from $PATH for gpg as well might help in some usecases.

We might want to open a discussion with the GnuPG people about why pinentry discovery there is the way it is, and why it's not defaulting to looking for it in $PATH if it's neither set in gpg-agent.conf, nor in --pinentry-program.

I'd like to understand the reasoning on why it currently doesn't do that (and if yubikey-agent/pass doing differently is actually a footgun - if it's not, I'm all for adding it to its $PATH as suggested in #92936 (comment)).

philandstuff · 2020-07-12T16:23:57Z

Let’s assume we go with your proposal to just look for pinentry in PATH (for both gnupg and yubikey-agent). What happens if there is no pinentry anywhere on the PATH?

FiloSottile · 2020-07-12T16:46:03Z

yubikey-agent intentionally doesn't have a config file. Would a PINENTRY env var work for you? Happy to add it.

philandstuff · 2020-07-13T08:14:36Z

Okay, I've pushed a couple of updates. Key unresolved issues:

I agree that I should reuse the gnupg.agent.pinentryFlavor but i get an error
We need to decide how, and where, to configure pinentry

To expand on the pinentry discussion: as a starting point, I feel that a nix closure should contain everything it needs to function correctly. This is why, in general, nix packages depend on other packages via absolute store paths and not via PATH. If you have a dependency via PATH, that does not form part of the closure, and when you copy the closure to another system, you won't copy the dependency.

However, I can see how pinentry is a bit of a weird edge case: users might reasonably want to configure different types of pinentry, and they shouldn't have to install all the pinentries in order to use just one of them.

It's also worth noting that the dynamics on macOS are different: on macOS, there is only one pinentry implementation, and I think gnupg and yubikey-agent should just use it without any extra configuration. But on Linux (and probably other unixes) it's useful to be able to dynamically choose a pinentry program.

I must confess I read some of the discussions that @flokli linked to above (such as #90214 (comment)) but it's really hard to get context by jumping partway through a conversation and chasing back through many linked issues so it's mostly going above my head. But my understanding is: 1) gnupg has a baked-in pinentry choice, 2) gnupg allows users to choose a different pinentry at invocation time, 3) there is some desire (I don't know why) that gnupg should be able to use pinentry from PATH. In any case, the design of gnupg sounds like a live issue, so we shouldn't assume the current gnupg packaging is the correct way to go.

Right now my feeling is:

on macOS, the built yubikey-agent binary should use pinentry-mac without any extra configuration
on Linux, the built yubikey-agent could just get whatever pinentry it finds in PATH, and we don't need to bake in a default.
if the gnupg side of the discussion is resolved, I'm happy to try to ensure yubikey-agent is consistent with it.

How does that sound?

philandstuff · 2020-07-13T08:15:11Z

@FiloSottile Thanks for the offer! I think we need to discuss things a bit further here but we may well take you up on this.

philandstuff · 2020-07-14T20:55:01Z

Added @rawkode as a maintainer, ref: #92965 (comment)

philandstuff · 2020-07-15T20:41:23Z

I pushed another update. I've pulled in the systemd unit from FiloSottile/yubikey-agent#43 but the relative path trick gave me the error: Executable "yubikey-agent" not found in path "/nix/store/zzr1p57r1ii6y0gm7kycid0gkw1hskp3-systemd-245.6/bin/" so I still need to substitute the nix store path into it.

I have removed baking in pinentry to PATH, except on macOS where we bake in pinentry_mac since there's no other choice.

I still haven't come up with a solution to reusing the gpg-agent pinentry selection (see #92936 (comment)).

flokli · 2020-07-15T21:28:08Z

I pushed another update. I've pulled in the systemd unit from FiloSottile/yubikey-agent#43 but the relative path trick gave me the error: Executable "yubikey-agent" not found in path "/nix/store/zzr1p57r1ii6y0gm7kycid0gkw1hskp3-systemd-245.6/bin/" so I still need to substitute the nix store path into it.

Yeah, this is expected.systemd luckily doesn't pick up things from $PATH when setting just a binary name in ExecStart= but looks up from paths baked into systemd.

On NixOS, we set this to $systemd/bin for reproducibility.

I have removed baking in pinentry to PATH, except on macOS where we bake in pinentry_mac since there's no other choice.

Is there not a native pinentry provided in $PATH?

I still haven't come up with a solution to reusing the gpg-agent pinentry selection (see #92936 (comment)).

Yeah, this is still somewhat pending. I'd probably prefer moving pinentry selection into a module both used from the gnupg module and this module over duplicating it…

philandstuff · 2020-07-15T21:33:19Z

Is there not a native pinentry provided in $PATH?

macOS doesn’t provide one, no. The user might have installed one via gpgtools or homebrew, but plain macOS doesn’t have a pinentry.

philandstuff · 2020-07-16T12:50:06Z

@flokli I've just pushed an update. It turns out the magic word I needed was mkMerge https://nixos.org/nixos/manual/#sec-option-definitions-merging

I think this resolves all the issues we had. PTAL 😄

rawkode · 2020-07-16T12:59:37Z

I'm learning a lot from monitoring this, thank you for your persistence @philandstuff

flokli

Thanks for the persistence too :-) I think the pinentry problem could be solved without the mkMerge, sent another review.

nixos/modules/programs/gnupg.nix

This adds yubikey-agent as a package and a nixos module. On macOS, we use `wrapProgram` to set pinentry_mac as default in PATH; on Linux we rely on the user to set their preferred pinentry in PATH. In particular, we use a systemd override to prefix PATH to select a chosen pinentry program if specified. On Linux, we need libnotify to provide the notify-send utility for desktop notifications (such as "Waiting for Yubikey touch..."). This might work on other flavors of unix, but I haven't tested. We reuse the programs.gnupg.agent.pinentryFlavor option for yubikey-agent, but in doing so I hit a problem: pinentryFlavour's default value is specified in a mkDefault, but only conditionally. We ought to be able to pick up the pinentryFlavour whether or not gpg-agent is running. As a result, this commit moves the default value to the definition of programs.gnupg.agent.enable.

philandstuff · 2020-07-16T14:30:29Z

@flokli pushed another update

flokli · 2020-07-20T22:25:49Z

This LGTM. I poked upstream for a new release containing the unit file in FiloSottile/yubikey-agent#43 (comment), so we could avoid duplicating this.

cc @Ma27 on the pinentry changes.

flokli · 2020-07-23T15:52:26Z

Okay, let's merge this as-is. We can always move to the upstream-provided systemd unit, once there's a new release. 👍

Thanks for your work!

Mea culpa: in NixOS#92936, I did originally test on macOS but I forgot to retest after adding the piv-go patch. Unfortunately, the piv-go patch is broken on macOS (but fortunately it is unnecessary).

Mea culpa: in NixOS#92936, I did originally test on macOS but I forgot to retest after adding the piv-go patch. Unfortunately, the piv-go patch was broken on macOS. This pulls in the latest version of go-piv/piv-go#75 which works on macOS now.

vcunat · 2020-07-26T06:51:32Z

Another problematic platform is i686. I expect it would be OK not to build on unusual platforms unless it made default NixOS depend on the package.

I checked it's not fixed by #93850. You can test on x86_64 by building pkgsi686Linux.yubikey-agent. Example failure on Hydra. Almost all i686 NixOS tests are broken now, EDIT: and nixos-unstable channel is blocked by it as well.

flokli · 2020-07-26T07:37:11Z

@vcunat Uh, somehow I missed this was missing a mkIf cfg.enable, pulling in yubikey-agent unconditionally 🤦

PR at #93892.

nixos/modules/services/security/yubikey-agent.nix

flokli · 2020-07-26T07:55:41Z

The PR was merged, so we can try to fix i686 at a later time. I opened go-piv/piv-go#76 for investigation.

philandstuff mentioned this pull request Jul 11, 2020

Linux packages FiloSottile/yubikey-agent#11

Open

philandstuff force-pushed the add-yubikey-agent branch from 3a4131e to 7092274 Compare July 11, 2020 15:52

philandstuff added 8.has: module (new) 8.has: package (new) labels Jul 11, 2020

ofborg bot added 6.topic: nixos 8.has: module (update) labels Jul 11, 2020

ofborg bot requested a review from kalbasit July 11, 2020 22:54

ofborg bot added 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Jul 11, 2020

f4814 mentioned this pull request Jul 12, 2020

yubikey-agent: init at 0.1.3 #92965

Closed

10 tasks

flokli requested changes Jul 12, 2020

View reviewed changes

flokli reviewed Jul 12, 2020

View reviewed changes

pkgs/tools/security/yubikey-agent/yubikey-agent.service Show resolved Hide resolved

dasJ removed the 8.has: module (update) label Jul 12, 2020

philandstuff force-pushed the add-yubikey-agent branch 2 times, most recently from 128ff96 to d883cfa Compare July 12, 2020 20:46

ofborg bot added the 8.has: module (update) label Jul 12, 2020

philandstuff force-pushed the add-yubikey-agent branch from d883cfa to 70fcbc1 Compare July 14, 2020 20:54

ofborg bot requested a review from rawkode July 14, 2020 21:02

philandstuff force-pushed the add-yubikey-agent branch from 70fcbc1 to ab5373b Compare July 15, 2020 20:38

philandstuff force-pushed the add-yubikey-agent branch from ab5373b to e340c13 Compare July 16, 2020 12:48

flokli requested changes Jul 16, 2020

View reviewed changes

nixos/modules/programs/gnupg.nix Outdated Show resolved Hide resolved

philandstuff force-pushed the add-yubikey-agent branch from e340c13 to e4029c3 Compare July 16, 2020 14:30

flokli merged commit 8f7a623 into NixOS:master Jul 23, 2020

philandstuff mentioned this pull request Jul 24, 2020

yubikey-agent: fix on darwin #93769

Closed

10 tasks

philandstuff mentioned this pull request Jul 25, 2020

yubikey-agent: fix on darwin #93850

Merged

10 tasks

philandstuff commented Jul 26, 2020

View reviewed changes

nixos/modules/services/security/yubikey-agent.nix Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

yubikey-agent: init at 0.1.3 #92936

yubikey-agent: init at 0.1.3 #92936

philandstuff commented Jul 11, 2020

flokli left a comment •

edited

Loading

flokli commented Jul 12, 2020

philandstuff commented Jul 12, 2020

flokli commented Jul 12, 2020 •

edited

Loading

philandstuff commented Jul 12, 2020

FiloSottile commented Jul 12, 2020

philandstuff commented Jul 13, 2020 •

edited

Loading

philandstuff commented Jul 13, 2020

philandstuff commented Jul 14, 2020

philandstuff commented Jul 15, 2020

flokli commented Jul 15, 2020 •

edited

Loading

philandstuff commented Jul 15, 2020

philandstuff commented Jul 16, 2020

rawkode commented Jul 16, 2020

flokli left a comment

philandstuff commented Jul 16, 2020

flokli commented Jul 20, 2020

flokli commented Jul 23, 2020

vcunat commented Jul 26, 2020 •

edited

Loading

flokli commented Jul 26, 2020

flokli commented Jul 26, 2020

yubikey-agent: init at 0.1.3 #92936

yubikey-agent: init at 0.1.3 #92936

Conversation

philandstuff commented Jul 11, 2020

Motivation for this change

Things done

flokli left a comment • edited Loading

Choose a reason for hiding this comment

flokli commented Jul 12, 2020

philandstuff commented Jul 12, 2020

flokli commented Jul 12, 2020 • edited Loading

philandstuff commented Jul 12, 2020

FiloSottile commented Jul 12, 2020

philandstuff commented Jul 13, 2020 • edited Loading

philandstuff commented Jul 13, 2020

philandstuff commented Jul 14, 2020

philandstuff commented Jul 15, 2020

flokli commented Jul 15, 2020 • edited Loading

philandstuff commented Jul 15, 2020

philandstuff commented Jul 16, 2020

rawkode commented Jul 16, 2020

flokli left a comment

Choose a reason for hiding this comment

philandstuff commented Jul 16, 2020

flokli commented Jul 20, 2020

flokli commented Jul 23, 2020

vcunat commented Jul 26, 2020 • edited Loading

flokli commented Jul 26, 2020

flokli commented Jul 26, 2020

flokli left a comment •

edited

Loading

flokli commented Jul 12, 2020 •

edited

Loading

philandstuff commented Jul 13, 2020 •

edited

Loading

flokli commented Jul 15, 2020 •

edited

Loading

vcunat commented Jul 26, 2020 •

edited

Loading