Hide refs/meta/config on git protocol v2
Fix a problem in the refs advertisement with git protocol v2
where the refs/meta/config was leaked even though the client
did not have any read access to it.
The leak was limited to the value of the SHA1 and not to the
actual content of the refs/meta/config branch. A fetch of the
SHA1 would have resulted in a WantNotValidException.
Bug: Issue 11962
Change-Id: I2e4d1d7c8d487166fe00e7d0946b6927bc1cd9c8
diff --git a/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
index 8f7e684..b80e543 100644
--- a/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
+++ b/java/com/google/gerrit/server/git/PermissionAwareReadOnlyRefDatabase.java
@@ -28,6 +28,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.function.Function;
+import java.util.stream.Collectors;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.lib.Ref;
@@ -101,11 +103,35 @@
Map<String, Ref> result;
try {
- result = forProject.filter(refs, getDelegate(), RefFilterOptions.defaults());
+ // The security filtering assumes to receive the same refMap, independently from the ref
+ // prefix offset
+ result =
+ forProject.filter(
+ prefixIndependentRefMap(prefix, refs), getDelegate(), RefFilterOptions.defaults());
} catch (PermissionBackendException e) {
throw new IOException("");
}
- return result;
+ return applyPrefixRefMap(prefix, result);
+ }
+
+ private Map<String, Ref> prefixIndependentRefMap(String prefix, Map<String, Ref> refs) {
+ if (prefix.length() > 0) {
+ return refs.values().stream().collect(Collectors.toMap(Ref::getName, Function.identity()));
+ }
+
+ return refs;
+ }
+
+ private Map<String, Ref> applyPrefixRefMap(String prefix, Map<String, Ref> refs) {
+ int prefixSlashPos = prefix.lastIndexOf('/') + 1;
+ if (prefixSlashPos > 0) {
+ return refs.values().stream()
+ .collect(
+ Collectors.toMap(
+ (Ref ref) -> ref.getName().substring(prefixSlashPos), Function.identity()));
+ }
+
+ return refs;
}
@Override
diff --git a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
index 4acd8b8..9c4bdef4 100644
--- a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
+++ b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
@@ -29,7 +29,10 @@
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.entities.Change;
+import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -48,6 +51,8 @@
new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f"};
private final String[] GIT_LS_REMOTE =
new String[] {"git", "-c", "protocol.version=2", "ls-remote", "-o", "trace=12345"};
+ private final String[] GIT_CLONE_MIRROR =
+ new String[] {"git", "-c", "protocol.version=2", "clone", "--mirror"};
private final String GIT_SSH_COMMAND =
"ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i";
@@ -197,6 +202,61 @@
}
}
+ @Test
+ public void testGitWireProtocolV2HidesRefMetaConfig() throws Exception {
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+ String url = config.getString("gerrit", null, "canonicalweburl");
+
+ // Create project
+ Project.NameKey allRefsVisibleProject = Project.nameKey("all-refs-visible");
+ gApi.projects().create(allRefsVisibleProject.get());
+
+ // Set protocol.version=2 in target repository
+ execute(
+ ImmutableList.of("git", "config", "protocol.version", "2"),
+ sitePaths
+ .site_path
+ .resolve("git")
+ .resolve(allRefsVisibleProject.get() + Constants.DOT_GIT)
+ .toFile());
+
+ // Set up project permission to allow reading all refs
+ projectOperations
+ .project(allRefsVisibleProject)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/heads/*").group(SystemGroupBackend.ANONYMOUS_USERS))
+ .add(
+ allow(Permission.READ)
+ .ref("refs/changes/*")
+ .group(SystemGroupBackend.ANONYMOUS_USERS))
+ .update();
+
+ // Create new change and retrieve refs for the created patch set
+ ChangeInput visibleChangeIn =
+ new ChangeInput(allRefsVisibleProject.get(), "master", "Test public change");
+ visibleChangeIn.newBranch = true;
+ int visibleChangeNumber = gApi.changes().create(visibleChangeIn).info()._number;
+ Change.Id changeId = Change.id(visibleChangeNumber);
+ String visibleChangeNumberRef = RefNames.patchSetRef(PatchSet.id(changeId, 1));
+ String visibleChangeNumberMetaRef = RefNames.changeMetaRef(changeId);
+
+ // Read refs from target repository using git wire protocol v2 over HTTP anonymously
+ String outAnonymousLsRemote =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_CLONE_MIRROR)
+ .add(url + "/" + allRefsVisibleProject.get())
+ .build(),
+ ImmutableMap.of("GIT_TRACE_PACKET", "1"));
+
+ assertThat(outAnonymousLsRemote).contains("git< version 2");
+ assertThat(outAnonymousLsRemote).doesNotContain(RefNames.REFS_CONFIG);
+ assertThat(outAnonymousLsRemote).contains(visibleChangeNumberRef);
+ assertThat(outAnonymousLsRemote).contains(visibleChangeNumberMetaRef);
+ }
+ }
+
private void setUpUserAuthentication(String username) throws Exception {
// Assign HTTP password to user
gApi.accounts().id(username).setHttpPassword("secret");
