Repository: randombit/botan
Pull request #3701 (2023-09-15 07:34:39)

FIX: cSHAKE should not depend on the user calling start()
[Similar to the `MessageAuthenticationCode`](https://github.com/randombit/botan/pull/3689#discussion_r1326853886), XOFs provide a `start()` method that is optional when a user doesn't want to use a salt (or the XOF itself doesn't support one). Nevertheless, we need to make sure that preconditioning of the XOF's state is not dependent on the user calling `start()`.

The `XOF` base class is taking care of this, by keeping track whether `start()` was called before the first `update()` invocation. If not, it calls it with default values (i.e. "empty"). Also, this adds a test to make sure that `start()` is optional (similar to #3699).

Also, this adds a test that cSHAKE fails when neither `N` nor `S` is set. This introduces many `try {} catch {}` blocks in test_xof.cpp. It's beneficial to review the two commits separately.


Patch:
diff --git a/src/lib/xof/cshake_xof/cshake_xof.cpp b/src/lib/xof/cshake_xof/cshake_xof.cpp
index f63cb444a..873237ab0 100644
--- a/src/lib/xof/cshake_xof/cshake_xof.cpp
+++ b/src/lib/xof/cshake_xof/cshake_xof.cpp
@@ -29,7 +29,7 @@ cSHAKE_XOF::cSHAKE_XOF(size_t capacity, std::string_view function_name) :
                  std::vector<uint8_t>{cast_char_ptr_to_uint8(function_name.data()),
                                       cast_char_ptr_to_uint8(function_name.data()) + function_name.size()}) {}
 
-void cSHAKE_XOF::clear() {
+void cSHAKE_XOF::reset() {
    m_keccak.clear();
    m_output_generated = false;
 }
diff --git a/src/lib/xof/cshake_xof/cshake_xof.h b/src/lib/xof/cshake_xof/cshake_xof.h
index 57c7b5e9c..d32440d5c 100644
--- a/src/lib/xof/cshake_xof/cshake_xof.h
+++ b/src/lib/xof/cshake_xof/cshake_xof.h
@@ -36,8 +36,6 @@ class BOTAN_TEST_API cSHAKE_XOF : public XOF {
    public:
       std::string provider() const final;
 
-      void clear() final;
-
       bool valid_salt_length(size_t salt_length) const final;
 
       size_t block_size() const final;
@@ -55,6 +53,7 @@ class BOTAN_TEST_API cSHAKE_XOF : public XOF {
       void start_msg(std::span<const uint8_t> salt, std::span<const uint8_t> key) final;
       void add_data(std::span<const uint8_t> input) final;
       void generate_bytes(std::span<uint8_t> output) final;
+      void reset() final;
 
    private:
       Keccak_Permutation m_keccak;
diff --git a/src/lib/xof/shake_xof/shake_xof.cpp b/src/lib/xof/shake_xof/shake_xof.cpp
index 11c2821d8..c4e9dcba8 100644
--- a/src/lib/xof/shake_xof/shake_xof.cpp
+++ b/src/lib/xof/shake_xof/shake_xof.cpp
@@ -15,7 +15,7 @@ SHAKE_XOF::SHAKE_XOF(size_t capacity) : m_keccak(capacity, 0b1111, 4), m_output_
    BOTAN_ASSERT_NOMSG(capacity == 256 || capacity == 512);
 }
 
-void SHAKE_XOF::clear() {
+void SHAKE_XOF::reset() {
    m_keccak.clear();
    m_output_generated = false;
 }
diff --git a/src/lib/xof/shake_xof/shake_xof.h b/src/lib/xof/shake_xof/shake_xof.h
index 2b0c1a1fc..f25ca1189 100644
--- a/src/lib/xof/shake_xof/shake_xof.h
+++ b/src/lib/xof/shake_xof/shake_xof.h
@@ -32,8 +32,6 @@ class SHAKE_XOF : public XOF {
    public:
       std::string provider() const final { return m_keccak.provider(); }
 
-      void clear() final;
-
       size_t block_size() const final { return m_keccak.byte_rate(); }
 
       bool accepts_input() const final { return !m_output_generated; }
@@ -41,6 +39,7 @@ class SHAKE_XOF : public XOF {
    private:
       void add_data(std::span<const uint8_t> input) final;
       void generate_bytes(std::span<uint8_t> output) final;
+      void reset() final;
 
    private:
       Keccak_Permutation m_keccak;
diff --git a/src/lib/xof/xof.cpp b/src/lib/xof/xof.cpp
index db0edc9e0..5153e8080 100644
--- a/src/lib/xof/xof.cpp
+++ b/src/lib/xof/xof.cpp
@@ -64,6 +64,7 @@ void XOF::start(std::span<const uint8_t> salt, std::span<const uint8_t> key) {
       throw Invalid_Argument(fmt("{} cannot accept a salt length of {}", name(), salt.size()));
    }
 
+   m_xof_started = true;
    start_msg(salt, key);
 }
 
diff --git a/src/lib/xof/xof.h b/src/lib/xof/xof.h
index 3ab5adec9..b6a3bd6cc 100644
--- a/src/lib/xof/xof.h
+++ b/src/lib/xof/xof.h
@@ -28,6 +28,8 @@ namespace Botan {
  */
 class BOTAN_PUBLIC_API(3, 2) XOF {
    public:
+      XOF() : m_xof_started(false) {}
+
       virtual ~XOF() = default;
 
       /**
@@ -61,7 +63,10 @@ class BOTAN_PUBLIC_API(3, 2) XOF {
       /**
        * Reset the state.
        */
-      virtual void clear() = 0;
+      void clear() {
+         m_xof_started = false;
+         reset();
+      }
 
       /**
        * @return the hash function name
@@ -134,7 +139,14 @@ class BOTAN_PUBLIC_API(3, 2) XOF {
        *
        * @param input  the data that shall be
        */
-      void update(std::span<const uint8_t> input) { add_data(input); }
+      void update(std::span<const uint8_t> input) {
+         if(!m_xof_started) {
+            // If the user didn't start() before the first input, we enforce
+            // it with a default value, here.
+            start();
+         }
+         add_data(input);
+      }
 
       /**
        * @return the next @p bytes output bytes as the specified container type @p T.
@@ -203,6 +215,14 @@ class BOTAN_PUBLIC_API(3, 2) XOF {
        * @param output  the span to be filled entirely with output bytes
        */
       virtual void generate_bytes(std::span<uint8_t> output) = 0;
+
+      /**
+       * Clear the XOF's internal state and allow for new input.
+       */
+      virtual void reset() = 0;
+
+   private:
+      bool m_xof_started;
 };
 
 }  // namespace Botan


Test patch:
diff --git a/src/tests/data/xof/cshake.vec b/src/tests/data/xof/cshake.vec
index 4fcbed08f..c1a4b6659 100644
--- a/src/tests/data/xof/cshake.vec
+++ b/src/tests/data/xof/cshake.vec
@@ -20,6 +20,11 @@ Salt = 456d61696c205369676e6174757265
 In = 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F303132333435363738393A3B3C3D3E3F404142434445464748494A4B4C4D4E4F505152535455565758595A5B5C5D5E5F606162636465666768696A6B6C6D6E6F707172737475767778797A7B7C7D7E7F808182838485868788898A8B8C8D8E8F909192939495969798999A9B9C9D9E9FA0A1A2A3A4A5A6A7A8A9AAABACADAEAFB0B1B2B3B4B5B6B7B8B9BABBBCBDBEBFC0C1C2C3C4C5C6C7
 Out = C5221D50E4F822D96A2E8881A961420F294B7B24FE3D2094BAED2C6524CC166B
 
+# cSHAKE instantiation with a Name but no Salt should work
+Name = 426f74616e2d54657374
+In = 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F303132333435363738393A3B3C3D3E3F404142434445464748494A4B4C4D4E4F505152535455565758595A5B5C5D5E5F606162636465666768696A6B6C6D6E6F707172737475767778797A7B7C7D7E7F808182838485868788898A8B8C8D8E8F909192939495969798999A9B9C9D9E9FA0A1A2A3A4A5A6A7A8A9AAABACADAEAFB0B1B2B3B4B5B6B7B8B9BABBBCBDBEBFC0C1C2C3C4C5C6C7
+Out = 429F4B1FFD82BAAD7EB191BCFD1C9A89486C2C8D
+
 [cSHAKE-256]
 
 # Created with https://asecuritysite.com/golang/cs
@@ -37,3 +42,8 @@ Out = D008828E2B80AC9D2218FFEE1D070C48B8E4C87BFF32C9699D5B6896EEE0EDD164020E2BE0
 Salt = 456d61696c205369676e6174757265
 In = 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F303132333435363738393A3B3C3D3E3F404142434445464748494A4B4C4D4E4F505152535455565758595A5B5C5D5E5F606162636465666768696A6B6C6D6E6F707172737475767778797A7B7C7D7E7F808182838485868788898A8B8C8D8E8F909192939495969798999A9B9C9D9E9FA0A1A2A3A4A5A6A7A8A9AAABACADAEAFB0B1B2B3B4B5B6B7B8B9BABBBCBDBEBFC0C1C2C3C4C5C6C7
 Out = 07DC27B11E51FBAC75BC7B3C1D983E8B4B85FB1DEFAF218912AC86430273091727F42B17ED1DF63E8EC118F04B23633C1DFB1574C8FB55CB45DA8E25AFB092BB
+
+# cSHAKE instantiation with a Name but no Salt should work
+Name = 426f74616e2d54657374
+In = 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F303132333435363738393A3B3C3D3E3F404142434445464748494A4B4C4D4E4F505152535455565758595A5B5C5D5E5F606162636465666768696A6B6C6D6E6F707172737475767778797A7B7C7D7E7F808182838485868788898A8B8C8D8E8F909192939495969798999A9B9C9D9E9FA0A1A2A3A4A5A6A7A8A9AAABACADAEAFB0B1B2B3B4B5B6B7B8B9BABBBCBDBEBFC0C1C2C3C4C5C6C7
+Out = 8DAD539ABCD9E573D35D898F48AA615320DB0286
diff --git a/src/tests/test_xof.cpp b/src/tests/test_xof.cpp
index 5f4ac4b8a..f7aa56212 100644
--- a/src/tests/test_xof.cpp
+++ b/src/tests/test_xof.cpp
@@ -89,6 +89,13 @@ class XOF_Tests final : public Text_Based_Test {
             result.test_eq("generated output", xof->output_stdvec(expected.size()), expected);
             result.confirm("object does not accept input after first output", !xof->accepts_input());
 
+            // if not necessary, invoking start() should be optional
+            if(salt.empty() && key.empty()) {
+               xof->clear();
+               xof->update(in);
+               result.test_eq("generated output (w/o start())", xof->output_stdvec(expected.size()), expected);
+            }
+
             // input again and output bytewise
             xof->clear();
             result.test_eq("object might accept input after clear()", xof->accepts_input(), new_accepts_input);
@@ -154,6 +161,25 @@ class XOF_Tests final : public Text_Based_Test {
 
          return result;
       }
+
+      std::vector<Test::Result> run_final_tests() override {
+         return {
+   #if defined(BOTAN_HAS_CSHAKE_XOF)
+            Botan_Tests::CHECK("cSHAKE without a name", [](Test::Result& result) {
+               std::vector<std::unique_ptr<Botan::XOF>> cshakes;
+               cshakes.push_back(std::make_unique<Botan::cSHAKE_128_XOF>(""));
+               cshakes.push_back(std::make_unique<Botan::cSHAKE_256_XOF>(""));
+
+               for(auto& cshake : cshakes) {
+                  result.confirm("cSHAKE without a name rejects empty salt", !cshake->valid_salt_length(0));
+                  result.confirm("cSHAKE without a name requests at least one byte of salt",
+                                 cshake->valid_salt_length(1));
+                  result.test_throws("cSHAKE without a name throws without salt", [&]() { cshake->start({}); });
+               }
+            }),
+   #endif
+         };
+      }
 };
 
 BOTAN_REGISTER_SERIALIZED_TEST("xof", "extendable_output_functions", XOF_Tests);