Skip to content

Commit ab56b9b

Browse files
committed
fix(sgx-dcap-quoteverify-rs): don't assert in tee_qv_get_collateral()
use `TryFrom<&sgx_ql_qve_collateral_t>` instead of `From<sgx_ql_qve_collateral_t>` * return an error, if something goes wrong, instead of panic * don't take ownership of the memory Signed-off-by: Harald Hoyer <harald@matterlabs.dev>
1 parent 621a085 commit ab56b9b

File tree

1 file changed

+28
-22
lines changed
  • QuoteVerification/dcap_quoteverify/sgx-dcap-quoteverify-rs/src

1 file changed

+28
-22
lines changed

QuoteVerification/dcap_quoteverify/sgx-dcap-quoteverify-rs/src/lib.rs

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -69,34 +69,38 @@ pub struct QuoteCollateral {
6969
pub qe_identity: Vec<c_char>,
7070
}
7171

72-
impl From<sgx_ql_qve_collateral_t> for QuoteCollateral {
73-
fn from(collateral: sgx_ql_qve_collateral_t) -> Self {
74-
fn raw_ptr_to_vec(data: *mut c_char, len: u32) -> Vec<c_char> {
75-
assert!(!data.is_null());
76-
unsafe { slice::from_raw_parts(data, len as _) }.to_vec()
72+
impl TryFrom<&sgx_ql_qve_collateral_t> for QuoteCollateral {
73+
type Error = ();
74+
75+
fn try_from(collateral: &sgx_ql_qve_collateral_t) -> Result<Self, Self::Error> {
76+
fn raw_ptr_to_vec(data: *mut c_char, len: u32) -> Result<Vec<c_char>, ()> {
77+
if data.is_null() {
78+
return Err(());
79+
}
80+
Ok(unsafe { slice::from_raw_parts(data, len as _) }.to_vec())
7781
}
7882

79-
QuoteCollateral {
83+
Ok(QuoteCollateral {
8084
major_version: unsafe { collateral.__bindgen_anon_1.__bindgen_anon_1.major_version },
8185
minor_version: unsafe { collateral.__bindgen_anon_1.__bindgen_anon_1.minor_version },
8286
tee_type: collateral.tee_type,
8387
pck_crl_issuer_chain: raw_ptr_to_vec(
8488
collateral.pck_crl_issuer_chain,
8589
collateral.pck_crl_issuer_chain_size,
86-
),
87-
root_ca_crl: raw_ptr_to_vec(collateral.root_ca_crl, collateral.root_ca_crl_size),
88-
pck_crl: raw_ptr_to_vec(collateral.pck_crl, collateral.pck_crl_size),
90+
)?,
91+
root_ca_crl: raw_ptr_to_vec(collateral.root_ca_crl, collateral.root_ca_crl_size)?,
92+
pck_crl: raw_ptr_to_vec(collateral.pck_crl, collateral.pck_crl_size)?,
8993
tcb_info_issuer_chain: raw_ptr_to_vec(
9094
collateral.tcb_info_issuer_chain,
9195
collateral.tcb_info_issuer_chain_size,
92-
),
93-
tcb_info: raw_ptr_to_vec(collateral.tcb_info, collateral.tcb_info_size),
96+
)?,
97+
tcb_info: raw_ptr_to_vec(collateral.tcb_info, collateral.tcb_info_size)?,
9498
qe_identity_issuer_chain: raw_ptr_to_vec(
9599
collateral.qe_identity_issuer_chain,
96100
collateral.qe_identity_issuer_chain_size,
97-
),
98-
qe_identity: raw_ptr_to_vec(collateral.qe_identity, collateral.qe_identity_size),
99-
}
101+
)?,
102+
qe_identity: raw_ptr_to_vec(collateral.qe_identity, collateral.qe_identity_size)?,
103+
})
100104
}
101105
}
102106

@@ -415,15 +419,17 @@ pub fn tee_qv_get_collateral(quote: &[u8]) -> Result<QuoteCollateral, quote3_err
415419
qvl_sys::tee_qv_get_collateral(quote.as_ptr(), quote.len() as u32, &mut buf, &mut buf_len)
416420
} {
417421
quote3_error_t::SGX_QL_SUCCESS => {
418-
assert!(!buf.is_null());
419-
assert!(buf_len > 0);
420-
assert_eq!(
421-
buf.align_offset(mem::align_of::<sgx_ql_qve_collateral_t>()),
422-
0
423-
);
422+
if buf.is_null()
423+
|| buf_len == 0
424+
|| buf.align_offset(mem::align_of::<sgx_ql_qve_collateral_t>()) != 0
425+
{
426+
return Err(quote3_error_t::SGX_QL_NO_QUOTE_COLLATERAL_DATA);
427+
}
424428

425-
let collateral =
426-
QuoteCollateral::from(unsafe { *(buf as *const sgx_ql_qve_collateral_t) });
429+
// SAFETY: buf is not null, buf_len is not zero, and buf is aligned.
430+
let orig_collateral = &unsafe { *(buf as *const sgx_ql_qve_collateral_t) };
431+
let collateral = QuoteCollateral::try_from(orig_collateral)
432+
.map_err(|_| quote3_error_t::SGX_QL_NO_QUOTE_COLLATERAL_DATA)?;
427433

428434
match unsafe { qvl_sys::tee_qv_free_collateral(buf) } {
429435
quote3_error_t::SGX_QL_SUCCESS => Ok(collateral),

0 commit comments

Comments
 (0)