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"