From 9db0d8906d7baeded5de66090faf9c11729b61bb Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 20 Oct 2015 15:44:34 -0400 Subject: [PATCH 1/7] TestKeys: Use descriptive names Make tests slightly more readable by describing the necessary keys. (Note that this does not apply to TestTrustKeys, which have more opaque names that refer to diagrams in the class Javadoc; readers are expected to have the diagrams in front of them.) Change-Id: I30642d37d2563084c0a372549bf41668f280c433 --- .../acceptance/api/accounts/AccountIT.java | 23 ++++++------ .../acceptance/api/revision/RevisionIT.java | 2 +- .../gpg/GerritPublicKeyCheckerTest.java | 12 +++---- .../gerrit/gpg/PublicKeyCheckerTest.java | 17 +++++---- .../google/gerrit/gpg/PublicKeyStoreTest.java | 36 ++++++++++--------- .../gpg/PushCertificateCheckerTest.java | 21 ++++++----- .../google/gerrit/gpg/testutil/TestKeys.java | 16 ++++----- .../change/AbstractQueryChangesTest.java | 1 - 8 files changed, 70 insertions(+), 58 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java index 45e5c22569..39296d0f43 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -19,6 +19,10 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assert_; import static com.google.gerrit.gpg.PublicKeyStore.REFS_GPG_KEYS; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; +import static com.google.gerrit.gpg.testutil.TestKeys.allValidKeys; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithExpiration; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithSecondUserId; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithoutExpiration; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.Function; @@ -38,7 +42,6 @@ import com.google.gerrit.gpg.Fingerprint; import com.google.gerrit.gpg.PublicKeyStore; import com.google.gerrit.gpg.server.GpgKeys; import com.google.gerrit.gpg.testutil.TestKey; -import com.google.gerrit.gpg.testutil.TestKeys; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountExternalId; import com.google.gerrit.server.config.AllUsersName; @@ -204,7 +207,7 @@ public class AccountIT extends AbstractDaemonTest { @Test public void addGpgKey() throws Exception { - TestKey key = TestKeys.key1(); + TestKey key = validKeyWithoutExpiration(); String id = key.getKeyIdString(); addExternalIdEmail(admin, "test1@example.com"); @@ -220,7 +223,7 @@ public class AccountIT extends AbstractDaemonTest { @Test public void reAddExistingGpgKey() throws Exception { addExternalIdEmail(admin, "test5@example.com"); - TestKey key = TestKeys.key5(); + TestKey key = validKeyWithSecondUserId(); String id = key.getKeyIdString(); PGPPublicKey pk = key.getPublicKey(); @@ -243,7 +246,7 @@ public class AccountIT extends AbstractDaemonTest { db.accountExternalIds().insert(Collections.singleton(extId)); - TestKey key = TestKeys.key5(); + TestKey key = validKeyWithSecondUserId(); addGpgKey(key.getPublicKeyArmored()); setApiUser(user); @@ -254,7 +257,7 @@ public class AccountIT extends AbstractDaemonTest { @Test public void listGpgKeys() throws Exception { - List keys = TestKeys.allValidKeys(); + List keys = allValidKeys(); List toAdd = new ArrayList<>(keys.size()); for (TestKey key : keys) { addExternalIdEmail(admin, @@ -267,7 +270,7 @@ public class AccountIT extends AbstractDaemonTest { @Test public void deleteGpgKey() throws Exception { - TestKey key = TestKeys.key1(); + TestKey key = validKeyWithoutExpiration(); String id = key.getKeyIdString(); addExternalIdEmail(admin, "test1@example.com"); addGpgKey(key.getPublicKeyArmored()); @@ -283,13 +286,13 @@ public class AccountIT extends AbstractDaemonTest { @Test public void addAndRemoveGpgKeys() throws Exception { - for (TestKey key : TestKeys.allValidKeys()) { + for (TestKey key : allValidKeys()) { addExternalIdEmail(admin, PushCertificateIdent.parse(key.getFirstUserId()).getEmailAddress()); } - TestKey key1 = TestKeys.key1(); - TestKey key2 = TestKeys.key2(); - TestKey key5 = TestKeys.key5(); + TestKey key1 = validKeyWithoutExpiration(); + TestKey key2 = validKeyWithExpiration(); + TestKey key5 = validKeyWithSecondUserId(); Map infos = gApi.accounts().self().putGpgKeys( ImmutableList.of( diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java index 7ecf1f3e88..3e970e434b 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -18,8 +18,8 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.acceptance.PushOneCommit.FILE_CONTENT; import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME; import static com.google.gerrit.acceptance.PushOneCommit.PATCH; -import static org.eclipse.jgit.lib.Constants.HEAD; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.eclipse.jgit.lib.Constants.HEAD; import com.google.common.base.Predicate; import com.google.common.collect.Iterables; diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java index 05617ff1a2..ce0c912646 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.gpg.GerritPublicKeyChecker.toExtIdKey; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithSecondUserId; import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyA; import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyB; import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyC; @@ -32,7 +33,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; import com.google.gerrit.extensions.common.GpgKeyInfo.Status; import com.google.gerrit.gpg.testutil.TestKey; -import com.google.gerrit.gpg.testutil.TestKeys; import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountExternalId; @@ -171,7 +171,7 @@ public class GerritPublicKeyCheckerTest { @Test public void defaultGpgCertificationMatchesEmail() throws Exception { - TestKey key = TestKeys.key5(); + TestKey key = validKeyWithSecondUserId(); GerritPublicKeyChecker checker = checkerFactory.create(user); assertProblems( checker.check(key.getPublicKey()), Status.BAD, @@ -190,7 +190,7 @@ public class GerritPublicKeyCheckerTest { addExternalId("test", "test", "nobody@example.com"); GerritPublicKeyChecker checker = checkerFactory.create(user); assertProblems( - checker.check(TestKeys.key5().getPublicKey()), Status.BAD, + checker.check(validKeyWithSecondUserId().getPublicKey()), Status.BAD, "Key must contain a valid certification for one of the following " + "identities:\n" + " gerrit:user\n" @@ -203,7 +203,7 @@ public class GerritPublicKeyCheckerTest { public void manualCertificationMatchesExternalId() throws Exception { addExternalId("foo", "myId", null); GerritPublicKeyChecker checker = checkerFactory.create(user); - assertNoProblems(checker.check(TestKeys.key5().getPublicKey())); + assertNoProblems(checker.check(validKeyWithSecondUserId().getPublicKey())); } @Test @@ -211,7 +211,7 @@ public class GerritPublicKeyCheckerTest { addExternalId("foo", "otherId", null); GerritPublicKeyChecker checker = checkerFactory.create(user); assertProblems( - checker.check(TestKeys.key5().getPublicKey()), Status.BAD, + checker.check(validKeyWithSecondUserId().getPublicKey()), Status.BAD, "Key must contain a valid certification for one of the following " + "identities:\n" + " foo:otherId\n" @@ -225,7 +225,7 @@ public class GerritPublicKeyCheckerTest { db.accountExternalIds().byAccount(user.getAccountId())); reloadUser(); - TestKey key = TestKeys.key5(); + TestKey key = validKeyWithSecondUserId(); GerritPublicKeyChecker checker = checkerFactory.create(user); assertProblems( checker.check(key.getPublicKey()), Status.BAD, 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 d1f5731781..5c47cfe571 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 @@ -15,6 +15,10 @@ package com.google.gerrit.gpg; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; +import static com.google.gerrit.gpg.testutil.TestKeys.expiredKey; +import static com.google.gerrit.gpg.testutil.TestKeys.selfRevokedKey; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithExpiration; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithoutExpiration; import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyA; import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyB; import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyC; @@ -28,7 +32,6 @@ import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyJ; import static org.junit.Assert.assertEquals; import com.google.gerrit.gpg.testutil.TestKey; -import com.google.gerrit.gpg.testutil.TestKeys; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; @@ -72,22 +75,22 @@ public class PublicKeyCheckerTest { @Test public void validKey() throws Exception { - assertProblems(TestKeys.key1()); + assertProblems(validKeyWithoutExpiration()); } @Test public void keyExpiringInFuture() throws Exception { - assertProblems(TestKeys.key2()); + assertProblems(validKeyWithExpiration()); } @Test - public void expiredKey() throws Exception { - assertProblems(TestKeys.key3(), "Key is expired"); + public void expiredKeyIsExpired() throws Exception { + assertProblems(expiredKey(), "Key is expired"); } @Test - public void selfRevokedKey() throws Exception { - assertProblems(TestKeys.key4(), "Key is revoked"); + public void selfRevokedKeyIsRevoked() throws Exception { + assertProblems(selfRevokedKey(), "Key is revoked"); } // Test keys specific to this test are at the bottom of this class. Each test diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java index b1e64042a0..9c0a9084e4 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyStoreTest.java @@ -18,13 +18,15 @@ import static com.google.gerrit.gpg.PublicKeyStore.REFS_GPG_KEYS; import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString; import static com.google.gerrit.gpg.PublicKeyStore.keyObjectId; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithExpiration; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithSecondUserId; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithoutExpiration; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import com.google.gerrit.gpg.testutil.TestKey; -import com.google.gerrit.gpg.testutil.TestKeys; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPPublicKeyRing; @@ -61,13 +63,13 @@ public class PublicKeyStoreTest { @Test public void testKeyIdToString() throws Exception { - PGPPublicKey key = TestKeys.key1().getPublicKey(); + PGPPublicKey key = validKeyWithoutExpiration().getPublicKey(); assertEquals("46328A8C", keyIdToString(key.getKeyID())); } @Test public void testKeyToString() throws Exception { - PGPPublicKey key = TestKeys.key1().getPublicKey(); + PGPPublicKey key = validKeyWithoutExpiration().getPublicKey(); assertEquals("46328A8C Testuser One " + " (04AE A7ED 2F82 1133 E5B1 28D1 ED06 25DC 4632 8A8C)", keyToString(key)); @@ -75,7 +77,7 @@ public class PublicKeyStoreTest { @Test public void testKeyObjectId() throws Exception { - PGPPublicKey key = TestKeys.key1().getPublicKey(); + PGPPublicKey key = validKeyWithoutExpiration().getPublicKey(); String objId = keyObjectId(key.getKeyID()).name(); assertEquals("ed0625dc46328a8c000000000000000000000000", objId); assertEquals(keyIdToString(key.getKeyID()).toLowerCase(), @@ -84,13 +86,13 @@ public class PublicKeyStoreTest { @Test public void testGet() throws Exception { - TestKey key1 = TestKeys.key1(); + TestKey key1 = validKeyWithoutExpiration(); tr.branch(REFS_GPG_KEYS) .commit() .add(keyObjectId(key1.getKeyId()).name(), key1.getPublicKeyArmored()) .create(); - TestKey key2 = TestKeys.key2(); + TestKey key2 = validKeyWithExpiration(); tr.branch(REFS_GPG_KEYS) .commit() .add(keyObjectId(key2.getKeyId()).name(), @@ -103,8 +105,8 @@ public class PublicKeyStoreTest { @Test public void testGetMultiple() throws Exception { - TestKey key1 = TestKeys.key1(); - TestKey key2 = TestKeys.key2(); + TestKey key1 = validKeyWithoutExpiration(); + TestKey key2 = validKeyWithExpiration(); tr.branch(REFS_GPG_KEYS) .commit() .add(keyObjectId(key1.getKeyId()).name(), @@ -117,8 +119,8 @@ public class PublicKeyStoreTest { @Test public void save() throws Exception { - TestKey key1 = TestKeys.key1(); - TestKey key2 = TestKeys.key2(); + TestKey key1 = validKeyWithoutExpiration(); + TestKey key2 = validKeyWithExpiration(); store.add(key1.getPublicKeyRing()); store.add(key2.getPublicKeyRing()); @@ -130,8 +132,8 @@ public class PublicKeyStoreTest { @Test public void saveAppendsToExistingList() throws Exception { - TestKey key1 = TestKeys.key1(); - TestKey key2 = TestKeys.key2(); + TestKey key1 = validKeyWithoutExpiration(); + TestKey key2 = validKeyWithExpiration(); tr.branch(REFS_GPG_KEYS) .commit() // Mismatched for this key ID, but we can still read it out. @@ -161,7 +163,7 @@ public class PublicKeyStoreTest { @Test public void updateExisting() throws Exception { - TestKey key5 = TestKeys.key5(); + TestKey key5 = validKeyWithSecondUserId(); PGPPublicKeyRing keyRing = key5.getPublicKeyRing(); PGPPublicKey key = keyRing.getPublicKey(); store.add(keyRing); @@ -185,7 +187,7 @@ public class PublicKeyStoreTest { @Test public void remove() throws Exception { - TestKey key1 = TestKeys.key1(); + TestKey key1 = validKeyWithoutExpiration(); store.add(key1.getPublicKeyRing()); assertEquals(RefUpdate.Result.NEW, store.save(newCommitBuilder())); assertKeys(key1.getKeyId(), key1); @@ -197,11 +199,11 @@ public class PublicKeyStoreTest { @Test public void removeNonexisting() throws Exception { - TestKey key1 = TestKeys.key1(); + TestKey key1 = validKeyWithoutExpiration(); store.add(key1.getPublicKeyRing()); assertEquals(RefUpdate.Result.NEW, store.save(newCommitBuilder())); - TestKey key2 = TestKeys.key2(); + TestKey key2 = validKeyWithExpiration(); store.remove(key2.getPublicKey().getFingerprint()); assertEquals(RefUpdate.Result.NO_CHANGE, store.save(newCommitBuilder())); assertKeys(key1.getKeyId(), key1); @@ -209,7 +211,7 @@ public class PublicKeyStoreTest { @Test public void addThenRemove() throws Exception { - TestKey key1 = TestKeys.key1(); + TestKey key1 = validKeyWithoutExpiration(); store.add(key1.getPublicKeyRing()); store.remove(key1.getPublicKey().getFingerprint()); assertEquals(RefUpdate.Result.NO_CHANGE, store.save(newCommitBuilder())); 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 9bee0ef288..1cc0132e79 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 @@ -17,11 +17,13 @@ package com.google.gerrit.gpg; import static com.google.gerrit.gpg.PublicKeyStore.REFS_GPG_KEYS; import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; +import static com.google.gerrit.gpg.testutil.TestKeys.expiredKey; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithExpiration; +import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithoutExpiration; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; import com.google.gerrit.gpg.testutil.TestKey; -import com.google.gerrit.gpg.testutil.TestKeys; import org.bouncycastle.bcpg.ArmoredOutputStream; import org.bouncycastle.bcpg.BCPGOutputStream; @@ -53,8 +55,8 @@ public class PushCertificateCheckerTest { @Before public void setUp() throws Exception { - TestKey key1 = TestKeys.key1(); - TestKey key3 = TestKeys.key3(); + TestKey key1 = validKeyWithoutExpiration(); + TestKey key3 = expiredKey(); tr = new TestRepository<>(new InMemoryRepository( new DfsRepositoryDescription("repo"))); tr.branch(REFS_GPG_KEYS).commit() @@ -85,26 +87,29 @@ public class PushCertificateCheckerTest { @Test public void validCert() throws Exception { - PushCertificate cert = newSignedCert(validNonce(), TestKeys.key1()); + PushCertificate cert = + newSignedCert(validNonce(), validKeyWithoutExpiration()); assertProblems(cert); } @Test public void invalidNonce() throws Exception { - PushCertificate cert = newSignedCert("invalid-nonce", TestKeys.key1()); + PushCertificate cert = + newSignedCert("invalid-nonce", validKeyWithoutExpiration()); assertProblems(cert, "Invalid nonce"); } @Test public void invalidNonceNotChecked() throws Exception { checker = newChecker(false); - PushCertificate cert = newSignedCert("invalid-nonce", TestKeys.key1()); + PushCertificate cert = + newSignedCert("invalid-nonce", validKeyWithoutExpiration()); assertProblems(cert); } @Test public void missingKey() throws Exception { - TestKey key2 = TestKeys.key2(); + TestKey key2 = validKeyWithExpiration(); PushCertificate cert = newSignedCert(validNonce(), key2); assertProblems(cert, "No public keys found for key ID " + keyIdToString(key2.getKeyId())); @@ -112,7 +117,7 @@ public class PushCertificateCheckerTest { @Test public void invalidKey() throws Exception { - TestKey key3 = TestKeys.key3(); + TestKey key3 = expiredKey(); PushCertificate cert = newSignedCert(validNonce(), key3); assertProblems(cert, "Invalid public key " + keyToString(key3.getPublicKey()) diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java index 3abe689585..3b3ab88a15 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java @@ -20,9 +20,9 @@ import com.google.common.collect.ImmutableList; public class TestKeys { public static ImmutableList allValidKeys() { return ImmutableList.of( - TestKeys.key1(), - TestKeys.key2(), - TestKeys.key5()); + validKeyWithoutExpiration(), + validKeyWithExpiration(), + validKeyWithSecondUserId()); } /** @@ -35,7 +35,7 @@ public class TestKeys { * sub 2048R/F0AF69C0 2015-07-08 * */ - public static TestKey key1() { + public static TestKey validKeyWithoutExpiration() { return new TestKey("-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + "Version: GnuPG v1\n" + "\n" @@ -135,7 +135,7 @@ public class TestKeys { * sub 2048R/46D4F204 2015-07-08 [expires: 2065-06-25] * */ - public static final TestKey key2() { + public static final TestKey validKeyWithExpiration() { return new TestKey( "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + "Version: GnuPG v1\n" @@ -236,7 +236,7 @@ public class TestKeys { * uid Testuser Three <test3@example.com> * */ - public static final TestKey key3() { + public static final TestKey expiredKey() { return new TestKey( "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + "Version: GnuPG v1\n" @@ -337,7 +337,7 @@ public class TestKeys { * uid Testuser Four <test4@example.com> * */ - public static final TestKey key4() { + public static final TestKey selfRevokedKey() { return new TestKey( "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + "Version: GnuPG v1\n" @@ -445,7 +445,7 @@ public class TestKeys { * sub 2048R/C781A9E3 2015-07-30 * */ - public static TestKey key5() { + public static TestKey validKeyWithSecondUserId() { return new TestKey("-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + "Version: GnuPG v1\n" + "\n" diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 2f2f2348c4..1d79400002 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.query.change; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.TruthJUnit.assume; import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED; - import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.MILLISECONDS; From 2737821ae388e194acb9e1851da67ce5062d8b47 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 20 Oct 2015 13:09:07 -0400 Subject: [PATCH 2/7] PublicKeyChecker: Convert to a builder pattern Instead of overloading the constructor and check methods with optional arguments, expose optional arguments via setters. We are about to add another one, and this was getting unwieldy. We still use a Factory to create GerritPublicKeyChecker, which parses the config once in a @Singleton and subsequently configures all checkers with that config. Since the checker is initially created when a store might not be available (because the repo hasn't been opened), we need to separate the trusted key config setter from the store setter, and check state at use time. Change-Id: Ide6bc76a8180f91caa6f03f1a0eb25ec6d20ba45 --- .../com/google/gerrit/gpg/Fingerprint.java | 11 +++ .../gerrit/gpg/GerritPublicKeyChecker.java | 72 ++++++-------- .../gpg/GerritPushCertificateChecker.java | 2 +- .../google/gerrit/gpg/PublicKeyChecker.java | 98 ++++++++++--------- .../gerrit/gpg/PushCertificateChecker.java | 2 +- .../com/google/gerrit/gpg/server/GpgKeys.java | 6 +- .../google/gerrit/gpg/server/PostGpgKeys.java | 12 ++- .../gpg/GerritPublicKeyCheckerTest.java | 76 +++++++++----- .../gerrit/gpg/PublicKeyCheckerTest.java | 17 ++-- 9 files changed, 168 insertions(+), 128 deletions(-) diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java index 6fd8bace11..54c12c6433 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java @@ -19,6 +19,9 @@ import static com.google.common.base.Preconditions.checkArgument; import org.eclipse.jgit.util.NB; import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; public class Fingerprint { private final byte[] fp; @@ -33,6 +36,14 @@ public class Fingerprint { NB.decodeUInt16(fp, 16), NB.decodeUInt16(fp, 18)); } + public static Map byId(Iterable fps) { + Map result = new HashMap<>(); + for (Fingerprint fp : fps) { + result.put(fp.getId(), fp); + } + return Collections.unmodifiableMap(result); + } + private static byte[] checkLength(byte[] fp) { checkArgument(fp.length == 20, "fingerprint must be 20 bytes, got %s", fp.length); 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 d942c7576c..88ebc8dcf1 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 @@ -14,14 +14,14 @@ package com.google.gerrit.gpg; -import static com.google.common.base.Preconditions.checkNotNull; import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString; import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_GPGKEY; import com.google.common.base.CharMatcher; import com.google.common.base.MoreObjects; import com.google.common.collect.FluentIterable; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.google.common.collect.Ordering; import com.google.common.io.BaseEncoding; import com.google.gerrit.common.PageLinks; @@ -44,11 +44,10 @@ import org.eclipse.jgit.transport.PushCertificateIdent; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; -import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -61,18 +60,13 @@ public class GerritPublicKeyChecker extends PublicKeyChecker { private static final Logger log = LoggerFactory.getLogger(GerritPublicKeyChecker.class); - private final Provider db; - private final String webUrl; - private final IdentifiedUser.GenericFactory userFactory; - private final IdentifiedUser expectedUser; - @Singleton public static class Factory { private final Provider db; private final String webUrl; private final IdentifiedUser.GenericFactory userFactory; private final int maxTrustDepth; - private final ImmutableList trusted; + private final ImmutableMap trusted; @Inject Factory(@GerritServerConfig Config cfg, @@ -86,52 +80,50 @@ public class GerritPublicKeyChecker extends PublicKeyChecker { String[] strs = cfg.getStringList("receive", null, "trustedKey"); if (strs.length != 0) { - List fps = new ArrayList<>(strs.length); + Map fps = + Maps.newHashMapWithExpectedSize(strs.length); for (String str : strs) { str = CharMatcher.WHITESPACE.removeFrom(str).toUpperCase(); - fps.add(new Fingerprint(BaseEncoding.base16().decode(str))); + Fingerprint fp = new Fingerprint(BaseEncoding.base16().decode(str)); + fps.put(fp.getId(), fp); } - trusted = ImmutableList.copyOf(fps); + trusted = ImmutableMap.copyOf(fps); } else { trusted = null; } } - /** - * Create a checker that can check arbitrary public keys. - *

- * Each key is checked against the set of identities in the database - * belonging to the same user as the key. - * - * @return a new checker. - */ public GerritPublicKeyChecker create() { - return new GerritPublicKeyChecker(this, null); - } - - /** - * Create a checker for checking a single public key against a known user. - *

- * The top-level key passed to {@link #check(PGPPublicKey, PublicKeyStore)} - * must belong to the given user. (Other keys checked in the course of - * verifying the web of trust are checked against the set of identities in - * the database belonging to the same user as the key.) - * - * @param expectedUser the user - * @return a new checker. - */ - public GerritPublicKeyChecker create(IdentifiedUser expectedUser) { - checkNotNull(expectedUser); - return new GerritPublicKeyChecker(this, expectedUser); + return new GerritPublicKeyChecker(this); } } - private GerritPublicKeyChecker(Factory factory, IdentifiedUser expectedUser) { - super(factory.maxTrustDepth, factory.trusted); + private final Provider db; + private final String webUrl; + private final IdentifiedUser.GenericFactory userFactory; + + private IdentifiedUser expectedUser; + + private GerritPublicKeyChecker(Factory factory) { this.db = factory.db; this.webUrl = factory.webUrl; this.userFactory = factory.userFactory; + if (factory.trusted != null) { + enableTrust(factory.maxTrustDepth, factory.trusted); + } + } + + /** + * Set the expected user for this checker. + *

+ * If set, the top-level key passed to {@link #check(PGPPublicKey)} must + * belong to the given user. (Other keys checked in the course of verifying + * the web of trust are checked against the set of identities in the database + * belonging to the same user as the key.) + */ + public GerritPublicKeyChecker setExpectedUser(IdentifiedUser expectedUser) { this.expectedUser = expectedUser; + return this; } @Override 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 fbc3d4472f..6491ef144e 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 @@ -40,7 +40,7 @@ public class GerritPushCertificateChecker extends PushCertificateChecker { AllUsersName allUsers, @Assisted IdentifiedUser expectedUser, @Assisted boolean checkNonce) { - super(keyCheckerFactory.create(expectedUser), checkNonce); + super(keyCheckerFactory.create().setExpectedUser(expectedUser), checkNonce); this.repoManager = repoManager; this.allUsers = allUsers; } 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 725a6e1036..d19264a29c 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 @@ -32,8 +32,6 @@ import org.bouncycastle.openpgp.PGPSignature; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -45,66 +43,71 @@ public class PublicKeyChecker { // https://tools.ietf.org/html/rfc4880#section-5.2.3.13 private static final int COMPLETE_TRUST = 120; - private final Map trusted; - private final int maxTrustDepth; - - /** Create a new checker that does not check the web of trust. */ - public PublicKeyChecker() { - this(0, null); - } + private PublicKeyStore store; + private Map trusted; + private int maxTrustDepth; /** + * Enable web-of-trust checks. + *

+ * If enabled, a store must be set with {@link #setStore(PublicKeyStore)}. + * (These methods are separate since the store is a closeable resource that + * may not be available when reading trusted keys from a config.) + * * @param maxTrustDepth maximum depth to search while looking for a trusted * key. - * @param trusted ultimately trusted key fingerprints; may not be empty. If - * null, disable web-of-trust checks. + * @param trusted ultimately trusted key fingerprints, keyed by fingerprint; + * may not be empty. To construct a map, see {@link + * Fingerprint#byId(Iterable)}. + * @return a reference to this object. */ - public PublicKeyChecker(int maxTrustDepth, Collection trusted) { - if (trusted != null) { - if (maxTrustDepth <= 0) { + public PublicKeyChecker enableTrust(int maxTrustDepth, + Map trusted) { + if (maxTrustDepth <= 0) { + throw new IllegalArgumentException( + "maxTrustDepth must be positive, got: " + maxTrustDepth); + } + if (trusted == null || trusted.isEmpty()) { throw new IllegalArgumentException( - "maxTrustDepth must be positive, got: " + maxTrustDepth); - } - if (trusted.isEmpty()) { - throw new IllegalArgumentException("at least one trusted key required"); - } - this.trusted = new HashMap<>(); - for (Fingerprint fp : trusted) { - this.trusted.put(fp.getId(), fp); - } - } else { - this.trusted = null; + "at least one trusted key is required"); } this.maxTrustDepth = maxTrustDepth; + this.trusted = trusted; + return this; + } + + /** Disable web-of-trust checks. */ + public PublicKeyChecker disableTrust() { + store = null; + trusted = null; + return this; } /** - * Check a public key, including its web of trust. - * - * @param key the public key. - * @param store a store to read public keys from for trust checks. If this - * store is not configured for web-of-trust checks, this argument is - * ignored. - * @return the result of the check. + * Set the public key store for web-of-trust checks. + *

+ * If set, {@link #enableTrust(int, Map)} must also be called. */ - public final CheckResult check(PGPPublicKey key, PublicKeyStore store) { - if (trusted == null) { - return check(key); - } else if (store == null) { - throw new IllegalArgumentException( - "PublicKeyStore required for web of trust checks"); + public PublicKeyChecker setStore(PublicKeyStore store) { + if (store == null) { + throw new IllegalArgumentException("PublicKeyStore is required"); } - return check(key, store, 0, true, new HashSet()); + this.store = store; + return this; } /** - * Check only a public key, not including its web of trust. + * Check a public key. * * @param key the public key. * @return the result of the check. */ public final CheckResult check(PGPPublicKey key) { - return check(key, null, 0, false, null); + if (store == null && trusted != null) { + throw new IllegalStateException("PublicKeyStore is required"); + } + return check(key, 0, true, + trusted != null ? new HashSet() : null); } /** @@ -115,16 +118,16 @@ public class PublicKeyChecker { * * @param key the public key. * @param depth the depth from the initial key passed to {@link #check( - * PGPPublicKey, PublicKeyStore)}: 0 if this was the initial key, up to a - * maximum of {@code maxTrustDepth}. + * PGPPublicKey)}: 0 if this was the initial key, up to a maximum of + * {@code maxTrustDepth}. * @return the result of the custom check. */ public CheckResult checkCustom(PGPPublicKey key, int depth) { return CheckResult.ok(); } - private CheckResult check(PGPPublicKey key, PublicKeyStore store, int depth, - boolean expand, Set seen) { + private CheckResult check(PGPPublicKey key, int depth, boolean expand, + Set seen) { CheckResult basicResult = checkBasic(key); CheckResult customResult = checkCustom(key, depth); CheckResult trustResult = checkWebOfTrust(key, store, depth, seen); @@ -181,7 +184,7 @@ public class PublicKeyChecker { private CheckResult checkWebOfTrust(PGPPublicKey key, PublicKeyStore store, int depth, Set seen) { - if (trusted == null || store == null) { + if (trusted == null) { // Trust checking not configured, server trusts all OK keys. return CheckResult.trusted(); } @@ -222,8 +225,7 @@ public class PublicKeyChecker { } String subpacketProblem = checkTrustSubpacket(sig, depth); if (subpacketProblem == null) { - CheckResult signerResult = - check(signer, store, depth + 1, false, seen); + CheckResult signerResult = check(signer, depth + 1, false, seen); if (signerResult.isTrusted()) { return CheckResult.trusted(); } 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 71068b8a5c..1236aa2f6d 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 @@ -202,7 +202,7 @@ public abstract class PushCertificateChecker { CheckResult.bad("Signature by " + keyIdToString(sig.getKeyID()) + " is not valid")); } - CheckResult result = publicKeyChecker.check(signer, store); + CheckResult result = publicKeyChecker.check(signer); if (!result.getProblems().isEmpty()) { StringBuilder err = new StringBuilder("Invalid public key ") .append(keyToString(signer)) diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java index 900bcaf077..63d4fe803c 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java @@ -163,7 +163,7 @@ public class GpgKeys implements found = true; GpgKeyInfo info = toJson( keyRing.getPublicKey(), - checkerFactory.create(rsrc.getUser()), + checkerFactory.create().setExpectedUser(rsrc.getUser()), store); keys.put(info.id, info); info.id = null; @@ -197,7 +197,7 @@ public class GpgKeys implements try (PublicKeyStore store = storeProvider.get()) { return toJson( rsrc.getKeyRing().getPublicKey(), - checkerFactory.create(rsrc.getUser()), + checkerFactory.create().setExpectedUser(rsrc.getUser()), store); } } @@ -261,7 +261,7 @@ public class GpgKeys implements static GpgKeyInfo toJson(PGPPublicKey key, PublicKeyChecker checker, PublicKeyStore store) throws IOException { - return toJson(key, checker.check(key, store)); + return toJson(key, checker.setStore(store).check(key)); } public static void toJson(GpgKeyInfo info, CheckResult checkResult) { diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java index 9b18ea2d73..0ed1f7b0e9 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java @@ -36,6 +36,7 @@ import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.gpg.CheckResult; import com.google.gerrit.gpg.Fingerprint; import com.google.gerrit.gpg.GerritPublicKeyChecker; +import com.google.gerrit.gpg.PublicKeyChecker; import com.google.gerrit.gpg.PublicKeyStore; import com.google.gerrit.gpg.server.PostGpgKeys.Input; import com.google.gerrit.reviewdb.client.AccountExternalId; @@ -193,7 +194,10 @@ public class PostGpgKeys implements RestModifyView { for (PGPPublicKeyRing keyRing : keyRings) { PGPPublicKey key = keyRing.getPublicKey(); // Don't check web of trust; admins can fill in certifications later. - CheckResult result = checkerFactory.create(rsrc.getUser()).check(key); + CheckResult result = checkerFactory.create() + .setExpectedUser(rsrc.getUser()) + .disableTrust() + .check(key); if (!result.isOk()) { throw new BadRequestException(String.format( "Problems with public key %s:\n%s", @@ -245,12 +249,14 @@ public class PostGpgKeys implements RestModifyView { throws IOException { // Unlike when storing keys, include web-of-trust checks when producing // result JSON, so the user at least knows of any issues. - GerritPublicKeyChecker checker = checkerFactory.create(user); + PublicKeyChecker checker = checkerFactory.create() + .setExpectedUser(user) + .setStore(store); Map infos = Maps.newHashMapWithExpectedSize(keys.size() + deleted.size()); for (PGPPublicKeyRing keyRing : keys) { PGPPublicKey key = keyRing.getPublicKey(); - CheckResult result = checker.check(key, store); + CheckResult result = checker.check(key); GpgKeyInfo info = GpgKeys.toJson(key, result); infos.put(info.id, info); info.id = null; diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java index ce0c912646..edc0559498 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java @@ -172,7 +172,9 @@ public class GerritPublicKeyCheckerTest { @Test public void defaultGpgCertificationMatchesEmail() throws Exception { TestKey key = validKeyWithSecondUserId(); - GerritPublicKeyChecker checker = checkerFactory.create(user); + PublicKeyChecker checker = checkerFactory.create() + .setExpectedUser(user) + .disableTrust(); assertProblems( checker.check(key.getPublicKey()), Status.BAD, "Key must contain a valid certification for one of the following " @@ -181,14 +183,18 @@ public class GerritPublicKeyCheckerTest { + " username:user"); addExternalId("test", "test", "test5@example.com"); - checker = checkerFactory.create(user); + checker = checkerFactory.create() + .setExpectedUser(user) + .disableTrust(); assertNoProblems(checker.check(key.getPublicKey())); } @Test public void defaultGpgCertificationDoesNotMatchEmail() throws Exception { addExternalId("test", "test", "nobody@example.com"); - GerritPublicKeyChecker checker = checkerFactory.create(user); + PublicKeyChecker checker = checkerFactory.create() + .setExpectedUser(user) + .disableTrust(); assertProblems( checker.check(validKeyWithSecondUserId().getPublicKey()), Status.BAD, "Key must contain a valid certification for one of the following " @@ -202,14 +208,18 @@ public class GerritPublicKeyCheckerTest { @Test public void manualCertificationMatchesExternalId() throws Exception { addExternalId("foo", "myId", null); - GerritPublicKeyChecker checker = checkerFactory.create(user); + PublicKeyChecker checker = checkerFactory.create() + .setExpectedUser(user) + .disableTrust(); assertNoProblems(checker.check(validKeyWithSecondUserId().getPublicKey())); } @Test public void manualCertificationDoesNotMatchExternalId() throws Exception { addExternalId("foo", "otherId", null); - GerritPublicKeyChecker checker = checkerFactory.create(user); + PublicKeyChecker checker = checkerFactory.create() + .setExpectedUser(user) + .disableTrust(); assertProblems( checker.check(validKeyWithSecondUserId().getPublicKey()), Status.BAD, "Key must contain a valid certification for one of the following " @@ -226,13 +236,16 @@ public class GerritPublicKeyCheckerTest { reloadUser(); TestKey key = validKeyWithSecondUserId(); - GerritPublicKeyChecker checker = checkerFactory.create(user); + PublicKeyChecker checker = checkerFactory.create() + .setExpectedUser(user) + .disableTrust(); assertProblems( checker.check(key.getPublicKey()), Status.BAD, "No identities found for user; check" + " http://test/#/settings/web-identities"); - checker = checkerFactory.create(); + checker = checkerFactory.create() + .disableTrust(); assertProblems( checker.check(key.getPublicKey()), Status.BAD, "Key is not associated with any users"); @@ -264,14 +277,18 @@ public class GerritPublicKeyCheckerTest { add(keyE(), addUser("userE")); // Checker for A, checking A. - GerritPublicKeyChecker checkerA = checkerFactory.create(user); - assertNoProblems(checkerA.check(keyA.getPublicKey(), store)); + PublicKeyChecker checkerA = checkerFactory.create() + .setExpectedUser(user) + .setStore(store); + assertNoProblems(checkerA.check(keyA.getPublicKey())); // Checker for B, checking B. Trust chain and IDs are correct, so the only // problem is with the key itself. - GerritPublicKeyChecker checkerB = checkerFactory.create(userB); + PublicKeyChecker checkerB = checkerFactory.create() + .setExpectedUser(userB) + .setStore(store); assertProblems( - checkerB.check(keyB.getPublicKey(), store), Status.BAD, + checkerB.check(keyB.getPublicKey()), Status.BAD, "Key is expired"); } @@ -294,9 +311,11 @@ public class GerritPublicKeyCheckerTest { add(keyE(), addUser("userE")); // Checker for A, checking B. - GerritPublicKeyChecker checkerA = checkerFactory.create(user); + PublicKeyChecker checkerA = checkerFactory.create() + .setExpectedUser(user) + .setStore(store); assertProblems( - checkerA.check(keyB.getPublicKey(), store), Status.BAD, + checkerA.check(keyB.getPublicKey()), Status.BAD, "Key is expired", "Key must contain a valid certification for one of the following" + " identities:\n" @@ -306,9 +325,11 @@ public class GerritPublicKeyCheckerTest { + " username:user"); // Checker for B, checking A. - GerritPublicKeyChecker checkerB = checkerFactory.create(userB); + PublicKeyChecker checkerB = checkerFactory.create() + .setExpectedUser(userB) + .setStore(store); assertProblems( - checkerB.check(keyA.getPublicKey(), store), Status.BAD, + checkerB.check(keyA.getPublicKey()), Status.BAD, "Key must contain a valid certification for one of the following" + " identities:\n" + " gerrit:userB\n" @@ -325,9 +346,11 @@ public class GerritPublicKeyCheckerTest { TestKey keyA = add(keyA(), user); TestKey keyB = add(keyB(), addUser("userB")); - GerritPublicKeyChecker checker = checkerFactory.create(user); + PublicKeyChecker checker = checkerFactory.create() + .setExpectedUser(user) + .setStore(store); assertProblems( - checker.check(keyA.getPublicKey(), store), Status.OK, + checker.check(keyA.getPublicKey()), Status.OK, "No path to a trusted key", "Certification by " + keyToString(keyB.getPublicKey()) + " is valid, but key is not trusted", @@ -352,15 +375,16 @@ public class GerritPublicKeyCheckerTest { // This checker can check any key, so the only problems come from issues // with the keys themselves, not having invalid user IDs. - GerritPublicKeyChecker checker = checkerFactory.create(); - assertNoProblems(checker.check(keyA.getPublicKey(), store)); + PublicKeyChecker checker = checkerFactory.create() + .setStore(store); + assertNoProblems(checker.check(keyA.getPublicKey())); assertProblems( - checker.check(keyB.getPublicKey(), store), Status.BAD, + checker.check(keyB.getPublicKey()), Status.BAD, "Key is expired"); - assertNoProblems(checker.check(keyC.getPublicKey(), store)); - assertNoProblems(checker.check(keyD.getPublicKey(), store)); + assertNoProblems(checker.check(keyC.getPublicKey())); + assertNoProblems(checker.check(keyD.getPublicKey())); assertProblems( - checker.check(keyE.getPublicKey(), store), Status.BAD, + checker.check(keyE.getPublicKey()), Status.BAD, "Key is expired", "No path to a trusted key"); } @@ -382,8 +406,10 @@ public class GerritPublicKeyCheckerTest { keyRingB = PGPPublicKeyRing.insertPublicKey(keyRingB, keyB); add(keyRingB, addUser("userB")); - GerritPublicKeyChecker checkerA = checkerFactory.create(user); - assertProblems(checkerA.check(keyA.getPublicKey(), store), Status.OK, + PublicKeyChecker checkerA = checkerFactory.create() + .setExpectedUser(user) + .setStore(store); + assertProblems(checkerA.check(keyA.getPublicKey()), Status.OK, "No path to a trusted key", "Certification by " + keyToString(keyB) + " is valid, but key is not trusted", 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 5c47cfe571..6f69b38021 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 @@ -44,9 +44,9 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; +import java.util.HashMap; +import java.util.Map; public class PublicKeyCheckerTest { @Rule @@ -175,11 +175,14 @@ public class PublicKeyCheckerTest { } private PublicKeyChecker newChecker(int maxTrustDepth, TestKey... trusted) { - List fps = new ArrayList<>(trusted.length); + Map fps = new HashMap<>(); for (TestKey k : trusted) { - fps.add(new Fingerprint(k.getPublicKey().getFingerprint())); + Fingerprint fp = new Fingerprint(k.getPublicKey().getFingerprint()); + fps.put(fp.getId(), fp); } - return new PublicKeyChecker(maxTrustDepth, fps); + return new PublicKeyChecker() + .enableTrust(maxTrustDepth, fps) + .setStore(store); } private TestKey add(TestKey k) { @@ -205,12 +208,12 @@ public class PublicKeyCheckerTest { private void assertProblems(PublicKeyChecker checker, TestKey k, String... expected) { - CheckResult result = checker.check(k.getPublicKey(), store); + CheckResult result = checker.check(k.getPublicKey()); assertEquals(Arrays.asList(expected), result.getProblems()); } private void assertProblems(TestKey tk, String... expected) throws Exception { - CheckResult result = new PublicKeyChecker().check(tk.getPublicKey(), store); + CheckResult result = new PublicKeyChecker().check(tk.getPublicKey()); assertEquals(Arrays.asList(expected), result.getProblems()); } From 674844faf6a725bffeeef3bbd011dd404eb6aac3 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 20 Oct 2015 13:20:23 -0400 Subject: [PATCH 3/7] PushCertificateChecker: Convert to a builder pattern Change-Id: Ic32ad26ea89b5cbe9c0c4bfa33ace23d09a35750 --- .../gerrit/gpg/GerritPushCertificateChecker.java | 8 +++----- .../google/gerrit/gpg/PushCertificateChecker.java | 12 +++++++++--- .../google/gerrit/gpg/SignedPushPreReceiveHook.java | 3 ++- .../com/google/gerrit/gpg/api/GpgApiAdapterImpl.java | 6 ++++-- .../gerrit/gpg/PushCertificateCheckerTest.java | 4 ++-- 5 files changed, 20 insertions(+), 13 deletions(-) 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 6491ef144e..30983ac9a0 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,8 +26,7 @@ import java.io.IOException; public class GerritPushCertificateChecker extends PushCertificateChecker { public interface Factory { - GerritPushCertificateChecker create(IdentifiedUser expectedUser, - boolean checkNonce); + GerritPushCertificateChecker create(IdentifiedUser expectedUser); } private final GitRepositoryManager repoManager; @@ -38,9 +37,8 @@ public class GerritPushCertificateChecker extends PushCertificateChecker { GerritPublicKeyChecker.Factory keyCheckerFactory, GitRepositoryManager repoManager, AllUsersName allUsers, - @Assisted IdentifiedUser expectedUser, - @Assisted boolean checkNonce) { - super(keyCheckerFactory.create().setExpectedUser(expectedUser), checkNonce); + @Assisted IdentifiedUser expectedUser) { + super(keyCheckerFactory.create().setExpectedUser(expectedUser)); 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 1236aa2f6d..87d778a8b0 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 @@ -67,12 +67,18 @@ public abstract class PushCertificateChecker { } private final PublicKeyChecker publicKeyChecker; - private final boolean checkNonce; - protected PushCertificateChecker(PublicKeyChecker publicKeyChecker, - boolean checkNonce) { + private boolean checkNonce; + + protected PushCertificateChecker(PublicKeyChecker publicKeyChecker) { this.publicKeyChecker = publicKeyChecker; + checkNonce = true; + } + + /** Set whether to check the status of the nonce; defaults to true. */ + public PushCertificateChecker setCheckNonce(boolean checkNonce) { this.checkNonce = checkNonce; + return this; } /** 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 875c77c853..035b083cb9 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,8 @@ public class SignedPushPreReceiveHook implements PreReceiveHook { if (cert == null) { return; } - CheckResult result = checkerFactory.create(user.get(), true) + CheckResult result = checkerFactory.create(user.get()) + .setCheckNonce(true) .check(cert) .getCheckResult(); if (!isAllowed(result, commands)) { diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java index 3821135468..e6720dbe24 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java @@ -95,8 +95,10 @@ public class GpgApiAdapterImpl implements GpgApiAdapter { IdentifiedUser expectedUser) throws GpgException { try { PushCertificate cert = PushCertificateParser.fromString(certStr); - PushCertificateChecker.Result result = - pushCertCheckerFactory.create(expectedUser, false).check(cert); + PushCertificateChecker.Result result = pushCertCheckerFactory + .create(expectedUser) + .setCheckNonce(false) + .check(cert); PushCertificateInfo info = new PushCertificateInfo(); info.certificate = certStr; info.key = GpgKeys.toJson(result.getPublicKey(), result.getCheckResult()); 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 1cc0132e79..dc1b5576fe 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 @@ -72,7 +72,7 @@ public class PushCertificateCheckerTest { } private PushCertificateChecker newChecker(boolean checkNonce) { - return new PushCertificateChecker(new PublicKeyChecker(), checkNonce) { + return new PushCertificateChecker(new PublicKeyChecker()) { @Override protected Repository getRepository() { return tr.getRepository(); @@ -82,7 +82,7 @@ public class PushCertificateCheckerTest { protected boolean shouldClose(Repository repo) { return false; } - }; + }.setCheckNonce(checkNonce); } @Test From 0f8480d2e5828fb2e6721d19431b966f5875cd72 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 20 Oct 2015 15:28:28 -0400 Subject: [PATCH 4/7] PublicKeyChecker: Fix revocation check (mostly) Bouncy Castle's isRevoked() method is so dumb as to be harmful: it only checks whether there is a revocation packet in the key, without doing any sort of validity checks on the packet. Improve the check by verifying the signature against the key that provided it, and for non-self-signatures, verifying that the signer is an authorized revocation key. This requires always passing in a store to any PublicKeyChecker. However, we still have the same initialization order problem as before, where a store is not always available at PublicKeyChecker creation, in particular in the PushCertificateChecker codepath. Argument precondition checks are the best we can do. Also add a convenience method to GerritPublicKeyChecker.Factory for the very common case (particularly in tests) where we do have both an expected user and a store. Change-Id: Id15714f0395a200fcb33fb199c57355b860187a3 --- .../com/google/gerrit/gpg/Fingerprint.java | 23 +- .../gerrit/gpg/GerritPublicKeyChecker.java | 8 + .../google/gerrit/gpg/PublicKeyChecker.java | 170 +++++++++++- .../com/google/gerrit/gpg/PublicKeyStore.java | 42 ++- .../gerrit/gpg/PushCertificateChecker.java | 4 +- .../com/google/gerrit/gpg/server/GpgKeys.java | 2 +- .../google/gerrit/gpg/server/PostGpgKeys.java | 7 +- .../gpg/GerritPublicKeyCheckerTest.java | 43 +-- .../gerrit/gpg/PublicKeyCheckerTest.java | 73 +++++- .../gpg/PushCertificateCheckerTest.java | 36 +-- .../google/gerrit/gpg/testutil/TestKeys.java | 244 +++++++++++++++++- 11 files changed, 572 insertions(+), 80 deletions(-) diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java index 54c12c6433..fa78f01561 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/Fingerprint.java @@ -36,6 +36,10 @@ public class Fingerprint { NB.decodeUInt16(fp, 16), NB.decodeUInt16(fp, 18)); } + public static long getId(byte[] fp) { + return NB.decodeInt64(fp, 12); + } + public static Map byId(Iterable fps) { Map result = new HashMap<>(); for (Fingerprint fp : fps) { @@ -65,6 +69,23 @@ public class Fingerprint { this.fp = checkLength(fp); } + /** + * Wrap a portion of a fingerprint byte array. + *

+ * Unlike {@link #Fingerprint(byte[])}, creates a new copy of the byte array. + * + * @param buf byte array to wrap; must have at least {@code off + 20} bytes. + * @param off offset in buf. + */ + public Fingerprint(byte[] buf, int off) { + int expected = 20 + off; + checkArgument(buf.length >= expected, + "fingerprint buffer must have at least %s bytes, got %s", + expected, buf.length); + this.fp = new byte[20]; + System.arraycopy(buf, off, fp, 0, 20); + } + public byte[] get() { return fp; } @@ -90,6 +111,6 @@ public class Fingerprint { } public long getId() { - return NB.decodeInt64(fp, 12); + return getId(fp); } } 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 88ebc8dcf1..c3c886f479 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 @@ -96,6 +96,14 @@ public class GerritPublicKeyChecker extends PublicKeyChecker { public GerritPublicKeyChecker create() { return new GerritPublicKeyChecker(this); } + + public GerritPublicKeyChecker create(IdentifiedUser expectedUser, + PublicKeyStore store) { + GerritPublicKeyChecker checker = new GerritPublicKeyChecker(this); + checker.setExpectedUser(expectedUser); + checker.setStore(store); + return checker; + } } private final Provider db; 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 d19264a29c..3bbad9baf0 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 @@ -19,19 +19,34 @@ import static com.google.gerrit.extensions.common.GpgKeyInfo.Status.OK; import static com.google.gerrit.extensions.common.GpgKeyInfo.Status.TRUSTED; import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; +import static org.bouncycastle.bcpg.SignatureSubpacketTags.REVOCATION_KEY; +import static org.bouncycastle.bcpg.SignatureSubpacketTags.REVOCATION_REASON; +import static org.bouncycastle.bcpg.sig.RevocationReasonTags.KEY_COMPROMISED; +import static org.bouncycastle.bcpg.sig.RevocationReasonTags.KEY_RETIRED; +import static org.bouncycastle.bcpg.sig.RevocationReasonTags.KEY_SUPERSEDED; +import static org.bouncycastle.bcpg.sig.RevocationReasonTags.NO_REASON; +import static org.bouncycastle.openpgp.PGPSignature.DIRECT_KEY; +import static org.bouncycastle.openpgp.PGPSignature.KEY_REVOCATION; import com.google.gerrit.extensions.common.GpgKeyInfo.Status; import org.bouncycastle.bcpg.SignatureSubpacket; import org.bouncycastle.bcpg.SignatureSubpacketTags; +import org.bouncycastle.bcpg.sig.RevocationKey; +import org.bouncycastle.bcpg.sig.RevocationReason; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPublicKey; +import org.bouncycastle.openpgp.PGPPublicKeyRing; import org.bouncycastle.openpgp.PGPPublicKeyRingCollection; import org.bouncycastle.openpgp.PGPSignature; +import org.bouncycastle.openpgp.operator.bc.BcPGPContentVerifierBuilderProvider; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -40,6 +55,9 @@ import java.util.Set; /** Checker for GPG public keys for use in a push certificate. */ public class PublicKeyChecker { + private static final Logger log = + LoggerFactory.getLogger(PublicKeyChecker.class); + // https://tools.ietf.org/html/rfc4880#section-5.2.3.13 private static final int COMPLETE_TRUST = 120; @@ -78,16 +96,11 @@ public class PublicKeyChecker { /** Disable web-of-trust checks. */ public PublicKeyChecker disableTrust() { - store = null; trusted = null; return this; } - /** - * Set the public key store for web-of-trust checks. - *

- * If set, {@link #enableTrust(int, Map)} must also be called. - */ + /** Set the public key store for reading keys referenced in signatures. */ public PublicKeyChecker setStore(PublicKeyStore store) { if (store == null) { throw new IllegalArgumentException("PublicKeyStore is required"); @@ -103,7 +116,7 @@ public class PublicKeyChecker { * @return the result of the check. */ public final CheckResult check(PGPPublicKey key) { - if (store == null && trusted != null) { + if (store == null) { throw new IllegalStateException("PublicKeyStore is required"); } return check(key, 0, true, @@ -165,11 +178,7 @@ public class PublicKeyChecker { private CheckResult checkBasic(PGPPublicKey key) { List problems = new ArrayList<>(2); - if (key.isRevoked()) { - // TODO(dborowitz): isRevoked is overeager: - // http://www.bouncycastle.org/jira/browse/BJB-45 - problems.add("Key is revoked"); - } + gatherRevocationProblems(key, problems); long validSecs = key.getValidSeconds(); if (validSecs != 0) { @@ -182,6 +191,143 @@ public class PublicKeyChecker { return CheckResult.create(problems); } + private void gatherRevocationProblems(PGPPublicKey key, List problems) { + try { + List revocations = new ArrayList<>(); + Map revokers = new HashMap<>(); + PGPSignature selfRevocation = scanRevocations(key, revocations, revokers); + if (selfRevocation != null) { + problems.add(reasonToString(getRevocationReason(selfRevocation))); + } else { + checkRevocations(key, revocations, revokers, problems); + } + } catch (PGPException | IOException e) { + problems.add("Error checking key revocation"); + } + } + + private PGPSignature scanRevocations(PGPPublicKey key, + List revocations, Map revokers) + throws PGPException { + @SuppressWarnings("unchecked") + Iterator allSigs = key.getSignatures(); + while (allSigs.hasNext()) { + PGPSignature sig = allSigs.next(); + switch (sig.getSignatureType()) { + case KEY_REVOCATION: + if (sig.getKeyID() == key.getKeyID()) { + sig.init(new BcPGPContentVerifierBuilderProvider(), key); + if (sig.verifyCertification(key)) { + return sig; + } + } else { + revocations.add(sig); + } + break; + case DIRECT_KEY: + RevocationKey r = getRevocationKey(key, sig); + if (r != null) { + revokers.put(Fingerprint.getId(r.getFingerprint()), r); + } + break; + } + } + return null; + } + + private RevocationKey getRevocationKey(PGPPublicKey key, PGPSignature sig) + throws PGPException { + if (sig.getKeyID() != key.getKeyID()) { + return null; + } + SignatureSubpacket sub = + sig.getHashedSubPackets().getSubpacket(REVOCATION_KEY); + if (sub == null) { + return null; + } + sig.init(new BcPGPContentVerifierBuilderProvider(), key); + if (!sig.verifyCertification(key)) { + return null; + } + + return new RevocationKey(sub.isCritical(), sub.getData()); + } + + private void checkRevocations(PGPPublicKey key, + List revocations, Map revokers, + List problems) + throws PGPException, IOException { + for (PGPSignature revocation : revocations) { + RevocationKey revoker = revokers.get(revocation.getKeyID()); + if (revoker == null) { + continue; // Not a designated revoker. + } + byte[] rfp = revoker.getFingerprint(); + PGPPublicKeyRing rkr = store.get(rfp); + if (rkr == null + || rkr.getPublicKey().getAlgorithm() != revoker.getAlgorithm()) { + // Revoker is authorized and there is a revocation signature by this + // revoker, but the key is not in the store so we can't verify the + // signature. + log.info("Key " + Fingerprint.toString(key.getFingerprint()) + + " is revoked by " + Fingerprint.toString(rfp) + + ", which is not in the store. Assuming revocation is valid."); + problems.add(reasonToString(getRevocationReason(revocation))); + continue; + } + revocation.init( + new BcPGPContentVerifierBuilderProvider(), rkr.getPublicKey()); + if (revocation.verifyCertification(key)) { + problems.add(reasonToString(getRevocationReason(revocation))); + } + } + } + + private static RevocationReason getRevocationReason(PGPSignature sig) { + if (sig.getSignatureType() != KEY_REVOCATION) { + throw new IllegalArgumentException( + "Expected KEY_REVOCATION signature, got " + sig.getSignatureType()); + } + SignatureSubpacket sub = + sig.getHashedSubPackets().getSubpacket(REVOCATION_REASON); + if (sub == null) { + return null; + } + return new RevocationReason(sub.isCritical(), sub.getData()); + } + + private static String reasonToString(RevocationReason reason) { + StringBuilder r = new StringBuilder("Key is revoked ("); + if (reason == null) { + return r.append("no reason provided)").toString(); + } + switch (reason.getRevocationReason()) { + case NO_REASON: + r.append("no reason code specified"); + break; + case KEY_SUPERSEDED: + r.append("superseded"); + break; + case KEY_COMPROMISED: + r.append("key material has been compromised"); + break; + case KEY_RETIRED: + r.append("retired and no longer valid"); + break; + default: + r.append("reason code ") + .append(Integer.toString(reason.getRevocationReason())) + .append(')'); + break; + } + r.append(')'); + String desc = reason.getRevocationDescription(); + if (!desc.isEmpty()) { + r.append(": ").append(desc); + } + return r.toString(); + } + private CheckResult checkWebOfTrust(PGPPublicKey key, PublicKeyStore store, int depth, Set seen) { if (trusted == null) { 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 b2798f5333..8f614d7dc8 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 @@ -178,15 +178,39 @@ public class PublicKeyStore implements AutoCloseable { */ public PGPPublicKeyRingCollection get(long keyId) throws PGPException, IOException { + return new PGPPublicKeyRingCollection(get(keyId, null)); + } + + /** + * Read public key with the given fingerprint. + *

+ * Keys should not be trusted unless checked with {@link PublicKeyChecker}. + *

+ * Multiple calls to this method use the same state of the key ref; to reread + * the ref, call {@link #close()} first. + * + * @param fingerprint key fingerprint. + * @return the key if found, or {@code null}. + * @throws PGPException if an error occurred parsing the key data. + * @throws IOException if an error occurred reading the repository data. + */ + public PGPPublicKeyRing get(byte[] fingerprint) + throws PGPException, IOException { + List keyRings = + get(Fingerprint.getId(fingerprint), fingerprint); + return !keyRings.isEmpty() ? keyRings.get(0) : null; + } + + private List get(long keyId, byte[] fp) throws IOException { if (reader == null) { load(); } if (notes == null) { - return empty(); + return Collections.emptyList(); } Note note = notes.getNote(keyObjectId(keyId)); if (note == null) { - return empty(); + return Collections.emptyList(); } List keys = new ArrayList<>(); @@ -200,12 +224,16 @@ public class PublicKeyStore implements AutoCloseable { } Object obj = it.next(); if (obj instanceof PGPPublicKeyRing) { - keys.add((PGPPublicKeyRing) obj); + PGPPublicKeyRing kr = (PGPPublicKeyRing) obj; + if (fp == null + || Arrays.equals(fp, kr.getPublicKey().getFingerprint())) { + keys.add(kr); + } } checkState(!it.hasNext(), "expected one PGP object per ArmoredInputStream"); } - return new PGPPublicKeyRingCollection(keys); + return keys; } } @@ -375,12 +403,6 @@ public class PublicKeyStore implements AutoCloseable { return out.toByteArray(); } - private static PGPPublicKeyRingCollection empty() - throws PGPException, IOException { - return new PGPPublicKeyRingCollection( - Collections. emptyList()); - } - public static String keyToString(PGPPublicKey key) { @SuppressWarnings("unchecked") Iterator it = key.getUserIDs(); 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 87d778a8b0..e87a0ee6dd 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 @@ -208,7 +208,9 @@ public abstract class PushCertificateChecker { CheckResult.bad("Signature by " + keyIdToString(sig.getKeyID()) + " is not valid")); } - CheckResult result = publicKeyChecker.check(signer); + CheckResult result = publicKeyChecker + .setStore(store) + .check(signer); if (!result.getProblems().isEmpty()) { StringBuilder err = new StringBuilder("Invalid public key ") .append(keyToString(signer)) diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java index 63d4fe803c..a136007924 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java @@ -163,7 +163,7 @@ public class GpgKeys implements found = true; GpgKeyInfo info = toJson( keyRing.getPublicKey(), - checkerFactory.create().setExpectedUser(rsrc.getUser()), + checkerFactory.create(rsrc.getUser(), store), store); keys.put(info.id, info); info.id = null; diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java index 0ed1f7b0e9..91c4494722 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java @@ -194,8 +194,7 @@ public class PostGpgKeys implements RestModifyView { for (PGPPublicKeyRing keyRing : keyRings) { PGPPublicKey key = keyRing.getPublicKey(); // Don't check web of trust; admins can fill in certifications later. - CheckResult result = checkerFactory.create() - .setExpectedUser(rsrc.getUser()) + CheckResult result = checkerFactory.create(rsrc.getUser(), store) .disableTrust() .check(key); if (!result.isOk()) { @@ -249,9 +248,7 @@ public class PostGpgKeys implements RestModifyView { throws IOException { // Unlike when storing keys, include web-of-trust checks when producing // result JSON, so the user at least knows of any issues. - PublicKeyChecker checker = checkerFactory.create() - .setExpectedUser(user) - .setStore(store); + PublicKeyChecker checker = checkerFactory.create(user, store); Map infos = Maps.newHashMapWithExpectedSize(keys.size() + deleted.size()); for (PGPPublicKeyRing keyRing : keys) { diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java index edc0559498..db0bb88ef7 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java @@ -172,8 +172,7 @@ public class GerritPublicKeyCheckerTest { @Test public void defaultGpgCertificationMatchesEmail() throws Exception { TestKey key = validKeyWithSecondUserId(); - PublicKeyChecker checker = checkerFactory.create() - .setExpectedUser(user) + PublicKeyChecker checker = checkerFactory.create(user, store) .disableTrust(); assertProblems( checker.check(key.getPublicKey()), Status.BAD, @@ -183,8 +182,7 @@ public class GerritPublicKeyCheckerTest { + " username:user"); addExternalId("test", "test", "test5@example.com"); - checker = checkerFactory.create() - .setExpectedUser(user) + checker = checkerFactory.create(user, store) .disableTrust(); assertNoProblems(checker.check(key.getPublicKey())); } @@ -192,8 +190,7 @@ public class GerritPublicKeyCheckerTest { @Test public void defaultGpgCertificationDoesNotMatchEmail() throws Exception { addExternalId("test", "test", "nobody@example.com"); - PublicKeyChecker checker = checkerFactory.create() - .setExpectedUser(user) + PublicKeyChecker checker = checkerFactory.create(user, store) .disableTrust(); assertProblems( checker.check(validKeyWithSecondUserId().getPublicKey()), Status.BAD, @@ -208,8 +205,7 @@ public class GerritPublicKeyCheckerTest { @Test public void manualCertificationMatchesExternalId() throws Exception { addExternalId("foo", "myId", null); - PublicKeyChecker checker = checkerFactory.create() - .setExpectedUser(user) + PublicKeyChecker checker = checkerFactory.create(user, store) .disableTrust(); assertNoProblems(checker.check(validKeyWithSecondUserId().getPublicKey())); } @@ -217,8 +213,7 @@ public class GerritPublicKeyCheckerTest { @Test public void manualCertificationDoesNotMatchExternalId() throws Exception { addExternalId("foo", "otherId", null); - PublicKeyChecker checker = checkerFactory.create() - .setExpectedUser(user) + PublicKeyChecker checker = checkerFactory.create(user, store) .disableTrust(); assertProblems( checker.check(validKeyWithSecondUserId().getPublicKey()), Status.BAD, @@ -236,8 +231,7 @@ public class GerritPublicKeyCheckerTest { reloadUser(); TestKey key = validKeyWithSecondUserId(); - PublicKeyChecker checker = checkerFactory.create() - .setExpectedUser(user) + PublicKeyChecker checker = checkerFactory.create(user, store) .disableTrust(); assertProblems( checker.check(key.getPublicKey()), Status.BAD, @@ -245,6 +239,7 @@ public class GerritPublicKeyCheckerTest { + " http://test/#/settings/web-identities"); checker = checkerFactory.create() + .setStore(store) .disableTrust(); assertProblems( checker.check(key.getPublicKey()), Status.BAD, @@ -277,16 +272,12 @@ public class GerritPublicKeyCheckerTest { add(keyE(), addUser("userE")); // Checker for A, checking A. - PublicKeyChecker checkerA = checkerFactory.create() - .setExpectedUser(user) - .setStore(store); + PublicKeyChecker checkerA = checkerFactory.create(user, store); assertNoProblems(checkerA.check(keyA.getPublicKey())); // Checker for B, checking B. Trust chain and IDs are correct, so the only // problem is with the key itself. - PublicKeyChecker checkerB = checkerFactory.create() - .setExpectedUser(userB) - .setStore(store); + PublicKeyChecker checkerB = checkerFactory.create(userB, store); assertProblems( checkerB.check(keyB.getPublicKey()), Status.BAD, "Key is expired"); @@ -311,9 +302,7 @@ public class GerritPublicKeyCheckerTest { add(keyE(), addUser("userE")); // Checker for A, checking B. - PublicKeyChecker checkerA = checkerFactory.create() - .setExpectedUser(user) - .setStore(store); + PublicKeyChecker checkerA = checkerFactory.create(user, store); assertProblems( checkerA.check(keyB.getPublicKey()), Status.BAD, "Key is expired", @@ -325,9 +314,7 @@ public class GerritPublicKeyCheckerTest { + " username:user"); // Checker for B, checking A. - PublicKeyChecker checkerB = checkerFactory.create() - .setExpectedUser(userB) - .setStore(store); + PublicKeyChecker checkerB = checkerFactory.create(userB, store); assertProblems( checkerB.check(keyA.getPublicKey()), Status.BAD, "Key must contain a valid certification for one of the following" @@ -346,9 +333,7 @@ public class GerritPublicKeyCheckerTest { TestKey keyA = add(keyA(), user); TestKey keyB = add(keyB(), addUser("userB")); - PublicKeyChecker checker = checkerFactory.create() - .setExpectedUser(user) - .setStore(store); + PublicKeyChecker checker = checkerFactory.create(user, store); assertProblems( checker.check(keyA.getPublicKey()), Status.OK, "No path to a trusted key", @@ -406,9 +391,7 @@ public class GerritPublicKeyCheckerTest { keyRingB = PGPPublicKeyRing.insertPublicKey(keyRingB, keyB); add(keyRingB, addUser("userB")); - PublicKeyChecker checkerA = checkerFactory.create() - .setExpectedUser(user) - .setStore(store); + PublicKeyChecker checkerA = checkerFactory.create(user, store); assertProblems(checkerA.check(keyA.getPublicKey()), Status.OK, "No path to a trusted key", "Certification by " + keyToString(keyB) 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 6f69b38021..a611ec9484 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 @@ -16,6 +16,8 @@ package com.google.gerrit.gpg; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; import static com.google.gerrit.gpg.testutil.TestKeys.expiredKey; +import static com.google.gerrit.gpg.testutil.TestKeys.revokedCompromisedKey; +import static com.google.gerrit.gpg.testutil.TestKeys.revokedNoLongerUsedKey; import static com.google.gerrit.gpg.testutil.TestKeys.selfRevokedKey; import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithExpiration; import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithoutExpiration; @@ -29,10 +31,15 @@ import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyG; import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyH; import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyI; import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyJ; +import static org.bouncycastle.bcpg.SignatureSubpacketTags.REVOCATION_KEY; +import static org.bouncycastle.openpgp.PGPSignature.DIRECT_KEY; import static org.junit.Assert.assertEquals; import com.google.gerrit.gpg.testutil.TestKey; +import org.bouncycastle.openpgp.PGPPublicKey; +import org.bouncycastle.openpgp.PGPPublicKeyRing; +import org.bouncycastle.openpgp.PGPSignature; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.lib.CommitBuilder; @@ -46,6 +53,7 @@ import org.junit.rules.ExpectedException; import java.util.Arrays; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; public class PublicKeyCheckerTest { @@ -90,7 +98,8 @@ public class PublicKeyCheckerTest { @Test public void selfRevokedKeyIsRevoked() throws Exception { - assertProblems(selfRevokedKey(), "Key is revoked"); + assertProblems(selfRevokedKey(), + "Key is revoked (key material has been compromised)"); } // Test keys specific to this test are at the bottom of this class. Each test @@ -174,6 +183,57 @@ public class PublicKeyCheckerTest { "No path to a trusted key", notTrusted(ki)); } + @Test + public void revokedKeyDueToCompromise() throws Exception { + TestKey k = add(revokedCompromisedKey()); + add(validKeyWithoutExpiration()); + save(); + + assertProblems(k, + "Key is revoked (key material has been compromised):" + + " test6 compromised"); + + PGPPublicKeyRing kr = removeRevokers(k.getPublicKeyRing()); + store.add(kr); + save(); + + // Key no longer specified as revoker. + assertProblems(kr.getPublicKey()); + } + + @Test + public void revokedByKeyNotPresentInStore() throws Exception { + TestKey k = add(revokedCompromisedKey()); + save(); + + assertProblems(k, + "Key is revoked (key material has been compromised):" + + " test6 compromised"); + } + + @Test + public void revokedKeyDueToNoLongerBeingUsed() throws Exception { + TestKey k = add(revokedNoLongerUsedKey()); + add(validKeyWithoutExpiration()); + save(); + + assertProblems(k, + "Key is revoked (retired and no longer valid): test7 not used"); + } + + private PGPPublicKeyRing removeRevokers(PGPPublicKeyRing kr) { + PGPPublicKey k = kr.getPublicKey(); + @SuppressWarnings("unchecked") + Iterator sigs = k.getSignaturesOfType(DIRECT_KEY); + while (sigs.hasNext()) { + PGPSignature sig = sigs.next(); + if (sig.getHashedSubPackets().hasSubpacket(REVOCATION_KEY)) { + k = PGPPublicKey.removeCertification(k, sig); + } + } + return PGPPublicKeyRing.insertPublicKey(kr, k); + } + private PublicKeyChecker newChecker(int maxTrustDepth, TestKey... trusted) { Map fps = new HashMap<>(); for (TestKey k : trusted) { @@ -208,12 +268,19 @@ public class PublicKeyCheckerTest { private void assertProblems(PublicKeyChecker checker, TestKey k, String... expected) { - CheckResult result = checker.check(k.getPublicKey()); + CheckResult result = checker.setStore(store) + .check(k.getPublicKey()); assertEquals(Arrays.asList(expected), result.getProblems()); } private void assertProblems(TestKey tk, String... expected) throws Exception { - CheckResult result = new PublicKeyChecker().check(tk.getPublicKey()); + assertProblems(tk.getPublicKey(), expected); + } + + private void assertProblems(PGPPublicKey k, String... expected) throws Exception { + CheckResult result = new PublicKeyChecker() + .setStore(store) + .check(k); assertEquals(Arrays.asList(expected), 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 dc1b5576fe..9b1c058c2e 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 @@ -14,7 +14,6 @@ package com.google.gerrit.gpg; -import static com.google.gerrit.gpg.PublicKeyStore.REFS_GPG_KEYS; import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; import static com.google.gerrit.gpg.testutil.TestKeys.expiredKey; @@ -33,7 +32,9 @@ import org.bouncycastle.openpgp.PGPUtil; import org.bouncycastle.openpgp.operator.bc.BcPGPContentSignerBuilder; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; -import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.PushCertificate; import org.eclipse.jgit.transport.PushCertificateIdent; @@ -49,7 +50,8 @@ import java.io.Reader; import java.util.Arrays; public class PushCertificateCheckerTest { - private TestRepository tr; + private InMemoryRepository repo; + private PublicKeyStore store; private SignedPushConfig signedPushConfig; private PushCertificateChecker checker; @@ -57,14 +59,17 @@ public class PushCertificateCheckerTest { public void setUp() throws Exception { TestKey key1 = validKeyWithoutExpiration(); TestKey key3 = expiredKey(); - tr = new TestRepository<>(new InMemoryRepository( - new DfsRepositoryDescription("repo"))); - tr.branch(REFS_GPG_KEYS).commit() - .add(PublicKeyStore.keyObjectId(key1.getPublicKey().getKeyID()).name(), - key1.getPublicKeyArmored()) - .add(PublicKeyStore.keyObjectId(key3.getPublicKey().getKeyID()).name(), - key3.getPublicKeyArmored()) - .create(); + repo = new InMemoryRepository(new DfsRepositoryDescription("repo")); + store = new PublicKeyStore(repo); + store.add(key1.getPublicKeyRing()); + store.add(key3.getPublicKeyRing()); + + PersonIdent ident = new PersonIdent("A U Thor", "author@example.com"); + CommitBuilder cb = new CommitBuilder(); + cb.setAuthor(ident); + cb.setCommitter(ident); + assertEquals(RefUpdate.Result.NEW, store.save(cb)); + signedPushConfig = new SignedPushConfig(); signedPushConfig.setCertNonceSeed("sekret"); signedPushConfig.setCertNonceSlopLimit(60 * 24); @@ -72,10 +77,11 @@ public class PushCertificateCheckerTest { } private PushCertificateChecker newChecker(boolean checkNonce) { - return new PushCertificateChecker(new PublicKeyChecker()) { + PublicKeyChecker keyChecker = new PublicKeyChecker().setStore(store); + return new PushCertificateChecker(keyChecker) { @Override protected Repository getRepository() { - return tr.getRepository(); + return repo; } @Override @@ -126,7 +132,7 @@ public class PushCertificateCheckerTest { private String validNonce() { return signedPushConfig.getNonceGenerator() - .createNonce(tr.getRepository(), System.currentTimeMillis() / 1000); + .createNonce(repo, System.currentTimeMillis() / 1000); } private PushCertificate newSignedCert(String nonce, TestKey signingKey) @@ -158,7 +164,7 @@ public class PushCertificateCheckerTest { Reader reader = new InputStreamReader(new ByteArrayInputStream(cert.getBytes(UTF_8))); PushCertificateParser parser = - new PushCertificateParser(tr.getRepository(), signedPushConfig); + new PushCertificateParser(repo, signedPushConfig); return parser.parse(reader); } diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java index 3b3ab88a15..97a94b90d2 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java @@ -542,6 +542,246 @@ public class TestKeys { + "-----END PGP PRIVATE KEY BLOCK-----\n"); } - // TODO(dborowitz): Figure out how to get gpg to revoke a key for someone - // else. + /** + * A key revoked by a valid key, due to key compromise. + *

+ * Revoked by {@link #validKeyWithoutExpiration()}. + * + * 

+   * pub   2048R/3434B39F 2015-10-20 [revoked: 2015-10-20]
+   *       Key fingerprint = 931F 047D 7D01 DDEF 367A  8D90 8C4F D28E 3434 B39F
+   * uid                  Testuser Six <test6@example.com>
+   *
+ */ + public static TestKey revokedCompromisedKey() throws Exception { + return new TestKey("-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + + "Version: GnuPG v1\n" + + "\n" + + "mQENBFYmpXkBCACqaLz51DcWQmfOJnat9iHSySfSHwbKfVvoN43Ba2cf/D/PadRc\n" + + "HLgc+91k2yk1kV1LnMdvUGj5zZ84ZqrQx3f1WeItnzZpqxtmQS/GSxCp9EY/s7w6\n" + + "5i86R/k9Tzgvk0B7dKZJXbM/OWxxDkkxHWE3Un9wreX7bDU5b9D2knHRiNFqH9ZJ\n" + + "KqDIFZqH9WTUxNZcHz20sTCRIMfvsAwf2vRU5N5xTu4Mbk6JFc7BAj7h1f/mYEPo\n" + + "CTyB1jV/DSDVdn1FjJVocSg6W/CvsYF9hKFYjJHl4VXdePTpnOjHhJLL0QWk0TMe\n" + + "xYeUi/xDr5DeMxTmi7F7BFaQEF+KmUM46e+9ABEBAAGJATAEIAECABoFAlYmq1gT\n" + + "HQJ0ZXN0NiBjb21wcm9taXNlZAAKCRDtBiXcRjKKjIm6B/9YwkyG4w+9KUNESywM\n" + + "bxC2WWGWrFcQGoKxixzt0uT251UY8qxa1IED0wnLsIQmffTQcnrK3B9svd4HhQlk\n" + + "pheKQ3w5iluLeGmGljhDBdAVyS07jYoFUGTXjwzPAgJ3Dxzul8Q8Zj+fOmRcfsP9\n" + + "72kl6g2yEEbevnydWIiOj/vWHVLFb54G8bwXTNwH/FXQsHuPYxXZifwyDwdwEQMq\n" + + "0VTZcrukgeJ+VbSSuq+uX4I3+kJw5hL49KYAQltQBmTo3yhuY/Q+LkgcBv/umtY/\n" + + "DrUqSCBV1bTnfq5SfaObkUu22HWjrtSFSjnXYyh+wyTG3AXG3N9VPrjGQIJIW1j6\n" + + "9QM0iQE3BB8BAgAhBQJWJqYUFwyAAQSup+0vghEz5bEo0e0GJdxGMoqMAgcAAAoJ\n" + + "EIxP0o40NLOfYd4H/3GpfxfJ+nMzBChn1JqFrKOqqYiOO4sUwubXzpRO33V2jUrU\n" + + "V75PTWG/6NlgDbPfKFcU0qZud6M2EQxSS9/I20i/MpRB7qJnWMM/6HxdMDJ0o/pN\n" + + "4ImIGj38QTIWx0DS9n3bwlcobl7ZlM8g2N1kv5jQPEuurffeJRS4ny4pEvCCm2IS\n" + + "SGOuB0DVtYHGDrJLQ0k4mDkEJuU8fP5un8mN8I8eAINlsTFpsTswMXMiptZTm5SI\n" + + "5QZlG3m5MvvckngYdhynvCWc6JHGt1EHXlI4A5Qetr/4FbNE4uYcEEhyzBy4WQfi\n" + + "QCPiIzzm3O4cMnr9N+5HzYqRhu2OveYm86G2Rxq0IFRlc3R1c2VyIFNpeCA8dGVz\n" + + "dDZAZXhhbXBsZS5jb20+iQE4BBMBAgAiBQJWJqV5AhsDBgsJCAcDAgYVCAIJCgsE\n" + + "FgIDAQIeAQIXgAAKCRCMT9KONDSzn2XtB/4wl4ctc3cW9Fwp17cktFi6md8fjRiR\n" + + "wE/ruVKIKmAHzeMLBoZn4LZVunyNCRGLZfP+MUs4JhLkp8ioTzUB7xPl9k94FXel\n" + + "bObn9F0T7htjFLiFAOMeykneylk2kalTt6IBKtaOPn+V6onBwO+YHbwt+xLMhAWj\n" + + "Z/WA0TIC1RIukdzWErhd+9lG8B9kupGC5bPo/AgCPoajPhS1qLrth+lCsNJXT/Rt\n" + + "k6Jx5omypxMXPzgzNtULMFONszaRnHnrCHQg/yJZDCw3ffW5ShfyfWdFM65jgEKo\n" + + "nMKLzy9XV+BM6IJQlgHCBAP8WHKSf4qMG4/hEWLrwA/bTQ7w0DSV88msuQENBFYm\n" + + "pXkBCACzIMFDC6kcV58uvF3XwOrS3DmKNPDNzO/4Ay/iOxZbm+9NP8QWEEm+AzCt\n" + + "ZMfYdZ8C3DjuzxkhcacI/E5agZICds6bs0+VS7VKEeNYp/UrTF9pkZNXseCrJPgr\n" + + "U31eoGVc5bE5c0TGLhAjbMKtR5LZFMpAXgpA7hXJSSuAXGs8gjkJkYSJYnJwIOyd\n" + + "xOi5jmnE/U5QuMjBG0bwxFXxkaDa5mcebJ/6C8mgkKyATbQkCe7YJGl1JLK4vY28\n" + + "ybSMhMDtZiwgvKzd+HcQr+xUQvmgSMApJaMxKPHRA1IrP/STXUEAjcGfk/HCz/0j\n" + + "7mJG2cvCxeOMAmp/pTzhSoXiqUNlABEBAAGJAR8EGAECAAkFAlYmpXkCGwwACgkQ\n" + + "jE/SjjQ0s5/kVAf/QvHOhuoBSlSxPcgvnvCl8V3zbNR1P9lgjYGwMsvLhwCT7Wvm\n" + + "mkUKvtT913uER93N8xJD2svGhKabpiPj9/eo0p3p64dicijsP1UQfpmWKPa/V9sv\n" + + "zep08cpDl/eczSiLqgcTXCoZeewWXoQGqqoXnwa4lwQv4Zvj7TTCN2wRzoGwbRcm\n" + + "G2hmc27uOwA+hXbF+bLe6HOZR/7U93j8a22g2X9OgST/QCsLgyiUSw3YYaEan9tn\n" + + "wuEgAEY/rchOvgeXe5Sl0wTFLHH6OS4BBGgc1LRKnSCM2dgZqvhOOxOvuuieBWY6\n" + + "tULvIEIjNNP8Qizfc4u2O8h7HP2b3yYSrp9MMQ==\n" + + "=Dxr7\n" + + "-----END PGP PUBLIC KEY BLOCK-----\n", + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: GnuPG v1\n" + + "\n" + + "lQOYBFYmpXkBCACqaLz51DcWQmfOJnat9iHSySfSHwbKfVvoN43Ba2cf/D/PadRc\n" + + "HLgc+91k2yk1kV1LnMdvUGj5zZ84ZqrQx3f1WeItnzZpqxtmQS/GSxCp9EY/s7w6\n" + + "5i86R/k9Tzgvk0B7dKZJXbM/OWxxDkkxHWE3Un9wreX7bDU5b9D2knHRiNFqH9ZJ\n" + + "KqDIFZqH9WTUxNZcHz20sTCRIMfvsAwf2vRU5N5xTu4Mbk6JFc7BAj7h1f/mYEPo\n" + + "CTyB1jV/DSDVdn1FjJVocSg6W/CvsYF9hKFYjJHl4VXdePTpnOjHhJLL0QWk0TMe\n" + + "xYeUi/xDr5DeMxTmi7F7BFaQEF+KmUM46e+9ABEBAAEAB/wOspbuA1A3AsY6QRYG\n" + + "Xg6/w+rD1Do9N7+4ESaQUqej2hlU1d9jjHSSx2RqgP6WaLG/xkdrQeez9/iuICjG\n" + + "dhXSGw0He05xobjswl2RAENxLSjr8KAhAl57a97C23TQoaYzn7WB6Wt+3gCM5bsJ\n" + + "WevbHinwuYb2/ve+OvcudSYM+Nhtpv0DoTaizhi9wzc3g/XLbturlpdCffbw4y+h\n" + + "gBPd/t3cc/0Ams8Wi2RlmDOoe73ls23nBHcNomgydyIYBn7U5Z3v3YkPNp9VBiXx\n" + + "rC4mDtB1ugucMhqjRNAYqinaLP35CiBTU/IB0WLu7ZyytnjY5frly1ShAG8wFL0B\n" + + "MOMxBADJjGy1NwGSd/7eMeYyYThyhXDxo5so91/O1+RLnSUVv/Nz6VOPp2TtuVN5\n" + + "uTJkpSXtUFyWbf8mkQiFz4++vHW5E/Q6+KomXRalK7JeBzeFMtax64ykQHID9cSu\n" + + "TaSHBhOEEeZZuf6BlulYEJEBHYK6EFlPJn+cpZtTFaqDoKh22QQA2HKjfyeppNre\n" + + "WRFJ9h1x1hBlSRR+XIPYmDmZUjL37jQUlw8iF+txPclfyNBw2I2Om+Jhcf25peOx\n" + + "ow4yvjt8r3qDjNhI2zLE9u4zrQ9xU8CUingT0t4k3NO2vigpKlmp1/w2IHSMctry\n" + + "v1v3+BAS8qGIYDY1lgI7QBvle5hxGYUD/00zMyHOIgYg/cM5sR0qafesoj9kRff5\n" + + "UMnSy1dw+pGMv6GqKGbcZDoC060hUO9GhQRPZXF8PlYzD30lOLS2Uw4mPXjOmQVv\n" + + "lDiyl/vLkfkVfP/alYH0FW6mErDrjtHhrZewqDm3iPLGMVGfGCJsL+N37VBSe+jr\n" + + "4rZCnjk/Jo5JRoKJATcEHwECACEFAlYmphQXDIABBK6n7S+CETPlsSjR7QYl3EYy\n" + + "iowCBwAACgkQjE/SjjQ0s59h3gf/cal/F8n6czMEKGfUmoWso6qpiI47ixTC5tfO\n" + + "lE7fdXaNStRXvk9NYb/o2WANs98oVxTSpm53ozYRDFJL38jbSL8ylEHuomdYwz/o\n" + + "fF0wMnSj+k3giYgaPfxBMhbHQNL2fdvCVyhuXtmUzyDY3WS/mNA8S66t994lFLif\n" + + "LikS8IKbYhJIY64HQNW1gcYOsktDSTiYOQQm5Tx8/m6fyY3wjx4Ag2WxMWmxOzAx\n" + + "cyKm1lOblIjlBmUbebky+9ySeBh2HKe8JZzokca3UQdeUjgDlB62v/gVs0Ti5hwQ\n" + + "SHLMHLhZB+JAI+IjPObc7hwyev037kfNipGG7Y695ibzobZHGrQgVGVzdHVzZXIg\n" + + "U2l4IDx0ZXN0NkBleGFtcGxlLmNvbT6JATgEEwECACIFAlYmpXkCGwMGCwkIBwMC\n" + + "BhUIAgkKCwQWAgMBAh4BAheAAAoJEIxP0o40NLOfZe0H/jCXhy1zdxb0XCnXtyS0\n" + + "WLqZ3x+NGJHAT+u5UogqYAfN4wsGhmfgtlW6fI0JEYtl8/4xSzgmEuSnyKhPNQHv\n" + + "E+X2T3gVd6Vs5uf0XRPuG2MUuIUA4x7KSd7KWTaRqVO3ogEq1o4+f5XqicHA75gd\n" + + "vC37EsyEBaNn9YDRMgLVEi6R3NYSuF372UbwH2S6kYLls+j8CAI+hqM+FLWouu2H\n" + + "6UKw0ldP9G2TonHmibKnExc/ODM21QswU42zNpGceesIdCD/IlkMLDd99blKF/J9\n" + + "Z0UzrmOAQqicwovPL1dX4EzoglCWAcIEA/xYcpJ/iowbj+ERYuvAD9tNDvDQNJXz\n" + + "yaydA5gEVialeQEIALMgwUMLqRxXny68XdfA6tLcOYo08M3M7/gDL+I7Flub700/\n" + + "xBYQSb4DMK1kx9h1nwLcOO7PGSFxpwj8TlqBkgJ2zpuzT5VLtUoR41in9StMX2mR\n" + + "k1ex4Ksk+CtTfV6gZVzlsTlzRMYuECNswq1HktkUykBeCkDuFclJK4BcazyCOQmR\n" + + "hIlicnAg7J3E6LmOacT9TlC4yMEbRvDEVfGRoNrmZx5sn/oLyaCQrIBNtCQJ7tgk\n" + + "aXUksri9jbzJtIyEwO1mLCC8rN34dxCv7FRC+aBIwCklozEo8dEDUis/9JNdQQCN\n" + + "wZ+T8cLP/SPuYkbZy8LF44wCan+lPOFKheKpQ2UAEQEAAQAH/A1Os+Tb9yiGnuoN\n" + + "LuiSKa/YEgNBOxmC7dnuPK6xJpBQNZc200WzWJMf8AwVpl4foNxIyYb+Rjbsl1Ts\n" + + "z5JcOWFq+57oE5O7D+EMkqf5tFZO4nC4kqprac41HSW02mW/A0DDRKcIt/WEIwlK\n" + + "sWzHmjJ736moAtl/holRYQS0ePgB8bUPDQcFovH6X3SUxlPGTYD1DEX+WNvYRk3r\n" + + "pa9YXH65qbG9CEJIFTmwZIRDl+CBtBlN/fKadyMJr9fXtv7Fu9hNsK1K1pUtLqCa\n" + + "nc22Zak+o+LCPlZ8vmw/UmOGtp2iZlEragmh2rOywp0dHF7gsdlgoafQf8Q4NIag\n" + + "TFyHf1kEAMSOKUUwLBEmPnDVfoEOt5spQLVtlF8sh/Okk9zVazWmw0n/b1Ef72z6\n" + + "EZqCW9/XhH5pXfKJeV+08hroHI6a5UESa7/xOIx50TaQdRqjwGciMnH2LJcpIU/L\n" + + "f0cGXcnTLKt4Z2GeSPKFTj4VzwmwH5F/RYdc5eiVb7VNoy9DC5RZBADpTVH5pklS\n" + + "44VDJIcwSNy1LBEU3oj+Nu+sufCimJ5B7HLokoJtm6q8VQRga5hN1TZkdQcLy+b2\n" + + "wzxHYoIsIsYFfG/mqLZ3LJNDFqze1/Kj987DYSUGeNYexMN2Fkzbo35Jf0cpOiao\n" + + "390JFOS7qecUak5/yJ/V4xy8/nds37617QP9GWlFBykDoESBC2AIz8wXcpUBVNeH\n" + + "BNSthmC+PJPhsS6jTQuipqtXUZBgZBrMHp/bA8gTOkI4rPXycH3+ACbuQMAjbFny\n" + + "Kt69lPHD8VWw/82E4EY2J9LmHli+2BcATz89ouC4kqC5zF90qJseviSZPihpnFxA\n" + + "1UqMU2ZjsPb4CM9C/YkBHwQYAQIACQUCVialeQIbDAAKCRCMT9KONDSzn+RUB/9C\n" + + "8c6G6gFKVLE9yC+e8KXxXfNs1HU/2WCNgbAyy8uHAJPta+aaRQq+1P3Xe4RH3c3z\n" + + "EkPay8aEppumI+P396jSnenrh2JyKOw/VRB+mZYo9r9X2y/N6nTxykOX95zNKIuq\n" + + "BxNcKhl57BZehAaqqhefBriXBC/hm+PtNMI3bBHOgbBtFyYbaGZzbu47AD6FdsX5\n" + + "st7oc5lH/tT3ePxrbaDZf06BJP9AKwuDKJRLDdhhoRqf22fC4SAARj+tyE6+B5d7\n" + + "lKXTBMUscfo5LgEEaBzUtEqdIIzZ2Bmq+E47E6+66J4FZjq1Qu8gQiM00/xCLN9z\n" + + "i7Y7yHsc/ZvfJhKun0wx\n" + + "=M/kw\n" + + "-----END PGP PRIVATE KEY BLOCK-----\n"); + } + + /** + * A key revoked by a valid key, due to no longer being used. + *

+ * Revoked by {@link #validKeyWithoutExpiration()}. + * + * 

+   * pub   2048R/3D6C52D0 2015-10-20 [revoked: 2015-10-20]
+   *       Key fingerprint = 32DB 6C31 2ED7 A98D 11B2  43EA FAD2 ABE2 3D6C 52D0
+   * uid                  Testuser Seven <test7@example.com>
+   *
+ */ + public static TestKey revokedNoLongerUsedKey() throws Exception { + return new TestKey("-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + + "Version: GnuPG v1\n" + + "\n" + + "mQENBFYmq3EBCAC9ssY6QhFsnZqKEPlQrx8Zomblj8qV93/B448isOT2L6OVY7UC\n" + + "kKPj6afW5UDkYeyZSmLZfTrpePcbAB8FB3uvd/AS9mHC+6zuBlwlkl9xIXlwUXQP\n" + + "KER4LKYNTP21AM+/vTJm4+u26tlZECIZlez31KEeqM30EAm+/pO8VkEp8+1ImfLv\n" + + "otndIjMoq9gxJvn6KZeexJT2eKCsSa20vVsmAhuFjLZitU3lEjIROfDiyHUZ2cZ+\n" + + "qynfppJCKlHJRu/T9L/yxDFVUFDFSajNzSfjG1g3FEveDITyAhRetVfZbhyJptnV\n" + + "jfiHSQkLamPsBmMoKfP+aO5SfsTHTJvxgLUdABEBAAGJAS0EIAECABcFAlYmq8AQ\n" + + "HQN0ZXN0NyBub3QgdXNlZAAKCRDtBiXcRjKKjPKqB/sF+ypJZaZ5M4jFdoH/YA3s\n" + + "4+VkA/NbLKcrlMI0lbnIrax02jdyTo7rBUJfTwuBs5QeQ25+VfaBcz9fWSv4Z8Bk\n" + + "9+w61bQZLQkExZ9W7hnhaapyR0aT0rY48KGtHOPNoMQu9Si+RnRiI024jMUUjrau\n" + + "w/exgCteY261VtCPRgyZOlpbX43rsBhF8ott0ZzSfLwaNTHhsjFsD1uH6TSFO8La\n" + + "/H1nO31sORlY3+rCGiQVuYIJD1qI7bEjDHYO0nq/f7JjfYKmVBg9grwLsX3h1qZ2\n" + + "L3Yz+0eCi7/6T/Sm7PavQ+EGL7+WBXX3qJpwc+EFNHs6VxQp86k6csba0c5mNcaQ\n" + + "iQE3BB8BAgAhBQJWJqusFwyAAQSup+0vghEz5bEo0e0GJdxGMoqMAgcAAAoJEPrS\n" + + "q+I9bFLQ2BYH/jm+t7pZuv8WqZdb8FiBa9CFfhcSKjYarMHjBw7GxWZJMd5VR4DC\n" + + "r4T/ZSAGRKBRKQ2uXrkm9H0NPDp0c/UKCHtQMFDnqTk7B63mwSR1d7W0qaRPXYQ1\n" + + "bbatnzkEDOj0e+rX6aiqVRMo/q6uMNUFl6UMrUZPSNB5PVRQWPnQ7K11mw3vg0e5\n" + + "ycqJbyFvER6EtyDUXGBo8a5/4bK8VBNBMTAIy6GeGpeSM5b7cpQk7/j4dXugCJAV\n" + + "fhFNUOgLduoIKM4u+VcFjk3Km/YxOtGi1dLqCbTX/0LiCRA9mgQpyNVyA+Sm48LM\n" + + "LUkbcrN/F3SHX1ao/5lm19r8Biu1ziQnLgC0IlRlc3R1c2VyIFNldmVuIDx0ZXN0\n" + + "N0BleGFtcGxlLmNvbT6JATgEEwECACIFAlYmq3ECGwMGCwkIBwMCBhUIAgkKCwQW\n" + + "AgMBAh4BAheAAAoJEPrSq+I9bFLQvjQH/0K7aBsGU2U/rm4I+u+uPa6BnFTYQJqg\n" + + "034pwdD0WfM3M/XgVh7ERjnR9ZViCMVej+K3kW5d2DNaXu5vVpcD6L6jjWwiJHBw\n" + + "LIcmpqQrL0TdoCr4F4FKQnBbcH1fNvP8A/hLDHB3k3ERPvEFIo1AkVuK4s/v7yZY\n" + + "HAowX0r4ok4ndu/wAc0HI1FkApkAfh18JDTuui53dkKhnkDp7Xnfm/ElAZYjB7Se\n" + + "ivxOD9vdhViWSx1VhttPZo5hSyJrEYaJ5u9hsXNUN85DxgLqCmS1v8n3pN1lVY/Q\n" + + "TYXtgocakQgHGEG0Tl6a3xpNkn9ihnyCr80mHCxXTyUUBGfygccelB+5AQ0EViar\n" + + "cQEIAKxwXb6HGV9QjepADyWW7GMxc2JVZ7pZM2sdf8wrgnQqV2G1rc9gAgwTX4jt\n" + + "OY0vSKT1vBq09ZXS3qpYHi/Wwft0KkaX/a7e6vKabDSfhilxC2LuGz2+56f6UOzj\n" + + "ggwf5k4LFTQvkDUZumwPjoeC2hqQO3Q/9PW39C6GnvsCr5L0MRdO3PbVJM7lJaOk\n" + + "MbGwgysErWgiZXKlxMpIvffIsLC4BAxnjXaCy6zHuBcPMPaRMs7sDRBzeuTV2wnX\n" + + "Sd+IXZgdpd1hF7VkuXenzwOqvBGS66C3ILW0ZTFaOtgrloIkTvtYEcJFWvxqWl2F\n" + + "+JQ5V6eu2aJ3HIGyr9L1R8MUA6EAEQEAAYkBHwQYAQIACQUCViarcQIbDAAKCRD6\n" + + "0qviPWxS0M0PB/9Rbk4/pNW67+uE1lwtaIG7uFiMbJqu8jK6MkD8GdayflroWEZA\n" + + "x0Xow9HL8UaRfeRPTZMrDRpjl+fJIXT5qnlB0FPmzSXAKr3piC8migBcbp5m6hWh\n" + + "c3ScAqWOeMt9j0TTWHh4hKS8Q+lK392ht65cI/kpFhxm9EEaXmajplNL/2G3PVrl\n" + + "fFUgCdOn2DYdVSgJsfBhkcoiy17G3vqtb+We6ulhziae4SIrkUSqdYmRjiFyvqZz\n" + + "tmMEoF6CQNCUb1NK0TsSDeIdDacYjUwyq0Qj6TaXrWcbC3kW0GtWoFTNIiX4q9bN\n" + + "+B6paw/s8P7XCWznTBRdlFWWgrhcpzQ8fefC\n" + + "=CHer\n" + + "-----END PGP PUBLIC KEY BLOCK-----\n", + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: GnuPG v1\n" + + "\n" + + "lQOYBFYmq3EBCAC9ssY6QhFsnZqKEPlQrx8Zomblj8qV93/B448isOT2L6OVY7UC\n" + + "kKPj6afW5UDkYeyZSmLZfTrpePcbAB8FB3uvd/AS9mHC+6zuBlwlkl9xIXlwUXQP\n" + + "KER4LKYNTP21AM+/vTJm4+u26tlZECIZlez31KEeqM30EAm+/pO8VkEp8+1ImfLv\n" + + "otndIjMoq9gxJvn6KZeexJT2eKCsSa20vVsmAhuFjLZitU3lEjIROfDiyHUZ2cZ+\n" + + "qynfppJCKlHJRu/T9L/yxDFVUFDFSajNzSfjG1g3FEveDITyAhRetVfZbhyJptnV\n" + + "jfiHSQkLamPsBmMoKfP+aO5SfsTHTJvxgLUdABEBAAEAB/9AdCtFJSidcolNKwpC\n" + + "/1V+VL9IdYxcWx02CDccjuUkvrgCrL+WcQW2jS/hZMChOKJ2zR78DcBEDr1LF8Xy\n" + + "ZAIC8yoHj15VLUUrFM8fVvYFzt1fq9VWxxRIjscW0teLNgzgdYzYB84RtwcFa2Vi\n" + + "sx2ycTUTYUClEgP1uLMCtX3rnibJh4vR+lVgnDtKSoh4CLAlW6grAAVdw5sSuV7Q\n" + + "i9EJcPezGw1RvBU5PooqNDG6kyw/QqsAS4q3WP4uVJKK1e7S9oqXFEN8k/zfllI0\n" + + "SSkoyP2flzz71rJF/wQMfJ8uf/CelKXd+gPO4FbCWiZSTLe20JR23qiOyvZkfCwg\n" + + "eFmzBADIJUzspDrg5yaqE+HMc8U3O9G9FHoDSweZTbhiq3aK0BqMAn34u0ps6chy\n" + + "VMO6aPWVzgcSHNfTlzpjuN9lwDoimYBH5vZa1HlCHt5eeqTORixkxSerOmILabTi\n" + + "QWq5JPdJwYZiSvK45G5k3G37RTd6/QyhTlRYXj59RXYajrYngwQA8qMZRkRYcTop\n" + + "aG+5M0x44k6NgIyH7Ap+2vRPpDdUlHs+z+6iRvoutkSfKHeZUYBQjgt+tScfn1hM\n" + + "BRB+x146ecmSVh/Dh8yu6uCrhitFlKpyJqNptZo5o+sH41zjefpMd/bc8rtHTw3n\n" + + "GiFl57ZbXbze2O8UimUVgRI2DtOebt8EAJHM/8vZahzF0chzL4sNVAb8FcNYxAyn\n" + + "95VpnWeAtKX7f0bqUvIN4BNV++o6JdMNvBoYEQpKeQIda7QM59hNiS8f/bxkRikF\n" + + "OiHB5YGy2zRX5T1G5rVQ0YqrOu959eEwdGZmOQ8GOqq5B/NoHXUtotV6SGE3R+Tl\n" + + "grlV4U5/PT0fM3KJATcEHwECACEFAlYmq6wXDIABBK6n7S+CETPlsSjR7QYl3EYy\n" + + "iowCBwAACgkQ+tKr4j1sUtDYFgf+Ob63ulm6/xapl1vwWIFr0IV+FxIqNhqsweMH\n" + + "DsbFZkkx3lVHgMKvhP9lIAZEoFEpDa5euSb0fQ08OnRz9QoIe1AwUOepOTsHrebB\n" + + "JHV3tbSppE9dhDVttq2fOQQM6PR76tfpqKpVEyj+rq4w1QWXpQytRk9I0Hk9VFBY\n" + + "+dDsrXWbDe+DR7nJyolvIW8RHoS3INRcYGjxrn/hsrxUE0ExMAjLoZ4al5Izlvty\n" + + "lCTv+Ph1e6AIkBV+EU1Q6At26ggozi75VwWOTcqb9jE60aLV0uoJtNf/QuIJED2a\n" + + "BCnI1XID5KbjwswtSRtys38XdIdfVqj/mWbX2vwGK7XOJCcuALQiVGVzdHVzZXIg\n" + + "U2V2ZW4gPHRlc3Q3QGV4YW1wbGUuY29tPokBOAQTAQIAIgUCViarcQIbAwYLCQgH\n" + + "AwIGFQgCCQoLBBYCAwECHgECF4AACgkQ+tKr4j1sUtC+NAf/QrtoGwZTZT+ubgj6\n" + + "7649roGcVNhAmqDTfinB0PRZ8zcz9eBWHsRGOdH1lWIIxV6P4reRbl3YM1pe7m9W\n" + + "lwPovqONbCIkcHAshyampCsvRN2gKvgXgUpCcFtwfV828/wD+EsMcHeTcRE+8QUi\n" + + "jUCRW4riz+/vJlgcCjBfSviiTid27/ABzQcjUWQCmQB+HXwkNO66Lnd2QqGeQOnt\n" + + "ed+b8SUBliMHtJ6K/E4P292FWJZLHVWG209mjmFLImsRhonm72Gxc1Q3zkPGAuoK\n" + + "ZLW/yfek3WVVj9BNhe2ChxqRCAcYQbROXprfGk2Sf2KGfIKvzSYcLFdPJRQEZ/KB\n" + + "xx6UH50DmARWJqtxAQgArHBdvocZX1CN6kAPJZbsYzFzYlVnulkzax1/zCuCdCpX\n" + + "YbWtz2ACDBNfiO05jS9IpPW8GrT1ldLeqlgeL9bB+3QqRpf9rt7q8ppsNJ+GKXEL\n" + + "Yu4bPb7np/pQ7OOCDB/mTgsVNC+QNRm6bA+Oh4LaGpA7dD/09bf0Loae+wKvkvQx\n" + + "F07c9tUkzuUlo6QxsbCDKwStaCJlcqXEyki998iwsLgEDGeNdoLLrMe4Fw8w9pEy\n" + + "zuwNEHN65NXbCddJ34hdmB2l3WEXtWS5d6fPA6q8EZLroLcgtbRlMVo62CuWgiRO\n" + + "+1gRwkVa/GpaXYX4lDlXp67ZonccgbKv0vVHwxQDoQARAQABAAf5Ae8xa1mPns1E\n" + + "B5yCrvzDl79Dw0F1rED46IWIW/ghpVTzmFHV6ngcvcRFM5TZquxHXSuxLv7YVxRq\n" + + "UVszXNJaEwyJYYkDRwAS1E2IKN+gknwapm2eWkchySAajUsQt+XEYHFpDPtQRlA3\n" + + "Z6PrCOPJDOLmT9Zcf0R6KurGrhvTGrZkKU6ZCFqZWETfZy5cPfq2qxtw3YEUI+eT\n" + + "09AgMmPJ9nDPI3cA69tvy/phVFgpglsS76qgd6uFJ5kcDoIB+YepmJoHnzJeowYt\n" + + "lvnmmyGqmVS/KCgvILaD0c73Dp2X0BN64hSZHa3nUU67WbKJzo2OXr+yr0hvofcf\n" + + "8vhKJe5+2wQAy+rRKSAOPaFiKT8ZenRucx1pTJLoB8JdediOdR4dtXB2Z59Ze7N3\n" + + "sedfrJn1ao+jJEpnKeudlDq7oa9THd7ZojN4gBF/lz0duzfertuQ/MrHaTPeK8YI\n" + + "dEPg3SgYVOLDBptaKmo0xr2f6aslGLPHgxCgzOcLuuUNGKJSigZvhdMEANh7VKsX\n" + + "nb5shZh+KRET84us/uu74q4iIfc8Q10oXuN9+IPlqfAIclo4uMhvo5rtI9ApFtxs\n" + + "oZzqqc+gt+OAbn/fHeb61eT36BA+r61Ka+erxkpWU5r1BPVIqq+biTY/HHchqroJ\n" + + "aw81qWudO9h5a0yP1alDiBSwhZWIMCKzp6Q7A/472amrSzgs7u8ToQ/2THDxaMf3\n" + + "Se0HgMrIT1/+5es2CWiEoZGSZTXlimDYXJULu/DFC7ia7kXOLrMsO85bEi7SHagA\n" + + "eO+mAw3xP3OuNkZDt9x4qtal28fNIz22DH5qg2wtsGdCWXz5C6OdcrtQ736kNxa2\n" + + "5QemZ/0VWxHPnvXz40RtiQEfBBgBAgAJBQJWJqtxAhsMAAoJEPrSq+I9bFLQzQ8H\n" + + "/1FuTj+k1brv64TWXC1ogbu4WIxsmq7yMroyQPwZ1rJ+WuhYRkDHRejD0cvxRpF9\n" + + "5E9NkysNGmOX58khdPmqeUHQU+bNJcAqvemILyaKAFxunmbqFaFzdJwCpY54y32P\n" + + "RNNYeHiEpLxD6Urf3aG3rlwj+SkWHGb0QRpeZqOmU0v/Ybc9WuV8VSAJ06fYNh1V\n" + + "KAmx8GGRyiLLXsbe+q1v5Z7q6WHOJp7hIiuRRKp1iZGOIXK+pnO2YwSgXoJA0JRv\n" + + "U0rROxIN4h0NpxiNTDKrRCPpNpetZxsLeRbQa1agVM0iJfir1s34HqlrD+zw/tcJ\n" + + "bOdMFF2UVZaCuFynNDx958I=\n" + + "=aoJv\n" + + "-----END PGP PRIVATE KEY BLOCK-----\n"); + } + } From 5ed11440fc10cd13b225db6b59238252f8274d86 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 21 Oct 2015 13:54:25 -0400 Subject: [PATCH 5/7] PublicKeyChecker: Support checking at a given effective date When checking push certificates, we only care about whether the key used for signing was valid at the time the signature was created; it may have since expired or been superseded, and that's ok. Add an optional setter to PublicKeyChecker to tell it to use a different effective date, only considering revocations and expirations prior to that date. There are some cases where we explicitly do not want to respect this effective date: - If a key is compromised, all signatures made with that key are invalid. - Allow after-the-fact web-of-trust assertions, so for example the push certificate stored in a PatchSet can go from OK to TRUSTED simply by adding the proper certification today. Change-Id: I078fd0f4b431af8279948961a99e340f932229b7 --- .../google/gerrit/gpg/PublicKeyChecker.java | 89 +++++-- .../gerrit/gpg/PushCertificateChecker.java | 1 + .../gerrit/gpg/PublicKeyCheckerTest.java | 81 +++++- .../gpg/PushCertificateCheckerTest.java | 25 ++ .../google/gerrit/gpg/testutil/TestKeys.java | 241 ++++++++++++++++++ 5 files changed, 418 insertions(+), 19 deletions(-) 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 3bbad9baf0..e4c81dfd0a 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 @@ -46,6 +46,7 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -64,6 +65,7 @@ public class PublicKeyChecker { private PublicKeyStore store; private Map trusted; private int maxTrustDepth; + private Date effectiveTime = new Date(); /** * Enable web-of-trust checks. @@ -109,6 +111,24 @@ public class PublicKeyChecker { return this; } + /** + * Set the effective time for checking the key. + *

+ * If set, check whether the key should be considered valid (e.g. unexpired) + * as of this time. + * + * @param effectiveTime effective time. + * @return a reference to this object. + */ + public PublicKeyChecker setEffectiveTime(Date effectiveTime) { + this.effectiveTime = effectiveTime; + return this; + } + + protected Date getEffectiveTime() { + return effectiveTime; + } + /** * Check a public key. * @@ -141,7 +161,7 @@ public class PublicKeyChecker { private CheckResult check(PGPPublicKey key, int depth, boolean expand, Set seen) { - CheckResult basicResult = checkBasic(key); + CheckResult basicResult = checkBasic(key, effectiveTime); CheckResult customResult = checkCustom(key, depth); CheckResult trustResult = checkWebOfTrust(key, store, depth, seen); if (!expand && !trustResult.isTrusted()) { @@ -176,28 +196,32 @@ public class PublicKeyChecker { return CheckResult.create(status, problems); } - private CheckResult checkBasic(PGPPublicKey key) { + private CheckResult checkBasic(PGPPublicKey key, Date now) { List problems = new ArrayList<>(2); - gatherRevocationProblems(key, problems); + gatherRevocationProblems(key, now, problems); - long validSecs = key.getValidSeconds(); - if (validSecs != 0) { - long createdSecs = key.getCreationTime().getTime() / 1000; - long nowSecs = System.currentTimeMillis() / 1000; - if (nowSecs - createdSecs > validSecs) { + long validMs = key.getValidSeconds() * 1000; + if (validMs != 0) { + long msSinceCreation = now.getTime() - key.getCreationTime().getTime(); + if (msSinceCreation > validMs) { problems.add("Key is expired"); } } return CheckResult.create(problems); } - private void gatherRevocationProblems(PGPPublicKey key, List problems) { + private void gatherRevocationProblems(PGPPublicKey key, Date now, + List problems) { try { List revocations = new ArrayList<>(); Map revokers = new HashMap<>(); - PGPSignature selfRevocation = scanRevocations(key, revocations, revokers); + PGPSignature selfRevocation = + scanRevocations(key, now, revocations, revokers); if (selfRevocation != null) { - problems.add(reasonToString(getRevocationReason(selfRevocation))); + RevocationReason reason = getRevocationReason(selfRevocation); + if (isRevocationValid(selfRevocation, reason, now)) { + problems.add(reasonToString(reason)); + } } else { checkRevocations(key, revocations, revokers, problems); } @@ -206,7 +230,21 @@ public class PublicKeyChecker { } } - private PGPSignature scanRevocations(PGPPublicKey key, + private static boolean isRevocationValid(PGPSignature revocation, + RevocationReason reason, Date now) { + // RFC4880 states: + // "If a key has been revoked because of a compromise, all signatures + // created by that key are suspect. However, if it was merely superseded or + // retired, old signatures are still valid." + // + // Note that GnuPG does not implement this correctly, as it does not + // consider the revocation reason and timestamp when checking whether a + // signature (data or certification) is valid. + return reason.getRevocationReason() == KEY_COMPROMISED + || revocation.getCreationTime().before(now); + } + + private PGPSignature scanRevocations(PGPPublicKey key, Date now, List revocations, Map revokers) throws PGPException { @SuppressWarnings("unchecked") @@ -221,7 +259,10 @@ public class PublicKeyChecker { return sig; } } else { - revocations.add(sig); + RevocationReason reason = getRevocationReason(sig); + if (reason != null && isRevocationValid(sig, reason, now)) { + revocations.add(sig); + } } break; case DIRECT_KEY: @@ -263,9 +304,8 @@ public class PublicKeyChecker { continue; // Not a designated revoker. } byte[] rfp = revoker.getFingerprint(); - PGPPublicKeyRing rkr = store.get(rfp); - if (rkr == null - || rkr.getPublicKey().getAlgorithm() != revoker.getAlgorithm()) { + PGPPublicKeyRing revokerKeyRing = store.get(rfp); + if (revokerKeyRing == null) { // Revoker is authorized and there is a revocation signature by this // revoker, but the key is not in the store so we can't verify the // signature. @@ -275,8 +315,16 @@ public class PublicKeyChecker { problems.add(reasonToString(getRevocationReason(revocation))); continue; } - revocation.init( - new BcPGPContentVerifierBuilderProvider(), rkr.getPublicKey()); + PGPPublicKey rk = revokerKeyRing.getPublicKey(); + if (rk.getAlgorithm() != revoker.getAlgorithm()) { + continue; + } + if (!checkBasic(rk, revocation.getCreationTime()).isOk()) { + // Revoker's key was expired or revoked at time of revocation, so the + // revocation is invalid. + continue; + } + revocation.init(new BcPGPContentVerifierBuilderProvider(), rk); if (revocation.verifyCertification(key)) { problems.add(reasonToString(getRevocationReason(revocation))); } @@ -353,8 +401,13 @@ public class PublicKeyChecker { Iterator userIds = key.getUserIDs(); while (userIds.hasNext()) { String userId = userIds.next(); + + // Don't check the timestamp of these certifications. This allows admins + // to correct untrusted keys by signing them with a trusted key, such that + // older signatures created by those keys retroactively appear valid. @SuppressWarnings("unchecked") Iterator sigs = key.getSignaturesForID(userId); + while (sigs.hasNext()) { PGPSignature sig = sigs.next(); // TODO(dborowitz): Handle CERTIFICATION_REVOCATION. 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 e87a0ee6dd..0a0fff769f 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 @@ -210,6 +210,7 @@ public abstract class PushCertificateChecker { } CheckResult result = publicKeyChecker .setStore(store) + .setEffectiveTime(sig.getCreationTime()) .check(signer); if (!result.getProblems().isEmpty()) { StringBuilder err = new StringBuilder("Invalid public key ") 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 a611ec9484..576ffbd488 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 @@ -16,6 +16,8 @@ package com.google.gerrit.gpg; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; import static com.google.gerrit.gpg.testutil.TestKeys.expiredKey; +import static com.google.gerrit.gpg.testutil.TestKeys.keyRevokedByExpiredKeyAfterExpiration; +import static com.google.gerrit.gpg.testutil.TestKeys.keyRevokedByExpiredKeyBeforeExpiration; import static com.google.gerrit.gpg.testutil.TestKeys.revokedCompromisedKey; import static com.google.gerrit.gpg.testutil.TestKeys.revokedNoLongerUsedKey; import static com.google.gerrit.gpg.testutil.TestKeys.selfRevokedKey; @@ -51,7 +53,9 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import java.text.SimpleDateFormat; import java.util.Arrays; +import java.util.Date; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -88,7 +92,17 @@ public class PublicKeyCheckerTest { @Test public void keyExpiringInFuture() throws Exception { - assertProblems(validKeyWithExpiration()); + TestKey k = validKeyWithExpiration(); + + PublicKeyChecker checker = new PublicKeyChecker() + .setStore(store); + assertProblems(checker, k); + + checker.setEffectiveTime(parseDate("2015-07-10 12:00:00 -0400")); + assertProblems(checker, k); + + checker.setEffectiveTime(parseDate("2075-07-10 12:00:00 -0400")); + assertProblems(checker, k, "Key is expired"); } @Test @@ -201,6 +215,24 @@ public class PublicKeyCheckerTest { assertProblems(kr.getPublicKey()); } + @Test + public void revokedKeyDueToCompromiseRevokesKeyRetroactively() + throws Exception { + TestKey k = add(revokedCompromisedKey()); + add(validKeyWithoutExpiration()); + save(); + + String problem = + "Key is revoked (key material has been compromised): test6 compromised"; + assertProblems(k, problem); + + SimpleDateFormat df = new SimpleDateFormat("YYYY-MM-dd HH:mm:ss"); + PublicKeyChecker checker = new PublicKeyChecker() + .setStore(store) + .setEffectiveTime(df.parse("2010-01-01 12:00:00")); + assertProblems(checker, k, problem); + } + @Test public void revokedByKeyNotPresentInStore() throws Exception { TestKey k = add(revokedCompromisedKey()); @@ -221,6 +253,49 @@ public class PublicKeyCheckerTest { "Key is revoked (retired and no longer valid): test7 not used"); } + @Test + public void revokedKeyDueToNoLongerBeingUsedDoesNotRevokeKeyRetroactively() + throws Exception { + TestKey k = add(revokedNoLongerUsedKey()); + add(validKeyWithoutExpiration()); + save(); + + assertProblems(k, + "Key is revoked (retired and no longer valid): test7 not used"); + + PublicKeyChecker checker = new PublicKeyChecker() + .setStore(store) + .setEffectiveTime(parseDate("2010-01-01 12:00:00 -0400")); + assertProblems(checker, k); + } + + @Test + public void keyRevokedByExpiredKeyAfterExpirationIsNotRevoked() + throws Exception { + TestKey k = add(keyRevokedByExpiredKeyAfterExpiration()); + add(expiredKey()); + save(); + + PublicKeyChecker checker = new PublicKeyChecker().setStore(store); + assertProblems(checker, k); + } + + @Test + public void keyRevokedByExpiredKeyBeforeExpirationIsRevoked() + throws Exception { + TestKey k = add(keyRevokedByExpiredKeyBeforeExpiration()); + add(expiredKey()); + save(); + + PublicKeyChecker checker = new PublicKeyChecker().setStore(store); + assertProblems(checker, k, + "Key is revoked (retired and no longer valid): test9 not used"); + + // Set time between key creation and revocation. + checker.setEffectiveTime(parseDate("2005-08-01 13:00:00 -0400")); + assertProblems(checker, k); + } + private PGPPublicKeyRing removeRevokers(PGPPublicKeyRing kr) { PGPPublicKey k = kr.getPublicKey(); @SuppressWarnings("unchecked") @@ -288,4 +363,8 @@ public class PublicKeyCheckerTest { return "Certification by " + keyToString(k.getPublicKey()) + " is valid, but key is not trusted"; } + + private static Date parseDate(String str) throws Exception { + return new SimpleDateFormat("YYYY-MM-dd HH:mm:ss Z").parse(str); + } } 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 9b1c058c2e..977895859d 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 @@ -28,6 +28,7 @@ import org.bouncycastle.bcpg.ArmoredOutputStream; import org.bouncycastle.bcpg.BCPGOutputStream; import org.bouncycastle.openpgp.PGPSignature; import org.bouncycastle.openpgp.PGPSignatureGenerator; +import org.bouncycastle.openpgp.PGPSignatureSubpacketGenerator; import org.bouncycastle.openpgp.PGPUtil; import org.bouncycastle.openpgp.operator.bc.BcPGPContentSignerBuilder; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; @@ -47,7 +48,9 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.InputStreamReader; import java.io.Reader; +import java.text.SimpleDateFormat; import java.util.Arrays; +import java.util.Date; public class PushCertificateCheckerTest { private InMemoryRepository repo; @@ -130,6 +133,15 @@ public class PushCertificateCheckerTest { + ":\n Key is expired"); } + @Test + public void signatureByExpiredKeyBeforeExpiration() throws Exception { + TestKey key3 = expiredKey(); + Date now = new SimpleDateFormat("YYYY-MM-dd HH:mm:ss Z") + .parse("2005-07-10 12:00:00 -0400"); + PushCertificate cert = newSignedCert(validNonce(), key3, now); + assertProblems(cert); + } + private String validNonce() { return signedPushConfig.getNonceGenerator() .createNonce(repo, System.currentTimeMillis() / 1000); @@ -137,6 +149,11 @@ public class PushCertificateCheckerTest { private PushCertificate newSignedCert(String nonce, TestKey signingKey) throws Exception { + return newSignedCert(nonce, signingKey, null); + } + + private PushCertificate newSignedCert(String nonce, TestKey signingKey, + Date now) throws Exception { PushCertificateIdent ident = new PushCertificateIdent( signingKey.getFirstUserId(), System.currentTimeMillis(), -7 * 60); String payload = "certificate version 0.1\n" @@ -150,6 +167,14 @@ public class PushCertificateCheckerTest { PGPSignatureGenerator gen = new PGPSignatureGenerator( new BcPGPContentSignerBuilder( signingKey.getPublicKey().getAlgorithm(), PGPUtil.SHA1)); + + if (now != null) { + PGPSignatureSubpacketGenerator subGen = + new PGPSignatureSubpacketGenerator(); + subGen.setSignatureCreationTime(false, now); + gen.setHashedSubpackets(subGen.generate()); + } + gen.init(PGPSignature.BINARY_DOCUMENT, signingKey.getPrivateKey()); gen.update(payload.getBytes(UTF_8)); PGPSignature sig = gen.generate(); diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java index 97a94b90d2..ad944c51d7 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/testutil/TestKeys.java @@ -784,4 +784,245 @@ public class TestKeys { + "-----END PGP PRIVATE KEY BLOCK-----\n"); } + /** + * Key revoked by an expired key, after that key's expiration. + *

+ * Revoked by {@link #expiredKey()}. + * + * 

+   * pub   2048R/78BF7D7E 2005-08-01 [revoked: 2015-10-20]
+   *       Key fingerprint = 916F AB22 5BE7 7585 F59A  994C 001A DF8B 78BF 7D7E
+   * uid                  Testuser Eight <test8@example.com>
+   *
+ */ + public static TestKey keyRevokedByExpiredKeyAfterExpiration() throws Exception { + return new TestKey("-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + + "Version: GnuPG v1\n" + + "\n" + + "mQENBELuRwABCAC56yhFKybBtuKT4nyb7RdLE98pZR54aGjcDcKH3VKVyBF8Z4Kx\n" + + "ptd7Sre0mLPCQiNWVOmCT+JG7GKVE6YeFmyXDUnhX9w4+HAeDEh23S4u9JvwWaF+\n" + + "wlJ6jLq/oe5gdT1F6Y2yqNpQ6CztOw52Ko9KSYz7/1zBMPcCkl/4k15ee4iebVdq\n" + + "c7qT5Qt49Poiozh0DI5prPQ624uckHkz2mXshjWQVuHWwrkIkCJZ2I/KQN2kBjKw\n" + + "/ALxumaWmiB9lQ0nIwLuGzHCh0Xg5RxuCrK8fJp47Aza3ikVuYlNzSxhJVav3OtK\n" + + "gftBihQXUlY3Uy/4QTCeH/BdVs5OALtXL3VhABEBAAGJAS0EIAECABcFAlYmr4kQ\n" + + "HQN0ZXN0OCBub3QgdXNlZAAKCRA87HgbF94azQJ5B/0TeQk7TSChNp+NqCKPTuw0\n" + + "wpflDyc+5ru/Gcs4r358cWzgiLUb3M0Q1+M8CF13BFQdrxT05vjheI9o5PCn3b//\n" + + "AHV8m+QFSnRi2J3QslbvuOqOnipz7vc7lyZ7q1sWNC33YN+ZcGZiMuu5HJi9iadf\n" + + "ZL7AdInpUb4Zb+XKphbMokDcN3yw7rqSMMcx+rKytUAqUnt9qvaSLrIH/zeazxlp\n" + + "YG4jaN53WPfLCcGG+Rw56mW+eCQD2rmzaNHCw8Qr+19sokXLB7OML+rd1wNwZT4q\n" + + "stWnL+nOj8ZkbFV0w3zClDYaARr7H+vTckwVStyDVRbnpRitSAtJwbRDzZBaS4Vx\n" + + "iQE3BB8BAgAhBQJC7lUQFwyAAR2e63ndOLBJk52crzzseBsX3hrNAgcAAAoJEAAa\n" + + "34t4v31+AS4H/0x3Y9E3q9DR5FCuYTXG4BHyrALo2WKoP0CfUWL98Fw9Txl0hF+9\n" + + "5wriNlnmd2zvM0quHs78x4/xehQO88cw0lqPx3RARq/ju5/VbOjoNlcHvfGYZiEd\n" + + "yWOwHu7O8sZrenFDjeDglD6NArrjncOcC51XIPSSTLvVQpSauQ1FS4tan5Q4aWMb\n" + + "s4DzE+Vqu2xMkO/X9toYAZKzyWP29OckpouMbt3GUnS6/o0A8Z7jVX+XOIk3XolP\n" + + "Li9tzTQB12Xl23mgFvearDoguR2Bu2SbmTJtdiXz8L3S54kGvxVqak5uOP2dagzU\n" + + "vBiqR4SVoAdGoXt6TI6mpA+qdYmPMG8v21S0IlRlc3R1c2VyIEVpZ2h0IDx0ZXN0\n" + + "OEBleGFtcGxlLmNvbT6JATgEEwECACIFAkLuRwACGwMGCwkIBwMCBhUIAgkKCwQW\n" + + "AgMBAh4BAheAAAoJEAAa34t4v31+8/sIAIuqd+dU8k9c5VQ12k7IfZGGYQHF2Mk/\n" + + "8FNuP7hFP/VOXBK3QIxIfGEOHbDX6uIxudYMaDmn2UJbdIqJd8NuQByh1gqXdX/x\n" + + "nteUa+4e7U6uTjkp/Ij5UzRed8suINA3NzVOy6qwCu3DTOXIZcjiOZtOA5GTqG6Z\n" + + "naDP0hwDssJp+LXIYTJgsvneJQFGSdQhhJSv19oV0JPSbb6Zc7gEIHtPcaJHjuZQ\n" + + "Ev+TRcRrI9HPTF0MvgOYgIDo2sbcSFV+8moKsHMC+j1Hmuuqgm/1yKGIZrt0V75s\n" + + "D9HYu0tiS3+Wlsry3y1hg/2XBQbwgh6sT/jWkpWar7+uzNxO5GdFYrC5AQ0EQu5H\n" + + "AAEIALPFTedbfyK+9B35Uo9cPsmFa3mT3qp/bAQtnOjiTTTiIO3tu0ALnaBjf6On\n" + + "fAV1HmGz6hRMRK4LGyHkNTaGDNNPoXO7+t9DWycSHmsCL5d5zp7VevQE8MPR8zHK\n" + + "Il2YQlCzdy5TWSUhunKd4guDNZ9GiOS6NQ9feYZ9DQ1kzC8nnu7jLkR2zNT02sYU\n" + + "kuOCZUktQhVNszUlavdIFjvToZo3RPcdb/E3kTTy2R9xi89AXjWZf3lSAZe3igkL\n" + + "jhwsd+u3RRx0ptOJym7zYl5ZdUZk4QrS7FPI6zEBpjawbS4/r6uEW89P3QAkanDI\n" + + "ridIAZP8awLZU3uSPtMwPIJpao0AEQEAAYkBHwQYAQIACQUCQu5HAAIbDAAKCRAA\n" + + "Gt+LeL99fqpHB/wOXhdMNtgeVW38bLk8YhcEB23FW6fDjFjBJb9m/yqRTh5CIeG2\n" + + "bm29ofT4PTamPb8Gt+YuDLnQQ3K2jURakxNDcYwiurvR/oHVdxsBRU7Px7UPeZk3\n" + + "BG5VnIJRT198dF7MWFJ+x5wHbNXwM8DDvUwTjXLH/TlGl1XIheSTHCYd9Pra4ejE\n" + + "ockkrDaZlPCQdTwY+P7K2ieb5tsqNpJkQeBrglF2bemY/CtQHnM9qwa6ZJqkyYNR\n" + + "F1nkSYn36BPuNpytYw1CaQV9GbePugPHtshECLwA160QzqISQUcJlKXttUqUGnoO\n" + + "0d0PyzZT3676mQwmFoebMR9vACAeHjvDxD4F\n" + + "=ihWb\n" + + "-----END PGP PUBLIC KEY BLOCK-----\n", + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: GnuPG v1\n" + + "\n" + + "lQOYBELuRwABCAC56yhFKybBtuKT4nyb7RdLE98pZR54aGjcDcKH3VKVyBF8Z4Kx\n" + + "ptd7Sre0mLPCQiNWVOmCT+JG7GKVE6YeFmyXDUnhX9w4+HAeDEh23S4u9JvwWaF+\n" + + "wlJ6jLq/oe5gdT1F6Y2yqNpQ6CztOw52Ko9KSYz7/1zBMPcCkl/4k15ee4iebVdq\n" + + "c7qT5Qt49Poiozh0DI5prPQ624uckHkz2mXshjWQVuHWwrkIkCJZ2I/KQN2kBjKw\n" + + "/ALxumaWmiB9lQ0nIwLuGzHCh0Xg5RxuCrK8fJp47Aza3ikVuYlNzSxhJVav3OtK\n" + + "gftBihQXUlY3Uy/4QTCeH/BdVs5OALtXL3VhABEBAAEAB/wLr88oGuxsoqIHRQZL\n" + + "eGm9jc4aQGmcDMcjpwdGilhrwyfrO6f84hWbQdD+rJcnI8hsH7oOd5ZMGkWfpJyt\n" + + "eUAh9iNB5ChYGfDVSLUg6KojqDtprj6vNMihvLkr/OI6xL/hZksikwfnLFMPpgXU\n" + + "knwPocQ3nn+egsUSL7CR8/SLiIm4MC0brer6jhDxB5LKweExNlfTe4c0MDeYTsWt\n" + + "0WGzNPlvRZQXRotJzqemt3wdNZXUnCKR0n7pSQ8EhZr2O6NXr+mUgp6PIOE/3un2\n" + + "YGiBEf5uy3qEFe7FjEGIHz+Z3ySRdUDfHOk82TKAzynoJIxRUvLIYVNw4eFB3l5U\n" + + "s1w5BADUzfciG7RVLa8UFKJfqQ/5M06QmdS1v1/hMQXg38+3vKe8RgfSSnMJ08Sc\n" + + "eAEsmugwpNXAxgRKHcmWzN3NMBHhE3KiyiogWaMGqmSo6swFpu0+dwMvZSxMlfD+\n" + + "ka/BWt8YsUdrqW06ow39aTgCV+icbNRV81C7NKe7u0X1JDx2CQQA36gbdo62h/Wd\n" + + "gJI8kdz/se3xrt8x6RoWvOnWPNmsZR5XkDqAMTL1dWiEEA/dQTphMcgAe9z3WaP+\n" + + "F1TPAfounbiurGCcS3kxJ5tY7ojyU7nYz4DA/V2OU0C/LUoLXhttG5HM+m/i3qn4\n" + + "K9bBoWIQY1ijliS7cTSwNqd6IHaQGpkEAMnp5GwSGhY+kUuLw06hmH4xnsuf6agz\n" + + "AfhbPylB2nf/ZaX6dt6/mFEAkvQNahcoWEskfS3LGCD8jHm8PvF8K0mciXPDweq2\n" + + "gW3/irE0RXNwn3Oa222VSvcgUlocBm9InkfvpFXh20OYFe3dFH7uYkwUqIHJeXjw\n" + + "TjpXUX/vC5QJQOyJATcEHwECACEFAkLuVRAXDIABHZ7red04sEmTnZyvPOx4Gxfe\n" + + "Gs0CBwAACgkQABrfi3i/fX4BLgf/THdj0Ter0NHkUK5hNcbgEfKsAujZYqg/QJ9R\n" + + "Yv3wXD1PGXSEX73nCuI2WeZ3bO8zSq4ezvzHj/F6FA7zxzDSWo/HdEBGr+O7n9Vs\n" + + "6Og2Vwe98ZhmIR3JY7Ae7s7yxmt6cUON4OCUPo0CuuOdw5wLnVcg9JJMu9VClJq5\n" + + "DUVLi1qflDhpYxuzgPMT5Wq7bEyQ79f22hgBkrPJY/b05ySmi4xu3cZSdLr+jQDx\n" + + "nuNVf5c4iTdeiU8uL23NNAHXZeXbeaAW95qsOiC5HYG7ZJuZMm12JfPwvdLniQa/\n" + + "FWpqTm44/Z1qDNS8GKpHhJWgB0ahe3pMjqakD6p1iY8wby/bVLQiVGVzdHVzZXIg\n" + + "RWlnaHQgPHRlc3Q4QGV4YW1wbGUuY29tPokBOAQTAQIAIgUCQu5HAAIbAwYLCQgH\n" + + "AwIGFQgCCQoLBBYCAwECHgECF4AACgkQABrfi3i/fX7z+wgAi6p351TyT1zlVDXa\n" + + "Tsh9kYZhAcXYyT/wU24/uEU/9U5cErdAjEh8YQ4dsNfq4jG51gxoOafZQlt0iol3\n" + + "w25AHKHWCpd1f/Ge15Rr7h7tTq5OOSn8iPlTNF53yy4g0Dc3NU7LqrAK7cNM5chl\n" + + "yOI5m04DkZOobpmdoM/SHAOywmn4tchhMmCy+d4lAUZJ1CGElK/X2hXQk9Jtvplz\n" + + "uAQge09xokeO5lAS/5NFxGsj0c9MXQy+A5iAgOjaxtxIVX7yagqwcwL6PUea66qC\n" + + "b/XIoYhmu3RXvmwP0di7S2JLf5aWyvLfLWGD/ZcFBvCCHqxP+NaSlZqvv67M3E7k\n" + + "Z0VisJ0DmARC7kcAAQgAs8VN51t/Ir70HflSj1w+yYVreZPeqn9sBC2c6OJNNOIg\n" + + "7e27QAudoGN/o6d8BXUeYbPqFExErgsbIeQ1NoYM00+hc7v630NbJxIeawIvl3nO\n" + + "ntV69ATww9HzMcoiXZhCULN3LlNZJSG6cp3iC4M1n0aI5Lo1D195hn0NDWTMLyee\n" + + "7uMuRHbM1PTaxhSS44JlSS1CFU2zNSVq90gWO9OhmjdE9x1v8TeRNPLZH3GLz0Be\n" + + "NZl/eVIBl7eKCQuOHCx367dFHHSm04nKbvNiXll1RmThCtLsU8jrMQGmNrBtLj+v\n" + + "q4Rbz0/dACRqcMiuJ0gBk/xrAtlTe5I+0zA8gmlqjQARAQABAAf+JNVkZOcGYaQm\n" + + "eI3BMMaBxuCjaMG3ec+p3iFKaR0VHKTIgneXSkQXA+nfGTUT4DpjAznN2GLYH6D+\n" + + "6i7MCGPm9NT4C7KUcHJoltTLjrlf7vVyNHEhRCZO/pBh9+2mpO6xh799x+wj88u5\n" + + "XAqlah50OjJFkjfk70VsrPWqWvgwLejkaQpGbE+pdL+vjy+ol5FHzidzmJvsXDR1\n" + + "I1as0vBu5g2XPpexyVanmHJglZdZX07OPYQBhxQKuPXT/2/IRnXsXEpitk4IyJT0\n" + + "U5D/iedEUldhBByep1lBcJnAap0CP7iuu2CYhRp6V2wVvdweNPng5Eo7f7LNyjnX\n" + + "UMAeaeCjAQQA1A0iKtg3Grxc9+lpFl1znc2/kO3p6ixM13uUvci+yGFNJJninnxo\n" + + "99KXEzqqVD0zerjiyyegQmzpITE/+hFIOJZInxEH08WQwZstV/KYeRSJkXf0Um48\n" + + "E+Zrh8fpJVW1w3ZCw9Ee2yE6fEhAA4w66+50pM+vBXanWOrG1HDrkxEEANkHc2Rz\n" + + "YJsO4v63xo/7/njLSQ31miOglb99ACKBA0Yl/jvj2KqLcomKILqvK3DKP+BHNq86\n" + + "LUBUglyKjKuj0wkSWT0tCnfgLzysUpowcoyFhJ36KzAz8hjqIn3TQpMF21HvkZdG\n" + + "Mtkcyhu5UDvbfOuWOBaKIeNQWCWv1rNzMme9A/9zU1+esEhKwGWEqa3/B/Te/xQh\n" + + "alk180n74sTZid6lXD8o8cEei0CUq7zBSV0P8v6kk8PP9/XyLRl3Rqa95fESUWrL\n" + + "xD6TBY1JlHBZS+N6rN/7Ilf5EXSELmnbDFsVxkNGp4elKxajvZxC6uEWYBu62AYy\n" + + "wS0dj8mZR3faCEps90YXiQEfBBgBAgAJBQJC7kcAAhsMAAoJEAAa34t4v31+qkcH\n" + + "/A5eF0w22B5VbfxsuTxiFwQHbcVbp8OMWMElv2b/KpFOHkIh4bZubb2h9Pg9NqY9\n" + + "vwa35i4MudBDcraNRFqTE0NxjCK6u9H+gdV3GwFFTs/HtQ95mTcEblWcglFPX3x0\n" + + "XsxYUn7HnAds1fAzwMO9TBONcsf9OUaXVciF5JMcJh30+trh6MShySSsNpmU8JB1\n" + + "PBj4/sraJ5vm2yo2kmRB4GuCUXZt6Zj8K1Aecz2rBrpkmqTJg1EXWeRJiffoE+42\n" + + "nK1jDUJpBX0Zt4+6A8e2yEQIvADXrRDOohJBRwmUpe21SpQaeg7R3Q/LNlPfrvqZ\n" + + "DCYWh5sxH28AIB4eO8PEPgU=\n" + + "=cSfw\n" + + "-----END PGP PRIVATE KEY BLOCK-----\n"); + } + + /** + * Key revoked by an expired key, before that key's expiration. + *

+ * Revoked by {@link #expiredKey()}. + * + * 

+   * pub   2048R/C43BF2E1 2005-08-01 [revoked: 2005-08-01]
+   *       Key fingerprint = 916D 6AD6 36A5 CBA6 B5A6  7274 6040 8661 C43B F2E1
+   * uid                  Testuser Nine <test9@example.com>
+   *
+ */ + public static TestKey keyRevokedByExpiredKeyBeforeExpiration() throws Exception { + return new TestKey("-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + + "Version: GnuPG v1\n" + + "\n" + + "mQENBELuRwABCADnf2z5dqp3BMFlpd6iUs5dhROrslfzswak1LmbGirK2IPIl4NX\n" + + "arAi76xXK9BcF/Cqcj/X/WqFKBd/qMGxwdvwbSN6PVBP6T1jvuVgrPTjd4x5xPUD\n" + + "xZ5VPy9hgQXs+1mugTkHYVTU8GI1eGpZ8Oj3PJIgVyqGxGkjWmcz5APbVIRan6L1\n" + + "482bZTidH9Nd9YnYlXNgiJcaOPAVBwO/j/myocQCIohvIo4IT8vc/ODhRgfwA0gD\n" + + "GVK+tXwT4f4x3qjG/YRpOOZZjBS09B/gJ9QfEnR6WNxg/Tm3T0uipoISOhR+cP/V\n" + + "e5o/73SM+w+WlILk/xpbbOfyCxD4Q3lb8EZFABEBAAGJAS0EIAECABcFAkLuYyAQ\n" + + "HQN0ZXN0OSBub3QgdXNlZAAKCRA87HgbF94azV2BB/9Rc1j3XOxKbDyUFAORAGnE\n" + + "ezQtpOmQhaSUhFC35GFOdTg4eX53FTFSXLJQleTVzvE+eVkQI5tvUZ+SqHoyjnhU\n" + + "DpWlmfRUQy4GTUjUTkpFOK07TVTjhUQwaAxN13UZgByopVKc7hLf+uh1xkRJIqAJ\n" + + "Tx6LIFZiSIGwStDO6TJlhl1e8h45J3rAV4N+DsGpMy9S4uYOU7erJDupdXK739/l\n" + + "VBsP2SeT85iuAv+4A9Jq3+iq+cjK9q3QZCw1O6iI2v3seAWCI6HH3tVw4THr+M6T\n" + + "EdTGmyESjdAl+f7/uK0QNfqIMpvUf+AvMakrLi7WOeDs8mpUIjonpeQVLfz6I0Zo\n" + + "iQE3BB8BAgAhBQJC7lUQFwyAAR2e63ndOLBJk52crzzseBsX3hrNAgcAAAoJEGBA\n" + + "hmHEO/LhHjUH/R/7+iNBLAfKYbpprkWy/8eXVEJhxfh6DI/ppsKLIA+687gX74R9\n" + + "6CM5k6fZDjeND26ZEA0rDZmYrbnGUfsu55aeM0/+jiSOZJ2uTlrLXiHMurbNY0pT\n" + + "xv215muhumPBzuL1jsAK2Kc/4oE7Z46jaStsPCvDOcx9PW76wR8/uCPvHVz5H/A7\n" + + "3erXAloC43jupXwZB32VZq8L0kZNVfuEsjHUcu3GUoZdGfTb4/Qq5a1FK+CGhwWC\n" + + "OwpUWZEIUImwUv4FNE4iNFYEHaHLU9fotmIxIkH8TC4NcO+GvkEyMyJ6NVkBBDP2\n" + + "EarncWAJxDBlx1CO4ET+/ULvzDnAcYuTc6G0IVRlc3R1c2VyIE5pbmUgPHRlc3Q5\n" + + "QGV4YW1wbGUuY29tPokBOAQTAQIAIgUCQu5HAAIbAwYLCQgHAwIGFQgCCQoLBBYC\n" + + "AwECHgECF4AACgkQYECGYcQ78uG78ggA1TjeOZtaXjXNG8Bx2sl4W+ypylWWB6yc\n" + + "IeR0suLhVlisZ33yOtV4MsvZw0TJNyYmFXiskPTyOcP8RJjS+a41IHc33i13MUnN\n" + + "RI5cqhqsWRhf9chlm7XqXtqv57IjojG9vgSUeZdXSTMdHIDDHAjJ/ryBXflzprSw\n" + + "2Sab8OXjLkyo9z6ZytFyfXSc8TNiWU6Duollh/bWIsgPETIe2wGn8LcFiVMfPpsI\n" + + "RhkphOdTJb+W/zQwLHUcS22A4xsJtBxIXTH/QSG3lAaw8IRbl25EIpaEAF+gExCr\n" + + "QM0haAVMmGgYYWpMHXrDhB7ff3kAiqD2qmhSySA6NLmTO+6qGPYJg7kBDQRC7kcA\n" + + "AQgA2wqE3DypQhTcYl26dXc9DZzABRQa6KFRqQbhmUBz95cQpAamQjrwOyl2fg84\n" + + "b9o9t+DuZcdLzLF/gPVSznOcNUV9mJNdLAxBPPOMUrP/+Snb83FkNpCscrXhIqSf\n" + + "BU5D+FOb3bEI2WTJ7lLe8oCrWPE3JIDVCrpAWgZk9puAk1Z7ZFaHsS6ezsZP0YIM\n" + + "qTWdoX0zHMPMnr9GG08c0mniXtvfcgtOCeIRU4WZws28sGYCoLeQXsHVDal+gcLp\n" + + "1enPh6dfEWBJuhhBBajzm53fzV2a7khEdffggVVylHPLpvms2nIqoearDQtVNpSK\n" + + "uhNiykJSMIUn/Y6g5LMySmL+MwARAQABiQEfBBgBAgAJBQJC7kcAAhsMAAoJEGBA\n" + + "hmHEO/LhdwcH/0wAxT1NGaR2boMjpTouVUcnEcEzHc0dSwuu+06mLRggSdAfBC8C\n" + + "9fdlAYHQ5tp1sRuPwLfQZjo8wLxJ+wLASnIPLaGrtpEHkIKvDwHqwkOXvXeGD/Bh\n" + + "40NbJUa7Ec3Jpo+FPFlM8hDsUyHf8IhUAdRd4d+znOVEaZ6S7c1RrtoVTUqzi59n\n" + + "nC6ZewL/Jp+znKZlMTM3X1onAGhd+/XdrS52LM8pE3xRjbTLTYWcjnjyLbm0yoO8\n" + + "G3yCfIibAaII4a/jGON2X9ZUwaFNIqJ4iIc8Nme86rD/flXsu6Zv+NXVQWylrIG/\n" + + "REW68wsnWjwTtrPG8bqo6cCsOzqGYVt81eU=\n" + + "=FnZg\n" + + "-----END PGP PUBLIC KEY BLOCK-----\n", + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: GnuPG v1\n" + + "\n" + + "lQOYBELuRwABCADnf2z5dqp3BMFlpd6iUs5dhROrslfzswak1LmbGirK2IPIl4NX\n" + + "arAi76xXK9BcF/Cqcj/X/WqFKBd/qMGxwdvwbSN6PVBP6T1jvuVgrPTjd4x5xPUD\n" + + "xZ5VPy9hgQXs+1mugTkHYVTU8GI1eGpZ8Oj3PJIgVyqGxGkjWmcz5APbVIRan6L1\n" + + "482bZTidH9Nd9YnYlXNgiJcaOPAVBwO/j/myocQCIohvIo4IT8vc/ODhRgfwA0gD\n" + + "GVK+tXwT4f4x3qjG/YRpOOZZjBS09B/gJ9QfEnR6WNxg/Tm3T0uipoISOhR+cP/V\n" + + "e5o/73SM+w+WlILk/xpbbOfyCxD4Q3lb8EZFABEBAAEAB/9GTcWLkUU9tf0B4LjX\n" + + "NSyk7ChIKXZadVEcN9pSR0Udq1mCTrk9kBID2iPNqWmyvjaBnQbUkoqJ+93/EAIa\n" + + "+NPRlWOD2SEN07ioFS5WCNCqUAEibfU2+woVu4WpJ+TjzoWy4F2wZxe7P3Gj6Xjq\n" + + "7aXih8uc9Lveh8GiUe8rrCCbt+BH1RzuV/khZw+2ZDPMCx7yfcfKobc3NWx75WLh\n" + + "pki512fawSC6eJHRI50ilPrqAmmhcccfwPji9P+oPj2S6wlhe5kp3R5yU85fWy3b\n" + + "C8AtLTfZIn4v6NAtBaurGEjRjzeNEGMJHxnRPWvFc4iD+xvPg6SNPJM/bbTE+yZ3\n" + + "16W1BADxjAQLMuGpemaVmOpZ3K02hcNjwniEK2QPp11BnfoQCIwegON+sUD/6AuZ\n" + + "S1vOVvS3//eGbPaMM45FK/SQAVHpC9IOL4Tql0C8B6csRhFL824yPfc3WDb4kayQ\n" + + "T5oLjlJ0W2r7tWcBcREEzZT6gNi4KI7C4oFF6tU9lsQJuQyAbwQA9Vl6VW/7oG0W\n" + + "CC+lcHJc+4rxUB3yak7d4mEccTNb+crOBRH/7dKZOe7A6Fz+ra++MmucDUzsAx0K\n" + + "MGT9Xoi5+CBBaNr+Y2lB9fF20N7eRNzQ3Xrz2OPl4cmU4gfECTZ1vZaKlmB+Vt8C\n" + + "E/nn49QGRI+BNBOdW+2aEpPoENczFosEAJXi5Cn2l0jOswDD7FU2PER1wfVY629i\n" + + "bICunudOSo64GKQslKkQWktc57DgdOQnH15qW1nVO7Z4H0GBxjSTRCu7Z7q08/qM\n" + + "ueWIvJ85HcFhOCl+vITOn0fZV0p8/IwsWz8G9h5bb2QgMAwDSdhnLuK/cXaGM09w\n" + + "n6k8O2rCvDtXRjqJATcEHwECACEFAkLuVRAXDIABHZ7red04sEmTnZyvPOx4Gxfe\n" + + "Gs0CBwAACgkQYECGYcQ78uEeNQf9H/v6I0EsB8phummuRbL/x5dUQmHF+HoMj+mm\n" + + "wosgD7rzuBfvhH3oIzmTp9kON40PbpkQDSsNmZitucZR+y7nlp4zT/6OJI5kna5O\n" + + "WsteIcy6ts1jSlPG/bXma6G6Y8HO4vWOwArYpz/igTtnjqNpK2w8K8M5zH09bvrB\n" + + "Hz+4I+8dXPkf8Dvd6tcCWgLjeO6lfBkHfZVmrwvSRk1V+4SyMdRy7cZShl0Z9Nvj\n" + + "9CrlrUUr4IaHBYI7ClRZkQhQibBS/gU0TiI0VgQdoctT1+i2YjEiQfxMLg1w74a+\n" + + "QTIzIno1WQEEM/YRqudxYAnEMGXHUI7gRP79Qu/MOcBxi5NzobQhVGVzdHVzZXIg\n" + + "TmluZSA8dGVzdDlAZXhhbXBsZS5jb20+iQE4BBMBAgAiBQJC7kcAAhsDBgsJCAcD\n" + + "AgYVCAIJCgsEFgIDAQIeAQIXgAAKCRBgQIZhxDvy4bvyCADVON45m1peNc0bwHHa\n" + + "yXhb7KnKVZYHrJwh5HSy4uFWWKxnffI61Xgyy9nDRMk3JiYVeKyQ9PI5w/xEmNL5\n" + + "rjUgdzfeLXcxSc1EjlyqGqxZGF/1yGWbtepe2q/nsiOiMb2+BJR5l1dJMx0cgMMc\n" + + "CMn+vIFd+XOmtLDZJpvw5eMuTKj3PpnK0XJ9dJzxM2JZToO6iWWH9tYiyA8RMh7b\n" + + "AafwtwWJUx8+mwhGGSmE51Mlv5b/NDAsdRxLbYDjGwm0HEhdMf9BIbeUBrDwhFuX\n" + + "bkQiloQAX6ATEKtAzSFoBUyYaBhhakwdesOEHt9/eQCKoPaqaFLJIDo0uZM77qoY\n" + + "9gmDnQOYBELuRwABCADbCoTcPKlCFNxiXbp1dz0NnMAFFBrooVGpBuGZQHP3lxCk\n" + + "BqZCOvA7KXZ+Dzhv2j234O5lx0vMsX+A9VLOc5w1RX2Yk10sDEE884xSs//5Kdvz\n" + + "cWQ2kKxyteEipJ8FTkP4U5vdsQjZZMnuUt7ygKtY8TckgNUKukBaBmT2m4CTVntk\n" + + "VoexLp7Oxk/RggypNZ2hfTMcw8yev0YbTxzSaeJe299yC04J4hFThZnCzbywZgKg\n" + + "t5BewdUNqX6BwunV6c+Hp18RYEm6GEEFqPObnd/NXZruSER19+CBVXKUc8um+aza\n" + + "ciqh5qsNC1U2lIq6E2LKQlIwhSf9jqDkszJKYv4zABEBAAEAB/0c76POOw6aazUT\n" + + "TZHUnhQ+WHHJefbKuoeWI7w+dD7y+02NzaRoZW7XnJ+fAZW8Dlb5k/O1FayUIEgE\n" + + "GjnT336dpE4g5NQkfdifG7Fy5NKGRkWx6viJI3g/OHsYX3+ebNDFMmO0gq7067/9\n" + + "WuHsTpvUMRwkF1zi1j4AETjZ7IBXdjuSCSu8OhEwr3d+WXibEmY5ec/d24l/APJx\n" + + "c3RMHw9PiDQeAKrByS6N10/yFgRpnouVx3wC7zFmhVewNV476Nyg34OvRoc+lCtk\n" + + "ixKdua6KuUJzGRWxgw+q2JD4goXxe0v2qU2KSU63gOYi0kg9tpwpn98lDNQykgmJ\n" + + "aQYdNIZJBADdlbkg9qbH1DREs7UF4jXN/SoYRbTh9639GfA4zkbfPmh/RmVIIEKd\n" + + "QN7qWK/Xy1bUS9vDzRfFgmoYGtqMmygOOFsVtfm8Y18lSXopN/3vhtai+dn+04Ef\n" + + "dl1irmGvm3p7y9Jh3s6uYTEJok0MywA7qBHvgSTVtc1PcZc6j6Bz1QQA/Q+nqyZY\n" + + "fLimt4KVYO1y6kSHgEqzggLTxyfGMW5RplTA0V1zCwjM6S+QWNqRxVNdB9Kkzn+S\n" + + "YDKHLYs8lXO2zvf8Yk9M7glgqvT4rJ51Zn2rc6lg1YUwFBXup5idTsuZwtqkvvKJ\n" + + "eS7L3cSBCqJMRjk47Y3V8zkrrN/HcYmyFecD/A+HPf4eSweUS025Bb+eCk4gTHbR\n" + + "uwmnKq7npk2XY4m0A/QdYF9dEWlpadsAr+ZwNQB3f21nQgKG0BudfL4FmpeW9RMt\n" + + "35aSIaV7RkxYOt5HEvjFRvLbeL1YYaj+D0dvz8SP1AUPvpWIVlQ03OjRlPyrPW50\n" + + "LoqyP8PTb6svnHvmQseJAR8EGAECAAkFAkLuRwACGwwACgkQYECGYcQ78uF3Bwf/\n" + + "TADFPU0ZpHZugyOlOi5VRycRwTMdzR1LC677TqYtGCBJ0B8ELwL192UBgdDm2nWx\n" + + "G4/At9BmOjzAvEn7AsBKcg8toau2kQeQgq8PAerCQ5e9d4YP8GHjQ1slRrsRzcmm\n" + + "j4U8WUzyEOxTId/wiFQB1F3h37Oc5URpnpLtzVGu2hVNSrOLn2ecLpl7Av8mn7Oc\n" + + "pmUxMzdfWicAaF379d2tLnYszykTfFGNtMtNhZyOePItubTKg7wbfIJ8iJsBogjh\n" + + "r+MY43Zf1lTBoU0ioniIhzw2Z7zqsP9+Vey7pm/41dVBbKWsgb9ERbrzCydaPBO2\n" + + "s8bxuqjpwKw7OoZhW3zV5Q==\n" + + "=JxsF\n" + + "-----END PGP PRIVATE KEY BLOCK-----\n"); + } } From 6ad5d8b90d12147028a93a047fbbcb7c8552b5b4 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 22 Oct 2015 10:57:22 -0700 Subject: [PATCH 6/7] PublicKeyStore: Use {@code null} in Javadoc The JDK Javadocs use "null" when used as a normal English adjective, and "{@code null}" when referring to a variable or return value having this value. Start being a little more consistent about this. Change-Id: I44ebde98f43696b4085c797620aec95ef9d9f35a --- .../src/main/java/com/google/gerrit/gpg/PublicKeyStore.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 8f614d7dc8..3d939a1f61 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 @@ -83,7 +83,7 @@ public class PublicKeyStore implements AutoCloseable { * @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. + * signature, or {@code null} if none was found. * @throws PGPException if an error occurred verifying the signature. */ public static PGPPublicKey getSigner(Iterable keyRings, @@ -107,7 +107,7 @@ public class PublicKeyStore implements AutoCloseable { * @param userId user ID being certified. * @param key key being certified. * @return the key chosen from {@code keyRings} that was able to verify the - * certification, or null if none was found. + * certification, or {@code null} if none was found. * @throws PGPException if an error occurred verifying the certification. */ public static PGPPublicKey getSigner(Iterable keyRings, From a278b95161276c03539ebf461d1eb4aff9cd55a1 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 22 Oct 2015 11:02:51 -0700 Subject: [PATCH 7/7] Distinguish between assertProblems and assertNoProblems assertProblems(foo) with an implicit empty list can be confusing to read. You can tell I figured this out, because GerritPublicKeyCheckerTest, which postdates the other tests, already used this pattern. Use it consistently everywhere. Use (String, String...) in method signatures to make invalid use of assertProblems with no expected problems a compile-time error. Change-Id: I5621c586ac16dff2a34d30efd1cca8949907316d --- .../gpg/GerritPublicKeyCheckerTest.java | 10 +-- .../gerrit/gpg/PublicKeyCheckerTest.java | 61 +++++++++++++------ .../gpg/PushCertificateCheckerTest.java | 23 +++++-- 3 files changed, 67 insertions(+), 27 deletions(-) diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java index db0bb88ef7..4df9d37633 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java @@ -14,7 +14,6 @@ package com.google.gerrit.gpg; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.gpg.GerritPublicKeyChecker.toExtIdKey; import static com.google.gerrit.gpg.PublicKeyStore.keyToString; @@ -67,6 +66,7 @@ import org.junit.Before; import org.junit.Test; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -433,11 +433,13 @@ public class GerritPublicKeyCheckerTest { } private void assertProblems(CheckResult result, Status expectedStatus, - String... expectedProblems) throws Exception { - checkArgument(expectedProblems.length > 0); + String first, String... rest) throws Exception { + List expectedProblems = new ArrayList<>(); + expectedProblems.add(first); + expectedProblems.addAll(Arrays.asList(rest)); assertThat(result.getStatus()).isEqualTo(expectedStatus); assertThat(result.getProblems()) - .containsExactly((Object[]) expectedProblems) + .containsExactlyElementsIn(expectedProblems) .inOrder(); } 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 576ffbd488..742bf1aa8b 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 @@ -54,10 +54,13 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import java.text.SimpleDateFormat; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.Iterator; +import java.util.List; import java.util.Map; public class PublicKeyCheckerTest { @@ -87,7 +90,7 @@ public class PublicKeyCheckerTest { @Test public void validKey() throws Exception { - assertProblems(validKeyWithoutExpiration()); + assertNoProblems(validKeyWithoutExpiration()); } @Test @@ -96,10 +99,10 @@ public class PublicKeyCheckerTest { PublicKeyChecker checker = new PublicKeyChecker() .setStore(store); - assertProblems(checker, k); + assertNoProblems(checker, k); checker.setEffectiveTime(parseDate("2015-07-10 12:00:00 -0400")); - assertProblems(checker, k); + assertNoProblems(checker, k); checker.setEffectiveTime(parseDate("2075-07-10 12:00:00 -0400")); assertProblems(checker, k, "Key is expired"); @@ -138,10 +141,10 @@ public class PublicKeyCheckerTest { save(); PublicKeyChecker checker = newChecker(2, kb, kd); - assertProblems(checker, ka); + assertNoProblems(checker, ka); assertProblems(checker, kb, "Key is expired"); - assertProblems(checker, kc); - assertProblems(checker, kd); + assertNoProblems(checker, kc); + assertNoProblems(checker, kd); assertProblems(checker, ke, "Key is expired", "No path to a trusted key"); } @@ -192,7 +195,7 @@ public class PublicKeyCheckerTest { // J trusts I to a depth of 1, so I itself is valid, but I's certification // of K is not valid. - assertProblems(checker, ki); + assertNoProblems(checker, ki); assertProblems(checker, kh, "No path to a trusted key", notTrusted(ki)); } @@ -212,7 +215,7 @@ public class PublicKeyCheckerTest { save(); // Key no longer specified as revoker. - assertProblems(kr.getPublicKey()); + assertNoProblems(kr.getPublicKey()); } @Test @@ -266,7 +269,7 @@ public class PublicKeyCheckerTest { PublicKeyChecker checker = new PublicKeyChecker() .setStore(store) .setEffectiveTime(parseDate("2010-01-01 12:00:00 -0400")); - assertProblems(checker, k); + assertNoProblems(checker, k); } @Test @@ -277,7 +280,7 @@ public class PublicKeyCheckerTest { save(); PublicKeyChecker checker = new PublicKeyChecker().setStore(store); - assertProblems(checker, k); + assertNoProblems(checker, k); } @Test @@ -293,7 +296,7 @@ public class PublicKeyCheckerTest { // Set time between key creation and revocation. checker.setEffectiveTime(parseDate("2005-08-01 13:00:00 -0400")); - assertProblems(checker, k); + assertNoProblems(checker, k); } private PGPPublicKeyRing removeRevokers(PGPPublicKeyRing kr) { @@ -342,21 +345,38 @@ public class PublicKeyCheckerTest { } private void assertProblems(PublicKeyChecker checker, TestKey k, - String... expected) { + String first, String... rest) { CheckResult result = checker.setStore(store) .check(k.getPublicKey()); - assertEquals(Arrays.asList(expected), result.getProblems()); + assertEquals(list(first, rest), result.getProblems()); } - private void assertProblems(TestKey tk, String... expected) throws Exception { - assertProblems(tk.getPublicKey(), expected); + private void assertNoProblems(PublicKeyChecker checker, TestKey k) { + CheckResult result = checker.setStore(store) + .check(k.getPublicKey()); + assertEquals(Collections.emptyList(), result.getProblems()); } - private void assertProblems(PGPPublicKey k, String... expected) throws Exception { + private void assertProblems(TestKey tk, String first, String... rest) { + assertProblems(tk.getPublicKey(), first, rest); + } + + private void assertNoProblems(TestKey tk) { + assertNoProblems(tk.getPublicKey()); + } + + private void assertProblems(PGPPublicKey k, String first, String... rest) { CheckResult result = new PublicKeyChecker() .setStore(store) .check(k); - assertEquals(Arrays.asList(expected), result.getProblems()); + assertEquals(list(first, rest), result.getProblems()); + } + + private void assertNoProblems(PGPPublicKey k) { + CheckResult result = new PublicKeyChecker() + .setStore(store) + .check(k); + assertEquals(Collections.emptyList(), result.getProblems()); } private static String notTrusted(TestKey k) { @@ -367,4 +387,11 @@ public class PublicKeyCheckerTest { private static Date parseDate(String str) throws Exception { return new SimpleDateFormat("YYYY-MM-dd HH:mm:ss Z").parse(str); } + + private static List list(String first, String[] rest) { + List all = new ArrayList<>(); + all.add(first); + all.addAll(Arrays.asList(rest)); + return all; + } } 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 977895859d..ee07d5580d 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 @@ -49,8 +49,11 @@ import java.io.ByteArrayOutputStream; import java.io.InputStreamReader; import java.io.Reader; import java.text.SimpleDateFormat; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; +import java.util.List; public class PushCertificateCheckerTest { private InMemoryRepository repo; @@ -98,7 +101,7 @@ public class PushCertificateCheckerTest { public void validCert() throws Exception { PushCertificate cert = newSignedCert(validNonce(), validKeyWithoutExpiration()); - assertProblems(cert); + assertNoProblems(cert); } @Test @@ -113,7 +116,7 @@ public class PushCertificateCheckerTest { checker = newChecker(false); PushCertificate cert = newSignedCert("invalid-nonce", validKeyWithoutExpiration()); - assertProblems(cert); + assertNoProblems(cert); } @Test @@ -139,7 +142,7 @@ public class PushCertificateCheckerTest { Date now = new SimpleDateFormat("YYYY-MM-dd HH:mm:ss Z") .parse("2005-07-10 12:00:00 -0400"); PushCertificate cert = newSignedCert(validNonce(), key3, now); - assertProblems(cert); + assertNoProblems(cert); } private String validNonce() { @@ -193,9 +196,17 @@ public class PushCertificateCheckerTest { return parser.parse(reader); } - private void assertProblems(PushCertificate cert, String... expected) - throws Exception { + private void assertProblems(PushCertificate cert, String first, + String... rest) throws Exception { + List expected = new ArrayList<>(); + expected.add(first); + expected.addAll(Arrays.asList(rest)); CheckResult result = checker.check(cert).getCheckResult(); - assertEquals(Arrays.asList(expected), result.getProblems()); + assertEquals(expected, result.getProblems()); + } + + private void assertNoProblems(PushCertificate cert) { + CheckResult result = checker.check(cert).getCheckResult(); + assertEquals(Collections.emptyList(), result.getProblems()); } }