Refactor internal usage of PK encryption padding

Revamp EME class with std::span and CT::Option

src/fuzzer/oaep.cpp

@@ -11,77 +11,46 @@
 
 namespace {
 
-Botan::secure_vector<uint8_t> ref_oaep_unpad(uint8_t& valid_mask,
-                                             const uint8_t in[],
-                                             size_t len,
-                                             const Botan::secure_vector<uint8_t>& Phash) {
+Botan::CT::Option<size_t> ref_oaep_unpad(const uint8_t in[], size_t len, const std::vector<uint8_t>& Phash) {
    const size_t hlen = Phash.size();
 
    if(len < 2 * hlen + 1) {
-      return Botan::secure_vector<uint8_t>();
+      return Botan::CT::Option<size_t>();
    }
 
    for(size_t i = hlen; i != 2 * hlen; ++i) {
       if(in[i] != Phash[i - hlen]) {
-         return Botan::secure_vector<uint8_t>();
+         return Botan::CT::Option<size_t>();
       }
    }
 
    for(size_t i = 2 * hlen; i != len; ++i) {
       if(in[i] != 0x00 && in[i] != 0x01) {
-         return Botan::secure_vector<uint8_t>();
+         return Botan::CT::Option<size_t>();
       }
 
       if(in[i] == 0x01) {
-         valid_mask = 0xFF;
-         return Botan::secure_vector<uint8_t>(in + i + 1, in + len);
+         return Botan::CT::Option<size_t>(i + 1);
       }
    }
 
-   return Botan::secure_vector<uint8_t>();
-}
-
-inline bool all_zeros(const Botan::secure_vector<uint8_t>& v) {
-   for(size_t i = 0; i != v.size(); ++i) {
-      if(v[i] != 0) {
-         return false;
-      }
-   }
-   return true;
+   return Botan::CT::Option<size_t>();
 }
 
 }  // namespace
 
 void fuzz(const uint8_t in[], size_t len) {
-   static const Botan::secure_vector<uint8_t> Phash = {1, 2, 3, 4};
-
-   uint8_t lib_valid_mask = 0;
-   const Botan::secure_vector<uint8_t> lib_output = Botan::oaep_find_delim(lib_valid_mask, in, len, Phash);
-   FUZZER_ASSERT_TRUE(lib_valid_mask == 0 || lib_valid_mask == 0xFF);
+   static const std::vector<uint8_t> Phash = {1, 2, 3, 4};
 
-   uint8_t ref_valid_mask = 0;
-   const Botan::secure_vector<uint8_t> ref_output = ref_oaep_unpad(ref_valid_mask, in, len, Phash);
-   FUZZER_ASSERT_TRUE(ref_valid_mask == 0 || ref_valid_mask == 0xFF);
+   auto lib_idx = Botan::oaep_find_delim(std::span{in, len}, std::span{Phash});
 
-   if(ref_valid_mask == 0xFF && lib_valid_mask == 0x00) {
-      FUZZER_WRITE_AND_CRASH("Ref accepted but library rejected, output " << Botan::hex_encode(ref_output) << "\n");
-   } else if(ref_valid_mask == 0x00 && lib_valid_mask == 0xFF) {
-      FUZZER_WRITE_AND_CRASH("Lib accepted but ref rejected, output = " << Botan::hex_encode(lib_output) << "\n");
-   }
+   auto ref_idx = ref_oaep_unpad(in, len, Phash);
 
-   if(ref_valid_mask == 0x00) {
-      FUZZER_ASSERT_TRUE(all_zeros(ref_output));
-   }
-
-   if(lib_valid_mask == 0x00) {
-      FUZZER_ASSERT_TRUE(all_zeros(lib_output));
-   }
-
-   if(ref_valid_mask && lib_valid_mask) {
-      if(ref_output != lib_output) {
-         FUZZER_WRITE_AND_CRASH("Ref and lib both accepted but produced different output:"
-                                << " ref = " << Botan::hex_encode(ref_output)
-                                << " lib = " << Botan::hex_encode(lib_output));
-      }
+   if(lib_idx.has_value().as_bool() && ref_idx.has_value().as_bool()) {
+      FUZZER_ASSERT_EQUAL(lib_idx.value(), ref_idx.value());
+   } else if(lib_idx.has_value().as_bool() && !ref_idx.has_value().as_bool()) {
+      FUZZER_WRITE_AND_CRASH("Ref accepted but lib rejected\n");
+   } else if(!lib_idx.has_value().as_bool() && ref_idx.has_value().as_bool()) {
+      FUZZER_WRITE_AND_CRASH("Lib accepted but ref rejected\n");
    }
 }

src/fuzzer/pkcs1.cpp

@@ -37,21 +37,16 @@ std::vector<uint8_t> simple_pkcs1_unpad(const uint8_t in[], size_t len) {
 void fuzz(const uint8_t in[], size_t len) {
    static Botan::EME_PKCS1v15 pkcs1;
 
-   Botan::secure_vector<uint8_t> lib_result;
+   std::vector<uint8_t> lib_result;
    std::vector<uint8_t> ref_result;
    bool lib_rejected = false, ref_rejected = false;
 
    try {
-      uint8_t valid_mask = 0;
-      Botan::secure_vector<uint8_t> decoded = (static_cast<Botan::EME*>(&pkcs1))->unpad(valid_mask, in, len);
+      lib_result.resize(len);
+      auto written = (static_cast<Botan::EME*>(&pkcs1))->unpad(lib_result, std::span{in, len});
+      lib_rejected = !written.has_value().as_bool();
 
-      if(valid_mask == 0) {
-         lib_rejected = true;
-      } else if(valid_mask == 0xFF) {
-         lib_rejected = false;
-      } else {
-         FUZZER_WRITE_AND_CRASH("Invalid valid_mask from unpad");
-      }
+      lib_result.resize(written.value_or(0));
    } catch(Botan::Decoding_Error&) {
       lib_rejected = true;
    }

src/lib/pk_pad/eme.cpp

@@ -64,23 +64,6 @@ std::unique_ptr<EME> EME::create(std::string_view algo_spec) {
    throw Algorithm_Not_Found(algo_spec);
 }
 
-/*
-* Encode a message
-*/
-secure_vector<uint8_t> EME::encode(const uint8_t msg[],
-                                   size_t msg_len,
-                                   size_t key_bits,
-                                   RandomNumberGenerator& rng) const {
-   return pad(msg, msg_len, key_bits, rng);
-}
-
-/*
-* Encode a message
-*/
-secure_vector<uint8_t> EME::encode(const secure_vector<uint8_t>& msg,
-                                   size_t key_bits,
-                                   RandomNumberGenerator& rng) const {
-   return pad(msg.data(), msg.size(), key_bits, rng);
-}
+EME::~EME() = default;
 
 }  // namespace Botan

src/lib/pk_pad/eme.h

@@ -1,15 +1,17 @@
 /*
-* EME Classes
-* (C) 1999-2007 Jack Lloyd
+* (C) 1999-2007,2024 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
 
-#ifndef BOTAN_PUBKEY_EME_ENCRYPTION_PAD_H_
-#define BOTAN_PUBKEY_EME_ENCRYPTION_PAD_H_
+#ifndef BOTAN_PUBKEY_EME_H_
+#define BOTAN_PUBKEY_EME_H_
 
-#include <botan/secmem.h>
-#include <string>
+#include <botan/types.h>
+#include <botan/internal/ct_utils.h>
+#include <memory>
+#include <span>
+#include <string_view>
 
 namespace Botan {
 
@@ -18,9 +20,9 @@ class RandomNumberGenerator;
 /**
 * Encoding Method for Encryption
 */
-class EME {
+class BOTAN_TEST_API EME {
    public:
-      virtual ~EME() = default;
+      virtual ~EME();
 
       /**
       * Factory method for EME (message-encoding methods for encryption) objects
@@ -38,50 +40,26 @@ class EME {
 
       /**
       * Encode an input
-      * @param in the plaintext
-      * @param in_length length of plaintext in bytes
+      * @param output buffer that is written to
+      * @param input the plaintext
       * @param key_length length of the key in bits
       * @param rng a random number generator
-      * @return encoded plaintext
+      * @return number of bytes written to output
       */
-      secure_vector<uint8_t> encode(const uint8_t in[],
-                                    size_t in_length,
-                                    size_t key_length,
-                                    RandomNumberGenerator& rng) const;
-
-      /**
-      * Encode an input
-      * @param in the plaintext
-      * @param key_length length of the key in bits
-      * @param rng a random number generator
-      * @return encoded plaintext
-      */
-      secure_vector<uint8_t> encode(const secure_vector<uint8_t>& in,
-                                    size_t key_length,
-                                    RandomNumberGenerator& rng) const;
+      virtual size_t pad(std::span<uint8_t> output,
+                         std::span<const uint8_t> input,
+                         size_t key_length,
+                         RandomNumberGenerator& rng) const = 0;
 
       /**
       * Decode an input
-      * @param valid_mask written to specifies if output is valid
-      * @param in the encoded plaintext
-      * @param in_len length of encoded plaintext in bytes
-      * @return bytes of out[] written to along with
-      *         validity mask (0xFF if valid, else 0x00)
-      */
-      virtual secure_vector<uint8_t> unpad(uint8_t& valid_mask, const uint8_t in[], size_t in_len) const = 0;
-
-      /**
-      * Encode an input
-      * @param in the plaintext
-      * @param in_length length of plaintext in bytes
-      * @param key_length length of the key in bits
-      * @param rng a random number generator
-      * @return encoded plaintext
+      * @param output buffer where output is placed
+      * @param input the encoded plaintext
+      * @return number of bytes written to output if valid,
+      *  or an empty option if invalid. If an empty option is
+      *  returned the contents of output are undefined
       */
-      virtual secure_vector<uint8_t> pad(const uint8_t in[],
-                                         size_t in_length,
-                                         size_t key_length,
-                                         RandomNumberGenerator& rng) const = 0;
+      virtual CT::Option<size_t> unpad(std::span<uint8_t> output, std::span<const uint8_t> input) const = 0;
 };
 
 }  // namespace Botan

src/lib/pk_pad/eme_oaep/oaep.cpp

@@ -1,6 +1,6 @@
 /*
 * OAEP
-* (C) 1999-2010,2015,2018 Jack Lloyd
+* (C) 1999-2010,2015,2018,2024 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -18,38 +18,45 @@ namespace Botan {
 /*
 * OAEP Pad Operation
 */
-secure_vector<uint8_t> OAEP::pad(const uint8_t in[],
-                                 size_t in_length,
-                                 size_t key_length,
-                                 RandomNumberGenerator& rng) const {
+size_t OAEP::pad(std::span<uint8_t> output,
+                 std::span<const uint8_t> input,
+                 size_t key_length,
+                 RandomNumberGenerator& rng) const {
    key_length /= 8;
 
-   if(in_length > maximum_input_size(key_length * 8)) {
+   if(input.size() > maximum_input_size(key_length * 8)) {
       throw Invalid_Argument("OAEP: Input is too large");
    }
 
-   secure_vector<uint8_t> out(key_length);
-   BufferStuffer stuffer(out);
+   const size_t output_size = key_length;
+
+   output = output.first(output_size);  // remainder ignored
+
+   BufferStuffer stuffer(output);
 
    // We always use a seed len equal to the underlying hash
    rng.randomize(stuffer.next(m_Phash.size()));
    stuffer.append(m_Phash);
-   stuffer.append(0x00, stuffer.remaining_capacity() - (1 + in_length));
+   stuffer.append(0x00, stuffer.remaining_capacity() - (1 + input.size()));
    stuffer.append(0x01);
-   stuffer.append({in, in_length});
+   stuffer.append(input);
    BOTAN_ASSERT_NOMSG(stuffer.full());
 
-   mgf1_mask(*m_mgf1_hash, out.data(), m_Phash.size(), &out[m_Phash.size()], out.size() - m_Phash.size());
+   const size_t hlen = m_Phash.size();
+
+   mgf1_mask(*m_mgf1_hash, output.first(hlen), output.subspan(hlen));
 
-   mgf1_mask(*m_mgf1_hash, &out[m_Phash.size()], out.size() - m_Phash.size(), out.data(), m_Phash.size());
+   mgf1_mask(*m_mgf1_hash, output.subspan(hlen), output.first(hlen));
 
-   return out;
+   return key_length;
 }
 
 /*
 * OAEP Unpad Operation
 */
-secure_vector<uint8_t> OAEP::unpad(uint8_t& valid_mask, const uint8_t in[], size_t in_length) const {
+CT::Option<size_t> OAEP::unpad(std::span<uint8_t> output, std::span<const uint8_t> input) const {
+   BOTAN_ASSERT_NOMSG(output.size() >= input.size());
+
    /*
    Must be careful about error messages here; if an attacker can
    distinguish them, it is easy to use the differences as an oracle to
@@ -70,41 +77,41 @@ secure_vector<uint8_t> OAEP::unpad(uint8_t& valid_mask, const uint8_t in[], size
    Therefore, the first byte should always be zero.
    */
 
-   const auto leading_0 = CT::Mask<uint8_t>::is_zero(in[0]);
+   if(input.empty()) {
+      return {};
+   }
+
+   auto scope = CT::scoped_poison(input);
+
+   const auto has_leading_0 = CT::Mask<uint8_t>::is_zero(input[0]).as_choice();
 
-   secure_vector<uint8_t> input(in + 1, in + in_length);
+   secure_vector<uint8_t> decoded(input.begin() + 1, input.end());
+   auto buf = std::span{decoded};
 
    const size_t hlen = m_Phash.size();
 
-   mgf1_mask(*m_mgf1_hash, &input[hlen], input.size() - hlen, input.data(), hlen);
+   mgf1_mask(*m_mgf1_hash, buf.subspan(hlen), buf.first(hlen));
 
-   mgf1_mask(*m_mgf1_hash, input.data(), hlen, &input[hlen], input.size() - hlen);
+   mgf1_mask(*m_mgf1_hash, buf.first(hlen), buf.subspan(hlen));
 
-   auto unpadded = oaep_find_delim(valid_mask, input.data(), input.size(), m_Phash);
-   valid_mask &= leading_0.unpoisoned_value();
-   return unpadded;
-}
+   auto delim = oaep_find_delim(buf, m_Phash);
 
-secure_vector<uint8_t> oaep_find_delim(uint8_t& valid_mask,
-                                       const uint8_t input[],
-                                       size_t input_len,
-                                       const secure_vector<uint8_t>& Phash) {
-   const size_t hlen = Phash.size();
+   return CT::copy_output(delim.has_value() && has_leading_0, output, buf, delim.value_or(0));
+}
 
+CT::Option<size_t> oaep_find_delim(std::span<const uint8_t> input, std::span<const uint8_t> phash) {
    // Too short to be valid, reject immediately
-   if(input_len < 1 + 2 * hlen) {
-      return secure_vector<uint8_t>();
+   if(input.size() < 1 + 2 * phash.size()) {
+      return {};
    }
 
-   CT::poison(input, input_len);
-
-   size_t delim_idx = 2 * hlen;
+   size_t delim_idx = 2 * phash.size();
    CT::Mask<uint8_t> waiting_for_delim = CT::Mask<uint8_t>::set();
    CT::Mask<uint8_t> bad_input_m = CT::Mask<uint8_t>::cleared();
 
-   for(size_t i = delim_idx; i < input_len; ++i) {
-      const auto zero_m = CT::Mask<uint8_t>::is_zero(input[i]);
-      const auto one_m = CT::Mask<uint8_t>::is_equal(input[i], 1);
+   for(uint8_t ib : input.subspan(2 * phash.size())) {
+      const auto zero_m = CT::Mask<uint8_t>::is_zero(ib);
+      const auto one_m = CT::Mask<uint8_t>::is_equal(ib, 1);
 
       const auto add_m = waiting_for_delim & zero_m;
 
@@ -119,16 +126,13 @@ secure_vector<uint8_t> oaep_find_delim(uint8_t& valid_mask,
    bad_input_m |= waiting_for_delim;
 
    // If the P hash is wrong, then it's not valid
-   bad_input_m |= CT::is_not_equal(&input[hlen], Phash.data(), hlen);
+   bad_input_m |= CT::is_not_equal(&input[phash.size()], phash.data(), phash.size());
 
    delim_idx += 1;
 
-   valid_mask = (~bad_input_m).unpoisoned_value();
-   auto output = CT::copy_output(bad_input_m, input, input_len, delim_idx);
-
-   CT::unpoison(input, input_len);
+   const auto accept = !(bad_input_m.as_choice());
 
-   return output;
+   return CT::Option(delim_idx, accept);
 }
 
 /*