From e3f7a06764b5599ae47f23005ed6dccaf38ba7c8 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 9 Nov 2019 22:05:53 +0100 Subject: [PATCH] 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 --- .../internal/BouncyCastleGpgKeyLocator.java | 61 ++++++++++++++----- .../BouncyCastleGpgKeyPassphrasePrompt.java | 11 +++- .../bc/internal/BouncyCastleGpgSigner.java | 31 +++++++--- 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 e6a48f859..eca45072b 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 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 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 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 740f597a9..e47f64f1a 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 fc6c156d4..c6ecdbe6d 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(),