Validate recovery snapshot endorsement chains in memory

CHANGELOG.md

@@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 [7.0.10]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.10
 
+### Added
+
+- Recovery can now use a COSE snapshot signed by an earlier service identity after one or more disaster recoveries. Before deserialising the snapshot, the node reads previous-service-identity endorsement candidates from the public ledger suffix, validates a complete chain against the operator-provided identity, and retains it only for the current recovery attempt. Invalid or incomplete endorsement chains fall back to full-ledger replay (#8092).
+
 ### Changed
 
 - `ccf::http::ParsedQuery` (in `include/ccf/http_query.h`), returned by `ccf::http::parse_query()`, is now a `std::multimap<std::string, std::string, std::less<>>` that owns its decoded keys and values, rather than a `std::multimap<std::string_view, std::string_view>` pointing into the source query string. Owned storage is required because each key and value is now URL-decoded individually after splitting, which produces bytes not present in the original query. Application code that consumed the previous `std::string_view` keys/values may need to be updated (#8024).

CMakeLists.txt

@@ -1109,6 +1109,12 @@ if(BUILD_TESTS)
     ADDITIONAL_ARGS --regex ^recovery_intermediate_snapshot_join$
   )
 
+  add_e2e_test(
+    NAME recovery_snapshot_endorsements_test
+    PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/recovery_snapshot_endorsements.py
+    BUCKET bucket_b
+  )
+
   add_e2e_test(
     NAME recovery_test_suite
     PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/e2e_suite.py

src/kv/generic_serialise_wrapper.h

@@ -347,7 +347,14 @@ namespace ccf::kv
       }
 
       serialized::skip(data_, size_, crypto_util->get_header_length());
-      auto public_domain_length = serialized::read<size_t>(data_, size_);
+      const auto public_domain_length = serialized::read<size_t>(data_, size_);
+      if (public_domain_length > size_)
+      {
+        throw std::logic_error(fmt::format(
+          "Public domain length {} exceeds remaining entry size {}",
+          public_domain_length,
+          size_));
+      }
 
       const auto* data_public = data_;
       public_reader.init(data_public, public_domain_length);

src/kv/raw_serialise.h

@@ -6,7 +6,9 @@
 #include "generic_serialise_wrapper.h"
 
 #include <array>
+#include <limits>
 #include <small_vector/SmallVector.h>
+#include <tuple>
 #include <type_traits>
 
 namespace ccf::kv
@@ -126,53 +128,100 @@ namespace ccf::kv
 
   class RawReader
   {
-  public:
-    const uint8_t* data_ptr;
+  private:
+    [[nodiscard]] size_t remaining_bytes() const
+    {
+      if (data_offset > data_size)
+      {
+        throw std::logic_error(fmt::format(
+          "Raw reader offset {} exceeds data size {}", data_offset, data_size));
+      }
+      return data_size - data_offset;
+    }
+
+    void require_bytes(size_t required, const char* description) const
+    {
+      const auto remaining = remaining_bytes();
+      if (required > remaining)
+      {
+        throw std::runtime_error(fmt::format(
+          "Expected {} bytes for {}, found only {}",
+          required,
+          description,
+          remaining));
+      }
+    }
+
+    [[nodiscard]] const uint8_t* current_data() const
+    {
+      if (data_ptr == nullptr)
+      {
+        if (data_size != 0)
+        {
+          throw std::logic_error("Raw reader has non-zero size with null data");
+        }
+        return nullptr;
+      }
+      return data_ptr + data_offset;
+    }
+
+    void advance(size_t size)
+    {
+      require_bytes(size, "reader advance");
+      data_offset += size;
+    }
+
+    const uint8_t* data_ptr{nullptr};
     size_t data_offset{0};
-    size_t data_size;
+    size_t data_size{0};
 
+  public:
     /** Reads the next entry, advancing data_offset
      */
     template <typename T>
     T read_entry()
     {
-      auto remainder = data_size - data_offset;
-      const auto* data = data_ptr + data_offset;
+      require_bytes(sizeof(T), "fixed-size entry");
+      const auto before = remaining_bytes();
+      auto remainder = before;
+      const auto* data = current_data();
       const auto entry = serialized::read<T>(data, remainder);
-      const auto bytes_read = data_size - data_offset - remainder;
-      data_offset += bytes_read;
+      if (remainder > before)
+      {
+        throw std::logic_error("Raw reader remaining size increased");
+      }
+      advance(before - remainder);
       return entry;
     }
 
     /** Reads the next size-prefixed entry
      */
     size_t read_size_prefixed_entry(size_t& start_offset)
     {
-      auto remainder = data_size - data_offset;
-      auto entry_size = read_entry<size_t>();
-
-      if (remainder < entry_size)
-      {
-        throw std::runtime_error(fmt::format(
-          "Expected {} byte entry, found only {}", entry_size, remainder));
-      }
+      const auto entry_size = read_entry<size_t>();
+      require_bytes(entry_size, "size-prefixed entry");
 
       start_offset = data_offset;
-      data_offset += entry_size;
+      advance(entry_size);
 
       return entry_size;
     }
 
     RawReader(const RawReader& other) = delete;
     RawReader& operator=(const RawReader& other) = delete;
 
-    RawReader(const uint8_t* data_in_ptr = nullptr, size_t data_in_size = 0) :
-      data_ptr(data_in_ptr),
-      data_size(data_in_size)
-    {}
+    RawReader(const uint8_t* data_in_ptr = nullptr, size_t data_in_size = 0)
+    {
+      init(data_in_ptr, data_in_size);
+    }
 
     void init(const uint8_t* data_in_ptr, size_t data_in_size)
     {
+      if (data_in_ptr == nullptr && data_in_size != 0)
+      {
+        throw std::invalid_argument(
+          "Cannot initialise raw reader with null data and non-zero size");
+      }
       data_offset = 0;
       data_ptr = data_in_ptr;
       data_size = data_in_size;
@@ -186,9 +235,30 @@ namespace ccf::kv
         std::is_same_v<T, ccf::kv::serialisers::SerialisedEntry>)
       {
         size_t entry_offset = 0;
-        size_t entry_size = read_size_prefixed_entry(entry_offset);
+        const auto entry_size = read_size_prefixed_entry(entry_offset);
+        using Element = typename T::value_type;
+        if (entry_size % sizeof(Element) != 0)
+        {
+          throw std::runtime_error(fmt::format(
+            "Size-prefixed entry of {} bytes is not divisible by element size "
+            "{}",
+            entry_size,
+            sizeof(Element)));
+        }
 
-        T ret(entry_size / sizeof(typename T::value_type));
+        T ret;
+        const auto element_count = entry_size / sizeof(Element);
+        if (element_count > ret.max_size())
+        {
+          throw std::length_error(fmt::format(
+            "Size-prefixed entry contains too many elements ({})",
+            element_count));
+        }
+        ret.resize(element_count);
+        if (entry_size == 0)
+        {
+          return ret;
+        }
         auto* data_dest = reinterpret_cast<uint8_t*>(ret.data());
         auto capacity = entry_size;
         // NOLINTNEXTLINE(readability-suspicious-call-argument)
@@ -201,10 +271,15 @@ namespace ccf::kv
       {
         T ret{};
         auto* data_ = reinterpret_cast<uint8_t*>(ret.data());
-        constexpr size_t size = ret.size() * sizeof(typename T::value_type);
+        constexpr auto element_count = std::tuple_size_v<T>;
+        static_assert(
+          element_count <=
+          std::numeric_limits<size_t>::max() / sizeof(typename T::value_type));
+        constexpr size_t size = element_count * sizeof(typename T::value_type);
+        require_bytes(size, "fixed-size array");
         auto size_ = size;
-        serialized::write(data_, size_, data_ptr + data_offset, size);
-        data_offset += size;
+        serialized::write(data_, size_, current_data(), size);
+        advance(size);
 
         return ret;
       }
@@ -240,7 +315,7 @@ namespace ccf::kv
 
     [[nodiscard]] bool is_eos() const
     {
-      return data_offset >= data_size;
+      return remaining_bytes() == 0;
     }
   };
 

src/kv/test/kv_serialisation.cpp

@@ -2,12 +2,16 @@
 // Licensed under the Apache 2.0 License.
 #include "ds/internal_logger.h"
 #include "kv/kv_serialiser.h"
+#include "kv/raw_serialise.h"
 #include "kv/store.h"
 #include "kv/test/null_encryptor.h"
 #include "kv/test/stub_consensus.h"
 
 #include <doctest/doctest.h>
 #undef FAIL
+#include <array>
+#include <cstring>
+#include <limits>
 #include <string>
 #include <vector>
 
@@ -19,6 +23,138 @@ struct MapTypes
   using StringNum = ccf::kv::Map<std::string, size_t>;
 };
 
+static std::vector<uint8_t> make_size_prefixed_bytes(
+  size_t declared_size, size_t actual_size)
+{
+  std::vector<uint8_t> bytes(sizeof(size_t) + actual_size);
+  std::memcpy(bytes.data(), &declared_size, sizeof(declared_size));
+  return bytes;
+}
+
+TEST_CASE(
+  "Raw reader rejects truncated entries" * doctest::test_suite("serialisation"))
+{
+  SUBCASE("Fixed-size integral")
+  {
+    std::vector<uint8_t> bytes(sizeof(uint64_t) - 1);
+    ccf::kv::RawReader reader(bytes.data(), bytes.size());
+    REQUIRE_THROWS(reader.read_next<uint64_t>());
+  }
+
+  SUBCASE("Fixed-size array")
+  {
+    std::vector<uint8_t> bytes(ccf::crypto::Sha256Hash::SIZE - 1);
+    ccf::kv::RawReader reader(bytes.data(), bytes.size());
+    REQUIRE_THROWS(reader.read_next<ccf::crypto::Sha256Hash::Representation>());
+  }
+
+  SUBCASE("Short size prefix")
+  {
+    std::vector<uint8_t> bytes(sizeof(size_t) - 1);
+    ccf::kv::RawReader reader(bytes.data(), bytes.size());
+    REQUIRE_THROWS(reader.read_next<std::vector<uint8_t>>());
+  }
+
+  SUBCASE("Prefix without payload")
+  {
+    auto bytes = make_size_prefixed_bytes(1, 0);
+    ccf::kv::RawReader reader(bytes.data(), bytes.size());
+    REQUIRE_THROWS(reader.read_next<std::vector<uint8_t>>());
+  }
+
+  SUBCASE("Nine bytes remaining declare two payload bytes")
+  {
+    auto bytes = make_size_prefixed_bytes(2, 1);
+    ccf::kv::RawReader reader(bytes.data(), bytes.size());
+    REQUIRE_THROWS(reader.read_next<std::vector<uint8_t>>());
+  }
+
+  SUBCASE("Impossible payload length")
+  {
+    auto bytes =
+      make_size_prefixed_bytes(std::numeric_limits<size_t>::max(), 0);
+    ccf::kv::RawReader reader(bytes.data(), bytes.size());
+    REQUIRE_THROWS(reader.read_next<std::vector<uint8_t>>());
+  }
+
+  SUBCASE("Payload is not a whole number of elements")
+  {
+    auto bytes = make_size_prefixed_bytes(1, 1);
+    ccf::kv::RawReader reader(bytes.data(), bytes.size());
+    REQUIRE_THROWS(reader.read_next<std::vector<uint64_t>>());
+  }
+
+  SUBCASE("Null data with non-zero size")
+  {
+    REQUIRE_THROWS(ccf::kv::RawReader(nullptr, 1));
+  }
+}
+
+static std::vector<uint8_t> make_public_domain_entry(
+  size_t declared_public_domain_size, const std::vector<uint8_t>& public_domain)
+{
+  ccf::kv::SerialisedEntryHeader header;
+  header.set_size(sizeof(size_t) + public_domain.size());
+  std::vector<uint8_t> entry(sizeof(header) + header.size);
+  auto* data = entry.data();
+  auto remaining = entry.size();
+  serialized::write(data, remaining, header);
+  serialized::write(data, remaining, declared_public_domain_size);
+  serialized::write(
+    data, remaining, public_domain.data(), public_domain.size());
+  return entry;
+}
+
+TEST_CASE(
+  "KV deserialiser rejects invalid public domains" *
+  doctest::test_suite("serialisation"))
+{
+  auto encryptor = std::make_shared<ccf::kv::NullTxEncryptor>();
+
+  const auto initialise = [&](const std::vector<uint8_t>& entry) {
+    ccf::kv::RawKvStoreDeserialiser deserialiser(
+      encryptor, ccf::kv::SecurityDomain::PUBLIC);
+    ccf::kv::Term term = 0;
+    ccf::kv::EntryFlags flags = {};
+    return deserialiser.init(entry.data(), entry.size(), term, flags, false);
+  };
+
+  SUBCASE("Public domain exceeds remaining entry")
+  {
+    const auto entry = make_public_domain_entry(2, {0});
+    REQUIRE_THROWS(initialise(entry));
+  }
+
+  SUBCASE("Public domain length prefix is truncated")
+  {
+    ccf::kv::SerialisedEntryHeader header;
+    header.set_size(sizeof(size_t) - 1);
+    std::vector<uint8_t> entry(sizeof(header) + header.size);
+    std::memcpy(entry.data(), &header, sizeof(header));
+    REQUIRE_THROWS(initialise(entry));
+  }
+
+  SUBCASE("Public domain length is impossible")
+  {
+    const auto entry =
+      make_public_domain_entry(std::numeric_limits<size_t>::max(), {});
+    REQUIRE_THROWS(initialise(entry));
+  }
+
+  SUBCASE("Claims digest is truncated")
+  {
+    std::vector<uint8_t> public_domain(
+      sizeof(ccf::kv::EntryType) + sizeof(ccf::kv::Version));
+    auto* data = public_domain.data();
+    *data++ = static_cast<uint8_t>(ccf::kv::EntryType::WriteSetWithClaims);
+    const ccf::kv::Version version = 1;
+    std::memcpy(data, &version, sizeof(version));
+    const auto entry =
+      make_public_domain_entry(public_domain.size(), public_domain);
+    REQUIRE_THROWS(initialise(entry));
+  }
+}
+
 TEST_CASE(
   "Serialise/deserialise public map only" *
   doctest::test_suite("serialisation"))