PushCertificateChecker: Optionally skip nonce check This needs to be checked by the server during push to prevent replay attacks, but when checking certificates after the fact we can blindly trust the nonce. Change-Id: I6256cd2409f9e90dfd2f2dd8d4c1c3c55070a9e9
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPushCertificateChecker.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPushCertificateChecker.java index 2755fbf..fbc3d44 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPushCertificateChecker.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/GerritPushCertificateChecker.java
@@ -26,7 +26,8 @@ public class GerritPushCertificateChecker extends PushCertificateChecker { public interface Factory { - GerritPushCertificateChecker create(IdentifiedUser expectedUser); + GerritPushCertificateChecker create(IdentifiedUser expectedUser, + boolean checkNonce); } private final GitRepositoryManager repoManager; @@ -37,8 +38,9 @@ GerritPublicKeyChecker.Factory keyCheckerFactory, GitRepositoryManager repoManager, AllUsersName allUsers, - @Assisted IdentifiedUser expectedUser) { - super(keyCheckerFactory.create(expectedUser)); + @Assisted IdentifiedUser expectedUser, + @Assisted boolean checkNonce) { + super(keyCheckerFactory.create(expectedUser), checkNonce); this.repoManager = repoManager; this.allUsers = allUsers; }
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 8e50cfe..d7c00df 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
@@ -49,9 +49,12 @@ LoggerFactory.getLogger(PushCertificateChecker.class); private final PublicKeyChecker publicKeyChecker; + private final boolean checkNonce; - protected PushCertificateChecker(PublicKeyChecker publicKeyChecker) { + protected PushCertificateChecker(PublicKeyChecker publicKeyChecker, + boolean checkNonce) { this.publicKeyChecker = publicKeyChecker; + this.checkNonce = checkNonce; } /** @@ -60,7 +63,7 @@ * @return result of the check. */ public final CheckResult check(PushCertificate cert) { - if (cert.getNonceStatus() != NonceStatus.OK) { + if (checkNonce && cert.getNonceStatus() != NonceStatus.OK) { return CheckResult.bad("Invalid nonce"); } List<CheckResult> results = new ArrayList<>(2);
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/SignedPushPreReceiveHook.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/SignedPushPreReceiveHook.java index 02794a8..b10b3f3 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/SignedPushPreReceiveHook.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/SignedPushPreReceiveHook.java
@@ -54,7 +54,7 @@ if (cert == null) { return; } - CheckResult result = checkerFactory.create(user.get()) + CheckResult result = checkerFactory.create(user.get(), true) .check(cert); if (!isAllowed(result, commands)) { for (String problem : result.getProblems()) {
diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PushCertificateCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PushCertificateCheckerTest.java index d30057b..82d183b 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PushCertificateCheckerTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PushCertificateCheckerTest.java
@@ -66,8 +66,11 @@ signedPushConfig = new SignedPushConfig(); signedPushConfig.setCertNonceSeed("sekret"); signedPushConfig.setCertNonceSlopLimit(60 * 24); + checker = newChecker(true); + } - checker = new PushCertificateChecker(new PublicKeyChecker()) { + private PushCertificateChecker newChecker(boolean checkNonce) { + return new PushCertificateChecker(new PublicKeyChecker(), checkNonce) { @Override protected Repository getRepository() { return tr.getRepository(); @@ -93,6 +96,13 @@ } @Test + public void invalidNonceNotChecked() throws Exception { + checker = newChecker(false); + PushCertificate cert = newSignedCert("invalid-nonce", TestKeys.key1()); + assertProblems(cert); + } + + @Test public void missingKey() throws Exception { TestKey key2 = TestKeys.key2(); PushCertificate cert = newSignedCert(validNonce(), key2);