PublicKeyChecker: Remove expected key ID checks
The only place where we passed in a key ID that was not directly
extracted from the public key in question was in
PushCertificateChecker, where we used the key ID from the signature.
However, the only class of problems this would catch is where a key
in the PublicKeyStore returned by get(id) has an ID other than "id".
At this point we're confident enough in the implementation of
PublicKeyStore that we don't need to worry about this.
Change-Id: I7220b987c38f939d7564a2be4684c08c3013a14c
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
index 52cddf2..8a8269f 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPublicKeyChecker.java
@@ -65,8 +65,7 @@
}
@Override
- public void checkCustom(PGPPublicKey key, long expectedKeyId,
- List<String> problems) {
+ public void checkCustom(PGPPublicKey key, List<String> problems) {
try {
Set<String> allowedUserIds = getAllowedUserIds();
if (allowedUserIds.isEmpty()) {
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java
index 7b7aabb..46fbcbc 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/PublicKeyChecker.java
@@ -14,8 +14,6 @@
package com.google.gerrit.gpg;
-import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString;
-
import org.bouncycastle.openpgp.PGPPublicKey;
import java.util.ArrayList;
@@ -29,21 +27,7 @@
* @param key the public key.
*/
public final CheckResult check(PGPPublicKey key) {
- return check(key, key.getKeyID());
- }
-
- /**
- * Check a public key.
- *
- * @param key the public key.
- * @param expectedKeyId the key ID that the caller expects.
- */
- public final CheckResult check(PGPPublicKey key, long expectedKeyId) {
List<String> problems = new ArrayList<>();
- if (key.getKeyID() != expectedKeyId) {
- problems.add(
- "Public key does not match ID " + keyIdToString(expectedKeyId));
- }
if (key.isRevoked()) {
// TODO(dborowitz): isRevoked is overeager:
// http://www.bouncycastle.org/jira/browse/BJB-45
@@ -58,7 +42,7 @@
problems.add("Key is expired");
}
}
- checkCustom(key, expectedKeyId, problems);
+ checkCustom(key, problems);
return new CheckResult(problems);
}
@@ -68,11 +52,9 @@
* Default implementation does nothing, but may be overridden by subclasses.
*
* @param key the public key.
- * @param expectedKeyId the key ID that the caller expects.
* @param problems list to which any problems should be added.
*/
- public void checkCustom(PGPPublicKey key, long expectedKeyId,
- List<String> problems) {
+ public void checkCustom(PGPPublicKey key, List<String> problems) {
// Default implementation does nothing.
}
}
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 9dc3e69..687729e 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
@@ -147,7 +147,7 @@
"Signature not valid with public key: " + keyToString(k));
continue;
}
- CheckResult result = publicKeyChecker.check(k, sig.getKeyID());
+ CheckResult result = publicKeyChecker.check(k);
if (result.isOk()) {
return;
}
diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java
index ebc3e58..0dbef36 100644
--- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java
+++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java
@@ -37,16 +37,6 @@
}
@Test
- public void wrongKeyId() throws Exception {
- TestKey k = TestKey.key1();
- long badId = k.getKeyId() + 1;
- CheckResult result = checker.check(k.getPublicKey(), badId);
- assertEquals(
- Arrays.asList("Public key does not match ID 46328A8D"),
- result.getProblems());
- }
-
- @Test
public void keyExpiringInFuture() throws Exception {
assertProblems(TestKey.key2());
}
@@ -62,7 +52,7 @@
}
private void assertProblems(TestKey tk, String... expected) throws Exception {
- CheckResult result = checker.check(tk.getPublicKey(), tk.getKeyId());
+ CheckResult result = checker.check(tk.getPublicKey());
assertEquals(Arrays.asList(expected), result.getProblems());
}
}
