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

Verify new message type2 #1452

Merged
merged 6 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
8 changes: 8 additions & 0 deletions cosmwasm/enclaves/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cosmwasm/enclaves/execute/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ enclave_contract_engine = { path = "../shared/contract-engine" }
enclave_crypto = { path = "../shared/crypto" }
enclave_utils = { path = "../shared/utils" }
enclave_cosmos_types = { path = "../shared/cosmos-types", optional = true }
cosmos_proto = { path = "../shared/cosmos-proto" }
Cashmaney marked this conversation as resolved.
Show resolved Hide resolved

serde = { git = "https://github.com/mesalock-linux/serde-sgx", features = [
"derive"
Expand Down
12 changes: 12 additions & 0 deletions cosmwasm/enclaves/execute/src/registration/onchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ use enclave_utils::{
use super::cert::verify_ra_cert;
use super::seed_exchange::encrypt_seed;

#[cfg(feature = "light-client-validation")]
use block_verifier::VERIFIED_MESSAGES;

#[cfg(feature = "light-client-validation")]
use block_verifier::registration::verify_reg_msg;

///
/// `ecall_authenticate_new_node`
///
Expand Down Expand Up @@ -45,8 +51,14 @@ pub unsafe extern "C" fn ecall_authenticate_new_node(

validate_mut_ptr!(seed.as_mut_ptr(), seed.len(), NodeAuthResult::InvalidInput);
validate_const_ptr!(cert, cert_len as usize, NodeAuthResult::InvalidInput);

let cert_slice = std::slice::from_raw_parts(cert, cert_len as usize);

#[cfg(feature = "light-client-validation")]
if !verify_reg_msg(cert_slice) {
return NodeAuthResult::SignatureInvalid;
}

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, None)?;
Expand Down
4 changes: 3 additions & 1 deletion cosmwasm/enclaves/shared/block-verifier/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ edition = "2018"

[features]
default = ["random"]
test = []
test = ["base64"]
random = []
production = []
verify-validator-whitelist = []
Expand All @@ -32,6 +32,8 @@ cosmos_proto = {path="../cosmos-proto"}
protobuf = { version = "2.25.2" }
hex = { version = "0.4.3" }

base64 = { version = "0.21.0", optional = true }

# cosmrs = { version = "0.11.0", default-features = false }


Expand Down
5 changes: 5 additions & 0 deletions cosmwasm/enclaves/shared/block-verifier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ pub mod wasm_messages;

pub use wasm_messages::VERIFIED_MESSAGES;

pub use verify::registration;

mod txs;

#[cfg(any(feature = "verify-validator-whitelist", feature = "test"))]
Expand Down Expand Up @@ -51,6 +53,9 @@ pub mod tests {
crate::wasm_messages::tests::parse_tx_multiple_msg_non_wasm();
crate::wasm_messages::tests::parse_tx_multisig();
crate::wasm_messages::tests::check_message_is_wasm();
crate::wasm_messages::tests::check_message_is_reg();
crate::wasm_messages::tests::check_parse_reg_bytes();
crate::wasm_messages::tests::check_parse_reg_from_tx();
crate::wasm_messages::tests::test_check_message_not_wasm();
crate::wasm_messages::tests::test_wasm_msg_tracker();
crate::wasm_messages::tests::test_wasm_msg_tracker_multiple_msgs();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub unsafe fn submit_block_signatures_impl(
sgx_status_t::SGX_ERROR_INVALID_PARAMETER
}));

message_verifier.append_wasm_from_tx(parsed_tx);
message_verifier.append_msg_from_tx(parsed_tx);
}

message_verifier.set_block_info(
Expand Down
7 changes: 5 additions & 2 deletions cosmwasm/enclaves/shared/block-verifier/src/verify/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
pub mod txs;
pub mod validator_set;
pub mod block;
pub mod commit;
pub mod header;
pub mod txs;
pub mod validator_set;

#[cfg(feature = "random")]
pub mod random;

// external messages
pub mod registration;
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use crate::VERIFIED_MESSAGES;
use log::{debug, error};
use protobuf::Message;

pub fn verify_reg_msg(certificate: &[u8]) -> bool {
let mut verified_msgs = VERIFIED_MESSAGES.lock().unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Not relevant specifically to this PR but just a thought, should we log a certain error in case this lock().unwrap() fails? Debugging this would be hell

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually think it would be kind of dramatic, as this would mean a failure in the lock or something? Not sure when this is even possible

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's going to give no details if it fails, but it should never fail.

let next = verified_msgs.get_next();

let result = if let Some(msg) = next {
// I assume @assaf will add the message type to the verified messages, so I'm going to leave this here for now
// without a check - it will fail in the parse either way

// if !check_message_is_reg(&msg) {
// error!("Error failed to validate registration message - 0x7535");
// false
// }
Cashmaney marked this conversation as resolved.
Show resolved Hide resolved

match cosmos_proto::registration::v1beta1::msg::RaAuthenticate::parse_from_bytes(&msg) {
Ok(ra_msg) => {
if ra_msg.certificate == certificate {
return true;
}
error!("Error failed to validate registration message - 0x7535");
false
}
Err(e) => {
debug!("Error decoding registation protobuf: {}", e);
error!("Error decoding msg from block validator - 0xA0F2");
Copy link
Member

Choose a reason for hiding this comment

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

What's the deal with this hex 0xA0F2?

Copy link
Member Author

Choose a reason for hiding this comment

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

I kind of like adding random error codes - makes finding the exact error easier in a month or two when someone randomly pastes their error. Also we can one day create a codex of error codes & their meaning/more info, etc.

false
}
}
} else {
error!("Cannot verify new node unless msg is part of the current block");
false
};

if !result {
// if validation failed clear the message queue and prepare for next tx... or apphash
verified_msgs.clear();
}

result
}
60 changes: 56 additions & 4 deletions cosmwasm/enclaves/shared/block-verifier/src/wasm_messages.rs

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions cosmwasm/enclaves/shared/cosmos-proto/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ mod protobuf {
.expect(&format!("could not canonicalize {:?}", path))
}

fn from_reg(path: &str) -> PathBuf {
let mut full_path = PathBuf::from("../../../../proto/secret/registration");
full_path.push(path);
full_path
.canonicalize()
.expect(&format!("could not canonicalize {:?}", path))
}

pub fn build_protobuf_parsers() {
let protoc_err_msg = "protoc failed to generate protobuf parsers";
let mut library_dir = dirs::home_dir().unwrap();
Expand All @@ -61,6 +69,10 @@ mod protobuf {
from_cosmos("crypto/multisig/v1beta1/multisig.proto"),
],
),
(
"src/registration/v1beta1/",
&[from_reg("v1beta1/msg.proto")],
),
(
"src/crypto/secp256k1",
&[from_cosmos("crypto/secp256k1/keys.proto")],
Expand Down
6 changes: 6 additions & 0 deletions cosmwasm/enclaves/shared/cosmos-proto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,9 @@ pub mod cosmwasm {

use super::base::coin;
}

pub mod registration {
pub mod v1beta1 {
pub mod msg;
}
}
Loading