Repository: randombit/botan (cpp)

Issue:
AES-CTR (and probably more stream ciphers) are slow when used via the FFI's Stream_Cipher mode.
Hello everyone,

I have created a benchmarking tool to compare performance between libraries and have noticed a significant drop in performance with Botan's AES CTR. In my benchmark, I generated plaintexts with a length of 4096 Bytes for each round and measured the time it takes for 1 million iterations. For my performance test I have used the Botan C interface ffi.h.

Results:

AES-NI and SSSE3 enabled:

| Library   | Algorithm    | Time (seconds) |
|-----------|--------------|----------------|
|Botan 3.2.0|     	 AES-128     	 |0.554971       
|Botan 3.2.0 |   	 AES-128/CTR 	|77.982720
|Botan 3.2.0  |   	 AES-128/CBC 	|5.963908
|Botan 3.2.0   |  	 AES-128/CFB 	|7.617746

AES-NI disabled:

| Library   | Algorithm    | Time (seconds) |
|-----------|--------------|----------------|
| Botan 3.2.0 | AES-128      | 9.187737       |
| Botan 3.2.0 | AES-128/CTR  | 81.234424      |
| Botan 3.2.0 | AES-128/CBC  | 19.137964      |
| Botan 3.2.0 | AES-128/CFB  | 20.943194      |

 With AES-NI and SSSE3 disabled:

| Library    | Algorithm    | Time (seconds) |
|------------|--------------|----------------|
| Botan 3.2.0 | AES-128      | 66.311669     |
| Botan 3.2.0 | AES-128/CTR  | 135.916029    |
| Botan 3.2.0 | AES-128/CBC  | 153.400266    |
| Botan 3.2.0 | AES-128/CFB  | 151.787276    |


### Environment:
- **Operating System:** Ubuntu 22.04.3 LTS (Codename: jammy)
- **CPU:** AMD Ryzen 5 5600U with Radeon Graphics
  - **Architecture:** x86_64
  - **CPU(s):** 12
  - **Thread(s) per core:** 2
  - **Core(s) per socket:** 6
  - **CPU max MHz:** 4289,0000
  - **CPU min MHz:** 400,0000
 ```bash
lscpu | grep "Byte Order"
Byte Order:                         Little Endian
```
I suspect that the AES CTR encryption process is not being parallelized, which could be causing the significant drop in performance. However, I'm uncertain about the exact reason for this issue and would appreciate your assistance in investigating further.

Thank you and best regards,
Ahmed 

Edit 1: add Byte Order
Edit 2: fix a comma typo 

Pull request:
Improve FFI's botan_cipher_update() performance for stream ciphers
### Pull Request Dependencies

* #3969 
* #3971 

### Description

This reworks the implementation of FFI `botan_cipher_update()`, making it comply with [the general "rules of engagement" of the FFI](https://botan.randombit.net/handbook/api_ref/ffi.html#rules-of-engagement). **Note that this subtly changes the behavior when finalizing a cipher without providing a big enough output buffer up-front.**

When the caller failed to provide a big enough output buffer, it now returns `BOTAN_FFI_ERROR_INSUFFICIENT_BUFFER_SPACE` instead of `BOTAN_FFI_ERROR_INVALID_INPUT` and sets `output_written` to the required number of bytes. In that case, re-invoking `botan_cipher_update()` with the "final" flag and a sufficiently sized output buffer then obtains the remaining bytes and resets the cipher object. 

Before, the re-invocation had to happen _without_ setting the final flag again to obtain the same behavior.
@randombit Frankly, I doubt that the existing implementation supported such a usage explicitly. It did feel like an inadvertent hack to me. That's why I changed it to an (IMHO) more logical interface. Nevertheless, there might be an argument to keep the existing behavior, albeit inconsistent with the generic "rules of engagement".

The rework should also significantly improve the performance of stream cipher modes (e.g. CTR-mode) via the FFI by opportunistically leveraging the `ideal_update_granularity()` of the cipher mode. Before, we generally processed data in chunks of `update_granularity()`, which is "1 byte" for stream ciphers. That resulted in very poor performance, as described in #3925.

**TODO:** We might look into the internal function `ffi_choose_update_size()` and whether it still makes sense in this new setup. Frankly, I'm lacking the historic context to conclusively judge that.

Closes #3925.

Patch:
diff --git a/src/cli/asn1.cpp b/src/cli/asn1.cpp
index e72b35548..1b9154844 100644
--- a/src/cli/asn1.cpp
+++ b/src/cli/asn1.cpp
@@ -102,7 +102,7 @@ class OID_Info final : public Command {
             }
 
             return;
-         } catch(Botan::Exception&) {}
+         } catch(Botan::Decoding_Error&) {}
 
          // This throws if the string is not known
          Botan::OID oid = Botan::OID::from_string(oid_str);
diff --git a/src/lib/asn1/asn1_obj.h b/src/lib/asn1/asn1_obj.h
index 889b88226..8a5b70bd3 100644
--- a/src/lib/asn1/asn1_obj.h
+++ b/src/lib/asn1/asn1_obj.h
@@ -12,7 +12,6 @@
 #include <chrono>
 #include <iosfwd>
 #include <optional>
-#include <span>
 #include <string>
 #include <string_view>
 #include <unordered_map>
@@ -152,8 +151,6 @@ class BOTAN_PUBLIC_API(2, 0) BER_Object final {
 
       size_t length() const { return m_value.size(); }
 
-      std::span<const uint8_t> data() const { return std::span{m_value}; }
-
       void assert_is_a(ASN1_Type type_tag, ASN1_Class class_tag, std::string_view descr = "object") const;
 
       bool is_a(ASN1_Type type_tag, ASN1_Class class_tag) const;
@@ -222,21 +219,23 @@ class BOTAN_PUBLIC_API(2, 0) OID final : public ASN1_Object {
 
       /**
       * Construct an OID from a string.
-      * @param str a string in the form "a.b.c" etc., where a,b,c are integers
-      *
-      * Note: it is currently required that each integer fit into 32 bits
+      * @param str a string in the form "a.b.c" etc., where a,b,c are numbers
       */
       explicit OID(std::string_view str);
 
       /**
       * Initialize an OID from a sequence of integer values
       */
-      explicit OID(std::initializer_list<uint32_t> init);
+      explicit OID(std::initializer_list<uint32_t> init) : m_id(init) {
+         BOTAN_ARG_CHECK(m_id.size() > 2 && m_id[0] <= 2 && (m_id[0] != 2 || m_id[1] <= 39), "Invalid OID");
+      }
 
       /**
       * Initialize an OID from a vector of integer values
       */
-      BOTAN_DEPRECATED("Use another contructor") explicit OID(std::vector<uint32_t>&& init);
+      explicit OID(std::vector<uint32_t>&& init) : m_id(init) {
+         BOTAN_ARG_CHECK(m_id.size() > 2 && m_id[0] <= 2 && (m_id[0] != 2 || m_id[1] <= 39), "Invalid OID");
+      }
 
       /**
       * Construct an OID from a string.
@@ -269,7 +268,15 @@ class BOTAN_PUBLIC_API(2, 0) OID final : public ASN1_Object {
       * Find out whether this OID has a value
       * @return true is this OID has a value
       */
-      bool has_value() const { return !empty(); }
+      bool has_value() const { return (m_id.empty() == false); }
+
+      /**
+      * Get this OID as list (vector) of its components.
+      * @return vector representing this OID
+      */
+      const std::vector<uint32_t>& get_components() const { return m_id; }
+
+      const std::vector<uint32_t>& get_id() const { return get_components(); }
 
       /**
       * Get this OID as a dotted-decimal string
@@ -301,28 +308,14 @@ class BOTAN_PUBLIC_API(2, 0) OID final : public ASN1_Object {
       */
       bool operator==(const OID& other) const { return m_id == other.m_id; }
 
-      /**
-      * Get this OID as list (vector) of its components.
-      * @return vector representing this OID
-      */
-      BOTAN_DEPRECATED("Do not access the integer values, use eg to_string")
-      const std::vector<uint32_t>& get_components() const {
-         return m_id;
-      }
-
-      BOTAN_DEPRECATED("Do not access the integer values, use eg to_string")
-      const std::vector<uint32_t>& get_id() const {
-         return m_id;
-      }
-
    private:
+      std::unordered_map<std::string, std::string> load_oid2str_map();
+      std::unordered_map<std::string, OID> load_str2oid_map();
+
       std::vector<uint32_t> m_id;
 };
 
-inline std::ostream& operator<<(std::ostream& out, const OID& oid) {
-   out << oid.to_string();
-   return out;
-}
+std::ostream& operator<<(std::ostream& out, const OID& oid);
 
 /**
 * Compare two OIDs.
diff --git a/src/lib/asn1/asn1_oid.cpp b/src/lib/asn1/asn1_oid.cpp
index 56f10d29b..4b0d0dbd5 100644
--- a/src/lib/asn1/asn1_oid.cpp
+++ b/src/lib/asn1/asn1_oid.cpp
@@ -1,6 +1,6 @@
 /*
 * ASN.1 OID
-* (C) 1999-2007,2024 Jack Lloyd
+* (C) 1999-2007 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -11,27 +11,15 @@
 #include <botan/der_enc.h>
 #include <botan/internal/bit_ops.h>
 #include <botan/internal/fmt.h>
-#include <botan/internal/int_utils.h>
 #include <botan/internal/oid_map.h>
 #include <botan/internal/parsing.h>
-#include <botan/internal/stl_util.h>
 #include <algorithm>
-#include <span>
 #include <sstream>
 
 namespace Botan {
 
 namespace {
 
-void oid_valid_check(std::span<const uint32_t> oid) {
-   BOTAN_ARG_CHECK(oid.size() >= 2, "OID too short to be valid");
-   BOTAN_ARG_CHECK(oid[0] <= 2, "OID root out of range");
-   BOTAN_ARG_CHECK(oid[1] <= 39 || oid[0] == 2, "OID second arc too large");
-   // This last is a limitation of using 32 bit integers when decoding
-   // not a limitation of ASN.1 object identifiers in general
-   BOTAN_ARG_CHECK(oid[1] <= 0xFFFFFFAF, "OID second arc too large");
-}
-
 // returns empty on invalid
 std::vector<uint32_t> parse_oid_str(std::string_view oid) {
    try {
@@ -55,8 +43,8 @@ std::vector<uint32_t> parse_oid_str(std::string_view oid) {
       }
 
       return oid_elems;
-   } catch(Invalid_Argument&) {
-      // thrown by to_u32bit
+   } catch(Invalid_Argument&)  // thrown by to_u32bit
+   {
       return std::vector<uint32_t>();
    }
 }
@@ -93,20 +81,13 @@ OID OID::from_string(std::string_view str) {
       return o;
    }
 
-   // Try to parse as a dotted decimal
-   try {
-      return OID(str);
-   } catch(...) {}
+   std::vector<uint32_t> raw = parse_oid_str(str);
 
-   throw Lookup_Error(fmt("No OID associated with name '{}'", str));
-}
-
-OID::OID(std::initializer_list<uint32_t> init) : m_id(init) {
-   oid_valid_check(m_id);
-}
+   if(!raw.empty()) {
+      return OID(std::move(raw));
+   }
 
-OID::OID(std::vector<uint32_t>&& init) : m_id(std::move(init)) {
-   oid_valid_check(m_id);
+   throw Lookup_Error(fmt("No OID associated with name '{}'", str));
 }
 
 /*
@@ -115,7 +96,9 @@ OID::OID(std::vector<uint32_t>&& init) : m_id(std::move(init)) {
 OID::OID(std::string_view oid_str) {
    if(!oid_str.empty()) {
       m_id = parse_oid_str(oid_str);
-      oid_valid_check(m_id);
+      if(m_id.size() < 2 || m_id[0] > 2 || (m_id[0] < 2 && m_id[1] > 39)) {
+         throw Decoding_Error(fmt("Invalid OID '{}'", oid_str));
+      }
    }
 }
 
@@ -124,15 +107,7 @@ OID::OID(std::string_view oid_str) {
 */
 std::string OID::to_string() const {
    std::ostringstream out;
-
-   for(size_t i = 0; i != m_id.size(); ++i) {
-      // avoid locale issues with integer formatting
-      out << std::to_string(m_id[i]);
-      if(i != m_id.size() - 1) {
-         out << ".";
-      }
-   }
-
+   out << (*this);
    return out.str();
 }
 
@@ -162,6 +137,20 @@ bool operator<(const OID& a, const OID& b) {
    return std::lexicographical_compare(oid1.begin(), oid1.end(), oid2.begin(), oid2.end());
 }
 
+std::ostream& operator<<(std::ostream& out, const OID& oid) {
+   const auto& val = oid.get_components();
+
+   for(size_t i = 0; i != val.size(); ++i) {
+      // avoid locale issues with integer formatting
+      out << std::to_string(val[i]);
+      if(i != val.size() - 1) {
+         out << ".";
+      }
+   }
+
+   return out;
+}
+
 /*
 * DER encode an OBJECT IDENTIFIER
 */
@@ -170,33 +159,28 @@ void OID::encode_into(DER_Encoder& der) const {
       throw Invalid_Argument("OID::encode_into: OID is invalid");
    }
 
-   auto append = [](std::vector<uint8_t>& encoding, uint32_t z) {
-      if(z <= 0x7F) {
-         encoding.push_back(static_cast<uint8_t>(z));
-      } else {
-         size_t z7 = (high_bit(z) + 7 - 1) / 7;
-
-         for(size_t j = 0; j != z7; ++j) {
-            uint8_t zp = static_cast<uint8_t>(z >> (7 * (z7 - j - 1)) & 0x7F);
-
-            if(j != z7 - 1) {
-               zp |= 0x80;
-            }
-
-            encoding.push_back(zp);
-         }
-      }
-   };
-
    std::vector<uint8_t> encoding;
 
-   // We know 40 * root can't overflow because root is between 0 and 2
-   auto first = BOTAN_ASSERT_IS_SOME(checked_add(40 * m_id[0], m_id[1]));
+   if(m_id[0] > 2 || m_id[1] >= 40) {
+      throw Encoding_Error("Invalid OID prefix, cannot encode");
+   }
 
-   append(encoding, first);
+   encoding.push_back(static_cast<uint8_t>(40 * m_id[0] + m_id[1]));
 
    for(size_t i = 2; i != m_id.size(); ++i) {
-      append(encoding, m_id[i]);
+      if(m_id[i] == 0) {
+         encoding.push_back(0);
+      } else {
+         size_t blocks = high_bit(m_id[i]) + 6;
+         blocks = (blocks - (blocks % 7)) / 7;
+
+         BOTAN_ASSERT(blocks > 0, "Math works");
+
+         for(size_t j = 0; j != blocks - 1; ++j) {
+            encoding.push_back(0x80 | ((m_id[i] >> 7 * (blocks - j - 1)) & 0x7F));
+         }
+         encoding.push_back(m_id[i] & 0x7F);
+      }
    }
    der.add_object(ASN1_Type::ObjectId, ASN1_Class::Universal, encoding);
 }
@@ -210,75 +194,35 @@ void OID::decode_from(BER_Decoder& decoder) {
       throw BER_Bad_Tag("Error decoding OID, unknown tag", obj.tagging());
    }
 
-   if(obj.length() == 0) {
+   const size_t length = obj.length();
+   const uint8_t* bits = obj.bits();
+
+   if(length < 2 && !(length == 1 && bits[0] == 0)) {
       throw BER_Decoding_Error("OID encoding is too short");
    }
 
-   auto consume = [](BufferSlicer& data) -> uint32_t {
-      BOTAN_ASSERT_NOMSG(!data.empty());
-      uint32_t b = data.take_byte();
+   m_id.clear();
+   m_id.push_back(bits[0] / 40);
+   m_id.push_back(bits[0] % 40);
 
-      if(b > 0x7F) {
-         b &= 0x7F;
+   size_t i = 0;
+   while(i != length - 1) {
+      uint32_t component = 0;
+      while(i != length - 1) {
+         ++i;
 
-         // Even BER requires that the OID have minimal length, ie that
-         // the first byte of a multibyte encoding cannot be zero
-         // See X.690 section 8.19.2
-         if(b == 0) {
-            throw Decoding_Error("Leading zero byte in multibyte OID encoding");
+         if(component >> (32 - 7)) {
+            throw Decoding_Error("OID component overflow");
          }
 
-         while(true) {
-            if(data.empty()) {
-               throw Decoding_Error("Truncated OID value");
-            }
-
-            const uint8_t next = data.take_byte();
-            const bool more = (next & 0x80);
-            const uint8_t value = next & 0x7F;
+         component = (component << 7) + (bits[i] & 0x7F);
 
-            if((b >> (32 - 7)) != 0) {
-               throw Decoding_Error("OID component overflow");
-            }
-
-            b = (b << 7) | value;
-
-            if(!more) {
-               break;
-            }
+         if(!(bits[i] & 0x80)) {
+            break;
          }
       }
-
-      return b;
-   };
-
-   BufferSlicer data(obj.data());
-   std::vector<uint32_t> parts;
-   while(!data.empty()) {
-      const uint32_t comp = consume(data);
-
-      if(parts.empty()) {
-         // divide into root and second arc
-
-         const uint32_t root_arc = [](uint32_t b0) -> uint32_t {
-            if(b0 < 40) {
-               return 0;
-            } else if(b0 < 80) {
-               return 1;
-            } else {
-               return 2;
-            }
-         }(comp);
-
-         parts.push_back(root_arc);
-         BOTAN_ASSERT_NOMSG(comp >= 40 * root_arc);
-         parts.push_back(comp - 40 * root_arc);
-      } else {
-         parts.push_back(comp);
-      }
+      m_id.push_back(component);
    }
-
-   m_id = parts;
 }
 
 }  // namespace Botan
diff --git a/src/lib/ffi/ffi.h b/src/lib/ffi/ffi.h
index 506507a2b..bf26d1d87 100644
--- a/src/lib/ffi/ffi.h
+++ b/src/lib/ffi/ffi.h
@@ -621,7 +621,25 @@ BOTAN_FFI_EXPORT(2, 0) int botan_cipher_start(botan_cipher_t cipher, const uint8
 #define BOTAN_CIPHER_UPDATE_FLAG_FINAL (1U << 0)
 
 /**
-* Encrypt some data
+* @brief Encrypt/Decrypt some data and/or finalize the encryption/decryption
+*
+* This encrypts as many bytes from @p input_bytes into @p output_bytes as
+* possible. Unless ``BOTAN_CIPHER_UPDATE_FLAG_FINAL`` is set, this function will
+* consume bytes in multiples of botan_cipher_get_update_granularity().
+* @p input_consumed and @p output_written will be set accordingly and it is the
+* caller's responsibility to adapt their buffers accordingly before calling this
+* function again.
+*
+* Eventually, the caller must set the ``BOTAN_CIPHER_UPDATE_FLAG_FINAL`` flag to
+* indicate that no more input will be provided. This will cause the cipher to
+* consume all given input bytes and produce the final output; or return a
+* ``BOTAN_FFI_ERROR_INSUFFICIENT_BUFFER_SPACE`` error if the given output buffer
+* was too small. In the latter case, @p output_written will be set to the
+* required buffer size. Calling again with ``BOTAN_CIPHER_UPDATE_FLAG_FINAL``, a
+* big enough buffer and no further input will then produce the final output.
+*
+* Note that some ciphers require the entire message to be provided before any
+* output is produced. @sa botan_cipher_requires_entire_message().
 */
 BOTAN_FFI_EXPORT(2, 0)
 int botan_cipher_update(botan_cipher_t cipher,
diff --git a/src/lib/ffi/ffi_cipher.cpp b/src/lib/ffi/ffi_cipher.cpp
index 23239c0d8..a72ea50f5 100644
--- a/src/lib/ffi/ffi_cipher.cpp
+++ b/src/lib/ffi/ffi_cipher.cpp
@@ -8,6 +8,7 @@
 
 #include <botan/aead.h>
 #include <botan/internal/ffi_util.h>
+#include <botan/internal/stl_util.h>
 
 extern "C" {
 
@@ -15,18 +16,24 @@ using namespace Botan_FFI;
 
 struct botan_cipher_struct final : public botan_struct<Botan::Cipher_Mode, 0xB4A2BF9C> {
    public:
-      explicit botan_cipher_struct(std::unique_ptr<Botan::Cipher_Mode> x, size_t update_size) :
-            botan_struct(std::move(x)), m_update_size(update_size) {
-         m_buf.reserve(m_update_size);
+      explicit botan_cipher_struct(std::unique_ptr<Botan::Cipher_Mode> x,
+                                   size_t update_size,
+                                   size_t ideal_update_size) :
+            botan_struct(std::move(x)), m_update_size(update_size), m_ideal_update_size(ideal_update_size) {
+         BOTAN_DEBUG_ASSERT(ideal_update_size >= update_size);
+         m_buf.reserve(m_ideal_update_size);
       }
 
       Botan::secure_vector<uint8_t>& buf() { return m_buf; }
 
       size_t update_size() const { return m_update_size; }
 
+      size_t ideal_update_size() const { return m_ideal_update_size; }
+
    private:
       Botan::secure_vector<uint8_t> m_buf;
       size_t m_update_size;
+      size_t m_ideal_update_size;
 };
 
 namespace {
@@ -71,8 +78,9 @@ int botan_cipher_init(botan_cipher_t* cipher, const char* cipher_name, uint32_t
       }
 
       const size_t update_size = ffi_choose_update_size(*mode);
+      const size_t ideal_update_size = std::max(mode->ideal_granularity(), update_size);
 
-      *cipher = new botan_cipher_struct(std::move(mode), update_size);
+      *cipher = new botan_cipher_struct(std::move(mode), update_size, ideal_update_size);
       return BOTAN_FFI_SUCCESS;
    });
 }
@@ -132,81 +140,117 @@ int botan_cipher_start(botan_cipher_t cipher_obj, const uint8_t* nonce, size_t n
 
 int botan_cipher_update(botan_cipher_t cipher_obj,
                         uint32_t flags,
-                        uint8_t output_ptr[],
-                        size_t orig_output_size,
+                        uint8_t output[],
+                        size_t output_size,
                         size_t* output_written,
-                        const uint8_t input_ptr[],
-                        size_t orig_input_size,
+                        const uint8_t input[],
+                        size_t input_size,
                         size_t* input_consumed) {
    return ffi_guard_thunk(__func__, [=]() -> int {
-      size_t input_size = orig_input_size;
-      size_t output_size = orig_output_size;
-      const uint8_t* input = input_ptr;
-      uint8_t* output = output_ptr;
-
       using namespace Botan;
       Cipher_Mode& cipher = safe_get(cipher_obj);
       secure_vector<uint8_t>& mbuf = cipher_obj->buf();
 
+      // If the cipher object's internal buffer contains residual data from
+      // a previous invocation, we can be sure that botan_cipher_update() was
+      // called with the final flag set but not enough buffer space was provided
+      // to accommodate the final output.
+      const bool was_finished_before = !mbuf.empty();
       const bool final_input = (flags & BOTAN_CIPHER_UPDATE_FLAG_FINAL);
 
-      if(final_input) {
-         mbuf.assign(input, input + input_size);
-         *input_consumed = input_size;
-         *output_written = 0;
-
-         try {
-            cipher.finish(mbuf);
-         } catch(Invalid_Authentication_Tag&) {
-            return BOTAN_FFI_ERROR_BAD_MAC;
-         }
+      // Bring the output variables into a defined state.
+      *output_written = 0;
+      *input_consumed = 0;
 
-         *output_written = mbuf.size();
-
-         if(mbuf.size() <= output_size) {
-            copy_mem(output, mbuf.data(), mbuf.size());
-            mbuf.clear();
-            return BOTAN_FFI_SUCCESS;
-         }
-
-         return -1;
+      // Once the final flag was set once, it must always be set for
+      // consecutive invocations.
+      if(was_finished_before && !final_input) {
+         return BOTAN_FFI_ERROR_INVALID_OBJECT_STATE;
       }
 
-      if(input_size == 0) {
-         // Currently must take entire buffer in this case
-         *output_written = mbuf.size();
-         if(output_size >= mbuf.size()) {
-            copy_mem(output, mbuf.data(), mbuf.size());
-            mbuf.clear();
-            return BOTAN_FFI_SUCCESS;
-         }
-
-         return -1;
+      // If the final flag was set in a previous invocation, no more input
+      // data can be processed.
+      if(was_finished_before && input_size > 0) {
+         return BOTAN_FFI_ERROR_BAD_PARAMETER;
       }
 
-      const size_t ud = cipher_obj->update_size();
+      // Make sure that we always clear the internal buffer before returning
+      // or aborting this invocation due to an exception.
+      auto clean_buffer = scoped_cleanup([&mbuf] { mbuf.clear(); });
 
-      mbuf.resize(ud);
-      size_t taken = 0, written = 0;
-
-      while(input_size >= ud && output_size >= ud) {
-         copy_mem(mbuf.data(), input, ud);
-         const size_t bytes_produced = cipher.process(mbuf);
+      if(final_input) {
+         // If the final flag is set for the first time, we need to process the
+         // remaining input data and then finalize the cipher object.
+         if(!was_finished_before) {
+            *input_consumed = input_size;
+            mbuf.resize(input_size);
+            copy_mem(mbuf, std::span(input, input_size));
+
+            try {
+               cipher.finish(mbuf);
+            } catch(Invalid_Authentication_Tag&) {
+               return BOTAN_FFI_ERROR_BAD_MAC;
+            }
+         }
 
-         input_size -= ud;
-         input += ud;
-         taken += ud;
+         // At this point, the cipher object is finalized (potentially in a
+         // previous invocation) and we can copy the final output to the caller.
+         *output_written = mbuf.size();
 
-         if(bytes_produced > 0) {
-            copy_mem(output, mbuf.data(), bytes_produced);
-            output_size -= bytes_produced;
-            output += bytes_produced;
-            written += bytes_produced;
+         // Not enough space to copy the final output out to the caller.
+         // Inform them how much space we need for a successful operation.
+         if(output_size < mbuf.size()) {
+            // This is the only place where mbuf is not cleared before returning.
+            clean_buffer.disengage();
+            return BOTAN_FFI_ERROR_INSUFFICIENT_BUFFER_SPACE;
          }
-      }
 
-      *output_written = written;
-      *input_consumed = taken;
+         // Copy the final output to the caller, mbuf is cleared afterwards.
+         copy_mem(std::span(output, mbuf.size()), mbuf);
+      } else {
+         // Process data in a streamed fashion without finalizing. No data is
+         // ever retained in the cipher object's internal buffer. If we run out
+         // of either input data or output capacity, we stop and report that not
+         // all bytes were processed via *output_written and *input_consumed.
+
+         BufferSlicer in({input, input_size});
+         BufferStuffer out({output, output_size});
+
+         // Helper function to do blockwise processing of data.
+         auto blockwise_update = [&](const size_t granularity) {
+            if(granularity == 0) {
+               return;
+            }
+
+            const size_t expected_output_per_iteration = cipher.requires_entire_message() ? 0 : granularity;
+            mbuf.resize(granularity);
+
+            while(in.remaining() >= granularity && out.remaining_capacity() >= expected_output_per_iteration) {
+               copy_mem(mbuf, in.take(granularity));
+               const auto written_bytes = cipher.process(mbuf);
+               BOTAN_DEBUG_ASSERT(written_bytes == expected_output_per_iteration);
+               if(written_bytes > 0) {
+                  BOTAN_ASSERT_NOMSG(written_bytes <= granularity);
+                  copy_mem(out.next(written_bytes), std::span(mbuf).first(written_bytes));
+               }
+            }
+         };
+
+         // First, process as much data as possible in chunks of ideal granularity
+         blockwise_update(cipher_obj->ideal_update_size());
+
+         // Then process the remaining bytes in chunks of update_size() or, in one go
+         // if update_size() is equal to 1 --> i.e. likely a stream cipher.
+         const bool is_stream_cipher = (cipher_obj->update_size() == 1);
+         const size_t tail_granularity =
+            is_stream_cipher ? std::min(in.remaining(), out.remaining_capacity()) : cipher_obj->update_size();
+         BOTAN_DEBUG_ASSERT(tail_granularity < cipher_obj->ideal_update_size());
+         blockwise_update(tail_granularity);
+
+         // Inform the caller about the amount of data processed.
+         *output_written = output_size - out.remaining_capacity();
+         *input_consumed = input_size - in.remaining();
+      }
 
       return BOTAN_FFI_SUCCESS;
    });


Test patch:
diff --git a/src/tests/data/asn1_oid.vec b/src/tests/data/asn1_oid.vec
deleted file mode 100644
index c997c4369..000000000
--- a/src/tests/data/asn1_oid.vec
+++ /dev/null
@@ -1,62 +0,0 @@
-
-# Generated using OpenSSL
-
-[Valid]
-OID = 1.2.3.4
-DER = 06032A0304
-
-OID = 1.39.0.0.0.1
-DER = 06054F00000001
-
-OID = 1.39.0.0.0.1.20000000
-DER = 06094F0000000189C4DA00
-
-OID = 1.39.0.0.0.1.4294967295.1.4294967295
-DER = 06104F000000018FFFFFFF7F018FFFFFFF7F
-
-OID = 1.2
-DER = 06012A
-
-OID = 0.9.0.0
-DER = 0603090000
-
-OID = 1.37.1.0
-DER = 06034D0100
-
-OID = 1.39
-DER = 06014F
-
-OID = 0.1
-DER = 060101
-
-OID = 0.1.0
-DER = 06020100
-
-OID = 0.0.0
-DER = 06020000
-
-OID = 2.40.0
-DER = 06027800
-
-# GH 4023
-OID = 2.41.1
-DER = 06027901
-
-OID = 2.999.1234
-DER = 060488378952
-
-OID = 2.168.0
-DER = 0603817800
-
-OID = 2.48.0
-DER = 0603810000
-
-OID = 2.127.128.129.255
-DER = 0608814F81008101817F
-
-# Maximum 2.<x> we can accept while decoding
-OID = 2.4294967215.0.4294967215
-DER = 060B8FFFFFFF7F008FFFFFFF2F
-
-OID = 2.4294967215.0.4294967295
-DER = 060B8FFFFFFF7F008FFFFFFF7F
diff --git a/src/tests/data/asn1_oid_invalid.vec b/src/tests/data/asn1_oid_invalid.vec
deleted file mode 100644
index 8a28850b9..000000000
--- a/src/tests/data/asn1_oid_invalid.vec
+++ /dev/null
@@ -1,34 +0,0 @@
-
-# Wrong tag
-DER = 050100
-
-# Empty OID
-DER = 0600
-
-# Leading zero byte in multibyte encoding
-DER = 0603500180
-
-# Truncated
-DER = 060350018101
-
-# Multibyte encoding that doesn't end with high bit cleared
-DER = 060450018181
-
-# 2.1.4294967296 - this is valid, we just can't decode it due to 32 bit limbs
-DER = 0606519080808000
-
-# 2.4294967216 - this is valid, we just can't decode it due to 32 bit limbs
-DER = 06059080808000
-
-# From https://misc.daniel-marschall.de/asn.1/oid_facts.html
-
-DER = 060000
-DER = 068000
-DER = 06FF00
-DER = 080100
-DER = 06070180808080807F
-DER = 06028001
-DER = 0602807F
-
-# BUG: we accept constructed encoding of an OID, but should not
-#DER = 06800000
diff --git a/src/tests/test_ffi.cpp b/src/tests/test_ffi.cpp
index 0ed73ee8b..d74c41500 100644
--- a/src/tests/test_ffi.cpp
+++ b/src/tests/test_ffi.cpp
@@ -1296,7 +1296,7 @@ class FFI_AEAD_Test final : public FFI_Test {
             // botan_cipher_update().
             size_t final_output_written = 42;
             size_t final_input_consumed = 1337;
-            TEST_FFI_RC(BOTAN_FFI_ERROR_INVALID_INPUT,
+            TEST_FFI_RC(BOTAN_FFI_ERROR_INSUFFICIENT_BUFFER_SPACE,
                         botan_cipher_update,
                         (cipher_encrypt,
                          BOTAN_CIPHER_UPDATE_FLAG_FINAL,
@@ -1316,7 +1316,7 @@ class FFI_AEAD_Test final : public FFI_Test {
 
             TEST_FFI_OK(botan_cipher_update,
                         (cipher_encrypt,
-                         0 /* explicitly not setting final flag, to fall into the 'input-size == 0' case */,
+                         BOTAN_CIPHER_UPDATE_FLAG_FINAL,
                          final_ct_chunk.data(),
                          final_ct_chunk.size(),
                          &final_output_written,
diff --git a/src/tests/test_oid.cpp b/src/tests/test_oid.cpp
index ad6bfb69d..a08676272 100644
--- a/src/tests/test_oid.cpp
+++ b/src/tests/test_oid.cpp
@@ -1,6 +1,6 @@
 /*
 * (C) 2016 Daniel Neus, Rohde & Schwarz Cybersecurity
-*     2023,2024 Jack Lloyd
+*     2023 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -9,8 +9,6 @@
 
 #if defined(BOTAN_HAS_ASN1)
    #include <botan/asn1_obj.h>
-   #include <botan/ber_dec.h>
-   #include <botan/der_enc.h>
    #include <botan/oids.h>
 #endif
 
@@ -137,68 +135,6 @@ class OID_Tests final : public Test {
 
 BOTAN_REGISTER_TEST("asn1", "oid", OID_Tests);
 
-class OID_Encoding_Tests : public Text_Based_Test {
-   public:
-      OID_Encoding_Tests() : Text_Based_Test("asn1_oid.vec", "OID,DER") {}
-
-      Test::Result run_one_test(const std::string&, const VarMap& vars) override {
-         const auto oid_str = vars.get_req_str("OID");
-         const auto expected_der = vars.get_req_bin("DER");
-
-         Test::Result result("OID DER encode/decode");
-
-         const Botan::OID oid(oid_str);
-
-         try {
-            std::vector<uint8_t> der;
-            Botan::DER_Encoder enc(der);
-            enc.encode(oid);
-            result.test_eq("Encoding correct", der, expected_der);
-         } catch(std::exception& e) {
-            result.test_failure("Encoding OID failed", e.what());
-         }
-
-         try {
-            Botan::BER_Decoder dec(expected_der);
-            Botan::OID dec_oid;
-            dec.decode(dec_oid);
-            dec.verify_end();
-            result.test_eq("Decoding OID correct", dec_oid.to_string(), oid_str);
-         } catch(std::exception& e) {
-            result.test_failure("Decoding OID failed", e.what());
-         }
-
-         return result;
-      }
-};
-
-BOTAN_REGISTER_TEST("asn1", "oid_enc", OID_Encoding_Tests);
-
-class OID_Invalid_Encoding_Tests : public Text_Based_Test {
-   public:
-      OID_Invalid_Encoding_Tests() : Text_Based_Test("asn1_oid_invalid.vec", "DER") {}
-
-      Test::Result run_one_test(const std::string&, const VarMap& vars) override {
-         const auto test_der = vars.get_req_bin("DER");
-
-         Test::Result result("OID DER decode invalid");
-
-         try {
-            Botan::BER_Decoder dec(test_der);
-            Botan::OID oid;
-            dec.decode(oid);
-            dec.verify_end();
-            result.test_failure("Accepted invalid OID encoding", oid.to_string());
-         } catch(Botan::Decoding_Error&) {
-            result.test_success("Rejected invalid OID with Decoding_Error");
-         }
-
-         return result;
-      }
-};
-
-BOTAN_REGISTER_TEST("asn1", "oid_dec_invalid", OID_Invalid_Encoding_Tests);
-
 #endif
 
 }  // namespace