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

GnuPG: add scdaemon shared access patch #97951

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

uosis
Copy link
Contributor

@uosis uosis commented Sep 14, 2020

Motivation for this change

GnuPG stubbornly refuses to share access to smartcards, which, among other things, makes it not work properly with Yubikey-manager.

This adds a patch maintained by MacOS GPGTools which adds an option to enable shared smartcard access.

There is no change in default behavior, and patch applies cleanly without modifications, so I see no reason why Nix should not have it as well.

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.

@nh2
Copy link
Contributor

nh2 commented Sep 14, 2020

Added description with links. Let me know if you would like to see something more.

@uosis That's great, comments thanks.

Should this patch use fetchpatch (making it easier to update the patch given that https://github.com/GPGTools/MacGPG2/blob/dev/patches/gnupg/scdaemon_shared-access.patch seems to change every now and then with newer GPG releases) or does that not work for this package?

@nh2
Copy link
Contributor

nh2 commented Sep 14, 2020

For other reviewers:

GPG upstream seems to find shared card access controversial because it may (apparently?) allow other apps to interact with a hardware-button security key than the one that requested the operation (GPG), see https://wiki.archlinux.org/index.php/GnuPG#Shared_access_with_pcscd (now linked in the comment).

It seems a legitimate use case to me to make shared card access an optional add-in feature for people who want to trade usability for a changed threat model, so carrying this patch looks sensible to me.

@nh2
Copy link
Contributor

nh2 commented Sep 14, 2020

@uosis How is this added feature supposed to be used? The Arch wiki says

After patching your scdaemon you can enable shared access by modifying your scdaemon.conf file and adding shared-access line end of it.

Should there also be a NixOS setting to enable that easily?

@uosis
Copy link
Contributor Author

uosis commented Sep 14, 2020

Should this patch use fetchpatch (making it easier to update the patch given that https://github.com/GPGTools/MacGPG2/blob/dev/patches/gnupg/scdaemon_shared-access.patch seems to change every now and then with newer GPG releases) or does that not work for this package?

I see no reason to not use fetchpatch - I was just following the same pattern as other patches in this package. Changed to use fetchpatch.

GPG upstream seems to find shared card access controversial because it may (apparently?) allow other apps to interact with a hardware-button security key than the one that requested the operation (GPG), see https://wiki.archlinux.org/index.php/GnuPG#Shared_access_with_pcscd (now linked in the comment).

And just to add to this, while the security concern may have had some weight in the old days of plain "dumb" smartcards, now that smartcards are actually USB tokens with physical button authorization for each operation, the whole argument is moot.

@uosis How is this added feature supposed to be used? The Arch wiki says

After patching your scdaemon you can enable shared access by modifying your scdaemon.conf file and adding shared-access line end of it.

Should there also be a NixOS setting to enable that easily?

It is a user level configuration setting in ~/.gnupg, so I don't think there is anything to do here at NixOS level. I have home-manager branch that adds this option that I will try to get merged in after this PR is in.

@uosis uosis requested a review from nh2 September 14, 2020 22:29
@stale
Copy link

stale bot commented Mar 16, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 16, 2021
@uosis
Copy link
Contributor Author

uosis commented Mar 16, 2021

@peti @vrthra @fpletz any opinion re: merging this?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 16, 2021
@stale
Copy link

stale bot commented Sep 14, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 14, 2021
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 7, 2023
@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank marked this pull request as draft March 20, 2024 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants