From: Shawn O. Pearce Date: Wed, 19 Jan 2011 00:33:33 +0000 (-0800) Subject: Permit disabling birthday attack checks in PackParser X-Git-Tag: v0.11.1~52 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fchanges%2F94%2F2294%2F2;p=jgit.git Permit disabling birthday attack checks in PackParser Reading a repository for millions of missing objects might be very expensive to perform, especially if the repository is on a network filesystem or some other costly RPC backend. A repository owner might choose to accept some risk in return for better performance, so allow disabling collision checking when receiving a pack. Currently there is no way for an end-user to disable this feature. This is intentional, because it is generally *NOT* a good idea to skip this check. Instead this feature is supplied for storage implementations to bypass the default checking logic, should they have their own custom routines that is just as effective but can be handled more efficiently. Change-Id: I90c801bb40e86412209de0c43e294a28f6a767a5 Signed-off-by: Shawn O. Pearce --- 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 06819d3efe..9d75328af4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java @@ -130,6 +130,8 @@ public abstract class PackParser { private boolean allowThin; + private boolean checkObjectCollisions; + private boolean needBaseObjectIds; private long objectCount; @@ -192,6 +194,7 @@ public abstract class PackParser { objectDigest = Constants.newMessageDigest(); tempObjectId = new MutableObjectId(); packDigest = Constants.newMessageDigest(); + checkObjectCollisions = true; } /** @return true if a thin pack (missing base objects) is permitted. */ @@ -212,6 +215,35 @@ public abstract class PackParser { allowThin = allow; } + /** @return if true received objects are verified to prevent collisions. */ + public boolean isCheckObjectCollisions() { + return checkObjectCollisions; + } + + /** + * Enable checking for collisions with existing objects. + *

+ * By default PackParser looks for each received object in the repository. + * If the object already exists, the existing object is compared + * byte-for-byte with the newly received copy to ensure they are identical. + * The receive is aborted with an exception if any byte differs. This check + * is necessary to prevent an evil attacker from supplying a replacement + * object into this repository in the event that a discovery enabling SHA-1 + * collisions is made. + *

+ * This check may be very costly to perform, and some repositories may have + * other ways to segregate newly received object data. The check is enabled + * by default, but can be explicitly disabled if the implementation can + * provide the same guarantee, or is willing to accept the risks associated + * with bypassing the check. + * + * @param check + * true to enable collision checking (strongly encouraged). + */ + public void setCheckObjectCollisions(boolean check) { + checkObjectCollisions = check; + } + /** * Configure this index pack instance to keep track of new objects. *

@@ -830,7 +862,8 @@ public abstract class PackParser { } inf.close(); tempObjectId.fromRaw(objectDigest.digest(), 0); - checkContentLater = readCurs.has(tempObjectId); + checkContentLater = isCheckObjectCollisions() + && readCurs.has(tempObjectId); } else { final byte[] data = inflateAndReturn(Source.INPUT, sz); @@ -859,17 +892,19 @@ public abstract class PackParser { } } - try { - final ObjectLoader ldr = readCurs.open(id, type); - final byte[] existingData = ldr.getCachedBytes(data.length); - if (!Arrays.equals(data, existingData)) { - throw new IOException(MessageFormat.format( - JGitText.get().collisionOn, id.name())); + if (isCheckObjectCollisions()) { + try { + final ObjectLoader ldr = readCurs.open(id, type); + final byte[] existingData = ldr.getCachedBytes(data.length); + if (!Arrays.equals(data, existingData)) { + throw new IOException(MessageFormat.format( + JGitText.get().collisionOn, id.name())); + } + } catch (MissingObjectException notLocal) { + // This is OK, we don't have a copy of the object locally + // but the API throws when we try to read it as usually its + // an error to read something that doesn't exist. } - } catch (MissingObjectException notLocal) { - // This is OK, we don't have a copy of the object locally - // but the API throws when we try to read it as usually its - // an error to read something that doesn't exist. } }