Repository: Add getIdentifier() method to avoid instanceof operator

This change is needed to implement permission aware ref database in
Gerrit: [1], that is a pre-requisite to re-enable Git v2 protocol in
Gerrit: [2].

Background: Last year Git v2 protocol was enabled in Gerrit. The fact,
that JGit layer was not calling ref advertise filter for Git v2
protocol, introduced security vulnerability.

The lesson learned from this security incident: Gerrit should not rely
on ref advertise filter being called by JGit to implement crictical
security checks. Instead, the idea is to use the same approach as
currently used by Google's internal code on googlesource.com that
didn't suffer from this vulnerability: provide a custom repository to
JGit. The repository provides a RefDatabase that is permission-aware
and will only ever return refs that the user has access to.

However, due to hard coded instanceof operator usages in JGit code
base, some tests in Gerrit are failing with: [1] in place. This change
addresses this problem.

[1] https://gerrit-review.googlesource.com/c/gerrit/+/212874
[2] https://gerrit-review.googlesource.com/c/gerrit/+/226754

Change-Id: I67c0f53ca33b149442e7ee3e51910d19e3f348d5
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

5 files changed, 35 insertions, 15 deletions

diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index dc1df59633..7f93191ca6 100644
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -22,6 +22,14 @@
             </message_arguments>
         </filter>
     </resource>
+    <resource path="src/org/eclipse/jgit/lib/Repository.java" type="org.eclipse.jgit.lib.Repository">
+        <filter id="336695337">
+            <message_arguments>
+                <message_argument value="org.eclipse.jgit.lib.Repository"/>
+                <message_argument value="getIdentifier()"/>
+            </message_arguments>
+        </filter>
+    </resource>
     <resource path="src/org/eclipse/jgit/revwalk/ObjectWalk.java" type="org.eclipse.jgit.revwalk.ObjectWalk">
         <filter comment="ignore the risk subclasses could define the same field and cause a name clash" id="336658481">
             <message_arguments>
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsRepository.java
index 5169e929e4..8e5c5a7f75 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsRepository.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsRepository.java
@@ -126,6 +126,12 @@ public abstract class DfsRepository extends Repository {
 
 	/** {@inheritDoc} */
 	@Override
+	public String getIdentifier() {
+		return getDescription().getRepositoryName();
+	}
+
+	/** {@inheritDoc} */
+	@Override
 	public void scanForRepoChanges() throws IOException {
 		getRefDatabase().refresh();
 		getObjectDatabase().clearCache();
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java
index d82d29e4cf..90772970ae 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java
@@ -390,6 +390,17 @@ public class FileRepository extends Repository {
 
 	/** {@inheritDoc} */
 	@Override
+	public String getIdentifier() {
+		File directory = getDirectory();
+		if (directory != null) {
+			return directory.getPath();
+		} else {
+			throw new IllegalStateException();
+		}
+	}
+
+	/** {@inheritDoc} */
+	@Override
 	public FileBasedConfig getConfig() {
 		if (systemConfig.isOutdated()) {
 			try {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
index aac63e9d24..d53b0c926a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
@@ -240,6 +240,15 @@ public abstract class Repository implements AutoCloseable {
 	}
 
 	/**
+	 * Get repository identifier.
+	 *
+	 * @return repository identifier. The returned identifier has to be unique
+	 *         within a given Git server.
+	 * @since 5.4
+	 */
+	public abstract String getIdentifier();
+
+	/**
 	 * Get the object database which stores this repository's data.
 	 *
 	 * @return the object database which stores this repository's data.
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HMACSHA1NonceGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HMACSHA1NonceGenerator.java
index 53eaa6a7f9..01f6fec7e4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HMACSHA1NonceGenerator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HMACSHA1NonceGenerator.java
@@ -45,14 +45,12 @@ package org.eclipse.jgit.transport;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-import java.io.File;
 import java.security.InvalidKeyException;
 import java.security.NoSuchAlgorithmException;
 
 import javax.crypto.Mac;
 import javax.crypto.spec.SecretKeySpec;
 
-import org.eclipse.jgit.internal.storage.dfs.DfsRepository;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.PushCertificate.NonceStatus;
 
@@ -87,19 +85,7 @@ public class HMACSHA1NonceGenerator implements NonceGenerator {
 	@Override
 	public synchronized String createNonce(Repository repo, long timestamp)
 			throws IllegalStateException {
-		String path;
-		if (repo instanceof DfsRepository) {
-			path = ((DfsRepository) repo).getDescription().getRepositoryName();
-		} else {
-			File directory = repo.getDirectory();
-			if (directory != null) {
-				path = directory.getPath();
-			} else {
-				throw new IllegalStateException();
-			}
-		}
-
-		String input = path + ":" + String.valueOf(timestamp); //$NON-NLS-1$
+		String input = repo.getIdentifier() + ":" + String.valueOf(timestamp); //$NON-NLS-1$
 		byte[] rawHmac = mac.doFinal(input.getBytes(UTF_8));
 		return Long.toString(timestamp) + "-" + toHex(rawHmac); //$NON-NLS-1$
 	}