replace an over-explained Bool with a data type

This also highlights several places where a Read/Show or similar for the
new data type could avoid redundant strings.

Crypto.hs

@@ -38,9 +38,9 @@ import Types.Key
 import Types.Crypto
 
 {- The beginning of a Cipher is used for MAC'ing; the remainder is used
- - as the GPG symmetric encryption passphrase. Note that the cipher
+ - as the GPG symmetric encryption passphrase when using the hybrid
- - itself is base-64 encoded, hence the string is longer than
+ - scheme. Note that the cipher itself is base-64 encoded, hence the
- - 'cipherSize': 683 characters, padded to 684.
+ - string is longer than 'cipherSize': 683 characters, padded to 684.
  -
  - The 256 first characters that feed the MAC represent at best 192
  - bytes of entropy.  However that's more than enough for both the
@@ -64,17 +64,16 @@ cipherPassphrase (Cipher c) = drop cipherBeginning c
 cipherMac :: Cipher -> String
 cipherMac (Cipher c) = take cipherBeginning c
 
-{- Creates a new Cipher, encrypted to the specified key id. If the
+{- Creates a new Cipher, encrypted to the specified key id. -}
- - boolean 'symmetric' is true, use that cipher not only for MAC'ing,
+genEncryptedCipher :: String -> EncryptedCipherVariant -> Bool -> IO StorableCipher
- - but also to symmetrically encrypt annexed file contents. Otherwise,
+genEncryptedCipher keyid variant highQuality = do
- - we don't bother to generate so much random data. -}
-genEncryptedCipher :: String -> Bool -> Bool -> IO StorableCipher
-genEncryptedCipher keyid symmetric highQuality = do
 	ks <- Gpg.findPubKeys keyid
 	random <- Gpg.genRandom highQuality size
-	encryptCipher (Cipher random) symmetric ks
+	encryptCipher (Cipher random) variant ks
   where
-	size = if symmetric then cipherSize else cipherBeginning
+	size = case variant of
+		HybridCipher -> cipherSize -- used for MAC + symmetric
+		PubKeyCipher -> cipherBeginning -- only used for MAC
 
 {- Creates a new, shared Cipher. -}
 genSharedCipher :: Bool -> IO StorableCipher
@@ -100,27 +99,25 @@ updateEncryptedCipher newkeys encipher@(EncryptedCipher _ symmetric (KeyIds ks))
 	listKeyIds = mapM (Gpg.findPubKeys >=*> keyIds) >=*> concat
 
 describeCipher :: StorableCipher -> String
-describeCipher SharedCipher{} = "shared cipher"
+describeCipher (SharedCipher _) = "shared cipher"
-describeCipher (EncryptedCipher _ symmetric (KeyIds ks)) =
+describeCipher (EncryptedCipher _ variant (KeyIds ks)) =
 	scheme ++ " with gpg " ++ keys ks ++ " " ++ unwords ks
   where
-	scheme = if symmetric then "hybrid cipher" else "pubkey crypto"
+	scheme = case variant of
+		HybridCipher -> "hybrid cipher"
+		PubKeyCipher -> "pubkey crypto"
 	keys [_] = "key"
 	keys _ = "keys"
 
-{- Encrypts a Cipher to the specified KeyIds. The boolean indicates
+{- Encrypts a Cipher to the specified KeyIds. -}
- - whether to encrypt a hybrid cipher (True), which is going to be used
+encryptCipher :: Cipher -> EncryptedCipherVariant -> KeyIds -> IO StorableCipher
- - both for MAC'ing and symmetric encryption of file contents, or for
+encryptCipher (Cipher c) variant (KeyIds ks) = do
- - MAC'ing only (False), while pubkey crypto is used for file contents.
- - -}
-encryptCipher :: Cipher -> Bool -> KeyIds -> IO StorableCipher
-encryptCipher (Cipher c) symmetric (KeyIds ks) = do
 	-- gpg complains about duplicate recipient keyids
 	let ks' = nub $ sort ks
 	-- The cipher itself is always encrypted to the given public keys
 	let params = Gpg.pkEncTo ks' ++ Gpg.stdEncryptionParams False
 	encipher <- Gpg.pipeStrict params c
-	return $ EncryptedCipher encipher symmetric (KeyIds ks')
+	return $ EncryptedCipher encipher variant (KeyIds ks')
 
 {- Decrypting an EncryptedCipher is expensive; the Cipher should be cached. -}
 decryptCipher :: StorableCipher -> IO Cipher

Remote/Helper/Encryptable.hs

@@ -36,9 +36,9 @@ encryptionSetup c = maybe genCipher updateCipher $ extractCipher c
 		-- hybrid encryption is the default when a keyid is
                 -- specified but no encryption
 		_ | maybe (M.member "keyid" c) (== "hybrid") encryption ->
-			use "encryption setup" . genEncryptedCipher key True
+			use "encryption setup" . genEncryptedCipher key HybridCipher
 				=<< highRandomQuality
-		Just "pubkey" -> use "encryption setup" . genEncryptedCipher key False
+		Just "pubkey" -> use "encryption setup" . genEncryptedCipher key PubKeyCipher
 				=<< highRandomQuality
 		_ -> error $ "Specify " ++ intercalate " or "
 			(map ("encryption=" ++)
@@ -51,10 +51,9 @@ encryptionSetup c = maybe genCipher updateCipher $ extractCipher c
 	-- Update an existing cipher if possible.
 	updateCipher v = case v of
 		SharedCipher{} | maybe True (== "shared") encryption -> return c'
-		EncryptedCipher _ symmetric _
+		EncryptedCipher _ variant _
-			| maybe True (== if symmetric then "hybrid" else "pubkey")
+			| maybe True (== if variant == HybridCipher then "hybrid" else "pubkey") encryption ->
-				encryption ->
+				use "encryption update" $ updateEncryptedCipher newkeys v
-			use "encryption update" $ updateEncryptedCipher newkeys v
 		_ -> cannotchange
 	use m a = do
 		showNote m
@@ -162,9 +161,9 @@ extractCipher c = case (M.lookup "cipher" c,
 			M.lookup "cipherkeys" c,
 			M.lookup "encryption" c) of
 	(Just t, Just ks, encryption) | maybe True (== "hybrid") encryption ->
-		Just $ EncryptedCipher (fromB64 t) True (readkeys ks)
+		Just $ EncryptedCipher (fromB64 t) HybridCipher (readkeys ks)
 	(Just t, Just ks, Just "pubkey") ->
-		Just $ EncryptedCipher (fromB64 t) False (readkeys ks)
+		Just $ EncryptedCipher (fromB64 t) PubKeyCipher (readkeys ks)
 	(Just t, Nothing, encryption) | maybe True (== "shared") encryption ->
 		Just $ SharedCipher (fromB64 t)
 	_ -> Nothing

Test.hs

@@ -920,26 +920,26 @@ test_crypto env = "git-annex crypto" ~: TestList $ flip map ["shared","hybrid","
 	 - that all keys are encrypted properly on the given directory remote. -}
 	testEncryptedRemote scheme ks c keys = case Remote.Helper.Encryptable.extractCipher c of
 		Just cip@Crypto.SharedCipher{} | scheme == "shared" && isNothing ks ->
-			checkKeys cip True
+			checkKeys cip Nothing
-		Just cip@(Crypto.EncryptedCipher encipher sym ks')
+		Just cip@(Crypto.EncryptedCipher encipher v ks')
-			| checkScheme sym && keysMatch ks' ->
+			| checkScheme v && keysMatch ks' ->
-				checkKeys cip sym <&&> checkCipher encipher ks'
+				checkKeys cip (Just v) <&&> checkCipher encipher ks'
 		_ -> return False
 	  where
 		keysMatch (Utility.Gpg.KeyIds ks') =
 			maybe False (\(Utility.Gpg.KeyIds ks2) ->
 					sort (nub ks2) == sort (nub ks')) ks
 		checkCipher encipher = Utility.Gpg.checkEncryptionStream encipher . Just
-		checkScheme True = scheme == "hybrid"
+		checkScheme Types.Crypto.HybridCipher = scheme == "hybrid"
-		checkScheme False = scheme == "pubkey"
+		checkScheme Types.Crypto.PubKeyCipher = scheme == "pubkey"
-		checkKeys cip sym = do
+		checkKeys cip mvariant = do
 			cipher <- Crypto.decryptCipher cip
 			files <- filterM doesFileExist $
 				map ("dir" </>) $ concatMap (key2files cipher) keys
-			return (not $ null files) <&&> allM (checkFile sym) files
+			return (not $ null files) <&&> allM (checkFile mvariant) files
-		checkFile sym filename =
+		checkFile mvariant filename =
 			Utility.Gpg.checkEncryptionFile filename $
-				if sym then Nothing else ks
+				if mvariant == Just Types.Crypto.PubKeyCipher then ks else Nothing
 		key2files cipher = Locations.keyPaths .
 			Crypto.encryptKey Types.Crypto.HmacSha1 cipher
 #else

Types/Crypto.hs

@@ -8,6 +8,7 @@
 module Types.Crypto (
 	Cipher(..),
 	StorableCipher(..),
+	EncryptedCipherVariant(..),
 	KeyIds(..),
 	Mac(..),
 	readMac,
@@ -24,16 +25,11 @@ import Utility.Gpg (KeyIds(..))
 -- XXX ideally, this would be a locked memory region
 newtype Cipher = Cipher String
 
-data StorableCipher = EncryptedCipher String Bool KeyIds
+data StorableCipher = EncryptedCipher String EncryptedCipherVariant KeyIds
-		-- ^ The Boolean indicates whether the cipher is used
-		-- both for symmetric encryption of file content and
-		-- MAC'ing of file names (True), or only for MAC'ing,
-		-- while file content is encrypted using public-key
-		-- crypto (False). In the latter case the cipher is
-		-- twice as short, but we don't want to rely on that
-		-- only.
 		| SharedCipher String
 	deriving (Ord, Eq)
+data EncryptedCipherVariant = HybridCipher | PubKeyCipher
+	deriving (Ord, Eq)
 
 {- File names are (client-side) MAC'ed on special remotes.
  - The chosen MAC algorithm needs to be same for all files stored on the