diff options
author | Joey Hess <joeyh@joeyh.name> | 2016-04-27 12:54:43 -0400 |
---|---|---|
committer | Joey Hess <joeyh@joeyh.name> | 2016-04-27 12:54:43 -0400 |
commit | d34a125d5a20bec416ff28d32648b9119677f3c7 (patch) | |
tree | 2dffa4196fc4e179ad1d0d86d4976eedafd70fb9 | |
parent | 0328c7d3c46344be3570f5813aa8a2c5e57f89c3 (diff) |
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.
-rw-r--r-- | Remote/Helper/Chunked.hs | 10 | ||||
-rw-r--r-- | Remote/Helper/Special.hs | 3 | ||||
-rw-r--r-- | debian/changelog | 3 | ||||
-rw-r--r-- | doc/bugs/External_special_remote_broken__63__.mdwn | 2 | ||||
-rw-r--r-- | doc/bugs/External_special_remote_broken__63__/comment_1_904a186a6400506303cad772ac1a6751._comment | 18 |
5 files changed, 30 insertions, 6 deletions
diff --git a/Remote/Helper/Chunked.hs b/Remote/Helper/Chunked.hs index 8098abc4f..e3cf0d27b 100644 --- a/Remote/Helper/Chunked.hs +++ b/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 diff --git a/Remote/Helper/Special.hs b/Remote/Helper/Special.hs index fdadc97b9..f9b5deae4 100644 --- a/Remote/Helper/Special.hs +++ b/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 diff --git a/debian/changelog b/debian/changelog index 938ed35c0..f1a54eaab 100644 --- a/debian/changelog +++ b/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 diff --git a/doc/bugs/External_special_remote_broken__63__.mdwn b/doc/bugs/External_special_remote_broken__63__.mdwn index ac7627d1b..b217e143b 100644 --- a/doc/bugs/External_special_remote_broken__63__.mdwn +++ b/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]] diff --git a/doc/bugs/External_special_remote_broken__63__/comment_1_904a186a6400506303cad772ac1a6751._comment b/doc/bugs/External_special_remote_broken__63__/comment_1_904a186a6400506303cad772ac1a6751._comment new file mode 100644 index 000000000..e50f00afb --- /dev/null +++ b/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. +"""]] |