From 14a52d45a53c30e48b22035557992a200341424d Mon Sep 17 00:00:00 2001 From: Joris Dral Date: Mon, 17 Nov 2025 12:17:47 +0100 Subject: [PATCH] Fix an "insufficient buffer space" bug in `Botan.Low.Cipher.cipherUpdate` If the final update is performed and the output buffer is too small, we now retry with the proper output buffer size. --- botan-low/CHANGELOG.md | 3 + botan-low/src/Botan/Low/Cipher.hs | 74 +++++++++++++++---------- botan-low/test/Test/Botan/Low/Cipher.hs | 28 +++++++--- 3 files changed, 70 insertions(+), 35 deletions(-) diff --git a/botan-low/CHANGELOG.md b/botan-low/CHANGELOG.md index b756d1f..53b072b 100644 --- a/botan-low/CHANGELOG.md +++ b/botan-low/CHANGELOG.md @@ -42,6 +42,9 @@ * PATCH: fix an "insufficient buffer space" bug in `Botan.Low.PubKey.Encrypt.encrypt` and `Botan.Low.PubKey.Decrypt.decrypt`. See PR [#79](https://github.com/haskell-cryptography/botan/pull/79). +* PATCH: Fix an "insufficient buffer space" bug in + `Botan.Low.Cipher.cipherUpdate`. See PR + [#84](https://github.com/haskell-cryptography/botan/pull/84) ## 0.0.2.0 -- 2025-09-17 diff --git a/botan-low/src/Botan/Low/Cipher.hs b/botan-low/src/Botan/Low/Cipher.hs index f6ec7f7..b222e36 100644 --- a/botan-low/src/Botan/Low/Cipher.hs +++ b/botan-low/src/Botan/Low/Cipher.hs @@ -97,15 +97,14 @@ module Botan.Low.Cipher ( ) where -import qualified Data.ByteString as ByteString - import Botan.Bindings.Cipher - import Botan.Low.BlockCipher import Botan.Low.Error import Botan.Low.Make import Botan.Low.Prelude import Botan.Low.Remake +import Data.Bits ((.&.)) +import qualified Data.ByteString as ByteString {- $introduction @@ -442,31 +441,50 @@ cipherStart = mkWithObjectSetterCBytesLen withCipher botan_cipher_start -- https://github.com/randombit/botan/blob/72dc18bbf598f2c3bef81a4fb2915e9c3c524ac4/src/lib/ffi/ffi_cipher.cpp#L133 -- -- Some ciphers (ChaChaPoly, EAX) may consume less input than the reported ideal granularity -cipherUpdate - :: Cipher -- ^ __cipher__ - -> CipherUpdateFlags -- ^ __flags__ - -> Int -- ^ __output_size__ - -> ByteString -- ^ __input_bytes[]__ - -> IO (Int,ByteString) -- ^ __(input_consumed,output[])__ -cipherUpdate ctx flags outputSz input = withCipher ctx $ \ ctxPtr -> do - unsafeAsBytesLen input $ \ inputPtr inputSz -> do - alloca $ \ consumedPtr -> do - alloca $ \ writtenPtr -> do - output <- allocBytes outputSz $ \ outputPtr -> do - throwBotanIfNegative_ $ botan_cipher_update - ctxPtr - (fromIntegral flags) - outputPtr - (fromIntegral outputSz) - writtenPtr - (ConstPtr inputPtr) - inputSz - consumedPtr - consumed <- fromIntegral <$> peek consumedPtr - written <- fromIntegral <$> peek writtenPtr - -- NOTE: The safety of this function is suspect - may require deepseq - let processed = ByteString.take written output - in processed `seq` return (consumed,processed) +cipherUpdate :: + Cipher -- ^ __cipher__ + -> CipherUpdateFlags -- ^ __flags__ + -> Int -- ^ __output_size__ + -> ByteString -- ^ __input_bytes[]__ + -> IO (Int,ByteString) -- ^ __(input_consumed,output[])__ +cipherUpdate ctx flags outputSz input = + withCipher ctx $ \ ctxPtr -> + unsafeAsBytesLen input $ \ inputPtr inputSz -> + alloca $ \ consumedPtr -> + alloca $ \ writtenPtr -> do + eithOutput <- + try $ allocBytes outputSz $ \ outputPtr ->do + throwBotanIfNegative_ $ botan_cipher_update + ctxPtr + (fromIntegral flags) + outputPtr + (fromIntegral outputSz) + writtenPtr + (ConstPtr inputPtr) + inputSz + consumedPtr + -- If inssuficient buffer space, try again + output <- case eithOutput of + Left InsufficientBufferSpaceException{} -> do + outputSz' <- peek writtenPtr + allocBytes (fromIntegral outputSz') $ \ outputPtr ->do + throwBotanIfNegative_ $ botan_cipher_update + ctxPtr + (fromIntegral flags) + outputPtr + outputSz' + writtenPtr + (ConstPtr inputPtr) + -- No input should be provided on the second try if the first + -- try had the FINAL flag set + (if flags .&. BOTAN_CIPHER_UPDATE_FLAG_FINAL /= 0 then 0 else inputSz) + consumedPtr + Right bs -> pure bs + consumed <- fromIntegral <$> peek consumedPtr + written <- fromIntegral <$> peek writtenPtr + -- NOTE: The safety of this function is suspect - may require deepseq + let processed = ByteString.take written output + in processed `seq` return (consumed,processed) {- | Encrypt and finalize a complete piece of data. diff --git a/botan-low/test/Test/Botan/Low/Cipher.hs b/botan-low/test/Test/Botan/Low/Cipher.hs index c2c4db0..0253452 100644 --- a/botan-low/test/Test/Botan/Low/Cipher.hs +++ b/botan-low/test/Test/Botan/Low/Cipher.hs @@ -3,9 +3,14 @@ module Test.Botan.Low.Cipher (tests) where +import Botan.Bindings.Version (botan_version_major, + botan_version_minor) import Botan.Low.Cipher import Botan.Low.RNG import Control.Monad +import Data.ByteString (ByteString) +import qualified Data.ByteString as BS +import System.IO.Unsafe (unsafePerformIO) import Test.Hspec import Test.Tasty import Test.Tasty.Hspec @@ -17,13 +22,25 @@ tests = do specs <- testSpec "spec_cipher" spec_cipher pure $ testGroup "Test.Botan.Low.Cipher" [ specs - -- TODO: temporarily disabled because the test suite fails. See issue - -- #33. - | False ] +testModes :: [ByteString] +testModes = filter p (cipherModes ++ aeads) + where + p s + -- TODO: also test "Lion" and "Cascade" + | "Lion" `BS.isPrefixOf` s || "Cascade" `BS.isPrefixOf` s + = False + -- SIV and CCM have bugs on versions earlier than 3.5 + | unsafePerformIO botan_version_major == 3 + , unsafePerformIO botan_version_minor <= 4 + , "SIV" `BS.isSuffixOf` s || "CCM" `BS.isSuffixOf` s + = False + | otherwise + = True + spec_cipher :: Spec -spec_cipher = testSuite (cipherModes ++ aeads) chars $ \ cipher -> do +spec_cipher = testSuite testModes chars $ \ cipher -> do it "can initialize a cipher encryption context" $ do _ctx <- cipherInit cipher Encrypt pass @@ -221,10 +238,7 @@ spec_cipher = testSuite (cipherModes ++ aeads) chars $ \ cipher -> do cipherStart offlinectx n g <- cipherGetIdealUpdateGranularity onlinectx msg <- systemRNGGet (8 * g) - putStrLn " Testing online encryption:" onlinemsg <- cipherEncryptOnline onlinectx msg - putStrLn " Testing offline encryption:" offlinemsg <- cipherEncrypt offlinectx msg - putStrLn " Result:" onlinemsg `shouldBe` offlinemsg pass