diff --git a/CHANGELOG.md b/CHANGELOG.md index f20ea138a..38ea90f8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - sdk: the Go and Python compose-hash helpers silently dropped every app-compose field they did not declare, so `getComposeHash` returned a digest for an app-compose that was not the one being deployed — and that digest is what gets whitelisted on chain. The missing fields are named above; both now keep unrecognised keys as well, so a guest that gains a field before the SDK does still hashes correctly ### Changed +- kms: client certificates are authenticated by the attestation they carry rather than by their issuer. Rocket configures mutual TLS through rustls' `WebPkiClientVerifier`, which pins a CA — but an RA-TLS certificate is self-issued and carries its identity in a TEE quote, so there is nothing to chain to. `GetTempCaCert` bridged the gap by handing every caller a shared CA private key purely so the minted certificate would chain somewhere; the CA established nothing (its key is public by design, and the endpoint is unauthenticated) and the check that has always carried the meaning is the quote verification that runs afterwards. The KMS now hands rustls a verifier that requires an attestation and ignores the issuer. Nothing changes for callers: guests and KMS-to-KMS onboarding still mint their client certificates from the temp CA, and those are now accepted for the attestation they carry. What changes is that the TLS layer went from admitting any certificate signed by a public key to requiring an attested one, and that a self-issued certificate is now accepted — which is what lets callers be migrated off `GetTempCaCert` in a follow-up. `[rpc.tls.mutual]` is no longer the trust anchor and is dropped from `kms.toml` and the KMS config templates; leaving it in an existing deployment's config is inert. The gateway's `[tls.mutual]` is unaffected — it pins the KMS root CA, which is a real trust anchor - guest-agent: the `/metrics` exposition gains a conventions-compliant `dstack_guest_*` series set (application prefix; `_bytes`/`_seconds` unit suffixes; no `_total` suffix on gauges, which reads as a counter to every tool; the four OS/kernel/CPU gauges folded into one `dstack_guest_info`; `disk_used_ratio` 0–1 instead of a percentage). The old `system_*`/`disk_*` names are still emitted verbatim, marked deprecated, and will be removed in a future release — this endpoint is tenant-facing, so existing dashboards get a migration window - dstack-util: a CVM re-registers with the gateway node that last accepted it, before falling back to the configured order. The list used to be walked from the top every time, so every CVM piled onto the first URL and the whole fleet snapped back to it the moment it recovered from an outage — and each move rewrites the instance record from a different node's memory - vmm: optionally randomize the KMS and gateway URL orders written to each CVM's system configuration so new CVMs distribute their initial requests across service nodes; both are enabled by default in `vmm.toml` diff --git a/docs/security/public-security-reports.md b/docs/security/public-security-reports.md index bf935c4de..d04b40628 100644 --- a/docs/security/public-security-reports.md +++ b/docs/security/public-security-reports.md @@ -37,7 +37,7 @@ These issues were filed as concrete vulnerability reports, security audit findin | [#558](https://github.com/Dstack-TEE/dstack/issues/558) Path traversal in KMS `remove_cache` | Closed | Valid report, fixed | [#601](https://github.com/Dstack-TEE/dstack/pull/601) validates cache paths before deletion | | [#559](https://github.com/Dstack-TEE/dstack/issues/559) Zero `mr_config_id` bypasses verification and weakens `mr_aggregated` identity | Closed | Not a production vulnerability | Zero `mr_config_id` remains an unset-value compatibility case, and configuration changes are still reflected through RTMR-based measurements. No code fix was applied | | [#560](https://github.com/Dstack-TEE/dstack/issues/560) Admin token comparison not constant-time | Closed | Not a production vulnerability | The comparison is over a SHA-256 digest of a high-entropy token, not the raw token. No code fix was applied | -| [#561](https://github.com/Dstack-TEE/dstack/issues/561) KMS TLS client certificates are non-mandatory in Rocket config | Closed | Valid report, documented | The TLS listener allows unauthenticated bootstrap, temp-CA bootstrap, and public endpoints. `GetTempCaCert` returns temp CA private material for bootstrap. App/KMS key release requires verified caller attestation, and certificate signing verifies the CSR signature and embedded attestation. No code fix was applied | +| [#561](https://github.com/Dstack-TEE/dstack/issues/561) KMS TLS client certificates are non-mandatory in Rocket config | Closed | Valid report, fixed | [#1106](https://github.com/Dstack-TEE/dstack/pull/1106) makes the KMS verify client certificates by the attestation they carry instead of by an issuer CA, so a presented certificate must now be attested at the handshake. The listener still allows unauthenticated bootstrap and public endpoints. App/KMS key release requires verified caller attestation, and certificate signing verifies the CSR signature and embedded attestation | | [#562](https://github.com/Dstack-TEE/dstack/issues/562) Configfs path overridable through an environment variable | Closed | Not a production vulnerability | A process that can choose its own quote path is already inside the measured CVM behavior. No code fix has been applied. A production guard for `DCAP_TDX_QUOTE_CONFIGFS_PATH` remains possible hardening | | [#563](https://github.com/Dstack-TEE/dstack/issues/563) `simulate_quote` runtime path in production guest agent | Closed | Valid report, fixed | [#582](https://github.com/Dstack-TEE/dstack/pull/582) isolates the simulator into a dedicated binary | | [#564](https://github.com/Dstack-TEE/dstack/issues/564) `GetAppEnvEncryptPubKey` unauthenticated app ID enumeration | Closed | Not a production vulnerability | The RPC returns a public encryption key before an app has an attested identity, and `app_id` is not treated as secret. No code fix was applied | @@ -59,7 +59,7 @@ These issues were filed as concrete vulnerability reports, security audit findin | [#616](https://github.com/Dstack-TEE/dstack/issues/616) Host-controlled Docker registry mirror enables image substitution attacks | Closed | Not a production vulnerability | Registry mirrors are untrusted transport. Digest-pinned image references and measured compose configuration protect against substitution. No code fix was applied | | [#617](https://github.com/Dstack-TEE/dstack/issues/617) Guest agent exposes raw private keys to all local processes | Closed | Not a production vulnerability | dstack treats a CVM as one application trust domain. It does not provide per-container key isolation inside the same measured application. No code fix was applied | | [#618](https://github.com/Dstack-TEE/dstack/issues/618) Disk encryption disableable via kernel cmdline, not measured in RTMR | Closed | Not a production vulnerability | The kernel command line is measured into RTMR2, so changing `dstack.storage_encrypted=false` changes attestation evidence. No code fix was applied | -| [#619](https://github.com/Dstack-TEE/dstack/issues/619) KMS `get_temp_ca_cert` returns temp CA private key without authentication | Closed | Duplicate | The report duplicates the private advisory response for the temp CA bootstrap flow | +| [#619](https://github.com/Dstack-TEE/dstack/issues/619) KMS `get_temp_ca_cert` returns temp CA private key without authentication | Closed | Duplicate | The report duplicates the private advisory response for the temp CA bootstrap flow. [#1106](https://github.com/Dstack-TEE/dstack/pull/1106) removes the reason the temp CA existed - the KMS no longer pins it - so callers can be migrated off it; the RPC itself is retained until guests and KMS-to-KMS onboarding are | ## Related security roadmap and hardening diff --git a/docs/security/security-best-practices.md b/docs/security/security-best-practices.md index c9afbf23e..91b3c0c2e 100644 --- a/docs/security/security-best-practices.md +++ b/docs/security/security-best-practices.md @@ -111,7 +111,7 @@ Development settings are intentionally easy to audit, but they are not productio - The KMS contract pins a concrete gateway app id. Do not use `gateway_app_id = "any"` for production traffic. - TEE quotes are evaluated by deployment policy, including TCB status and expected OS/application measurements. -The KMS TLS listener may keep `rpc.tls.mutual.mandatory = false` because bootstrap, temp-CA bootstrap, and public metadata endpoints need to be reachable before a client has an RA-TLS certificate. `GetTempCaCert` returns temp CA private material for the bootstrap flow; treat it as bootstrap-sensitive. +The KMS TLS listener verifies client certificates by the attestation they carry rather than by an issuer CA, so it needs no `rpc.tls.mutual` section. It still accepts connections without a client certificate, because bootstrap and public metadata endpoints must be reachable before a client has an RA-TLS certificate. `GetTempCaCert` remains in use by guests and by KMS-to-KMS onboarding, which still mint their client certificates from that CA; it returns temp CA private material, so treat it as bootstrap-sensitive. App key release and KMS key handover still require verified caller attestation from the RA-TLS client certificate. Certificate signing verifies the CSR signature and embedded attestation before signing. diff --git a/docs/security/security-model.md b/docs/security/security-model.md index 4f48fe824..72791944e 100644 --- a/docs/security/security-model.md +++ b/docs/security/security-model.md @@ -337,11 +337,11 @@ Production verifiers should reject deployments that use these development settin ### KMS mTLS is route-enforced for sensitive operations -The KMS Rocket TLS listener permits connections without a client certificate because some bootstrap and public metadata endpoints must be reachable before a client has an RA-TLS certificate. That listener setting is not the authorization boundary for key material. +The KMS Rocket TLS listener permits connections without a client certificate because some bootstrap and public metadata endpoints must be reachable before a client has an RA-TLS certificate. A certificate that is presented must carry an attestation, but the issuer that signed it is not checked and is not the authorization boundary for key material. App key release and KMS key handover require verified caller attestation from the RA-TLS client certificate. Certificate signing verifies the CSR signature and the attestation embedded in the CSR before signing. -The unauthenticated or non-client-certificate surface includes bootstrap and temp-CA bootstrap material retrieval, env-encryption public-key retrieval, metadata, health, and metrics behavior documented for operators. `GetTempCaCert` returns temp CA private material for the bootstrap flow, so operators must treat it as bootstrap-sensitive rather than harmless public metadata. +The unauthenticated or non-client-certificate surface includes bootstrap and temp-CA bootstrap material retrieval, env-encryption public-key retrieval, metadata, health, and metrics behavior documented for operators. `GetTempCaCert` returns temp CA private material and remains in use by guests and by KMS-to-KMS onboarding, which mint their client certificates from that CA; operators must treat it as bootstrap-sensitive rather than harmless public metadata. ## Limitations diff --git a/docs/tutorials/kms-build-configuration.md b/docs/tutorials/kms-build-configuration.md index 1315d3f22..f8264e3a2 100644 --- a/docs/tutorials/kms-build-configuration.md +++ b/docs/tutorials/kms-build-configuration.md @@ -183,14 +183,11 @@ port = 9100 key = "/etc/kms/certs/rpc.key" certs = "/etc/kms/certs/rpc.crt" -# Mutual TLS (mTLS) Configuration -[rpc.tls.mutual] -ca_certs = "/etc/kms/certs/tmp-ca.crt" -# Keep the TLS listener optional because bootstrap/public endpoints must be -# reachable before a client has an RA-TLS certificate. Temp-CA bootstrap material -# is bootstrap-sensitive. Key-release RPCs still require verified caller +# No mutual-TLS section: client certificates are verified by the attestation they +# carry, not by an issuer CA, so there is nothing to pin. Connections without a +# certificate are still accepted, because bootstrap and public endpoints must be +# reachable before a client has one. Key-release RPCs still require verified caller # attestation; certificate signing verifies CSR signature and attestation. -mandatory = false # Core KMS Configuration [core] @@ -457,14 +454,11 @@ port = 9100 key = "/etc/kms/certs/rpc.key" certs = "/etc/kms/certs/rpc.crt" -# Mutual TLS (mTLS) Configuration -[rpc.tls.mutual] -ca_certs = "/etc/kms/certs/tmp-ca.crt" -# Keep the TLS listener optional because bootstrap/public endpoints must be -# reachable before a client has an RA-TLS certificate. Temp-CA bootstrap material -# is bootstrap-sensitive. Key-release RPCs still require verified caller +# No mutual-TLS section: client certificates are verified by the attestation they +# carry, not by an issuer CA, so there is nothing to pin. Connections without a +# certificate are still accepted, because bootstrap and public endpoints must be +# reachable before a client has one. Key-release RPCs still require verified caller # attestation; certificate signing verifies CSR signature and attestation. -mandatory = false # Core KMS Configuration [core] diff --git a/docs/tutorials/kms-cvm-deployment.md b/docs/tutorials/kms-cvm-deployment.md index a0dacaf5e..2c3cb278b 100644 --- a/docs/tutorials/kms-cvm-deployment.md +++ b/docs/tutorials/kms-cvm-deployment.md @@ -73,8 +73,8 @@ Inside the CVM at `/etc/kms/certs/`: | `root-ca.key` | Root CA signing key (P256 ECDSA) | | `rpc.crt` | TLS certificate for RPC server | | `rpc.key` | RPC server private key | -| `tmp-ca.crt` | Temporary CA for mutual TLS | -| `tmp-ca.key` | Temporary CA private key | +| `tmp-ca.crt` | Temp CA, served by the legacy `GetTempCaCert` bootstrap RPC | +| `tmp-ca.key` | Temp CA private key, served alongside it | | `root-k256.key` | Ethereum signing key (secp256k1) | | `bootstrap-info.json` | Public keys and TDX attestation quote | @@ -165,13 +165,11 @@ configs: key = "/etc/kms/certs/rpc.key" certs = "/etc/kms/certs/rpc.crt" - [rpc.tls.mutual] - ca_certs = "/etc/kms/certs/tmp-ca.crt" - # Keep the TLS listener optional because bootstrap/public endpoints must be - # reachable before a client has an RA-TLS certificate. Temp-CA bootstrap - # material is bootstrap-sensitive. Key-release RPCs still require verified - # caller attestation; certificate signing verifies CSR signature and attestation. - mandatory = false + # No mutual-TLS section: client certificates are verified by the attestation + # they carry, not by an issuer CA. Connections without a certificate are still + # accepted, because bootstrap and public endpoints must be reachable before a + # client has one. Key-release RPCs still require verified caller attestation; + # certificate signing verifies CSR signature and attestation. [core] cert_dir = "/etc/kms/certs" diff --git a/dstack/Cargo.lock b/dstack/Cargo.lock index 0938f2ee7..aad37febb 100644 --- a/dstack/Cargo.lock +++ b/dstack/Cargo.lock @@ -5694,6 +5694,7 @@ dependencies = [ "reqwest", "rocket", "rocket-vsock-listener", + "rustls", "serde", "serde_json", "tokio", diff --git a/dstack/crates/dstack-cli-core/src/config.rs b/dstack/crates/dstack-cli-core/src/config.rs index f62cba0ff..4309bd9bb 100644 --- a/dstack/crates/dstack-cli-core/src/config.rs +++ b/dstack/crates/dstack-cli-core/src/config.rs @@ -130,10 +130,6 @@ port = 8000 key = "/kms/certs/rpc.key" certs = "/kms/certs/rpc.crt" -[rpc.tls.mutual] -ca_certs = "/kms/certs/tmp-ca.crt" -mandatory = false - [core] cert_dir = "/kms/certs" # single-node: the KMS does not self-attest to its own auth API before diff --git a/dstack/kms/dstack-app/compose-dev.yaml b/dstack/kms/dstack-app/compose-dev.yaml index 718e95f03..a642b3218 100644 --- a/dstack/kms/dstack-app/compose-dev.yaml +++ b/dstack/kms/dstack-app/compose-dev.yaml @@ -57,10 +57,6 @@ configs: key = "/kms/certs/rpc.key" certs = "/kms/certs/rpc.crt" - [rpc.tls.mutual] - ca_certs = "/kms/certs/tmp-ca.crt" - mandatory = false - [core] cert_dir = "/kms/certs" diff --git a/dstack/kms/dstack-app/compose-simple.yaml b/dstack/kms/dstack-app/compose-simple.yaml index d268c38f1..711b06cad 100644 --- a/dstack/kms/dstack-app/compose-simple.yaml +++ b/dstack/kms/dstack-app/compose-simple.yaml @@ -33,10 +33,6 @@ configs: key = "/kms/certs/rpc.key" certs = "/kms/certs/rpc.crt" - [rpc.tls.mutual] - ca_certs = "/kms/certs/tmp-ca.crt" - mandatory = false - [core] cert_dir = "/kms/certs" diff --git a/dstack/kms/kms.toml b/dstack/kms/kms.toml index 935fdbd7b..5beb28d80 100644 --- a/dstack/kms/kms.toml +++ b/dstack/kms/kms.toml @@ -18,9 +18,9 @@ port = 8000 key = "/etc/kms/certs/rpc.key" certs = "/etc/kms/certs/rpc.crt" -[rpc.tls.mutual] -ca_certs = "/etc/kms/certs/tmp-ca.crt" -mandatory = false +# No `[rpc.tls.mutual]`: client certificates are verified by the attestation they +# carry, not by an issuer CA (see ra-rpc's ratls_client_verifier). Leaving a +# `mutual` section here is harmless — it is no longer the trust anchor. [core] cert_dir = "/etc/kms/certs" diff --git a/dstack/kms/rpc/proto/kms_rpc.proto b/dstack/kms/rpc/proto/kms_rpc.proto index de44e28a0..e491d9335 100644 --- a/dstack/kms/rpc/proto/kms_rpc.proto +++ b/dstack/kms/rpc/proto/kms_rpc.proto @@ -74,6 +74,11 @@ message KmsKeyResponse { repeated KmsKeys keys = 2; } +// See the note on `GetTempCaCert`. +// +// `temp_ca_key` is a CA private key handed to any caller, so it authenticates +// nobody. It exists because the KMS pinned this CA for mutual TLS, which left a +// self-issued RA-TLS certificate with nothing to chain to. message GetTempCaCertResponse { string temp_ca_cert = 1; string temp_ca_key = 2; @@ -101,7 +106,19 @@ service KMS { rpc GetAppEnvEncryptPubKey(AppId) returns (PublicKeyResponse); // Request the KMS instance metadata for use as a probe and health check. rpc GetMeta(google.protobuf.Empty) returns (GetMetaResponse); - // Request the temporary CA certificate and key + // Request the temporary CA certificate and key. + // + // Both current callers - guests at boot, and KMS-to-KMS onboarding - fetch this CA + // and mint their client certificate from it, because the KMS used to pin the CA for + // mutual TLS and a self-issued certificate had nothing to chain to. + // + // That pin is gone: the KMS now verifies client certificates by the attestation + // they carry and ignores the issuer, so a self-issued certificate would be accepted + // on the same terms. Neither caller has been migrated yet, so this RPC still has to + // work; it is scheduled for removal once both are. + // + // Do not build new callers on it. A caller that only needs the KMS root CA can read + // `ca_cert` from `GetMeta` instead. rpc GetTempCaCert(google.protobuf.Empty) returns (GetTempCaCertResponse); // Sign a certificate rpc SignCert(SignCertRequest) returns (SignCertResponse); diff --git a/dstack/kms/src/main.rs b/dstack/kms/src/main.rs index dd52622cd..b6dae8f59 100644 --- a/dstack/kms/src/main.rs +++ b/dstack/kms/src/main.rs @@ -6,12 +6,14 @@ use anyhow::{anyhow, bail, Context, Result}; use clap::Parser; use config::KmsConfig; use main_service::{KmsState, RpcHandler}; +use ra_rpc::ratls_client_verifier::RaTlsClientAuth; use ra_rpc::rocket_helper::QuoteVerifier; use ra_tls::attestation::AttestationVerifier; use rocket::{ fairing::AdHoc, figment::{providers::Serialized, Figment}, response::content::{RawHtml, RawText}, + tls::Resolver as _, Shutdown, State, }; use tracing::{info, warn}; @@ -180,6 +182,11 @@ async fn main() -> Result<()> { .clone() .merge(Serialized::defaults(figment.find_value("rpc")?)); let mut rocket = rocket::custom(figment) + // Verify client certificates by their attestation rather than by issuer. The + // certificates guests and onboarding mint from the temp CA today keep working + // unchanged - they are now accepted for the attestation they carry rather than + // for who signed them - and a self-issued certificate would be accepted too. + .attach(RaTlsClientAuth::fairing()) .attach(AdHoc::on_response("Add app version header", |_req, res| { Box::pin(async move { res.set_raw_header("X-App-Version", app_version()); diff --git a/dstack/kms/src/main_service.rs b/dstack/kms/src/main_service.rs index 339fbd3bc..726a0a341 100644 --- a/dstack/kms/src/main_service.rs +++ b/dstack/kms/src/main_service.rs @@ -494,6 +494,20 @@ impl KmsRpc for RpcHandler { }) } + /// Serve the temp CA certificate and key. + /// + /// Both current callers - guests at boot, and KMS-to-KMS onboarding + /// ([`crate::onboard_service`]) - fetch this CA and mint their client certificate + /// from it, because the KMS used to pin the CA for mutual TLS and a self-issued + /// certificate had nothing to chain to. + /// + /// That pin is gone: client certificates are now verified by the attestation they + /// carry (`ra_rpc::ratls_client_verifier`), so a self-issued certificate would be + /// accepted on the same terms. Neither caller has been migrated yet, so this RPC + /// still has to work. + /// + /// The key it returns authenticates nobody: it is handed to any caller. Removing + /// this RPC needs both callers migrated first. async fn get_temp_ca_cert(self) -> Result { let self_boot_info = self .ensure_self_allowed() diff --git a/dstack/ra-rpc/Cargo.toml b/dstack/ra-rpc/Cargo.toml index 622e55c33..a3f4537bc 100644 --- a/dstack/ra-rpc/Cargo.toml +++ b/dstack/ra-rpc/Cargo.toml @@ -13,6 +13,7 @@ license.workspace = true anyhow.workspace = true prpc.workspace = true rocket = { workspace = true, features = ["mtls"], optional = true } +rustls = { workspace = true, optional = true } serde_json.workspace = true tracing.workspace = true reqwest = { workspace = true, default-features = false, features = ["rustls", "charset"], optional = true } @@ -27,9 +28,13 @@ or-panic.workspace = true [dev-dependencies] tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +reqwest = { workspace = true, default-features = false, features = [ + "rustls", + "charset", +] } [features] default = ["rocket", "client"] -rocket = ["dep:rocket", "dep:rocket-vsock-listener"] +rocket = ["dep:rocket", "dep:rocket-vsock-listener", "dep:rustls"] client = ["reqwest"] openapi = ["dep:prost-types"] diff --git a/dstack/ra-rpc/src/lib.rs b/dstack/ra-rpc/src/lib.rs index e690f9a8c..40f6353a3 100644 --- a/dstack/ra-rpc/src/lib.rs +++ b/dstack/ra-rpc/src/lib.rs @@ -13,6 +13,9 @@ use tracing::{error, info}; pub use ra_tls::attestation::{Attestation, VerifiedAttestation}; +#[cfg(feature = "rocket")] +pub mod ratls_client_verifier; + #[cfg(feature = "rocket")] pub mod rocket_helper; diff --git a/dstack/ra-rpc/src/ratls_client_verifier.rs b/dstack/ra-rpc/src/ratls_client_verifier.rs new file mode 100644 index 000000000..d3c467808 --- /dev/null +++ b/dstack/ra-rpc/src/ratls_client_verifier.rs @@ -0,0 +1,385 @@ +// SPDX-FileCopyrightText: © 2024-2025 Phala Network +// +// SPDX-License-Identifier: Apache-2.0 + +//! Verify RA-TLS client certificates by attestation instead of by issuer. +//! +//! Rocket configures mutual TLS through rustls' [`WebPkiClientVerifier`], which pins +//! a CA: a presented client certificate must chain to it or the handshake fails. +//! RA-TLS certificates are self-issued and carry their identity in a TEE quote, so +//! there is no CA to pin. dstack worked around that by having the KMS hand every +//! caller a shared "temp CA" private key purely so the minted certificate would +//! chain somewhere — the CA established nothing, and its key was public by design. +//! +//! [`RaTlsClientVerifier`] replaces the chain check with the check that actually +//! carries meaning: the certificate must carry an attestation. Certificates minted +//! from that temp CA — which is what guests and KMS-to-KMS onboarding still send — +//! are accepted for that attestation rather than for their issuer, so nothing has to +//! change on the client side for them to keep working. Verifying that +//! attestation — and deciding whether the app behind it is authorized — needs +//! network I/O (collateral fetch, auth API) and stays where it already is, in +//! [`crate::rocket_helper`] and the service handlers. Keeping the expensive half +//! out of the handshake also keeps unauthenticated peers from driving it. +//! +//! [`WebPkiClientVerifier`]: https://docs.rs/rustls/latest/rustls/server/struct.WebPkiClientVerifier.html + +use std::sync::Arc; + +use rocket::tls::{CipherSuite, ClientHello, Resolver, ServerConfig, TlsConfig}; +use rocket::{Build, Rocket}; +use rustls::crypto::CryptoProvider; +use rustls::pki_types::{CertificateDer, PrivateKeyDer, UnixTime}; +use rustls::server::danger::{ClientCertVerified, ClientCertVerifier}; +use rustls::server::ServerSessionMemoryCache; +use rustls::{DigitallySignedStruct, DistinguishedName, SignatureScheme}; +use tracing::{debug, info, warn}; +use x509_parser::prelude::FromDer as _; + +/// A rustls client-certificate verifier that accepts any certificate carrying an +/// RA-TLS attestation, whatever signed it. +#[derive(Debug)] +pub struct RaTlsClientVerifier { + provider: Arc, +} + +impl RaTlsClientVerifier { + pub fn new(provider: Arc) -> Self { + Self { provider } + } +} + +impl ClientCertVerifier for RaTlsClientVerifier { + /// Advertise no acceptable CAs, so clients send whatever certificate they hold + /// rather than filtering against a list we do not have. + fn root_hint_subjects(&self) -> &[DistinguishedName] { + &[] + } + + fn offer_client_auth(&self) -> bool { + true + } + + /// Anonymous connections stay allowed, matching the previous + /// `[rpc.tls.mutual] mandatory = false`. Handlers that need an attested caller + /// enforce it themselves; the unauthenticated RPCs (`GetMeta`, `GetTempCaCert`) + /// are reachable without a certificate by design. + fn client_auth_mandatory(&self) -> bool { + false + } + + fn verify_client_cert( + &self, + end_entity: &CertificateDer<'_>, + _intermediates: &[CertificateDer<'_>], + now: UnixTime, + ) -> Result { + let (_, cert) = x509_parser::certificate::X509Certificate::from_der(end_entity.as_ref()) + .map_err(|err| { + rustls::Error::General(format!("failed to parse client certificate: {err}")) + })?; + + // webpki used to enforce the validity window; keep doing so now that it does not. + // Compare as i64: x509 timestamps are signed, and a certificate whose window + // predates the epoch must read as expired rather than as unbounded. + let now_secs = now.as_secs() as i64; + let not_before = cert.validity().not_before.timestamp(); + let not_after = cert.validity().not_after.timestamp(); + if now_secs < not_before { + return Err(rustls::Error::General( + "client certificate is not yet valid".into(), + )); + } + if now_secs > not_after { + return Err(rustls::Error::General( + "client certificate has expired".into(), + )); + } + + // Only presence and decodability are checked here. The quote itself, and its + // binding to this certificate's public key, are verified asynchronously in + // `rocket_helper::handle_prpc_impl` before any handler sees the caller. + match ra_tls::attestation::from_cert(&cert) { + Ok(Some(_)) => Ok(ClientCertVerified::assertion()), + Ok(None) => Err(rustls::Error::General( + "client certificate carries no attestation".into(), + )), + Err(err) => Err(rustls::Error::General(format!( + "failed to decode client attestation: {err}" + ))), + } + } + + fn verify_tls12_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> Result { + rustls::crypto::verify_tls12_signature( + message, + cert, + dss, + &self.provider.signature_verification_algorithms, + ) + } + + fn verify_tls13_signature( + &self, + message: &[u8], + cert: &CertificateDer<'_>, + dss: &DigitallySignedStruct, + ) -> Result { + rustls::crypto::verify_tls13_signature( + message, + cert, + dss, + &self.provider.signature_verification_algorithms, + ) + } + + fn supported_verify_schemes(&self) -> Vec { + self.provider + .signature_verification_algorithms + .supported_schemes() + } +} + +/// A Rocket TLS resolver that serves one [`ServerConfig`] wired to +/// [`RaTlsClientVerifier`]. +/// +/// Attach it with `rocket.attach(RaTlsClientAuth::fairing())`. Rustls keeps +/// `ServerConfig::verifier` private, so the verifier cannot be swapped into the config +/// rocket already built: this resolver builds its own from the same `[tls]` section and +/// resolves every connection to it. Rocket still builds its default config, but nothing +/// ever uses it, which is what makes `[tls.mutual]` inert. +/// +/// Building a second config means mirroring what the stock listener sets, and the +/// mirror is deliberate rather than incidental: protocol versions, ALPN, +/// `prefer_server_cipher_order`, the session cache and the ticketer are all set to +/// rocket's values. The single exception is `tls.ciphers`, which selects a crypto +/// provider through a mapping rocket keeps private; a non-default cipher list is +/// warned about at startup rather than silently dropped. +pub struct RaTlsClientAuth { + config: Option>, +} + +#[rocket::async_trait] +impl Resolver for RaTlsClientAuth { + async fn init(rocket: &Rocket) -> rocket::tls::Result { + let figment = rocket.figment(); + if figment.find_value("tls").is_err() { + // No TLS configured at all (plain-HTTP dev servers). Resolving to `None` + // lets rocket keep whatever it would have done without us. + warn!("no tls section configured, RA-TLS client auth is inactive"); + return Ok(Self { config: None }); + } + // The section exists, so it must parse. Degrading to `None` here would leave + // rocket serving a listener that never asks for a client certificate, and + // every attested RPC would then fail far from the cause. Fail the launch + // instead - including when the section is only half-written, which is the + // case a plain `.ok()` used to swallow. + let tls: TlsConfig = figment.extract_inner("tls")?; + let config = build_server_config(&tls) + .map_err(|err| rocket::figment::Error::from(err.to_string()))?; + info!("RA-TLS client certificate verification enabled"); + Ok(Self { + config: Some(Arc::new(config)), + }) + } + + async fn resolve(&self, _hello: ClientHello<'_>) -> Option> { + self.config.clone() + } +} + +fn build_server_config(tls: &TlsConfig) -> anyhow::Result { + use anyhow::Context as _; + use rustls::pki_types::pem::PemObject as _; + + let provider = CryptoProvider::get_default() + .cloned() + .unwrap_or_else(|| Arc::new(rustls::crypto::ring::default_provider())); + + // Read through rocket's own readers so a path and inline PEM bytes are both + // accepted, exactly as the stock listener accepts them. + let mut certs_reader = tls.certs_reader().context("failed to read tls.certs")?; + let certs: Vec> = CertificateDer::pem_reader_iter(&mut certs_reader) + .collect::>() + .context("failed to parse tls.certs")?; + let mut key_reader = tls.key_reader().context("failed to read tls.key")?; + let key = PrivateKeyDer::from_pem_reader(&mut key_reader).context("failed to parse tls.key")?; + + if tls.ciphers().ne(CipherSuite::DEFAULT_SET) { + // Rocket derives a crypto provider from this list through a private mapping, + // and only when no process-default provider is installed. Rather than + // duplicating that table, say so instead of dropping the setting silently. + warn!("tls.ciphers is not applied by the RA-TLS resolver; the default provider's suites are used"); + } + + let mut config = rustls::ServerConfig::builder_with_provider(provider.clone()) + .with_safe_default_protocol_versions() + .context("failed to select TLS protocol versions")? + .with_client_cert_verifier(Arc::new(RaTlsClientVerifier::new(provider))) + .with_single_cert(certs, key) + .context("failed to load server certificate")?; + // Everything below mirrors rocket's own listener, so replacing the verifier is the + // only observable difference. Dropping the ticketer and the larger session cache + // would quietly change resumption behaviour for every existing deployment. + config.ignore_client_order = tls.prefer_server_cipher_order(); + config.session_storage = ServerSessionMemoryCache::new(1024); + config.ticketer = rustls::crypto::ring::Ticketer::new().context("failed to build ticketer")?; + config.alpn_protocols = vec![b"h2".to_vec(), b"http/1.1".to_vec()]; + debug!("built RA-TLS server config"); + Ok(config) +} + +#[cfg(test)] +mod tests { + use super::*; + use ra_tls::attestation::{Attestation, AttestationQuote, TdxQuote, VersionedAttestation}; + use ra_tls::cert::CertRequest; + use ra_tls::rcgen::{Certificate, KeyPair, PKCS_ECDSA_P256_SHA256}; + use std::time::{Duration, SystemTime}; + + fn verifier() -> RaTlsClientVerifier { + RaTlsClientVerifier::new(Arc::new(rustls::crypto::ring::default_provider())) + } + + fn now() -> UnixTime { + UnixTime::since_unix_epoch( + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap(), + ) + } + + fn attestation() -> VersionedAttestation { + Attestation { + quote: AttestationQuote::DstackTdx(TdxQuote { + quote: vec![1, 2, 3], + event_log: vec![], + }), + runtime_events: vec![], + report_data: [0u8; 64], + config: "".into(), + report: (), + } + .into_versioned() + } + + fn leaf( + att: Option<&VersionedAttestation>, + not_after: Option, + issuer: Option<(&Certificate, &KeyPair)>, + ) -> Vec { + let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap(); + let req = CertRequest::builder() + .subject("test client") + .key(&key) + .maybe_attestation(att) + .maybe_not_after(not_after) + .usage_client_auth(true) + .build(); + let cert = match issuer { + Some((ca_cert, ca_key)) => req.signed_by(ca_cert, ca_key).unwrap(), + None => req.self_signed().unwrap(), + }; + cert.der().to_vec() + } + + fn verify(der: Vec) -> Result { + verifier().verify_client_cert(&CertificateDer::from(der), &[], now()) + } + + #[test] + fn accepts_self_signed_cert_carrying_attestation() { + let att = attestation(); + assert!(verify(leaf(Some(&att), None, None)).is_ok()); + } + + #[test] + fn accepts_ca_signed_cert() { + // What guests and KMS-to-KMS onboarding send today: a certificate minted from + // the KMS temp CA. That chain is no longer pinned, so it is accepted for the + // attestation it carries instead. + let ca_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap(); + let ca_cert = CertRequest::builder() + .subject("Dstack Client Temp CA") + .key(&ca_key) + .ca_level(0) + .build() + .self_signed() + .unwrap(); + let att = attestation(); + assert!(verify(leaf(Some(&att), None, Some((&ca_cert, &ca_key)))).is_ok()); + } + + #[test] + fn rejects_cert_without_attestation() { + let err = verify(leaf(None, None, None)).unwrap_err(); + assert!( + err.to_string().contains("no attestation"), + "unexpected error: {err}" + ); + } + + #[test] + fn rejects_expired_cert() { + let att = attestation(); + let expired = SystemTime::now() - Duration::from_secs(3600); + let err = verify(leaf(Some(&att), Some(expired), None)).unwrap_err(); + assert!( + err.to_string().contains("expired"), + "unexpected error: {err}" + ); + } + + fn server_tls_config(prefer_server_cipher_order: bool) -> TlsConfig { + let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap(); + let cert = CertRequest::builder() + .subject("test server") + .key(&key) + .usage_server_auth(true) + .build() + .self_signed() + .unwrap(); + TlsConfig::from_bytes(cert.pem().as_bytes(), key.serialize_pem().as_bytes()) + .with_preferred_server_cipher_order(prefer_server_cipher_order) + } + + /// Building a second `ServerConfig` is only safe if it carries rocket's own + /// settings; anything left at a rustls default is a silent behaviour change for + /// deployments that used the stock listener. + #[test] + fn mirrors_rockets_tls_settings() { + let config = build_server_config(&server_tls_config(true)).unwrap(); + assert!( + config.ignore_client_order, + "tls.prefer_server_cipher_order must reach the config" + ); + assert!( + config.ticketer.enabled(), + "rocket configures a ticketer; dropping it changes resumption" + ); + assert_eq!( + config.alpn_protocols, + vec![b"h2".to_vec(), b"http/1.1".to_vec()] + ); + + let config = build_server_config(&server_tls_config(false)).unwrap(); + assert!(!config.ignore_client_order); + } + + #[test] + fn rejects_cert_that_expired_before_the_epoch() { + // x509 timestamps are signed. A notAfter before 1970 must read as expired, + // not as "no upper bound". + let att = attestation(); + let expired = SystemTime::UNIX_EPOCH - Duration::from_secs(365 * 86400); + let err = verify(leaf(Some(&att), Some(expired), None)).unwrap_err(); + assert!( + err.to_string().contains("expired"), + "unexpected error: {err}" + ); + } +} diff --git a/dstack/ra-rpc/tests/ratls_client_auth.rs b/dstack/ra-rpc/tests/ratls_client_auth.rs new file mode 100644 index 000000000..9719c508c --- /dev/null +++ b/dstack/ra-rpc/tests/ratls_client_auth.rs @@ -0,0 +1,241 @@ +// SPDX-FileCopyrightText: © 2025 Phala Network +// +// SPDX-License-Identifier: Apache-2.0 + +//! End-to-end check that a Rocket server using [`RaTlsClientAuth`] authenticates +//! clients by the attestation in their certificate rather than by its issuer. +//! +//! The cases that matter: +//! * a certificate minted from a temp CA is accepted (what guests and KMS-to-KMS +//! onboarding send today, and what used to be the only accepted shape); +//! * a self-issued RA-TLS certificate is accepted too, which is what lets clients +//! stop fetching CA material; +//! * a certificate with no attestation is rejected during the handshake; +//! * an anonymous connection still reaches the handler, so the unauthenticated +//! RPCs stay reachable. + +use anyhow::{Context, Result}; +use ra_rpc::ratls_client_verifier::RaTlsClientAuth; +use ra_tls::attestation::{Attestation, AttestationQuote, TdxQuote, VersionedAttestation}; +use ra_tls::cert::CertRequest; +use ra_tls::rcgen::{Certificate, KeyPair, PKCS_ECDSA_P256_SHA256}; +use rocket::tls::Resolver as _; + +struct CertPair { + cert_pem: String, + key_pem: String, +} + +fn attestation() -> VersionedAttestation { + Attestation { + quote: AttestationQuote::DstackTdx(TdxQuote { + quote: vec![1, 2, 3], + event_log: vec![], + }), + runtime_events: vec![], + report_data: [0u8; 64], + config: "".into(), + report: (), + } + .into_versioned() +} + +fn server_cert() -> CertPair { + let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap(); + let alt_names = vec!["localhost".to_string()]; + let cert = CertRequest::builder() + .subject("test server") + .key(&key) + .alt_names(&alt_names) + .usage_server_auth(true) + .build() + .self_signed() + .unwrap(); + CertPair { + cert_pem: cert.pem(), + key_pem: key.serialize_pem(), + } +} + +fn client_cert(with_attestation: bool, issuer: Option<(&Certificate, &KeyPair)>) -> CertPair { + let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap(); + let att = attestation(); + let req = CertRequest::builder() + .subject("test client") + .key(&key) + .maybe_attestation(with_attestation.then_some(&att)) + .usage_client_auth(true) + .build(); + let cert = match issuer { + Some((ca_cert, ca_key)) => req.signed_by(ca_cert, ca_key).unwrap(), + None => req.self_signed().unwrap(), + }; + CertPair { + cert_pem: cert.pem(), + key_pem: key.serialize_pem(), + } +} + +fn temp_ca() -> (Certificate, KeyPair) { + let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap(); + let cert = CertRequest::builder() + .subject("Dstack Client Temp CA") + .key(&key) + .ca_level(0) + .build() + .self_signed() + .unwrap(); + (cert, key) +} + +#[rocket::get("/whoami")] +fn whoami(cert: Option>) -> String { + match cert { + None => "anonymous".to_string(), + Some(c) => format!("cn={}", c.subject().common_name().unwrap_or("")), + } +} + +/// Drive the server the way a real caller does: an HTTPS client that presents (or +/// withholds) a client certificate. Server-cert checking is off because the test +/// server is self-issued; the client certificate is the subject here. +async fn probe(port: u16, client: Option<&CertPair>) -> Result { + let mut builder = reqwest::Client::builder() + .danger_accept_invalid_certs(true) + .timeout(std::time::Duration::from_secs(10)); + if let Some(pair) = client { + let identity_pem = format!("{}\n{}", pair.cert_pem, pair.key_pem); + builder = builder.identity(reqwest::Identity::from_pem(identity_pem.as_bytes())?); + } + let response = builder + .build()? + .get(format!("https://127.0.0.1:{port}/whoami")) + .send() + .await + .context("request failed")? + .error_for_status()?; + response.text().await.context("failed to read body") +} + +fn free_port() -> u16 { + let listener = std::net::TcpListener::bind(("127.0.0.1", 0)).unwrap(); + listener.local_addr().unwrap().port() +} + +#[tokio::test(flavor = "multi_thread")] +async fn client_certs_are_authenticated_by_attestation_not_issuer() { + let dir = tempdir(); + let server = server_cert(); + let certs_path = dir.join("server.crt"); + let key_path = dir.join("server.key"); + std::fs::write(&certs_path, &server.cert_pem).unwrap(); + std::fs::write(&key_path, &server.key_pem).unwrap(); + + let port = free_port(); + // Note: no `[tls.mutual]` section at all — the resolver supplies the verifier. + let figment = rocket::Config::figment() + .merge(("port", port)) + .merge(("address", "127.0.0.1")) + .merge(("log_level", "off")) + .merge(("shutdown.ctrlc", false)) + .merge(("tls.certs", certs_path.to_str().unwrap())) + .merge(("tls.key", key_path.to_str().unwrap())); + let rocket = rocket::custom(figment) + .attach(RaTlsClientAuth::fairing()) + .mount("/", rocket::routes![whoami]); + let server_task = tokio::spawn(async move { rocket.launch().await }); + + let mut up = false; + for _ in 0..100 { + assert!(!server_task.is_finished(), "rocket exited during launch"); + if tokio::net::TcpStream::connect(("127.0.0.1", port)) + .await + .is_ok() + { + up = true; + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert!(up, "server never came up on port {port}"); + + // A self-issued RA-TLS cert is accepted and reaches the handler. No client sends + // one yet; this is the shape the CA pin used to reject. + let self_signed = client_cert(true, None); + assert_eq!( + probe(port, Some(&self_signed)).await.unwrap(), + "cn=test client" + ); + + // A cert minted from the temp CA — what guests and KMS-to-KMS onboarding present + // today — keeps working. + let (ca_cert, ca_key) = temp_ca(); + let ca_signed = client_cert(true, Some((&ca_cert, &ca_key))); + assert_eq!( + probe(port, Some(&ca_signed)).await.unwrap(), + "cn=test client" + ); + + // A cert with no attestation is refused during the handshake. + let no_attestation = client_cert(false, None); + let err = probe(port, Some(&no_attestation)) + .await + .expect_err("cert without attestation must be rejected"); + assert!( + format!("{err:#}").contains("HandshakeFailure"), + "expected the handshake to be refused, got: {err:#}" + ); + + // Anonymous connections still work, so unauthenticated RPCs stay reachable. + assert_eq!(probe(port, None).await.unwrap(), "anonymous"); + + server_task.abort(); +} + +fn tempdir() -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("ra-rpc-ratls-test-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +/// A `tls` section that exists but does not parse must stop the launch. Degrading to +/// "no resolver" would leave rocket serving a listener that never asks for a client +/// certificate, so every attested RPC would fail somewhere far from the cause. +#[tokio::test] +async fn a_half_written_tls_section_fails_init() { + let figment = rocket::Config::figment().merge(("tls.certs", "/nonexistent.pem")); + let rocket = rocket::custom(figment); + let err = RaTlsClientAuth::init(&rocket) + .await + .err() + .expect("a tls section without a key must not be ignored"); + assert!( + format!("{err}").contains("key"), + "expected the missing key to be named, got: {err}" + ); +} + +/// No `tls` section at all is the plain-HTTP dev case: init must succeed and stay +/// inactive rather than fail the launch. (`resolve` is unreachable here - rocket only +/// calls it once a handshake is in flight, and there is no TLS listener to have one.) +#[tokio::test] +async fn no_tls_section_still_launches() { + let rocket = rocket::custom(rocket::Config::figment()); + RaTlsClientAuth::init(&rocket) + .await + .expect("a plain-HTTP server must still launch"); +} + +/// Inline PEM bytes are what rocket's own listener accepts alongside paths. The +/// resolver reads through rocket's readers, so both shapes work. +#[tokio::test] +async fn inline_pem_bytes_are_accepted() { + let server = server_cert(); + let figment = rocket::Config::figment() + .merge(("tls.certs", server.cert_pem.as_bytes().to_vec())) + .merge(("tls.key", server.key_pem.as_bytes().to_vec())); + let rocket = rocket::custom(figment); + RaTlsClientAuth::init(&rocket) + .await + .expect("inline PEM must be accepted, as it is by the stock listener"); +} diff --git a/test-suites/full-stack-compose/scripts/app_compose.py b/test-suites/full-stack-compose/scripts/app_compose.py index 6a8cd579a..35a5cab84 100755 --- a/test-suites/full-stack-compose/scripts/app_compose.py +++ b/test-suites/full-stack-compose/scripts/app_compose.py @@ -60,10 +60,6 @@ def kms(args: argparse.Namespace) -> None: key = "/kms/certs/rpc.key" certs = "/kms/certs/rpc.crt" -[rpc.tls.mutual] -ca_certs = "/kms/certs/tmp-ca.crt" -mandatory = false - [core] cert_dir = "/kms/certs" admin_token_hash = "" diff --git a/tools/dev-stack.sh b/tools/dev-stack.sh index 442fb745b..c30bc398c 100755 --- a/tools/dev-stack.sh +++ b/tools/dev-stack.sh @@ -175,10 +175,6 @@ port = $KMS_RPC_LISTEN_PORT key = "$CERTS_DIR/rpc.key" certs = "$CERTS_DIR/rpc.crt" -[rpc.tls.mutual] -ca_certs = "$CERTS_DIR/tmp-ca.crt" -mandatory = false - [core] cert_dir = "$CERTS_DIR" attest_rpc_cert = false