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

gnupg21: Fix scdaemon for usb smartcards #22891

Merged
merged 2 commits into from Feb 20, 2017

Conversation

danielfullmer
Copy link
Contributor

The use of smartcard functionality for yubikeys (and presumably other
usb smartcards) was broken in gnupg 2.1.18. This has apparently already been
fixed in gnupg master, and debian backports the included patches for 2.1.18.

See also:
https://bugs.gnupg.org/gnupg/issue2933
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=852702
#21991

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

@mention-bot
Copy link

@danielfullmer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @wkennington, @oxij and @kirelagin to be potential reviewers.

@periklis
Copy link
Contributor

@danielfullmer Do you know if these patches have any impact on the problem discussed in issue #20484 ? We, OSX users, have currently imho no clue why scdaemon crashes and suspect the error in OSX impure header incomatibilities. Any clue?

@danielfullmer
Copy link
Contributor Author

@periklis This is a guess on my part, but I would expect not. The included patches fix a regression introduced by 2.1.18, and since OSX users have had this problem with previous versions, I would be surprised if this fixed things for them. Still, it seems worthwhile for someone to test this on OSX.

@kirelagin
Copy link
Member

I confirm, this makes my YubiKey at least visible to scdaemon (I can’t test it with complete gnupg right now as I’m on mac, but I’m working on it).

@periklis

We, OSX users, have currently imho no clue why scdaemon crashes

I’m not sure what you mean, maybe you were thinking about some other issue, but the one you referred to is well-known, and the comments explain pretty clearly what is the cause and how to work it around. Of course, it is in no way related to gnupg itself.

@kirelagin
Copy link
Member

Yes, everything works. I managed to generate a set of keys on the YubiKey and then use it to decrypt a file. I guess, this sould be merged as soon as possible.

./fix-libusb-include-path.patch
# TODO: Remove the following patches on the next gnupg release after 2.1.18
./0028-scd-Backport-two-fixes-from-master.patch
./0029-scd-Fix-use-case-of-PC-SC.patch
Copy link
Member

Choose a reason for hiding this comment

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

Can we maybe fetch these patches from gnupg's version control system instead of adding them to nixpkgs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fpletz While I can find the "Fix-use-case-of-PC-SC" commit in the gnupg repo, I can't find the "Backport two fixes from master" patch in the gnupg repo---probably because the changes in that patch were spread across other commits in master. I've opted to fetch these patches from a debian source. Let me know if you'd prefer something else.

The use of smartcard functionality for yubikeys (and presumably other
usb smartcards) was broken in gnupg 2.1.18.  This has apparently already
been fixed in gnupg master, and debian backports the included patches
for 2.1.18.

See also:
https://bugs.gnupg.org/gnupg/issue2933
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=852702
NixOS#21991
@periklis
Copy link
Contributor

@kirelagin I meant only the issue #20484. However, i check it again right now and read the updates. Thx for the notice, i am still using the DYLD_FRAMEWORK_PATH workaround.

@fpletz fpletz self-assigned this Feb 19, 2017
@fpletz fpletz merged commit a8c7387 into NixOS:master Feb 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants