Fix bug that prevented resuming of uploads to encrypted special remotes that used chunking. This bug could also expose the names of keys to such remotes.

This is a low-severity security hole.

Remote/Helper/Chunked.hs

@@ -99,13 +99,14 @@ numChunks = pred . fromJust . keyChunkNum . fst . nextChunkKeyStream
 storeChunks
 	:: UUID
 	-> ChunkConfig
+	-> EncKey
 	-> Key
 	-> FilePath
 	-> MeterUpdate
 	-> Storer
 	-> CheckPresent
 	-> Annex Bool
-storeChunks u chunkconfig k f p storer checker = 
+storeChunks u chunkconfig encryptor k f p storer checker = 
 	case chunkconfig of
 		(UnpaddedChunks chunksize) | isStableKey k -> 
 			bracketIO open close (go chunksize)
@@ -121,7 +122,7 @@ storeChunks u chunkconfig k f p storer checker =
 		return False
 	go chunksize (Right h) = do
 		let chunkkeys = chunkKeyStream k chunksize
-		(chunkkeys', startpos) <- seekResume h chunkkeys checker
+		(chunkkeys', startpos) <- seekResume h encryptor chunkkeys checker
 		b <- liftIO $ L.hGetContents h
 		gochunks p startpos chunksize b chunkkeys'
 
@@ -165,10 +166,11 @@ storeChunks u chunkconfig k f p storer checker =
  -}
 seekResume
 	:: Handle
+	-> EncKey
 	-> ChunkKeyStream
 	-> CheckPresent
 	-> Annex (ChunkKeyStream, BytesProcessed)
-seekResume h chunkkeys checker = do
+seekResume h encryptor chunkkeys checker = do
 	sz <- liftIO (hFileSize h)
 	if sz <= fromMaybe 0 (keyChunkSize $ fst $ nextChunkKeyStream chunkkeys)
 		then return (chunkkeys, zeroBytesProcessed)
@@ -180,7 +182,7 @@ seekResume h chunkkeys checker = do
 			liftIO $ hSeek h AbsoluteSeek sz
 			return (cks, toBytesProcessed sz)
 		| otherwise = do
-			v <- tryNonAsync (checker k)
+			v <- tryNonAsync (checker (encryptor k))
 			case v of
 				Right True ->
 					check pos' cks' sz

Remote/Helper/Special.hs

@@ -189,11 +189,12 @@ specialRemote' cfg c preparestorer prepareretriever prepareremover preparecheckp
 		go Nothing = return False
 		go' storer (Just checker) = sendAnnex k rollback $ \src ->
 			displayprogress p k $ \p' ->
-				storeChunks (uuid baser) chunkconfig k src p'
+				storeChunks (uuid baser) chunkconfig enck k src p'
 					(storechunk enc storer)
 					checker
 		go' _ Nothing = return False
 		rollback = void $ removeKey encr k
+		enck = maybe id snd enc
 
 	storechunk Nothing storer k content p = storer k content p
 	storechunk (Just (cipher, enck)) storer k content p = do

debian/changelog

@@ -22,6 +22,9 @@ git-annex (6.20160419) UNRELEASED; urgency=medium
     this behavior change when the assistant merges. Note though that this is
     not done for git annex sync's merges, so it will follow git's default or
     configured behavior.
+  * Fix bug that prevented resuming of uploads to encrypted special remotes
+    that used chunking. This bug could also expose the names of keys to
+    such remotes.
 
  -- Joey Hess <id@joeyh.name>  Tue, 19 Apr 2016 12:57:15 -0400
 

doc/bugs/External_special_remote_broken__63__.mdwn

@@ -61,4 +61,4 @@ upgrade supported from repository versions: 0 1 2 4 5
 
 ### Have you had any luck using git-annex before? (Sometimes we get tired of reading bug reports all day and a lil' positive end note does wonders)
 
-
+> [[fixed|done]] --[[Joey]]

doc/bugs/External_special_remote_broken__63__/comment_1_904a186a6400506303cad772ac1a6751._comment

@@ -0,0 +1,18 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2016-04-27T16:23:43Z"
+ content="""
+Reproduced this using a directory special remote.
+
+The first checkpresent is because a file can be present on a remote in
+non-chunked form, since a remote can be reconfigured to add chunking.
+So it's nothing to worry about.
+
+The lack of encryption of the key when checking to resume is definitely a
+bug. A bit of a security bug too, although it only happens when resuming
+uploads. (I double checked the other operations and they all encrypt keys)
+I suppose that if the server was hostile, it could randomly make
+uploads fail, in order to get git-annex to expose content keys via
+this bug when resuming.
+"""]]