On recovery, set UVM descriptor SVN to minimum of existing KV value and startup endorsements (#7717)

Copilot · achamayou · web-flow · commit 5a41a9519f2a · 2026-03-06T19:20:19.000Z
Co-authored-by: copilot-swe-agent[bot] &lt;198982749+Copilot@users.noreply.github.com&gt;
Co-authored-by: achamayou &lt;4016369+achamayou@users.noreply.github.com&gt;
Co-authored-by: Amaury Chamayou &lt;amchamay@microsoft.com&gt;
Co-authored-by: Amaury Chamayou &lt;amaury@xargs.fr&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Changed
 
+- On recovery, the UVM descriptor SVN is now set to the minimum of the previously stored value in the KV and the value found in the new node's startup endorsements. On start, the behaviour is unchanged (#7716).
 - Refactored the user facing surface of self-healing-open and local sealing. The whole feature is now `sealing-recovery` with `self-healing-open` now referred to as the `recovery-decision-protocol`. (#7679)
   - Local sealing is enabled by setting the `sealing-recovery` config field (for both the sealing node, and the unsealing recovery node)
   - The local sealing identity is under `sealing-recovery.location.name`
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -819,6 +819,13 @@ if(BUILD_TESTS)
               ccf_tasks
     )
 
+    add_unit_test(
+      internal_tables_access_test
+      ${CMAKE_CURRENT_SOURCE_DIR}/src/node/rpc/test/internal_tables_access_test.cpp
+      ${CCF_DIR}/src/node/uvm_endorsements.cpp
+    )
+    target_link_libraries(internal_tables_access_test PRIVATE ccfcrypto ccf_kv)
+
     add_unit_test(
       merkle_test ${CMAKE_CURRENT_SOURCE_DIR}/src/node/test/merkle_test.cpp
     )
diff --git a/src/node/rpc/node_frontend.h b/src/node/rpc/node_frontend.h
@@ -1615,7 +1615,7 @@ namespace ccf
               ctx.tx, host_data, in.snp_security_policy);
 
             InternalTablesAccess::trust_node_uvm_endorsements(
-              ctx.tx, in.snp_uvm_endorsements);
+              ctx.tx, in.snp_uvm_endorsements, recovering);
 
             auto attestation =
               AttestationProvider::get_snp_attestation(in.quote_info).value();
diff --git a/src/node/rpc/test/internal_tables_access_test.cpp b/src/node/rpc/test/internal_tables_access_test.cpp
@@ -0,0 +1,243 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the Apache 2.0 License.
+
+#include "ccf/app_interface.h"
+#include "ccf/service/tables/host_data.h"
+#include "ccf/service/tables/service.h"
+#include "service/tables/config.h"
+#include "service/tables/signatures.h"
+
+#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
+
+#include "kv/store.h"
+#include "kv/test/null_encryptor.h"
+#include "service/internal_tables_access.h"
+
+#include <doctest/doctest.h>
+
+using namespace ccf;
+
+TEST_CASE("trust_node_uvm_endorsements - not recovering, empty map")
+{
+  ccf::kv::Store kv_store;
+  auto encryptor = std::make_shared<ccf::kv::NullTxEncryptor>();
+  kv_store.set_encryptor(encryptor);
+
+  SNPUVMEndorsements table(Tables::NODE_SNP_UVM_ENDORSEMENTS);
+
+  pal::UVMEndorsements endorsement{"did:x509:test", "test-feed", "42"};
+
+  {
+    auto tx = kv_store.create_tx();
+    InternalTablesAccess::trust_node_uvm_endorsements(
+      tx, endorsement, false /* recovering */);
+    REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS);
+  }
+
+  {
+    auto tx = kv_store.create_read_only_tx();
+    auto handle = tx.ro(table);
+    auto result = handle->get("did:x509:test");
+    REQUIRE(result.has_value());
+    REQUIRE(result->size() == 1);
+    auto it = result->find("test-feed");
+    REQUIRE(it != result->end());
+    REQUIRE(it->second.svn == "42");
+  }
+}
+
+TEST_CASE("trust_node_uvm_endorsements - recovering, new DID not in map")
+{
+  ccf::kv::Store kv_store;
+  auto encryptor = std::make_shared<ccf::kv::NullTxEncryptor>();
+  kv_store.set_encryptor(encryptor);
+
+  SNPUVMEndorsements table(Tables::NODE_SNP_UVM_ENDORSEMENTS);
+
+  // Pre-populate with an existing DID
+  {
+    auto tx = kv_store.create_tx();
+    auto handle = tx.rw(table);
+    FeedToEndorsementsDataMap existing;
+    existing["existing-feed"] = {"100"};
+    handle->put("did:x509:existing", existing);
+    REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS);
+  }
+
+  // Call with a different DID while recovering
+  pal::UVMEndorsements endorsement{"did:x509:new", "new-feed", "50"};
+
+  {
+    auto tx = kv_store.create_tx();
+    InternalTablesAccess::trust_node_uvm_endorsements(
+      tx, endorsement, true /* recovering */);
+    REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS);
+  }
+
+  // Verify new DID was written
+  {
+    auto tx = kv_store.create_read_only_tx();
+    auto handle = tx.ro(table);
+
+    auto new_result = handle->get("did:x509:new");
+    REQUIRE(new_result.has_value());
+    REQUIRE(new_result->size() == 1);
+    auto it = new_result->find("new-feed");
+    REQUIRE(it != new_result->end());
+    REQUIRE(it->second.svn == "50");
+
+    // Prior contents unchanged
+    auto existing_result = handle->get("did:x509:existing");
+    REQUIRE(existing_result.has_value());
+    REQUIRE(existing_result->size() == 1);
+    auto eit = existing_result->find("existing-feed");
+    REQUIRE(eit != existing_result->end());
+    REQUIRE(eit->second.svn == "100");
+  }
+}
+
+TEST_CASE("trust_node_uvm_endorsements - recovering, existing DID, new feed")
+{
+  ccf::kv::Store kv_store;
+  auto encryptor = std::make_shared<ccf::kv::NullTxEncryptor>();
+  kv_store.set_encryptor(encryptor);
+
+  SNPUVMEndorsements table(Tables::NODE_SNP_UVM_ENDORSEMENTS);
+
+  // Pre-populate with an existing DID and feed
+  {
+    auto tx = kv_store.create_tx();
+    auto handle = tx.rw(table);
+    FeedToEndorsementsDataMap existing;
+    existing["feed-A"] = {"100"};
+    handle->put("did:x509:shared", existing);
+    REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS);
+  }
+
+  // Call with the same DID but a different feed while recovering
+  pal::UVMEndorsements endorsement{"did:x509:shared", "feed-B", "75"};
+
+  {
+    auto tx = kv_store.create_tx();
+    InternalTablesAccess::trust_node_uvm_endorsements(
+      tx, endorsement, true /* recovering */);
+    REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS);
+  }
+
+  // Verify both feeds are present
+  {
+    auto tx = kv_store.create_read_only_tx();
+    auto handle = tx.ro(table);
+
+    auto result = handle->get("did:x509:shared");
+    REQUIRE(result.has_value());
+    REQUIRE(result->size() == 2);
+
+    auto it_a = result->find("feed-A");
+    REQUIRE(it_a != result->end());
+    REQUIRE(it_a->second.svn == "100");
+
+    auto it_b = result->find("feed-B");
+    REQUIRE(it_b != result->end());
+    REQUIRE(it_b->second.svn == "75");
+  }
+}
+
+TEST_CASE(
+  "trust_node_uvm_endorsements - recovering, existing DID and feed, lower SVN")
+{
+  ccf::kv::Store kv_store;
+  auto encryptor = std::make_shared<ccf::kv::NullTxEncryptor>();
+  kv_store.set_encryptor(encryptor);
+
+  SNPUVMEndorsements table(Tables::NODE_SNP_UVM_ENDORSEMENTS);
+
+  // Pre-populate with SVN 100, plus a separate unrelated DID
+  {
+    auto tx = kv_store.create_tx();
+    auto handle = tx.rw(table);
+    FeedToEndorsementsDataMap existing;
+    existing["the-feed"] = {"100"};
+    handle->put("did:x509:the-did", existing);
+
+    FeedToEndorsementsDataMap other;
+    other["other-feed"] = {"999"};
+    handle->put("did:x509:other-did", other);
+    REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS);
+  }
+
+  // Call with strictly lower SVN while recovering
+  pal::UVMEndorsements endorsement{"did:x509:the-did", "the-feed", "42"};
+
+  {
+    auto tx = kv_store.create_tx();
+    InternalTablesAccess::trust_node_uvm_endorsements(
+      tx, endorsement, true /* recovering */);
+    REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS);
+  }
+
+  // SVN should be updated to the lower value
+  {
+    auto tx = kv_store.create_read_only_tx();
+    auto handle = tx.ro(table);
+
+    auto result = handle->get("did:x509:the-did");
+    REQUIRE(result.has_value());
+    REQUIRE(result->size() == 1);
+    auto it = result->find("the-feed");
+    REQUIRE(it != result->end());
+    REQUIRE(it->second.svn == "42");
+
+    // Pre-existing unrelated DID is unchanged
+    auto other_result = handle->get("did:x509:other-did");
+    REQUIRE(other_result.has_value());
+    REQUIRE(other_result->size() == 1);
+    auto oit = other_result->find("other-feed");
+    REQUIRE(oit != other_result->end());
+    REQUIRE(oit->second.svn == "999");
+  }
+}
+
+TEST_CASE(
+  "trust_node_uvm_endorsements - recovering, existing DID and feed, higher "
+  "SVN")
+{
+  ccf::kv::Store kv_store;
+  auto encryptor = std::make_shared<ccf::kv::NullTxEncryptor>();
+  kv_store.set_encryptor(encryptor);
+
+  SNPUVMEndorsements table(Tables::NODE_SNP_UVM_ENDORSEMENTS);
+
+  // Pre-populate with SVN 42
+  {
+    auto tx = kv_store.create_tx();
+    auto handle = tx.rw(table);
+    FeedToEndorsementsDataMap existing;
+    existing["the-feed"] = {"42"};
+    handle->put("did:x509:the-did", existing);
+    REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS);
+  }
+
+  // Call with strictly higher SVN while recovering
+  pal::UVMEndorsements endorsement{"did:x509:the-did", "the-feed", "100"};
+
+  {
+    auto tx = kv_store.create_tx();
+    InternalTablesAccess::trust_node_uvm_endorsements(
+      tx, endorsement, true /* recovering */);
+    REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS);
+  }
+
+  // Map should be unchanged - SVN stays at 42
+  {
+    auto tx = kv_store.create_read_only_tx();
+    auto handle = tx.ro(table);
+
+    auto result = handle->get("did:x509:the-did");
+    REQUIRE(result.has_value());
+    REQUIRE(result->size() == 1);
+    auto it = result->find("the-feed");
+    REQUIRE(it != result->end());
+    REQUIRE(it->second.svn == "42");
+  }
+}
diff --git a/src/node/uvm_endorsements.cpp b/src/node/uvm_endorsements.cpp
@@ -9,34 +9,27 @@
 
 namespace ccf
 {
+  size_t parse_svn(const std::string& svn_str)
+  {
+    size_t svn = 0;
+    auto result =
+      std::from_chars(svn_str.data(), svn_str.data() + svn_str.size(), svn);
+    if (result.ec != std::errc())
+    {
+      throw std::runtime_error(
+        fmt::format("Unable to parse svn value {} to unsigned", svn_str));
+    }
+    return svn;
+  }
+
   bool matches_uvm_roots_of_trust(
     const pal::UVMEndorsements& endorsements,
     const std::vector<pal::UVMEndorsements>& uvm_roots_of_trust)
   {
     return std::ranges::any_of(
       uvm_roots_of_trust, [&](const auto& uvm_root_of_trust) {
-        size_t root_of_trust_svn = 0;
-        auto result = std::from_chars(
-          uvm_root_of_trust.svn.data(),
-          uvm_root_of_trust.svn.data() + uvm_root_of_trust.svn.size(),
-          root_of_trust_svn);
-        if (result.ec != std::errc())
-        {
-          throw std::runtime_error(fmt::format(
-            "Unable to parse svn value {} to unsigned in UVM root of trust",
-            uvm_root_of_trust.svn));
-        }
-        size_t endorsement_svn = 0;
-        result = std::from_chars(
-          endorsements.svn.data(),
-          endorsements.svn.data() + endorsements.svn.size(),
-          endorsement_svn);
-        if (result.ec != std::errc())
-        {
-          throw std::runtime_error(fmt::format(
-            "Unable to parse svn value {} to unsigned in UVM endorsements",
-            endorsements.svn));
-        }
+        auto root_of_trust_svn = parse_svn(uvm_root_of_trust.svn);
+        auto endorsement_svn = parse_svn(endorsements.svn);
 
         return uvm_root_of_trust.did == endorsements.did &&
           uvm_root_of_trust.feed == endorsements.feed &&
diff --git a/src/node/uvm_endorsements.h b/src/node/uvm_endorsements.h
@@ -45,6 +45,8 @@ namespace ccf
     const pal::PlatformAttestationMeasurement& uvm_measurement,
     const std::vector<pal::UVMEndorsements>& uvm_roots_of_trust);
 
+  size_t parse_svn(const std::string& svn_str);
+
   bool matches_uvm_roots_of_trust(
     const pal::UVMEndorsements& endorsements,
     const std::vector<pal::UVMEndorsements>& uvm_roots_of_trust);
diff --git a/src/service/internal_tables_access.h b/src/service/internal_tables_access.h
diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py

Original file line number	Diff line number	Diff line change
`@@ -819,6 +819,13 @@ if(BUILD_TESTS)`
`819`	`819`	`ccf_tasks`
`820`	`820`	`)`
`821`	`821`
	`822`	`+ add_unit_test(`
	`823`	`+ internal_tables_access_test`
	`824`	`+ ${CMAKE_CURRENT_SOURCE_DIR}/src/node/rpc/test/internal_tables_access_test.cpp`
	`825`	`+ ${CCF_DIR}/src/node/uvm_endorsements.cpp`
	`826`	`+ )`
	`827`	`+ target_link_libraries(internal_tables_access_test PRIVATE ccfcrypto ccf_kv)`
	`828`	`+`
`822`	`829`	`add_unit_test(`
`823`	`830`	`merkle_test ${CMAKE_CURRENT_SOURCE_DIR}/src/node/test/merkle_test.cpp`
`824`	`831`	`)`