Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return encoded extrinsics without padding #1505

Merged
merged 3 commits into from
Dec 4, 2023
Merged

Return encoded extrinsics without padding #1505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
82ee4c0
Trim padded xt
Kailai-Wang Nov 28, 2023
eac4997
fix compile
Kailai-Wang Nov 28, 2023
224587e
Merge branch 'master' into trim-padded-xt
Kailai-Wang Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions core-primitives/enclave-api/ffi/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ extern "C" {
quote: *const u8,
quote_size: u32,
unchecked_extrinsic: *mut u8,
unchecked_extrinsic_size: u32,
unchecked_extrinsic_max_size: u32,
unchecked_extrinsic_size: *mut u32,
) -> sgx_status_t;

pub fn init(
Expand Down Expand Up @@ -139,7 +140,8 @@ extern "C" {
w_url: *const u8,
w_url_size: u32,
unchecked_extrinsic: *mut u8,
unchecked_extrinsic_size: u32,
unchecked_extrinsic_max_size: u32,
unchecked_extrinsic_size: *mut u32,
skip_ra: c_int,
) -> sgx_status_t;

Expand All @@ -149,7 +151,8 @@ extern "C" {
w_url: *const u8,
w_url_size: u32,
unchecked_extrinsic: *mut u8,
unchecked_extrinsic_size: u32,
unchecked_extrinsic_max_size: u32,
unchecked_extrinsic_size: *mut u32,
skip_ra: c_int,
quoting_enclave_target_info: Option<&sgx_target_info_t>,
quote_size: Option<&u32>,
Expand All @@ -170,15 +173,17 @@ extern "C" {
retval: *mut sgx_status_t,
collateral: *const sgx_ql_qve_collateral_t,
unchecked_extrinsic: *mut u8,
unchecked_extrinsic_size: u32,
unchecked_extrinsic_max_size: u32,
unchecked_extrinsic_size: *mut u32,
) -> sgx_status_t;

pub fn generate_register_tcb_info_extrinsic(
eid: sgx_enclave_id_t,
retval: *mut sgx_status_t,
collateral: *const sgx_ql_qve_collateral_t,
unchecked_extrinsic: *mut u8,
unchecked_extrinsic_size: u32,
unchecked_extrinsic_max_size: u32,
unchecked_extrinsic_size: *mut u32,
) -> sgx_status_t;

pub fn dump_ias_ra_cert_to_disk(
Expand Down Expand Up @@ -218,7 +223,8 @@ extern "C" {
fiat_currency: *const u8,
fiat_currency_size: u32,
unchecked_extrinsic: *mut u8,
unchecked_extrinsic_size: u32,
unchecked_extrinsic_max_size: u32,
unchecked_extrinsic_size: *mut u32,
) -> sgx_status_t;

pub fn update_weather_data_xt(
Expand All @@ -229,7 +235,8 @@ extern "C" {
weather_info_latitude: *const u8,
weather_info_latitude_size: u32,
unchecked_extrinsic: *mut u8,
unchecked_extrinsic_size: u32,
unchecked_extrinsic_max_size: u32,
unchecked_extrinsic_size: *mut u32,
) -> sgx_status_t;

pub fn run_state_provisioning_server(
Expand Down
23 changes: 16 additions & 7 deletions core-primitives/enclave-api/src/remote_attestation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,9 @@ mod impl_ffi {
let mut retval = sgx_status_t::SGX_SUCCESS;

let mut unchecked_extrinsic: Vec<u8> = vec![0u8; EXTRINSIC_MAX_SIZE];
let mut unchecked_extrinsic_size: u32 = 0;

trace!("Generating dcap_ra_extrinsic with URL: {}", w_url);
trace!("Generating ias_ra_extrinsic with URL: {}", w_url);
Kailai-Wang marked this conversation as resolved.
Show resolved Hide resolved

let url = w_url.encode();

Expand All @@ -157,14 +158,15 @@ mod impl_ffi {
url.len() as u32,
unchecked_extrinsic.as_mut_ptr(),
unchecked_extrinsic.len() as u32,
&mut unchecked_extrinsic_size as *mut u32,
skip_ra.into(),
)
};

ensure!(result == sgx_status_t::SGX_SUCCESS, Error::Sgx(result));
ensure!(retval == sgx_status_t::SGX_SUCCESS, Error::Sgx(retval));

Ok(unchecked_extrinsic)
Ok(Vec::from(&unchecked_extrinsic[..unchecked_extrinsic_size as usize]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a check that unchecked_extrinsic_size < unchecked_extrinsic.len() to prevent panics. This true for all the following usages too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is implicitly guaranteed by the ffi impl, isn't it

If it's out of index, it should have error'ed out within write_slice_and_whitespace_pad

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yes and no. If we have implemented everything correctly, yes, but the compiler can't make any checks across the ffi-boundaries. So I prefer being defensive here, as it just needs an error of some programmer in the future to introduce potential panics here.

Sorry, for being nitpicky here. 🙏

Copy link
Contributor

@clangenb clangenb Nov 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, on the other hand the panic is on the untrusted side, so returning an error or panicking here, is essentially the same from the enclave's perspective.

hmm, still I would like to push the best practices as much as possible, so please fix it. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it is actually not the same, one case writes it in the, mut sgx_staus_t result and the other one returns the sgx error from the function. :)

}
fn generate_dcap_ra_extrinsic_from_quote(
&self,
Expand All @@ -173,6 +175,7 @@ mod impl_ffi {
) -> EnclaveResult<Vec<u8>> {
let mut retval = sgx_status_t::SGX_SUCCESS;
let mut unchecked_extrinsic: Vec<u8> = vec![0u8; EXTRINSIC_MAX_SIZE];
let mut unchecked_extrinsic_size: u32 = 0;
let url = url.encode();

let result = unsafe {
Expand All @@ -185,13 +188,14 @@ mod impl_ffi {
quote.len() as u32,
unchecked_extrinsic.as_mut_ptr(),
unchecked_extrinsic.len() as u32,
&mut unchecked_extrinsic_size as *mut u32,
)
};

ensure!(result == sgx_status_t::SGX_SUCCESS, Error::Sgx(result));
ensure!(retval == sgx_status_t::SGX_SUCCESS, Error::Sgx(retval));

Ok(unchecked_extrinsic.to_vec())
Ok(Vec::from(&unchecked_extrinsic[..unchecked_extrinsic_size as usize]))
}

fn generate_dcap_ra_quote(&self, skip_ra: bool) -> EnclaveResult<Vec<u8>> {
Expand Down Expand Up @@ -250,7 +254,7 @@ mod impl_ffi {
trace!("Generating dcap_ra_extrinsic with URL: {}", w_url);

let mut unchecked_extrinsic: Vec<u8> = vec![0u8; EXTRINSIC_MAX_SIZE];

let mut unchecked_extrinsic_size: u32 = 0;
let url = w_url.encode();

let result = unsafe {
Expand All @@ -261,6 +265,7 @@ mod impl_ffi {
url.len() as u32,
unchecked_extrinsic.as_mut_ptr(),
unchecked_extrinsic.len() as u32,
&mut unchecked_extrinsic_size as *mut u32,
skip_ra.into(),
quoting_enclave_target_info.as_ref(),
quote_size.as_ref(),
Expand All @@ -270,7 +275,7 @@ mod impl_ffi {
ensure!(result == sgx_status_t::SGX_SUCCESS, Error::Sgx(result));
ensure!(retval == sgx_status_t::SGX_SUCCESS, Error::Sgx(retval));

Ok(unchecked_extrinsic)
Ok(Vec::from(&unchecked_extrinsic[..unchecked_extrinsic_size as usize]))
}

fn generate_register_quoting_enclave_extrinsic(
Expand All @@ -279,6 +284,7 @@ mod impl_ffi {
) -> EnclaveResult<Vec<u8>> {
let mut retval = sgx_status_t::SGX_SUCCESS;
let mut unchecked_extrinsic: Vec<u8> = vec![0u8; EXTRINSIC_MAX_SIZE];
let mut unchecked_extrinsic_size: u32 = 0;

trace!("Generating register quoting enclave");

Expand All @@ -291,6 +297,7 @@ mod impl_ffi {
collateral_ptr,
unchecked_extrinsic.as_mut_ptr(),
unchecked_extrinsic.len() as u32,
&mut unchecked_extrinsic_size as *mut u32,
)
};
let free_status = unsafe { sgx_ql_free_quote_verification_collateral(collateral_ptr) };
Expand All @@ -301,12 +308,13 @@ mod impl_ffi {
Error::SgxQuote(free_status)
);

Ok(unchecked_extrinsic)
Ok(Vec::from(&unchecked_extrinsic[..unchecked_extrinsic_size as usize]))
}

fn generate_register_tcb_info_extrinsic(&self, fmspc: Fmspc) -> EnclaveResult<Vec<u8>> {
let mut retval = sgx_status_t::SGX_SUCCESS;
let mut unchecked_extrinsic: Vec<u8> = vec![0u8; EXTRINSIC_MAX_SIZE];
let mut unchecked_extrinsic_size: u32 = 0;

trace!("Generating tcb_info registration");

Expand All @@ -319,6 +327,7 @@ mod impl_ffi {
collateral_ptr,
unchecked_extrinsic.as_mut_ptr(),
unchecked_extrinsic.len() as u32,
&mut unchecked_extrinsic_size as *mut u32,
)
};
let free_status = unsafe { sgx_ql_free_quote_verification_collateral(collateral_ptr) };
Expand All @@ -329,7 +338,7 @@ mod impl_ffi {
Error::SgxQuote(free_status)
);

Ok(unchecked_extrinsic)
Ok(Vec::from(&unchecked_extrinsic[..unchecked_extrinsic_size as usize]))
}

fn dump_ias_ra_cert_to_disk(&self) -> EnclaveResult<()> {
Expand Down
20 changes: 12 additions & 8 deletions core-primitives/enclave-api/src/teeracle_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ mod impl_ffi {
crypto_currency, fiat_currency
);
let mut retval = sgx_status_t::SGX_SUCCESS;
let response_len = 8192;
let mut response: Vec<u8> = vec![0u8; response_len as usize];
let response_max_len = 8192;
let mut response: Vec<u8> = vec![0u8; response_max_len as usize];
let mut response_len: u32 = 0;

let crypto_curr = crypto_currency.encode();
let fiat_curr = fiat_currency.encode();
Expand All @@ -64,14 +65,15 @@ mod impl_ffi {
fiat_curr.as_ptr(),
fiat_curr.len() as u32,
response.as_mut_ptr(),
response_len,
response_max_len,
&mut response_len as *mut u32,
)
};

ensure!(res == sgx_status_t::SGX_SUCCESS, Error::Sgx(res));
ensure!(retval == sgx_status_t::SGX_SUCCESS, Error::Sgx(retval));

Ok(response)
Ok(Vec::from(&response[..response_len as usize]))
}
fn update_weather_data_xt(
&self,
Expand All @@ -83,8 +85,9 @@ mod impl_ffi {
latitude, longitude
);
let mut retval = sgx_status_t::SGX_SUCCESS;
let response_len = 8192;
let mut response: Vec<u8> = vec![0u8; response_len as usize];
let response_max_len = 8192;
let mut response: Vec<u8> = vec![0u8; response_max_len as usize];
let mut response_len: u32 = 0;

let longitude_encoded: Vec<u8> = longitude.encode();
let latitude_encoded: Vec<u8> = latitude.encode();
Expand All @@ -98,13 +101,14 @@ mod impl_ffi {
latitude_encoded.as_ptr(),
latitude_encoded.len() as u32,
response.as_mut_ptr(),
response_len,
response_max_len,
&mut response_len as *mut u32,
)
};

ensure!(res == sgx_status_t::SGX_SUCCESS, Error::Sgx(res));
ensure!(retval == sgx_status_t::SGX_SUCCESS, Error::Sgx(retval));
Ok(response)
Ok(Vec::from(&response[..response_len as usize]))
}
}
}
15 changes: 13 additions & 2 deletions core-primitives/utils/src/buffer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@
use alloc::vec::Vec;

/// Fills a given buffer with data and the left over buffer space with white spaces.
/// Throw an error if the buffer size is not enough to hold `data`,
/// return the length of `data` otherwise.
pub fn write_slice_and_whitespace_pad(
clangenb marked this conversation as resolved.
Show resolved Hide resolved
writable: &mut [u8],
data: Vec<u8>,
) -> Result<(), BufferError> {
) -> Result<usize, BufferError> {
if data.len() > writable.len() {
return Err(BufferError::InsufficientBufferSize {
actual: writable.len(),
Expand All @@ -34,7 +36,7 @@ pub fn write_slice_and_whitespace_pad(
left.clone_from_slice(&data);
// fill the right side with whitespace
right.iter_mut().for_each(|x| *x = 0x20);
Ok(())
Ok(data.len())
}

#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
Expand All @@ -47,6 +49,15 @@ mod tests {
use super::*;
use alloc::vec;

#[test]
fn write_slice_and_whitespace_pad_works() {
let mut writable = vec![0; 32];
let data = vec![1; 30];
assert_eq!(write_slice_and_whitespace_pad(&mut writable, data), Ok(30));
assert_eq!(&writable[..30], vec![1; 30]);
assert_eq!(&writable[30..], vec![0x20; 2]);
}
Kailai-Wang marked this conversation as resolved.
Show resolved Hide resolved

#[test]
fn write_slice_and_whitespace_pad_returns_error_if_buffer_too_small() {
let mut writable = vec![0; 32];
Expand Down
21 changes: 14 additions & 7 deletions enclave-runtime/Enclave.edl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ enclave {

public sgx_status_t generate_ias_ra_extrinsic(
[in, size=w_url_size] uint8_t* w_url, uint32_t w_url_size,
[out, size=unchecked_extrinsic_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_size,
[out, size=unchecked_extrinsic_max_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_max_size,
[out] uint32_t* unchecked_extrinsic_size,
int skip_ra
);
public sgx_status_t generate_dcap_ra_quote(
Expand All @@ -113,37 +114,43 @@ enclave {
public sgx_status_t generate_dcap_ra_extrinsic_from_quote(
[in, size=w_url_size] uint8_t* w_url, uint32_t w_url_size,
[in, size=quote_size] uint8_t* quote, uint32_t quote_size,
[out, size=unchecked_extrinsic_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_size
[out, size=unchecked_extrinsic_max_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_max_size,
[out] uint32_t* unchecked_extrinsic_size
);

public sgx_status_t generate_dcap_ra_extrinsic(
[in, size=w_url_size] uint8_t* w_url, uint32_t w_url_size,
[out, size=unchecked_extrinsic_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_size,
[out, size=unchecked_extrinsic_max_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_max_size,
[out] uint32_t* unchecked_extrinsic_size,
int skip_ra,
[in] const sgx_target_info_t* quoting_enclave_target_info,
[in] uint32_t* quote_size
);

public sgx_status_t generate_register_quoting_enclave_extrinsic(
[in] const sgx_ql_qve_collateral_t *p_quote_collateral,
[out, size=unchecked_extrinsic_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_size
[out, size=unchecked_extrinsic_max_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_max_size,
[out] uint32_t* unchecked_extrinsic_size
);

public sgx_status_t generate_register_tcb_info_extrinsic(
[in] const sgx_ql_qve_collateral_t *p_quote_collateral,
[out, size=unchecked_extrinsic_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_size
[out, size=unchecked_extrinsic_max_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_max_size,
[out] uint32_t* unchecked_extrinsic_size
);

public sgx_status_t update_market_data_xt(
[in, size=crypto_currency_size] uint8_t* crypto_currency, uint32_t crypto_currency_size,
[in, size=fiat_currency_size] uint8_t* fiat_currency, uint32_t fiat_currency_size,
[out, size=unchecked_extrinsic_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_size
[out, size=unchecked_extrinsic_max_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_max_size,
[out] uint32_t* unchecked_extrinsic_size
);

public sgx_status_t update_weather_data_xt(
[in, size=weather_info_logitude_size] uint8_t* weather_info_logitude, uint32_t weather_info_logitude_size,
[in, size=weather_info_latitude_size] uint8_t* weather_info_latitude, uint32_t weather_info_latitude_size,
[out, size=unchecked_extrinsic_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_size
[out, size=unchecked_extrinsic_max_size] uint8_t* unchecked_extrinsic, uint32_t unchecked_extrinsic_max_size,
[out] uint32_t* unchecked_extrinsic_size
);

public sgx_status_t dump_ias_ra_cert_to_disk();
Expand Down
Loading
Loading