Add PUSH_CERTIFICATES option to output push certs in REST API

To do this, we need to teach PushCertificateChecker to include in its
result type the key that it looked up from the store, in addition to
the results of the certificate check. This in turn needs to be
converted to the REST API return type in the GpgApiAdapter.

Change-Id: Ic83e3d4b66126629de6e32470089fa96173af5a9
This commit is contained in:
Dave Borowitz
2015-09-14 12:34:31 -04:00
parent 5ad43bbcfe
commit 6f58dbe334
14 changed files with 226 additions and 49 deletions

View File

@@ -315,6 +315,14 @@ default. Optional fields are:
link:#revision-info[RevisionInfo].
--
[[push-certificates]]
--
* `PUSH_CERTIFICATES`: include push certificate information in the
link:#revision-info[RevisionInfo]. Ignored if signed push is not
link:config-gerrit.html#receive.enableSignedPush[enabled] on the
server.
--
.Request
----
GET /changes/?q=97&o=CURRENT_REVISION&o=CURRENT_COMMIT&o=CURRENT_FILES&o=DOWNLOAD_COMMANDS HTTP/1.0
@@ -4355,6 +4363,23 @@ If `status` is set, an additional plaintext message describing the
outcome of the fix.
|===========================
[[push-certificate-info]]
=== PushCertificateInfo
The `PushCertificateInfo` entity contains information about a push
certificate provided when the user pushed for review with `git push
--signed HEAD:refs/for/<branch>`. Only used when signed push is
link:config-gerrit.html#receive.enableSignedPush[enabled] on the server.
[options="header",cols="1,6"]
|===========================
|Field Name|Description
|`certificate`|Signed certificate payload and GPG signature block.
|`key` |
Information about the key that signed the push, along with any problems
found while checking the signature or the key itself, as a
link:rest-api-accounts.html#gpg-key-info[GpgKeyInfo] entity.
|===========================
[[rebase-input]]
=== RebaseInput
The `RebaseInput` entity contains information for changing parent when rebasing.
@@ -4566,6 +4591,12 @@ this is the current patch set, contains the full commit message with
Gerrit-specific commit footers, as if this revision were submitted
using the link:project-configuration.html#cherry_pick[Cherry Pick]
submit type.
|`push_certificate` |optional|
If the link:#push-certificates[PUSH_CERTIFICATES] option is requested,
contains the push certificate provided by the user when uploading this
patch set as a link:#push-certificate-info[PushCertificateInfo] entity.
This field is always set if the option is requested; if no push
certificate was provided, it is set to an empty object.
|===========================
[[rule-input]]

View File

@@ -511,4 +511,28 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(approval._accountId).isEqualTo(user.id.get());
assertThat(approval.value).isNull();
}
@Test
public void pushCertificates() throws Exception {
PushOneCommit.Result r1 = createChange();
PushOneCommit.Result r2 = amendChange(r1.getChangeId());
ChangeInfo info = gApi.changes()
.id(r1.getChangeId())
.get(EnumSet.of(
ListChangesOption.ALL_REVISIONS,
ListChangesOption.PUSH_CERTIFICATES));
RevisionInfo rev1 = info.revisions.get(r1.getCommit().name());
assertThat(rev1).isNotNull();
assertThat(rev1.pushCertificate).isNotNull();
assertThat(rev1.pushCertificate.certificate).isNull();
assertThat(rev1.pushCertificate.key).isNull();
RevisionInfo rev2 = info.revisions.get(r2.getCommit().name());
assertThat(rev2).isNotNull();
assertThat(rev2.pushCertificate).isNotNull();
assertThat(rev2.pushCertificate.certificate).isNull();
assertThat(rev2.pushCertificate.key).isNull();
}
}

View File

@@ -59,7 +59,10 @@ public enum ListChangesOption {
CHANGE_ACTIONS(16),
/** Include a copy of commit messages including review footers. */
COMMIT_FOOTERS(17);
COMMIT_FOOTERS(17),
/** Include push certificate information along with any patch sets. */
PUSH_CERTIFICATES(18);
private final int value;

View File

@@ -0,0 +1,20 @@
// Copyright (C) 2015 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.extensions.common;
public class PushCertificateInfo {
public String certificate;
public GpgKeyInfo key;
}

View File

@@ -29,4 +29,5 @@ public class RevisionInfo {
public Map<String, FileInfo> files;
public Map<String, ActionInfo> actions;
public String commitWithFooters;
public PushCertificateInfo pushCertificate;
}

View File

@@ -19,6 +19,7 @@ import static com.google.gerrit.server.account.AccountResource.ACCOUNT_KIND;
import com.google.gerrit.extensions.api.accounts.GpgKeyApi;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.common.PushCertificateInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.NotImplementedException;
@@ -29,6 +30,7 @@ import com.google.gerrit.gpg.server.DeleteGpgKey;
import com.google.gerrit.gpg.server.GpgKeys;
import com.google.gerrit.gpg.server.PostGpgKeys;
import com.google.gerrit.server.EnableSignedPush;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.api.accounts.GpgApiAdapter;
@@ -96,5 +98,11 @@ public class GpgModule extends RestApiModule {
public GpgKeyApi gpgKey(AccountResource account, IdString idStr) {
throw new NotImplementedException(MSG);
}
@Override
public PushCertificateInfo checkPushCertificate(String certStr,
IdentifiedUser expectedUser) {
throw new NotImplementedException(MSG);
}
}
}

View File

@@ -48,6 +48,24 @@ public abstract class PushCertificateChecker {
private static final Logger log =
LoggerFactory.getLogger(PushCertificateChecker.class);
public static class Result {
private final PGPPublicKey key;
private final CheckResult checkResult;
private Result(PGPPublicKey key, CheckResult checkResult) {
this.key = key;
this.checkResult = checkResult;
}
public PGPPublicKey getPublicKey() {
return key;
}
public CheckResult getCheckResult() {
return checkResult;
}
}
private final PublicKeyChecker publicKeyChecker;
private final boolean checkNonce;
@@ -62,12 +80,12 @@ public abstract class PushCertificateChecker {
*
* @return result of the check.
*/
public final CheckResult check(PushCertificate cert) {
public final Result check(PushCertificate cert) {
if (checkNonce && cert.getNonceStatus() != NonceStatus.OK) {
return CheckResult.bad("Invalid nonce");
return new Result(null, CheckResult.bad("Invalid nonce"));
}
List<CheckResult> results = new ArrayList<>(2);
CheckResult sigResult = null;
Result sigResult = null;
try {
PGPSignature sig = readSignature(cert);
if (sig != null) {
@@ -93,8 +111,7 @@ public abstract class PushCertificateChecker {
return combine(sigResult, results);
}
private static CheckResult combine(CheckResult sigResult,
List<CheckResult> results) {
private static Result combine(Result sigResult, List<CheckResult> results) {
// Combine results:
// - If any input result is BAD, the final result is bad.
// - If sigResult is TRUSTED and no other result is BAD, the final result
@@ -108,15 +125,20 @@ public abstract class PushCertificateChecker {
}
Status status = bad ? BAD : OK;
PGPPublicKey key;
if (sigResult != null) {
problems.addAll(sigResult.getProblems());
if (sigResult.getStatus() == BAD) {
key = sigResult.getPublicKey();
CheckResult cr = sigResult.getCheckResult();
problems.addAll(cr.getProblems());
if (cr.getStatus() == BAD) {
status = BAD;
} else if (!bad && sigResult.getStatus() == TRUSTED) {
} else if (!bad && cr.getStatus() == TRUSTED) {
status = TRUSTED;
}
} else {
key = null;
}
return CheckResult.create(status, problems);
return new Result(key, CheckResult.create(status, problems));
}
/**
@@ -165,18 +187,20 @@ public abstract class PushCertificateChecker {
return null;
}
private CheckResult checkSignature(PGPSignature sig, PushCertificate cert,
private Result checkSignature(PGPSignature sig, PushCertificate cert,
PublicKeyStore store) throws PGPException, IOException {
PGPPublicKeyRingCollection keys = store.get(sig.getKeyID());
if (!keys.getKeyRings().hasNext()) {
return CheckResult.bad("No public keys found for key ID "
+ keyIdToString(sig.getKeyID()));
return new Result(null,
CheckResult.bad("No public keys found for key ID "
+ keyIdToString(sig.getKeyID())));
}
PGPPublicKey signer =
PublicKeyStore.getSigner(keys, sig, Constants.encode(cert.toText()));
if (signer == null) {
return CheckResult.bad("Signature by " + keyIdToString(sig.getKeyID())
+ " is not valid");
return new Result(null,
CheckResult.bad("Signature by " + keyIdToString(sig.getKeyID())
+ " is not valid"));
}
CheckResult result = publicKeyChecker.check(signer, store);
if (!result.getProblems().isEmpty()) {
@@ -184,8 +208,9 @@ public abstract class PushCertificateChecker {
.append(keyToString(signer))
.append(":\n ")
.append(Joiner.on("\n ").join(result.getProblems()));
return CheckResult.create(result.getStatus(), err.toString());
return new Result(
signer, CheckResult.create(result.getStatus(), err.toString()));
}
return result;
return new Result(signer, result);
}
}

View File

@@ -55,7 +55,8 @@ public class SignedPushPreReceiveHook implements PreReceiveHook {
return;
}
CheckResult result = checkerFactory.create(user.get(), true)
.check(cert);
.check(cert)
.getCheckResult();
if (!isAllowed(result, commands)) {
for (String problem : result.getProblems()) {
rp.sendMessage(problem);

View File

@@ -16,17 +16,23 @@ package com.google.gerrit.gpg.api;
import com.google.gerrit.extensions.api.accounts.GpgKeyApi;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.common.PushCertificateInfo;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.gpg.GerritPushCertificateChecker;
import com.google.gerrit.gpg.PushCertificateChecker;
import com.google.gerrit.gpg.server.GpgKeys;
import com.google.gerrit.gpg.server.PostGpgKeys;
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.api.accounts.GpgApiAdapter;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import org.bouncycastle.openpgp.PGPException;
import org.eclipse.jgit.transport.PushCertificate;
import org.eclipse.jgit.transport.PushCertificateParser;
import java.io.IOException;
import java.util.List;
@@ -36,15 +42,18 @@ public class GpgApiAdapterImpl implements GpgApiAdapter {
private final PostGpgKeys postGpgKeys;
private final GpgKeys gpgKeys;
private final GpgKeyApiImpl.Factory gpgKeyApiFactory;
private final GerritPushCertificateChecker.Factory pushCertCheckerFactory;
@Inject
GpgApiAdapterImpl(
PostGpgKeys postGpgKeys,
GpgKeys gpgKeys,
GpgKeyApiImpl.Factory gpgKeyApiFactory) {
GpgKeyApiImpl.Factory gpgKeyApiFactory,
GerritPushCertificateChecker.Factory pushCertCheckerFactory) {
this.postGpgKeys = postGpgKeys;
this.gpgKeys = gpgKeys;
this.gpgKeyApiFactory = gpgKeyApiFactory;
this.pushCertCheckerFactory = pushCertCheckerFactory;
}
@Override
@@ -80,4 +89,21 @@ public class GpgApiAdapterImpl implements GpgApiAdapter {
throw new GpgException(e);
}
}
@Override
public PushCertificateInfo checkPushCertificate(String certStr,
IdentifiedUser expectedUser) throws GpgException {
try {
PushCertificate cert = PushCertificateParser.fromString(certStr);
PushCertificateChecker.Result result =
pushCertCheckerFactory.create(expectedUser, false).check(cert);
PushCertificateInfo info = new PushCertificateInfo();
info.certificate = certStr;
info.key = GpgKeys.toJson(result.getPublicKey(), result.getCheckResult());
return info;
} catch (IOException e) {
throw new GpgException(e);
}
}
}

View File

@@ -162,7 +162,7 @@ public class GpgKeys implements
if (Arrays.equals(keyRing.getPublicKey().getFingerprint(), fp)) {
found = true;
GpgKeyInfo info = toJson(
keyRing,
keyRing.getPublicKey(),
checkerFactory.create(rsrc.getUser()),
store);
keys.put(info.id, info);
@@ -196,7 +196,7 @@ public class GpgKeys implements
public GpgKeyInfo apply(GpgKey rsrc) throws IOException {
try (PublicKeyStore store = storeProvider.get()) {
return toJson(
rsrc.getKeyRing(),
rsrc.getKeyRing().getPublicKey(),
checkerFactory.create(rsrc.getUser()),
store);
}
@@ -231,10 +231,11 @@ public class GpgKeys implements
}
}
static GpgKeyInfo toJson(PGPPublicKeyRing keyRing, PublicKeyChecker checker,
PublicKeyStore store) throws IOException {
PGPPublicKey key = keyRing.getPublicKey();
public static GpgKeyInfo toJson(PGPPublicKey key, CheckResult checkResult)
throws IOException {
GpgKeyInfo info = new GpgKeyInfo();
if (key != null) {
info.id = PublicKeyStore.keyIdToString(key.getKeyID());
info.fingerprint = Fingerprint.toString(key.getFingerprint());
@SuppressWarnings("unchecked")
@@ -250,11 +251,21 @@ public class GpgKeys implements
key.encode(aout);
info.key = new String(out.toByteArray(), UTF_8);
}
}
CheckResult checkResult = checker.check(key, store);
info.status = checkResult.getStatus();
info.problems = checkResult.getProblems();
return info;
}
static GpgKeyInfo toJson(PGPPublicKey key, PublicKeyChecker checker,
PublicKeyStore store) throws IOException {
return toJson(key, checker.check(key, store));
}
public static void toJson(GpgKeyInfo info, CheckResult checkResult) {
info.status = checkResult.getStatus();
info.problems = checkResult.getProblems();
}
}

View File

@@ -249,7 +249,9 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
Map<String, GpgKeyInfo> infos =
Maps.newHashMapWithExpectedSize(keys.size() + deleted.size());
for (PGPPublicKeyRing keyRing : keys) {
GpgKeyInfo info = GpgKeys.toJson(keyRing, checker, store);
PGPPublicKey key = keyRing.getPublicKey();
CheckResult result = checker.check(key, store);
GpgKeyInfo info = GpgKeys.toJson(key, result);
infos.put(info.id, info);
info.id = null;
}

View File

@@ -159,7 +159,7 @@ public class PushCertificateCheckerTest {
private void assertProblems(PushCertificate cert, String... expected)
throws Exception {
CheckResult result = checker.check(cert);
CheckResult result = checker.check(cert).getCheckResult();
assertEquals(Arrays.asList(expected), result.getProblems());
}
}

View File

@@ -16,9 +16,11 @@ package com.google.gerrit.server.api.accounts;
import com.google.gerrit.extensions.api.accounts.GpgKeyApi;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.common.PushCertificateInfo;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResource;
import java.util.List;
@@ -27,8 +29,13 @@ import java.util.Map;
public interface GpgApiAdapter {
Map<String, GpgKeyInfo> listGpgKeys(AccountResource account)
throws RestApiException, GpgException;
Map<String, GpgKeyInfo> putGpgKeys(AccountResource account, List<String> add,
List<String> delete) throws RestApiException, GpgException;
GpgKeyApi gpgKey(AccountResource account, IdString idStr)
throws RestApiException, GpgException;
PushCertificateInfo checkPushCertificate(String certStr,
IdentifiedUser expectedUser) throws GpgException;
}

View File

@@ -29,6 +29,7 @@ import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LAB
import static com.google.gerrit.extensions.client.ListChangesOption.DOWNLOAD_COMMANDS;
import static com.google.gerrit.extensions.client.ListChangesOption.LABELS;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static com.google.gerrit.extensions.client.ListChangesOption.PUSH_CERTIFICATES;
import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED;
import static com.google.gerrit.extensions.client.ListChangesOption.WEB_LINKS;
import static com.google.gerrit.server.CommonConverters.toGitPerson;
@@ -67,6 +68,7 @@ import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.FetchInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.ProblemInfo;
import com.google.gerrit.extensions.common.PushCertificateInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.config.DownloadCommand;
@@ -84,9 +86,11 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GpgException;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.api.accounts.GpgApiAdapter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LabelNormalizer;
import com.google.gerrit.server.git.MergeUtil;
@@ -149,6 +153,7 @@ public class ChangeJson {
private final ChangeMessagesUtil cmUtil;
private final Provider<ConsistencyChecker> checkerProvider;
private final ActionJson actionJson;
private final GpgApiAdapter gpgApi;
private AccountLoader accountLoader;
private FixInput fix;
@@ -173,6 +178,7 @@ public class ChangeJson {
ChangeMessagesUtil cmUtil,
Provider<ConsistencyChecker> checkerProvider,
ActionJson actionJson,
GpgApiAdapter gpgApi,
@Assisted Set<ListChangesOption> options) {
this.db = db;
this.labelNormalizer = ln;
@@ -192,6 +198,7 @@ public class ChangeJson {
this.cmUtil = cmUtil;
this.checkerProvider = checkerProvider;
this.actionJson = actionJson;
this.gpgApi = gpgApi;
this.options = options.isEmpty()
? EnumSet.noneOf(ListChangesOption.class)
: EnumSet.copyOf(options);
@@ -254,8 +261,8 @@ public class ChangeJson {
} else {
return toChangeInfo(cd, limitToPsId);
}
} catch (PatchListNotAvailableException | OrmException | IOException
| RuntimeException e) {
} catch (PatchListNotAvailableException | GpgException | OrmException
| IOException | RuntimeException e) {
if (!has(CHECK)) {
Throwables.propagateIfPossible(e, OrmException.class);
throw new OrmException(e);
@@ -315,8 +322,8 @@ public class ChangeJson {
if (i == null) {
try {
i = toChangeInfo(cd, Optional.<PatchSet.Id> absent());
} catch (PatchListNotAvailableException | OrmException | IOException
| RuntimeException e) {
} catch (PatchListNotAvailableException | GpgException | OrmException
| IOException | RuntimeException e) {
if (has(CHECK)) {
i = checkOnly(cd);
} else {
@@ -359,8 +366,8 @@ public class ChangeJson {
}
private ChangeInfo toChangeInfo(ChangeData cd,
Optional<PatchSet.Id> limitToPsId)
throws PatchListNotAvailableException, OrmException, IOException {
Optional<PatchSet.Id> limitToPsId) throws PatchListNotAvailableException,
GpgException, OrmException, IOException {
ChangeInfo out = new ChangeInfo();
if (has(CHECK)) {
@@ -816,8 +823,8 @@ public class ChangeJson {
}
private Map<String, RevisionInfo> revisions(ChangeControl ctl,
Map<PatchSet.Id, PatchSet> map)
throws PatchListNotAvailableException, OrmException, IOException {
Map<PatchSet.Id, PatchSet> map) throws PatchListNotAvailableException,
GpgException, OrmException, IOException {
Map<String, RevisionInfo> res = Maps.newLinkedHashMap();
for (PatchSet in : map.values()) {
if ((has(ALL_REVISIONS)
@@ -858,7 +865,8 @@ public class ChangeJson {
}
private RevisionInfo toRevisionInfo(ChangeControl ctl, PatchSet in)
throws PatchListNotAvailableException, OrmException, IOException {
throws PatchListNotAvailableException, GpgException, OrmException,
IOException {
Change c = ctl.getChange();
RevisionInfo out = new RevisionInfo();
out.isCurrent = in.getId().equals(c.currentPatchSetId());
@@ -903,6 +911,16 @@ public class ChangeJson {
new RevisionResource(new ChangeResource(ctl), in));
}
if (has(PUSH_CERTIFICATES)) {
if (in.getPushCertificate() != null) {
out.pushCertificate = gpgApi.checkPushCertificate(
in.getPushCertificate(),
userFactory.create(db, in.getUploader()));
} else {
out.pushCertificate = new PushCertificateInfo();
}
}
return out;
}