Avoid more than 1 gpg password prompt at the same time
Which could happen occasionally before when concurrency is enabled. While not much of a problem when it did happen, better to avoid it. Also, since it seems likely the gpg-agent sometimes fails in such a situation, this makes it not happen when running a single git-annex command with concurrency enabled. This commit was sponsored by Jake Vosloo on Patreon.
This commit is contained in:
		
					parent
					
						
							
								42cacb4099
							
						
					
				
			
			
				commit
				
					
						0f73b6d03a
					
				
			
		
					 4 changed files with 42 additions and 14 deletions
				
			
		
							
								
								
									
										5
									
								
								Annex.hs
									
										
									
									
									
								
							
							
						
						
									
										5
									
								
								Annex.hs
									
										
									
									
									
								
							|  | @ -122,6 +122,7 @@ data AnnexRead = AnnexRead | ||||||
| 	, transferrerpool :: TransferrerPool | 	, transferrerpool :: TransferrerPool | ||||||
| 	, debugenabled :: Bool | 	, debugenabled :: Bool | ||||||
| 	, debugselector :: DebugSelector | 	, debugselector :: DebugSelector | ||||||
|  | 	, ciphers :: TMVar (M.Map StorableCipher Cipher) | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| newAnnexRead :: GitConfig -> IO AnnexRead | newAnnexRead :: GitConfig -> IO AnnexRead | ||||||
|  | @ -132,6 +133,7 @@ newAnnexRead c = do | ||||||
| 	sc <- newTMVarIO False | 	sc <- newTMVarIO False | ||||||
| 	si <- newTVarIO M.empty | 	si <- newTVarIO M.empty | ||||||
| 	tp <- newTransferrerPool | 	tp <- newTransferrerPool | ||||||
|  | 	cm <- newTMVarIO M.empty | ||||||
| 	return $ AnnexRead | 	return $ AnnexRead | ||||||
| 		{ activekeys = emptyactivekeys | 		{ activekeys = emptyactivekeys | ||||||
| 		, activeremotes = emptyactiveremotes | 		, activeremotes = emptyactiveremotes | ||||||
|  | @ -141,6 +143,7 @@ newAnnexRead c = do | ||||||
| 		, transferrerpool = tp | 		, transferrerpool = tp | ||||||
| 		, debugenabled = annexDebug c | 		, debugenabled = annexDebug c | ||||||
| 		, debugselector = debugSelectorFromGitConfig c | 		, debugselector = debugSelectorFromGitConfig c | ||||||
|  | 		, ciphers = cm | ||||||
| 		} | 		} | ||||||
| 
 | 
 | ||||||
| -- Values that can change while running an Annex action. | -- Values that can change while running an Annex action. | ||||||
|  | @ -178,7 +181,6 @@ data AnnexState = AnnexState | ||||||
| 	, forcetrust :: TrustMap | 	, forcetrust :: TrustMap | ||||||
| 	, trustmap :: Maybe TrustMap | 	, trustmap :: Maybe TrustMap | ||||||
| 	, groupmap :: Maybe GroupMap | 	, groupmap :: Maybe GroupMap | ||||||
| 	, ciphers :: M.Map StorableCipher Cipher |  | ||||||
| 	, lockcache :: LockCache | 	, lockcache :: LockCache | ||||||
| 	, flags :: M.Map String Bool | 	, flags :: M.Map String Bool | ||||||
| 	, fields :: M.Map String String | 	, fields :: M.Map String String | ||||||
|  | @ -237,7 +239,6 @@ newAnnexState c r = do | ||||||
| 		, forcetrust = M.empty | 		, forcetrust = M.empty | ||||||
| 		, trustmap = Nothing | 		, trustmap = Nothing | ||||||
| 		, groupmap = Nothing | 		, groupmap = Nothing | ||||||
| 		, ciphers = M.empty |  | ||||||
| 		, lockcache = M.empty | 		, lockcache = M.empty | ||||||
| 		, flags = M.empty | 		, flags = M.empty | ||||||
| 		, fields = M.empty | 		, fields = M.empty | ||||||
|  |  | ||||||
|  | @ -25,6 +25,8 @@ git-annex (8.20210331) UNRELEASED; urgency=medium | ||||||
|     mincopies is satisfied. Before, it assumed a sane configuration would |     mincopies is satisfied. Before, it assumed a sane configuration would | ||||||
|     have numcopies larger or equal to mincopies. It's still a good idea |     have numcopies larger or equal to mincopies. It's still a good idea | ||||||
|     not to configure git-annex this way. |     not to configure git-annex this way. | ||||||
|  |   * Avoid more than 1 gpg password prompt at the same time, which | ||||||
|  |     could happen occasionally before when concurrency is enabled. | ||||||
|   * Fix build with persistent-2.12.0.1 |   * Fix build with persistent-2.12.0.1 | ||||||
| 
 | 
 | ||||||
|  -- Joey Hess <id@joeyh.name>  Thu, 01 Apr 2021 12:17:26 -0400 |  -- Joey Hess <id@joeyh.name>  Thu, 01 Apr 2021 12:17:26 -0400 | ||||||
|  |  | ||||||
|  | @ -1,6 +1,6 @@ | ||||||
| {- common functions for encryptable remotes | {- common functions for encryptable remotes | ||||||
|  - |  - | ||||||
|  - Copyright 2011-2020 Joey Hess <id@joeyh.name> |  - Copyright 2011-2021 Joey Hess <id@joeyh.name> | ||||||
|  - |  - | ||||||
|  - Licensed under the GNU AGPL version 3 or higher. |  - Licensed under the GNU AGPL version 3 or higher. | ||||||
|  -} |  -} | ||||||
|  | @ -30,6 +30,7 @@ import qualified Data.Map as M | ||||||
| import qualified Data.Set as S | import qualified Data.Set as S | ||||||
| import qualified "sandi" Codec.Binary.Base64 as B64 | import qualified "sandi" Codec.Binary.Base64 as B64 | ||||||
| import qualified Data.ByteString as B | import qualified Data.ByteString as B | ||||||
|  | import Control.Concurrent.STM | ||||||
| 
 | 
 | ||||||
| import Annex.Common | import Annex.Common | ||||||
| import Types.Remote | import Types.Remote | ||||||
|  | @ -218,17 +219,28 @@ remoteCipher c gc = fmap fst <$> remoteCipher' c gc | ||||||
| {- Gets encryption Cipher. The decrypted Ciphers are cached in the Annex | {- Gets encryption Cipher. The decrypted Ciphers are cached in the Annex | ||||||
|  - state. -} |  - state. -} | ||||||
| remoteCipher' :: ParsedRemoteConfig -> RemoteGitConfig -> Annex (Maybe (Cipher, StorableCipher)) | remoteCipher' :: ParsedRemoteConfig -> RemoteGitConfig -> Annex (Maybe (Cipher, StorableCipher)) | ||||||
| remoteCipher' c gc = go $ extractCipher c | remoteCipher' c gc = case extractCipher c of | ||||||
|   where | 	Nothing -> return Nothing | ||||||
| 	go Nothing = return Nothing | 	Just encipher -> do | ||||||
| 	go (Just encipher) = do | 		cachev <- Annex.getRead Annex.ciphers | ||||||
| 		cache <- Annex.getState Annex.ciphers | 		cachedciper <- liftIO $ atomically $  | ||||||
| 		case M.lookup encipher cache of | 			M.lookup encipher <$> readTMVar cachev | ||||||
|  | 		case cachedciper of | ||||||
| 			Just cipher -> return $ Just (cipher, encipher) | 			Just cipher -> return $ Just (cipher, encipher) | ||||||
| 			Nothing -> do | 			-- Not cached; decrypt it, making sure | ||||||
|  | 			-- to only decrypt one at a time. Avoids | ||||||
|  | 			-- prompting for decrypting the same thing twice | ||||||
|  | 			-- when this is run concurrently. | ||||||
|  | 			Nothing -> bracketOnError | ||||||
|  | 				(liftIO $ atomically $ takeTMVar cachev) | ||||||
|  | 				(liftIO . atomically . putTMVar cachev) | ||||||
|  | 				(go cachev encipher) | ||||||
|  |   where | ||||||
|  | 	go cachev encipher cache = do | ||||||
| 		cmd <- gpgCmd <$> Annex.getGitConfig | 		cmd <- gpgCmd <$> Annex.getGitConfig | ||||||
| 		cipher <- liftIO $ decryptCipher cmd (c, gc) encipher | 		cipher <- liftIO $ decryptCipher cmd (c, gc) encipher | ||||||
| 				Annex.changeState (\s -> s { Annex.ciphers = M.insert encipher cipher cache }) | 		liftIO $ atomically $ putTMVar cachev $ | ||||||
|  | 			M.insert encipher cipher cache | ||||||
| 		return $ Just (cipher, encipher) | 		return $ Just (cipher, encipher) | ||||||
| 
 | 
 | ||||||
| {- Checks if the remote's config allows storing creds in the remote's config. | {- Checks if the remote's config allows storing creds in the remote's config. | ||||||
|  |  | ||||||
|  | @ -0,0 +1,13 @@ | ||||||
|  | [[!comment format=mdwn | ||||||
|  |  username="joey" | ||||||
|  |  subject="""comment 3""" | ||||||
|  |  date="2021-04-27T20:33:41Z" | ||||||
|  |  content=""" | ||||||
|  | I've implemented the race protection. Pretty sure this fixes it, at least | ||||||
|  | to the extent it can be fixed if it's a gpg bug that any 2 concurrent | ||||||
|  | password prompts can sometimes trigger. I'll run git-annex fsck in a | ||||||
|  | loop for an hour or so to be more sure. | ||||||
|  | 
 | ||||||
|  | It would be nice if someone wants to file a bug on gpg, but I don't have | ||||||
|  | time right now. | ||||||
|  | """]] | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Joey Hess
				Joey Hess