Bug 548763 - [GPG signing] Support unprotected keys
Summary: [GPG signing] Support unprotected keys
Status: RESOLVED FIXED
Alias: None
Product: JGit
Classification: Technology
Component: JGit (show other bugs)
Version: 5.4   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 5.8   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 563350 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-06-28 10:45 EDT by Gael Lalire CLA
Modified: 2020-06-04 16:34 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gael Lalire CLA 2019-06-28 10:45:28 EDT
You first have to update Bouncy castle to at least 1.62.
Then we have partial unprotected key support.

Currently JGIT ask for a key password and use it to find the correct private key.

JGIT should make a first loop on private key without asking for a password, if it fail it ask for password and do a second loop.
Comment 1 Thomas Wolf CLA 2019-06-28 12:29:43 EDT
What is this about? GPG signing? Or SSH keys?
Comment 2 Gael Lalire CLA 2019-06-28 12:35:54 EDT
GPG unprotected key
Comment 3 Thomas Wolf CLA 2019-06-28 12:43:26 EDT
Actually, if it calculated the correct file name instead of iterating over all keys it wouldn't ask needlessly for passwords. Compare bug 547536.
Comment 4 Gael Lalire CLA 2019-06-28 12:56:14 EDT
You will have to change some code. Because currently it is asking and then the loop begins.
It is not looping and asking each time a key is protected.

Going directly to correct file seems way better.

According to https://lists.gnupg.org/pipermail/gcrypt-devel/2013-June/002205.html
It is a sha-1. You can calculate it with MessageDigest.getInstance("SHA-1");
Comment 5 Gael Lalire CLA 2019-06-28 13:27:36 EDT
for RSA key it is just the modulus alone

I think it will be better if the method is provided by PGPPublicKey class inside bouncy castle, but in the mean time.

class BouncyCastleGpgKeyLocator {

	private final static char[] hexArray = "0123456789ABCDEF".toCharArray();

	public static String bytesToHex(byte[] bytes) {
		char[] hexChars = new char[bytes.length * 2];
		for (int j = 0; j < bytes.length; j++) {
			int v = bytes[j] & 0xFF;
			hexChars[j * 2] = hexArray[v >>> 4];
			hexChars[j * 2 + 1] = hexArray[v & 0x0F];
		}
		return new String(hexChars);
	}

	private BouncyCastleGpgKey findSecretKeyForKeyBoxPublicKey(
               ....

		try {
			MessageDigest instance = MessageDigest.getInstance("SHA-1");
			BigInteger modulus = ((RSAPublicBCPGKey) publicKey
					.getPublicKeyPacket().getKey()).getModulus();
			instance.update(modulus.toByteArray());
			System.out.println(bytesToHex(instance.digest()));
		} catch (NoSuchAlgorithmException e1) {
			// TODO Auto-generated catch block
			e1.printStackTrace();
		}
Comment 6 Thomas Wolf CLA 2019-06-28 17:12:40 EDT
(In reply to Gael Lalire from comment #5)
> for RSA key it is just the modulus alone

For other key types it's a little bit more complicated. If the code at [1] is correct, they hash the S-expression (including parentheses and names). And there may be a special case when the high bit is set; see line 875 of [1]. They have some tests at [2], too. To double-check what exactly happens, the libgcrypt code is available at [3].

[1] https://github.com/riboseinc/rnp/blob/e6c7fef/src/librekey/rnp_key_store.cpp#L952
[2] https://github.com/riboseinc/rnp/blob/e6c7fef71b99ef8f7c27fd9922ce23584865d3fd/src/tests/key-grip.cpp
[3] https://github.com/gpg/libgcrypt

> I think it will be better if the method is provided by PGPPublicKey class
> inside bouncy castle, but in the mean time.

Yes, it would be best if Bouncy Castle had a function to compute the key grip for the key types supported by GPG.
Comment 7 Gael Lalire CLA 2019-09-25 08:14:24 EDT
Please update Bouncy castle to at least 1.62
Comment 8 Matthias Sohn CLA 2019-09-25 19:45:03 EDT
(In reply to Gael Lalire from comment #7)
> Please update Bouncy castle to at least 1.62

I filed the following CQs for the latest Bouncycastle 1.63
bcpg-jdk15on 1.63   https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20879
bcprov-jdk15on 1.63 https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20880
bcpkix-jdk15on 1.63 https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20881
when they are approved we can add this version to Orbit and start using it in JGit
Comment 9 Erik Dannenberg CLA 2019-12-29 05:57:38 EST
Just a fyi from the JGit 5.6.0 release notes:

* Update bouncycastle version to 1.64
Comment 10 Gael Lalire CLA 2020-01-07 08:21:14 EST
OK the support is here thanks to BC upgrade.
However we need to validate an empty password dialog on each commit.

We can keep / rename this ticket to support unprotected keys without the confirm dialog.

There are multiples way to do this :
- try with empty password first and if fail ask for a password
- save the password somewhere (gpg agent ? eclipse specific ?)
- find the private key name (see comment 5), check if it need a password and ask for it only if needed
Comment 11 Thomas Wolf CLA 2020-05-19 16:49:02 EDT
*** Bug 563350 has been marked as a duplicate of this bug. ***
Comment 12 Eclipse Genie CLA 2020-05-19 16:50:34 EDT
New Gerrit change created: https://git.eclipse.org/r/163269
Comment 13 Eclipse Genie CLA 2020-06-04 16:16:26 EDT
Gerrit change https://git.eclipse.org/r/163269 was merged to [master].
Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=e3f7a06764b5599ae47f23005ed6dccaf38ba7c8