Add integration test for git protocol version 2
Start gerrit server using StandaloneSiteTest and configure git client
connection using git-core client and SSH and HTTP protocols. The minimum
git-core version that supports git protocol v2 is 2.18.0. Check the
locally installed git version, and abort the test with assumption
violation if the version is older than 2.18.0.
Continue with the test and create the test project. To activate git
protocol version 2 for the target repository, protocol.version = 2
config option must be set.
Using Gerrit API set HTTP password for admin and non admin users, and
using ssh-keygen command generate private/public keys for admin and non
admin users, and using Gerrit API set the public SSH keys for admin and
non admin users. Execute git ls-remote command to list the refs from the
target repository, using SSH and HTTP protocols for admin and non admin
users. Given that the -c protocol.version=2 git option is specified and
given that the git protocol v2 is enabled unconditionally in server, git
protocol v2 communication is expected to take place and can be verified.
For verification the refs visibility is tested. This was the security
vulnerability that was not detected during fist attempt to activate Git
wire protocl v2 in Gerrit server:
1. Start a test Gerrit server
2. Enable support for git protocol v2 (not needed, as git wire protocol
v2 is activated per default)
3. Create a project with two branches: refs/heads/master and
refs/heads/secret
4. Remove read access for “Anonymous Users” on “refs/*” from the
All-Projects project
5. Setup the following ACL on the new project to make refs/heads/secret
only accessible by admins:
[access "refs/heads/master"]
read = group Registered Users
[access "refs/heads/secret"]
read = group Administrators
6. Clone repository with a non-admin user and do
‘git ls-remote origin’ -> refs/heads/secret branch is not listed
Test Plan:
$ bazel test javatests/com/google/gerrit/integration/git:git
Change-Id: Ica7d2b57b4296e1c39f93528f17bef799d5ac824
diff --git a/java/com/google/gerrit/acceptance/GitClientVersion.java b/java/com/google/gerrit/acceptance/GitClientVersion.java
new file mode 100644
index 0000000..acfb2fa
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/GitClientVersion.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2019 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.acceptance;
+
+import static java.util.stream.Collectors.joining;
+
+import java.util.stream.IntStream;
+
+/** Class to parse and represent version of git-core client */
+public class GitClientVersion implements Comparable<GitClientVersion> {
+ private final int v[];
+
+ /**
+ * Constructor to represent instance for minimum supported git-core version
+ *
+ * @param parts version passed as single digits
+ */
+ public GitClientVersion(int... parts) {
+ this.v = parts;
+ }
+
+ /**
+ * Parse the git-core version as returned by git version command
+ *
+ * @param version String returned by git version command
+ */
+ public GitClientVersion(String version) {
+ // "git version x.y.z"
+ String parts[] = version.split(" ")[2].split("\\.");
+ v = new int[parts.length];
+ for (int i = 0; i < parts.length; i++) {
+ v[i] = Integer.valueOf(parts[i]);
+ }
+ }
+
+ @Override
+ public int compareTo(GitClientVersion o) {
+ int m = Math.max(v.length, o.v.length);
+ for (int i = 0; i < m; i++) {
+ int l = i < v.length ? v[i] : 0;
+ int r = i < o.v.length ? o.v[i] : 0;
+ if (l != r) {
+ return l < r ? -1 : 1;
+ }
+ }
+ return 0;
+ }
+
+ @Override
+ public String toString() {
+ return IntStream.of(v).mapToObj(String::valueOf).collect(joining("."));
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
index 933c4e1..a095daa 100644
--- a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
+++ b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
@@ -15,16 +15,20 @@
package com.google.gerrit.acceptance;
import static com.google.common.truth.Truth.assertWithMessage;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
import static org.junit.Assert.fail;
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Streams;
+import com.google.common.io.ByteStreams;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.launcher.GerritLauncher;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.util.ManualRequestContext;
@@ -35,6 +39,9 @@
import com.google.inject.Injector;
import com.google.inject.Module;
import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InterruptedIOException;
import java.util.Arrays;
import java.util.Collections;
import org.eclipse.jgit.lib.Config;
@@ -59,10 +66,10 @@
private ServerContext(GerritServer server) throws Exception {
this.server = server;
Injector i = server.getTestInjector();
- if (adminId == null) {
- adminId = i.getInstance(AccountCreator.class).admin().id();
+ if (admin == null) {
+ admin = i.getInstance(AccountCreator.class).admin();
}
- ctx = i.getInstance(OneOffRequestContext.class).openAs(adminId);
+ ctx = i.getInstance(OneOffRequestContext.class).openAs(admin.id());
GerritApi gApi = i.getInstance(GerritApi.class);
try {
@@ -117,7 +124,7 @@
@Rule public RuleChain ruleChain = RuleChain.outerRule(tempSiteDir).around(testRunner);
protected SitePaths sitePaths;
- protected Account.Id adminId;
+ protected TestAccount admin;
private GerritServer.Description serverDesc;
private SystemReader oldSystemReader;
@@ -190,4 +197,33 @@
protected static void runGerrit(Iterable<String>... multiArgs) throws Exception {
runGerrit(Arrays.stream(multiArgs).flatMap(Streams::stream).toArray(String[]::new));
}
+
+ protected static String execute(
+ ImmutableList<String> cmd, File dir, ImmutableMap<String, String> env) throws IOException {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.directory(dir).redirectErrorStream(true);
+ pb.environment().putAll(env);
+ Process p = pb.start();
+ byte[] out;
+ try (InputStream in = p.getInputStream()) {
+ out = ByteStreams.toByteArray(in);
+ } finally {
+ p.getOutputStream().close();
+ }
+
+ int status;
+ try {
+ status = p.waitFor();
+ } catch (InterruptedException e) {
+ throw new InterruptedIOException(
+ "interrupted waiting for: " + Joiner.on(' ').join(pb.command()));
+ }
+
+ String result = new String(out, UTF_8);
+ if (status != 0) {
+ throw new IOException(result);
+ }
+
+ return result.trim();
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
index f8176a5..f633842 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
@@ -74,13 +74,13 @@
.containsExactly(changeId);
// Query account index
assertThat(gApi.accounts().query("admin").get().stream().map(a -> a._accountId))
- .containsExactly(adminId.get());
+ .containsExactly(admin.id().get());
// Query group index
assertThat(
gApi.groups().query("Group").withOption(MEMBERS).get().stream()
.flatMap(g -> g.members.stream())
.map(a -> a._accountId))
- .containsExactly(adminId.get());
+ .containsExactly(admin.id().get());
// Query project index
assertThat(gApi.projects().query(project.get()).get().stream().map(p -> p.name))
.containsExactly(project.get());
diff --git a/javatests/com/google/gerrit/integration/git/BUILD b/javatests/com/google/gerrit/integration/git/BUILD
new file mode 100644
index 0000000..6a6f5ad
--- /dev/null
+++ b/javatests/com/google/gerrit/integration/git/BUILD
@@ -0,0 +1,7 @@
+load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
+
+acceptance_tests(
+ srcs = glob(["*IT.java"]),
+ group = "git",
+ labels = ["git"],
+)
diff --git a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
new file mode 100644
index 0000000..4314a7a
--- /dev/null
+++ b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
@@ -0,0 +1,236 @@
+// Copyright (C) 2019 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.integration.git;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.deny;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.AccountCreator;
+import com.google.gerrit.acceptance.GerritServer.TestSshServerAddress;
+import com.google.gerrit.acceptance.GitClientVersion;
+import com.google.gerrit.acceptance.StandaloneSiteTest;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.inject.Inject;
+import java.io.File;
+import java.net.InetSocketAddress;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Constants;
+import org.junit.Test;
+
+@UseSsh
+public class GitProtocolV2IT extends StandaloneSiteTest {
+ private final String[] SSH_KEYGEN_CMD =
+ new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f"};
+ private final String[] GIT_LS_REMOTE =
+ new String[] {"git", "-c", "protocol.version=2", "ls-remote"};
+ private final String GIT_SSH_COMMAND =
+ "ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i";
+
+ @Inject private GerritApi gApi;
+ @Inject private AccountCreator accountCreator;
+ @Inject private ProjectOperations projectOperations;
+ @Inject private @TestSshServerAddress InetSocketAddress sshAddress;
+ @Inject private @GerritServerConfig Config config;
+
+ @Test
+ public void testGitWireProtocolV2WithSsh() throws Exception {
+ // Minimum required git-core version that supports wire protocol v2 is 2.18.0
+ GitClientVersion requiredGitVersion = new GitClientVersion(2, 18, 0);
+ GitClientVersion actualGitVersion =
+ new GitClientVersion(execute(ImmutableList.of("git", "version")));
+ // If not found, test succeeds with assumption violation
+ assume().that(actualGitVersion).isAtLeast(requiredGitVersion);
+
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+
+ // Create project
+ Project.NameKey project = Project.nameKey("foo");
+ gApi.projects().create(project.get());
+
+ // Set up project permission
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(deny(Permission.READ).ref("refs/*").group(SystemGroupBackend.ANONYMOUS_USERS))
+ .add(
+ allow(Permission.READ)
+ .ref("refs/heads/master")
+ .group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+
+ // Set protocol.version=2 in target repository
+ execute(
+ ImmutableList.of("git", "config", "protocol.version", "2"),
+ sitePaths.site_path.resolve("git").resolve(project.get() + Constants.DOT_GIT).toFile());
+
+ // Retrieve HTTP url
+ String url = config.getString("gerrit", null, "canonicalweburl");
+ String urlDestinationTemplate =
+ url.substring(0, 7)
+ + "%s:secret@"
+ + url.substring(7, url.length())
+ + "/a/"
+ + project.get();
+
+ // Retrieve SSH host and port
+ String sshDestinationTemplate =
+ "ssh://%s@" + sshAddress.getHostName() + ":" + sshAddress.getPort() + "/" + project.get();
+
+ // Admin user was already created by the base class
+ setUpUserAuthentication(admin.username());
+
+ // Create non-admin user
+ TestAccount user = accountCreator.user();
+ setUpUserAuthentication(user.username());
+
+ // Prepare data for new change on master branch
+ ChangeInput in = new ChangeInput(project.get(), "master", "Test public change");
+ in.newBranch = true;
+
+ // Create new change and retrieve SHA1 for the created patch set
+ String commit =
+ gApi.changes()
+ .id(gApi.changes().create(in).info().changeId)
+ .current()
+ .commit(false)
+ .commit;
+
+ // Prepare new change on secret branch
+ in = new ChangeInput(project.get(), "secret", "Test secret change");
+ in.newBranch = true;
+
+ // Create new change and retrieve SHA1 for the created patch set
+ String secretCommit =
+ gApi.changes()
+ .id(gApi.changes().create(in).info().changeId)
+ .current()
+ .commit(false)
+ .commit;
+
+ // Read refs from target repository using git wire protocol v2 over HTTP for admin user
+ String out =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_LS_REMOTE)
+ .add(String.format(urlDestinationTemplate, admin.username()))
+ .build(),
+ ImmutableMap.of("GIT_TRACE_PACKET", "1"));
+
+ assertGitProtocolV2Refs(commit, out);
+ assertThat(out).contains(secretCommit);
+
+ // Read refs from target repository using git wire protocol v2 over SSH for admin user
+ out =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_LS_REMOTE)
+ .add(String.format(sshDestinationTemplate, admin.username()))
+ .build(),
+ ImmutableMap.of(
+ "GIT_SSH_COMMAND",
+ GIT_SSH_COMMAND
+ + sitePaths.data_dir.resolve(String.format("id_rsa_%s", admin.username())),
+ "GIT_TRACE_PACKET",
+ "1"));
+
+ assertGitProtocolV2Refs(commit, out);
+ assertThat(out).contains(secretCommit);
+
+ // Read refs from target repository using git wire protocol v2 over HTTP for non-admin user
+ out =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_LS_REMOTE)
+ .add(String.format(urlDestinationTemplate, user.username()))
+ .build(),
+ ImmutableMap.of("GIT_TRACE_PACKET", "1"));
+
+ assertGitProtocolV2Refs(commit, out);
+ assertThat(out).doesNotContain(secretCommit);
+
+ // Read refs from target repository using git wire protocol v2 over SSH for non-admin user
+ out =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_LS_REMOTE)
+ .add(String.format(sshDestinationTemplate, user.username()))
+ .build(),
+ ImmutableMap.of(
+ "GIT_SSH_COMMAND",
+ GIT_SSH_COMMAND
+ + sitePaths.data_dir.resolve(String.format("id_rsa_%s", user.username())),
+ "GIT_TRACE_PACKET",
+ "1"));
+
+ assertGitProtocolV2Refs(commit, out);
+ assertThat(out).doesNotContain(secretCommit);
+ }
+ }
+
+ private void setUpUserAuthentication(String username) throws Exception {
+ // Assign HTTP password to user
+ gApi.accounts().id(username).setHttpPassword("secret");
+
+ // Generate private/public key for user
+ execute(
+ ImmutableList.<String>builder()
+ .add(SSH_KEYGEN_CMD)
+ .add(String.format("id_rsa_%s", username))
+ .build());
+
+ // Read the content of generated public key and add it for the user in Gerrit
+ gApi.accounts()
+ .id(username)
+ .addSshKey(
+ new String(
+ java.nio.file.Files.readAllBytes(
+ sitePaths.data_dir.resolve(String.format("id_rsa_%s.pub", username))),
+ UTF_8));
+ }
+
+ private static void assertGitProtocolV2Refs(String commit, String out) {
+ assertThat(out).contains("git< version 2");
+ assertThat(out).contains("refs/changes/01/1/1");
+ assertThat(out).contains("refs/changes/01/1/meta");
+ assertThat(out).contains(commit);
+ }
+
+ private String execute(ImmutableList<String> cmd) throws Exception {
+ return execute(cmd, sitePaths.data_dir.toFile(), ImmutableMap.of());
+ }
+
+ private String execute(ImmutableList<String> cmd, ImmutableMap<String, String> env)
+ throws Exception {
+ return execute(cmd, sitePaths.data_dir.toFile(), env);
+ }
+
+ private static String execute(ImmutableList<String> cmd, File dir) throws Exception {
+ return execute(cmd, dir, ImmutableMap.of());
+ }
+}
