GPG: don't prompt for a passphrase for unprotected keys

BouncyCastle supports reading GPG keys without passphrase since 1.62.
Handle this in JGit, too, and don't prompt for a passphrase unless
it's necessary.

Make two passes over the private key files, a first pass without
passphrase provider. If that succeeds it has managed to read a
matching key without passphrase. Otherwise, ask the user for
the passphrase and make a second pass over the key files.

BouncyCastle 1.65 still has no method to get the GPG "key grip" from
a given public key, so JGit still cannot determine the correct file
to read up front. (The file name is the key grip as 40 hex digits,
upper case, with extension ".key").

Bug: 548763
Change-Id: I448181276548c08716d913c7ba1b4bc64c62f952
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

3 files changed, 80 insertions, 23 deletions

diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgKeyLocator.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgKeyLocator.java
index e6a48f859b..eca45072bf 100644
--- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgKeyLocator.java
+++ b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgKeyLocator.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018, Salesforce. and others
+ * Copyright (C) 2018, 2020 Salesforce and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -25,7 +25,9 @@ import java.nio.file.Paths;
 import java.security.NoSuchAlgorithmException;
 import java.security.NoSuchProviderException;
 import java.text.MessageFormat;
+import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Locale;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -76,6 +78,13 @@ public class BouncyCastleGpgKeyLocator {
 
 	}
 
+	/** Thrown if we try to read an encrypted private key without password. */
+	private static class EncryptedPgpKeyException extends RuntimeException {
+
+		private static final long serialVersionUID = 1L;
+
+	}
+
 	private static final Logger log = LoggerFactory
 			.getLogger(BouncyCastleGpgKeyLocator.class);
 
@@ -433,22 +442,46 @@ public class BouncyCastleGpgKeyLocator {
 		PGPDigestCalculatorProvider calculatorProvider = new JcaPGPDigestCalculatorProviderBuilder()
 				.build();
 
-		PBEProtectionRemoverFactory passphraseProvider = new JcePBEProtectionRemoverFactory(
-				passphrasePrompt.getPassphrase(publicKey.getFingerprint(),
-						userKeyboxPath));
-
 		try (Stream<Path> keyFiles = Files.walk(USER_SECRET_KEY_DIR)) {
-			for (Path keyFile : keyFiles.filter(Files::isRegularFile)
-					.collect(Collectors.toList())) {
-				PGPSecretKey secretKey = attemptParseSecretKey(keyFile,
-						calculatorProvider, passphraseProvider, publicKey);
-				if (secretKey != null) {
-					if (!secretKey.isSigningKey()) {
-						throw new PGPException(MessageFormat.format(
-								BCText.get().gpgNotASigningKey, signingKey));
+			List<Path> allPaths = keyFiles.filter(Files::isRegularFile)
+					.collect(Collectors.toCollection(ArrayList::new));
+			if (allPaths.isEmpty()) {
+				return null;
+			}
+			PBEProtectionRemoverFactory passphraseProvider = p -> {
+				throw new EncryptedPgpKeyException();
+			};
+			for (int attempts = 0; attempts < 2; attempts++) {
+				// Second pass will traverse only the encrypted keys with a real
+				// passphrase provider.
+				Iterator<Path> pathIterator = allPaths.iterator();
+				while (pathIterator.hasNext()) {
+					Path keyFile = pathIterator.next();
+					try {
+						PGPSecretKey secretKey = attemptParseSecretKey(keyFile,
+								calculatorProvider, passphraseProvider,
+								publicKey);
+						pathIterator.remove();
+						if (secretKey != null) {
+							if (!secretKey.isSigningKey()) {
+								throw new PGPException(MessageFormat.format(
+										BCText.get().gpgNotASigningKey,
+										signingKey));
+							}
+							return new BouncyCastleGpgKey(secretKey,
+									userKeyboxPath);
+						}
+					} catch (EncryptedPgpKeyException e) {
+						// Ignore; we'll try again.
 					}
-					return new BouncyCastleGpgKey(secretKey, userKeyboxPath);
 				}
+				if (attempts > 0 || allPaths.isEmpty()) {
+					break;
+				}
+				// allPaths contains only the encrypted keys now.
+				passphraseProvider = new JcePBEProtectionRemoverFactory(
+						passphrasePrompt.getPassphrase(
+								publicKey.getFingerprint(), userKeyboxPath));
 			}
 
 			passphrasePrompt.clear();
diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgKeyPassphrasePrompt.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgKeyPassphrasePrompt.java
index 740f597a9f..e47f64f1a6 100644
--- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgKeyPassphrasePrompt.java
+++ b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgKeyPassphrasePrompt.java
@@ -1,5 +1,5 @@
 /*-
- * Copyright (C) 2019, Salesforce. and others
+ * Copyright (C) 2019, 2020 Salesforce and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -97,4 +97,13 @@ class BouncyCastleGpgKeyPassphrasePrompt implements AutoCloseable {
 		return passphrase.getValue();
 	}
 
+	/**
+	 * Determines whether a passphrase was already obtained.
+	 *
+	 * @return {@code true} if a passphrase is already set, {@code false}
+	 *         otherwise
+	 */
+	public boolean hasPassphrase() {
+		return passphrase != null && passphrase.getValue() != null;
+	}
 }
diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgSigner.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgSigner.java
index fc6c156d47..c6ecdbe6da 100644
--- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgSigner.java
+++ b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/BouncyCastleGpgSigner.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018, Salesforce. and others
+ * Copyright (C) 2018, 2020, Salesforce and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -104,13 +104,28 @@ public class BouncyCastleGpgSigner extends GpgSigner {
 				throw new JGitInternalException(
 						BCText.get().unableToSignCommitNoSecretKey);
 			}
-			char[] passphrase = passphrasePrompt.getPassphrase(
-					secretKey.getPublicKey().getFingerprint(),
-					gpgKey.getOrigin());
-			PGPPrivateKey privateKey = secretKey
-					.extractPrivateKey(new JcePBESecretKeyDecryptorBuilder()
-							.setProvider(BouncyCastleProvider.PROVIDER_NAME)
-							.build(passphrase));
+			JcePBESecretKeyDecryptorBuilder decryptorBuilder = new JcePBESecretKeyDecryptorBuilder()
+					.setProvider(BouncyCastleProvider.PROVIDER_NAME);
+			PGPPrivateKey privateKey = null;
+			if (!passphrasePrompt.hasPassphrase()) {
+				// Either the key is not encrypted, or it was read from the
+				// legacy secring.gpg. Try getting the private key without
+				// passphrase first.
+				try {
+					privateKey = secretKey.extractPrivateKey(
+							decryptorBuilder.build(new char[0]));
+				} catch (PGPException e) {
+					// Ignore and try again with passphrase below
+				}
+			}
+			if (privateKey == null) {
+				// Try using a passphrase
+				char[] passphrase = passphrasePrompt.getPassphrase(
+						secretKey.getPublicKey().getFingerprint(),
+						gpgKey.getOrigin());
+				privateKey = secretKey
+						.extractPrivateKey(decryptorBuilder.build(passphrase));
+			}
 			PGPSignatureGenerator signatureGenerator = new PGPSignatureGenerator(
 					new JcaPGPContentSignerBuilder(
 							secretKey.getPublicKey().getAlgorithm(),