From 53b0e7ffb506fe80e016368c2fce20218f806627 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 29 Jan 2010 10:29:38 -0800 Subject: [PATCH] Require branch deletion permission for pushes over HTTP Since smart HTTP can perform a branch deletion over HTTP requests, we need to disambiguate web requests from the web UI from HTTP requests coming from a git client tool such as git push. Moving all git commands into the AccessPath.GIT category and making a different category for the web UI allows us to tell these apart, so we can correctly require delete branch permission when removing a branch through a git command. This is a safety feature to prevent project owners from accidentally creating or deleting branches over git push, even though they can do this through the web UI without additional access controls. Bug: issue 393 Change-Id: I14cc68e31f5263913f5d9715a8f2241b5766bf23 Signed-off-by: Shawn O. Pearce Reviewed-by: Nico Sallembien --- .../gerrit/httpd/ProjectAccessPathFilter.java | 55 +++++++++++++++++++ .../com/google/gerrit/httpd/UrlModule.java | 1 + .../com/google/gerrit/httpd/WebSession.java | 8 ++- .../com/google/gerrit/server/AccessPath.java | 11 ++-- .../google/gerrit/server/PeerDaemonUser.java | 2 +- .../gerrit/server/project/RefControl.java | 6 +- .../gerrit/sshd/DatabasePasswordAuth.java | 13 +++-- .../gerrit/sshd/DatabasePubKeyAuth.java | 13 +++-- .../java/com/google/gerrit/sshd/SuExec.java | 2 +- 9 files changed, 89 insertions(+), 22 deletions(-) create mode 100644 gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectAccessPathFilter.java diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectAccessPathFilter.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectAccessPathFilter.java new file mode 100644 index 0000000000..934c4a5866 --- /dev/null +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectAccessPathFilter.java @@ -0,0 +1,55 @@ +// 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.httpd; + +import com.google.gerrit.server.AccessPath; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +import java.io.IOException; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; + +/** Set the WebSession to {@link AccessPath#GIT}. */ +@Singleton +class ProjectAccessPathFilter implements Filter { + private final Provider session; + + @Inject + ProjectAccessPathFilter(final Provider session) { + this.session = session; + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, + FilterChain chain) throws IOException, ServletException { + session.get().setAccessPath(AccessPath.GIT); + chain.doFilter(request, response); + } + + @Override + public void init(FilterConfig config) { + } + + @Override + public void destroy() { + } +} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/UrlModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/UrlModule.java index b7553b1745..7ffd3faafb 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/UrlModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/UrlModule.java @@ -50,6 +50,7 @@ class UrlModule extends ServletModule { serve("/ssh_info").with(SshInfoServlet.class); serve("/static/*").with(StaticServlet.class); + filter("/p/*").through(ProjectAccessPathFilter.class); filter("/p/*").through(ProjectDigestFilter.class); serve("/p/*").with(ProjectServlet.class); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java index ee3e987b95..90ccdc5083 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java @@ -64,6 +64,7 @@ public final class WebSession { private final WebSessionManager manager; private final AnonymousUser anonymous; private final IdentifiedUser.RequestFactory identified; + private AccessPath accessPath = AccessPath.WEB_UI; private Cookie outCookie; private Key key; @@ -132,7 +133,7 @@ public final class WebSession { CurrentUser getCurrentUser() { if (isSignedIn()) { - return identified.create(AccessPath.WEB, val.getAccountId()); + return identified.create(accessPath, val.getAccountId()); } return anonymous; } @@ -148,6 +149,11 @@ public final class WebSession { saveCookie(); } + /** Change the access path from the default of {@link AccessPath#WEB_UI}. */ + void setAccessPath(AccessPath path) { + accessPath = path; + } + /** Set the user account for this current request only. */ void setUserAccountId(Account.Id id) { key = new Key("id:" + id); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/AccessPath.java b/gerrit-server/src/main/java/com/google/gerrit/server/AccessPath.java index 8188824904..4d1d6319dd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/AccessPath.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/AccessPath.java @@ -19,11 +19,14 @@ public enum AccessPath { /** An unknown access path, probably should not be special. */ UNKNOWN, - /** Access through the web interface. */ - WEB, + /** Access through the web UI. */ + WEB_UI, - /** Access through an SSH command, e.g. git fetch or push. */ - SSH, + /** Access through an SSH command that is not invoked by Git. */ + SSH_COMMAND, + + /** Access from a Git client using any Git protocol. */ + GIT, /** Access through replication */ REPLICATION; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/PeerDaemonUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/PeerDaemonUser.java index f5b8f26350..26e510f9d3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/PeerDaemonUser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/PeerDaemonUser.java @@ -39,7 +39,7 @@ public class PeerDaemonUser extends CurrentUser { @Inject protected PeerDaemonUser(AuthConfig authConfig, @Assisted SocketAddress peer) { - super(AccessPath.SSH, authConfig); + super(AccessPath.SSH_COMMAND, authConfig); final HashSet g = new HashSet(); g.add(authConfig.getAdministratorsGroup()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java index b19cbf4725..efc1e364ed 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java @@ -121,7 +121,7 @@ public class RefControl { public boolean canCreate(RevWalk rw, RevObject object) { boolean owner; switch (getCurrentUser().getAccessPath()) { - case WEB: + case WEB_UI: owner = isOwner(); break; @@ -179,10 +179,10 @@ public class RefControl { */ public boolean canDelete() { switch (getCurrentUser().getAccessPath()) { - case WEB: + case WEB_UI: return isOwner() || canPerform(PUSH_HEAD, PUSH_HEAD_REPLACE); - case SSH: + case GIT: return canPerform(PUSH_HEAD, PUSH_HEAD_REPLACE); default: diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePasswordAuth.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePasswordAuth.java index aff64216a9..a346ab6087 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePasswordAuth.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePasswordAuth.java @@ -105,11 +105,12 @@ class DatabasePasswordAuth implements PasswordAuthenticator { private IdentifiedUser createUser(final SshSession sd, final AccountState state) { - return userFactory.create(AccessPath.SSH, new Provider() { - @Override - public SocketAddress get() { - return sd.getRemoteAddress(); - } - }, state.getAccount().getId()); + return userFactory.create(AccessPath.SSH_COMMAND, + new Provider() { + @Override + public SocketAddress get() { + return sd.getRemoteAddress(); + } + }, state.getAccount().getId()); } } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java index fa320b74b9..6c96b702f1 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java @@ -184,12 +184,13 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator { private IdentifiedUser createUser(final SshSession sd, final SshKeyCacheEntry key) { - return userFactory.create(AccessPath.SSH, new Provider() { - @Override - public SocketAddress get() { - return sd.getRemoteAddress(); - } - }, key.getAccount()); + return userFactory.create(AccessPath.SSH_COMMAND, + new Provider() { + @Override + public SocketAddress get() { + return sd.getRemoteAddress(); + } + }, key.getAccount()); } private SshKeyCacheEntry find(final Iterable keyList, 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 6300a7e2aa..620e389bc5 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 @@ -114,7 +114,7 @@ public final class SuExec extends BaseCommand { } return new SshSession(session.get(), peer, userFactory.create( - AccessPath.SSH, new Provider() { + AccessPath.SSH_COMMAND, new Provider() { @Override public SocketAddress get() { return peer;