Revert "Revert "Allow to enable git protocol version 2 for upload pack""
Reason for revert: The original revert was done because of security
vulnerability in JGit. This vulnerability was fixed meantime.
Another argument for procrastination with Git v2 protocol support in
upcoming Gerrit release may be the fact that we do not have any
integration tests for execution of AdvertiseRefsHook hook.
However, this argument is not specific for git v2 protocol, as
mentioned in JGit 5.0 release: [1], but other Git protocol versions
were affected as well, and thus does not hold. In fact bidirectional
v0 HTTP protocol also has not executed AdvertiseRefsHook hook and
this was not discovered for 8 years and all released Gerrit versions
were affected.
Test Plan:
1. Install git version 2.18 or later
2. Activate git v2 protocol on the client side
3. Enable git v2 protocol support in gerrit, by setting this config:
receive.enableProtocolV2 = true
3. Run $ GIT_TRACE_PACKET=1 git ls-remote
4. Confirm that git v2 protocol is used
5. Confirm that the method: DefaultRefFilter#filter is get called
6. Confirm that the call stack looks sane: [2]
This reverts commit f51a97d523.
[1] https://projects.eclipse.org/projects/technology.jgit/releases/5.0.0
[2] http://paste.openstack.org/show/742267
Change-Id: I5e2e5c7c69cc1433bc8539a8c36cb2ebd70804fc
This commit is contained in:
committed by
Luca Milanesio
parent
40d2be69db
commit
fb64381f7c
@@ -3663,6 +3663,25 @@ link:https://www.gnupg.org/documentation/manuals/gnupg/OpenPGP-Key-Management.ht
|
||||
If no keys are specified, web-of-trust checks are disabled. This is the
|
||||
default behavior.
|
||||
|
||||
[[receive.enableProtocolV2]]receive.enableProtocolV2::
|
||||
+
|
||||
[NOTE]
|
||||
Support for Git protocol version 2 is still experimental. Do not use it in
|
||||
production!
|
||||
+
|
||||
Enable support for git protocol version 2.
|
||||
+
|
||||
When this option is enabled, clients may send upload pack using git
|
||||
protocol version 2.
|
||||
+
|
||||
The repository must also be configured on the server side to use protocol
|
||||
version 2 by setting `protocol.version = 2` either in the gerrit user's
|
||||
`~/.gitconfig` file (which will enable it for all repositories) or on
|
||||
a per repository basis by setting the option in the `.git/config` file
|
||||
of the repository.
|
||||
+
|
||||
Defaults to false, git protocol version 2 is not enabled.
|
||||
|
||||
[[repository]]
|
||||
=== Section repository
|
||||
|
||||
|
||||
@@ -51,6 +51,7 @@ import com.google.inject.TypeLiteral;
|
||||
import com.google.inject.name.Named;
|
||||
import java.io.IOException;
|
||||
import java.time.Duration;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.HashSet;
|
||||
import java.util.Map;
|
||||
@@ -327,6 +328,13 @@ public class GitOverHttpServlet extends GitServlet {
|
||||
up.setTimeout(config.getTimeout());
|
||||
up.setPreUploadHook(PreUploadHookChain.newChain(Lists.newArrayList(preUploadHooks)));
|
||||
up.setPostUploadHook(PostUploadHookChain.newChain(Lists.newArrayList(postUploadHooks)));
|
||||
if (config.enableProtocolV2()) {
|
||||
String header = req.getHeader("Git-Protocol");
|
||||
if (header != null) {
|
||||
String[] params = header.split(":");
|
||||
up.setExtraParameters(Arrays.asList(params));
|
||||
}
|
||||
}
|
||||
ProjectState state = (ProjectState) req.getAttribute(ATT_STATE);
|
||||
uploadPackInitializers.runEach(initializer -> initializer.init(state.getNameKey(), up));
|
||||
return up;
|
||||
|
||||
@@ -29,6 +29,7 @@ public class TransferConfig {
|
||||
private final long maxObjectSizeLimit;
|
||||
private final String maxObjectSizeLimitFormatted;
|
||||
private final boolean inheritProjectMaxObjectSizeLimit;
|
||||
private final boolean enableProtocolV2;
|
||||
|
||||
@Inject
|
||||
TransferConfig(@GerritServerConfig Config cfg) {
|
||||
@@ -45,6 +46,7 @@ public class TransferConfig {
|
||||
maxObjectSizeLimitFormatted = cfg.getString("receive", null, "maxObjectSizeLimit");
|
||||
inheritProjectMaxObjectSizeLimit =
|
||||
cfg.getBoolean("receive", "inheritProjectMaxObjectSizeLimit", false);
|
||||
enableProtocolV2 = cfg.getBoolean("receive", "enableProtocolV2", false);
|
||||
|
||||
packConfig = new PackConfig();
|
||||
packConfig.setDeltaCompress(false);
|
||||
@@ -72,4 +74,8 @@ public class TransferConfig {
|
||||
public boolean inheritProjectMaxObjectSizeLimit() {
|
||||
return inheritProjectMaxObjectSizeLimit;
|
||||
}
|
||||
|
||||
public boolean enableProtocolV2() {
|
||||
return enableProtocolV2;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,6 +29,8 @@ import org.eclipse.jgit.lib.Repository;
|
||||
import org.kohsuke.args4j.Argument;
|
||||
|
||||
public abstract class AbstractGitCommand extends BaseCommand {
|
||||
private static final String GIT_PROTOCOL = "GIT_PROTOCOL";
|
||||
|
||||
@Argument(index = 0, metaVar = "PROJECT.git", required = true, usage = "project name")
|
||||
protected ProjectState projectState;
|
||||
|
||||
@@ -45,9 +47,15 @@ public abstract class AbstractGitCommand extends BaseCommand {
|
||||
protected Repository repo;
|
||||
protected Project.NameKey projectName;
|
||||
protected Project project;
|
||||
protected String[] extraParameters;
|
||||
|
||||
@Override
|
||||
public void start(Environment env) {
|
||||
String gitProtocol = env.getEnv().get(GIT_PROTOCOL);
|
||||
if (gitProtocol != null) {
|
||||
extraParameters = gitProtocol.split(":");
|
||||
}
|
||||
|
||||
Context ctx = context.subContext(newSession(), context.getCommandLine());
|
||||
final Context old = sshScope.set(ctx);
|
||||
try {
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
|
||||
package com.google.gerrit.sshd.commands;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.Lists;
|
||||
import com.google.gerrit.extensions.registration.DynamicSet;
|
||||
import com.google.gerrit.extensions.restapi.AuthException;
|
||||
@@ -52,7 +53,6 @@ final class Upload extends AbstractGitCommand {
|
||||
PermissionBackend.ForProject perm =
|
||||
permissionBackend.user(user).project(projectState.getNameKey());
|
||||
try {
|
||||
|
||||
perm.check(ProjectPermission.RUN_UPLOAD_PACK);
|
||||
} catch (AuthException e) {
|
||||
throw new Failure(1, "fatal: upload-pack not permitted on this server");
|
||||
@@ -65,6 +65,9 @@ final class Upload extends AbstractGitCommand {
|
||||
up.setPackConfig(config.getPackConfig());
|
||||
up.setTimeout(config.getTimeout());
|
||||
up.setPostUploadHook(PostUploadHookChain.newChain(Lists.newArrayList(postUploadHooks)));
|
||||
if (config.enableProtocolV2() && extraParameters != null) {
|
||||
up.setExtraParameters(ImmutableList.copyOf(extraParameters));
|
||||
}
|
||||
|
||||
List<PreUploadHook> allPreUploadHooks = Lists.newArrayList(preUploadHooks);
|
||||
allPreUploadHooks.add(
|
||||
|
||||
Reference in New Issue
Block a user