Skip to content

Commit 98f03f8

Browse files
kvinwangkvinwang
committed
fix: harden attestation V1 wire format and verification
- Fix is_cbor_map_prefix to cover full CBOR map range (0xa0..=0xbf) - Remove SCALE Encode/Decode impls for VersionedAttestation to avoid consuming all remaining input on decode - Remove ambiguous 0x01 prefix fallback in from_bytes - Change to_bytes to return Result instead of panicking via or_panic - Add report_data_payload binding validation in verify_with_time
1 parent ae8a935 commit 98f03f8

6 files changed

Lines changed: 30 additions & 55 deletions

File tree

‎dstack-attest/src/attestation.rs‎

Lines changed: 17 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use dstack_types::SysConfig;
2222
use dstack_types::{Platform, VmConfig};
2323
use ez_hash::{sha256, Hasher, Sha384};
2424
use or_panic::ResultOrPanic;
25-
use scale::{Decode, Encode, Error as ScaleError, Input, Output};
25+
use scale::{Decode, Encode};
2626
use serde::{Deserialize, Serialize};
2727
use serde_human_bytes as hex_bytes;
2828
use sha2::Digest as _;
@@ -53,7 +53,7 @@ fn read_vm_config() -> Result<String> {
5353
}
5454

5555
fn is_cbor_map_prefix(byte: u8) -> bool {
56-
matches!(byte, 0xa0..=0xbb | 0xbf)
56+
matches!(byte, 0xa0..=0xbf)
5757
}
5858

5959
impl From<Attestation> for AttestationV1 {
@@ -344,34 +344,6 @@ pub enum VersionedAttestation {
344344
},
345345
}
346346

347-
impl Encode for VersionedAttestation {
348-
fn size_hint(&self) -> usize {
349-
self.to_bytes().len()
350-
}
351-
352-
fn encode_to<T: Output + ?Sized>(&self, dest: &mut T) {
353-
dest.write(&self.to_bytes());
354-
}
355-
}
356-
357-
impl Decode for VersionedAttestation {
358-
fn decode<I: Input>(input: &mut I) -> Result<Self, ScaleError> {
359-
let Some(remaining_len) = input.remaining_len()? else {
360-
return Err(ScaleError::from(
361-
"VersionedAttestation requires a bounded input to decode",
362-
));
363-
};
364-
let mut bytes = vec![0u8; remaining_len];
365-
input.read(&mut bytes)?;
366-
Self::from_bytes(&bytes).map_err(|err| {
367-
ScaleError::from(std::io::Error::new(
368-
std::io::ErrorKind::InvalidData,
369-
err.to_string(),
370-
))
371-
})
372-
}
373-
}
374-
375347
impl VersionedAttestation {
376348
/// Decode versioned attestation bytes.
377349
pub fn from_bytes(bytes: &[u8]) -> Result<Self> {
@@ -389,23 +361,17 @@ impl VersionedAttestation {
389361
let attestation = AttestationV1::from_cbor(bytes)?;
390362
return Ok(Self::V1 { attestation });
391363
}
392-
if first == 0x01 && bytes.get(1).is_some_and(|byte| is_cbor_map_prefix(*byte)) {
393-
let attestation = AttestationV1::from_cbor(&bytes[1..])?;
394-
return Ok(Self::V1 { attestation });
395-
}
396364
bail!("Unknown attestation wire format");
397365
}
398366

399367
/// Encode versioned attestation bytes.
400-
pub fn to_bytes(&self) -> Vec<u8> {
368+
pub fn to_bytes(&self) -> Result<Vec<u8>> {
401369
match self {
402-
Self::V0 { attestation } => LegacyVersionedAttestation::V0 {
370+
Self::V0 { attestation } => Ok(LegacyVersionedAttestation::V0 {
403371
attestation: attestation.clone(),
404372
}
405-
.encode(),
406-
Self::V1 { attestation } => attestation
407-
.to_cbor()
408-
.or_panic("attestation schema should encode as CBOR"),
373+
.encode()),
374+
Self::V1 { attestation } => attestation.to_cbor(),
409375
}
410376
}
411377

@@ -415,7 +381,7 @@ impl VersionedAttestation {
415381
}
416382

417383
#[doc(hidden)]
418-
pub fn to_scale(&self) -> Vec<u8> {
384+
pub fn to_scale(&self) -> Result<Vec<u8>> {
419385
self.to_bytes()
420386
}
421387

@@ -548,6 +514,15 @@ impl AttestationV1 {
548514
platform,
549515
stack,
550516
} = self;
517+
// Verify report_data_payload binding: if present, the report_data must
518+
// be derived from the payload via the AppData content type scheme.
519+
if let Some(payload) = stack.report_data_payload() {
520+
let report_data: [u8; 64] = stack.report_data()?;
521+
let expected = QuoteContentType::AppData.to_report_data(payload.as_bytes());
522+
if report_data != expected {
523+
bail!("report_data does not match report_data_payload");
524+
}
525+
}
551526
let (report_data, runtime_events, config) = match stack {
552527
StackEvidence::Dstack {
553528
report_data,
@@ -1308,7 +1283,7 @@ mod tests {
13081283
let attestation = dummy_tdx_attestation(report_data)
13091284
.into_v1()
13101285
.into_dstack_pod(payload.clone());
1311-
let encoded = VersionedAttestation::V1 { attestation }.to_bytes();
1286+
let encoded = VersionedAttestation::V1 { attestation }.to_bytes().unwrap();
13121287
assert!(matches!(encoded.first(), Some(0xa0..=0xbf)));
13131288
let decoded = VersionedAttestation::from_bytes(&encoded)
13141289
.expect("decode attestation")

‎guest-agent-simulator/src/main.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,11 @@ impl PlatformBackend for SimulatorPlatform {
7070
}
7171

7272
fn certificate_attestation(&self, pubkey: &[u8]) -> Result<VersionedAttestation> {
73-
Ok(simulator::simulated_certificate_attestation(
73+
simulator::simulated_certificate_attestation(
7474
&self.attestation,
7575
pubkey,
7676
self.patch_report_data,
77-
))
77+
)
7878
}
7979

8080
fn quote_response(&self, report_data: [u8; 64], vm_config: &str) -> Result<GetQuoteResponse> {
@@ -87,11 +87,11 @@ impl PlatformBackend for SimulatorPlatform {
8787
}
8888

8989
fn attest_response(&self, report_data: [u8; 64]) -> Result<AttestResponse> {
90-
Ok(simulator::simulated_attest_response(
90+
simulator::simulated_attest_response(
9191
&self.attestation,
9292
report_data,
9393
self.patch_report_data,
94-
))
94+
)
9595
}
9696

9797
fn emit_event(&self, event: &str, _payload: &[u8]) -> Result<()> {

‎guest-agent-simulator/src/simulator.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ pub fn simulated_attest_response(
4848
attestation: &VersionedAttestation,
4949
report_data: [u8; 64],
5050
patch_report_data: bool,
51-
) -> AttestResponse {
51+
) -> Result<AttestResponse> {
5252
let attestation =
5353
maybe_patch_report_data(attestation, report_data, patch_report_data, "attest");
54-
AttestResponse {
55-
attestation: VersionedAttestation::V1 { attestation }.to_bytes(),
56-
}
54+
Ok(AttestResponse {
55+
attestation: VersionedAttestation::V1 { attestation }.to_bytes()?,
56+
})
5757
}
5858

5959
pub fn simulated_info_attestation(attestation: &VersionedAttestation) -> VersionedAttestation {
@@ -64,15 +64,15 @@ pub fn simulated_certificate_attestation(
6464
attestation: &VersionedAttestation,
6565
pubkey: &[u8],
6666
patch_report_data: bool,
67-
) -> VersionedAttestation {
67+
) -> Result<VersionedAttestation> {
6868
let report_data = QuoteContentType::RaTlsCert.to_report_data(pubkey);
6969
let attestation = maybe_patch_report_data(
7070
attestation,
7171
report_data,
7272
patch_report_data,
7373
"certificate_attestation",
7474
);
75-
VersionedAttestation::V1 { attestation }
75+
Ok(VersionedAttestation::V1 { attestation })
7676
}
7777

7878
fn maybe_patch_report_data(

‎guest-agent/src/backend.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ impl PlatformBackend for RealPlatform {
4848
fn attest_response(&self, report_data: [u8; 64]) -> Result<AttestResponse> {
4949
let attestation = Attestation::quote(&report_data).context("Failed to get attestation")?;
5050
Ok(AttestResponse {
51-
attestation: attestation.into_versioned().to_bytes(),
51+
attestation: attestation.into_versioned().to_bytes()?,
5252
})
5353
}
5454

‎guest-agent/src/rpc_service.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,7 @@ pNs85uhOZE8z2jr8Pg==
899899
fn attest_response(&self, report_data: [u8; 64]) -> Result<AttestResponse> {
900900
let attestation = patch_report_data(&self.attestation, report_data);
901901
Ok(AttestResponse {
902-
attestation: VersionedAttestation::V1 { attestation }.to_bytes(),
902+
attestation: VersionedAttestation::V1 { attestation }.to_bytes()?,
903903
})
904904
}
905905

‎ra-tls/src/cert.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ impl<Key> CertRequest<'_, Key> {
388388
add_ext(&mut params, PHALA_RATLS_CERT_USAGE, usage);
389389
}
390390
if let Some(ver_att) = self.attestation {
391-
let attestation_bytes = ver_att.clone().into_stripped().to_bytes();
391+
let attestation_bytes = ver_att.clone().into_stripped().to_bytes()?;
392392
add_ext(&mut params, PHALA_RATLS_ATTESTATION, &attestation_bytes);
393393
}
394394
if let Some(ca_level) = self.ca_level {

0 commit comments

Comments
 (0)