From 09b5ca51a022d6727cfec4652f245db7ba7f0ea3 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sat, 8 Aug 2026 02:41:42 -0700 Subject: [PATCH 1/4] fix(ra-tls): encode app info as a named msgpack map --- dstack/ra-tls/src/cert.rs | 131 +++++++++++++++++++++++++++++++++++++- 1 file changed, 130 insertions(+), 1 deletion(-) diff --git a/dstack/ra-tls/src/cert.rs b/dstack/ra-tls/src/cert.rs index 9b8b3ff52..dd009785e 100644 --- a/dstack/ra-tls/src/cert.rs +++ b/dstack/ra-tls/src/cert.rs @@ -382,8 +382,13 @@ impl CertRequest<'_, Key> { add_ext(&mut params, PHALA_RATLS_APP_ID, app_id); } if let Some(app_info) = self.app_info { + // Encode as a MessagePack map keyed by field name rather than a positional + // array. Field-name keys let readers tolerate fields they do not know, so + // appending a field to `AppInfo` no longer breaks peers built against an + // older definition. Decoding accepts both forms, so certificates issued by + // older releases (positional) stay readable. let app_info_bytes = - rmp_serde::to_vec(&app_info).context("Failed to serialize app info")?; + rmp_serde::to_vec_named(&app_info).context("failed to serialize app info")?; add_ext(&mut params, PHALA_RATLS_APP_INFO, app_info_bytes); } if let Some(usage) = self.special_usage { @@ -759,4 +764,128 @@ mod tests { let expected = "44706c65617365207369676e20636572743a0c0102030040746573742e6578616d706c652e636f6d000100010000000000040900000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"; assert_eq!(actual, expected); } + + /// `AppInfo` travels inside RA-TLS certificates, so its MessagePack encoding is a + /// cross-release wire contract: a certificate issued by one build gets parsed by + /// peers built from another, and certificates outlive the process that issued them. + /// + /// Positional (array) encoding made that contract the *field order and count*, so + /// appending a field broke every peer that had not been rebuilt. Encoding as a map + /// keyed by field name makes it the *field names*, which readers can skip when + /// unknown. These tests pin both directions across the v0.5.11 boundary. + mod app_info_encoding { + use super::*; + use serde::{Deserialize, Serialize}; + use serde_human_bytes as hex_bytes; + + /// The exact `AppInfo` layout shipped in v0.5.6 through v0.5.11: eight fields, + /// no `init_script_hashes`. Stands in for a peer built against those releases. + #[derive(Debug, Serialize, Deserialize)] + struct LegacyAppInfo { + #[serde(with = "hex_bytes")] + app_id: Vec, + #[serde(with = "hex_bytes")] + compose_hash: Vec, + #[serde(with = "hex_bytes")] + instance_id: Vec, + #[serde(with = "hex_bytes")] + device_id: Vec, + #[serde(with = "hex_bytes")] + mr_system: [u8; 32], + #[serde(with = "hex_bytes")] + mr_aggregated: [u8; 32], + #[serde(with = "hex_bytes")] + os_image_hash: Vec, + #[serde(with = "hex_bytes")] + key_provider_info: Vec, + } + + /// A v0.5.11 app-info extension body: `LegacyAppInfo` in positional MessagePack, + /// exactly as `rmp_serde::to_vec` emitted it. Leading `0x98` is a fixarray of 8. + const LEGACY_POSITIONAL_APP_INFO: &str = concat!( + "98c403a1a2a3c402b1b2c402c1c2c401d1c42051515151515151515151515151", + "51515151515151515151515151515151515151c4206262626262626262626262", + "626262626262626262626262626262626262626262c402e1e2c4036b6d73", + ); + + fn sample_app_info() -> AppInfo { + AppInfo { + app_id: vec![0xa1, 0xa2, 0xa3], + compose_hash: vec![0xb1, 0xb2], + instance_id: vec![0xc1, 0xc2], + device_id: vec![0xd1], + mr_system: [0x51; 32], + mr_aggregated: [0x62; 32], + os_image_hash: vec![0xe1, 0xe2], + key_provider_info: b"kms".to_vec(), + init_script_hashes: Some(vec![[0xf1; 32].to_vec(), [0xf2; 32].to_vec()]), + } + } + + /// Old certificate, new reader. Certificates issued before this change carry a + /// positional array and must keep parsing; fields added since default in. + #[test] + fn legacy_positional_app_info_still_decodes() { + let bytes = hex::decode(LEGACY_POSITIONAL_APP_INFO).unwrap(); + let decoded: AppInfo = rmp_serde::from_slice(&bytes).unwrap(); + + assert_eq!(decoded.app_id, vec![0xa1, 0xa2, 0xa3]); + assert_eq!(decoded.compose_hash, vec![0xb1, 0xb2]); + assert_eq!(decoded.instance_id, vec![0xc1, 0xc2]); + assert_eq!(decoded.device_id, vec![0xd1]); + assert_eq!(decoded.mr_system, [0x51; 32]); + assert_eq!(decoded.mr_aggregated, [0x62; 32]); + assert_eq!(decoded.os_image_hash, vec![0xe1, 0xe2]); + assert_eq!(decoded.key_provider_info, b"kms".to_vec()); + assert_eq!( + decoded.init_script_hashes, None, + "a field absent from the legacy layout must decode as unbound, not fail" + ); + } + + /// New certificate, old reader. This is the direction positional encoding broke: + /// a v0.5.11 peer decoding a certificate issued by this build. + #[test] + fn named_app_info_decodes_against_legacy_field_set() { + let encoded = rmp_serde::to_vec_named(&sample_app_info()).unwrap(); + + assert_eq!( + encoded[0] & 0xf0, + 0x80, + "app info must encode as a MessagePack map, not a positional array" + ); + + let legacy: LegacyAppInfo = rmp_serde::from_slice(&encoded) + .expect("a reader without init_script_hashes must skip it, not fail"); + assert_eq!(legacy.app_id, vec![0xa1, 0xa2, 0xa3]); + assert_eq!(legacy.mr_system, [0x51; 32]); + assert_eq!(legacy.key_provider_info, b"kms".to_vec()); + } + + /// The encoding change must not drop or reshape any field on the way through a + /// real certificate extension. + #[test] + fn app_info_survives_a_certificate_round_trip() { + let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap(); + let app_info = sample_app_info(); + let cert = CertRequest::builder() + .key(&key) + .subject("test.example.com") + .app_info(&app_info) + .build() + .self_signed() + .unwrap(); + + let decoded = cert.get_app_info().unwrap().expect("app info extension"); + assert_eq!(decoded.app_id, app_info.app_id); + assert_eq!(decoded.compose_hash, app_info.compose_hash); + assert_eq!(decoded.instance_id, app_info.instance_id); + assert_eq!(decoded.device_id, app_info.device_id); + assert_eq!(decoded.mr_system, app_info.mr_system); + assert_eq!(decoded.mr_aggregated, app_info.mr_aggregated); + assert_eq!(decoded.os_image_hash, app_info.os_image_hash); + assert_eq!(decoded.key_provider_info, app_info.key_provider_info); + assert_eq!(decoded.init_script_hashes, app_info.init_script_hashes); + } + } } From 581c20d4a595a427fa8497fcc622fcde49c17c11 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sat, 8 Aug 2026 02:41:42 -0700 Subject: [PATCH 2/4] fix(gateway): encode kv values as named msgpack maps --- dstack/gateway/src/kv/mod.rs | 125 ++++++++++++++++++++++++++++++++++- 1 file changed, 124 insertions(+), 1 deletion(-) diff --git a/dstack/gateway/src/kv/mod.rs b/dstack/gateway/src/kv/mod.rs index c1e6ecb02..33002913f 100644 --- a/dstack/gateway/src/kv/mod.rs +++ b/dstack/gateway/src/kv/mod.rs @@ -367,8 +367,15 @@ pub mod keys { } } +/// Encode a KV value as MessagePack. +/// +/// Structs are encoded as maps keyed by field name rather than as positional +/// arrays. Field-name keys let a reader skip fields it does not know and fill +/// in `#[serde(default)]` fields it does not receive, so the value types below +/// can gain fields without breaking gateways running an older build. Decoding +/// accepts both forms, so values written by older releases stay readable. pub fn encode(value: &T) -> Result> { - rmp_serde::encode::to_vec(value).context("failed to encode value") + rmp_serde::encode::to_vec_named(value).context("failed to encode value") } pub fn decode Deserialize<'de>>(bytes: &[u8]) -> Result { @@ -1221,6 +1228,122 @@ mod acme_credentials_tests { } } +/// KV values replicate between gateways, so their MessagePack encoding is a wire +/// contract across a mixed-version cluster and across a node's own restart. Named +/// maps keep that contract on field names rather than field order, so a value type +/// can gain a field without breaking gateways still running an older build. +#[cfg(test)] +mod value_encoding_tests { + use super::*; + use serde::{Deserialize, Serialize}; + + /// A value type that has since gained a field. Stands in for an older gateway + /// reading a record written by this build. + #[derive(Debug, Serialize, Deserialize)] + struct ReducedCertRenewLock { + started_at: u64, + } + + #[test] + fn values_are_encoded_as_named_maps() { + let encoded = encode(&CertRenewLock { + started_at: 1_700_000_000, + started_by: 7, + }) + .expect("encode should succeed"); + + assert_eq!( + encoded[0] & 0xf0, + 0x80, + "values must encode as MessagePack maps, not positional arrays" + ); + } + + /// New writer, old reader: a peer whose struct predates a field must skip it. + #[test] + fn named_values_decode_against_a_reduced_field_set() { + let encoded = encode(&CertRenewLock { + started_at: 1_700_000_000, + started_by: 7, + }) + .expect("encode should succeed"); + + let reduced: ReducedCertRenewLock = + decode(&encoded).expect("a reader without started_by must skip the field, not fail"); + assert_eq!(reduced.started_at, 1_700_000_000); + } + + /// Old writer, new reader: records written before this change are positional + /// arrays and must keep decoding after an upgrade. + #[test] + fn legacy_positional_records_are_still_readable() { + let dir = tempfile::tempdir().expect("failed to create temp dir"); + let kv = KvStore::new(1, vec![], dir.path()).expect("failed to create kv store"); + + let legacy = rmp_serde::encode::to_vec(&CertRenewLock { + started_at: 1_700_000_000, + started_by: 7, + }) + .expect("legacy encode should succeed"); + assert_eq!( + legacy[0] & 0xf0, + 0x90, + "fixture must be a positional array to exercise the legacy path" + ); + + kv.persistent + .write() + .put(keys::cert_lock("legacy.example"), legacy) + .expect("put should succeed"); + + let decoded = kv + .get_cert_lock("legacy.example") + .expect("a record written by an older gateway must stay readable"); + assert_eq!(decoded.started_at, 1_700_000_000); + assert_eq!(decoded.started_by, 7); + } + + /// `DnsCredential` is the most demanding value type in the set: it nests an + /// internally tagged enum and a field with a custom `serde(with)` codec, both of + /// which behave differently across self-describing and positional encodings. + #[test] + fn nested_tagged_enums_and_custom_codecs_survive_both_encodings() { + let credential = DnsCredential { + id: "cred-1".to_string(), + name: "primary".to_string(), + provider: DnsProvider::Cloudflare { + api_token: "token".to_string(), + api_url: Some("https://api.cloudflare.com/client/v4".to_string()), + }, + max_dns_wait: Duration::from_secs(90), + dns_txt_ttl: 60, + created_at: 1_700_000_000, + updated_at: 1_700_000_001, + }; + + for (label, encoded) in [ + ("named", encode(&credential).expect("named encode")), + ( + "legacy positional", + rmp_serde::encode::to_vec(&credential).expect("positional encode"), + ), + ] { + let decoded: DnsCredential = + decode(&encoded).unwrap_or_else(|err| panic!("{label} decode failed: {err}")); + assert_eq!(decoded.id, credential.id, "{label}"); + assert_eq!(decoded.max_dns_wait, credential.max_dns_wait, "{label}"); + assert_eq!(decoded.dns_txt_ttl, credential.dns_txt_ttl, "{label}"); + let DnsProvider::Cloudflare { api_token, api_url } = decoded.provider; + assert_eq!(api_token, "token", "{label}"); + assert_eq!( + api_url.as_deref(), + Some("https://api.cloudflare.com/client/v4"), + "{label}" + ); + } + } +} + #[cfg(test)] mod peer_url_tests { use super::validate_peer_url; From af205625963d80a20ca3123f8a89effeb078aec8 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sat, 8 Aug 2026 02:41:43 -0700 Subject: [PATCH 3/4] fix(ra-tls): correct the app info decode error message --- dstack/ra-tls/src/traits.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dstack/ra-tls/src/traits.rs b/dstack/ra-tls/src/traits.rs index 5cd8e7c58..0916039c1 100644 --- a/dstack/ra-tls/src/traits.rs +++ b/dstack/ra-tls/src/traits.rs @@ -43,8 +43,8 @@ pub trait CertExt { let Some(app_info_bytes) = self.get_extension_bytes(PHALA_RATLS_APP_INFO)? else { return Ok(None); }; - let app_info = - rmp_serde::from_slice(&app_info_bytes).context("Failed to decode app info as json")?; + let app_info = rmp_serde::from_slice(&app_info_bytes) + .context("failed to decode app info as msgpack")?; Ok(app_info) } } From 93f738f7a9b99581d7dc3ea92f6c205a42b6b9f4 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sat, 8 Aug 2026 03:09:20 -0700 Subject: [PATCH 4/4] test: accept msgpack map and array headers of any width --- dstack/gateway/src/kv/mod.rs | 23 +++++++++++++++++------ dstack/ra-tls/src/cert.rs | 13 ++++++++++--- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/dstack/gateway/src/kv/mod.rs b/dstack/gateway/src/kv/mod.rs index 33002913f..09392798c 100644 --- a/dstack/gateway/src/kv/mod.rs +++ b/dstack/gateway/src/kv/mod.rs @@ -1244,6 +1244,19 @@ mod value_encoding_tests { started_at: u64, } + /// True when `bytes` opens with a MessagePack map header of any width. The header + /// widens from fixmap to map16 at 16 entries, so matching on the fixmap range alone + /// would start failing precisely when a value type grows past 15 fields — the case + /// this encoding exists to support. + fn starts_with_msgpack_map(bytes: &[u8]) -> bool { + matches!(bytes.first().copied(), Some(0x80..=0x8f | 0xde | 0xdf)) + } + + /// The positional counterpart: fixarray, array16, or array32. + fn starts_with_msgpack_array(bytes: &[u8]) -> bool { + matches!(bytes.first().copied(), Some(0x90..=0x9f | 0xdc | 0xdd)) + } + #[test] fn values_are_encoded_as_named_maps() { let encoded = encode(&CertRenewLock { @@ -1252,9 +1265,8 @@ mod value_encoding_tests { }) .expect("encode should succeed"); - assert_eq!( - encoded[0] & 0xf0, - 0x80, + assert!( + starts_with_msgpack_map(&encoded), "values must encode as MessagePack maps, not positional arrays" ); } @@ -1285,9 +1297,8 @@ mod value_encoding_tests { started_by: 7, }) .expect("legacy encode should succeed"); - assert_eq!( - legacy[0] & 0xf0, - 0x90, + assert!( + starts_with_msgpack_array(&legacy), "fixture must be a positional array to exercise the legacy path" ); diff --git a/dstack/ra-tls/src/cert.rs b/dstack/ra-tls/src/cert.rs index dd009785e..ed3d71583 100644 --- a/dstack/ra-tls/src/cert.rs +++ b/dstack/ra-tls/src/cert.rs @@ -808,6 +808,14 @@ mod tests { "626262626262626262626262626262626262626262c402e1e2c4036b6d73", ); + /// True when `bytes` opens with a MessagePack map header of any width. The + /// header widens from fixmap to map16 at 16 entries, so matching on the fixmap + /// range alone would start failing precisely when `AppInfo` grows past 15 + /// fields — the case this encoding exists to support. + fn starts_with_msgpack_map(bytes: &[u8]) -> bool { + matches!(bytes.first().copied(), Some(0x80..=0x8f | 0xde | 0xdf)) + } + fn sample_app_info() -> AppInfo { AppInfo { app_id: vec![0xa1, 0xa2, 0xa3], @@ -849,9 +857,8 @@ mod tests { fn named_app_info_decodes_against_legacy_field_set() { let encoded = rmp_serde::to_vec_named(&sample_app_info()).unwrap(); - assert_eq!( - encoded[0] & 0xf0, - 0x80, + assert!( + starts_with_msgpack_map(&encoded), "app info must encode as a MessagePack map, not a positional array" );