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

Support configurable gpg.<format>.programs #92

Open
jsoo1 opened this issue Sep 25, 2024 · 7 comments
Open

Support configurable gpg.<format>.programs #92

jsoo1 opened this issue Sep 25, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@jsoo1
Copy link

jsoo1 commented Sep 25, 2024

Description

openssl, gpg and ssh-keygen are only the default signing/verifying programs in git. jgit could provide a lot of flexibility by allowing the user to specify the sign/verify programs themselves by honoring the existing gpg.<format>.program configuration option: https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgltformatgtprogram

Gitsign, for example, uses a gpg.x509.program configuration to sign.

Motivation

There are open issues for S/MIME (#49) and ssh (#44) support. Allowing the user to specify an external program would provide that flexibility (so long as gpg.format was honored).

Moreover the current arrangement does not allow much in the way of customizing the trust database - a separate program can provide the functionality of only trusting some set of keys or some keyserver (or other more elaborate configuration).

Alternatives considered

Not many, frankly. I just know that the current bouncycastle implementation will not work for our signature verification.

Additional context

I would be happy to pick up work on this if there is any interest and no one else is already doing it.

@jsoo1 jsoo1 changed the title Support configuable gpg.<format>.programs Support configurable gpg.<format>.programs Sep 25, 2024
@msohn msohn added the enhancement New feature or request label Sep 25, 2024
@tomaswolf
Copy link
Contributor

Note that EGit has implemented calling an external gpg/gpgsm. See package org.eclipse.egit.core.internal.signing. Some of that could be moved to JGit, but JGit will have to provide suitable means to customize calling the external program. For instance, when calling gpg from Eclipse, we must make sure that certain environment variables are not set (to ensure that gpg doesn't try to use a terminal for asking for passphrases). The same problem will also exist for SSH signatures if done via calling ssh-keygen: if called from Eclipse, prompts must not happen in a terminal but in a dialog. SSH signatures can be done fully in Java, though, with no loss of functionality and no problems finding keys. I am currently in the process of implementing exactly that (signing is easy, verifying signatures is complicated).

@jsoo1
Copy link
Author

jsoo1 commented Sep 30, 2024

Note that EGit has implemented calling an external gpg/gpgsm. See package org.eclipse.egit.core.internal.signing. Some of that could be moved to JGit, but JGit will have to provide suitable means to customize calling the external program.

Ok I looked at the EGit implementation and I'm a bit confused. The docs for the gpg.program
(and by extension gpg.<format>.program) say:

Use this custom program instead of "gpg" found on $PATH when making or verifying a PGP signature. The program must support the same command-line interface as GPG, namely, to verify a detached signature, "gpg --verify $signature - <$file" is run, and the program is expected to signal a good signature by exiting with code 0. To generate an ASCII-armored detached signature, the standard input of "gpg -bsau $key" is fed with the contents to be signed, and the program is expected to send the result to its standard output.

But the egit version also adds --signal-fd here: https://github.com/eclipse-egit/egit/blob/e0f8ba40a4b346cfa00d229cd69bc71d2c428548/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/signing/ExternalGpgSignatureVerifier.java#L108

Second, the egit version does not use the exit code to determine if verification failed (which is all it should do): https://github.com/eclipse-egit/egit/blob/e0f8ba40a4b346cfa00d229cd69bc71d2c428548/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/signing/ExternalGpgSignatureVerifier.java#L176

Third the egit version adds too many flags to the signer: https://github.com/eclipse-egit/egit/blob/e0f8ba40a4b346cfa00d229cd69bc71d2c428548/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/signing/ExternalGpgSigner.java#L110 (it should only do what the config says it does).

Are these because the docs are wrong, git needs fixing or egit needs fixing?

For instance, when calling gpg from Eclipse, we must make sure that certain environment variables are not set (to ensure that gpg doesn't try to use a terminal for asking for passphrases). The same problem will also exist for SSH signatures if done via calling ssh-keygen: if called from Eclipse, prompts must not happen in a terminal but in a dialog. SSH signatures can be done fully in Java, though, with no loss of functionality and no problems finding keys. I am currently in the process of implementing exactly that (signing is easy, verifying signatures is complicated).

Could you elaborate on why inheriting the environ of the calling process won't be sufficient?

I would think that the correct place to handle environ would be in the calling process.

@tomaswolf
Copy link
Contributor

tomaswolf commented Oct 1, 2024

Ok I looked at the EGit implementation and I'm a bit confused. The docs for the gpg.program (and by extension gpg.<format>.program) say:

Use this custom program instead of "gpg" found on $PATH when making or verifying a PGP signature. The program must support the same command-line interface as GPG, namely, to verify a detached signature, "gpg --verify $signature - <$file" is run, and the program is expected to signal a good signature by exiting with code 0. To generate an ASCII-armored detached signature, the standard input of "gpg -bsau $key" is fed with the contents to be signed, and the program is expected to send the result to its standard output.

But the egit version also adds --signal-fd here: https://github.com/eclipse-egit/egit/blob/e0f8ba40a4b346cfa00d229cd69bc71d2c428548/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/signing/ExternalGpgSignatureVerifier.java#L108

You mean --status-fd. Yes we do, and that's perfectly fine. As the comment says, this sends "additional status lines" to the file descriptor given (in that case, 1, i.e., stdout.) These are the lines starting with "[GNUPG:]"; they give us a way to parse results from gpg in a standard way independent of any localizations in gpg. The texts that gpg returns in these "[GNUPG:]" status lines is API and is guaranteed to remain stable. C git also uses these lines.

Besides, take a look a the C git code:

C git also passes these --status-fd arguments, and expects the program to return "[GNUPG:]" lines. In fact, for signing it expects gpg to exit with exit code zero and to produce a line "[GNUPG:] SIG_CREATED "; see https://github.com/git/git/blob/3857aae53f36/gpg-interface.c#L997-L1007

Second, the egit version does not use the exit code to determine if verification failed (which is all it should do): https://github.com/eclipse-egit/egit/blob/e0f8ba40a4b346cfa00d229cd69bc71d2c428548/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/signing/ExternalGpgSignatureVerifier.java#L176

This is wrong. EGit does consider the exit code of the program called; see https://github.com/eclipse-egit/egit/blob/e0f8ba40/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/signing/ExternalProcessRunner.java#L69 .

Third the egit version adds too many flags to the signer: https://github.com/eclipse-egit/egit/blob/e0f8ba40a4b346cfa00d229cd69bc71d2c428548/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/signing/ExternalGpgSigner.java#L110 (it should only do what the config says it does).

No, it doesn't. This is canLocateSigningKey, which shall list the wanted key. Besides, this is EGit, not C git. We are absolutely free to call gpg with whatever arguments we want as long as we can implement the wanted functionality. If this breaks a custom gpg.program, so be it. It could be documented as a restriction in the JGit documentation. (Note that the EGit GPG signing feature was rolled out three years ago, and there has not been any complaint about EGit expecting the program to be able to make sense of more options.)

Are these because the docs are wrong, git needs fixing or egit needs fixing?

Perhaps the C git documentation could be fixed to say more clearly what exactly is expected of a custom gpg.program.
In general, I would think whatever that program is, it must accept any gpg command line options and produce the same output as gpg. Note that the paragraph you quoted above from the C git dcoumentation says "The program must support the same command-line interface as GPG, namely, [...]". Perhaps "namely" should be replaced with "in particular".

For instance, when calling gpg from Eclipse, we must make sure that certain environment variables are not set (to ensure that gpg doesn't try to use a terminal for asking for passphrases). The same problem will also exist for SSH signatures if done via calling ssh-keygen: if called from Eclipse, prompts must not happen in a terminal but in a dialog. SSH signatures can be done fully in Java, though, with no loss of functionality and no problems finding keys. I am currently in the process of implementing exactly that (signing is easy, verifying signatures is complicated).

Could you elaborate on why inheriting the environ of the calling process won't be sufficient?

Because the calling process may have these environment variables set. For instance, when you start Eclipse from a terminal. In the terminal, these environment variables might make gpg prompt inside the terminal. Eclipse will inherit these variables, but when gpg is called from Eclipse, you'll want the gpg prompt to use a dialog. If gpg prompts within the terminal, the user may not even see the prompt and wonder why Eclipse or EGit "hangs". Therefore EGit unsets these variables for the child process.

I would think that the correct place to handle environ would be in the calling process.

Exactly. And the calling process is the one executing the EGit code, hence EGit takes care to unset these variables so that the child process executing gpg gets the correct environment.

@jsoo1
Copy link
Author

jsoo1 commented Oct 1, 2024

Sorry for the initial frustrated tone. As I looked into how things actually work I got increasingly disappointed and I understood more the reasons the egit version does it the way it does.

You mean --status-fd. Yes we do, and that's perfectly fine. As the comment says, this sends "additional status lines" to the file descriptor given (in that case, 1, i.e., stdout.) These are the lines starting with "[GNUPG:]"; they give us a way to parse results from gpg in a standard way independent of any localizations in gpg. The texts that gpg returns in these "[GNUPG:]" status lines is API and is guaranteed to remain stable. C git also uses these lines.

Besides, take a look a the C git code:

C git also passes these --status-fd arguments, and expects the program to return "[GNUPG:]" lines. In fact, for signing it expects gpg to exit with exit code zero and to produce a line "[GNUPG:] SIG_CREATED "; see https://github.com/git/git/blob/3857aae53f36/gpg-interface.c#L997-L1007

Thanks for the correction, yes I meant --status-fd. After some looking I saw that it does also use these lines. but this makes me sad because it severely limits the kinds of programs {j,e,c}git might otherwise use. I almost want to send something to the list about this. I definitely don't blame you for doing the same (in fact, how else would one go about this? use libgpg-error?).

This is wrong. EGit does consider the exit code of the program called; see https://github.com/eclipse-egit/egit/blob/e0f8ba40/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/signing/ExternalProcessRunner.java#L69 .

I see, so the IOException would bubble up, presumably to the caller of verify? That seems like a reasonable first step. I see why you did that and I agree. My only quibble is maybe a more precise exception type would help the caller deduce the exception reason a little more easily.

No, it doesn't. This is canLocateSigningKey, which shall list the wanted key. Besides, this is EGit, not C git. We are absolutely free to call gpg with whatever arguments we want as long as we can implement the wanted functionality. If this breaks a custom gpg.program, so be it. It could be documented as a restriction in the JGit documentation. (Note that the EGit GPG signing feature was rolled out three years ago, and there has not been any complaint about EGit expecting the program to be able to make sense of more options.)

I understand that sentiment. I think this is my only real concern here. I mentioned the other two issues for x509 and ssh sigs because I would like to give the user maximum flexibility to implement their own sig/verify infrastructure. Any additional requirements {j,e,c}git like requiring a --status-fd flag or a particular output format limits that user choice. Not only is this nice for the user but it limits the amount of work required on the {j,e,c}git side if the interface has a smaller surface.

Again I understand why it was done this way but I want to explore the benefit of requiring less of the gpg.<format>.program (i.e. using fewer flags, requiring less precisely formatted output, etc) than is currently required. This is a good place to deviate from cgit imo.

Perhaps the C git documentation could be fixed to say more clearly what exactly is expected of a custom gpg.program. In general, I would think whatever that program is, it must accept any gpg command line options and produce the same output as gpg. Note that the paragraph you quoted above from the C git dcoumentation says "The program must support the same command-line interface as GPG, namely, [...]". Perhaps "namely" should be replaced with "in particular".

The docs gave me the strong impression that those were the only requirements. I'll follow up on the list about that. The colons output, for instance, is not mentioned.

I would think that the correct place to handle environ would be in the calling process.

Exactly. And the calling process is the one executing the EGit code, hence EGit takes care to unset these variables so that the child process executing gpg gets the correct environment.

Glad we agree, here :) - I was curious if there was a footgun I need to avoid, in particular. (I'm not a ProcessBuilder expert).

@tomaswolf
Copy link
Contributor

EGit does things a bit a differently, mainly because it is intended for a program with a graphical UI. We want to have a canLocateSigningKey so that we can disable UI widgets related to signing if signing isn't even configured. It isn't called anywhere in JGit as far as I know. A port to JGit may provide a dumb implementation always returning true if an external gpg is used. But there must be a way for EGit to override/customize it.

The other options for signing that differ from C git (like --batch, --no-tty, ...) also have to do with making sure that gpg does not prompt in a terminal but uses its UI support and gives the user nice dialogs. Again, for the JGit library they would not be necessary, but users of the JGit library will need a way to customize the command-line options. (Which, of course, in the end still means that the called program would have to accept such extra options.)

A port to the JGit library also would not need to fiddle with the environment like EGit does, but JGit would need to provide means such that EGit could customize the environment the way it does. We really need to avoid gpg prompting in a terminal when calling it from a graphical UI application like Eclipse.

Here's an idea, though: perhaps most of the EGit specifics can be avoided in Java if EGit has its own wrapper script around whatever gpg.program is configured, and tweaks the GpgConfig such that this script is called, so JGit might in the end not need to be configurable in that respect. But to work in EGit, the called program would of course still need to support these extra arguments. And EGit would need to have at least two such scripts (a shell script for Unix/Linux/Mac OS, and something for Windows), and they'd need to be portable across OS versions (and shells; cannot assume bash).

--status-fd is probably here to stay given that C git also uses it. It is also the only way to distinguish a cancelled signing attempt (user didn't enter a passphrase, clicked "cancel", or just closed the gpg dialog) from a true signing failure. In an IDE, this is kind of important for a good user experience.

@jsoo1
Copy link
Author

jsoo1 commented Oct 7, 2024

EGit does things a bit a differently, mainly because it is intended for a program with a graphical UI. We want to have a canLocateSigningKey so that we can disable UI widgets related to signing if signing isn't even configured. It isn't called anywhere in JGit as far as I know. A port to JGit may provide a dumb implementation always returning true if an external gpg is used. But there must be a way for EGit to override/customize it.

Do you think the canLocateSigningKey can be an EGit specific? It seems like an odd choice to include it in JGit.

The other options for signing that differ from C git (like --batch, --no-tty, ...) also have to do with making sure that gpg does not prompt in a terminal but uses its UI support and gives the user nice dialogs. Again, for the JGit library they would not be necessary, but users of the JGit library will need a way to customize the command-line options. (Which, of course, in the end still means that the called program would have to accept such extra options.)

A port to the JGit library also would not need to fiddle with the environment like EGit does, but JGit would need to provide means such that EGit could customize the environment the way it does. We really need to avoid gpg prompting in a terminal when calling it from a graphical UI application like Eclipse.

+1

Here's an idea, though: perhaps most of the EGit specifics can be avoided in Java if EGit has its own wrapper script around whatever gpg.program is configured, and tweaks the GpgConfig such that this script is called, so JGit might in the end not need to be configurable in that respect. But to work in EGit, the called program would of course still need to support these extra arguments. And EGit would need to have at least two such scripts (a shell script for Unix/Linux/Mac OS, and something for Windows), and they'd need to be portable across OS versions (and shells; cannot assume bash).

I don't think I really know enough about the design goals of Eclipse/EGit to say what the ideal design would be. That said I would think the goal of being able to specify a program for sign/verify is to allow the user freedom to configure it themselves. Does Eclipse offer a way to do that at the moment? If not, I think that would be the direction I would take a IDE of my design. But again I don't fully understand the EGit design values.

Could you live with EGit shipping a wrapper or allowing the user to configure it? It seems to me that JGit should either try to a) improve on cgit or b) emulate cgit as closely as possible to avoid surprise.

--status-fd is probably here to stay given that C git also uses it. It is also the only way to distinguish a cancelled signing attempt (user didn't enter a passphrase, clicked "cancel", or just closed the gpg dialog) from a true signing failure. In an IDE, this is kind of important for a good user experience.

If emulating cgit is the desire, it sure seems so. Oof.

@tomaswolf
Copy link
Contributor

Do you think the canLocateSigningKey can be an EGit specific? It seems like an odd choice to include it in JGit.

I don't think so. How exactly it is implemented is an implementation detail of individual signers. EGit has no way of knowing how the BouncyCastle GPG bundle locates keys.

Here's an idea, though: perhaps most of the EGit specifics can be avoided in Java if EGit has its own wrapper script around whatever gpg.program is configured, and tweaks the GpgConfig such that this script is called, so JGit might in the end not need to be configurable in that respect. But to work in EGit, the called program would of course still need to support these extra arguments. And EGit would need to have at least two such scripts (a shell script for Unix/Linux/Mac OS, and something for Windows), and they'd need to be portable across OS versions (and shells; cannot assume bash).

I don't think I really know enough about the design goals of Eclipse/EGit to say what the ideal design would be. That said I would think the goal of being able to specify a program for sign/verify is to allow the user freedom to configure it themselves. Does Eclipse offer a way to do that at the moment? If not, I think that would be the direction I would take a IDE of my design. But again I don't fully understand the EGit design values.

EGit uses either the program defined in the git config, or the one defined in the EGit preferences.

Could you live with EGit shipping a wrapper or allowing the user to configure it?

Wrapper scripts would be a significant development effort, especially for Windows. But it might be a possibility. It was just an idea; if calling a program is implemented in JGit, I would expect the implementation in EGit to be changed/updated accordingly to make use of the new JGit functionality. Then we'll see whether a wrapper script is really a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants