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 4facd6e00..ea5aaf5dd 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.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 void testFindNextOffsetWrongOffset() { } } + @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 b4849f99f..38b28b501 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.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; @@ -146,6 +149,26 @@ public void read_objectCountTooLarge() { () -> null)); } + @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 void findObjectByPosition_badOffset() { () -> 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( diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index bf8a1ef5d..c73d85f07 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -223,6 +223,7 @@ corruptObjectTruncatedInMode=truncated in mode corruptObjectTruncatedInName=truncated in name corruptObjectTruncatedInObjectId=truncated in object id corruptObjectZeroId=entry points to null SHA-1 +corruptReverseIndexChecksumIncorrect=Reverse index checksum incorrect: written as {0} but digest was {1} corruptUseCnt=close() called when useCnt is already zero for {0} couldNotGetAdvertisedRef=Remote {0} did not advertise Ref for branch {1}. This Ref may not exist in the remote or may be hidden by permission settings. couldNotGetRepoStatistics=Could not get repository statistics @@ -566,7 +567,7 @@ openingConnection=Opening connection operationCanceled=Operation {0} was canceled outputHasAlreadyBeenStarted=Output has already been started. overflowedReftableBlock=Overflowed reftable block -packChecksumMismatch=Pack checksum mismatch detected for pack file {0}: .pack has {1} whilst .idx has {2} +packChecksumMismatch=Pack checksum mismatch detected for pack file {0}: {1} has {2} whilst {3} has {4} packCorruptedWhileWritingToFilesystem=Pack corrupted while writing to filesystem packedRefsHandleIsStale=packed-refs handle is stale, {0}. retry packetSizeMustBeAtLeast=packet size {0} must be >= {1} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index a032d24c7..91d53220a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -252,6 +252,7 @@ public static JGitText get() { /***/ public String corruptObjectTruncatedInName; /***/ public String corruptObjectTruncatedInObjectId; /***/ public String corruptObjectZeroId; + /***/ public String corruptReverseIndexChecksumIncorrect; /***/ public String corruptPack; /***/ public String corruptUseCnt; /***/ public String couldNotFindTabInLine; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java index 782cbea05..8d9fe23ae 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java @@ -50,12 +50,14 @@ import org.eclipse.jgit.errors.UnsupportedPackVersionException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.pack.BinaryDelta; +import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.internal.storage.pack.PackOutputStream; import org.eclipse.jgit.lib.AbbreviatedObjectId; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; +import org.eclipse.jgit.util.Hex; import org.eclipse.jgit.util.LongList; import org.eclipse.jgit.util.NB; import org.eclipse.jgit.util.RawParseUtils; @@ -177,10 +179,10 @@ private PackIndex idx() throws IOException { throw new PackMismatchException(MessageFormat .format(JGitText.get().packChecksumMismatch, packFile.getPath(), - ObjectId.fromRaw(packChecksum) - .name(), - ObjectId.fromRaw(idx.packChecksum) - .name())); + PackExt.PACK.getExtension(), + Hex.toHexString(packChecksum), + PackExt.INDEX.getExtension(), + Hex.toHexString(idx.packChecksum))); } loadedIdx = idx; } catch (InterruptedIOException e) { @@ -765,11 +767,11 @@ private void onOpenPack() throws IOException { fd.seek(length - 20); fd.readFully(buf, 0, 20); if (!Arrays.equals(buf, packChecksum)) { - throw new PackMismatchException(MessageFormat.format( - JGitText.get().packChecksumMismatch, - getPackFile(), - ObjectId.fromRaw(buf).name(), - ObjectId.fromRaw(idx.packChecksum).name())); + throw new PackMismatchException( + MessageFormat.format(JGitText.get().packChecksumMismatch, + getPackFile(), PackExt.PACK.getExtension(), + Hex.toHexString(buf), PackExt.INDEX.getExtension(), + Hex.toHexString(idx.packChecksum))); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndex.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndex.java index 74ea89025..ef9753cd7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndex.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndex.java @@ -11,6 +11,7 @@ package org.eclipse.jgit.internal.storage.file; import org.eclipse.jgit.errors.CorruptObjectException; +import org.eclipse.jgit.errors.PackMismatchException; import org.eclipse.jgit.lib.ObjectId; /** @@ -34,6 +35,17 @@ public interface PackReverseIndex { */ int VERSION_1 = 1; + /** + * Verify that the pack checksum found in the reverse index matches that + * from the pack file. + * + * @param packFilePath + * the path to display in event of a mismatch + * @throws PackMismatchException + * if the checksums do not match + */ + void verifyPackChecksum(String packFilePath) throws PackMismatchException; + /** * Search for object id with the specified start offset in this pack * (reverse) index. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputed.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputed.java index d6eaa965d..0b487a281 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputed.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputed.java @@ -12,6 +12,7 @@ import java.text.MessageFormat; import org.eclipse.jgit.errors.CorruptObjectException; +import org.eclipse.jgit.errors.PackMismatchException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry; import org.eclipse.jgit.lib.ObjectId; @@ -143,6 +144,12 @@ final class PackReverseIndexComputed implements PackReverseIndex { } } + @Override + public void verifyPackChecksum(String packFilePath) + throws PackMismatchException { + // There is no file with a checksum. + } + @Override public ObjectId findObject(long offset) { final int ith = binarySearch(offset); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1.java index 76f0793cc..c77a8eb76 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1.java @@ -16,10 +16,14 @@ import java.io.UncheckedIOException; import java.security.DigestInputStream; import java.text.MessageFormat; +import java.util.Arrays; import org.eclipse.jgit.errors.CorruptObjectException; +import org.eclipse.jgit.errors.PackMismatchException; import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.util.Hex; import org.eclipse.jgit.util.IO; /** @@ -89,7 +93,20 @@ final class PackReverseIndexV1 implements PackReverseIndex { parseChecksums(); } - private void parseBody() throws IOException { + @Override + public void verifyPackChecksum(String packFilePath) + throws PackMismatchException { + if (!Arrays.equals(packChecksum, getPackIndex().getChecksum())) { + throw new PackMismatchException( + MessageFormat.format(JGitText.get().packChecksumMismatch, + packFilePath, PackExt.INDEX.getExtension(), + Hex.toHexString(getPackIndex().getChecksum()), + PackExt.REVERSE_INDEX.getExtension(), + Hex.toHexString(packChecksum))); + } + } + + private void parseBody() throws IOException { for (int i = 0; i < objectCount; i++) { indexPositionsSortedByOffset[i] = dataIn.readInt(); } @@ -98,10 +115,19 @@ private void parseBody() throws IOException { private void parseChecksums() throws IOException { packChecksum = new byte[SHA1_BYTES]; IO.readFully(inputStream, packChecksum); - // TODO: verify checksum + + // Take digest before reading the self checksum changes it. + byte[] observedSelfChecksum = inputStream.getMessageDigest().digest(); byte[] readSelfChecksum = new byte[SHA1_BYTES]; IO.readFully(inputStream, readSelfChecksum); + + if (!Arrays.equals(readSelfChecksum, observedSelfChecksum)) { + throw new CorruptObjectException(MessageFormat.format( + JGitText.get().corruptReverseIndexChecksumIncorrect, + Hex.toHexString(readSelfChecksum), + Hex.toHexString(observedSelfChecksum))); + } } @Override