TMP store hex string of cpuid

Author: cjen1-msft

include/ccf/pal/attestation_sev_snp.h

@@ -282,10 +282,10 @@ QPHfbkH0CyPfhl1jWhJFZasCAwEAAQ==
     bool operator==(const CPUID&) const = default;
     std::string hex_str() const
     {
-      uint8_t buf[sizeof(CPUID)];
-      memcpy(buf, this, sizeof(CPUID));
-      std::reverse(buf, buf + sizeof(CPUID)); // fix little endianness of AMD
-      return ccf::ds::to_hex(buf, buf + sizeof(CPUID));
+      CPUID buf = *this;
+      auto buf_ptr = reinterpret_cast<uint8_t*>(&buf);
+      std::reverse(buf_ptr, buf_ptr + sizeof(CPUID));
+      return ccf::ds::to_hex(buf_ptr, buf_ptr + sizeof(CPUID));
     }
     inline uint8_t get_family_id() const
     {
@@ -312,18 +312,20 @@ QPHfbkH0CyPfhl1jWhJFZasCAwEAAQ==
     return ret;
   }
 
-  union UnionedCPUID
-  {
-    uint32_t eax;
-    CPUID cpuid;
-  };
-
   static CPUID get_cpuid()
   {
-    UnionedCPUID cpuid_eax;
-    cpuid_eax.eax = 0;
-    asm volatile("cpuid" : "=a"(cpuid_eax.eax) : "a"(1));
-    return cpuid_eax.cpuid;
+    uint32_t ieax = 1;
+    uint64_t iebx = 0;
+    uint64_t iecx = 0;
+    uint64_t iedx = 0;
+    uint32_t oeax = 0;
+    uint64_t oebx = 0;
+    uint64_t oecx = 0;
+    uint64_t oedx = 0;
+    // pass in e{b,c,d}x to prevent cpuid from blatting other registers
+    asm volatile("cpuid" : "=a"(oeax), "=b"(oebx), "=c"(oecx), "=d"(oedx): "a"(ieax), "b"(iebx), "c"(iecx), "d"(iedx));
+    auto cpuid =  *reinterpret_cast<CPUID*>(&oeax);
+    return cpuid;
   }
 }
 

include/ccf/service/tables/tcb_verification.h

@@ -8,7 +8,7 @@
 
 namespace ccf
 {
-  using SnpTcbVersionMap = ServiceMap<pal::snp::CPUID, pal::snp::TcbVersion>;
+  using SnpTcbVersionMap = ServiceMap<std::string, pal::snp::TcbVersion>;
 
   namespace Tables
   {

samples/constitutions/default/actions.js

@@ -1117,14 +1117,13 @@ const actions = new Map([
           "tcb_version.microcode",
         );
       },
-      function (args) {
+      function (args, proposalId) {
         ccf.kv["public:ccf.gov.nodes.snp.tcb_versions"].set(
-          hexStrToBuf(args.cpuid),
+          ccf.strToBuf(args.cpuid),
           ccf.jsonToSnpTcbVersion(args.tcb_version),
         );
 
-        // Is this required?
-        //invalidateOtherOpenProposals(proposalId);
+        invalidateOtherOpenProposals(proposalId);
       },
     ),
   ],

src/node/gov/handlers/service_state.h

@@ -619,11 +619,12 @@ namespace ccf::gov::endpoints
             auto tcb_versions_handle =
               ctx.tx.template ro<ccf::SnpTcbVersionMap>(
                 ccf::Tables::SNP_TCB_VERSIONS);
+
             tcb_versions_handle->foreach(
               [&snp_tcb_versions](
-                const pal::snp::CPUID& cpuid,
+                const std::string& cpuid,
                 const pal::snp::TcbVersion& tcb_version) {
-                snp_tcb_versions[cpuid.hex_str()] = tcb_version;
+                snp_tcb_versions[cpuid] = tcb_version;
                 return true;
               });
             snp_policy["tcbVersions"] = snp_tcb_versions;

src/node/quote.cpp

@@ -280,7 +280,8 @@ namespace ccf
     auto h = tx.ro<SnpTcbVersionMap>(Tables::SNP_TCB_VERSIONS);
     // expensive but there should not be many entries
     h->foreach([&min_tcb_opt, &attestation](
-                 const pal::snp::CPUID& cpuid, const pal::snp::TcbVersion& v) {
+                 const std::string cpuid_hex, const pal::snp::TcbVersion& v) {
+      auto cpuid = pal::snp::cpuid_from_hex(cpuid_hex);
       if (
         cpuid.get_family_id() == attestation.cpuid_fam_id &&
         cpuid.get_model_id() == attestation.cpuid_mod_id &&

src/service/internal_tables_access.h

@@ -841,7 +841,7 @@ namespace ccf
         .reserved = {0},
         .snp = 0x18,
         .microcode = 0xDB};
-      h->put(milan_chip_id, milan_tcb_version);
+      h->put(milan_chip_id.hex_str(), milan_tcb_version);
 
       constexpr pal::snp::CPUID milan_x_chip_id{
         .stepping = 0x2,
@@ -857,7 +857,7 @@ namespace ccf
         .reserved = {0},
         .snp = 0x18,
         .microcode = 0x44};
-      h->put(milan_x_chip_id, milan_x_tcb_version);
+      h->put(milan_x_chip_id.hex_str(), milan_x_tcb_version);
 
       constexpr pal::snp::CPUID genoa_chip_id{
         .stepping = 0x1,
@@ -873,7 +873,7 @@ namespace ccf
         .reserved = {0},
         .snp = 0x17,
         .microcode = 0x54};
-      h->put(genoa_chip_id, genoa_tcb_version);
+      h->put(genoa_chip_id.hex_str(), genoa_tcb_version);
 
       constexpr pal::snp::CPUID genoa_x_chip_id{
         .stepping = 0x2,
@@ -889,7 +889,7 @@ namespace ccf
         .reserved = {0},
         .snp = 0x17,
         .microcode = 0x4F};
-      h->put(genoa_x_chip_id, genoa_x_tcb_version);
+      h->put(genoa_x_chip_id.hex_str(), genoa_x_tcb_version);
     }
 
     static void trust_node_snp_tcb_version(
@@ -919,7 +919,7 @@ namespace ccf
         return;
       }
       auto h = tx.wo<ccf::SnpTcbVersionMap>(Tables::SNP_TCB_VERSIONS);
-      h->put(cpuid, attestation.reported_tcb);
+      h->put(cpuid.hex_str(), attestation.reported_tcb);
     }
 
     static void init_configuration(

tests/code_update.py

@@ -286,6 +286,16 @@ def test_tcb_version_tables(network, args):
         assert len(versions) == 1, f"Expected one TCB version, {versions}"
         cpuid, tcb_version = next(iter(versions.items()))
 
+    LOG.info("Change current cpuid's TCB version")
+    test_tcb_version = {"boot_loader": 0, "microcode": 0, "snp": 0, "tee": 0}
+    network.consortium.add_snp_tcb_version(primary, cpuid, test_tcb_version)
+    with primary.api_versioned_client(api_version=args.gov_api_version) as client:
+        r = client.get("/gov/service/join-policy")
+        assert r.status_code == http.HTTPStatus.OK, r
+        versions = r.body.json()["snp"]["tcbVersions"]
+        assert cpuid in versions, f"Expected {cpuid} in TCB versions, {versions}"
+        assert versions[cpuid] == test_tcb_version, f"TCB version does not match, {versions}"
+
     LOG.info("Removing current cpuid's TCB version")
     network.consortium.remove_snp_tcb_version(primary, cpuid)
     with primary.api_versioned_client(api_version=args.gov_api_version) as client:
@@ -768,43 +778,43 @@ def run(args):
     ) as network:
         network.start_and_open(args)
 
-        test_verify_quotes(network, args)
-
-        # Measurements
-        test_measurements_tables(network, args)
-        if not snp.IS_SNP:
-            test_add_node_with_untrusted_measurement(network, args)
-
-        # Host data/security policy
-        test_host_data_tables(network, args)
-        test_add_node_with_untrusted_host_data(network, args)
-
+#        test_verify_quotes(network, args)
+#
+#        # Measurements
+#        test_measurements_tables(network, args)
+#        if not snp.IS_SNP:
+#            test_add_node_with_untrusted_measurement(network, args)
+#
+#        # Host data/security policy
+#        test_host_data_tables(network, args)
+#        test_add_node_with_untrusted_host_data(network, args)
+#
         if snp.IS_SNP:
-            # Virtual has no security policy, _only_ host data (unassociated with anything)
-            test_add_node_with_stubbed_security_policy(network, args)
-            test_start_node_with_mismatched_host_data(network, args)
-            test_add_node_without_security_policy(network, args)
+#            # Virtual has no security policy, _only_ host data (unassociated with anything)
+#            test_add_node_with_stubbed_security_policy(network, args)
+#            test_start_node_with_mismatched_host_data(network, args)
+#            test_add_node_without_security_policy(network, args)
             test_tcb_version_tables(network, args)
-
-            # Endorsements
-            test_endorsements_tables(network, args)
-            test_add_node_with_no_uvm_endorsements(network, args)
-
-        if not snp.IS_SNP:
-            # NB: Assumes the current nodes are still using args.package, so must run before test_update_all_nodes
-            test_proposal_invalidation(network, args)
-
-            # This is in practice equivalent to either "unknown measurement" or "unknown host data", but is explicitly
-            # testing that (without artifically removing/corrupting those values) a replacement package differs
-            # in one of these values
-            test_add_node_with_different_package(network, args)
-            test_update_all_nodes(network, args)
-
-        # Run again at the end to confirm current nodes are acceptable
-        test_verify_quotes(network, args)
-
-        if snp.IS_SNP:
-            test_add_node_with_no_uvm_endorsements_in_kv(network, args)
+#
+#            # Endorsements
+#            test_endorsements_tables(network, args)
+#            test_add_node_with_no_uvm_endorsements(network, args)
+#
+#        if not snp.IS_SNP:
+#            # NB: Assumes the current nodes are still using args.package, so must run before test_update_all_nodes
+#            test_proposal_invalidation(network, args)
+#
+#            # This is in practice equivalent to either "unknown measurement" or "unknown host data", but is explicitly
+#            # testing that (without artifically removing/corrupting those values) a replacement package differs
+#            # in one of these values
+#            test_add_node_with_different_package(network, args)
+#            test_update_all_nodes(network, args)
+#
+#        # Run again at the end to confirm current nodes are acceptable
+#        test_verify_quotes(network, args)
+#
+#        if snp.IS_SNP:
+#            test_add_node_with_no_uvm_endorsements_in_kv(network, args)
 
 
 if __name__ == "__main__":