From 114bc93d743c8e4d061373782ebc782e28ab0653 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 25 Aug 2026 19:58:48 -0700 Subject: [PATCH] feat(attest): accept several collateral endpoints instead of one Attestation verification depends on collateral only the vendor publishes, and until now each source was configured as exactly one URL. AMD KDS is the sharper case: a single global endpoint with no official mirror, rate limited to roughly one identical request per ten seconds, and unreachable for hours at a time -- during which nothing on the host can verify an SEV-SNP quote, so no AMD guest can obtain keys. `pccs` and `amd_kds` now take either a string or a list. Endpoints are tried in order and the first answer wins. Existing configs and already-serialized `SysConfig` blobs parse unchanged, and a one-element list serializes back as a bare string, because guests hash the sys-config they were given rather than what a newer binary would have written. Failing over relaxes nothing. Collateral is vendor-signed and checked against roots compiled into the binary, so an endpoint can be slow, stale or absent but cannot forge an answer; which endpoint replied has no bearing on whether the signature checks pass. That is what makes a cache or mirror a legitimate entry in the list rather than a hole in the trust model. The two clients fail over on different rules, because they can see different things: - AMD KDS is in-tree, so it classifies. Transport failures, 408, 429 and 5xx say something about the endpoint and move on; a 404 is an answer about the chip that every mirror repeats, so it surfaces as-is rather than being retried into a pile of identical errors. - PCCS goes through `dcap-qvl`, which returns `anyhow::Error` with no status to classify on. It fails over on any error. Matching on error strings to do better would be worse than the coarse rule it replaced. Tests drive real sockets rather than asserting on the shape of the code: a 429 and a refused connection each hand off, a 404 stops with the second endpoint never contacted, and an all-fail run names every endpoint it tried. --- dstack/dstack-attest/src/attestation.rs | 36 +--- dstack/dstack-attest/src/collateral.rs | 102 ++++++++++ dstack/dstack-attest/src/lib.rs | 1 + dstack/dstack-types/src/lib.rs | 191 ++++++++++++++++++- dstack/dstack-util/src/host_api.rs | 22 +-- dstack/gateway/gateway.toml | 7 + dstack/kms/kms.toml | 7 + dstack/sev-snp-qvl/Cargo.toml | 3 + dstack/sev-snp-qvl/src/failover_tests.rs | 140 ++++++++++++++ dstack/sev-snp-qvl/src/lib.rs | 229 ++++++++++++++++++----- dstack/verifier/dstack-verifier.toml | 7 + 11 files changed, 645 insertions(+), 100 deletions(-) create mode 100644 dstack/dstack-attest/src/collateral.rs create mode 100644 dstack/sev-snp-qvl/src/failover_tests.rs diff --git a/dstack/dstack-attest/src/attestation.rs b/dstack/dstack-attest/src/attestation.rs index e55d63827..9d0bcdf5d 100644 --- a/dstack/dstack-attest/src/attestation.rs +++ b/dstack/dstack-attest/src/attestation.rs @@ -11,10 +11,10 @@ pub const TDX_QUOTE_REPORT_DATA_RANGE: std::ops::Range = 568..632; use std::{borrow::Cow, time::SystemTime}; +use crate::collateral::{amd_kds_client, PccsClient}; use anyhow::{anyhow, bail, Context, Result}; use cc_eventlog::{EventLogVersion, RuntimeEvent, TdxEvent}; use dcap_qvl::{ - collateral::CollateralClient, quote::{EnclaveReport, Quote, Report, TDReport10, TDReport15}, verify::VerifiedReport as TdxVerifiedReport, }; @@ -57,7 +57,7 @@ pub struct AttestationVerifierConfig { pub struct AttestationVerifier { tdx: dcap_qvl::verify::QuoteVerifier, - tdx_collateral: CollateralClient, + tdx_collateral: PccsClient, gcp_tpm: tpm_qvl::QuoteVerifier, aws_nitro_enclave: nsm_qvl::QuoteVerifier, aws_nitro_tpm: nsm_qvl::QuoteVerifier, @@ -130,26 +130,14 @@ impl AttestationVerifier { sev_snp = sev_snp.with_root(product, root); } } - let pccs = config - .urls - .pccs - .as_deref() - .filter(|v| !v.trim().is_empty()) - .unwrap_or(dcap_qvl::collateral::PHALA_PCCS_URL); - let amd_kds = config - .urls - .amd_kds - .as_deref() - .filter(|v| !v.trim().is_empty()) - .unwrap_or(sev_snp_qvl::AMD_KDS_DEFAULT_BASE_URL); Ok(Self { tdx, - tdx_collateral: CollateralClient::with_default_http(pccs)?, + tdx_collateral: PccsClient::new(&config.urls.pccs)?, gcp_tpm, aws_nitro_enclave: nsm(aws_nitro_enclave.as_deref(), "AWS Nitro Enclave")?, aws_nitro_tpm: nsm(aws_nitro_tpm.as_deref(), "AWS NitroTPM")?, sev_snp, - amd_kds: AmdKdsClient::with_base_url(amd_kds)?, + amd_kds: amd_kds_client(&config.urls.amd_kds)?, }) } @@ -157,24 +145,12 @@ impl AttestationVerifier { let collateral_urls = collateral_urls.cloned().unwrap_or_default(); Ok(Self { tdx: dcap_qvl::verify::QuoteVerifier::new_prod(), - tdx_collateral: CollateralClient::with_default_http( - collateral_urls - .pccs - .as_deref() - .filter(|url| !url.trim().is_empty()) - .unwrap_or(dcap_qvl::collateral::PHALA_PCCS_URL), - )?, + tdx_collateral: PccsClient::new(&collateral_urls.pccs)?, gcp_tpm: tpm_qvl::QuoteVerifier::new_prod(Platform::Gcp)?, aws_nitro_enclave: nsm_qvl::QuoteVerifier::new_prod(), aws_nitro_tpm: nsm_qvl::QuoteVerifier::new_prod(), sev_snp: sev_snp_qvl::QuoteVerifier::new_prod(), - amd_kds: AmdKdsClient::with_base_url( - collateral_urls - .amd_kds - .as_deref() - .filter(|url| !url.trim().is_empty()) - .unwrap_or(sev_snp_qvl::AMD_KDS_DEFAULT_BASE_URL), - )?, + amd_kds: amd_kds_client(&collateral_urls.amd_kds)?, }) } diff --git a/dstack/dstack-attest/src/collateral.rs b/dstack/dstack-attest/src/collateral.rs new file mode 100644 index 000000000..56fc199f0 --- /dev/null +++ b/dstack/dstack-attest/src/collateral.rs @@ -0,0 +1,102 @@ +// SPDX-FileCopyrightText: © 2024-2025 Phala Network +// +// SPDX-License-Identifier: Apache-2.0 + +//! Failover across interchangeable collateral endpoints. +//! +//! Attestation verification depends on collateral that only the vendor +//! publishes: Intel's PCCS and AMD's KDS. Both were single points of failure +//! for verification until now -- one URL each, and no answer meant no +//! verification. AMD KDS in particular is a single global endpoint with no +//! official mirror, and it has been unreachable for hours at a time. +//! +//! Nothing here relaxes verification. Collateral is signed by the vendor and +//! checked against roots compiled into the binary, so an endpoint can serve a +//! stale or absent answer but cannot serve a forged one; which endpoint +//! answered has no bearing on whether the signature checks pass. + +use anyhow::{bail, Context, Result}; +use dcap_qvl::{ + collateral::{CollateralClient, PHALA_PCCS_URL}, + verify::VerifiedReport, + QuoteCollateralV3, +}; +use dstack_types::UrlList; +use sev_snp_qvl::{AmdKdsClient, AMD_KDS_DEFAULT_BASE_URL}; + +/// Build an AMD KDS client over the configured endpoints, or the vendor +/// default when none are configured. +pub(crate) fn amd_kds_client(urls: &UrlList) -> Result { + if urls.is_empty() { + return AmdKdsClient::with_base_url(AMD_KDS_DEFAULT_BASE_URL); + } + AmdKdsClient::with_base_urls(urls.as_slice()) +} + +/// Intel PCCS access over one or more interchangeable endpoints. +/// +/// Endpoints are tried in order and the first success wins. +/// +/// Unlike the AMD KDS client, this fails over on *any* error rather than only +/// on ones that look transient. `dcap-qvl` returns `anyhow::Error`, so there is +/// no status code to classify on, and inventing a classifier by matching error +/// strings would be worse than the thing it replaces. The cost of the coarser +/// rule is bounded: a genuinely absent FMSPC costs one extra request per +/// configured endpoint, and configuring an endpoint list is opt-in. +#[derive(Clone)] +pub struct PccsClient { + clients: Vec<(String, CollateralClient)>, +} + +impl PccsClient { + pub fn new(urls: &UrlList) -> Result { + let urls: Vec<&str> = if urls.is_empty() { + vec![PHALA_PCCS_URL] + } else { + urls.as_slice().iter().map(String::as_str).collect() + }; + let clients = urls + .into_iter() + .map(|url| { + CollateralClient::with_default_http(url) + .with_context(|| format!("failed to create PCCS client for {url}")) + .map(|client| (url.to_string(), client)) + }) + .collect::>>()?; + if clients.is_empty() { + bail!("PCCS endpoint list is empty"); + } + Ok(Self { clients }) + } + + pub async fn fetch(&self, quote: &[u8]) -> Result { + self.failover("fetch", |client| client.fetch(quote)).await + } + + pub async fn fetch_and_verify(&self, quote: &[u8]) -> Result { + self.failover("fetch_and_verify", |client| client.fetch_and_verify(quote)) + .await + } + + async fn failover<'a, T, F, Fut>(&'a self, label: &str, mut call: F) -> Result + where + F: FnMut(&'a CollateralClient) -> Fut, + Fut: std::future::Future>, + { + let mut errors: Vec = Vec::new(); + for (url, client) in &self.clients { + match call(client).await { + Ok(value) => return Ok(value), + Err(err) => errors.push(format!("{url}: {err:#}")), + } + } + match errors.len() { + 0 => bail!("PCCS {label} had no endpoint to try"), + 1 => bail!("PCCS {label} failed: {}", errors[0]), + n => bail!( + "PCCS {label} failed on all {n} endpoints: {}", + errors.join("; ") + ), + } + } +} diff --git a/dstack/dstack-attest/src/lib.rs b/dstack/dstack-attest/src/lib.rs index 0739059b1..dcbf307e9 100644 --- a/dstack/dstack-attest/src/lib.rs +++ b/dstack/dstack-attest/src/lib.rs @@ -15,6 +15,7 @@ pub mod amd_sev_snp; pub mod attestation; #[cfg(feature = "quote")] mod aws_nitro_tpm; +pub mod collateral; #[cfg(feature = "quote")] mod sev_snp; pub mod trust_anchors; diff --git a/dstack/dstack-types/src/lib.rs b/dstack/dstack-types/src/lib.rs index 928ae674a..23ca347fd 100644 --- a/dstack/dstack-types/src/lib.rs +++ b/dstack/dstack-types/src/lib.rs @@ -1238,20 +1238,114 @@ pub struct GatewayClusterConfig { pub urls: Vec, } +/// One or more interchangeable endpoints for the same collateral service. +/// +/// Deserializes from either a bare string or a list, so every config file and +/// every already-serialized `SysConfig` written by an older host keeps parsing +/// unchanged. Serializes back as a bare string when there is exactly one entry, +/// for the same reason. +/// +/// Order is meaningful: callers try entries front to back and keep the first +/// answer. Blank entries are dropped on the way in, because a config that says +/// `pccs = ["", "https://..."]` means the second one. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct UrlList(Vec); + +impl UrlList { + pub fn new(urls: impl IntoIterator>) -> Self { + Self( + urls.into_iter() + .map(Into::into) + .map(|url| url.trim().to_string()) + .filter(|url| !url.is_empty()) + .collect(), + ) + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + pub fn len(&self) -> usize { + self.0.len() + } + + /// The endpoints in the order they should be tried. + pub fn as_slice(&self) -> &[String] { + &self.0 + } + + pub fn iter(&self) -> std::slice::Iter<'_, String> { + self.0.iter() + } + + /// The endpoint a single-URL caller should use. `None` when the list is + /// empty, which callers read as "use the platform default". + pub fn first(&self) -> Option<&str> { + self.0.first().map(String::as_str) + } +} + +impl From for UrlList { + fn from(url: String) -> Self { + Self::new([url]) + } +} + +impl From<&str> for UrlList { + fn from(url: &str) -> Self { + Self::new([url]) + } +} + +impl From> for UrlList { + fn from(urls: Vec) -> Self { + Self::new(urls) + } +} + +impl Serialize for UrlList { + fn serialize(&self, serializer: S) -> Result { + match self.0.as_slice() { + [single] => serializer.serialize_str(single), + many => many.serialize(serializer), + } + } +} + +impl<'de> Deserialize<'de> for UrlList { + fn deserialize>(deserializer: D) -> Result { + #[derive(Deserialize)] + #[serde(untagged)] + enum OneOrMany { + One(String), + Many(Vec), + } + Ok(match OneOrMany::deserialize(deserializer)? { + OneOrMany::One(url) => Self::new([url]), + OneOrMany::Many(urls) => Self::new(urls), + }) + } +} + #[derive(Deserialize, Serialize, Debug, Clone, Default, PartialEq, Eq)] #[serde(deny_unknown_fields)] pub struct CollateralUrls { - #[serde(default, skip_serializing_if = "Option::is_none")] - pub pccs: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub amd_kds: Option, + /// Intel PCCS endpoints. Empty means the built-in default. + #[serde(default, skip_serializing_if = "UrlList::is_empty")] + pub pccs: UrlList, + /// AMD KDS endpoints, or mirrors of it. Empty means the built-in default. + #[serde(default, skip_serializing_if = "UrlList::is_empty")] + pub amd_kds: UrlList, } impl SysConfig { pub fn collateral_urls(&self) -> CollateralUrls { let mut urls = self.collateral_urls.clone().unwrap_or_default(); - if urls.pccs.is_none() { - urls.pccs.clone_from(&self.legacy_pccs_url); + if urls.pccs.is_empty() { + if let Some(legacy) = &self.legacy_pccs_url { + urls.pccs = UrlList::from(legacy.clone()); + } } urls } @@ -2682,3 +2776,88 @@ mod appcompose_sdk_parity { ); } } + +#[cfg(test)] +mod collateral_url_tests { + use super::*; + + #[test] + fn a_bare_string_and_a_list_both_parse() { + let single: CollateralUrls = + serde_json::from_str(r#"{"pccs":"https://pccs.example"}"#).unwrap(); + assert_eq!(single.pccs.as_slice(), ["https://pccs.example"]); + + let many: CollateralUrls = + serde_json::from_str(r#"{"amd_kds":["https://a/vcek/v1","https://b/vcek/v1"]}"#) + .unwrap(); + assert_eq!( + many.amd_kds.as_slice(), + ["https://a/vcek/v1", "https://b/vcek/v1"] + ); + } + + /// A `SysConfig` written by a host that predates lists must still round + /// trip through a new binary unchanged, because the guest hashes what it + /// is given rather than what it would have written itself. + #[test] + fn a_single_url_serializes_back_as_a_bare_string() { + let urls = CollateralUrls { + pccs: UrlList::from("https://pccs.example"), + amd_kds: UrlList::default(), + }; + assert_eq!( + serde_json::to_string(&urls).unwrap(), + r#"{"pccs":"https://pccs.example"}"# + ); + } + + #[test] + fn several_urls_serialize_as_a_list() { + let urls = CollateralUrls { + pccs: UrlList::default(), + amd_kds: UrlList::new(["https://a", "https://b"]), + }; + assert_eq!( + serde_json::to_string(&urls).unwrap(), + r#"{"amd_kds":["https://a","https://b"]}"# + ); + } + + #[test] + fn blank_entries_are_dropped_on_the_way_in() { + let urls: CollateralUrls = + serde_json::from_str(r#"{"pccs":["", " ", "https://real"]}"#).unwrap(); + assert_eq!(urls.pccs.as_slice(), ["https://real"]); + + let blank: CollateralUrls = serde_json::from_str(r#"{"pccs":" "}"#).unwrap(); + assert!( + blank.pccs.is_empty(), + "a blank string means 'unset', not 'an endpoint named blank'" + ); + } + + #[test] + fn the_legacy_pccs_url_field_still_wins_when_no_list_is_configured() { + let sys_config: SysConfig = serde_json::from_str( + r#"{"kms_urls":[],"gateway_urls":[],"pccs_url":"https://legacy.example","vm_config":"{}"}"#, + ) + .unwrap(); + assert_eq!( + sys_config.collateral_urls().pccs.as_slice(), + ["https://legacy.example"] + ); + } + + #[test] + fn an_explicit_list_takes_precedence_over_the_legacy_field() { + let sys_config: SysConfig = serde_json::from_str( + r#"{"kms_urls":[],"gateway_urls":[],"pccs_url":"https://legacy.example", + "collateral_urls":{"pccs":["https://new.example"]},"vm_config":"{}"}"#, + ) + .unwrap(); + assert_eq!( + sys_config.collateral_urls().pccs.as_slice(), + ["https://new.example"] + ); + } +} diff --git a/dstack/dstack-util/src/host_api.rs b/dstack/dstack-util/src/host_api.rs index e038ea3b2..bdf62bda0 100644 --- a/dstack/dstack-util/src/host_api.rs +++ b/dstack/dstack-util/src/host_api.rs @@ -4,10 +4,10 @@ use crate::utils::{deserialize_json_file, sha256, SysConfig}; use anyhow::{anyhow, bail, Context, Result}; -use dcap_qvl::collateral::{CollateralClient, PHALA_PCCS_URL}; +use dstack_attest::collateral::PccsClient; use dstack_types::{ shared_filenames::{HOST_SHARED_DIR, SYS_CONFIG}, - Platform, + Platform, UrlList, }; use host_api::{ client::{new_client, DefaultClient}, @@ -27,26 +27,26 @@ pub(crate) struct KeyProvision { pub(crate) struct HostApi { client: Option, - pccs_url: Option, + pccs_urls: UrlList, } impl Default for HostApi { fn default() -> Self { - Self::new(None, None) + Self::new(None, UrlList::default()) } } impl HostApi { - pub fn new(base_url: Option, pccs_url: Option) -> Self { + pub fn new(base_url: Option, pccs_urls: UrlList) -> Self { Self { client: base_url.map(new_client), - pccs_url, + pccs_urls, } } pub fn load_or_default(url: Option) -> Result { let api = match url { - Some(url) => Self::new(Some(url), None), + Some(url) => Self::new(Some(url), UrlList::default()), None => { let local_config: SysConfig = deserialize_json_file(format!("{HOST_SHARED_DIR}/{SYS_CONFIG}"))?; @@ -105,13 +105,7 @@ impl HostApi { .map_err(|err| anyhow!("Failed to get sealing key: {err:?}"))?; // verify the key provider quote - let pccs_url = self - .pccs_url - .as_deref() - .map(str::trim) - .filter(|url| !url.is_empty()) - .unwrap_or(PHALA_PCCS_URL); - let collateral_client = CollateralClient::with_default_http(pccs_url)?; + let collateral_client = PccsClient::new(&self.pccs_urls)?; let verified_report = tokio::time::timeout( PCCS_TIMEOUT, collateral_client.fetch_and_verify(&provision.provider_quote), diff --git a/dstack/gateway/gateway.toml b/dstack/gateway/gateway.toml index 4f963b4ff..1cae608e5 100644 --- a/dstack/gateway/gateway.toml +++ b/dstack/gateway/gateway.toml @@ -21,8 +21,15 @@ rpc_domain = "" insecure_allow_external_trust_anchors = false [core.attestation.urls] +# Each accepts a single URL or a list of interchangeable ones. With a list, +# endpoints are tried in order and the first answer wins, so put the endpoint +# you would rather load first. Failing over changes nothing about verification: +# collateral is vendor-signed and checked against roots built into the binary, +# so an endpoint can be slow, stale or absent but cannot be forged. # pccs = "https://pccs.phala.network" +# pccs = ["https://pccs.internal", "https://pccs.phala.network"] # amd_kds = "https://kdsintf.amd.com/vcek/v1" +# amd_kds = ["https://kds-cache.internal/vcek/v1", "https://kdsintf.amd.com/vcek/v1"] [core.attestation.root_ca] # tdx = "/etc/dstack/roots/intel-sgx-root-ca.der" diff --git a/dstack/kms/kms.toml b/dstack/kms/kms.toml index 5beb28d80..7f70a0b89 100644 --- a/dstack/kms/kms.toml +++ b/dstack/kms/kms.toml @@ -53,8 +53,15 @@ aws_nitro_tpm_key_release = false insecure_allow_external_trust_anchors = false [core.attestation.urls] +# Each accepts a single URL or a list of interchangeable ones. With a list, +# endpoints are tried in order and the first answer wins, so put the endpoint +# you would rather load first. Failing over changes nothing about verification: +# collateral is vendor-signed and checked against roots built into the binary, +# so an endpoint can be slow, stale or absent but cannot be forged. # pccs = "https://pccs.phala.network" +# pccs = ["https://pccs.internal", "https://pccs.phala.network"] # amd_kds = "https://kdsintf.amd.com/vcek/v1" +# amd_kds = ["https://kds-cache.internal/vcek/v1", "https://kdsintf.amd.com/vcek/v1"] [core.attestation.root_ca] # tdx = "/etc/dstack/roots/intel-sgx-root-ca.der" diff --git a/dstack/sev-snp-qvl/Cargo.toml b/dstack/sev-snp-qvl/Cargo.toml index 4d16e4500..83df86475 100644 --- a/dstack/sev-snp-qvl/Cargo.toml +++ b/dstack/sev-snp-qvl/Cargo.toml @@ -21,3 +21,6 @@ x509-parser = { workspace = true, features = ["verify"] } rustls-pki-types.workspace = true rustls-webpki = { workspace = true, features = ["ring"] } pem.workspace = true + +[dev-dependencies] +tokio = { workspace = true, features = ["rt", "time", "macros", "net", "io-util"] } diff --git a/dstack/sev-snp-qvl/src/failover_tests.rs b/dstack/sev-snp-qvl/src/failover_tests.rs new file mode 100644 index 000000000..459fecfa8 --- /dev/null +++ b/dstack/sev-snp-qvl/src/failover_tests.rs @@ -0,0 +1,140 @@ +// SPDX-FileCopyrightText: © 2025 Phala Network +// +// SPDX-License-Identifier: Apache-2.0 + +//! Failover behaviour of [`AmdKdsClient`] against real sockets. +//! +//! These cover the part that matters operationally and cannot be established +//! by reading the code: that a rate-limited, broken or absent endpoint really +//! does hand off to the next one, and that a decisive answer does not. + +use std::sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, +}; + +use crate::{AmdKdsClient, AmdSnpProduct}; +use tokio::{ + io::{AsyncReadExt, AsyncWriteExt}, + net::TcpListener, +}; + +/// A single-purpose HTTP origin that always answers the same way and counts +/// the requests it saw. +struct FakeKds { + base_url: String, + hits: Arc, +} + +impl FakeKds { + async fn serving(status_line: &'static str, body: &'static str) -> Self { + let listener = TcpListener::bind("127.0.0.1:0").await.expect("bind"); + let port = listener.local_addr().expect("addr").port(); + let hits = Arc::new(AtomicUsize::new(0)); + let counter = hits.clone(); + tokio::spawn(async move { + while let Ok((mut socket, _)) = listener.accept().await { + counter.fetch_add(1, Ordering::SeqCst); + let mut discard = [0u8; 1024]; + let _ = socket.read(&mut discard).await; + let response = format!( + "HTTP/1.1 {status_line}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + let _ = socket.write_all(response.as_bytes()).await; + let _ = socket.shutdown().await; + } + }); + Self { + base_url: format!("http://127.0.0.1:{port}/vcek/v1"), + hits, + } + } + + fn hits(&self) -> usize { + self.hits.load(Ordering::SeqCst) + } +} + +const BODY: &str = "collateral-bytes"; + +fn cert_chain_path() -> String { + format!("{}/cert_chain", AmdSnpProduct::Milan.kds_name()) +} + +#[tokio::test] +async fn a_rate_limited_endpoint_hands_off_to_the_next_one() { + let rate_limited = FakeKds::serving("429 Too Many Requests", "slow down").await; + let healthy = FakeKds::serving("200 OK", BODY).await; + + let client = AmdKdsClient::with_base_urls([&rate_limited.base_url, &healthy.base_url]).unwrap(); + let bytes = client + .fetch_path(&cert_chain_path(), "cert_chain") + .await + .expect("the healthy endpoint should have answered"); + + assert_eq!(bytes, BODY.as_bytes()); + assert_eq!(rate_limited.hits(), 1, "the first endpoint should be tried"); + assert_eq!(healthy.hits(), 1, "and the second should serve the answer"); +} + +#[tokio::test] +async fn a_refused_connection_hands_off_to_the_next_one() { + // Nothing listens on port 1, so the connection is refused outright. + let dead = "http://127.0.0.1:1/vcek/v1".to_string(); + let healthy = FakeKds::serving("200 OK", BODY).await; + + let client = AmdKdsClient::with_base_urls([&dead, &healthy.base_url]).unwrap(); + let bytes = client + .fetch_path(&cert_chain_path(), "cert_chain") + .await + .expect("a refused connection should fail over"); + + assert_eq!(bytes, BODY.as_bytes()); + assert_eq!(healthy.hits(), 1); +} + +/// A 404 is an answer about the chip, not about the endpoint. Every mirror +/// repeats it, so failing over would multiply load and bury the real error +/// under a list of identical ones. +#[tokio::test] +async fn a_not_found_stops_rather_than_asking_everyone_else() { + let not_found = FakeKds::serving("404 Not Found", "no such chip").await; + let healthy = FakeKds::serving("200 OK", BODY).await; + + let client = AmdKdsClient::with_base_urls([¬_found.base_url, &healthy.base_url]).unwrap(); + let err = client + .fetch_path(&cert_chain_path(), "cert_chain") + .await + .expect_err("a 404 should surface, not be retried away"); + + assert!( + format!("{err:#}").contains("404"), + "the original status should survive: {err:#}" + ); + assert_eq!(not_found.hits(), 1); + assert_eq!( + healthy.hits(), + 0, + "the second endpoint should never have been contacted" + ); +} + +#[tokio::test] +async fn every_endpoint_failing_reports_every_endpoint() { + let first = FakeKds::serving("503 Service Unavailable", "down").await; + let second = FakeKds::serving("500 Internal Server Error", "boom").await; + + let client = AmdKdsClient::with_base_urls([&first.base_url, &second.base_url]).unwrap(); + let err = client + .fetch_path(&cert_chain_path(), "cert_chain") + .await + .expect_err("both endpoints failed"); + + let rendered = format!("{err:#}"); + assert!(rendered.contains("2 endpoints"), "{rendered}"); + assert!(rendered.contains("503"), "{rendered}"); + assert!(rendered.contains("500"), "{rendered}"); + assert_eq!(first.hits(), 1); + assert_eq!(second.hits(), 1); +} diff --git a/dstack/sev-snp-qvl/src/lib.rs b/dstack/sev-snp-qvl/src/lib.rs index 1fa4da6f5..706c1c056 100644 --- a/dstack/sev-snp-qvl/src/lib.rs +++ b/dstack/sev-snp-qvl/src/lib.rs @@ -302,7 +302,7 @@ struct AmdKdsVcekCacheKey { #[derive(Clone)] pub struct AmdKdsClient { - base_url: String, + base_urls: Vec, http_client: reqwest::Client, ca_cache: Cache, vcek_cache: Cache, @@ -314,14 +314,32 @@ impl AmdKdsClient { } pub fn with_base_url(base_url: impl AsRef) -> Result { - let base_url = normalize_amd_kds_base_url(base_url.as_ref())?; + Self::with_base_urls([base_url.as_ref()]) + } + + /// Build a client over several interchangeable endpoints -- KDS itself, a + /// caching mirror of it, or both. + /// + /// Endpoints are tried in order and the first answer wins, so put the one + /// you would rather load first. Responses are cached by + /// `(product, chip_id, TCB)` rather than by URL, because the collateral is + /// a property of the chip and not of where it was fetched from: a hit + /// costs nothing regardless of which endpoint filled it. + pub fn with_base_urls(base_urls: impl IntoIterator>) -> Result { + let base_urls = base_urls + .into_iter() + .map(|url| normalize_amd_kds_base_url(url.as_ref())) + .collect::>>()?; + if base_urls.is_empty() { + bail!("amd sev-snp KDS base URL list is empty"); + } let http_client = reqwest::Client::builder() .connect_timeout(AMD_KDS_CONNECT_TIMEOUT) .timeout(AMD_KDS_REQUEST_TIMEOUT) .build() .context("failed to create amd sev-snp KDS HTTP client")?; Ok(Self { - base_url, + base_urls, http_client, ca_cache: Cache::new(AMD_KDS_CA_CACHE_CAPACITY), vcek_cache: Cache::new(AMD_KDS_VCEK_CACHE_CAPACITY), @@ -354,11 +372,8 @@ impl AmdKdsClient { return Ok(cached); } - let url = join_amd_kds_url( - &self.base_url, - &format!("{}/cert_chain", product.kds_name()), - ); - let chain = self.fetch_url(&url, "cert_chain").await?; + let path = format!("{}/cert_chain", product.kds_name()); + let chain = self.fetch_path(&path, "cert_chain").await?; let (_fetched_ark, ask) = extract_ark_ask_from_amd_kds_cert_chain(&chain)?; let collateral = (product.builtin_ark(), ask); self.ca_cache.insert(key, collateral.clone()); @@ -380,28 +395,83 @@ impl AmdKdsClient { return Ok(cached); } - let vcek_url = amd_kds_vcek_url_with_base(&self.base_url, product, chip_id, tcb)?; + let vcek_path = amd_kds_vcek_path(product, chip_id, tcb)?; let vcek = CertBytes { - bytes: self.fetch_url(&vcek_url, "vcek").await?, + bytes: self.fetch_path(&vcek_path, "vcek").await?, encoding: CertEncoding::Der, }; self.vcek_cache.insert(key, vcek.clone()); Ok(vcek) } - async fn fetch_url(&self, url: &str, label: &str) -> Result> { - Ok(self + /// Try each endpoint in turn, stopping at the first success. + /// + /// Only *retryable* failures move on to the next endpoint. A 404 is a + /// deterministic answer about this chip and this TCB -- every mirror of KDS + /// will say the same thing -- so retrying it just multiplies load and + /// replaces a clear error with a confusing one. Transport failures, 408, + /// 429 and 5xx are the opposite: they say nothing about the request, only + /// about the endpoint, which is exactly what a second endpoint can fix. + async fn fetch_path(&self, path: &str, label: &str) -> Result> { + let mut errors: Vec = Vec::new(); + for base_url in &self.base_urls { + let url = join_amd_kds_url(base_url, path); + match self.fetch_one(&url, label).await { + Ok(bytes) => return Ok(bytes), + Err(err) => { + let retryable = err.retryable; + errors.push(err.error); + if !retryable { + break; + } + } + } + } + match errors.len() { + // The constructor rejects an empty endpoint list, so the loop ran. + 0 => bail!("amd sev-snp {label} request had no endpoint to try"), + // One endpoint tried, or the first answer was decisive: report it + // as-is rather than wrapping a single error in list phrasing. + 1 => Err(errors.pop().expect("length checked")), + n => { + let joined = errors + .iter() + .map(|err| format!("{err:#}")) + .collect::>() + .join("; "); + bail!("amd sev-snp {label} request failed on {n} endpoints: {joined}") + } + } + } + + async fn fetch_one(&self, url: &str, label: &str) -> Result, AmdKdsFetchError> { + let response = self .http_client .get(url) .send() .await - .with_context(|| format!("failed to request amd sev-snp {label} from {url}"))? + .map_err(|err| AmdKdsFetchError { + retryable: true, + error: anyhow::Error::new(err) + .context(format!("failed to request amd sev-snp {label} from {url}")), + })?; + let status = response.status(); + let response = response .error_for_status() - .with_context(|| format!("amd sev-snp {label} request failed for {url}"))? + .map_err(|err| AmdKdsFetchError { + retryable: status_is_retryable(status), + error: anyhow::Error::new(err) + .context(format!("amd sev-snp {label} request failed for {url}")), + })?; + response .bytes() .await - .with_context(|| format!("failed to read amd sev-snp {label} response"))? - .to_vec()) + .map(|bytes| bytes.to_vec()) + .map_err(|err| AmdKdsFetchError { + retryable: true, + error: anyhow::Error::new(err) + .context(format!("failed to read amd sev-snp {label} response")), + }) } } @@ -735,8 +805,7 @@ fn amd_snp_product_from_report(report: &AttestationReport) -> Result join_amd_kds_url( - base_url, - &format!( - "{}/{}?blSPL={}&teeSPL={}&snpSPL={}&ucodeSPL={}", + format!( + "{}/{}?fmcSPL={}&blSPL={}&teeSPL={}&snpSPL={}&ucodeSPL={}", product.kds_name(), - hex::encode(chip_id), + hex::encode(&chip_id[..8]), + fmc, tcb.bootloader, tcb.tee, tcb.snp, tcb.microcode - ), + ) + } + AmdSnpProduct::Milan | AmdSnpProduct::Genoa => format!( + "{}/{}?blSPL={}&teeSPL={}&snpSPL={}&ucodeSPL={}", + product.kds_name(), + hex::encode(chip_id), + tcb.bootloader, + tcb.tee, + tcb.snp, + tcb.microcode ), }; Ok(url) } +/// A KDS fetch failure, tagged with whether another endpoint could plausibly +/// answer differently. +struct AmdKdsFetchError { + retryable: bool, + error: anyhow::Error, +} + +fn status_is_retryable(status: reqwest::StatusCode) -> bool { + status.is_server_error() + || status == reqwest::StatusCode::REQUEST_TIMEOUT + || status == reqwest::StatusCode::TOO_MANY_REQUESTS +} + fn extract_ark_ask_from_amd_kds_cert_chain(chain: &[u8]) -> Result<(CertBytes, CertBytes)> { let certs = extract_pem_certs(chain)?; if certs.len() < 2 { @@ -951,6 +1027,9 @@ fn parse_kernel_cert_table(auxblob: &[u8]) -> Result)>> { Ok(entries) } +#[cfg(test)] +mod failover_tests; + #[cfg(test)] mod tests { use super::*; @@ -1000,6 +1079,62 @@ mod tests { ); } + #[test] + fn kds_endpoints_are_normalized_and_ordered() { + let client = AmdKdsClient::with_base_urls([ + "https://mirror.example/vcek/v1/", + " https://kdsintf.amd.com/vcek/v1 ", + ]) + .unwrap(); + assert_eq!( + client.base_urls, + vec![ + "https://mirror.example/vcek/v1".to_string(), + "https://kdsintf.amd.com/vcek/v1".to_string(), + ] + ); + } + + #[test] + fn an_empty_endpoint_list_is_rejected_rather_than_silently_defaulted() { + let err = match AmdKdsClient::with_base_urls(Vec::::new()) { + Ok(_) => panic!("an empty endpoint list should not build a client"), + Err(err) => err, + }; + assert!( + err.to_string().contains("empty"), + "unexpected error: {err:#}" + ); + // A single blank string is the same mistake wearing a different hat. + assert!(AmdKdsClient::with_base_url(" ").is_err()); + } + + /// A 404 is an answer about the chip, not about the endpoint: every mirror + /// of KDS will repeat it, so trying the next one only multiplies load. + #[test] + fn only_endpoint_shaped_failures_are_retried_elsewhere() { + use reqwest::StatusCode; + for status in [ + StatusCode::TOO_MANY_REQUESTS, + StatusCode::REQUEST_TIMEOUT, + StatusCode::INTERNAL_SERVER_ERROR, + StatusCode::BAD_GATEWAY, + StatusCode::SERVICE_UNAVAILABLE, + ] { + assert!(status_is_retryable(status), "{status} should fail over"); + } + for status in [ + StatusCode::NOT_FOUND, + StatusCode::BAD_REQUEST, + StatusCode::FORBIDDEN, + ] { + assert!( + !status_is_retryable(status), + "{status} should not fail over" + ); + } + } + #[test] fn amd_kds_vcek_url_binds_chip_id_and_reported_tcb() { let chip_id = [0xab; 64]; @@ -1011,13 +1146,10 @@ mod tests { microcode: 4, }; - let url = amd_kds_vcek_url_with_base( + let url = join_amd_kds_url( AMD_KDS_DEFAULT_BASE_URL, - AmdSnpProduct::Genoa, - &chip_id, - tcb, - ) - .unwrap(); + &amd_kds_vcek_path(AmdSnpProduct::Genoa, &chip_id, tcb).unwrap(), + ); assert_eq!( url, @@ -1039,13 +1171,10 @@ mod tests { microcode: 4, }; - let url = amd_kds_vcek_url_with_base( + let url = join_amd_kds_url( AMD_KDS_DEFAULT_BASE_URL, - AmdSnpProduct::Turin, - &chip_id, - tcb, - ) - .unwrap(); + &amd_kds_vcek_path(AmdSnpProduct::Turin, &chip_id, tcb).unwrap(), + ); assert_eq!( url, diff --git a/dstack/verifier/dstack-verifier.toml b/dstack/verifier/dstack-verifier.toml index bdb09fe92..565d87df5 100644 --- a/dstack/verifier/dstack-verifier.toml +++ b/dstack/verifier/dstack-verifier.toml @@ -19,8 +19,15 @@ image_download_timeout_secs = 300 insecure_allow_external_trust_anchors = false [attestation.urls] +# Each accepts a single URL or a list of interchangeable ones. With a list, +# endpoints are tried in order and the first answer wins, so put the endpoint +# you would rather load first. Failing over changes nothing about verification: +# collateral is vendor-signed and checked against roots built into the binary, +# so an endpoint can be slow, stale or absent but cannot be forged. # pccs = "https://pccs.phala.network" +# pccs = ["https://pccs.internal", "https://pccs.phala.network"] # amd_kds = "https://kdsintf.amd.com/vcek/v1" +# amd_kds = ["https://kds-cache.internal/vcek/v1", "https://kdsintf.amd.com/vcek/v1"] [attestation.root_ca] # tdx = "/etc/dstack/roots/intel-sgx-root-ca.der"