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.

3 files changed, 10 insertions, 13 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 9f3c22414..459937cfd 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -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.
 
diff --git a/Key.hs b/Key.hs
index 5eaf3d56b..d1669bf05 100644
--- a/Key.hs
+++ b/Key.hs
@@ -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.