add retrievalSecurityPolicy

This will be used to protect against CVE-2018-10859, where an encrypted
special remote is fed the wrong encrypted data, and so tricked into
decrypting something that the user encrypted with their gpg key and did
not store in git-annex.

It also protects against CVE-2018-10857, where a remote follows a http
redirect to a file:// url or to a local private web server. While that's
already been prevented in git-annex's own use of http, external special
remotes, hooks, etc use other http implementations and could still be
vulnerable.

The policy is not yet enforced, this commit only adds the appropriate
metadata to remotes.

This commit was sponsored by Boyd Stephen Smith Jr. on Patreon.

17 files changed, 43 insertions, 0 deletions

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