Document caveats of mlsb{To,As}ByteString

IntersectMBO · Nov 29, 2022 · da20188 · da20188
1 parent f842dec
commit da20188
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 7 deletions.
diff --git a/cardano-crypto-class/src/Cardano/Crypto/DSIGN/Ed25519ML.hs b/cardano-crypto-class/src/Cardano/Crypto/DSIGN/Ed25519ML.hs
@@ -182,12 +182,13 @@ instance DSIGNMAlgorithm IO Ed25519DSIGNM where
       mlsbFinalize sk
 
     --
-    -- Ser/deser (dangerous)
+    -- Ser/deser (dangerous - do not use in production code)
     --
     rawSerialiseSignKeyDSIGNM sk = do
       seed <- getSeedDSIGNM (Proxy @Ed25519DSIGNM) sk
-      -- need to copy the seed into unsafe memory and finalize the MLSB, in
-      -- order to avoid leaking mlocked memory
+      -- We need to copy the seed into unsafe memory and finalize the MLSB, in
+      -- order to avoid leaking mlocked memory. This will, however, expose the
+      -- secret seed to the unprotected Haskell heap (see 'mlsbToByteString').
       raw <- mlsbToByteString seed
       mlsbFinalize seed
       return raw

diff --git a/cardano-crypto-class/src/Cardano/Crypto/KES/CompactSum.hs b/cardano-crypto-class/src/Cardano/Crypto/KES/CompactSum.hs
@@ -321,7 +321,7 @@ instance ( OptimizedKESAlgorithm d
         NaCl.releaseSafePinned r1
 
     --
-    -- raw serialise/deserialise
+    -- Raw serialise/deserialise - dangerous, do not use in production code.
     --
 
     rawSerialiseSignKeyKES (SignKeyCompactSumKES sk r_1 vk_0 vk_1) = do

diff --git a/cardano-crypto-class/src/Cardano/Crypto/KES/Sum.hs b/cardano-crypto-class/src/Cardano/Crypto/KES/Sum.hs
@@ -283,7 +283,7 @@ instance ( KESSignAlgorithm m d
       NaCl.releaseSafePinned r1
 
     --
-    -- raw serialise/deserialise
+    -- Raw serialise/deserialise - dangerous, do not use in production code.
     --
 
     rawSerialiseSignKeyKES (SignKeySumKES sk r_1 vk_0 vk_1) = do

diff --git a/cardano-crypto-class/src/Cardano/Crypto/Libsodium/MLockedBytes/Internal.hs b/cardano-crypto-class/src/Cardano/Crypto/Libsodium/MLockedBytes/Internal.hs
@@ -115,15 +115,20 @@ mlsbFromByteStringCheck bs
     size = fromInteger (natVal (Proxy @n))
 
 -- | /Note:/ the resulting 'BS.ByteString' will still refer to secure memory,
--- but the types don't prevent it from be exposed.
---
+-- but the types don't prevent it from be exposed. Note further that any
+-- subsequent operations (splicing & dicing, copying, conversion,
+-- packing/unpacking, etc.) on the resulting 'BS.ByteString' may create copies
+-- of the mlocked memory on the unprotected GHC heap, and thus leak secrets,
+-- so use this function with extreme care.
 mlsbAsByteString :: forall n. KnownNat n => MLockedSizedBytes n -> BS.ByteString
 mlsbAsByteString (MLSB (SFP fptr)) = BSI.PS (castForeignPtr fptr) 0 size
   where
     size  :: Int
     size = fromInteger (natVal (Proxy @n))
 
 
+-- | /Note:/ this function will leak mlocked memory to the Haskell heap
+-- and should not be used in production code.
 mlsbToByteString :: forall n. (KnownNat n) => MLockedSizedBytes n -> IO BS.ByteString
 mlsbToByteString mlsb =
   withMLSB mlsb $ \ptr ->

diff --git a/cardano-crypto-class/src/Cardano/Crypto/MonadSodium.hs b/cardano-crypto-class/src/Cardano/Crypto/MonadSodium.hs
@@ -67,6 +67,9 @@ class Monad m => MonadSodium m where
   mlsbUseAsCPtr :: forall n r. KnownNat n => MLockedSizedBytes n -> (Ptr Word8 -> m r) -> m r
   mlsbFromByteString :: forall n. KnownNat n => BS.ByteString -> m (MLockedSizedBytes n)
   mlsbFromByteStringCheck :: forall n. KnownNat n => BS.ByteString -> m (Maybe (MLockedSizedBytes n))
+
+  -- | Note that this function will leak mlocked memory to the Haskell heap
+  -- and should not be used in production code.
   mlsbToByteString :: forall n. KnownNat n => MLockedSizedBytes n -> m BS.ByteString
 
   -- * SafePinned