From f6e7913d143b4e35c6d17ec9b592d9820a7132d8 Mon Sep 17 00:00:00 2001
From: Antonio Barone <syntonyze@gmail.com>
Date: Mon, 8 Oct 2018 15:17:01 +0100
Subject: Fix access path propagation on git/ssh protocol

Honour force push ACL (and potentially other operations) by propagating
SshScope context which includes the original identity of the calling user
with its associated access path.

Previously the code was relying on ThreadLocal and thus was able to propagate
the context from parent to child threads. Now it needs to be done explicitly.

Bug: Issue 9823
Change-Id: I9eef144eee0d5831d38c4174e22018458db67669
---
 .../com/google/gerrit/sshd/AbstractGitCommand.java | 44 ++++++++++------------
 .../java/com/google/gerrit/sshd/BaseCommand.java   | 40 ++++++++++++++++----
 2 files changed, 52 insertions(+), 32 deletions(-)

diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
index 4144ed2d8b..b13a4d2597 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
@@ -31,8 +31,6 @@ public abstract class AbstractGitCommand extends BaseCommand {
   @Argument(index = 0, metaVar = "PROJECT.git", required = true, usage = "project name")
   protected ProjectControl projectControl;
 
-  @Inject private SshScope sshScope;
-
   @Inject private GitRepositoryManager repoManager;
 
   @Inject private SshSession session;
@@ -49,29 +47,25 @@ public abstract class AbstractGitCommand extends BaseCommand {
   @Override
   public void start(final Environment env) {
     Context ctx = context.subContext(newSession(), context.getCommandLine());
-    final Context old = sshScope.set(ctx);
-    try {
-      startThread(
-          new ProjectCommandRunnable() {
-            @Override
-            public void executeParseCommand() throws Exception {
-              parseCommandLine();
-            }
-
-            @Override
-            public void run() throws Exception {
-              AbstractGitCommand.this.service();
-            }
-
-            @Override
-            public Project.NameKey getProjectName() {
-              Project project = projectControl.getProjectState().getProject();
-              return project.getNameKey();
-            }
-          });
-    } finally {
-      sshScope.set(old);
-    }
+    startThreadWithContext(
+        ctx,
+        new ProjectCommandRunnable() {
+          @Override
+          public void executeParseCommand() throws Exception {
+            parseCommandLine();
+          }
+
+          @Override
+          public void run() throws Exception {
+            AbstractGitCommand.this.service();
+          }
+
+          @Override
+          public Project.NameKey getProjectName() {
+            Project project = projectControl.getProjectState().getProject();
+            return project.getNameKey();
+          }
+        });
   }
 
   private SshSession newSession() {
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java
index c83bcc7683..1f030de969 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java
@@ -43,6 +43,7 @@ import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.nio.charset.Charset;
+import java.util.Optional;
 import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicReference;
 import org.apache.sshd.common.SshException;
@@ -268,12 +269,12 @@ public abstract class BaseCommand implements Command {
   }
 
   /**
-   * Spawn a function into its own thread.
+   * Spawn a function into its own thread with the provided context.
    *
    * <p>Typically this should be invoked within {@link Command#start(Environment)}, such as:
    *
    * <pre>
-   * startThread(new CommandRunnable() {
+   * startThreadWithContext(SshScope.Context context, new CommandRunnable() {
    *   public void run() throws Exception {
    *     runImp();
    *   }
@@ -285,8 +286,8 @@ public abstract class BaseCommand implements Command {
    *
    * @param thunk the runnable to execute on the thread, performing the command's logic.
    */
-  protected void startThread(final CommandRunnable thunk) {
-    final TaskThunk tt = new TaskThunk(thunk);
+  protected void startThreadWithContext(SshScope.Context context, final CommandRunnable thunk) {
+    final TaskThunk tt = new TaskThunk(thunk, Optional.ofNullable(context));
 
     if (isAdminHighPriorityCommand() && user.getCapabilities().canAdministrateServer()) {
       // Admin commands should not block the main work threads (there
@@ -299,6 +300,28 @@ public abstract class BaseCommand implements Command {
     }
   }
 
+  /**
+   * Spawn a function into its own thread.
+   *
+   * <p>Typically this should be invoked within {@link Command#start(Environment)}, such as:
+   *
+   * <pre>
+   * startThread(new CommandRunnable() {
+   *   public void run() throws Exception {
+   *     runImp();
+   *   }
+   * });
+   * </pre>
+   *
+   * <p>If the function throws an exception, it is translated to a simple message for the client, a
+   * non-zero exit code, and the stack trace is logged.
+   *
+   * @param thunk the runnable to execute on the thread, performing the command's logic.
+   */
+  protected void startThread(final CommandRunnable thunk) {
+    startThreadWithContext(null, thunk);
+  }
+
   private boolean isAdminHighPriorityCommand() {
     return getClass().getAnnotation(AdminHighPriorityCommand.class) != null;
   }
@@ -413,18 +436,21 @@ public abstract class BaseCommand implements Command {
 
   private final class TaskThunk implements CancelableRunnable, ProjectRunnable {
     private final CommandRunnable thunk;
+    private final Context taskContext;
     private final String taskName;
+
     private Project.NameKey projectName;
 
-    private TaskThunk(final CommandRunnable thunk) {
+    private TaskThunk(final CommandRunnable thunk, Optional<Context> oneOffContext) {
       this.thunk = thunk;
       this.taskName = getTaskName();
+      this.taskContext = oneOffContext.orElse(context);
     }
 
     @Override
     public void cancel() {
       synchronized (this) {
-        final Context old = sshScope.set(context);
+        final Context old = sshScope.set(taskContext);
         try {
           onExit(STATUS_CANCEL);
         } finally {
@@ -439,7 +465,7 @@ public abstract class BaseCommand implements Command {
         final Thread thisThread = Thread.currentThread();
         final String thisName = thisThread.getName();
         int rc = 0;
-        final Context old = sshScope.set(context);
+        final Context old = sshScope.set(taskContext);
         try {
           context.started = TimeUtil.nowMs();
           thisThread.setName("SSH " + taskName);
-- 
cgit v1.2.3