diff --git a/Remote/Adb.hs b/Remote/Adb.hs index f1e87013de..8e99f1f749 100644 --- a/Remote/Adb.hs +++ b/Remote/Adb.hs @@ -48,6 +48,7 @@ gen r u c gc = do , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = \_ _ _ -> return False + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/BitTorrent.hs b/Remote/BitTorrent.hs index 8cc559d917..659cc4732a 100644 --- a/Remote/BitTorrent.hs +++ b/Remote/BitTorrent.hs @@ -59,6 +59,8 @@ gen r _ c gc = , storeKey = uploadKey , retrieveKeyFile = downloadKey , retrieveKeyFileCheap = downloadKeyCheap + -- Bittorrent does its own hash checks. + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = dropKey , lockContent = Nothing , checkPresent = checkKey diff --git a/Remote/Bup.hs b/Remote/Bup.hs index 7b9ed4b7da..024e06a1b5 100644 --- a/Remote/Bup.hs +++ b/Remote/Bup.hs @@ -59,6 +59,9 @@ gen r u c gc = do , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = retrieveCheap buprepo + -- Bup uses git, which cryptographically verifies content + -- (with SHA1, but sufficiently for this). + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/Ddar.hs b/Remote/Ddar.hs index c37abde82c..1ca1fda159 100644 --- a/Remote/Ddar.hs +++ b/Remote/Ddar.hs @@ -58,6 +58,8 @@ gen r u c gc = do , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = retrieveCheap + -- Unsure about this, safe default until Robie answers. + , retrievalSecurityPolicy = RetrievalVerifiableKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/Directory.hs b/Remote/Directory.hs index dd79ea04af..2fcb05d93a 100644 --- a/Remote/Directory.hs +++ b/Remote/Directory.hs @@ -58,6 +58,7 @@ gen r u c gc = do , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = retrieveKeyFileCheapM dir chunkconfig + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/External.hs b/Remote/External.hs index dada0d9d81..1427d61456 100644 --- a/Remote/External.hs +++ b/Remote/External.hs @@ -109,6 +109,11 @@ gen r u c gc , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = \_ _ _ -> return False + -- External special remotes use many http libraries + -- and have no protection against redirects to + -- local private web servers, or in some cases + -- to file:// urls. + , retrievalSecurityPolicy = RetrievalVerifiableKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/GCrypt.hs b/Remote/GCrypt.hs index 37b708579e..20c47334d4 100644 --- a/Remote/GCrypt.hs +++ b/Remote/GCrypt.hs @@ -113,6 +113,7 @@ gen' r u c gc = do , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = \_ _ _ -> return False + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/Git.hs b/Remote/Git.hs index da177bac58..3f85365acf 100644 --- a/Remote/Git.hs +++ b/Remote/Git.hs @@ -161,6 +161,7 @@ gen r u c gc , storeKey = copyToRemote new st , retrieveKeyFile = copyFromRemote new st , retrieveKeyFileCheap = copyFromRemoteCheap new st + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = dropKey new st , lockContent = Just (lockKey new st) , checkPresent = inAnnex new st diff --git a/Remote/Glacier.hs b/Remote/Glacier.hs index d5f66172ae..e6cd68fdf3 100644 --- a/Remote/Glacier.hs +++ b/Remote/Glacier.hs @@ -55,6 +55,9 @@ gen r u c gc = new <$> remoteCost gc veryExpensiveRemoteCost , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = retrieveCheap this + -- glacier-cli does not follow redirects and does + -- not support file://, so this is secure. + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/Helper/Special.hs b/Remote/Helper/Special.hs index 73486442b8..883dcc9cb1 100644 --- a/Remote/Helper/Special.hs +++ b/Remote/Helper/Special.hs @@ -162,6 +162,14 @@ specialRemote' cfg c preparestorer prepareretriever prepareremover preparecheckp (retrieveKeyFileCheap baser k f d) -- retrieval of encrypted keys is never cheap (\_ -> return False) + -- When encryption is used, the remote could provide + -- some other content encrypted by the user, and trick + -- git-annex into decrypting it, leaking the decryption + -- into the git-annex repository. Verifiable keys + -- are the main protection against this attack. + , retrievalSecurityPolicy = if isencrypted + then RetrievalVerifiableKeysSecure + else retrievalSecurityPolicy baser , removeKey = \k -> cip >>= removeKeyGen k , checkPresent = \k -> cip >>= checkPresentGen k , cost = if isencrypted diff --git a/Remote/Hook.hs b/Remote/Hook.hs index 54d0480145..a6e5339732 100644 --- a/Remote/Hook.hs +++ b/Remote/Hook.hs @@ -49,6 +49,9 @@ gen r u c gc = do , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = retrieveCheap hooktype + -- A hook could use http and be vulnerable to + -- redirect to file:// attacks, etc. + , retrievalSecurityPolicy = RetrievalVerifiableKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/P2P.hs b/Remote/P2P.hs index b5ff048639..c47a9e6327 100644 --- a/Remote/P2P.hs +++ b/Remote/P2P.hs @@ -53,6 +53,7 @@ chainGen addr r u c gc = do , storeKey = store (const protorunner) , retrieveKeyFile = retrieve (const protorunner) , retrieveKeyFileCheap = \_ _ _ -> return False + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = remove protorunner , lockContent = Just $ lock withconn runProtoConn u , checkPresent = checkpresent protorunner diff --git a/Remote/Rsync.hs b/Remote/Rsync.hs index 9ce9c7f148..8f4e8ac55a 100644 --- a/Remote/Rsync.hs +++ b/Remote/Rsync.hs @@ -72,6 +72,7 @@ gen r u c gc = do , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = retrieveCheap o + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/S3.hs b/Remote/S3.hs index 227d0c5e85..5665455809 100644 --- a/Remote/S3.hs +++ b/Remote/S3.hs @@ -84,6 +84,9 @@ gen r u c gc = do , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = retrieveCheap + -- HttpManagerRestricted is used here, so this is + -- secure. + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Remote/Tahoe.hs b/Remote/Tahoe.hs index ae3d6950bd..5615c4849d 100644 --- a/Remote/Tahoe.hs +++ b/Remote/Tahoe.hs @@ -73,6 +73,8 @@ gen r u c gc = do , storeKey = store u hdl , retrieveKeyFile = retrieve u hdl , retrieveKeyFileCheap = \_ _ _ -> return False + -- Tahoe cryptographically verifies content. + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = remove , lockContent = Nothing , checkPresent = checkKey u hdl diff --git a/Remote/Web.hs b/Remote/Web.hs index 03cbe706d5..eef6b696e6 100644 --- a/Remote/Web.hs +++ b/Remote/Web.hs @@ -48,6 +48,9 @@ gen r _ c gc = , storeKey = uploadKey , retrieveKeyFile = downloadKey , retrieveKeyFileCheap = downloadKeyCheap + -- HttpManagerRestricted is used here, so this is + -- secure. + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = dropKey , lockContent = Nothing , checkPresent = checkKey diff --git a/Remote/WebDAV.hs b/Remote/WebDAV.hs index 432b729ca0..d8fc8bee4f 100644 --- a/Remote/WebDAV.hs +++ b/Remote/WebDAV.hs @@ -72,6 +72,9 @@ gen r u c gc = new <$> remoteCost gc expensiveRemoteCost , storeKey = storeKeyDummy , retrieveKeyFile = retreiveKeyFileDummy , retrieveKeyFileCheap = retrieveCheap + -- HttpManagerRestricted is used here, so this is + -- secure. + , retrievalSecurityPolicy = RetrievalAllKeysSecure , removeKey = removeKeyDummy , lockContent = Nothing , checkPresent = checkPresentDummy diff --git a/Types/Key.hs b/Types/Key.hs index b3efc04633..4b81850d80 100644 --- a/Types/Key.hs +++ b/Types/Key.hs @@ -1,6 +1,6 @@ {- git-annex Key data type - - - Copyright 2011-2017 Joey Hess + - Copyright 2011-2018 Joey Hess - - Licensed under the GNU GPL version 3 or higher. -} @@ -88,6 +88,23 @@ cryptographicallySecure (Blake2sKey _ _) = True cryptographicallySecure (Blake2spKey _ _) = True cryptographicallySecure _ = False +{- Is the Key variety backed by a hash, which allows verifying content? + - It does not have to be cryptographically secure against eg birthday + - attacks. + -} +isVerifiable :: KeyVariety -> Bool +isVerifiable (SHA2Key _ _) = True +isVerifiable (SHA3Key _ _) = True +isVerifiable (SKEINKey _ _) = True +isVerifiable (Blake2bKey _ _) = True +isVerifiable (Blake2sKey _ _) = True +isVerifiable (Blake2spKey _ _) = True +isVerifiable (SHA1Key _) = True +isVerifiable (MD5Key _) = True +isVerifiable WORMKey = False +isVerifiable URLKey = False +isVerifiable (OtherKey _) = False + formatKeyVariety :: KeyVariety -> String formatKeyVariety v = case v of SHA2Key sz e -> adde e (addsz sz "SHA") diff --git a/Types/Remote.hs b/Types/Remote.hs index f50bcef693..9f61f7041d 100644 --- a/Types/Remote.hs +++ b/Types/Remote.hs @@ -2,7 +2,7 @@ - - Most things should not need this, using Types instead - - - Copyright 2011-2017 Joey Hess + - Copyright 2011-2018 Joey Hess - - Licensed under the GNU GPL version 3 or higher. -} @@ -18,6 +18,7 @@ module Types.Remote , Availability(..) , Verification(..) , unVerified + , RetrievalSecurityPolicy(..) , isExportSupported , ExportActions(..) ) @@ -85,6 +86,8 @@ data RemoteA a = Remote -- Retrieves a key's contents to a tmp file, if it can be done cheaply. -- It's ok to create a symlink or hardlink. , retrieveKeyFileCheap :: Key -> AssociatedFile -> FilePath -> a Bool + -- Security policy for reteiving keys from this remote. + , retrievalSecurityPolicy :: RetrievalSecurityPolicy -- Removes a key's contents (succeeds if the contents are not present) , removeKey :: Key -> a Bool -- Uses locking to prevent removal of a key's contents, @@ -165,6 +168,32 @@ unVerified a = do ok <- a return (ok, UnVerified) +-- Security policy indicating what keys can be safely retrieved from a +-- remote. +data RetrievalSecurityPolicy + = RetrievalVerifiableKeysSecure + -- ^ Transfer of keys whose content can be verified + -- with a hash check is secure; transfer of unverifiable keys is + -- not secure and should not be allowed. + -- + -- This is used eg, when HTTP to a remote could be redirected to a + -- local private web server or even a file:// url, causing private + -- data from it that is not the intended content of a key to make + -- its way into the git-annex repository. + -- + -- It's also used when content is stored encrypted on a remote, + -- which could replace it with a different encrypted file, and + -- trick git-annex into decrypting it and leaking the decryption + -- into the git-annex repository. + -- + -- It's not (currently) used when the remote could alter the + -- content stored on it, because git-annex does not provide + -- strong guarantees about the content of keys that cannot be + -- verified with a hash check. + -- (But annex.securehashesonly does provide such guarantees.) + | RetrievalAllKeysSecure + -- ^ Any key can be securely retrieved. + isExportSupported :: RemoteA a -> a Bool isExportSupported r = exportSupported (remotetype r) (config r) (gitconfig r)