Distinguish case where a public key must match a specific user
GerritPublicKeyChecker was always checking that the user IDs in the key matched at least one identity from AccountExternalIds for the current user in the request. This was broken when combined with web-of-trust checks, since only the initial key passed to check() matches the current user; all other keys match different users. Add a factory to create two different kinds of checkers, depending on whether we have a specific user in mind that we want to match the initial key against. Expose the depth to checkCustom() so we only use this user on the initial call and not on later recursive calls. Add tests to GerritPublicKeyCheckerTest for web-of-trust checks, which were previously only in PublicKeyCheckerTest. Change-Id: I1af584354f26c80e9ed3b7643ed6e3624820214c
This commit is contained in:
@@ -14,19 +14,23 @@
|
||||
|
||||
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.Ordering;
|
||||
import com.google.common.io.BaseEncoding;
|
||||
import com.google.gerrit.common.PageLinks;
|
||||
import com.google.gerrit.reviewdb.client.AccountExternalId;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.config.CanonicalWebUrl;
|
||||
import com.google.gerrit.server.config.GerritServerConfig;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
import com.google.inject.Singleton;
|
||||
@@ -53,69 +57,149 @@ import java.util.Set;
|
||||
* For Gerrit, keys must contain a self-signed user ID certification matching a
|
||||
* trusted external ID in the database, or an email address thereof.
|
||||
*/
|
||||
@Singleton
|
||||
public class GerritPublicKeyChecker extends PublicKeyChecker {
|
||||
private static final Logger log =
|
||||
LoggerFactory.getLogger(GerritPublicKeyChecker.class);
|
||||
|
||||
private final Provider<ReviewDb> db;
|
||||
private final String webUrl;
|
||||
private final Provider<IdentifiedUser> userProvider;
|
||||
private final IdentifiedUser.GenericFactory userFactory;
|
||||
private final IdentifiedUser expectedUser;
|
||||
|
||||
private static List<Fingerprint> getTrustedFingerprints(Config cfg) {
|
||||
String[] strs = cfg.getStringList("receive", null, "trustedKey");
|
||||
if (strs == null || strs.length == 0) {
|
||||
return null;
|
||||
@Singleton
|
||||
public static class Factory {
|
||||
private final Provider<ReviewDb> db;
|
||||
private final String webUrl;
|
||||
private final IdentifiedUser.GenericFactory userFactory;
|
||||
private final int maxTrustDepth;
|
||||
private final ImmutableList<Fingerprint> trusted;
|
||||
|
||||
@Inject
|
||||
Factory(@GerritServerConfig Config cfg,
|
||||
Provider<ReviewDb> db,
|
||||
IdentifiedUser.GenericFactory userFactory,
|
||||
@CanonicalWebUrl String webUrl) {
|
||||
this.db = db;
|
||||
this.webUrl = webUrl;
|
||||
this.userFactory = userFactory;
|
||||
this.maxTrustDepth = cfg.getInt("receive", null, "maxTrustDepth", 0);
|
||||
|
||||
String[] strs = cfg.getStringList("receive", null, "trustedKey");
|
||||
if (strs.length != 0) {
|
||||
List<Fingerprint> fps = new ArrayList<>(strs.length);
|
||||
for (String str : strs) {
|
||||
str = CharMatcher.WHITESPACE.removeFrom(str).toUpperCase();
|
||||
fps.add(new Fingerprint(BaseEncoding.base16().decode(str)));
|
||||
}
|
||||
trusted = ImmutableList.copyOf(fps);
|
||||
} else {
|
||||
trusted = null;
|
||||
}
|
||||
}
|
||||
List<Fingerprint> fps = new ArrayList<>(strs.length);
|
||||
for (String str : strs) {
|
||||
str = CharMatcher.WHITESPACE.removeFrom(str).toUpperCase();
|
||||
fps.add(new Fingerprint(BaseEncoding.base16().decode(str)));
|
||||
|
||||
/**
|
||||
* Create a checker that can check arbitrary public keys.
|
||||
* <p>
|
||||
* 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.
|
||||
* <p>
|
||||
* 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 fps;
|
||||
}
|
||||
|
||||
@Inject
|
||||
GerritPublicKeyChecker(
|
||||
@GerritServerConfig Config cfg,
|
||||
@CanonicalWebUrl String webUrl,
|
||||
Provider<IdentifiedUser> userProvider) {
|
||||
super(cfg.getInt("receive", null, "maxTrustDepth", 0),
|
||||
getTrustedFingerprints(cfg));
|
||||
this.webUrl = webUrl;
|
||||
this.userProvider = userProvider;
|
||||
private GerritPublicKeyChecker(Factory factory, IdentifiedUser expectedUser) {
|
||||
super(factory.maxTrustDepth, factory.trusted);
|
||||
this.db = factory.db;
|
||||
this.webUrl = factory.webUrl;
|
||||
this.userFactory = factory.userFactory;
|
||||
this.expectedUser = expectedUser;
|
||||
}
|
||||
|
||||
@Override
|
||||
public CheckResult checkCustom(PGPPublicKey key) {
|
||||
public CheckResult checkCustom(PGPPublicKey key, int depth) {
|
||||
try {
|
||||
Set<String> allowedUserIds = getAllowedUserIds();
|
||||
if (allowedUserIds.isEmpty()) {
|
||||
return CheckResult.bad("No identities found for user; check "
|
||||
+ webUrl + "#" + PageLinks.SETTINGS_WEBIDENT);
|
||||
if (depth == 0 && expectedUser != null) {
|
||||
return checkIdsForExpectedUser(key);
|
||||
} else {
|
||||
return checkIdsForArbitraryUser(key);
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
Iterator<String> userIds = key.getUserIDs();
|
||||
while (userIds.hasNext()) {
|
||||
String userId = userIds.next();
|
||||
if (isAllowed(userId, allowedUserIds)) {
|
||||
Iterator<PGPSignature> sigs = getSignaturesForId(key, userId);
|
||||
while (sigs.hasNext()) {
|
||||
if (isValidCertification(key, sigs.next(), userId)) {
|
||||
return CheckResult.trusted();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return CheckResult.bad(missingUserIds(allowedUserIds));
|
||||
} catch (PGPException e) {
|
||||
} catch (PGPException | OrmException e) {
|
||||
String msg = "Error checking user IDs for key";
|
||||
log.warn(msg + " " + keyIdToString(key.getKeyID()), e);
|
||||
return CheckResult.bad(msg);
|
||||
}
|
||||
}
|
||||
|
||||
private CheckResult checkIdsForExpectedUser(PGPPublicKey key)
|
||||
throws PGPException {
|
||||
Set<String> allowedUserIds = getAllowedUserIds(expectedUser);
|
||||
if (allowedUserIds.isEmpty()) {
|
||||
return CheckResult.bad("No identities found for user; check "
|
||||
+ webUrl + "#" + PageLinks.SETTINGS_WEBIDENT);
|
||||
}
|
||||
if (hasAllowedUserId(key, allowedUserIds)) {
|
||||
return CheckResult.trusted();
|
||||
}
|
||||
return CheckResult.bad(missingUserIds(allowedUserIds));
|
||||
}
|
||||
|
||||
private CheckResult checkIdsForArbitraryUser(PGPPublicKey key)
|
||||
throws PGPException, OrmException {
|
||||
AccountExternalId extId = db.get().accountExternalIds().get(
|
||||
toExtIdKey(key));
|
||||
if (extId == null) {
|
||||
return CheckResult.bad("Key is not associated with any users");
|
||||
}
|
||||
IdentifiedUser user = userFactory.create(db, extId.getAccountId());
|
||||
Set<String> allowedUserIds = getAllowedUserIds(user);
|
||||
if (allowedUserIds.isEmpty()) {
|
||||
return CheckResult.bad("No identities found for user");
|
||||
}
|
||||
if (hasAllowedUserId(key, allowedUserIds)) {
|
||||
return CheckResult.trusted();
|
||||
}
|
||||
return CheckResult.bad(
|
||||
"Key does not contain any valid certifications for user's identities");
|
||||
}
|
||||
|
||||
private boolean hasAllowedUserId(PGPPublicKey key, Set<String> allowedUserIds)
|
||||
throws PGPException {
|
||||
@SuppressWarnings("unchecked")
|
||||
Iterator<String> userIds = key.getUserIDs();
|
||||
while (userIds.hasNext()) {
|
||||
String userId = userIds.next();
|
||||
if (isAllowed(userId, allowedUserIds)) {
|
||||
Iterator<PGPSignature> sigs = getSignaturesForId(key, userId);
|
||||
while (sigs.hasNext()) {
|
||||
if (isValidCertification(key, sigs.next(), userId)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
private Iterator<PGPSignature> getSignaturesForId(PGPPublicKey key,
|
||||
String userId) {
|
||||
@@ -124,8 +208,7 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
|
||||
Collections.emptyIterator());
|
||||
}
|
||||
|
||||
private Set<String> getAllowedUserIds() {
|
||||
IdentifiedUser user = userProvider.get();
|
||||
private Set<String> getAllowedUserIds(IdentifiedUser user) {
|
||||
Set<String> result = new HashSet<>();
|
||||
result.addAll(user.getEmailAddresses());
|
||||
for (AccountExternalId extId : user.state().getExternalIds()) {
|
||||
@@ -175,4 +258,10 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
|
||||
}
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
static AccountExternalId.Key toExtIdKey(PGPPublicKey key) {
|
||||
return new AccountExternalId.Key(
|
||||
SCHEME_GPGKEY,
|
||||
BaseEncoding.base16().encode(key.getFingerprint()));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -114,16 +114,19 @@ public class PublicKeyChecker {
|
||||
* subclasses.
|
||||
*
|
||||
* @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}.
|
||||
* @return the result of the custom check.
|
||||
*/
|
||||
public CheckResult checkCustom(PGPPublicKey key) {
|
||||
public CheckResult checkCustom(PGPPublicKey key, int depth) {
|
||||
return CheckResult.ok();
|
||||
}
|
||||
|
||||
private CheckResult check(PGPPublicKey key, PublicKeyStore store, int depth,
|
||||
boolean expand, Set<Fingerprint> seen) {
|
||||
CheckResult basicResult = checkBasic(key);
|
||||
CheckResult customResult = checkCustom(key);
|
||||
CheckResult customResult = checkCustom(key, depth);
|
||||
CheckResult trustResult = checkWebOfTrust(key, store, depth, seen);
|
||||
if (!expand && !trustResult.isTrusted()) {
|
||||
trustResult = CheckResult.create(trustResult.getStatus(),
|
||||
|
||||
@@ -53,7 +53,6 @@ class SignedPushModule extends AbstractModule {
|
||||
if (!BouncyCastleUtil.havePGP()) {
|
||||
throw new ProvisionException("Bouncy Castle PGP not installed");
|
||||
}
|
||||
bind(PublicKeyChecker.class).to(GerritPublicKeyChecker.class);
|
||||
bind(PublicKeyStore.class).toProvider(StoreProvider.class);
|
||||
DynamicSet.bind(binder(), ReceivePackInitializer.class)
|
||||
.to(Initializer.class);
|
||||
|
||||
@@ -14,9 +14,11 @@
|
||||
|
||||
package com.google.gerrit.gpg;
|
||||
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.config.AllUsersName;
|
||||
import com.google.gerrit.server.git.GitRepositoryManager;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
import com.google.inject.Singleton;
|
||||
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
@@ -39,16 +41,19 @@ import java.util.Collection;
|
||||
public class SignedPushPreReceiveHook implements PreReceiveHook {
|
||||
private final GitRepositoryManager repoManager;
|
||||
private final AllUsersName allUsers;
|
||||
private final PublicKeyChecker keyChecker;
|
||||
private final Provider<IdentifiedUser> user;
|
||||
private final GerritPublicKeyChecker.Factory keyCheckerFactory;
|
||||
|
||||
@Inject
|
||||
public SignedPushPreReceiveHook(
|
||||
GitRepositoryManager repoManager,
|
||||
AllUsersName allUsers,
|
||||
PublicKeyChecker keyChecker) {
|
||||
Provider<IdentifiedUser> user,
|
||||
GerritPublicKeyChecker.Factory keyCheckerFactory) {
|
||||
this.repoManager = repoManager;
|
||||
this.allUsers = allUsers;
|
||||
this.keyChecker = keyChecker;
|
||||
this.user = user;
|
||||
this.keyCheckerFactory = keyCheckerFactory;
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -58,6 +63,7 @@ public class SignedPushPreReceiveHook implements PreReceiveHook {
|
||||
if (cert == null) {
|
||||
return;
|
||||
}
|
||||
PublicKeyChecker keyChecker = keyCheckerFactory.create(user.get());
|
||||
PushCertificateChecker checker = new PushCertificateChecker(keyChecker) {
|
||||
@Override
|
||||
protected Repository getRepository() throws IOException {
|
||||
|
||||
@@ -35,7 +35,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
|
||||
import com.google.gerrit.extensions.restapi.RestModifyView;
|
||||
import com.google.gerrit.gpg.CheckResult;
|
||||
import com.google.gerrit.gpg.Fingerprint;
|
||||
import com.google.gerrit.gpg.PublicKeyChecker;
|
||||
import com.google.gerrit.gpg.GerritPublicKeyChecker;
|
||||
import com.google.gerrit.gpg.PublicKeyStore;
|
||||
import com.google.gerrit.gpg.server.PostGpgKeys.Input;
|
||||
import com.google.gerrit.reviewdb.client.AccountExternalId;
|
||||
@@ -79,19 +79,19 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
|
||||
private final Provider<PersonIdent> serverIdent;
|
||||
private final Provider<ReviewDb> db;
|
||||
private final Provider<PublicKeyStore> storeProvider;
|
||||
private final PublicKeyChecker checker;
|
||||
private final GerritPublicKeyChecker.Factory checkerFactory;
|
||||
private final AddKeySender.Factory addKeyFactory;
|
||||
|
||||
@Inject
|
||||
PostGpgKeys(@GerritPersonIdent Provider<PersonIdent> serverIdent,
|
||||
Provider<ReviewDb> db,
|
||||
Provider<PublicKeyStore> storeProvider,
|
||||
PublicKeyChecker checker,
|
||||
GerritPublicKeyChecker.Factory checkerFactory,
|
||||
AddKeySender.Factory addKeyFactory) {
|
||||
this.serverIdent = serverIdent;
|
||||
this.db = db;
|
||||
this.storeProvider = storeProvider;
|
||||
this.checker = checker;
|
||||
this.checkerFactory = checkerFactory;
|
||||
this.addKeyFactory = addKeyFactory;
|
||||
}
|
||||
|
||||
@@ -192,7 +192,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
|
||||
for (PGPPublicKeyRing keyRing : keyRings) {
|
||||
PGPPublicKey key = keyRing.getPublicKey();
|
||||
// Don't check web of trust; admins can fill in certifications later.
|
||||
CheckResult result = checker.check(key);
|
||||
CheckResult result = checkerFactory.create(rsrc.getUser()).check(key);
|
||||
if (!result.isOk()) {
|
||||
throw new BadRequestException(String.format(
|
||||
"Problems with public key %s:\n%s",
|
||||
|
||||
@@ -16,8 +16,21 @@ 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;
|
||||
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;
|
||||
import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyD;
|
||||
import static com.google.gerrit.gpg.testutil.TestTrustKeys.keyE;
|
||||
import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_MAILTO;
|
||||
import static org.eclipse.jgit.lib.RefUpdate.Result.FAST_FORWARD;
|
||||
import static org.eclipse.jgit.lib.RefUpdate.Result.FORCED;
|
||||
import static org.eclipse.jgit.lib.RefUpdate.Result.NEW;
|
||||
|
||||
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;
|
||||
@@ -40,11 +53,22 @@ import com.google.inject.Injector;
|
||||
import com.google.inject.Provider;
|
||||
import com.google.inject.util.Providers;
|
||||
|
||||
import org.bouncycastle.openpgp.PGPPublicKey;
|
||||
import org.bouncycastle.openpgp.PGPPublicKeyRing;
|
||||
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
|
||||
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
|
||||
import org.eclipse.jgit.lib.CommitBuilder;
|
||||
import org.eclipse.jgit.lib.Config;
|
||||
import org.eclipse.jgit.lib.PersonIdent;
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
import org.eclipse.jgit.transport.PushCertificateIdent;
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
|
||||
/** Unit tests for {@link GerritPublicKeyChecker}. */
|
||||
public class GerritPublicKeyCheckerTest {
|
||||
@@ -55,7 +79,7 @@ public class GerritPublicKeyCheckerTest {
|
||||
private AccountManager accountManager;
|
||||
|
||||
@Inject
|
||||
private GerritPublicKeyChecker checker;
|
||||
private GerritPublicKeyChecker.Factory checkerFactory;
|
||||
|
||||
@Inject
|
||||
private IdentifiedUser.GenericFactory userFactory;
|
||||
@@ -73,10 +97,18 @@ public class GerritPublicKeyCheckerTest {
|
||||
private ReviewDb db;
|
||||
private Account.Id userId;
|
||||
private IdentifiedUser user;
|
||||
private Repository storeRepo;
|
||||
private PublicKeyStore store;
|
||||
|
||||
@Before
|
||||
public void setUpInjector() throws Exception {
|
||||
Injector injector = Guice.createInjector(new InMemoryModule());
|
||||
Config cfg = InMemoryModule.newDefaultConfig();
|
||||
cfg.setInt("receive", null, "maxTrustDepth", 2);
|
||||
cfg.setStringList("receive", null, "trustedKey", ImmutableList.of(
|
||||
Fingerprint.toString(keyB().getPublicKey().getFingerprint()),
|
||||
Fingerprint.toString(keyD().getPublicKey().getFingerprint())));
|
||||
Injector injector = Guice.createInjector(new InMemoryModule(cfg));
|
||||
|
||||
lifecycle = new LifecycleManager();
|
||||
lifecycle.add(injector);
|
||||
injector.injectMembers(this);
|
||||
@@ -103,6 +135,21 @@ public class GerritPublicKeyCheckerTest {
|
||||
return Providers.of(db);
|
||||
}
|
||||
});
|
||||
|
||||
storeRepo = new InMemoryRepository(new DfsRepositoryDescription("repo"));
|
||||
store = new PublicKeyStore(storeRepo);
|
||||
}
|
||||
|
||||
@After
|
||||
public void tearDown() throws Exception {
|
||||
store.close();
|
||||
storeRepo.close();
|
||||
}
|
||||
|
||||
private IdentifiedUser addUser(String name) throws Exception {
|
||||
AuthRequest req = AuthRequest.forUser(name);
|
||||
Account.Id id = accountManager.authenticate(req).getAccountId();
|
||||
return userFactory.create(Providers.of(db), id);
|
||||
}
|
||||
|
||||
private IdentifiedUser reloadUser() {
|
||||
@@ -125,22 +172,25 @@ public class GerritPublicKeyCheckerTest {
|
||||
@Test
|
||||
public void defaultGpgCertificationMatchesEmail() throws Exception {
|
||||
TestKey key = TestKeys.key5();
|
||||
GerritPublicKeyChecker checker = checkerFactory.create(user);
|
||||
assertProblems(
|
||||
TestKeys.key5(),
|
||||
checker.check(key.getPublicKey()), Status.BAD,
|
||||
"Key must contain a valid certification for one of the following "
|
||||
+ "identities:\n"
|
||||
+ " gerrit:user\n"
|
||||
+ " username:user");
|
||||
|
||||
addExternalId("test", "test", "test5@example.com");
|
||||
assertNoProblems(key);
|
||||
checker = checkerFactory.create(user);
|
||||
assertNoProblems(checker.check(key.getPublicKey()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void defaultGpgCertificationDoesNotMatchEmail() throws Exception {
|
||||
addExternalId("test", "test", "nobody@example.com");
|
||||
GerritPublicKeyChecker checker = checkerFactory.create(user);
|
||||
assertProblems(
|
||||
TestKeys.key5(),
|
||||
checker.check(TestKeys.key5().getPublicKey()), Status.BAD,
|
||||
"Key must contain a valid certification for one of the following "
|
||||
+ "identities:\n"
|
||||
+ " gerrit:user\n"
|
||||
@@ -152,14 +202,16 @@ public class GerritPublicKeyCheckerTest {
|
||||
@Test
|
||||
public void manualCertificationMatchesExternalId() throws Exception {
|
||||
addExternalId("foo", "myId", null);
|
||||
assertNoProblems(TestKeys.key5());
|
||||
GerritPublicKeyChecker checker = checkerFactory.create(user);
|
||||
assertNoProblems(checker.check(TestKeys.key5().getPublicKey()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void manualCertificationDoesNotExternalId() throws Exception {
|
||||
public void manualCertificationDoesNotMatchExternalId() throws Exception {
|
||||
addExternalId("foo", "otherId", null);
|
||||
GerritPublicKeyChecker checker = checkerFactory.create(user);
|
||||
assertProblems(
|
||||
TestKeys.key5(),
|
||||
checker.check(TestKeys.key5().getPublicKey()), Status.BAD,
|
||||
"Key must contain a valid certification for one of the following "
|
||||
+ "identities:\n"
|
||||
+ " foo:otherId\n"
|
||||
@@ -172,24 +224,219 @@ public class GerritPublicKeyCheckerTest {
|
||||
db.accountExternalIds().delete(
|
||||
db.accountExternalIds().byAccount(user.getAccountId()));
|
||||
reloadUser();
|
||||
|
||||
TestKey key = TestKeys.key5();
|
||||
GerritPublicKeyChecker checker = checkerFactory.create(user);
|
||||
assertProblems(
|
||||
TestKeys.key5(),
|
||||
checker.check(key.getPublicKey()), Status.BAD,
|
||||
"No identities found for user; check"
|
||||
+ " http://test/#/settings/web-identities");
|
||||
|
||||
checker = checkerFactory.create();
|
||||
assertProblems(
|
||||
checker.check(key.getPublicKey()), Status.BAD,
|
||||
"Key is not associated with any users");
|
||||
|
||||
db.accountExternalIds().insert(Collections.singleton(
|
||||
new AccountExternalId(
|
||||
user.getAccountId(), toExtIdKey(key.getPublicKey()))));
|
||||
reloadUser();
|
||||
assertProblems(
|
||||
checker.check(key.getPublicKey()), Status.BAD,
|
||||
"No identities found for user");
|
||||
}
|
||||
|
||||
private void assertNoProblems(TestKey key) throws Exception {
|
||||
assertThat(checker.check(key.getPublicKey()).getProblems()).isEmpty();
|
||||
@Test
|
||||
public void checkValidTrustChainAndCorrectExternalIds() throws Exception {
|
||||
// A---Bx
|
||||
// \
|
||||
// \---C---D
|
||||
// \
|
||||
// \---Ex
|
||||
//
|
||||
// The server ultimately trusts B and D.
|
||||
// D and E trust C to be a valid introducer of depth 2.
|
||||
IdentifiedUser userB = addUser("userB");
|
||||
TestKey keyA = add(keyA(), user);
|
||||
TestKey keyB = add(keyB(), userB);
|
||||
add(keyC(), addUser("userC"));
|
||||
add(keyD(), addUser("userD"));
|
||||
add(keyE(), addUser("userE"));
|
||||
|
||||
// Checker for A, checking A.
|
||||
GerritPublicKeyChecker checkerA = checkerFactory.create(user);
|
||||
assertNoProblems(checkerA.check(keyA.getPublicKey(), store));
|
||||
|
||||
// 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);
|
||||
assertProblems(
|
||||
checkerB.check(keyB.getPublicKey(), store), Status.BAD,
|
||||
"Key is expired");
|
||||
}
|
||||
|
||||
private void assertProblems(TestKey key, String... expected)
|
||||
@Test
|
||||
public void checkWithValidKeyButWrongExpectedUserInChecker()
|
||||
throws Exception {
|
||||
checkArgument(expected.length > 0);
|
||||
assertThat(checker.check(key.getPublicKey()).getProblems())
|
||||
.containsExactly((Object[]) expected)
|
||||
// A---Bx
|
||||
// \
|
||||
// \---C---D
|
||||
// \
|
||||
// \---Ex
|
||||
//
|
||||
// The server ultimately trusts B and D.
|
||||
// D and E trust C to be a valid introducer of depth 2.
|
||||
IdentifiedUser userB = addUser("userB");
|
||||
TestKey keyA = add(keyA(), user);
|
||||
TestKey keyB = add(keyB(), userB);
|
||||
add(keyC(), addUser("userC"));
|
||||
add(keyD(), addUser("userD"));
|
||||
add(keyE(), addUser("userE"));
|
||||
|
||||
// Checker for A, checking B.
|
||||
GerritPublicKeyChecker checkerA = checkerFactory.create(user);
|
||||
assertProblems(
|
||||
checkerA.check(keyB.getPublicKey(), store), Status.BAD,
|
||||
"Key is expired",
|
||||
"Key must contain a valid certification for one of the following"
|
||||
+ " identities:\n"
|
||||
+ " gerrit:user\n"
|
||||
+ " mailto:testa@example.com\n"
|
||||
+ " testa@example.com\n"
|
||||
+ " username:user");
|
||||
|
||||
// Checker for B, checking A.
|
||||
GerritPublicKeyChecker checkerB = checkerFactory.create(userB);
|
||||
assertProblems(
|
||||
checkerB.check(keyA.getPublicKey(), store), Status.BAD,
|
||||
"Key must contain a valid certification for one of the following"
|
||||
+ " identities:\n"
|
||||
+ " gerrit:userB\n"
|
||||
+ " mailto:testb@example.com\n"
|
||||
+ " testb@example.com\n"
|
||||
+ " username:userB");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void checkTrustChainWithExpiredKey() throws Exception {
|
||||
// A---Bx
|
||||
//
|
||||
// The server ultimately trusts B.
|
||||
TestKey keyA = add(keyA(), user);
|
||||
TestKey keyB = add(keyB(), addUser("userB"));
|
||||
|
||||
GerritPublicKeyChecker checker = checkerFactory.create(user);
|
||||
assertProblems(
|
||||
checker.check(keyA.getPublicKey(), store), Status.OK,
|
||||
"No path to a trusted key",
|
||||
"Certification by " + keyToString(keyB.getPublicKey())
|
||||
+ " is valid, but key is not trusted",
|
||||
"Key D24FE467 used for certification is not in store");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void checkTrustChainUsingCheckerWithoutExpectedKey() throws Exception {
|
||||
// A---Bx
|
||||
// \
|
||||
// \---C---D
|
||||
// \
|
||||
// \---Ex
|
||||
//
|
||||
// The server ultimately trusts B and D.
|
||||
// D and E trust C to be a valid introducer of depth 2.
|
||||
TestKey keyA = add(keyA(), user);
|
||||
TestKey keyB = add(keyB(), addUser("userB"));
|
||||
TestKey keyC = add(keyC(), addUser("userC"));
|
||||
TestKey keyD = add(keyD(), addUser("userD"));
|
||||
TestKey keyE = add(keyE(), addUser("userE"));
|
||||
|
||||
// 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));
|
||||
assertProblems(
|
||||
checker.check(keyB.getPublicKey(), store), Status.BAD,
|
||||
"Key is expired");
|
||||
assertNoProblems(checker.check(keyC.getPublicKey(), store));
|
||||
assertNoProblems(checker.check(keyD.getPublicKey(), store));
|
||||
assertProblems(
|
||||
checker.check(keyE.getPublicKey(), store), Status.BAD,
|
||||
"Key is expired",
|
||||
"No path to a trusted key");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void keyLaterInTrustChainMissingUserId() throws Exception {
|
||||
// A---Bx
|
||||
// \
|
||||
// \---C
|
||||
//
|
||||
// The server ultimately trusts B.
|
||||
// C signed A's key but is not in the store.
|
||||
TestKey keyA = add(keyA(), user);
|
||||
|
||||
PGPPublicKeyRing keyRingB = keyB().getPublicKeyRing();
|
||||
PGPPublicKey keyB = keyRingB.getPublicKey();
|
||||
keyB = PGPPublicKey.removeCertification(
|
||||
keyB, (String) keyB.getUserIDs().next());
|
||||
keyRingB = PGPPublicKeyRing.insertPublicKey(keyRingB, keyB);
|
||||
add(keyRingB, addUser("userB"));
|
||||
|
||||
GerritPublicKeyChecker checkerA = checkerFactory.create(user);
|
||||
assertProblems(checkerA.check(keyA.getPublicKey(), store), Status.OK,
|
||||
"No path to a trusted key",
|
||||
"Certification by " + keyToString(keyB)
|
||||
+ " is valid, but key is not trusted",
|
||||
"Key D24FE467 used for certification is not in store");
|
||||
}
|
||||
|
||||
private void add(PGPPublicKeyRing kr, IdentifiedUser user) throws Exception {
|
||||
Account.Id id = user.getAccountId();
|
||||
List<AccountExternalId> newExtIds = new ArrayList<>(2);
|
||||
newExtIds.add(new AccountExternalId(id, toExtIdKey(kr.getPublicKey())));
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
String userId = (String) Iterators.getOnlyElement(
|
||||
kr.getPublicKey().getUserIDs(), null);
|
||||
if (userId != null) {
|
||||
String email = PushCertificateIdent.parse(userId).getEmailAddress();
|
||||
assertThat(email).contains("@");
|
||||
AccountExternalId mailto = new AccountExternalId(
|
||||
id, new AccountExternalId.Key(SCHEME_MAILTO, email));
|
||||
mailto.setEmailAddress(email);
|
||||
newExtIds.add(mailto);
|
||||
}
|
||||
|
||||
store.add(kr);
|
||||
PersonIdent ident = new PersonIdent("A U Thor", "author@example.com");
|
||||
CommitBuilder cb = new CommitBuilder();
|
||||
cb.setAuthor(ident);
|
||||
cb.setCommitter(ident);
|
||||
assertThat(store.save(cb)).isAnyOf(NEW, FAST_FORWARD, FORCED);
|
||||
|
||||
db.accountExternalIds().insert(newExtIds);
|
||||
accountCache.evict(user.getAccountId());
|
||||
}
|
||||
|
||||
private TestKey add(TestKey k, IdentifiedUser user) throws Exception {
|
||||
add(k.getPublicKeyRing(), user);
|
||||
return k;
|
||||
}
|
||||
|
||||
private void assertProblems(CheckResult result, Status expectedStatus,
|
||||
String... expectedProblems) throws Exception {
|
||||
checkArgument(expectedProblems.length > 0);
|
||||
assertThat(result.getStatus()).isEqualTo(expectedStatus);
|
||||
assertThat(result.getProblems())
|
||||
.containsExactly((Object[]) expectedProblems)
|
||||
.inOrder();
|
||||
}
|
||||
|
||||
private void assertNoProblems(CheckResult result) {
|
||||
assertThat(result.getStatus()).isEqualTo(Status.TRUSTED);
|
||||
assertThat(result.getProblems()).isEmpty();
|
||||
}
|
||||
|
||||
private void addExternalId(String scheme, String id, String email)
|
||||
throws Exception {
|
||||
AccountExternalId extId = new AccountExternalId(user.getAccountId(),
|
||||
|
||||
Reference in New Issue
Block a user