From 9a4bd4aa01b3f94449932f2a2b7a23a95b0bb36a Mon Sep 17 00:00:00 2001 From: cjen1-msft Date: Tue, 9 Jun 2026 16:15:16 +0100 Subject: [PATCH 1/2] Pin ARK issuer and algorithm (#7934) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Amaury Chamayou Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Amaury Chamayou --- include/ccf/pal/attestation_sev_snp.h | 27 +++- src/pal/attestation.cpp | 91 +++++++++++-- src/pal/test/snp_attestation_validation.cpp | 143 ++++++++++++++++++++ 3 files changed, 247 insertions(+), 14 deletions(-) diff --git a/include/ccf/pal/attestation_sev_snp.h b/include/ccf/pal/attestation_sev_snp.h index b340c943e8aa..248b39cef33f 100644 --- a/include/ccf/pal/attestation_sev_snp.h +++ b/include/ccf/pal/attestation_sev_snp.h @@ -77,10 +77,29 @@ pRb21iI1NlNCfOGUPIhVpWECAwEAAQ== -----END PUBLIC KEY----- )"; - inline const std::map amd_root_signing_keys{ - {ProductName::Milan, amd_milan_root_signing_public_key}, - {ProductName::Genoa, amd_genoa_root_signing_public_key}, - {ProductName::Turin, amd_turin_root_signing_public_key}, + struct AmdRootSigningKey + { + const char* public_key; + const char* issuer; + const char* signature_algorithm; + }; + + inline const std::map amd_root_signing_keys{ + {ProductName::Milan, + {amd_milan_root_signing_public_key, + "CN=ARK-Milan,O=Advanced Micro Devices,ST=CA,L=Santa Clara,C=US," + "OU=Engineering", + "rsassaPss"}}, + {ProductName::Genoa, + {amd_genoa_root_signing_public_key, + "CN=ARK-Genoa,O=Advanced Micro Devices,ST=CA,L=Santa Clara,C=US," + "OU=Engineering", + "rsassaPss"}}, + {ProductName::Turin, + {amd_turin_root_signing_public_key, + "CN=ARK-Turin,O=Advanced Micro Devices,ST=CA,L=Santa Clara,C=US," + "OU=Engineering", + "rsassaPss"}}, }; #pragma pack(push, 1) diff --git a/src/pal/attestation.cpp b/src/pal/attestation.cpp index c7922f381c3c..f1caaffbcbf1 100644 --- a/src/pal/attestation.cpp +++ b/src/pal/attestation.cpp @@ -10,6 +10,7 @@ #include "ccf/pal/sev_snp_cpuid.h" #include +#include namespace ccf::pal { @@ -18,6 +19,76 @@ namespace ccf::pal using Unique_ASN1_INTEGER = ccf::crypto::OpenSSL:: Unique_SSL_OBJECT; + namespace + { + std::string x509_name_to_rfc2253_string(X509_NAME* name) + { + ccf::crypto::OpenSSL::CHECKNULL(name); + + ccf::crypto::OpenSSL::Unique_BIO mem; + const auto rc = X509_NAME_print_ex(mem, name, 0, XN_FLAG_RFC2253); + if (rc < 0) + { + const auto ec = ERR_get_error(); + throw std::runtime_error(fmt::format( + "OpenSSL error (rc={}, ec={}): {}", + rc, + ec, + ccf::crypto::OpenSSL::error_string(ec))); + } + + BUF_MEM* bptr = nullptr; + ccf::crypto::OpenSSL::CHECK1(BIO_get_mem_ptr(mem, &bptr)); + ccf::crypto::OpenSSL::CHECKNULL(bptr); + + return {bptr->data, bptr->length}; + } + + std::string signature_algorithm_name(int nid) + { + const auto* name = OBJ_nid2ln(nid); + if (name == nullptr) + { + return fmt::format("nid {}", nid); + } + return name; + } + + void verify_ark_certificate_matches_pinned_metadata( + const crypto::Pem& ark_cert, + snp::ProductName product_family, + const snp::AmdRootSigningKey& expected_ark) + { + ccf::crypto::OpenSSL::Unique_BIO mem_bio(ark_cert); + ccf::crypto::OpenSSL::Unique_X509 x509( + mem_bio, true, true /* check_null */); + + const auto issuer = + x509_name_to_rfc2253_string(X509_get_issuer_name(x509)); + if (issuer != expected_ark.issuer) + { + throw std::logic_error(fmt::format( + "SEV-SNP: The root of trust issuer for this attestation was not " + "the expected one for {}: {} != {}", + product_family, + issuer, + expected_ark.issuer)); + } + + const auto signature_algorithm = + signature_algorithm_name(X509_get_signature_nid(x509)); + if (signature_algorithm != expected_ark.signature_algorithm) + { + throw std::logic_error(fmt::format( + "SEV-SNP: The root of trust signature algorithm for this " + "attestation was not the expected one for {}: {} != {}", + product_family, + signature_algorithm, + expected_ark.signature_algorithm)); + } + } + } + void verify_virtual_attestation_report( const QuoteInfo& quote_info, PlatformAttestationMeasurement& measurement, @@ -259,17 +330,14 @@ namespace ccf::pal auto ark_verifier = ccf::crypto::make_verifier(ark_cert); - std::string expected_ark; + auto key = snp::amd_root_signing_keys.find(product_family); + if (key == snp::amd_root_signing_keys.end()) { - auto key = snp::amd_root_signing_keys.find(product_family); - if (key == snp::amd_root_signing_keys.end()) - { - throw std::logic_error(fmt::format( - "SEV-SNP: No known root certificate for {}", product_family)); - } - expected_ark = key->second; + throw std::logic_error(fmt::format( + "SEV-SNP: No known root certificate for {}", product_family)); } - if (ark_verifier->public_key_pem().str() != expected_ark) + const auto& expected_ark = key->second; + if (ark_verifier->public_key_pem().str() != expected_ark.public_key) { throw std::logic_error(fmt::format( "SEV-SNP: The root of trust public key for this attestation was not " @@ -278,9 +346,12 @@ namespace ccf::pal quote.cpuid_fam_id, quote.cpuid_mod_id, ark_verifier->public_key_pem().str(), - expected_ark)); + expected_ark.public_key)); } + verify_ark_certificate_matches_pinned_metadata( + ark_cert, product_family, expected_ark); + if (!ark_verifier->verify_certificate({&ark_cert})) { throw std::logic_error( diff --git a/src/pal/test/snp_attestation_validation.cpp b/src/pal/test/snp_attestation_validation.cpp index 6ee3e90b8202..d194a9854ed5 100644 --- a/src/pal/test/snp_attestation_validation.cpp +++ b/src/pal/test/snp_attestation_validation.cpp @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the Apache 2.0 License. +#include "ccf/crypto/openssl/openssl_wrappers.h" +#include "ccf/crypto/verifier.h" #include "ccf/ds/hex.h" #include "ccf/ds/logger.h" #include "ccf/ds/quote_info.h" @@ -16,9 +18,117 @@ #include "pal/test/attestation_sev_snp_endorsements.h" #include "pal/test/snp_attestation_validation_data.h" +#include +#include +#include + #define DOCTEST_CONFIG_IMPLEMENT #include +namespace +{ + std::vector milan_endorsement_certs() + { + return ccf::crypto::split_x509_cert_bundle( + ccf::pal::snp::testing::milan_endorsements); + } + + std::vector endorsement_bundle_with_ark( + const ccf::crypto::Pem& ark_cert) + { + auto certs = milan_endorsement_certs(); + REQUIRE(certs.size() == 3); + + certs[2] = ark_cert; + std::string bundle; + for (const auto& cert : certs) + { + bundle += cert.str(); + } + + return {bundle.begin(), bundle.end()}; + } + + ccf::QuoteInfo milan_quote_info_with_ark(const ccf::crypto::Pem& ark_cert) + { + return { + .format = ccf::QuoteFormat::amd_sev_snp_v1, + .quote = ccf::pal::snp::testing::milan_attestation, + .endorsements = endorsement_bundle_with_ark(ark_cert), + .uvm_endorsements = std::nullopt, + }; + } + + ccf::crypto::Pem pem_from_x509(X509* x509) + { + ccf::crypto::OpenSSL::CHECKNULL(x509); + + ccf::crypto::OpenSSL::Unique_BIO mem; + ccf::crypto::OpenSSL::CHECK1(PEM_write_bio_X509(mem, x509)); + + BUF_MEM* bptr = nullptr; + ccf::crypto::OpenSSL::CHECK1(BIO_get_mem_ptr(mem, &bptr)); + ccf::crypto::OpenSSL::CHECKNULL(bptr); + + return ccf::crypto::Pem( + reinterpret_cast(bptr->data), bptr->length); + } + + ccf::crypto::Pem ark_with_extra_issuer_entry() + { + const auto certs = milan_endorsement_certs(); + REQUIRE(certs.size() == 3); + + ccf::crypto::OpenSSL::Unique_BIO mem_bio(certs[2]); + ccf::crypto::OpenSSL::Unique_X509 x509( + mem_bio, true, true /* check_null */); + + std::unique_ptr issuer( + X509_NAME_dup(X509_get_issuer_name(x509)), X509_NAME_free); + ccf::crypto::OpenSSL::CHECKNULL(issuer.get()); + + static constexpr auto unexpected_ou = "Unexpected"; + ccf::crypto::OpenSSL::CHECK1(X509_NAME_add_entry_by_txt( + issuer.get(), + "OU", + MBSTRING_ASC, + reinterpret_cast(unexpected_ou), + -1, + -1, + 0)); + ccf::crypto::OpenSSL::CHECK1(X509_set_issuer_name(x509, issuer.get())); + + return pem_from_x509(x509); + } + + ccf::crypto::Pem ark_with_sha384_rsa_signature_algorithm() + { + const auto certs = milan_endorsement_certs(); + REQUIRE(certs.size() == 3); + + auto der = ccf::crypto::cert_pem_to_der(certs[2]); + static constexpr std::array rsassa_pss_oid = { + 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0a}; + static constexpr std::array sha384_rsa_oid = { + 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0c}; + + size_t replacements = 0; + for (size_t i = 0; i + rsassa_pss_oid.size() <= der.size(); ++i) + { + auto it = der.begin() + i; + if (std::equal(rsassa_pss_oid.begin(), rsassa_pss_oid.end(), it)) + { + std::copy(sha384_rsa_oid.begin(), sha384_rsa_oid.end(), it); + ++replacements; + } + } + + REQUIRE(replacements > 0); + + return ccf::crypto::cert_der_to_pem(der); + } +} + TEST_CASE("milan validation") { using namespace ccf; @@ -104,6 +214,39 @@ TEST_CASE("Mismatched attestation and endorsements fail") std::logic_error); } +TEST_CASE("ARK with unexpected issuer fails") +{ + auto quote_info = milan_quote_info_with_ark(ark_with_extra_issuer_entry()); + + ccf::pal::PlatformAttestationMeasurement measurement; + ccf::pal::PlatformAttestationReportData report_data; + + CHECK_THROWS_WITH_AS( + ccf::pal::verify_snp_attestation_report( + quote_info, measurement, report_data), + doctest::Contains( + "SEV-SNP: The root of trust issuer for this attestation was not " + "the expected one"), + std::logic_error); +} + +TEST_CASE("ARK with unexpected signature algorithm fails") +{ + auto quote_info = + milan_quote_info_with_ark(ark_with_sha384_rsa_signature_algorithm()); + + ccf::pal::PlatformAttestationMeasurement measurement; + ccf::pal::PlatformAttestationReportData report_data; + + CHECK_THROWS_WITH_AS( + ccf::pal::verify_snp_attestation_report( + quote_info, measurement, report_data), + doctest::Contains( + "SEV-SNP: The root of trust signature algorithm for this attestation " + "was not the expected one"), + std::logic_error); +} + TEST_CASE("Parsing of Tcb versions from strings") { const auto milan_tcb_version_raw = From 6e1ffe21f1880142ed6cd3fae332aeb3cf40abb7 Mon Sep 17 00:00:00 2001 From: cjen1-msft Date: Thu, 11 Jun 2026 15:54:15 +0100 Subject: [PATCH 2/2] Remove algo comparison --- include/ccf/pal/attestation_sev_snp.h | 10 ++--- src/pal/attestation.cpp | 34 +++++----------- src/pal/test/snp_attestation_validation.cpp | 43 --------------------- 3 files changed, 12 insertions(+), 75 deletions(-) diff --git a/include/ccf/pal/attestation_sev_snp.h b/include/ccf/pal/attestation_sev_snp.h index 248b39cef33f..786908f49771 100644 --- a/include/ccf/pal/attestation_sev_snp.h +++ b/include/ccf/pal/attestation_sev_snp.h @@ -81,25 +81,21 @@ pRb21iI1NlNCfOGUPIhVpWECAwEAAQ== { const char* public_key; const char* issuer; - const char* signature_algorithm; }; inline const std::map amd_root_signing_keys{ {ProductName::Milan, {amd_milan_root_signing_public_key, "CN=ARK-Milan,O=Advanced Micro Devices,ST=CA,L=Santa Clara,C=US," - "OU=Engineering", - "rsassaPss"}}, + "OU=Engineering"}}, {ProductName::Genoa, {amd_genoa_root_signing_public_key, "CN=ARK-Genoa,O=Advanced Micro Devices,ST=CA,L=Santa Clara,C=US," - "OU=Engineering", - "rsassaPss"}}, + "OU=Engineering"}}, {ProductName::Turin, {amd_turin_root_signing_public_key, "CN=ARK-Turin,O=Advanced Micro Devices,ST=CA,L=Santa Clara,C=US," - "OU=Engineering", - "rsassaPss"}}, + "OU=Engineering"}}, }; #pragma pack(push, 1) diff --git a/src/pal/attestation.cpp b/src/pal/attestation.cpp index f1caaffbcbf1..c956a178d9c4 100644 --- a/src/pal/attestation.cpp +++ b/src/pal/attestation.cpp @@ -4,8 +4,10 @@ #include "ccf/pal/attestation.h" #include "ccf/crypto/ecdsa.h" +#include "ccf/crypto/key_pair.h" #include "ccf/crypto/openssl/openssl_wrappers.h" #include "ccf/crypto/verifier.h" +#include "ccf/ds/hex.h" #include "ccf/pal/attestation_sev_snp.h" #include "ccf/pal/sev_snp_cpuid.h" @@ -44,16 +46,6 @@ namespace ccf::pal return {bptr->data, bptr->length}; } - std::string signature_algorithm_name(int nid) - { - const auto* name = OBJ_nid2ln(nid); - if (name == nullptr) - { - return fmt::format("nid {}", nid); - } - return name; - } - void verify_ark_certificate_matches_pinned_metadata( const crypto::Pem& ark_cert, snp::ProductName product_family, @@ -74,18 +66,6 @@ namespace ccf::pal issuer, expected_ark.issuer)); } - - const auto signature_algorithm = - signature_algorithm_name(X509_get_signature_nid(x509)); - if (signature_algorithm != expected_ark.signature_algorithm) - { - throw std::logic_error(fmt::format( - "SEV-SNP: The root of trust signature algorithm for this " - "attestation was not the expected one for {}: {} != {}", - product_family, - signature_algorithm, - expected_ark.signature_algorithm)); - } } } @@ -337,7 +317,11 @@ namespace ccf::pal "SEV-SNP: No known root certificate for {}", product_family)); } const auto& expected_ark = key->second; - if (ark_verifier->public_key_pem().str() != expected_ark.public_key) + const auto actual_public_key_der = ark_verifier->public_key_der(); + const auto expected_public_key_der = + ccf::crypto::make_public_key(ccf::crypto::Pem(expected_ark.public_key)) + ->public_key_der(); + if (actual_public_key_der != expected_public_key_der) { throw std::logic_error(fmt::format( "SEV-SNP: The root of trust public key for this attestation was not " @@ -345,8 +329,8 @@ namespace ccf::pal quote.version, quote.cpuid_fam_id, quote.cpuid_mod_id, - ark_verifier->public_key_pem().str(), - expected_ark.public_key)); + ccf::ds::to_hex(actual_public_key_der), + ccf::ds::to_hex(expected_public_key_der))); } verify_ark_certificate_matches_pinned_metadata( diff --git a/src/pal/test/snp_attestation_validation.cpp b/src/pal/test/snp_attestation_validation.cpp index d194a9854ed5..59f70fb787a5 100644 --- a/src/pal/test/snp_attestation_validation.cpp +++ b/src/pal/test/snp_attestation_validation.cpp @@ -101,32 +101,6 @@ namespace return pem_from_x509(x509); } - ccf::crypto::Pem ark_with_sha384_rsa_signature_algorithm() - { - const auto certs = milan_endorsement_certs(); - REQUIRE(certs.size() == 3); - - auto der = ccf::crypto::cert_pem_to_der(certs[2]); - static constexpr std::array rsassa_pss_oid = { - 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0a}; - static constexpr std::array sha384_rsa_oid = { - 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0c}; - - size_t replacements = 0; - for (size_t i = 0; i + rsassa_pss_oid.size() <= der.size(); ++i) - { - auto it = der.begin() + i; - if (std::equal(rsassa_pss_oid.begin(), rsassa_pss_oid.end(), it)) - { - std::copy(sha384_rsa_oid.begin(), sha384_rsa_oid.end(), it); - ++replacements; - } - } - - REQUIRE(replacements > 0); - - return ccf::crypto::cert_der_to_pem(der); - } } TEST_CASE("milan validation") @@ -230,23 +204,6 @@ TEST_CASE("ARK with unexpected issuer fails") std::logic_error); } -TEST_CASE("ARK with unexpected signature algorithm fails") -{ - auto quote_info = - milan_quote_info_with_ark(ark_with_sha384_rsa_signature_algorithm()); - - ccf::pal::PlatformAttestationMeasurement measurement; - ccf::pal::PlatformAttestationReportData report_data; - - CHECK_THROWS_WITH_AS( - ccf::pal::verify_snp_attestation_report( - quote_info, measurement, report_data), - doctest::Contains( - "SEV-SNP: The root of trust signature algorithm for this attestation " - "was not the expected one"), - std::logic_error); -} - TEST_CASE("Parsing of Tcb versions from strings") { const auto milan_tcb_version_raw =