Force validation of the author and committer lines during push

The new forge identity category can be used to control who can
forge the author and/or the committer lines in a commit, or the
tagger line in an annotated tag.

The access control Push Tag +3 was removed, as it previously had
been used to imply what Forge Identity +2 now supplies, a way to
work around the check for tagger matching the current user.

Documentation relating to Push Tag has been updated, including
describing the simple case that lightweight (non annotated) tags are
now supported through Push Branch +2 on the refs/tags/* namespace.

Bug: issue 421
Change-Id: Ie0dc8adc2538803021dec766e00ef6ea34d4d14c
Signed-off-by: Shawn O. Pearce <sop@google.com>
Reviewed-by: Nico Sallembien <nsallembien@google.com>
This commit is contained in:
Shawn O. Pearce
2010-01-29 07:35:58 -08:00
parent a517cb5510
commit f6a13f4eed
8 changed files with 280 additions and 44 deletions

View File

@@ -46,6 +46,7 @@ import com.google.gerrit.server.mail.EmailException;
import com.google.gerrit.server.mail.MergedSender;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
import com.google.gwtorm.client.AtomicUpdate;
@@ -156,6 +157,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
private ReceiveCommand newChange;
private Branch.NameKey destBranch;
private RefControl destBranchCtl;
private final List<Change.Id> allNewChanges = new ArrayList<Change.Id>();
private final Map<Change.Id, ReplaceRequest> replaceByChange =
@@ -464,6 +466,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
if (ctl.canCreate(rp.getRevWalk(), obj)) {
validateNewCommits(ctl, cmd);
// Let the core receive process handle it
} else {
reject(cmd);
@@ -473,6 +476,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
private void parseUpdate(final ReceiveCommand cmd) {
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
if (ctl.canUpdate()) {
validateNewCommits(ctl, cmd);
// Let the core receive process handle it
} else {
reject(cmd);
@@ -511,6 +515,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
if (oldObject instanceof RevCommit && newObject instanceof RevCommit
&& ctl.canForceUpdate()) {
validateNewCommits(ctl, cmd);
// Let the core receive process handle it
} else {
cmd.setResult(ReceiveCommand.Result.REJECTED_NONFASTFORWARD);
@@ -563,7 +568,8 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
return;
}
if (!projectControl.controlForRef(destBranch).canUpload()) {
destBranchCtl = projectControl.controlForRef(destBranch);
if (!destBranchCtl.canUpload()) {
reject(cmd);
}
@@ -710,7 +716,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
//
continue;
}
if (!validCommitter(newChange, c)) {
if (!validCommit(destBranchCtl, newChange, c)) {
// Not a change the user can propose? Abort as early as possible.
//
return;
@@ -905,9 +911,6 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
throws IOException, OrmException {
final RevCommit c = request.newCommit;
rp.getRevWalk().parseBody(c);
if (!validCommitter(request.cmd, c)) {
return null;
}
final Account.Id me = currentUser.getAccountId();
final Set<Account.Id> reviewers = new HashSet<Account.Id>(reviewerId);
@@ -940,10 +943,15 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
reject(request.cmd, "change " + request.ontoChange + " closed");
return null;
}
if (!projectControl.controlFor(change).canAddPatchSet()) {
final ChangeControl changeCtl = projectControl.controlFor(change);
if (!changeCtl.canAddPatchSet()) {
reject(request.cmd, "cannot replace " + request.ontoChange);
return null;
}
if (!validCommit(changeCtl.getRefControl(), request.cmd, c)) {
return null;
}
final PatchSet.Id priorPatchSet = change.currentPatchSetId();
for (final PatchSet ps : db.patchSets().byChange(request.ontoChange)) {
@@ -1244,8 +1252,34 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
String mergedIntoRef;
}
private boolean validCommitter(final ReceiveCommand cmd, final RevCommit c)
throws MissingObjectException, IOException {
private void validateNewCommits(RefControl ctl, ReceiveCommand cmd) {
final RevWalk walk = rp.getRevWalk();
walk.reset();
walk.sort(RevSort.NONE);
try {
walk.markStart(walk.parseCommit(cmd.getNewId()));
for (final Ref r : rp.getAdvertisedRefs().values()) {
try {
walk.markUninteresting(walk.parseCommit(r.getObjectId()));
} catch (IOException e) {
continue;
}
}
RevCommit c;
while ((c = walk.next()) != null) {
if (!validCommit(ctl, cmd, c)) {
break;
}
}
} catch (IOException err) {
cmd.setResult(Result.REJECTED_MISSING_OBJECT);
log.error("Invalid pack upload; one or more objects weren't sent", err);
}
}
private boolean validCommit(final RefControl ctl, final ReceiveCommand cmd,
final RevCommit c) throws MissingObjectException, IOException {
rp.getRevWalk().parseBody(c);
final PersonIdent committer = c.getCommitterIdent();
final PersonIdent author = c.getAuthorIdent();
@@ -1261,9 +1295,18 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
return false;
}
// Require that author matches the uploader.
//
if (!currentUser.getEmailAddresses().contains(author.getEmailAddress())
&& !ctl.canForgeAuthor()) {
reject(cmd, "you are not author " + author.getEmailAddress());
return false;
}
// Require that committer matches the uploader.
//
if (!currentUser.getEmailAddresses().contains(committer.getEmailAddress())) {
if (!currentUser.getEmailAddresses().contains(committer.getEmailAddress())
&& !ctl.canForgeCommitter()) {
reject(cmd, "you are not committer " + committer.getEmailAddress());
return false;
}
@@ -1283,7 +1326,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
}
}
}
if (!sboAuthor && !sboCommitter && !sboMe) {
if (!sboAuthor && !sboCommitter && !sboMe && !ctl.canForgeCommitter()) {
reject(cmd, "not Signed-off-by author/committer/uploader");
return false;
}
@@ -1308,6 +1351,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
while ((c = rw.next()) != null) {
final Ref ref = byCommit.get(c.copy());
if (ref != null) {
rw.parseBody(c);
closeChange(cmd, PatchSet.Id.fromRef(ref.getName()), c);
continue;
}

View File

@@ -14,6 +14,9 @@
package com.google.gerrit.server.project;
import static com.google.gerrit.reviewdb.ApprovalCategory.FORGE_AUTHOR;
import static com.google.gerrit.reviewdb.ApprovalCategory.FORGE_COMMITTER;
import static com.google.gerrit.reviewdb.ApprovalCategory.FORGE_IDENTITY;
import static com.google.gerrit.reviewdb.ApprovalCategory.OWN;
import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_HEAD;
import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_HEAD_CREATE;
@@ -21,7 +24,6 @@ import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_HEAD_REPLACE;
import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_HEAD_UPDATE;
import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_TAG;
import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_TAG_ANNOTATED;
import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_TAG_ANY;
import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_TAG_SIGNED;
import static com.google.gerrit.reviewdb.ApprovalCategory.READ;
@@ -50,6 +52,9 @@ public class RefControl {
private final ProjectControl projectControl;
private final String refName;
private Boolean canForgeAuthor;
private Boolean canForgeCommitter;
RefControl(final ProjectControl projectControl, final String refName) {
this.projectControl = projectControl;
this.refName = refName;
@@ -128,24 +133,28 @@ public class RefControl {
return owner || canPerform(PUSH_HEAD, PUSH_HEAD_CREATE);
} else if (object instanceof RevTag) {
final RevTag tag = (RevTag) object;
try {
rw.parseBody(object);
rw.parseBody(tag);
} catch (IOException e) {
return false;
}
final RevTag tag = (RevTag) object;
// Require the tagger to be present and match the current user's
// email address, unless PUSH_ANY_TAG was granted.
// If tagger is present, require it matches the user's email.
//
final PersonIdent tagger = tag.getTaggerIdent();
if (tagger == null || !(getCurrentUser() instanceof IdentifiedUser)) {
return owner || canPerform(PUSH_TAG, PUSH_TAG_ANY);
}
final IdentifiedUser user = (IdentifiedUser) getCurrentUser();
if (!user.getEmailAddresses().contains(tagger.getEmailAddress())) {
return owner || canPerform(PUSH_TAG, PUSH_TAG_ANY);
if (tagger != null) {
boolean valid;
if (getCurrentUser() instanceof IdentifiedUser) {
final IdentifiedUser user = (IdentifiedUser) getCurrentUser();
final String addr = tagger.getEmailAddress();
valid = user.getEmailAddresses().contains(addr);
} else {
valid = false;
}
if (!valid && !owner && !canPerform(FORGE_IDENTITY, FORGE_COMMITTER)) {
return false;
}
}
// If the tag has a PGP signature, allow a lower level of permission
@@ -181,6 +190,22 @@ public class RefControl {
}
}
/** @return true if this user can forge the author line in a commit. */
public boolean canForgeAuthor() {
if (canForgeAuthor == null) {
canForgeAuthor = canPerform(FORGE_IDENTITY, FORGE_AUTHOR);
}
return canForgeAuthor;
}
/** @return true if this user can forge the committer line in a commit. */
public boolean canForgeCommitter() {
if (canForgeCommitter == null) {
canForgeCommitter = canPerform(FORGE_IDENTITY, FORGE_COMMITTER);
}
return canForgeCommitter;
}
private boolean canPerform(ApprovalCategory.Id actionId, short level) {
final Set<AccountGroup.Id> groups = getCurrentUser().getEffectiveGroups();
int val = Integer.MIN_VALUE;

View File

@@ -91,6 +91,7 @@ public class SchemaCreator {
initSubmitCategory(db);
initPushTagCategory(db);
initPushUpdateBranchCategory(db);
initForgeIdentityCategory(db, sConfig);
initWildCardProject(db);
final SqlDialect d = jdbc.getDialect();
@@ -277,14 +278,13 @@ public class SchemaCreator {
final ApprovalCategory cat;
final ArrayList<ApprovalCategoryValue> vals;
cat = new ApprovalCategory(ApprovalCategory.PUSH_TAG, "Push Annotated Tag");
cat = new ApprovalCategory(ApprovalCategory.PUSH_TAG, "Push Tag");
cat.setPosition((short) -1);
cat.setFunctionName(NoOpFunction.NAME);
vals = new ArrayList<ApprovalCategoryValue>();
vals.add(value(cat, ApprovalCategory.PUSH_TAG_SIGNED, "Create Signed Tag"));
vals.add(value(cat, ApprovalCategory.PUSH_TAG_ANNOTATED,
"Create Annotated Tag"));
vals.add(value(cat, ApprovalCategory.PUSH_TAG_ANY, "Create Any Tag"));
c.approvalCategories().insert(Collections.singleton(cat));
c.approvalCategoryValues().insert(vals);
}
@@ -306,6 +306,32 @@ public class SchemaCreator {
c.approvalCategoryValues().insert(vals);
}
private void initForgeIdentityCategory(final ReviewDb c,
final SystemConfig sConfig) throws OrmException {
final ApprovalCategory cat;
final ArrayList<ApprovalCategoryValue> values;
cat =
new ApprovalCategory(ApprovalCategory.FORGE_IDENTITY, "Forge Identity");
cat.setPosition((short) -1);
cat.setFunctionName(NoOpFunction.NAME);
values = new ArrayList<ApprovalCategoryValue>();
values.add(value(cat, ApprovalCategory.FORGE_AUTHOR,
"Forge Author Identity"));
values.add(value(cat, ApprovalCategory.FORGE_COMMITTER,
"Forge Committer or Tagger Identity"));
c.approvalCategories().insert(Collections.singleton(cat));
c.approvalCategoryValues().insert(values);
RefRight right =
new RefRight(new RefRight.Key(sConfig.wildProjectName,
new RefRight.RefPattern("refs/*"), ApprovalCategory.FORGE_IDENTITY,
sConfig.registeredGroupId));
right.setMinValue(ApprovalCategory.FORGE_AUTHOR);
right.setMaxValue(ApprovalCategory.FORGE_AUTHOR);
c.refRights().insert(Collections.singleton(right));
}
private static ApprovalCategoryValue value(final ApprovalCategory cat,
final int value, final String name) {
return new ApprovalCategoryValue(new ApprovalCategoryValue.Id(cat.getId(),

View File

@@ -32,7 +32,7 @@ import java.util.List;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
private static final Class<? extends SchemaVersion> C = Schema_27.class;
private static final Class<? extends SchemaVersion> C = Schema_28.class;
public static class Module extends AbstractModule {
@Override

View File

@@ -0,0 +1,105 @@
// Copyright (C) 2010 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.server.schema;
import com.google.gerrit.reviewdb.ApprovalCategory;
import com.google.gerrit.reviewdb.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.RefRight;
import com.google.gerrit.reviewdb.ReviewDb;
import com.google.gerrit.reviewdb.SystemConfig;
import com.google.gerrit.server.workflow.NoOpFunction;
import com.google.gwtorm.client.OrmException;
import com.google.gwtorm.jdbc.JdbcSchema;
import com.google.gwtorm.schema.sql.DialectH2;
import com.google.gwtorm.schema.sql.DialectMySQL;
import com.google.gwtorm.schema.sql.DialectPostgreSQL;
import com.google.gwtorm.schema.sql.SqlDialect;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Collections;
class Schema_28 extends SchemaVersion {
@Inject
Schema_28(Provider<Schema_27> prior) {
super(prior);
}
@Override
protected void migrateData(ReviewDb db) throws OrmException, SQLException {
final SystemConfig cfg = db.systemConfig().get(new SystemConfig.Key());
ApprovalCategory cat;
initForgeIdentityCategory(db, cfg);
// Don't grant FORGE_COMMITTER to existing PUSH_HEAD rights. That
// is considered a bug that we are fixing with this schema upgrade.
// Administrators might need to relax permissions manually after the
// upgrade if that forgery is critical to their workflow.
cat = db.approvalCategories().get(ApprovalCategory.PUSH_TAG);
if (cat != null && "Push Annotated Tag".equals(cat.getName())) {
cat.setName("Push Tag");
db.approvalCategories().update(Collections.singleton(cat));
}
// Since we deleted Push Tags +3, drop anything using +3 down to +2.
//
Statement stmt = ((JdbcSchema) db).getConnection().createStatement();
try {
stmt.execute("UPDATE ref_rights SET max_value = "
+ ApprovalCategory.PUSH_TAG_ANNOTATED + " WHERE max_value >= 3");
stmt.execute("UPDATE ref_rights SET min_value = "
+ ApprovalCategory.PUSH_TAG_ANNOTATED + " WHERE min_value >= 3");
} finally {
stmt.close();
}
}
private void initForgeIdentityCategory(final ReviewDb c,
final SystemConfig sConfig) throws OrmException {
final ApprovalCategory cat;
final ArrayList<ApprovalCategoryValue> values;
cat =
new ApprovalCategory(ApprovalCategory.FORGE_IDENTITY, "Forge Identity");
cat.setPosition((short) -1);
cat.setFunctionName(NoOpFunction.NAME);
values = new ArrayList<ApprovalCategoryValue>();
values.add(value(cat, ApprovalCategory.FORGE_AUTHOR,
"Forge Author Identity"));
values.add(value(cat, ApprovalCategory.FORGE_COMMITTER,
"Forge Committer or Tagger Identity"));
c.approvalCategories().insert(Collections.singleton(cat));
c.approvalCategoryValues().insert(values);
RefRight right =
new RefRight(new RefRight.Key(sConfig.wildProjectName,
new RefRight.RefPattern("refs/*"), ApprovalCategory.FORGE_IDENTITY,
sConfig.registeredGroupId));
right.setMinValue(ApprovalCategory.FORGE_AUTHOR);
right.setMaxValue(ApprovalCategory.FORGE_AUTHOR);
c.refRights().insert(Collections.singleton(right));
}
private static ApprovalCategoryValue value(final ApprovalCategory cat,
final int value, final String name) {
return new ApprovalCategoryValue(new ApprovalCategoryValue.Id(cat.getId(),
(short) value), name);
}
}