opendev/gerrit
Code Issues Proposed changes

Fix access path propagation on git/ssh protocol

Browse Source 
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
This commit is contained in:
Antonio Barone
2018-10-08 15:17:01 +01:00
committed by David Pursehouse
parent 1f56ae4acb
commit f6e7913d14
2 changed files with 57 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
View File

@@ -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();
}
startThreadWithContext(
ctx,
new ProjectCommandRunnable() {
@Override
public void executeParseCommand() throws Exception {
parseCommandLine();
}
@Override
public void run() throws Exception {
AbstractGitCommand.this.service();
}
@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);
}
@Override
public Project.NameKey getProjectName() {
Project project = projectControl.getProjectState().getProject();
return project.getNameKey();
}
});
}
private SshSession newSession() {

54
gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java
View File

@@ -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;
@@ -267,6 +268,38 @@ public abstract class BaseCommand implements Command {
});
}
/**
* 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>
* startThreadWithContext(SshScope.Context context, 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 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
// might be an interactive shell there), nor should they wait
// for the main work threads.
//
new Thread(tt, tt.toString()).start();
} else {
task.set(executor.submit(tt));
}
}
/**
* Spawn a function into its own thread.
*
@@ -286,17 +319,7 @@ 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);
if (isAdminHighPriorityCommand() && user.getCapabilities().canAdministrateServer()) {
// Admin commands should not block the main work threads (there
// might be an interactive shell there), nor should they wait
// for the main work threads.
//
new Thread(tt, tt.toString()).start();
} else {
task.set(executor.submit(tt));
}
startThreadWithContext(null, thunk);
}
private boolean isAdminHighPriorityCommand() {
@@ -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);