Backport PR #7717: On recovery, set UVM descriptor SVN to minimum of existing KV value and startup endorsements

Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

Author: Copilot

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 - Fix race condition when initialising a ledger secret's commit secret (#7689)
 - Add missing cases for `FailedInvalidCPUID` and `FailedInvalidTcbVersion` in quote verification error handling (#7696).
+- 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).
 
 ## [6.0.23]
 

CMakeLists.txt

@@ -1005,6 +1005,15 @@ if(BUILD_TESTS)
       ${CMAKE_CURRENT_SOURCE_DIR}/src/node/rpc/test/tx_status_test.cpp
     )
 
+    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.host ccf_kv.host
+    )
+
     add_unit_test(
       node_frontend_test
       ${CMAKE_CURRENT_SOURCE_DIR}/src/node/rpc/test/node_frontend_test.cpp

src/node/rpc/node_frontend.h

@@ -1636,7 +1636,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();

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");
+  }
+}

src/node/uvm_endorsements.cpp

@@ -5,34 +5,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 inline 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 &&

src/node/uvm_endorsements.h

@@ -45,4 +45,6 @@ namespace ccf
     const std::vector<uint8_t>& uvm_endorsements_raw,
     const pal::PlatformAttestationMeasurement& uvm_measurement,
     const std::vector<pal::UVMEndorsements>& uvm_roots_of_trust);
+
+  size_t parse_svn(const std::string& svn_str);
 }