summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Joey Hess <joeyh@joeyh.name>2016-04-27 12:54:43 -0400
committerGravatar Joey Hess <joeyh@joeyh.name>2016-04-27 12:54:43 -0400
commitd34a125d5a20bec416ff28d32648b9119677f3c7 (patch)
tree2dffa4196fc4e179ad1d0d86d4976eedafd70fb9
parent0328c7d3c46344be3570f5813aa8a2c5e57f89c3 (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.hs10
-rw-r--r--Remote/Helper/Special.hs3
-rw-r--r--debian/changelog3
-rw-r--r--doc/bugs/External_special_remote_broken__63__.mdwn2
-rw-r--r--doc/bugs/External_special_remote_broken__63__/comment_1_904a186a6400506303cad772ac1a6751._comment18
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.
+"""]]