Skip to content

Fixes for gnupg agent module #26888

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.

Sign up for GitHub

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
Merged
merged 3 commits into from
Jul 13, 2017
Merged

Fixes for gnupg agent module #26888

merged 3 commits into from
Jul 13, 2017
+2 −57

Conversation

danielfullmer
Copy link
Contributor

Motivation for this change

1b6176e caused a few problems for me with the gnupg agent module in nixos.
With programs.gnupg.agent.enable and programs.gnupg.agent.enableSSHSupport both set to true, logging in freezes for me while running gpg-connect-agent.

Additionally, I get the following messages in the journal:

machine# [ 6.342172] gpg-agent[837]: gpg-agent (GnuPG) 2.1.21 starting in supervised mode.
machine# [ 6.343164] gpg-agent[837]: using fd 3 for std socket (/run/user/1000/gnupg/S.gpg-agent)
machine# [ 6.344159] gpg-agent[837]: cannot listen on more than one std socket
machine# [ 6.345041] gpg-agent[837]: using fd 5 for ssh socket (/run/user/1000/gnupg/S.gpg-agent.ssh)
machine# [ 6.346120] gpg-agent[837]: cannot listen on more than one ssh socket
machine# [ 6.347068] gpg-agent[837]: listening on: std=3 extra=-1 browser=-1 ssh=5

It appears that duplicating the systemd units in both systemd.packages as well as systemd.user.{sockets,services} causes gpg-agent to try to bind to multiple sockets. Perhaps this causes gpg-connect-agent to freeze? In any case, I don't see why we can't just use the upstream systemd units, which fixes the problem for me anyway.

In case it's useful, I'll mention that I also wrote some nixos tests to ensure the gpg agent config is working properly, since this seems to break for me frequently. These tests ensure pinentry displays properly when invoked through gpg-agent, both for console and x11 usage:
https://github.com/danielfullmer/nixos-config/blob/master/tests/gpg-agent.nix
https://github.com/danielfullmer/nixos-config/blob/master/tests/gpg-agent-x11.nix

While writing these tests, I also noticed that the nixos test driver fails with:

machine# [ 3.697077] backdoor-start[669]: gpg-connect-agent: failed to create temporary file '/root/.gnupg/.#lk0x0000000001795470.machine.675': No such file or directory

I can't think of any reason why noninteractive shells would need to call gpg-connect-agent, so I restricted that to interactiveShellInit, but I might be overlooking something by doing so.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • 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/)
  • Fits CONTRIBUTING.md.

CC @CMCDragonkai @fpletz

Sorry, something went wrong.

@danielfullmer
gnupg agent module: Remove unnecessary unit configuration

Verified

This commit was signed with the committer’s verified signature.
edwtjo Edward Tjörnhammar
GPG key ID: 7B82CE4A866B6845
Verified
Learn about vigilant mode
238996f 
These just seem to duplicate upstream systemd units, which are already
included in nixos configuration by systemd.packages
@danielfullmer
gnupg agent module: Fix dirmngr.enable option

Verified

This commit was signed with the committer’s verified signature.
edwtjo Edward Tjörnhammar
GPG key ID: 7B82CE4A866B6845
Verified
Learn about vigilant mode
a5c793d
@danielfullmer
gnupg agent module: Only set tty for interactive shells

Verified

This commit was signed with the committer’s verified signature.
edwtjo Edward Tjörnhammar
GPG key ID: 7B82CE4A866B6845
Verified
Learn about vigilant mode
aac71c7
@CMCDragonkai
Copy link
Member

CMCDragonkai commented Jun 28, 2017 •
edited
Loading

Does systemd.packages = [ pkgs.gnupg ]; place the sockets and services as systemd user-services and user-sockets as well (this is apparently because gnupg services are user-specific and not whole-system)? I'm not entirely sure how systemd.packages = [ pkgs.gnupg ]; is interacting with the systemd.user nix code. This is because my configuration had only the systemd.user config and not the that specific line. Also if the upstream systemd units supplied by gnupg package works, why bother specifying any systemd.user config at all, then they should not be needed.

Also there's a bit of a balance between reifying configuration as nix code, or leaving config as upstream. In some places of nixos, we've been moving configuration to use the nix language, this makes it somewhat more flexible when working in a nix configuration since overrides are possible. In other cases, porting the configuration takes too long and we have just left them as raw text that gets substituted into the relevant locations. I'm not sure whether it's better to have the systemd config for gnupg reified as nix config, or left as just systemd unit files.

0xABAB
0xABAB suggested changes Jun 28, 2017
View reviewed changes
Copy link
Contributor

@0xABAB 0xABAB left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I am not convinced, since @danielfullmer is not convinced of himself. I would like to delegate to CMCDragonkai the final decision.

@fpletz fpletz self-requested a review June 28, 2017 20:08
@fpletz fpletz self-assigned this Jun 28, 2017
@mdorman
Copy link
Contributor

mdorman commented Jul 3, 2017

I'll mention that the programs.gnupg.agent material doesn't work for me without these packages---as @danielfullmer observes, I just get a hang on login. With the changes, it works great!

fpletz
fpletz approved these changes Jul 13, 2017
View reviewed changes
Copy link
Member

@fpletz fpletz left a comment

Choose a reason for hiding this comment

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

I think the preferred solution is to just override the upstream units. Putting gnupg in systemd.packages works just fine for user units. We're just not reloading the systemd user instances on a system switch.

@fpletz
Copy link
Member

fpletz commented Jul 13, 2017

Thanks a lot! 🍻

@fpletz fpletz merged commit 627260d into NixOS:master Jul 13, 2017
@danielfullmer danielfullmer deleted the gnupg-agent-fix branch July 13, 2017 23:15
globin added a commit that referenced this pull request Jul 17, 2017
@globin
programs.gnupg: use extraInit instead of interactiveShellInit

Verified

This commit was signed with the committer’s verified signature.
edwtjo Edward Tjörnhammar
GPG key ID: 7B82CE4A866B6845
Verified
Learn about vigilant mode
b8d92a7 
Otherwise some programmes cannot use the GPG agent, e.g. applications
started from dmenu.

Behaviour was changed in #26888, this reverts that part.
@globin
Copy link
Member

globin commented Jul 17, 2017 •
edited
Loading

Reverted the interactiveShellInit in b8d92a7 as that caused software using the gpg-agent, started from dmenu, to stop working correctly. (virt-manager using the ssh keys)
That is now fixed.

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

@0xABAB 0xABAB

@fpletz fpletz

Assignees

@fpletz fpletz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@danielfullmer @CMCDragonkai @mdorman @fpletz @globin @0xABAB