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.

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

Node auth error messages - take 3 #397

Merged
merged 6 commits into from
Jul 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions cosmwasm/packages/enclave-ffi-types/cbindgen.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ style = "both"
include = [
"UserSpaceBuffer",
"EnclaveBuffer",
"NodeAuthResult",
"Ctx",
"InitResult",
"HandleResult",
Expand Down
4 changes: 2 additions & 2 deletions cosmwasm/packages/enclave-ffi-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
mod types;

pub use types::{
Ctx, EnclaveBuffer, EnclaveError, HandleResult, InitResult, OcallReturn, QueryResult,
UntrustedVmError, UserSpaceBuffer,
Ctx, EnclaveBuffer, EnclaveError, HandleResult, InitResult, NodeAuthResult, OcallReturn,
QueryResult, UntrustedVmError, UserSpaceBuffer,
};

pub const ENCRYPTED_SEED_SIZE: usize = 48;
Expand Down
21 changes: 21 additions & 0 deletions cosmwasm/packages/enclave-ffi-types/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,27 @@ pub enum EnclaveError {
Unknown,
}

/// This type represents the possible error conditions that can be encountered in the
/// enclave while authenticating a new node in the network.
/// cbindgen:prefix-with-name
#[repr(C)]
#[derive(Debug, Display, PartialEq, Eq)]
pub enum NodeAuthResult {
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we could add errors for specific isvEnclaveQuoteStatus? Like, return "failed to perform registration. Invalid quote status "KEY_REVOKED" " or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm... can you point me to the place in the code where this error originates? I'd probably have to fiddle with 1-3 functions to bubble this specific case out to the context where i instantiate variants of this type.
Note that the way i use this type is really high level, and unrelated to how most of the related code just uses sgx_status_t

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this will be somewhat involved.
I'm not sure I understand what exactly all the structs and enums mean, and the specific error you're referring to happens a couple layers into functions that obfuscate the specific errors that happen. If I change that, I'd need to add a couple of error types, and modify a bunch of other functions that use the same functions that I needed to change in the first place.

Success,
#[display(fmt = "Enclave received invalid inputs")]
InvalidInput,
#[display(fmt = "The provided certificate was invalid")]
InvalidCert,
#[display(fmt = "Writing to file system from the enclave failed")]
CantWriteToStorage,
#[display(fmt = "The public key in the certificate appears to be malformed")]
MalformedPublicKey,
#[display(fmt = "Encrypting the seed failed")]
SeedEncryptionFailed,
#[display(fmt = "Enclave panicked :( please file a bug report!")]
Panic,
}

/// This type holds a pointer to a VmError that is boxed on the untrusted side
// `VmError` is the standard error type for the `cosmwasm-sgx-vm` layer.
// During an ocall, we call into the original implementation of `db_read`, `db_write`, and `db_remove`.
Expand Down
20 changes: 11 additions & 9 deletions cosmwasm/packages/sgx-vm/src/attestation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use log::*;
use sgx_types::*;
use sgx_types::{sgx_status_t, SgxResult};

use enclave_ffi_types::ENCRYPTED_SEED_SIZE;
use enclave_ffi_types::{NodeAuthResult, ENCRYPTED_SEED_SIZE};

use crate::enclave::get_enclave;

Expand All @@ -18,7 +18,7 @@ extern "C" {
) -> sgx_status_t;
pub fn ecall_authenticate_new_node(
eid: sgx_enclave_id_t,
retval: *mut sgx_status_t,
retval: *mut NodeAuthResult,
cert: *const u8,
cert_len: u32,
seed: &mut [u8; ENCRYPTED_SEED_SIZE],
Expand Down Expand Up @@ -122,7 +122,7 @@ pub extern "C" fn ocall_get_update_info(
unsafe { sgx_report_attestation_status(platform_blob, enclave_trusted, update_info) }
}

pub fn create_attestation_report_u() -> SgxResult<sgx_status_t> {
pub fn create_attestation_report_u() -> SgxResult<()> {
let enclave = get_enclave()?;

let eid = enclave.geteid();
Expand All @@ -137,13 +137,15 @@ pub fn create_attestation_report_u() -> SgxResult<sgx_status_t> {
return Err(retval);
}

Ok(sgx_status_t::SGX_SUCCESS)
Ok(())
}

pub fn untrusted_get_encrypted_seed(cert: &[u8]) -> SgxResult<[u8; ENCRYPTED_SEED_SIZE]> {
pub fn untrusted_get_encrypted_seed(
cert: &[u8],
) -> SgxResult<Result<[u8; ENCRYPTED_SEED_SIZE], NodeAuthResult>> {
let enclave = get_enclave()?;
let eid = enclave.geteid();
let mut retval = sgx_status_t::SGX_SUCCESS;
let mut retval = NodeAuthResult::Success;
let mut seed = [0u8; ENCRYPTED_SEED_SIZE];
let status = unsafe {
ecall_authenticate_new_node(
Expand All @@ -159,16 +161,16 @@ pub fn untrusted_get_encrypted_seed(cert: &[u8]) -> SgxResult<[u8; ENCRYPTED_SEE
return Err(status);
}

if retval != sgx_status_t::SGX_SUCCESS {
return Err(retval);
if retval != NodeAuthResult::Success {
return Ok(Err(retval));
}

if seed.is_empty() {
error!("Got empty seed from encryption");
return Err(sgx_status_t::SGX_ERROR_UNEXPECTED);
}

Ok(seed)
Ok(Ok(seed))
}

#[cfg(test)]
Expand Down
6 changes: 3 additions & 3 deletions cosmwasm/packages/sgx-vm/src/seed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ extern "C" {
) -> sgx_status_t;
}

pub fn untrusted_init_node(master_cert: &[u8], encrypted_seed: &[u8]) -> SgxResult<sgx_status_t> {
debug!("Initializing enclave..");
pub fn untrusted_init_node(master_cert: &[u8], encrypted_seed: &[u8]) -> SgxResult<()> {
info!("Initializing enclave..");
let enclave = get_enclave()?;
debug!("Initialized enclave successfully!");

Expand All @@ -54,7 +54,7 @@ pub fn untrusted_init_node(master_cert: &[u8], encrypted_seed: &[u8]) -> SgxResu
return Err(ret);
}

Ok(sgx_status_t::SGX_SUCCESS)
Ok(())
}

pub fn untrusted_key_gen() -> SgxResult<[u8; 32]> {
Expand Down
2 changes: 1 addition & 1 deletion cosmwasm/packages/wasmi-runtime/Enclave.edl
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ enclave {

public sgx_status_t ecall_get_attestation_report();

public sgx_status_t ecall_authenticate_new_node(
public NodeAuthResult ecall_authenticate_new_node(
[in, count=cert_len] const uint8_t* cert,
uintptr_t cert_len,
[out, count=48] uint8_t* seed
Expand Down
5 changes: 1 addition & 4 deletions cosmwasm/packages/wasmi-runtime/src/registration/cert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,7 @@ pub fn verify_ra_cert(cert_der: &[u8]) -> SgxResult<Vec<u8>> {

// 2. Verify quote status (mandatory field)

match verify_quote_status(report.sgx_quote_status) {
Ok(_) => (),
Err(e) => return Err(e),
}
verify_quote_status(report.sgx_quote_status)?;

// verify certificate
match SIGNING_METHOD {
Expand Down
28 changes: 14 additions & 14 deletions cosmwasm/packages/wasmi-runtime/src/registration/onchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
/// These functions run on-chain and must be deterministic across all nodes
///
use log::*;
use sgx_types::{sgx_status_t, SgxResult};
use std::panic;

use enclave_ffi_types::NodeAuthResult;

use crate::consts::ENCRYPTED_SEED_SIZE;
use crate::crypto::PUBLIC_KEY_SIZE;
use crate::storage::write_to_untrusted;
Expand Down Expand Up @@ -32,24 +33,22 @@ pub unsafe extern "C" fn ecall_authenticate_new_node(
cert: *const u8,
cert_len: u32,
seed: &mut [u8; ENCRYPTED_SEED_SIZE],
) -> sgx_status_t {
) -> NodeAuthResult {
if let Err(_e) = validate_mut_ptr(seed.as_mut_ptr(), seed.len()) {
return sgx_status_t::SGX_ERROR_UNEXPECTED;
return NodeAuthResult::InvalidInput;
}

if let Err(_e) = validate_const_ptr(cert, cert_len as usize) {
return sgx_status_t::SGX_ERROR_UNEXPECTED;
return NodeAuthResult::InvalidInput;
}
let cert_slice = std::slice::from_raw_parts(cert, cert_len as usize);

let result = panic::catch_unwind(|| -> SgxResult<Vec<u8>> {
let result = panic::catch_unwind(|| -> Result<Vec<u8>, NodeAuthResult> {
// verify certificate, and return the public key in the extra data of the report
let pk = verify_ra_cert(cert_slice).map_err(|verification_status| {
error!("Error in validating certificate: {:?}", verification_status);
if let Err(write_status) = write_to_untrusted(cert_slice, "failed_cert.der") {
write_status
} else {
verification_status
match write_to_untrusted(cert_slice, "failed_cert.der") {
Err(_) => NodeAuthResult::CantWriteToStorage,
Ok(_) => NodeAuthResult::InvalidCert,
}
})?;

Expand All @@ -59,7 +58,7 @@ pub unsafe extern "C" fn ecall_authenticate_new_node(
"Got public key from certificate with the wrong size: {:?}",
pk.len()
);
return Err(sgx_status_t::SGX_ERROR_UNEXPECTED);
return Err(NodeAuthResult::MalformedPublicKey);
}

let mut target_public_key: [u8; 32] = [0u8; 32];
Expand All @@ -69,7 +68,8 @@ pub unsafe extern "C" fn ecall_authenticate_new_node(
&target_public_key.to_vec()
);

let res: Vec<u8> = encrypt_seed(target_public_key)?;
let res: Vec<u8> =
encrypt_seed(target_public_key).map_err(|_| NodeAuthResult::SeedEncryptionFailed)?;

Ok(res)
});
Expand All @@ -78,12 +78,12 @@ pub unsafe extern "C" fn ecall_authenticate_new_node(
match res {
Ok(res) => {
seed.copy_from_slice(&res);
sgx_status_t::SGX_SUCCESS
NodeAuthResult::Success
}
Err(e) => e,
}
} else {
error!("Enclave call ecall_authenticate_new_node panic!");
sgx_status_t::SGX_ERROR_UNEXPECTED
NodeAuthResult::Panic
}
}
13 changes: 13 additions & 0 deletions go-cosmwasm/src/error/rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ pub enum Error {
#[cfg(feature = "backtraces")]
backtrace: snafu::Backtrace,
},
#[snafu(display("{}", msg))]
GoCwEnclaveError {
msg: String,
#[cfg(feature = "backtraces")]
backtrace: snafu::Backtrace,
},
}

impl Error {
Expand All @@ -60,6 +66,13 @@ impl Error {
.build()
}

pub fn enclave_err<S: ToString>(msg: S) -> Self {
GoCwEnclaveError {
msg: msg.to_string(),
}
.build()
}

pub fn out_of_gas() -> Self {
OutOfGas {}.build()
}
Expand Down
41 changes: 22 additions & 19 deletions go-cosmwasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ use cosmwasm_sgx_vm::{
};

use ctor::ctor;
use log;
use log::*;

#[ctor]
Expand All @@ -54,30 +53,36 @@ pub extern "C" fn get_encrypted_seed(cert: Buffer, err: Option<&mut Buffer>) ->
debug!("Called get_encrypted_seed");
let cert_slice = match unsafe { cert.read() } {
None => {
set_error(Error::vm_err("Attestation Certificate is empty"), err);
set_error(Error::empty_arg("attestation_cert"), err);
return Buffer::default();
}
Some(r) => r,
};
let result = match untrusted_get_encrypted_seed(cert_slice) {
info!("Hello from right before untrusted_get_encrypted_seed");
match untrusted_get_encrypted_seed(cert_slice) {
Err(e) => {
set_error(Error::vm_err(e.to_string()), err);
return Buffer::default();
// An error happened in the SGX sdk.
set_error(Error::enclave_err(e.to_string()), err);
Buffer::default()
}
Ok(r) => {
Ok(Err(e)) => {
// An error was returned from the enclave.
set_error(Error::enclave_err(e.to_string()), err);
Buffer::default()
}
Ok(Ok(seed)) => {
clear_error();
Buffer::from_vec(r.to_vec())
Buffer::from_vec(seed.to_vec())
}
};
return result;
}
}

#[no_mangle]
pub extern "C" fn init_bootstrap(err: Option<&mut Buffer>) -> Buffer {
info!("Hello from right before init_bootstrap");
match untrusted_init_bootstrap() {
Err(e) => {
set_error(Error::vm_err(e.to_string()), err);
set_error(Error::enclave_err(e.to_string()), err);
Buffer::default()
}
Ok(r) => {
Expand All @@ -95,37 +100,35 @@ pub extern "C" fn init_node(
) -> bool {
let pk_slice = match unsafe { master_cert.read() } {
None => {
set_error(Error::vm_err("Public key is empty"), err);
set_error(Error::empty_arg("master_cert"), err);
return false;
}
Some(r) => r,
};
let encrypted_seed_slice = match unsafe { encrypted_seed.read() } {
None => {
set_error(Error::vm_err("Encrypted seed is empty"), err);
set_error(Error::empty_arg("encrypted_seed"), err);
return false;
}
Some(r) => r,
};

let result = match untrusted_init_node(pk_slice, encrypted_seed_slice) {
match untrusted_init_node(pk_slice, encrypted_seed_slice) {
Ok(_) => {
clear_error();
true
}
Err(e) => {
set_error(Error::vm_err(e.to_string()), err);
set_error(Error::enclave_err(e.to_string()), err);
false
}
};

result
}
}

#[no_mangle]
pub extern "C" fn create_attestation_report(err: Option<&mut Buffer>) -> bool {
if let Err(status) = create_attestation_report_u() {
set_error(Error::vm_err(status.to_string()), err);
set_error(Error::enclave_err(status.to_string()), err);
return false;
}
clear_error();
Expand Down Expand Up @@ -465,7 +468,7 @@ fn do_query(
pub extern "C" fn key_gen(err: Option<&mut Buffer>) -> Buffer {
match untrusted_key_gen() {
Err(e) => {
set_error(Error::vm_err(e.to_string()), err);
set_error(Error::enclave_err(e.to_string()), err);
Buffer::default()
}
Ok(r) => {
Expand Down