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>

org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java

@@ -64,7 +64,6 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.eclipse.jgit.internal.storage.dfs.DfsRepository;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
@@ -276,12 +275,11 @@ private static String etag(byte[] content) {
 	}
 
 	static String identify(Repository git) {
-		if (git instanceof DfsRepository) {
-			return ((DfsRepository) git).getDescription().getRepositoryName();
-		} else if (git.getDirectory() != null) {
-			return git.getDirectory().getPath();
+		String identifier = git.getIdentifier();
+		if (identifier == null) {
+			return "unknown";
 		}
-		return "unknown";
+		return identifier;
 	}
 
 	private ServletUtils() {

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>

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsRepository.java

@@ -124,6 +124,12 @@ public StoredConfig getConfig() {
 		return config;
 	}
 
+	/** {@inheritDoc} */
+	@Override
+	public String getIdentifier() {
+		return getDescription().getRepositoryName();
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	public void scanForRepoChanges() throws IOException {

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java

@@ -388,6 +388,17 @@ public RefDatabase getRefDatabase() {
 		return refs;
 	}
 
+	/** {@inheritDoc} */
+	@Override
+	public String getIdentifier() {
+		File directory = getDirectory();
+		if (directory != null) {
+			return directory.getPath();
+		} else {
+			throw new IllegalStateException();
+		}
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	public FileBasedConfig getConfig() {

org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java

@@ -239,6 +239,15 @@ public File getDirectory() {
 		return gitDir;
 	}
 
+	/**
+	 * 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.
 	 *

org.eclipse.jgit/src/org/eclipse/jgit/transport/HMACSHA1NonceGenerator.java

@@ -45,14 +45,12 @@
 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 HMACSHA1NonceGenerator(String seed) throws IllegalStateException {
 	@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$
 	}