diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index b016087594..fab71dda4c 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -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]] diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java index 80144c41cd..9968e2a633 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java @@ -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 diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java index 86a6ef82c6..ae8c6bf261 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java @@ -50,6 +50,20 @@ public abstract class CurrentUser { accessPath = path; } + /** + * Identity of the authenticated user. + *

+ * In the normal case where a user authenticates as themselves + * {@code getRealUser() == this}. + *

+ * 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. *

@@ -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; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java index 3826293a2f..8e61c9a86c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java @@ -97,13 +97,20 @@ public class IdentifiedUser extends CurrentUser { public IdentifiedUser create(Provider 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 starredChanges; private Collection 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 remotePeerProvider, - @Nullable final Provider dbProvider, final Account.Id id) { + @Nullable final Provider 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. diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java index 4c5bd97b87..193ea04dc9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java @@ -65,8 +65,12 @@ public class CapabilityControl { /** @return true if the user can administer this server. */ public boolean canAdministrateServer() { if (canAdministrateServer == null) { - canAdministrateServer = user instanceof PeerDaemonUser - || matchAny(capabilities.administrateServer, ALLOWED_RULE); + if (user.getRealUser() != user) { + canAdministrateServer = false; + } else { + canAdministrateServer = user instanceof PeerDaemonUser + || matchAny(capabilities.administrateServer, ALLOWED_RULE); + } } return canAdministrateServer; } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java index 54bb84c83c..298a0993bb 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java @@ -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 args) {