Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Custom pinentry wrapper for GPG #846

Closed
wants to merge 33 commits into from
Closed

Conversation

smashwilson
Copy link
Contributor

@smashwilson smashwilson commented May 18, 2017

Providing passwords to gpg non-interactively via --passphrase-fd is fragile and somewhat counter to gpg's architecture. As I understand it, gpg2 expects secret key material to only be handled by the gpg-agent process:

The reason why you need to enable the loopback mode is that this breaks a design goal of only allowing the gpg-agent (+scdaemon) to handle the private keys and the passphrases for it.

This only works if the agent was configured with --allow-loopback-pinentry when it was started and, in my version of gpg at least, if --pinentry-mode loopback is provided on the gpg command line, which has the side-effect of preventing user-configured pinentry programs from being attempted at all. At least it looks like aren't the only ones to run into this 😉

This pull request replaces the --passphrase-fd approach with a gpg-wrapper helper that:

  1. Attempts to run the gpg application as-is. If the user has a custom pinentry application that works without a tty, the commit will succeed.
  2. Otherwise, creates a temporary configuration directory within the one we already create for GitPromptServer and copies any existing gpg configuration and data into it.
  3. Launches a new, independent gpg-agent daemon using the temporary directory as a --homedir and specifying gpg-pinentry.sh as a pinentry program.
  4. Re-attempts the gpg operation with that --homedir so that it will connect to our isolated agent.
  5. gpg-pinentry.sh adapts the pinentry Assuan protocol to git-askpass-atom.sh, which connects to the GitPromptServer socket and requests a password just like it does for SSH connections.

Compatibility

The trickiest part here is ensuring that we have reasonably compatibility across both gpg versions and platforms. gpg uses a "home directory" to isolate its keychain, configuration, and so on, and it really doesn't like it if you share a home directory among different gpg versions.

To help with this a bit, I've created a test matrix repository that automates the testing of our driver scripts across gpg versions on macOS and Linux with isolated build prefixes and home directories. Calling gpg and calling our scripts are a bit tricky so that's a work in progress as well. To keep the combinatorial explosion in check somewhat I've limited it to the latest patch release of each minor version.

Here are the platforms I'll be testing on personally, and the current status of each on this branch:

platform gpg 1.4.x gpg 2.0.x, native pinentry gpg 2.0.x, Atom pinentry gpg 2.1.x, native pinentry gpg 2.1.x, Atom pinentry
macOS
Windows 10 N/A N/A
Linux Mint

Key:

  • blank: Not tested yet.
  • ❌: Known to be failing.
  • ☑️: Tested and verified manually by @smashwilson only.
  • ✅: Tested and verified by both @smashwilson and @ungb.

Remaining Work

  • Port gpg-wrapper.sh and gpg-pinentry.sh to .js scripts executed via :electron: processes. The bash scripts are too fragile on Windows. Dugite includes an sh shell that we can use, but it turns out that just about anything that wasn't a shell builtin (like, say, mkdir) was being picked up from my personal git installation and not playing nicely with the Dugite-bundled shell operations at all.
    • Port gpg-wrapper.sh.
    • Port gpg-pinentry.sh.
    • Screw it, going full async/await.
    • Support an explicit gpg-agent location configuration from git.
    • Infer the default gpg-agent location from the path to gpg.
    • Probe and adjust to gpg 1.4.x calls. gpg 1.4 has no gpg-agent, so to support 1.4, we'll need to revert to the --passphrase-fd method. I'd like to avoid an explicit version switch if possible. Right now I'm thinking that I can fall back to --passphrase-fd if no gpg-agent binary is found.
    • Add GitPromptServer and CredentialDialog support for error messages and passphrase retries.
    • Refactor GitShellOutStrategy to eliminate gpgExec calls in favor of regular useGitPromptServer calls.
    • Adjust strategy tests for improved gpg support.
    • (Maybe) 🔧 Refactor conditional diagnostic logging out of GitShellOutStrategy.
    • (Maybe) 🔧 Refactor common code out of GitPromptServer support scripts.
  • Find a way to launch gpg-pinentry.js correctly on Windows.
  • Log and report gpg operations to the dev console when github.gitDiagnostics are enabled. Debugging gpg operations is a royal pain because git eats anything the gpg binary writes to stdout or stderr and only reports success or failure. I'm going to shunt script output to a file in the GitPromptServer temporary directory and dump it to the terminal output if present after a git operation.

Fixes #814.

@smashwilson
Copy link
Contributor Author

@ungb Would you mind giving this PR a spin on Windows and Linux? On each OS we should test that a commit can successfully be signed with the default install of GPG and:

  • No pinentry application. GPG should attempt to use the curses one, fail, and prompt through Atom.
    • You can temporarily disable a pinentry that ships with a distro by commenting out the pinentry-program line in either ${HOME}/.gnupg/gpg-agent.conf (GPG2) or ${HOME}/.gnupg/gpg.conf (for GPG1). On GPG2 you'll also need to restart the gpg-agent with gpgconf --reload gpg-agent.
  • A custom pinentry application. GPG should prompt through the configured external pinentry application (usually a native password entry dialog).
    • I think gpg4win includes one already, so you should just need to un-comment that line and gpgconf --reload gpg-agent to pick it up.
    • On various Linux distros you may need to find one to install separately in the local package manager. On Ubuntu, for example, pinentry-gtk2 should do the trick.

@ungb
Copy link

ungb commented May 19, 2017

I was testing this on mac first, but clicking commit doesn't do anything

checkout this branch and link to atom:
image

on atom this is what I'm seeing.
cannotcommit

I don't get any error logs on console.

Off master this actually works fine with Gpg. I haven't gone on to test the other scenario yet. wanted to follow up if you have time

@smashwilson
Copy link
Contributor Author

I'm guessing that I broke our support for gpg 2.0.x when I fixed it for 2.1.x 😅 Silent failures are no good, though. gpg1 is unlikely to work either now that I think about it.

I suspect what we're going to need to make this really robust is something that lets us automatically exercise the scripts against a variety of gpg versions. I'll see what I can whip up.

@ajsb85
Copy link

ajsb85 commented May 23, 2017

I tested the feature with a GPG without passphrase.

image

image

@ungb
Copy link

ungb commented May 24, 2017

@ajsb85 are you testing this branch? Also, which version of GPG is git configured to use?

@ungb
Copy link

ungb commented May 26, 2017

Sorry for getting to you late, I have both gpg 1 and gpg 2 setup for windows for repro. If you need help getting this working, This tutorial was amazingly helpful. https://jamesmckay.net/2016/02/signing-git-commits-with-gpg-on-windows/

Here's the results, doesn't seem like it works :/ I think you need to quote the string since on windows spaces are weird... See stack trace below.

Also this is from master, I haven't tested this pr.

GPG Version: 1.1.4
OS: Windows 10

Result
Get the following error:

C:\Users\atom\AppData\Local\atom\app-1.18.0-beta2\resources\app\node_modules\github\bin\gpg-no-tty.sh: line 22: c:/Program: No such file or directory
error: gpg failed to sign the data
fatal: failed to write commit object

GPG Version: 2.0.3
OS: Windows 10

Result
Get the following error:

C:\Users\atom\AppData\Local\atom\app-1.18.0-beta2\resources\app\node_modules\github\bin\gpg-no-tty.sh: line 22: c:/Program: No such file or directory
error: gpg failed to sign the data
fatal: failed to write commit object

@smashwilson
Copy link
Contributor Author

Ah, nuts. Thanks for the data, @ungb... I'll likely revisit this next week.

@smashwilson
Copy link
Contributor Author

This is taking me a while, and is likely to take me a bit longer. I've updated the PR description with a checklist as a status update.

@smashwilson
Copy link
Contributor Author

smashwilson commented Jun 9, 2017

The good news is that gpg4win ships with a graphical pinentry application that works fine:

native-pinentry

git:git commit -m heeey look at that in C:\Users\smash\samples\ssh-remote
git-shell-out-strategy.js:214 exit status 0
git-shell-out-strategy.js:215 stdout
git-shell-out-strategy.js:216 [master 666845ce] heeey look at that
 1 file changed, 3 insertions(+)

git-shell-out-strategy.js:217 stderr
git-shell-out-strategy.js:218 10:18:25.625289 git.c:371               trace: built-in: git 'commit' '-m' 'heeey look at that'
10:18:25.787806 run-command.c:369       trace: run_command: 'C:\Users\smash\AppData\Local\Temp\github-11759-7408-crr4x2.6thcfecdi\gpg-wrapper.sh' '-bsau' 'FE4934D9'

git-shell-out-strategy.js:220 gpg
git-shell-out-strategy.js:221 gpg-wrapper: Discovered gpg.program = gpg2 from non-system git configuration.
gpg-wrapper: Attempting to execute gpg with native pinentry.
gpg-wrapper: Executing gpg2 --batch --no-tty --yes --homedir C:\Users\smash\AppData\Roaming\gnupg -bsau FE4934D9.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAABCAAGBQJZOq54AAoJEEN5AANsWkBJ2wsH/A9peo7ibVnDg/OTZAROHnDx
1DndPiTEJ6P8uuK+5EroMSws8tUVFxMW31O31gvATxn893L9D0x2KNqMkPSc2Mo1
x1CJU5Ywqj2Rahz+wQ7SL40gjFnD8OJosj7m4Kw1yUN0kHqkSBxE6TVBkjDdRQfY
6WwBm6mPgQC0AnQPYBHAp/weegZ5/8w3a+DHt0844MmwC7RR968egkn9VOkmM09u
oo7eDn23/CF9rfzXJzdjns0z37VovG/w4EOg4YfIeaKGwa4fpbQEDrvCu+XkusWT
JMuJtsuRCrFczKTtm+046ZkdUNYCi3Rp0+DTkdg1JRRqWBTP19BP4uHmJYJuj9M=
=Vz+4
-----END PGP SIGNATURE-----

🎉

@smashwilson
Copy link
Contributor Author

... And, on further inspection, gpg4win doesn't even ship with the curses pinentry. I'm just going to punt on Atom pinentry support on Windows.

@fynntimes
Copy link

fynntimes commented Jun 21, 2017

@ungb I found a solution for this error:

C:\Users\atom\AppData\Local\atom\app-1.18.0-beta2\resources\app\node_modules\github\bin\gpg-no-tty.sh: line 22: c:/Program: No such file or directory
error: gpg failed to sign the data
fatal: failed to write commit object

Open up C:\Users\atom\AppData\Local\atom\app-1.18.0-beta2\resources\app\node_modules\github\bin\gpg-no-tty.sh.

On line 22 of the file, change this:

exec ${GPG_PROGRAM} --batch --no-tty --yes ${PASSPHRASE_ARG} "$@" 3<<EOM

To this:

exec "${GPG_PROGRAM}" --batch --no-tty --yes ${PASSPHRASE_ARG} "$@" 3<<EOM

Notice the quotation marks. This tells the system that there are spaces in the path. Hope this helps!

@smashwilson
Copy link
Contributor Author

@SirFaizdat D'oh! Good catch. I've opened a PR at #966 since I've backburnered this larger gpg overhaul for a little while.

@the-j0k3r
Copy link

the-j0k3r commented Sep 22, 2017

A heads up because of the mentioning gpg2 here. https://www.gpg4win.org/ has now released v3.0.0
there's no gpg2 package in there...

@FaizaanD yep just tripped on that bug myself., thx. Though it actually is already done in that file, so eh, +1 to that bug.

@smashwilson
Copy link
Contributor Author

A heads up because of the mentioning gpg2 here. https://www.gpg4win.org/ has now released v3.0.0
there's no gpg2 package in there...

Ah, thanks for the heads-up. From the changelog it looks like gpg4win 3.0.0 bundles gpg 2.2.1. I'll make sure I include that in the test matrix.

@the-j0k3r
Copy link

Youre welcome, though I see after so many months of testing so many bugs were left behind :/

@the-j0k3r
Copy link

the-j0k3r commented Sep 22, 2017

Yea here it is again, I purposefully run into this issue, by dismissing the newly release gpg v3 native pin entry and using the embedded GitHub pinentry instead, so you can see what it complain about.

capture

Atom 1.20.1
GitHub 0.4.0
GnuPG4Win V3
GPG 2.2.1

@the-j0k3r
Copy link

@smashwilson hows this coming along? When you have any win 64 builds to test give me a shout if you need a tester.

@smashwilson
Copy link
Contributor Author

@the-j0k3r Well, I'm back from (a) getting atom/watcher shipped and (b) paternity leave, and this is high on our list so I should be getting back to it pretty soon.

I've been rethinking this a bit, too, and:

  • Interacting with gpg is sufficiently complex that I'd like to extract our interactions with it to its own npm package. (This would potentially let Desktop use it, as well.)
  • Given the degree of variance across gpg versions, I'd like to make another effort to bundle a recent gpg build rather than trying to always use the host's local gpg. I already have to create an isolated GNUPGHOME and copy user keys into it to work properly with gpg 2.1 and up, anyway.

@smashwilson
Copy link
Contributor Author

I'm closing this is favor of an issue that I just wrote up that captures my current thinking on this over in #1373. There's a bunch of work here that I'll likely be able to rescue in some other form, so I'll leave the branch around.

@chinedudara
Copy link

I don't get the native GPG graphical Pinentry when i try to commit from atom. I've ran gpgconf --kill gpg-agent countless times, I only get the atom prompt for passphrase. And that always fails to commit.
But i get the graphical Pinentry when i try to commit from git bash. and commit successfully too.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GPG key passphrase not accepted, Atom can't commit staged changes
6 participants