PublicKeyChecker: Add web-of-trust checks
Callers can specify a set of trusted keys and a maximum depth, and the checker will recursively verify certification signatures until reaching a root trusted key. Change-Id: I41df284302ea77d92515d87e7eb960f4d3d40857
This commit is contained in:
parent
833a8d81dd
commit
8e28957137
@ -21,6 +21,8 @@ import java.util.List;
|
||||
|
||||
/** Result of checking an object like a key or signature. */
|
||||
public class CheckResult {
|
||||
public static final CheckResult OK = new CheckResult();
|
||||
|
||||
private final List<String> problems;
|
||||
|
||||
CheckResult(String... problems) {
|
||||
|
@ -14,19 +14,108 @@
|
||||
|
||||
package com.google.gerrit.gpg;
|
||||
|
||||
import org.bouncycastle.openpgp.PGPPublicKey;
|
||||
import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString;
|
||||
import static com.google.gerrit.gpg.PublicKeyStore.keyToString;
|
||||
|
||||
import org.bouncycastle.bcpg.SignatureSubpacket;
|
||||
import org.bouncycastle.bcpg.SignatureSubpacketTags;
|
||||
import org.bouncycastle.openpgp.PGPException;
|
||||
import org.bouncycastle.openpgp.PGPPublicKey;
|
||||
import org.bouncycastle.openpgp.PGPPublicKeyRingCollection;
|
||||
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;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
/** Checker for GPG public keys for use in a push certificate. */
|
||||
public class PublicKeyChecker {
|
||||
// https://tools.ietf.org/html/rfc4880#section-5.2.3.13
|
||||
private static final int COMPLETE_TRUST = 120;
|
||||
|
||||
private final Map<Long, Fingerprint> trusted;
|
||||
private final int maxTrustDepth;
|
||||
|
||||
/** Create a new checker that does not check the web of trust. */
|
||||
public PublicKeyChecker() {
|
||||
this(0, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Check a public key.
|
||||
* @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.
|
||||
*/
|
||||
public PublicKeyChecker(int maxTrustDepth, Collection<Fingerprint> trusted) {
|
||||
if (trusted != null) {
|
||||
if (maxTrustDepth <= 0) {
|
||||
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;
|
||||
}
|
||||
this.maxTrustDepth = maxTrustDepth;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
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");
|
||||
}
|
||||
return check(key, store, 0, true, new HashSet<Fingerprint>());
|
||||
}
|
||||
|
||||
/**
|
||||
* Check only a public key, not including its web of trust.
|
||||
*
|
||||
* @param key the public key.
|
||||
* @return the result of the check.
|
||||
*/
|
||||
public final CheckResult check(PGPPublicKey key) {
|
||||
return check(key, null, 0, false, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Perform custom checks.
|
||||
* <p>
|
||||
* Default implementation does nothing, but may be overridden by subclasses.
|
||||
*
|
||||
* @param key the public key.
|
||||
* @param problems list to which any problems should be added.
|
||||
*/
|
||||
public void checkCustom(PGPPublicKey key, List<String> problems) {
|
||||
// Default implementation does nothing.
|
||||
}
|
||||
|
||||
private CheckResult check(PGPPublicKey key, PublicKeyStore store, int depth,
|
||||
boolean expand, Set<Fingerprint> seen) {
|
||||
List<String> problems = new ArrayList<>();
|
||||
if (key.isRevoked()) {
|
||||
// TODO(dborowitz): isRevoked is overeager:
|
||||
@ -43,18 +132,118 @@ public class PublicKeyChecker {
|
||||
}
|
||||
}
|
||||
checkCustom(key, problems);
|
||||
|
||||
CheckResult trustResult = checkWebOfTrust(key, store, depth, seen);
|
||||
if (expand) {
|
||||
problems.addAll(trustResult.getProblems());
|
||||
} else if (!trustResult.isOk()) {
|
||||
problems.add("Key is not trusted");
|
||||
}
|
||||
return new CheckResult(problems);
|
||||
}
|
||||
|
||||
/**
|
||||
* Perform custom checks.
|
||||
* <p>
|
||||
* Default implementation does nothing, but may be overridden by subclasses.
|
||||
*
|
||||
* @param key the public key.
|
||||
* @param problems list to which any problems should be added.
|
||||
*/
|
||||
public void checkCustom(PGPPublicKey key, List<String> problems) {
|
||||
// Default implementation does nothing.
|
||||
private CheckResult checkWebOfTrust(PGPPublicKey key, PublicKeyStore store,
|
||||
int depth, Set<Fingerprint> seen) {
|
||||
if (trusted == null || store == null) {
|
||||
return CheckResult.OK; // Trust checking not configured.
|
||||
}
|
||||
Fingerprint fp = new Fingerprint(key.getFingerprint());
|
||||
if (seen.contains(fp)) {
|
||||
return new CheckResult("Key is trusted in a cycle");
|
||||
}
|
||||
seen.add(fp);
|
||||
|
||||
Fingerprint trustedFp = trusted.get(key.getKeyID());
|
||||
if (trustedFp != null && trustedFp.equals(fp)) {
|
||||
return CheckResult.OK; // Directly trusted.
|
||||
} else if (depth >= maxTrustDepth) {
|
||||
return new CheckResult(
|
||||
"No path of depth <= " + maxTrustDepth + " to a trusted key");
|
||||
}
|
||||
|
||||
List<CheckResult> signerResults = new ArrayList<>();
|
||||
@SuppressWarnings("unchecked")
|
||||
Iterator<String> userIds = key.getUserIDs();
|
||||
while (userIds.hasNext()) {
|
||||
String userId = userIds.next();
|
||||
@SuppressWarnings("unchecked")
|
||||
Iterator<PGPSignature> sigs = key.getSignaturesForID(userId);
|
||||
while (sigs.hasNext()) {
|
||||
PGPSignature sig = sigs.next();
|
||||
// TODO(dborowitz): Handle CERTIFICATION_REVOCATION.
|
||||
if (sig.getSignatureType() != PGPSignature.DEFAULT_CERTIFICATION
|
||||
&& sig.getSignatureType() != PGPSignature.POSITIVE_CERTIFICATION) {
|
||||
continue; // Not a certification.
|
||||
}
|
||||
|
||||
PGPPublicKey signer = getSigner(store, sig, userId, key, signerResults);
|
||||
// TODO(dborowitz): Require self certification.
|
||||
if (signer == null
|
||||
|| Arrays.equals(signer.getFingerprint(), key.getFingerprint())) {
|
||||
continue;
|
||||
}
|
||||
CheckResult signerResult = checkTrustSubpacket(sig, depth);
|
||||
if (signerResult.isOk()) {
|
||||
signerResult = check(signer, store, depth + 1, false, seen);
|
||||
if (signerResult.isOk()) {
|
||||
return CheckResult.OK;
|
||||
}
|
||||
}
|
||||
signerResults.add(new CheckResult(
|
||||
"Certification by " + keyToString(signer)
|
||||
+ " is valid, but key is not trusted"));
|
||||
}
|
||||
}
|
||||
|
||||
List<String> problems = new ArrayList<>();
|
||||
problems.add("No path to a trusted key");
|
||||
for (CheckResult signerResult : signerResults) {
|
||||
problems.addAll(signerResult.getProblems());
|
||||
}
|
||||
return new CheckResult(problems);
|
||||
}
|
||||
|
||||
private static PGPPublicKey getSigner(PublicKeyStore store, PGPSignature sig,
|
||||
String userId, PGPPublicKey key, List<CheckResult> results) {
|
||||
try {
|
||||
PGPPublicKeyRingCollection signers = store.get(sig.getKeyID());
|
||||
if (!signers.getKeyRings().hasNext()) {
|
||||
results.add(new CheckResult(
|
||||
"Key " + keyIdToString(sig.getKeyID())
|
||||
+ " used for certification is not in store"));
|
||||
return null;
|
||||
}
|
||||
PGPPublicKey signer = PublicKeyStore.getSigner(signers, sig, userId, key);
|
||||
if (signer == null) {
|
||||
results.add(new CheckResult(
|
||||
"Certification by " + keyIdToString(sig.getKeyID())
|
||||
+ " is not valid"));
|
||||
return null;
|
||||
}
|
||||
return signer;
|
||||
} catch (PGPException | IOException e) {
|
||||
results.add(new CheckResult(
|
||||
"Error checking certification by " + keyIdToString(sig.getKeyID())));
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private CheckResult checkTrustSubpacket(PGPSignature sig, int depth) {
|
||||
SignatureSubpacket trustSub = sig.getHashedSubPackets().getSubpacket(
|
||||
SignatureSubpacketTags.TRUST_SIG);
|
||||
if (trustSub == null || trustSub.getData().length != 2) {
|
||||
return new CheckResult("Certification is missing trust information");
|
||||
}
|
||||
byte amount = trustSub.getData()[1];
|
||||
if (amount < COMPLETE_TRUST) {
|
||||
return new CheckResult("Certification does not fully trust key");
|
||||
}
|
||||
byte level = trustSub.getData()[0];
|
||||
int required = depth + 1;
|
||||
if (level < required) {
|
||||
return new CheckResult("Certification trusts to depth " + level
|
||||
+ ", but depth " + required + " is required");
|
||||
}
|
||||
return CheckResult.OK;
|
||||
}
|
||||
}
|
||||
|
@ -99,6 +99,29 @@ public class PublicKeyStore implements AutoCloseable {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Choose the public key that produced a certification.
|
||||
* <p>
|
||||
* @param keyRings candidate keys.
|
||||
* @param sig signature object.
|
||||
* @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.
|
||||
* @throws PGPException if an error occurred verifying the certification.
|
||||
*/
|
||||
public static PGPPublicKey getSigner(Iterable<PGPPublicKeyRing> keyRings,
|
||||
PGPSignature sig, String userId, PGPPublicKey key) throws PGPException {
|
||||
for (PGPPublicKeyRing kr : keyRings) {
|
||||
PGPPublicKey k = kr.getPublicKey();
|
||||
sig.init(new BcPGPContentVerifierBuilderProvider(), k);
|
||||
if (sig.verifyCertification(userId, key)) {
|
||||
return k;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private final Repository repo;
|
||||
private ObjectReader reader;
|
||||
private RevCommit tip;
|
||||
|
@ -143,7 +143,7 @@ public abstract class PushCertificateChecker {
|
||||
+ " is not valid");
|
||||
return;
|
||||
}
|
||||
CheckResult result = publicKeyChecker.check(signer);
|
||||
CheckResult result = publicKeyChecker.check(signer, store);
|
||||
if (!result.isOk()) {
|
||||
StringBuilder err = new StringBuilder("Invalid public key ")
|
||||
.append(keyToString(signer))
|
||||
|
@ -191,6 +191,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
|
||||
List<String> addedKeys = new ArrayList<>();
|
||||
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);
|
||||
if (!result.isOk()) {
|
||||
throw new BadRequestException(String.format(
|
||||
|
File diff suppressed because it is too large
Load Diff
@ -563,7 +563,7 @@ public class TestKey {
|
||||
private final PGPPublicKeyRing pubRing;
|
||||
private final PGPSecretKeyRing secRing;
|
||||
|
||||
private TestKey(String pubArmored, String secArmored) {
|
||||
public TestKey(String pubArmored, String secArmored) {
|
||||
this.pubArmored = pubArmored;
|
||||
this.secArmored = secArmored;
|
||||
BcKeyFingerprintCalculator fc = new BcKeyFingerprintCalculator();
|
||||
|
Loading…
Reference in New Issue
Block a user