Skip to content

Commit b95a2f7

Browse files
haitaohuangCopilot
authored andcommitted
fix(attestation): differentiate retriable errors from non-retriable ones
Classify tdvmcall_get_quote and buffer status errors into 2 categories: - Busy: VMCALL_RETRY, GET_QUOTE_IN_FLIGHT, GET_QUOTE_ERROR, GET_QUOTE_SERVICE_UNAVAILABLE Retried with exponential backoff (1s initial), up to 5 attempts. - Non-retriable: VMCALL_OPERAND_INVALID and other errors cause immediate failure. Propagate error categories through attest.rs and igvmattest.rs. Update mock quote emulation to return Ok with error status in buffer (matching real VMM behavior) instead of returning Err, and trigger the notification interrupt so ghci.rs wakes from wait_for_vmm_notification. Add igvm-attest feature to mock-quote-retry CI test since retry differentiation only applies to the igvm-attest path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
1 parent e6af7bd commit b95a2f7

9 files changed

Lines changed: 68 additions & 27 deletions

File tree

.github/workflows/integration-emu.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ jobs:
8989
test-type: "mock-quote-retry"
9090
install-jq: 'false'
9191
timeout-seconds: 900
92-
test-command: "./migtdemu.sh --mock-report --mock-quote-retry --both --no-sudo --log-level info"
92+
test-command: "./migtdemu.sh --mock-report --mock-quote-retry --features igvm-attest --both --no-sudo --log-level info"
9393
artifact-name: "mock-quote-retry-test-logs"
9494

9595
steps:

deps/td-shim-AzCVMEmu/tdx-tdcall/src/tdx_emu.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1454,9 +1454,16 @@ fn handle_quote_request(
14541454
Ok(quote) => quote,
14551455
Err(e) => {
14561456
error!("Failed to generate quote in AzCVMEmu mode: {:?}", e);
1457+
// Set error status in buffer. Return Ok so ghci.rs proceeds to
1458+
// check the buffer status and classifies it as a retriable error.
14571459
let error_status = 0x8000000000000000u64;
14581460
buffer[8..16].copy_from_slice(&error_status.to_le_bytes());
1459-
return Err(TdVmcallError::Other);
1461+
// Trigger event notification so ghci.rs wakes from wait_for_vmm_notification
1462+
if let Some(vector) = *EVENT_NOTIFY_VECTOR.lock() {
1463+
log::info!("AzCVMEmu: Triggering interrupt vector {} (error path)", vector);
1464+
intr::trigger(vector as u8);
1465+
}
1466+
return Ok(());
14601467
}
14611468
};
14621469

migtdemu.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ show_usage() {
6262
echo " --skip-ra Skip remote attestation (uses mock TD reports/quotes for non-TDX environments)"
6363
echo " --mock-report Use mock report data for RA and policy v2 (non-TDX, but full attestation flow)"
6464
echo " --mock-quote-file FILE Path to mock quote file (used with --mock-report, defaults to output_data_v4.bin)"
65-
echo " --mock-quote-retry Enable mock_quote_retry feature (get_quote fails first 8 times to test retry logic)"
65+
echo " --mock-quote-retry Enable mock_quote_retry feature (get_quote fails first 5 times to test retry logic)"
6666
echo " --both Start destination first, then source (same host)"
6767
echo " --no-sudo Run without sudo (useful for local testing)"
6868
echo " --features FEATURES Add extra cargo features (comma-separated, e.g., 'spdm_attestation,feature2')"

src/attestation/src/attest.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::binding::verify_quote_integrity_ex;
99
use crate::binding::get_quote as get_quote_inner;
1010

1111
use crate::{
12-
binding::{init_heap, verify_quote_integrity, QveCollateral},
12+
binding::{init_heap, verify_quote_integrity, AttestLibError, QveCollateral},
1313
root_ca::ROOT_CA_PUBLIC_KEY,
1414
Error, TD_VERIFIED_REPORT_SIZE,
1515
};
@@ -133,7 +133,10 @@ pub fn get_quote(td_report: &[u8]) -> Result<Vec<u8>, Error> {
133133
);
134134
if result != 0 {
135135
log::error!("get_quote_inner failed with error: {:#x}\n", result);
136-
return Err(Error::GetQuote);
136+
return Err(match result {
137+
x if x == AttestLibError::Busy as i32 => Error::Busy,
138+
_ => Error::GetQuote,
139+
});
137140
}
138141
}
139142
quote.truncate(quote_size as usize);

src/attestation/src/ghci.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use td_payload::arch::apic::{disable, enable_and_hlt};
99
use td_payload::arch::idt::{register_interrupt_callback, InterruptCallback, InterruptStack};
1010
use td_payload::mm::shared::SharedMemory;
1111
use tdx_tdcall::tdx::tdvmcall_get_quote;
12+
use tdx_tdcall::TdVmcallError;
1213

1314
use crate::binding::AttestLibError;
1415

@@ -18,6 +19,9 @@ const GET_QUOTE_MAX_SIZE: u64 = 32 * 0x1000;
1819
const GET_QUOTE_STATUS_FIELD: Range<usize> = 8..16;
1920
const GET_QUOTE_STATUS_SUCCESS: u64 = 0;
2021
const GET_QUOTE_STATUS_IN_FLIGHT: u64 = 0xFFFFFFFF_FFFFFFFF;
22+
// Any quote error: e.g., impactless update causing report verification failure, upper layer should retry.
23+
const GET_QUOTE_STATUS_ERROR: u64 = 0x80000000_00000000;
24+
const GET_QUOTE_STATUS_SERVICE_UNAVAILABLE: u64 = 0x80000000_00000001;
2125

2226
pub static NOTIFIER: AtomicU8 = AtomicU8::new(0);
2327

@@ -44,9 +48,16 @@ pub extern "C" fn servtd_get_quote(tdquote_req_buf: *mut c_void, len: u64) -> i3
4448

4549
let notify_registered = set_vmm_notification();
4650

47-
if let Err(e) = tdvmcall_get_quote(shared.as_mut_bytes()) {
48-
log::error!("tdvmcall_get_quote failed with error: {:?}\n", e);
49-
return AttestLibError::QuoteFailure as i32;
51+
match tdvmcall_get_quote(shared.as_mut_bytes()) {
52+
Ok(()) => {}
53+
Err(TdVmcallError::VmcallRetry) => {
54+
log::error!("tdvmcall_get_quote returned RETRY\n");
55+
return AttestLibError::Busy as i32;
56+
}
57+
Err(e) => {
58+
log::error!("tdvmcall_get_quote failed with error: {:?}\n", e);
59+
return AttestLibError::QuoteFailure as i32;
60+
}
5061
}
5162

5263
let wait_result = wait_for_quote_completion(notify_registered, shared.as_bytes());
@@ -116,11 +127,21 @@ fn wait_for_quote_completion(notify_registered: bool, buffer: &[u8]) -> Result<(
116127
}
117128
};
118129

119-
if status_code == GET_QUOTE_STATUS_SUCCESS {
120-
Ok(())
121-
} else {
122-
log::error!("Quote status indicates failure: {:#x}\n", status_code);
123-
Err(AttestLibError::QuoteFailure)
130+
match status_code {
131+
GET_QUOTE_STATUS_SUCCESS => Ok(()),
132+
GET_QUOTE_STATUS_SERVICE_UNAVAILABLE
133+
| GET_QUOTE_STATUS_ERROR
134+
| GET_QUOTE_STATUS_IN_FLIGHT => {
135+
log::error!(
136+
"Quote status indicates service unavailable or other retriable error: {:#x}\n",
137+
status_code
138+
);
139+
Err(AttestLibError::Busy)
140+
}
141+
_ => {
142+
log::error!("Quote status indicates failure: {:#x}\n", status_code);
143+
Err(AttestLibError::QuoteFailure)
144+
}
124145
}
125146
}
126147

src/attestation/src/igvmattest.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//
33
// SPDX-License-Identifier: BSD-2-Clause-Patent
44

5-
use crate::{attest::TD_QUOTE_SIZE, Error};
5+
use crate::{attest::TD_QUOTE_SIZE, binding::AttestLibError, Error};
66
#[cfg(feature = "igvm-attest")]
77
use alloc::{vec, vec::Vec};
88
use core::ffi::c_void;
@@ -92,7 +92,11 @@ pub fn get_quote_igvm(td_report: &[u8]) -> Result<Vec<u8>, Error> {
9292
(*hdr)
9393
);
9494
}
95-
return Err(Error::GetQuote);
95+
const BUSY: i32 = AttestLibError::Busy as i32;
96+
return Err(match servtd_get_quote_ret {
97+
BUSY => Error::Busy,
98+
_ => Error::GetQuote,
99+
});
96100
}
97101

98102
let hdr = get_quote_blob_ptr as *mut ServtdTdxQuoteHdr;

src/attestation/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,6 @@ pub enum Error {
8989
InvalidOutput,
9090
InvalidQuote,
9191
OutOfMemory,
92+
/// Service busy/unavailable/transient error, caller should retry
93+
Busy,
9294
}

src/migtd/src/quote.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ pub enum QuoteError {
3636
QuoteGenerationFailed,
3737
}
3838

39-
/// Get a quote with retry logic to handle potential security updates
39+
/// Get a quote with retry logic to handle transient and retriable errors
4040
///
41-
/// On quote failure, fetches a new TD REPORT and retries with exponential backoff.
41+
/// On retriable errors, retries with exponential backoff starting at 1s.
42+
/// Non-retriable errors cause immediate failure.
4243
///
4344
/// # Arguments
4445
/// * `additional_data` - The 64-byte additional data to include in the TD REPORT
@@ -63,21 +64,25 @@ pub fn get_quote_with_retry(additional_data: &[u8; 64]) -> Result<(Vec<u8>, Vec<
6364
log::info!("Quote generated successfully\n");
6465
return Ok((quote, report_bytes.to_vec()));
6566
}
66-
_ => {
67+
Err(attestation::Error::Busy) => {
6768
attempt += 1;
6869
if attempt > MAX_RETRIES {
6970
log::error!("GetQuote failed after {} attempts\n", attempt);
7071
return Err(QuoteError::QuoteGenerationFailed);
7172
}
7273
log::warn!(
73-
"GetQuote failed (attempt {}/{}), retrying in {}ms\n",
74+
"GetQuote returned Busy (attempt {}/{}), retrying in {}ms\n",
7475
attempt,
7576
MAX_RETRIES + 1,
7677
busy_delay_ms
7778
);
7879
delay_milliseconds(busy_delay_ms);
7980
busy_delay_ms *= 2;
8081
}
82+
Err(e) => {
83+
log::error!("GetQuote failed with non-retriable error: {:?}\n", e);
84+
return Err(QuoteError::QuoteGenerationFailed);
85+
}
8186
}
8287
}
8388
}

src/migtd/src/ratls/server_client.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -220,14 +220,19 @@ pub fn client_rebinding<T: AsyncRead + AsyncWrite + Unpin>(
220220
})
221221
}
222222

223-
fn gen_quote(public_key: &[u8]) -> Result<Vec<u8>> {
223+
fn prepare_report_data(public_key: &[u8]) -> Result<[u8; 64]> {
224224
let hash = digest_sha384(public_key).map_err(|e| {
225225
log::error!("Failed to compute SHA384 digest: {:?}\n", e);
226226
e
227227
})?;
228228

229229
let mut additional_data = [0u8; 64];
230230
additional_data[..hash.len()].copy_from_slice(hash.as_ref());
231+
Ok(additional_data)
232+
}
233+
234+
fn gen_quote(public_key: &[u8]) -> Result<Vec<u8>> {
235+
let additional_data = prepare_report_data(public_key)?;
231236

232237
let (quote, _report) = crate::quote::get_quote_with_retry(&additional_data).map_err(|e| {
233238
log::error!("get_quote_with_retry failed: {:?}\n", e);
@@ -238,15 +243,9 @@ fn gen_quote(public_key: &[u8]) -> Result<Vec<u8>> {
238243
}
239244

240245
pub fn gen_tdreport(public_key: &[u8]) -> Result<TdxReport> {
241-
let hash = digest_sha384(public_key).map_err(|e| {
242-
log::error!("Failed to compute SHA384 digest: {:?}\n", e);
243-
e
244-
})?;
246+
let additional_data = prepare_report_data(public_key)?;
245247

246248
// Generate the TD Report that contains the public key hash as nonce
247-
let mut additional_data = [0u8; 64];
248-
additional_data[..hash.len()].copy_from_slice(hash.as_ref());
249-
250249
tdx_tdcall::tdreport::tdcall_report(&additional_data).map_err(|e| {
251250
log::error!("Failed to get TD report via tdcall. Error: {:?}\n", e);
252251
e.into()

0 commit comments

Comments
 (0)