deal with old repositories with non-encrypted creds

See 2fb7ad68637cc4e1092f835055a974f141808ca0 for backstory about how a repo
could be in this state.

When decryption fails, the repo must be using non-encrypted creds. Note
that creds are encrypted/decrypted using the encryption cipher which is
stored in the repo, so the decryption cannot fail due to missing gpg keys
etc. (For !shared encryptiom, the cipher is iteself encrypted using some
gpg key(s), and the decryption of the cipher happens earlier, so not
affected by this change.

Print a warning message for !shared repos, and continue on using the
cipher. Wrote a page explaining what users hit by this bug should do.

This commit was sponsored by Samuel Tardieu.

4 files changed, 64 insertions, 11 deletions

diff --git a/Creds.hs b/Creds.hs
index aad3996bf..f9b8c4ec6 100644
--- a/Creds.hs
+++ b/Creds.hs
@@ -23,7 +23,7 @@ import Annex.Perms
 import Utility.FileMode
 import Crypto
 import Types.Remote (RemoteConfig, RemoteConfigKey)
-import Remote.Helper.Encryptable (remoteCipher, embedCreds, EncryptionIsSetup)
+import Remote.Helper.Encryptable (remoteCipher, remoteCipher', embedCreds, EncryptionIsSetup)
 import Utility.Env (getEnv)
 
 import qualified Data.ByteString.Lazy.Char8 as L
@@ -90,20 +90,32 @@ getRemoteCredPair c storage = maybe fromcache (return . Just) =<< fromenv
 	fromcache = maybe fromconfig (return . Just) =<< readCacheCredPair storage
 	fromconfig = case credPairRemoteKey storage of
 		Just key -> do
-			mcipher <- remoteCipher c
+			mcipher <- remoteCipher' c
 			case (M.lookup key c, mcipher) of
 				(Nothing, _) -> return Nothing
-				(Just enccreds, Just cipher) -> do
-					creds <- liftIO $ decrypt cipher
-						(feedBytes $ L.pack $ fromB64 enccreds)
-						(readBytes $ return . L.unpack)
-					fromcreds creds
+				(Just enccreds, Just (cipher, storablecipher)) ->
+					fromenccreds enccreds cipher storablecipher
 				(Just bcreds, Nothing) ->
 					fromcreds $ fromB64 bcreds
 		Nothing -> return Nothing
+	fromenccreds enccreds cipher storablecipher = do
+		mcreds <- liftIO $ catchMaybeIO $ decrypt cipher
+			(feedBytes $ L.pack $ fromB64 enccreds)
+			(readBytes $ return . L.unpack)
+		case mcreds of
+			Just creds -> fromcreds creds
+			Nothing -> do
+				-- Work around un-encrypted creds storage
+				-- bug in old S3 and glacier remotes.
+				-- Not a problem for shared cipher.
+				case storablecipher of
+					SharedCipher {} -> showLongNote "gpg error above was caused by an old git-annex bug in credentials storage. Working around it.."
+					_ -> warning "*** Insecure credentials storage detected for this remote! See https://git-annex.branchable.com/upgrades/insecure_embedded_creds/"
+				fromcreds $ fromB64 enccreds
 	fromcreds creds = case decodeCredPair creds of
 		Just credpair -> do
 			writeCacheCredPair credpair storage
+
 			return $ Just credpair
 		_ -> error "bad creds"
 
diff --git a/Remote/Helper/Encryptable.hs b/Remote/Helper/Encryptable.hs
index e8e9d3bf1..05f3fc3f9 100644
--- a/Remote/Helper/Encryptable.hs
+++ b/Remote/Helper/Encryptable.hs
@@ -11,6 +11,7 @@ module Remote.Helper.Encryptable (
 	noEncryptionUsed,
 	encryptionAlreadySetup,
 	remoteCipher,
+	remoteCipher',
 	embedCreds,
 	cipherKey,
 	storeCipher,
@@ -93,21 +94,24 @@ encryptionSetup c = maybe genCipher updateCipher $ extractCipher c
                 -- remotes (while being backward-compatible).
 		[ "keyid", "keyid+", "keyid-", "highRandomQuality" ]
 
+remoteCipher :: RemoteConfig -> Annex (Maybe Cipher)
+remoteCipher = fmap fst <$$> remoteCipher'
+
 {- Gets encryption Cipher. The decrypted Ciphers are cached in the Annex
  - state. -}
-remoteCipher :: RemoteConfig -> Annex (Maybe Cipher)
-remoteCipher c = go $ extractCipher c
+remoteCipher' :: RemoteConfig -> Annex (Maybe (Cipher, StorableCipher))
+remoteCipher' c = go $ extractCipher c
   where
 	go Nothing = return Nothing
 	go (Just encipher) = do
 		cache <- Annex.getState Annex.ciphers
 		case M.lookup encipher cache of
-			Just cipher -> return $ Just cipher
+			Just cipher -> return $ Just (cipher, encipher)
 			Nothing -> do
 				showNote "gpg"
 				cipher <- liftIO $ decryptCipher encipher
 				Annex.changeState (\s -> s { Annex.ciphers = M.insert encipher cipher cache })
-				return $ Just cipher
+				return $ Just (cipher, encipher)
 
 {- Checks if the remote's config allows storing creds in the remote's config.
  - 
diff --git a/debian/changelog b/debian/changelog
index ffb760b7b..5bfba77ed 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -3,6 +3,9 @@ git-annex (5.20140916) UNRELEASED; urgency=medium
   * Security fix for S3 and glacier when using embedcreds=yes with
     encryption=pubkey or encryption=hybrid. 
     The creds embedded in the git repo were *not* encrypted.
+    git-annex enableremote will warn when used on a remote that has
+    this problem. For details, see:
+    https://git-annex.branchable.com/upgrades/insecure_embedded_creds/
   * assistant: Detect when repository has been deleted or moved, and
     automatically shut down the assistant. Closes: #761261
   * Windows: Avoid crashing trying to list gpg secret keys, for gcrypt
diff --git a/doc/upgrades/insecure_embedded_creds.mdwn b/doc/upgrades/insecure_embedded_creds.mdwn
new file mode 100644
index 000000000..b0554a5d8
--- /dev/null
+++ b/doc/upgrades/insecure_embedded_creds.mdwn
@@ -0,0 +1,34 @@
+git-annex had a bug in the S3 and Glacier remotes where if embedcreds=yes
+was set, and the remote used encryption=pubkey or encryption=hybrid,
+the embedded AWS credentials were stored in the git repository
+in (effectively) plaintext, not encrypted as they were supposed to be.
+
+That means that anyone who gets a copy of the git repository can extract the
+AWS credentials from it. Which would be bad..
+
+Fixed versions of git-annex will detect this problem when enabling such a
+remote, and print a warning message (including a pointer to this web page).
+
+This message will only be printed one time, since git-annex will
+change the embedded credentials to be encrypted properly.
+However, this leaves the non-encrypted version still in the git history.
+
+If your repository has this problem, you have two courses of action to fix
+it:
+
+1. Change your AWS credentials, so the ones stored in the clear in git
+   won't be used. 
+   
+   After changing the credentials, make sure you have a
+   fixed version of git-annex, and you can then re-embed the new creds
+   into the repository, encrypted this time, by setting the
+   `AWS_SECRET_ACCESS_KEY` and `AWS_ACCESS_KEY_ID` environment variables,
+   and running `git annex enableremote $remotename embedcreds=yes`
+
+2. Remove the history of the git-annex branch of the repository.
+   You can use `git annex forget`` to do that; note that it will
+   remove other historical data too.
+
+3. If you're the only one who has access to the repository, you could decide
+   to leave it as-is. It's no more insecure than if you had used
+   encryption=shared in the first place when setting it up.