PackReverseIndex: verify checksums

The new version 1 file-based reverse index has a footer with the
checksum of the corresponding pack file and a checksum of its own
contents. The initial implementation doesn't enforce that the pack
checksum matches the checksum found in the forward index nor that the
self checksum matches the contents of the file just read in.

Offer a method for reverse index users to verify the checksums in a way
appropriate to the version being used. For the pre-existing computed
version, always succeed since it is not based on a file so there is no
possibility of corruption.

Check for corruption of the file itself during parsing the checksum
footer, by comparing the self checksum with the digest of the file
contents read.

Change-Id: I87ff3933cf1afa76663350400b616695e4966cb6
Signed-off-by: Anna Papitto <annapapitto@google.com>

2 files changed, 50 insertions, 0 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputedTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputedTest.java
index 4facd6e007..ea5aaf5dd4 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputedTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputedTest.java
@@ -17,6 +17,7 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import org.eclipse.jgit.errors.CorruptObjectException;
+import org.eclipse.jgit.errors.PackMismatchException;
 import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry;
 import org.eclipse.jgit.junit.JGitTestUtil;
 import org.eclipse.jgit.junit.RepositoryTestCase;
@@ -92,6 +93,12 @@ public class PackReverseIndexComputedTest extends RepositoryTestCase {
 		}
 	}
 
+	@Test
+	public void testVerifyChecksum() throws PackMismatchException {
+		// ComputedReverseIndex doesn't have a file containing a checksum.
+		reverseIdx.verifyPackChecksum(null);
+	}
+
 	private long findFirstOffset() {
 		long min = Long.MAX_VALUE;
 		for (MutableEntry me : idx)
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1Test.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1Test.java
index b4849f99f4..38b28b501b 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1Test.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1Test.java
@@ -15,11 +15,14 @@ import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThrows;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 
 import org.eclipse.jgit.errors.CorruptObjectException;
+import org.eclipse.jgit.errors.PackMismatchException;
 import org.eclipse.jgit.junit.JGitTestUtil;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.transport.PackedObjectInfo;
@@ -147,6 +150,26 @@ public class PackReverseIndexV1Test {
 	}
 
 	@Test
+	public void read_incorrectChecksum() {
+		byte[] badChecksum = new byte[] { 'R', 'I', 'D', 'X', // magic
+				0x00, 0x00, 0x00, 0x01, // file version
+				0x00, 0x00, 0x00, 0x01, // oid version
+				// pack checksum to copy into at byte 12
+				0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+				// checksum
+				(byte) 0xf2, 0x1a, 0x1a, (byte) 0xaa, 0x32, 0x2d, (byte) 0xb9,
+				(byte) 0xfd, 0x0f, (byte) 0xa5, 0x4c, (byte) 0xea, (byte) 0xcf,
+				(byte) 0xbb, (byte) 0x99, (byte) 0xde, (byte) 0xd3, 0x4e,
+				(byte) 0xb1, (byte) 0xee, // would be 0x74 if correct
+		};
+		System.arraycopy(FAKE_PACK_CHECKSUM, 0, badChecksum, 12,
+				FAKE_PACK_CHECKSUM.length);
+		ByteArrayInputStream in = new ByteArrayInputStream(badChecksum);
+		assertThrows(CorruptObjectException.class,
+				() -> PackReverseIndexFactory.readFromFile(in, 0, () -> null));
+	}
+
+	@Test
 	public void findObject_noObjects() {
 		assertNull(emptyReverseIndex.findObject(0));
 	}
@@ -235,6 +258,26 @@ public class PackReverseIndexV1Test {
 				() -> smallReverseIndex.findObjectByPosition(10));
 	}
 
+	@Test
+	public void verifyChecksum_match() throws IOException {
+		smallReverseIndex.verifyPackChecksum("smallPackFilePath");
+	}
+
+	@Test
+	public void verifyChecksum_mismatch() throws IOException {
+		ByteArrayInputStream in = new ByteArrayInputStream(NO_OBJECTS);
+		PackIndex mockForwardIndex = mock(PackIndex.class);
+		when(mockForwardIndex.getChecksum()).thenReturn(
+				new byte[] { 'D', 'I', 'F', 'F', 'P', 'A', 'C', 'K', 'C', 'H',
+						'E', 'C', 'K', 'S', 'U', 'M', '7', '8', '9', '0', });
+		PackReverseIndex reverseIndex = PackReverseIndexFactory.readFromFile(in,
+				0,
+				() -> mockForwardIndex);
+
+		assertThrows(PackMismatchException.class,
+				() -> reverseIndex.verifyPackChecksum("packFilePath"));
+	}
+
 	private static PackedObjectInfo objectInfo(String objectId, int type,
 			long offset) {
 		PackedObjectInfo objectInfo = new PackedObjectInfo(