From 0f25f64d4882f7853c9d3dc84ec382d8a78ae646 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Fri, 24 Feb 2017 23:33:18 -0800 Subject: [PATCH] Switch to pure Java SHA1 for ObjectId Generate names for objects using only the pure Java SHA1 implementation, but continue using MessageDigest in tests. This opens the possibility of changing the hashing function to incorporate additional safety measures, such as those used in sha1dc[1]. Since MessageDigest has higher throughput, continue using MessageDigest for computing pack, idx and DirCache trailers. These are less likely to be sensitive to SHAttered[2] types of attacks, as Git uses them to detect random bit flips during transfer, and not for content identity. [1] https://github.com/cr-marcstevens/sha1collisiondetection [2] https://shattered.it/ Change-Id: If6da98334201f7f20cb916e46f782c45f373784e --- .../internal/storage/dfs/DfsInserter.java | 5 +-- .../storage/file/ObjectDirectoryInserter.java | 33 +++++++++++++++---- .../org/eclipse/jgit/lib/MutableObjectId.java | 18 ++++++++++ .../src/org/eclipse/jgit/lib/ObjectId.java | 13 ++++++-- .../org/eclipse/jgit/lib/ObjectInserter.java | 19 ++++------- .../eclipse/jgit/transport/PackParser.java | 12 +++---- .../jgit/treewalk/WorkingTreeIterator.java | 15 +++------ .../src/org/eclipse/jgit/util/sha1/SHA1.java | 27 +++++++++++++++ 8 files changed, 104 insertions(+), 38 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java index 1551ad615..d3ac62650 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsInserter.java @@ -90,6 +90,7 @@ import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.NB; import org.eclipse.jgit.util.TemporaryBuffer; import org.eclipse.jgit.util.io.CountingOutputStream; +import org.eclipse.jgit.util.sha1.SHA1; /** Inserts objects into the DFS. */ public class DfsInserter extends ObjectInserter { @@ -168,7 +169,7 @@ public class DfsInserter extends ObjectInserter { } long offset = beginObject(type, len); - MessageDigest md = digest(); + SHA1 md = SHA1.newInstance(); md.update(Constants.encodedTypeString(type)); md.update((byte) ' '); md.update(Constants.encodeASCII(len)); @@ -183,7 +184,7 @@ public class DfsInserter extends ObjectInserter { len -= n; } packOut.compress.finish(); - return endObject(ObjectId.fromRaw(md.digest()), offset); + return endObject(md.toObjectId(), offset); } private byte[] insertBuffer(long len) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java index a510431f3..c37c4daa4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectoryInserter.java @@ -49,12 +49,11 @@ import java.io.EOFException; import java.io.File; import java.io.FileNotFoundException; import java.io.FileOutputStream; +import java.io.FilterOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.nio.channels.Channels; -import java.security.DigestOutputStream; -import java.security.MessageDigest; import java.text.MessageFormat; import java.util.zip.Deflater; import java.util.zip.DeflaterOutputStream; @@ -69,6 +68,7 @@ import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.transport.PackParser; import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.IO; +import org.eclipse.jgit.util.sha1.SHA1; /** Creates loose objects in a {@link ObjectDirectory}. */ class ObjectDirectoryInserter extends ObjectInserter { @@ -140,9 +140,9 @@ class ObjectDirectoryInserter extends ObjectInserter { return insert(type, buf, 0, actLen, createDuplicate); } else { - MessageDigest md = digest(); + SHA1 md = SHA1.newInstance(); File tmp = toTemp(md, type, len, is); - ObjectId id = ObjectId.fromRaw(md.digest()); + ObjectId id = md.toObjectId(); return insertOneObject(tmp, id, createDuplicate); } } @@ -193,7 +193,7 @@ class ObjectDirectoryInserter extends ObjectInserter { } @SuppressWarnings("resource" /* java 7 */) - private File toTemp(final MessageDigest md, final int type, long len, + private File toTemp(final SHA1 md, final int type, long len, final InputStream is) throws IOException, FileNotFoundException, Error { boolean delete = true; @@ -205,7 +205,7 @@ class ObjectDirectoryInserter extends ObjectInserter { if (config.getFSyncObjectFiles()) out = Channels.newOutputStream(fOut.getChannel()); DeflaterOutputStream cOut = compress(out); - DigestOutputStream dOut = new DigestOutputStream(cOut, md); + SHA1OutputStream dOut = new SHA1OutputStream(cOut, md); writeHeader(dOut, type, len); final byte[] buf = buffer(); @@ -285,4 +285,25 @@ class ObjectDirectoryInserter extends ObjectInserter { return new EOFException(MessageFormat.format( JGitText.get().inputDidntMatchLength, Long.valueOf(missing))); } + + private static class SHA1OutputStream extends FilterOutputStream { + private final SHA1 md; + + SHA1OutputStream(OutputStream out, SHA1 md) { + super(out); + this.md = md; + } + + @Override + public void write(int b) throws IOException { + md.update((byte) b); + out.write(b); + } + + @Override + public void write(byte[] in, int p, int n) throws IOException { + md.update(in, p, n); + out.write(in, p, n); + } + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/MutableObjectId.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/MutableObjectId.java index 1a49ae9d7..4b14d121e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/MutableObjectId.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/MutableObjectId.java @@ -208,6 +208,24 @@ public class MutableObjectId extends AnyObjectId { w5 = ints[p + 4]; } + /** + * Convert an ObjectId from binary representation expressed in integers. + * + * @param a + * @param b + * @param c + * @param d + * @param e + * @since 4.7 + */ + public void set(int a, int b, int c, int d, int e) { + w1 = a; + w2 = b; + w3 = c; + w4 = d; + w5 = e; + } + /** * Convert an ObjectId from hex characters (US-ASCII). * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectId.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectId.java index 2a2d67d25..991f03f82 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectId.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectId.java @@ -248,8 +248,17 @@ public class ObjectId extends AnyObjectId implements Serializable { } } - ObjectId(final int new_1, final int new_2, final int new_3, - final int new_4, final int new_5) { + /** + * Construct an ObjectId from 160 bits provided in 5 words. + * + * @param new_1 + * @param new_2 + * @param new_3 + * @param new_4 + * @param new_5 + * @since 4.7 + */ + public ObjectId(int new_1, int new_2, int new_3, int new_4, int new_5) { w1 = new_1; w2 = new_2; w3 = new_3; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java index 9cd2d1d29..b39603ada 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectInserter.java @@ -50,10 +50,10 @@ import java.io.ByteArrayInputStream; import java.io.EOFException; import java.io.IOException; import java.io.InputStream; -import java.security.MessageDigest; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.transport.PackParser; +import org.eclipse.jgit.util.sha1.SHA1; /** * Inserts objects into an existing {@code ObjectDatabase}. @@ -177,15 +177,11 @@ public abstract class ObjectInserter implements AutoCloseable { } } - /** Digest to compute the name of an object. */ - private final MessageDigest digest; - /** Temporary working buffer for streaming data through. */ private byte[] tempBuffer; /** Create a new inserter for a database. */ protected ObjectInserter() { - digest = Constants.newMessageDigest(); } /** @@ -220,9 +216,8 @@ public abstract class ObjectInserter implements AutoCloseable { } /** @return digest to help compute an ObjectId */ - protected MessageDigest digest() { - digest.reset(); - return digest; + protected SHA1 digest() { + return SHA1.newInstance(); } /** @@ -252,13 +247,13 @@ public abstract class ObjectInserter implements AutoCloseable { * @return the name of the object. */ public ObjectId idFor(int type, byte[] data, int off, int len) { - MessageDigest md = digest(); + SHA1 md = SHA1.newInstance(); md.update(Constants.encodedTypeString(type)); md.update((byte) ' '); md.update(Constants.encodeASCII(len)); md.update((byte) 0); md.update(data, off, len); - return ObjectId.fromRaw(md.digest()); + return md.toObjectId(); } /** @@ -277,7 +272,7 @@ public abstract class ObjectInserter implements AutoCloseable { */ public ObjectId idFor(int objectType, long length, InputStream in) throws IOException { - MessageDigest md = digest(); + SHA1 md = SHA1.newInstance(); md.update(Constants.encodedTypeString(objectType)); md.update((byte) ' '); md.update(Constants.encodeASCII(length)); @@ -290,7 +285,7 @@ public abstract class ObjectInserter implements AutoCloseable { md.update(buf, 0, n); length -= n; } - return ObjectId.fromRaw(md.digest()); + return md.toObjectId(); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java index 93e7952bd..7d48f2a25 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java @@ -83,6 +83,7 @@ import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.util.BlockList; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.NB; +import org.eclipse.jgit.util.sha1.SHA1; /** * Parses a pack stream and imports it for an {@link ObjectInserter}. @@ -116,8 +117,6 @@ public abstract class PackParser { private byte[] hdrBuf; - private final MessageDigest objectDigest; - private final MutableObjectId tempObjectId; private InputStream in; @@ -206,7 +205,6 @@ public abstract class PackParser { buf = new byte[BUFFER_SIZE]; tempBuffer = new byte[BUFFER_SIZE]; hdrBuf = new byte[64]; - objectDigest = Constants.newMessageDigest(); tempObjectId = new MutableObjectId(); packDigest = Constants.newMessageDigest(); checkObjectCollisions = true; @@ -667,12 +665,13 @@ public abstract class PackParser { JGitText.get().corruptionDetectedReReadingAt, Long.valueOf(visit.delta.position))); + SHA1 objectDigest = SHA1.newInstance(); objectDigest.update(Constants.encodedTypeString(type)); objectDigest.update((byte) ' '); objectDigest.update(Constants.encodeASCII(visit.data.length)); objectDigest.update((byte) 0); objectDigest.update(visit.data); - tempObjectId.fromRaw(objectDigest.digest(), 0); + objectDigest.digest(tempObjectId); verifySafeObject(tempObjectId, type, visit.data); @@ -1024,6 +1023,7 @@ public abstract class PackParser { private void whole(final long pos, final int type, final long sz) throws IOException { + SHA1 objectDigest = SHA1.newInstance(); objectDigest.update(Constants.encodedTypeString(type)); objectDigest.update((byte) ' '); objectDigest.update(Constants.encodeASCII(sz)); @@ -1043,7 +1043,7 @@ public abstract class PackParser { cnt += r; } inf.close(); - tempObjectId.fromRaw(objectDigest.digest(), 0); + objectDigest.digest(tempObjectId); checkContentLater = isCheckObjectCollisions() && readCurs.has(tempObjectId); data = null; @@ -1051,7 +1051,7 @@ public abstract class PackParser { } else { data = inflateAndReturn(Source.INPUT, sz); objectDigest.update(data); - tempObjectId.fromRaw(objectDigest.digest(), 0); + objectDigest.digest(tempObjectId); verifySafeObject(tempObjectId, type, data); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java index 0d3c78888..b1b146c85 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java @@ -56,7 +56,6 @@ import java.nio.ByteBuffer; import java.nio.CharBuffer; import java.nio.charset.CharacterCodingException; import java.nio.charset.CharsetEncoder; -import java.security.MessageDigest; import java.text.MessageFormat; import java.util.Arrays; import java.util.Collections; @@ -99,6 +98,7 @@ import org.eclipse.jgit.util.TemporaryBuffer; import org.eclipse.jgit.util.TemporaryBuffer.LocalFile; import org.eclipse.jgit.util.io.AutoLFInputStream; import org.eclipse.jgit.util.io.EolStreamTypeUtil; +import org.eclipse.jgit.util.sha1.SHA1; /** * Walks a working directory tree as part of a {@link TreeWalk}. @@ -364,7 +364,7 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator { if (is == null) return zeroid; try { - state.initializeDigestAndReadBuffer(); + state.initializeReadBuffer(); final long len = e.getLength(); InputStream filteredIs = possiblyFilteredInputStream(e, is, len, @@ -1099,10 +1099,9 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator { } private byte[] computeHash(InputStream in, long length) throws IOException { - final MessageDigest contentDigest = state.contentDigest; + SHA1 contentDigest = SHA1.newInstance(); final byte[] contentReadBuffer = state.contentReadBuffer; - contentDigest.reset(); contentDigest.update(hblob); contentDigest.update((byte) ' '); @@ -1330,9 +1329,6 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator { /** File name character encoder. */ final CharsetEncoder nameEncoder; - /** Digest computer for {@link #contentId} computations. */ - MessageDigest contentDigest; - /** Buffer used to perform {@link #contentId} computations. */ byte[] contentReadBuffer; @@ -1347,9 +1343,8 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator { this.nameEncoder = Constants.CHARSET.newEncoder(); } - void initializeDigestAndReadBuffer() { - if (contentDigest == null) { - contentDigest = Constants.newMessageDigest(); + void initializeReadBuffer() { + if (contentReadBuffer == null) { contentReadBuffer = new byte[BUFFER_SIZE]; } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/sha1/SHA1.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/sha1/SHA1.java index 625029d3a..0a1dc1046 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/sha1/SHA1.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/sha1/SHA1.java @@ -45,6 +45,8 @@ package org.eclipse.jgit.util.sha1; import java.util.Arrays; +import org.eclipse.jgit.lib.MutableObjectId; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.util.NB; /** @@ -259,4 +261,29 @@ public class SHA1 { NB.encodeInt32(b, 16, h4); return b; } + + /** + * Finish the digest and return the resulting hash. + *

+ * Once {@code digest()} is called, this instance should be discarded. + * + * @return the ObjectId for the resulting hash. + */ + public ObjectId toObjectId() { + finish(); + return new ObjectId(h0, h1, h2, h3, h4); + } + + /** + * Finish the digest and return the resulting hash. + *

+ * Once {@code digest()} is called, this instance should be discarded. + * + * @param id + * destination to copy the digest to. + */ + public void digest(MutableObjectId id) { + finish(); + id.set(h0, h1, h2, h3, h4); + } }