Disable administrative permissions during X-Gerrit-RunAs
When executing an action on behalf of an administrator, disable the administrateServer capability during the request. This may limit the damage a compromised role account could cause by avoiding any permissions that are not explicitly granted. Change-Id: I263e1d8e1a645617842f11b7712f79f5c009c6ca
This commit is contained in:
@@ -1271,10 +1271,21 @@ command, but also to the web UI results pagination size.
|
||||
Run As
|
||||
~~~~~~
|
||||
|
||||
Allow users to impersonate any other user with the X-Gerrit-RunAs HTTP
|
||||
header on REST API calls or the link:cmd-suexec.html[suexec] SSH
|
||||
command. Site administrators do not inherit this capability; it must
|
||||
be granted explicitly.
|
||||
Allow users to impersonate any other user with the `X-Gerrit-RunAs`
|
||||
HTTP header on REST API calls, or the link:cmd-suexec.html[suexec]
|
||||
SSH command.
|
||||
|
||||
When impersonating an administrator the Administrate Server capability
|
||||
is not honored. This security feature tries to prevent a role with
|
||||
Run As capability from modifying the access controls in All-Projects,
|
||||
however modification may still be possible if the impersonated user
|
||||
has permission to push or submit changes on `refs/meta/config`. Run
|
||||
As also blocks using most capabilities including Create User, Run
|
||||
Garbage Collection, etc., unless the capability is also explicitly
|
||||
granted to a group the administrator is a member of.
|
||||
|
||||
Administrators do not automatically inherit this capability; it must
|
||||
be explicitly granted.
|
||||
|
||||
|
||||
[[capability_runGC]]
|
||||
|
@@ -185,7 +185,7 @@ public final class CacheBasedWebSession implements WebSession {
|
||||
public void setUserAccountId(Account.Id id) {
|
||||
key = new Key("id:" + id);
|
||||
val = new Val(id, 0, false, null, 0, null, null);
|
||||
user = null;
|
||||
user = identified.runAs(id, user);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@@ -50,6 +50,20 @@ public abstract class CurrentUser {
|
||||
accessPath = path;
|
||||
}
|
||||
|
||||
/**
|
||||
* Identity of the authenticated user.
|
||||
* <p>
|
||||
* In the normal case where a user authenticates as themselves
|
||||
* {@code getRealUser() == this}.
|
||||
* <p>
|
||||
* If {@code X-Gerrit-RunAs} or {@code suexec} was used this method returns
|
||||
* the identity of the account that has permission to act on behalf of this
|
||||
* user.
|
||||
*/
|
||||
public CurrentUser getRealUser() {
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the set of groups the user is currently a member of.
|
||||
* <p>
|
||||
@@ -76,11 +90,9 @@ public abstract class CurrentUser {
|
||||
|
||||
/** Capabilities available to this user account. */
|
||||
public CapabilityControl getCapabilities() {
|
||||
CapabilityControl ctl = capabilities;
|
||||
if (ctl == null) {
|
||||
ctl = capabilityControlFactory.create(this);
|
||||
capabilities = ctl;
|
||||
if (capabilities == null) {
|
||||
capabilities = capabilityControlFactory.create(this);
|
||||
}
|
||||
return ctl;
|
||||
return capabilities;
|
||||
}
|
||||
}
|
||||
|
@@ -97,13 +97,20 @@ public class IdentifiedUser extends CurrentUser {
|
||||
public IdentifiedUser create(Provider<ReviewDb> db, Account.Id id) {
|
||||
return new IdentifiedUser(capabilityControlFactory,
|
||||
authConfig, anonymousCowardName, canonicalUrl, realm, accountCache,
|
||||
groupBackend, null, db, id);
|
||||
groupBackend, null, db, id, null);
|
||||
}
|
||||
|
||||
public IdentifiedUser create(SocketAddress remotePeer, Account.Id id) {
|
||||
return new IdentifiedUser(capabilityControlFactory,
|
||||
authConfig, anonymousCowardName, canonicalUrl, realm, accountCache,
|
||||
groupBackend, Providers.of(remotePeer), null, id);
|
||||
groupBackend, Providers.of(remotePeer), null, id, null);
|
||||
}
|
||||
|
||||
public CurrentUser runAs(SocketAddress remotePeer, Account.Id id,
|
||||
@Nullable CurrentUser caller) {
|
||||
return new IdentifiedUser(capabilityControlFactory,
|
||||
authConfig, anonymousCowardName, canonicalUrl, realm, accountCache,
|
||||
groupBackend, Providers.of(remotePeer), null, id, caller);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -152,7 +159,13 @@ public class IdentifiedUser extends CurrentUser {
|
||||
public IdentifiedUser create(Account.Id id) {
|
||||
return new IdentifiedUser(capabilityControlFactory,
|
||||
authConfig, anonymousCowardName, canonicalUrl, realm, accountCache,
|
||||
groupBackend, remotePeerProvider, dbProvider, id);
|
||||
groupBackend, remotePeerProvider, dbProvider, id, null);
|
||||
}
|
||||
|
||||
public IdentifiedUser runAs(Account.Id id, CurrentUser caller) {
|
||||
return new IdentifiedUser(capabilityControlFactory,
|
||||
authConfig, anonymousCowardName, canonicalUrl, realm, accountCache,
|
||||
groupBackend, remotePeerProvider, dbProvider, id, caller);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -183,6 +196,7 @@ public class IdentifiedUser extends CurrentUser {
|
||||
private GroupMembership effectiveGroups;
|
||||
private Set<Change.Id> starredChanges;
|
||||
private Collection<AccountProjectWatch> notificationFilters;
|
||||
private CurrentUser realUser;
|
||||
|
||||
private IdentifiedUser(
|
||||
CapabilityControl.Factory capabilityControlFactory,
|
||||
@@ -192,7 +206,9 @@ public class IdentifiedUser extends CurrentUser {
|
||||
final Realm realm, final AccountCache accountCache,
|
||||
final GroupBackend groupBackend,
|
||||
@Nullable final Provider<SocketAddress> remotePeerProvider,
|
||||
@Nullable final Provider<ReviewDb> dbProvider, final Account.Id id) {
|
||||
@Nullable final Provider<ReviewDb> dbProvider,
|
||||
final Account.Id id,
|
||||
@Nullable CurrentUser realUser) {
|
||||
super(capabilityControlFactory);
|
||||
this.canonicalUrl = canonicalUrl;
|
||||
this.accountCache = accountCache;
|
||||
@@ -202,6 +218,12 @@ public class IdentifiedUser extends CurrentUser {
|
||||
this.remotePeerProvider = remotePeerProvider;
|
||||
this.dbProvider = dbProvider;
|
||||
this.accountId = id;
|
||||
this.realUser = realUser != null ? realUser : this;
|
||||
}
|
||||
|
||||
@Override
|
||||
public CurrentUser getRealUser() {
|
||||
return realUser;
|
||||
}
|
||||
|
||||
// TODO(cranger): maybe get the state through the accountCache instead.
|
||||
|
@@ -65,9 +65,13 @@ public class CapabilityControl {
|
||||
/** @return true if the user can administer this server. */
|
||||
public boolean canAdministrateServer() {
|
||||
if (canAdministrateServer == null) {
|
||||
if (user.getRealUser() != user) {
|
||||
canAdministrateServer = false;
|
||||
} else {
|
||||
canAdministrateServer = user instanceof PeerDaemonUser
|
||||
|| matchAny(capabilities.administrateServer, ALLOWED_RULE);
|
||||
}
|
||||
}
|
||||
return canAdministrateServer;
|
||||
}
|
||||
|
||||
|
@@ -126,8 +126,12 @@ public final class SuExec extends BaseCommand {
|
||||
} else {
|
||||
peer = peerAddress;
|
||||
}
|
||||
CurrentUser self = caller.get();
|
||||
if (self instanceof PeerDaemonUser) {
|
||||
self = null;
|
||||
}
|
||||
return new SshSession(session.get(), peer,
|
||||
userFactory.create(peer, accountId));
|
||||
userFactory.runAs(peer, accountId, self));
|
||||
}
|
||||
|
||||
private static String join(List<String> args) {
|
||||
|
Reference in New Issue
Block a user