Address remaining review comments

src/cli/speed.cpp

@@ -2049,18 +2049,26 @@ class Speed final : public Command {
 
 #if defined(BOTAN_HAS_FRODOKEM)
       void bench_frodokem(const std::string& provider, std::chrono::milliseconds msec) {
-         std::vector<Botan::FrodoKEMMode> frodo_modes {
-            Botan::FrodoKEMMode::FrodoKEM640_SHAKE, Botan::FrodoKEMMode::FrodoKEM976_SHAKE,
-               Botan::FrodoKEMMode::FrodoKEM1344_SHAKE, Botan::FrodoKEMMode::eFrodoKEM640_SHAKE,
-               Botan::FrodoKEMMode::eFrodoKEM976_SHAKE, Botan::FrodoKEMMode::eFrodoKEM1344_SHAKE,
-   #if defined(BOTAN_HAS_AES)
-               Botan::FrodoKEMMode::FrodoKEM640_AES, Botan::FrodoKEMMode::FrodoKEM976_AES,
-               Botan::FrodoKEMMode::FrodoKEM1344_AES, Botan::FrodoKEMMode::eFrodoKEM640_AES,
-               Botan::FrodoKEMMode::eFrodoKEM976_AES, Botan::FrodoKEMMode::eFrodoKEM1344_AES,
-   #endif
+         std::vector<Botan::FrodoKEMMode> frodo_modes{
+            Botan::FrodoKEMMode::FrodoKEM640_SHAKE,
+            Botan::FrodoKEMMode::FrodoKEM976_SHAKE,
+            Botan::FrodoKEMMode::FrodoKEM1344_SHAKE,
+            Botan::FrodoKEMMode::eFrodoKEM640_SHAKE,
+            Botan::FrodoKEMMode::eFrodoKEM976_SHAKE,
+            Botan::FrodoKEMMode::eFrodoKEM1344_SHAKE,
+            Botan::FrodoKEMMode::FrodoKEM640_AES,
+            Botan::FrodoKEMMode::FrodoKEM976_AES,
+            Botan::FrodoKEMMode::FrodoKEM1344_AES,
+            Botan::FrodoKEMMode::eFrodoKEM640_AES,
+            Botan::FrodoKEMMode::eFrodoKEM976_AES,
+            Botan::FrodoKEMMode::eFrodoKEM1344_AES,
          };
 
          for(auto modet : frodo_modes) {
+            if(!modet.is_available()) {
+               continue;
+            }
+
             Botan::FrodoKEMMode mode(modet);
 
             auto keygen_timer = make_timer(mode.to_string(), provider, "keygen");

src/lib/pubkey/frodokem/frodo_constants.cpp

@@ -1,7 +1,5 @@
 /*
  * FrodoKEM modes and constants
- * Based on the MIT licensed reference implementation by the designers
- * (https://github.com/microsoft/PQCrypto-LWEKE/tree/master/src)
  *
  * The Fellowship of the FrodoKEM:
  * (C) 2023 Jack Lloyd
@@ -17,9 +15,7 @@
 namespace Botan {
 
 FrodoKEMConstants::FrodoKEMConstants(FrodoKEMMode mode) : m_mode(mode), m_len_a(128), m_n_bar(8) {
-#if !defined(BOTAN_HAS_FRODOKEM_AES)
-   BOTAN_ARG_CHECK(!mode.is_aes(), "cannot instantiate AES-based FrodoKEM: This build does not support AES");
-#endif
+   BOTAN_ASSERT(m_mode.is_available(), "Mode is not available.");
 
    if(mode.is_ephemeral()) {
       m_len_salt = 0;

src/lib/pubkey/frodokem/frodo_constants.h

@@ -1,7 +1,5 @@
 /*
  * FrodoKEM constants
- * Based on the MIT licensed reference implementation by the designers
- * (https://github.com/microsoft/PQCrypto-LWEKE/tree/master/src)
  *
  * The Fellowship of the FrodoKEM:
  * (C) 2023 Jack Lloyd

src/lib/pubkey/frodokem/frodo_matrix.cpp

@@ -17,7 +17,6 @@
 #include <botan/hex.h>
 #include <botan/mem_ops.h>
 #include <botan/internal/bit_ops.h>
-#include <botan/internal/ct_utils.h>
 #include <botan/internal/frodo_constants.h>
 #include <botan/internal/loadstor.h>
 #include <botan/internal/shake_xof.h>
@@ -101,6 +100,15 @@ FrodoMatrix FrodoMatrix::sample(const FrodoKEMConstants& constants,
    return FrodoMatrix(dimensions, std::move(elements));
 }
 
+std::function<FrodoMatrix(const FrodoMatrix::Dimensions& dimensions)> FrodoMatrix::make_sample_generator(
+   const FrodoKEMConstants& constants, Botan::XOF& shake) {
+   return [&constants, &shake](const FrodoMatrix::Dimensions& dimensions) mutable {
+      return sample(constants,
+                    dimensions,
+                    shake.output<FrodoSampleR>(sizeof(uint16_t) * std::get<0>(dimensions) * std::get<1>(dimensions)));
+   };
+}
+
 FrodoMatrix::FrodoMatrix(Dimensions dims) :
       m_dim1(std::get<0>(dims)), m_dim2(std::get<1>(dims)), m_elements(make_elements_vector(dims)) {}
 
@@ -303,13 +311,12 @@ FrodoMatrix FrodoMatrix::sub(const FrodoKEMConstants& constants, const FrodoMatr
    return FrodoMatrix(a.dimensions(), std::move(elements));
 }
 
-bool FrodoMatrix::constant_time_compare(const FrodoMatrix& other) const {
+CT::Mask<uint8_t> FrodoMatrix::constant_time_compare(const FrodoMatrix& other) const {
    BOTAN_ASSERT_NOMSG(dimensions() == other.dimensions());
-
-   // TODO: use range-based comparison after #3715 is merged
-   return Botan::constant_time_compare(reinterpret_cast<const uint8_t*>(m_elements.data()),
-                                       reinterpret_cast<const uint8_t*>(other.m_elements.data()),
-                                       sizeof(decltype(m_elements)::value_type) * m_elements.size());
+   // TODO: Possibly use range-based comparison after #3715 is merged
+   return CT::is_equal(reinterpret_cast<const uint8_t*>(m_elements.data()),
+                       reinterpret_cast<const uint8_t*>(other.m_elements.data()),
+                       sizeof(decltype(m_elements)::value_type) * m_elements.size());
 }
 
 FrodoMatrix FrodoMatrix::mul_bs(const FrodoKEMConstants& constants, const FrodoMatrix& b, const FrodoMatrix& s) {

src/lib/pubkey/frodokem/frodo_matrix.h

@@ -13,6 +13,7 @@
 #ifndef BOTAN_FRODOKEM_MATRIX_H_
 #define BOTAN_FRODOKEM_MATRIX_H_
 
+#include <botan/internal/ct_utils.h>
 #include <botan/internal/frodo_constants.h>
 #include <botan/internal/frodo_types.h>
 
@@ -71,6 +72,11 @@ class FrodoMatrix {
                                 const Dimensions& dimensions,
                                 StrongSpan<const FrodoSampleR> r);
 
+      // Helper function that calls FrodoMatrix::sample on initially provided consts and shake XOF.
+      // The output function calls shake.output at each invocation.
+      static std::function<FrodoMatrix(const Dimensions& dimensions)> make_sample_generator(
+         const FrodoKEMConstants& constants, Botan::XOF& shake);
+
       // Generate-and-multiply: generate matrix A (N x N) row-wise, multiply by s on the right.
       // Inputs: s^T (N_BAR x N), e (N x N_BAR), seed for matrix A
       // Output: The elements of the FrodoMatrix will correspond to A*s + e (N x N_BAR).
@@ -114,7 +120,7 @@ class FrodoMatrix {
 
       Dimensions dimensions() const { return {m_dim1, m_dim2}; }
 
-      bool constant_time_compare(const FrodoMatrix& other) const;
+      CT::Mask<uint8_t> constant_time_compare(const FrodoMatrix& other) const;
 
       size_t element_count() const { return m_elements.size(); }
 

src/lib/pubkey/frodokem/frodo_mode.cpp

@@ -1,7 +1,5 @@
 /*
  * FrodoKEM modes and constants
- * Based on the MIT licensed reference implementation by the designers
- * (https://github.com/microsoft/PQCrypto-LWEKE/tree/master/src)
  *
  * The Fellowship of the FrodoKEM:
  * (C) 2023 Jack Lloyd

src/lib/pubkey/frodokem/frodo_mode.h

@@ -1,7 +1,5 @@
 /*
  * FrodoKEM modes and constants
- * Based on the MIT licensed reference implementation by the designers
- * (https://github.com/microsoft/PQCrypto-LWEKE/tree/master/src)
  *
  * The Fellowship of the FrodoKEM:
  * (C) 2023 Jack Lloyd
@@ -65,6 +63,14 @@ class BOTAN_PUBLIC_API(3, 3) FrodoKEMMode {
                 m_mode == FrodoKEM640_AES || m_mode == FrodoKEM976_AES || m_mode == FrodoKEM1344_AES;
       }
 
+      bool is_available() const {
+         return
+#if defined(BOTAN_HAS_FRODOKEM_AES)
+            is_aes() ||
+#endif
+            is_shake();
+      }
+
       bool operator==(const FrodoKEMMode& other) const { return m_mode == other.m_mode; }
 
       bool operator!=(const FrodoKEMMode& other) const { return !(*this == other); }

src/lib/pubkey/frodokem/frodokem.cpp

@@ -91,6 +91,7 @@ class Frodo_KEM_Encryptor final : public PK_Ops::KEM_Encryption_with_KDF {
                            RandomNumberGenerator& rng) override {
          const auto& consts = m_public_key->constants();
          auto& shake = consts.SHAKE_XOF();
+         auto sample_generator = FrodoMatrix::make_sample_generator(consts, shake);
 
          BufferStuffer out_ct_bs(out_encapsulated_key);
 
@@ -113,24 +114,15 @@ class Frodo_KEM_Encryptor final : public PK_Ops::KEM_Encryption_with_KDF {
          shake.update(consts.encapsulation_domain_separator());
          shake.update(seed_se);
 
-         const auto s_p =
-            FrodoMatrix::sample(consts,
-                                std::tuple(consts.n_bar(), consts.n()),
-                                shake.output<FrodoSampleR>(sizeof(uint16_t) * consts.n_bar() * consts.n()));
+         const auto s_p = sample_generator(std::tuple(consts.n_bar(), consts.n()));
 
-         const auto e_p =
-            FrodoMatrix::sample(consts,
-                                std::tuple(consts.n_bar(), consts.n()),
-                                shake.output<FrodoSampleR>(sizeof(uint16_t) * consts.n_bar() * consts.n()));
+         const auto e_p = sample_generator(std::tuple(consts.n_bar(), consts.n()));
 
          const auto b_p = FrodoMatrix::mul_add_sa_plus_e(consts, s_p, e_p, m_public_key->seed_a());
 
          b_p.pack(consts, c_1);
 
-         const auto e_pp =
-            FrodoMatrix::sample(consts,
-                                std::tuple(consts.n_bar(), consts.n_bar()),
-                                shake.output<FrodoSampleR>(sizeof(uint16_t) * consts.n_bar() * consts.n_bar()));
+         const auto e_pp = sample_generator(std::tuple(consts.n_bar(), consts.n_bar()));
          shake.clear();
 
          const auto v = FrodoMatrix::mul_add_sb_plus_e(consts, m_public_key->b(), s_p, e_pp);
@@ -164,6 +156,7 @@ class Frodo_KEM_Decryptor final : public PK_Ops::KEM_Decryption_with_KDF {
       void raw_kem_decrypt(std::span<uint8_t> out_shared_key, std::span<const uint8_t> encapsulated_key) override {
          const auto& consts = m_public_key->constants();
          auto& shake = consts.SHAKE_XOF();
+         auto sample_generator = FrodoMatrix::make_sample_generator(consts, shake);
 
          if(encapsulated_key.size() != consts.len_ct_bytes()) {
             throw Invalid_Argument("FrodoKEM ciphertext does not have the correct byte count");
@@ -193,45 +186,33 @@ class Frodo_KEM_Decryptor final : public PK_Ops::KEM_Decryption_with_KDF {
 
          shake.update(consts.encapsulation_domain_separator());
          shake.update(seed_se_p);
-         const auto s_p =
-            FrodoMatrix::sample(consts,
-                                std::tuple(consts.n_bar(), consts.n()),
-                                shake.output<FrodoSampleR>(sizeof(uint16_t) * consts.n_bar() * consts.n()));
+         const auto s_p = sample_generator(std::tuple(consts.n_bar(), consts.n()));
 
-         const auto e_p =
-            FrodoMatrix::sample(consts,
-                                std::tuple(consts.n_bar(), consts.n()),
-                                shake.output<FrodoSampleR>(sizeof(uint16_t) * consts.n_bar() * consts.n()));
+         const auto e_p = sample_generator(std::tuple(consts.n_bar(), consts.n()));
 
          auto b_pp = FrodoMatrix::mul_add_sa_plus_e(consts, s_p, e_p, m_public_key->seed_a());
 
-         const auto e_pp =
-            FrodoMatrix::sample(consts,
-                                std::tuple(consts.n_bar(), consts.n_bar()),
-                                shake.output<FrodoSampleR>(sizeof(uint16_t) * consts.n_bar() * consts.n_bar()));
+         const auto e_pp = sample_generator(std::tuple(consts.n_bar(), consts.n_bar()));
          shake.clear();
 
          const auto v = FrodoMatrix::mul_add_sb_plus_e(consts, m_public_key->b(), s_p, e_pp);
 
          const auto encoded = FrodoMatrix::encode(consts, seed_u_p);
          auto c_p = FrodoMatrix::add(consts, v, encoded);
 
-         // The spec concats the matrices b_p and c (b_pp and c_p respectively)
-         // and performs a single CT comparison. For convenience we compare the
-         // matrices individually in CT and CT-&& the result.
-         //
-         // Note: b_p and c are unpacked values that are reduced by definition.
-         //       b_pp and c_p are calculated values that need the reduction for
-         //       an unambiguous comparison.
+         // b_p and c are unpacked values that are reduced by definition.
+         // b_pp and c_p are calculated values that need the reduction for
+         // an unambiguous comparison that is required next.
          b_pp.reduce(consts);
          c_p.reduce(consts);
-         const bool cmp_b_p_b_pp = b_p.constant_time_compare(b_pp);
-         const bool cmp_c_c_p = c.constant_time_compare(c_p);
-         const auto cmp = CT::Mask<uint8_t>::expand(cmp_b_p_b_pp) & CT::Mask<uint8_t>::expand(cmp_c_c_p);
+
+         // The spec concats the matrices b_p and c (b_pp and c_p respectively)
+         // and performs a single CT comparison. For convenience we compare the
+         // matrices individually in CT and CT-&& the resulting masks.
+         const auto cmp = b_p.constant_time_compare(b_pp) & c.constant_time_compare(c_p);
 
          std::vector<uint8_t> k_bar(consts.len_sec_bytes(), 0);
-         CT::conditional_copy_mem(
-            cmp.value(), k_bar.data(), k_p.data(), m_private_key->s().data(), consts.len_sec_bytes());
+         CT::conditional_copy_mem(cmp, k_bar.data(), k_p.data(), m_private_key->s().data(), consts.len_sec_bytes());
 
          shake.update(encapsulated_key);
          shake.update(k_bar);
@@ -284,7 +265,7 @@ AlgorithmIdentifier FrodoKEM_PublicKey::algorithm_identifier() const {
 }
 
 OID FrodoKEM_PublicKey::object_identifier() const {
-   return OID::from_string(m_public->constants().mode().to_string());
+   return m_public->constants().mode().object_identifier();
 }
 
 size_t FrodoKEM_PublicKey::key_length() const {
@@ -322,6 +303,7 @@ std::unique_ptr<PK_Ops::KEM_Encryption> FrodoKEM_PublicKey::create_kem_encryptio
 FrodoKEM_PrivateKey::FrodoKEM_PrivateKey(RandomNumberGenerator& rng, FrodoKEMMode mode) {
    FrodoKEMConstants consts(mode);
    auto& shake = consts.SHAKE_XOF();
+   auto sample_generator = FrodoMatrix::make_sample_generator(consts, shake);
 
    auto s = rng.random_vec<FrodoSeedS>(consts.len_sec_bytes());
    const auto seed_se = rng.random_vec<FrodoSeedSE>(consts.len_se_bytes());
@@ -333,12 +315,9 @@ FrodoKEM_PrivateKey::FrodoKEM_PrivateKey(RandomNumberGenerator& rng, FrodoKEMMod
 
    shake.update(consts.keygen_domain_separator());
    shake.update(seed_se);
-   const auto r_bytes_for_s_trans = sizeof(uint16_t) * consts.n() * consts.n_bar();
-   const auto r_bytes_for_e = sizeof(uint16_t) * consts.n() * consts.n_bar();
 
-   auto s_trans =
-      FrodoMatrix::sample(consts, {consts.n_bar(), consts.n()}, shake.output<FrodoSampleR>(r_bytes_for_s_trans));
-   auto e = FrodoMatrix::sample(consts, {consts.n(), consts.n_bar()}, shake.output<FrodoSampleR>(r_bytes_for_e));
+   auto s_trans = sample_generator(std::tuple(consts.n_bar(), consts.n()));
+   auto e = sample_generator(std::tuple(consts.n(), consts.n_bar()));
 
    auto b = FrodoMatrix::mul_add_as_plus_e(consts, s_trans, e, seed_a);
 

src/lib/tls/tls_algos.h

@@ -121,19 +121,23 @@ enum class Group_Params_Code : uint16_t {
    // https://datatracker.ietf.org/doc/draft-tls-westerbaan-xyber768d00/03/
    HYBRID_X25519_KYBER_512_R3_OQS = 0x2F39,
    HYBRID_X25519_KYBER_768_R3_OQS = 0x6399,
-   HYBRID_X25519_eFRODOKEM_640_SHAKE_OQS = 0x2F81,
-   HYBRID_X25519_eFRODOKEM_640_AES_OQS = 0x2F80,
 
    HYBRID_SECP256R1_KYBER_512_R3_OQS = 0x2F3A,
    HYBRID_SECP256R1_KYBER_768_R3_OQS = 0x639A,
+
+   HYBRID_SECP384R1_KYBER_768_R3_OQS = 0x2F3C,
+
+   HYBRID_SECP521R1_KYBER_1024_R3_OQS = 0x2F3D,
+
+   HYBRID_X25519_eFRODOKEM_640_SHAKE_OQS = 0x2F81,
+   HYBRID_X25519_eFRODOKEM_640_AES_OQS = 0x2F80,
+
    HYBRID_SECP256R1_eFRODOKEM_640_SHAKE_OQS = 0x2F01,
    HYBRID_SECP256R1_eFRODOKEM_640_AES_OQS = 0x2F00,
 
-   HYBRID_SECP384R1_KYBER_768_R3_OQS = 0x2F3C,
    HYBRID_SECP384R1_eFRODOKEM_976_SHAKE_OQS = 0x2F03,
    HYBRID_SECP384R1_eFRODOKEM_976_AES_OQS = 0x2F02,
 
-   HYBRID_SECP521R1_KYBER_1024_R3_OQS = 0x2F3D,
    HYBRID_SECP521R1_eFRODOKEM_1344_SHAKE_OQS = 0x2F05,
    HYBRID_SECP521R1_eFRODOKEM_1344_AES_OQS = 0x2F04,
 };