diff options
author | Joey Hess <joeyh@joeyh.name> | 2017-02-24 19:54:36 -0400 |
---|---|---|
committer | Joey Hess <joeyh@joeyh.name> | 2017-02-24 19:54:36 -0400 |
commit | a091af71fc8161427f8d9553042d0bc41507fff7 (patch) | |
tree | 69cf2785559ef7600ce4402abdaee4a6071fae36 | |
parent | 1630f299751d4e8b186cd176c8219f11257586d8 (diff) |
SHA1 collisions in key names was more exploitable than I thought
Yesterday's SHA1 collision attack could be used to generate eg:
SHA256-sfoo--whatever.good
SHA256-sfoo--whatever.bad
Such that they collide. A repository with the good one could have the
bad one swapped in and signed commits would still verify.
I've already mitigated this.
-rw-r--r-- | CHANGELOG | 5 | ||||
-rw-r--r-- | Key.hs | 5 | ||||
-rw-r--r-- | doc/todo/sha1_collision_embedding_in_git-annex_keys.mdwn | 13 |
3 files changed, 10 insertions, 13 deletions
@@ -33,8 +33,9 @@ git-annex (6.20170215) UNRELEASED; urgency=medium to wget, since curl is able to display only errors to stderr, unlike wget. * status: Pass --ignore-submodules=when option on to git status. - * Tighten key parser to mitigate against hypothetical SHA1 chosen-prefix - attacks. This ensures that signed git commits of annexed files + * Tighten key parser to prevent SHA1 collision attacks generating + two keys that have the same SHA1. (Only done for keys that contain + a hash). This ensures that signed git commits of annexed files will remain secure, as long as git-annex is using a secure hashing backend. @@ -127,12 +127,11 @@ file2key s | otherwise = Nothing {- When a key HasExt, the length of the extension is limited in order to - - mitigate against SHA1 collision attacks (specifically, chosen-prefix - - attacks). + - mitigate against SHA1 collision attacks. - - In such an attack, the extension of the key could be made to contain - the collision generation data, with the result that a signed git commit - - including such keys would not be secure. + - including such keys would not be secure. - - The maximum extension length ever generated for such a key was 8 - characters; 20 is used here to give a little future wiggle-room. diff --git a/doc/todo/sha1_collision_embedding_in_git-annex_keys.mdwn b/doc/todo/sha1_collision_embedding_in_git-annex_keys.mdwn index 7bb23a007..97f7b4f22 100644 --- a/doc/todo/sha1_collision_embedding_in_git-annex_keys.mdwn +++ b/doc/todo/sha1_collision_embedding_in_git-annex_keys.mdwn @@ -74,13 +74,10 @@ A few other potential problems: * `*E` backends could embed sha1 collision data in a long filename extension in a key. - Impact is limited, because even if an attacker does this, the key also - contains the checksum (eg SHA2) of the annexed data. The current SHA1 - attack is only a common-prefix attack; it does not allow creating two - colliding keys that contain two different SHA2 checksums. That would - need a chosen-prefix attack. - - It might be worth limiting the length + The recent SHA1 common-prefix attack could be used to exploit this; + the result would be two keys that have the same SHA1. + + This can be fixed by limiting the length of an extension allowed in such a key to the longest such extension git-annex has ever supported (probably < 20 bytes or so), which would be less than the size of the data needed for current SHA1 collision @@ -92,7 +89,7 @@ A few other potential problems: Need to review the code and see if such extra fields are allowed. Update: All fields are numeric, but could contain arbitrary data - after the number. Could have been used in a chosen-prefix attack. + after the number. Could have been used in a common-prefix attack. This has been fixed; git-annex refuses to parse such fields, so it won't work with files that try to exploit this. |