PublicKeyStore: Add method to choose a valid signer
Due to supporting multiple keys with the same key ID, finding whether
there is a key in the store to produce a given signature is a bit of a
pain. Add a static helper method to PublicKeyStore to make this
easier, and use it from PushCertificateChecker.
Change-Id: I18524412d95b56245755109f1e182cd5d8a43f7a
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyStore.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyStore.java
index 721b101..5d3128a 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyStore.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyStore.java
@@ -23,7 +23,9 @@
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPPublicKeyRingCollection;
+import org.bouncycastle.openpgp.PGPSignature;
import org.bouncycastle.openpgp.bc.BcPGPObjectFactory;
+import org.bouncycastle.openpgp.operator.bc.BcPGPContentVerifierBuilderProvider;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
@@ -74,6 +76,29 @@
/** Ref where GPG public keys are stored. */
public static final String REFS_GPG_KEYS = "refs/meta/gpg-keys";
+ /**
+ * Choose the public key that produced a signature.
+ * <p>
+ * @param keyRings candidate keys.
+ * @param sig signature object.
+ * @param data signed payload.
+ * @return the key chosen from {@code keyRings} that was able to verify the
+ * signature, or null if none was found.
+ * @throws PGPException if an error occurred verifying the signature.
+ */
+ public static PGPPublicKey getSigner(Iterable<PGPPublicKeyRing> keyRings,
+ PGPSignature sig, byte[] data) throws PGPException {
+ for (PGPPublicKeyRing kr : keyRings) {
+ PGPPublicKey k = kr.getPublicKey();
+ sig.init(new BcPGPContentVerifierBuilderProvider(), k);
+ sig.update(data);
+ if (sig.verify()) {
+ return k;
+ }
+ }
+ return null;
+ }
+
private final Repository repo;
private ObjectReader reader;
private RevCommit tip;
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PushCertificateChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PushCertificateChecker.java
index 687729e..4fc641d 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PushCertificateChecker.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PushCertificateChecker.java
@@ -21,12 +21,10 @@
import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPObjectFactory;
import org.bouncycastle.openpgp.PGPPublicKey;
-import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPPublicKeyRingCollection;
import org.bouncycastle.openpgp.PGPSignature;
import org.bouncycastle.openpgp.PGPSignatureList;
import org.bouncycastle.openpgp.bc.BcPGPObjectFactory;
-import org.bouncycastle.openpgp.operator.bc.BcPGPContentVerifierBuilderProvider;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.PushCertificate;
@@ -66,7 +64,7 @@
@SuppressWarnings("resource")
Repository repo = getRepository();
try (PublicKeyStore store = new PublicKeyStore(repo)) {
- checkSignature(sig, cert, store.get(sig.getKeyID()), problems);
+ checkSignature(sig, cert, store, problems);
checkCustom(repo, problems);
} finally {
if (shouldClose(repo)) {
@@ -129,47 +127,31 @@
return null;
}
- private void checkSignature(PGPSignature sig,
- PushCertificate cert, PGPPublicKeyRingCollection keys,
- List<String> problems) {
- List<String> deferredProblems = new ArrayList<>();
- boolean anyKeys = false;
- for (PGPPublicKeyRing kr : keys) {
- PGPPublicKey k = kr.getPublicKey();
- anyKeys = true;
- try {
- sig.init(new BcPGPContentVerifierBuilderProvider(), k);
- sig.update(Constants.encode(cert.toText()));
- if (!sig.verify()) {
- // TODO(dborowitz): Privacy issues with exposing fingerprint/user ID
- // of keys having the same ID as the pusher's key?
- deferredProblems.add(
- "Signature not valid with public key: " + keyToString(k));
- continue;
- }
- CheckResult result = publicKeyChecker.check(k);
- if (result.isOk()) {
- return;
- }
- StringBuilder err = new StringBuilder("Invalid public key ")
- .append(keyToString(k))
- .append(":");
- for (String problem : result.getProblems()) {
- err.append("\n ").append(problem);
- }
- problems.add(err.toString());
- return;
- } catch (PGPException e) {
- deferredProblems.add(
- "Error checking signature with public key " + keyToString(k)
- + ": " + e.getMessage());
- }
+ private void checkSignature(PGPSignature sig, PushCertificate cert,
+ PublicKeyStore store, List<String> problems)
+ throws PGPException, IOException {
+ PGPPublicKeyRingCollection keys = store.get(sig.getKeyID());
+ if (!keys.getKeyRings().hasNext()) {
+ problems.add("No public keys found for key ID "
+ + keyIdToString(sig.getKeyID()));
+ return;
}
- if (!anyKeys) {
- problems.add(
- "No public keys found for key ID " + keyIdToString(sig.getKeyID()));
- } else {
- problems.addAll(deferredProblems);
+ PGPPublicKey signer =
+ PublicKeyStore.getSigner(keys, sig, Constants.encode(cert.toText()));
+ if (signer == null) {
+ problems.add("Signature by " + keyIdToString(sig.getKeyID())
+ + " is not valid");
+ return;
+ }
+ CheckResult result = publicKeyChecker.check(signer);
+ if (!result.isOk()) {
+ StringBuilder err = new StringBuilder("Invalid public key ")
+ .append(keyToString(signer))
+ .append(":");
+ for (String problem : result.getProblems()) {
+ err.append("\n ").append(problem);
+ }
+ problems.add(err.toString());
}
}
}
