Fix light client verification for ibc-hooks & admin txs

.github/workflows/ci.yaml

@@ -284,7 +284,7 @@ jobs:
           build-args: |
             SECRET_NODE_TYPE=BOOTSTRAP
             CHAIN_ID=secretdev-1
-            FEATURES=debug-print,random,light-client-validation,test
+            FEATURES=debug-print,random,light-client-validation
             SGX_MODE=SW
           target: build-localsecret
           cache-from: type=gha

.vscode/launch.json

@@ -13,7 +13,9 @@
       "cwd": "${workspaceFolder}/x/compute/internal/keeper",
       "env": {
         "SGX_MODE": "SW",
-        "RUST_BACKTRACE": "1"
+        "RUST_BACKTRACE": "1",
+        "LOG_LEVEL": "TRACE",
+        "SKIP_LIGHT_CLIENT_VALIDATION": "true"
       },
       "args": [
         "test",

Makefile

@@ -165,7 +165,7 @@ build-secret: build-linux
 
 build-linux: _build-linux build_local_no_rust build_cli
 _build-linux:
-	BUILD_PROFILE=$(BUILD_PROFILE) FEATURES="$(FEATURES),light-client-validation,test" FEATURES_U=$(FEATURES_U) $(MAKE) -C go-cosmwasm build-rust
+	BUILD_PROFILE=$(BUILD_PROFILE) FEATURES="$(FEATURES)" FEATURES_U="$(FEATURES_U) light-client-validation go-tests" $(MAKE) -C go-cosmwasm build-rust
 
 build-tm-secret-enclave:
 	git clone https://github.com/scrtlabs/tm-secret-enclave.git /tmp/tm-secret-enclave || true
@@ -244,7 +244,7 @@ clean:
 
 localsecret:
 	DOCKER_BUILDKIT=1 docker build \
-			--build-arg FEATURES="${FEATURES},debug-print,random,light-client-validation,test" \
+			--build-arg FEATURES="${FEATURES},debug-print,random,light-client-validation" \
 			--build-arg FEATURES_U=${FEATURES_U} \
 			--secret id=API_KEY,src=.env.local \
 			--secret id=SPID,src=.env.local \

cosmwasm/enclaves/execute/Cargo.toml

@@ -24,47 +24,53 @@ production = [
   "block-verifier/verify-validator-whitelist"
 ]
 debug-print = ["enclave_contract_engine/debug-print"]
-test = ["enclave_contract_engine/test", "enclave_crypto/test", "enclave_cosmos_types/test", "block-verifier/test"]
+test = [
+  "enclave_contract_engine/test",
+  "enclave_crypto/test",
+  "enclave_cosmos_types/test",
+  "block-verifier/test"
+]
 use_seed_service_on_bootstrap = []
 epid_whitelist_disabled = []
-light-client-validation = ["enclave_contract_engine/light-client-validation", "block-verifier"]
+light-client-validation = [
+  "enclave_contract_engine/light-client-validation",
+  "block-verifier"
+]
 random = ["enclave_contract_engine/random", "enclave_crypto/random"]
-verify-validator-whitelist = ["block-verifier/verify-validator-whitelist", "light-client-validation"]
+verify-validator-whitelist = [
+  "block-verifier/verify-validator-whitelist",
+  "light-client-validation"
+]
+go-tests = []
 
 # This annotation is here to trick the IDE into showing us type information about this crate.
 # We always compile to the "sgx" target, so this will always be false.
 # when compiling to the "sgx" target, we pull this from the target root with an "extern crate" directive
 [target.'cfg(not(target_env = "sgx"))'.dependencies]
 sgx_tstd = { rev = "d2d339cbb005f676bb700059bd51dc689c025f6b", git = "https://github.com/apache/teaclave-sgx-sdk.git", features = [
-    "backtrace",
-    "untrusted_time"
+  "backtrace",
+  "untrusted_time"
 ] }
 sgx_types = { rev = "d2d339cbb005f676bb700059bd51dc689c025f6b", git = "https://github.com/apache/teaclave-sgx-sdk.git" }
 
 [dependencies]
 sgx_tse = { rev = "d2d339cbb005f676bb700059bd51dc689c025f6b", git = "https://github.com/apache/teaclave-sgx-sdk.git" }
 sgx_rand = { rev = "d2d339cbb005f676bb700059bd51dc689c025f6b", git = "https://github.com/apache/teaclave-sgx-sdk.git" }
 sgx_tcrypto = { rev = "d2d339cbb005f676bb700059bd51dc689c025f6b", git = "https://github.com/apache/teaclave-sgx-sdk.git" }
-
 enclave-ffi-types = { path = "../ffi-types" }
-
 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 }
-
 serde = { git = "https://github.com/mesalock-linux/serde-sgx", features = [
   "derive"
 ] }
 serde_json = { git = "https://github.com/mesalock-linux/serde-json-sgx" }
 ctor = "0.1.13"
 derive_more = "0.99"
-
 pwasm-utils = { version = "0.12.0", default-features = false }
 parity-wasm = { version = "0.41.0", default-features = false }
-
 base64 = { rev = "dc7389e10817b078f289386b3b6a852ab6c4c021", git = "https://github.com/mesalock-linux/rust-base64-sgx" }
-
 # for attestation
 chrono = { git = "https://github.com/mesalock-linux/chrono-sgx" }
 num-bigint = { git = "https://github.com/mesalock-linux/num-bigint-sgx" }
@@ -75,10 +81,10 @@ bit-vec = { version = "0.6", default-features = false }
 lazy_static = "1.4"
 hex = "0.4.2"
 log = "0.4.17"
-simple_logger = { version = "2.3.0", default-features = false, features = ["stderr"] }
-
+simple_logger = { version = "2.3.0", default-features = false, features = [
+  "stderr"
+] }
 block-verifier = { path = "../shared/block-verifier", optional = true }
-
 time = "=0.3.17"
 
 [dependencies.webpki]
@@ -100,7 +106,6 @@ git = "https://github.com/mesalock-linux/rustls"
 rev = "95b5e79dc24b02f3ce424437eb9698509d0baf58"
 default-features = false
 features = ["dangerous_configuration", "mesalock_sgx"]
-
 #[patch.'https://github.com/apache/teaclave-sgx-sdk.git']
 #sgx_align_struct_attribute = { path = "../../../third_party/incubator-teaclave-sgx-sdk/sgx_align_struct_attribute" }
 #sgx_alloc = { path = "../../../third_party/incubator-teaclave-sgx-sdk/sgx_alloc" }

cosmwasm/enclaves/execute/src/registration/onchain.rs

@@ -18,7 +18,7 @@ use super::cert::verify_ra_cert;
 use super::seed_exchange::encrypt_seed;
 
 #[cfg(feature = "light-client-validation")]
-use enclave_contract_engine::check_msg_in_current_block;
+use enclave_contract_engine::check_cert_in_current_block;
 
 ///
 /// `ecall_authenticate_new_node`
@@ -51,18 +51,17 @@ pub unsafe extern "C" fn ecall_authenticate_new_node(
 
     let cert_slice = std::slice::from_raw_parts(cert, cert_len as usize);
 
-    #[cfg(all(feature = "light-client-validation", not(feature = "test")))]
-    if !check_msg_in_current_block(cert_slice) {
+    #[cfg(all(feature = "light-client-validation", not(feature = "go-tests")))]
+    if !check_cert_in_current_block(cert_slice) {
         return NodeAuthResult::SignatureInvalid;
     }
-    #[cfg(all(feature = "light-client-validation", feature = "test"))]
+    #[cfg(all(feature = "light-client-validation", feature = "go-tests"))]
     {
-        // allow skipping light client validation in tests
+        // allow skipping light client validation in go-tests
         // if the env variable SKIP_LIGHT_CLIENT_VALIDATION is set
-        let is_skip_light_client_validation =
-            std::env::var("SKIP_LIGHT_CLIENT_VALIDATION").unwrap_or_default();
-        if is_skip_light_client_validation == "" {
-            if !check_msg_in_current_block(cert_slice) {
+        let is_skip_light_client_validation = std::env::var("SKIP_LIGHT_CLIENT_VALIDATION");
+        if is_skip_light_client_validation.is_err() {
+            if !check_cert_in_current_block(cert_slice) {
                 return NodeAuthResult::SignatureInvalid;
             }
         }

cosmwasm/enclaves/shared/contract-engine/Cargo.toml

@@ -8,6 +8,7 @@ edition = "2018"
 default = ["wasm3"]
 debug-print = []
 test = []
+go-tests = []
 wasm3 = []
 wasmi-engine = ["wasmi", "parity-wasm", "pwasm-utils"]
 light-client-validation = ["block-verifier"]

cosmwasm/enclaves/shared/contract-engine/src/contract_operations.rs

@@ -97,17 +97,16 @@ pub fn init(
     //let start = Instant::now();
     let base_env: BaseEnv = extract_base_env(env)?;
 
-    #[cfg(all(feature = "light-client-validation", not(feature = "test")))]
+    #[cfg(all(feature = "light-client-validation", not(feature = "go-tests")))]
     {
         verify_block_info(&base_env)?;
     }
-    #[cfg(all(feature = "light-client-validation", feature = "test"))]
+    #[cfg(all(feature = "light-client-validation", feature = "go-tests"))]
     {
-        // allow skipping light client validation in tests
+        // allow skipping light client validation in go-tests
         // if the env variable SKIP_LIGHT_CLIENT_VALIDATION is set
-        let is_skip_light_client_validation =
-            std::env::var("SKIP_LIGHT_CLIENT_VALIDATION").unwrap_or_default();
-        if is_skip_light_client_validation == "" {
+        let is_skip_light_client_validation = std::env::var("SKIP_LIGHT_CLIENT_VALIDATION");
+        if is_skip_light_client_validation.is_err() {
             verify_block_info(&base_env)?;
         }
     }
@@ -341,17 +340,16 @@ pub fn migrate(
     //let start = Instant::now();
     let base_env: BaseEnv = extract_base_env(env)?;
 
-    #[cfg(all(feature = "light-client-validation", not(feature = "test")))]
+    #[cfg(all(feature = "light-client-validation", not(feature = "go-tests")))]
     {
         verify_block_info(&base_env)?;
     }
-    #[cfg(all(feature = "light-client-validation", feature = "test"))]
+    #[cfg(all(feature = "light-client-validation", feature = "go-tests"))]
     {
-        // allow skipping light client validation in tests
+        // allow skipping light client validation in go-tests
         // if the env variable SKIP_LIGHT_CLIENT_VALIDATION is set
-        let is_skip_light_client_validation =
-            std::env::var("SKIP_LIGHT_CLIENT_VALIDATION").unwrap_or_default();
-        if is_skip_light_client_validation == "" {
+        let is_skip_light_client_validation = std::env::var("SKIP_LIGHT_CLIENT_VALIDATION");
+        if is_skip_light_client_validation.is_err() {
             verify_block_info(&base_env)?;
         }
     }
@@ -513,17 +511,16 @@ pub fn update_admin(
 
     let base_env: BaseEnv = extract_base_env(env)?;
 
-    #[cfg(all(feature = "light-client-validation", not(feature = "test")))]
+    #[cfg(all(feature = "light-client-validation", not(feature = "go-tests")))]
     {
         verify_block_info(&base_env)?;
     }
-    #[cfg(all(feature = "light-client-validation", feature = "test"))]
+    #[cfg(all(feature = "light-client-validation", feature = "go-tests"))]
     {
-        // allow skipping light client validation in tests
+        // allow skipping light client validation in go-tests
         // if the env variable SKIP_LIGHT_CLIENT_VALIDATION is set
-        let is_skip_light_client_validation =
-            std::env::var("SKIP_LIGHT_CLIENT_VALIDATION").unwrap_or_default();
-        if is_skip_light_client_validation == "" {
+        let is_skip_light_client_validation = std::env::var("SKIP_LIGHT_CLIENT_VALIDATION");
+        if is_skip_light_client_validation.is_err() {
             verify_block_info(&base_env)?;
         }
     }
@@ -606,17 +603,16 @@ pub fn handle(
 
     let base_env: BaseEnv = extract_base_env(env)?;
 
-    #[cfg(all(feature = "light-client-validation", not(feature = "test")))]
+    #[cfg(all(feature = "light-client-validation", not(feature = "go-tests")))]
     {
         verify_block_info(&base_env)?;
     }
-    #[cfg(all(feature = "light-client-validation", feature = "test"))]
+    #[cfg(all(feature = "light-client-validation", feature = "go-tests"))]
     {
-        // allow skipping light client validation in tests
+        // allow skipping light client validation in go-tests
         // if the env variable SKIP_LIGHT_CLIENT_VALIDATION is set
-        let is_skip_light_client_validation =
-            std::env::var("SKIP_LIGHT_CLIENT_VALIDATION").unwrap_or_default();
-        if is_skip_light_client_validation == "" {
+        let is_skip_light_client_validation = std::env::var("SKIP_LIGHT_CLIENT_VALIDATION");
+        if is_skip_light_client_validation.is_err() {
             verify_block_info(&base_env)?;
         }
     }

cosmwasm/enclaves/shared/contract-engine/src/contract_validation.rs

@@ -68,7 +68,8 @@ pub fn verify_block_info(base_env: &BaseEnv) -> Result<(), EnclaveError> {
 
 #[cfg(feature = "light-client-validation")]
 /// WARNING: this function must be called at most once per message!
-pub fn check_msg_in_current_block(msg: &[u8]) -> bool {
+/// Checks if there's a msg in the light client that's contained in tx_sign_bytes
+pub fn check_tx_in_current_block(tx_sign_bytes: &[u8]) -> bool {
     let mut verified_msgs = VERIFIED_BLOCK_MESSAGES.lock().unwrap();
     let remaining_msgs = verified_msgs.remaining();
 
@@ -81,8 +82,10 @@ pub fn check_msg_in_current_block(msg: &[u8]) -> bool {
     // all the messages available before we can determine that there has been a failure
     // this isn't an attack vector since this can happen anyway by manipulating the state between executions
     while verified_msgs.remaining() > 0 {
-        if let Some(expected_msg) = verified_msgs.get_next() {
-            if is_subslice(&expected_msg, msg) {
+        if let Some(verified_msg) = verified_msgs.get_next() {
+            trace!("input tx_sign_bytes: {:?}", hex::encode(&tx_sign_bytes));
+            trace!("light client msg: {:?}", hex::encode(&verified_msg));
+            if is_subslice(tx_sign_bytes, &verified_msg) {
                 return true;
             }
         }
@@ -96,6 +99,37 @@ pub fn check_msg_in_current_block(msg: &[u8]) -> bool {
     false
 }
 
+#[cfg(feature = "light-client-validation")]
+/// WARNING: this function must be called at most once per message!
+/// Checks if there's a msg in the light client that's containing cert
+pub fn check_cert_in_current_block(cert: &[u8]) -> bool {
+    let mut verified_msgs = VERIFIED_BLOCK_MESSAGES.lock().unwrap();
+    let remaining_msgs = verified_msgs.remaining();
+
+    if remaining_msgs == 0 {
+        error!("Failed to validate message, error 0x4555");
+        return false;
+    }
+
+    // Msgs might fail in the sdk before they reach the enclave. In this case we need to run through
+    // all the messages available before we can determine that there has been a failure
+    // this isn't an attack vector since this can happen anyway by manipulating the state between executions
+    while verified_msgs.remaining() > 0 {
+        if let Some(verified_msg) = verified_msgs.get_next() {
+            if is_subslice(&verified_msg, cert) {
+                return true;
+            }
+        }
+    }
+
+    error!("Failed to validate message, error 0x4255");
+
+    // if this message fails to verify we have to fail the rest of the block, so we won't get any other messages
+    verified_msgs.clear();
+
+    false
+}
+
 /// contract_key is a unique key for each contract
 /// it's used in state encryption to prevent the same
 /// encryption keys from being used for different contracts
@@ -525,6 +559,7 @@ fn verify_input(
     let sdk_messages = get_sdk_messages(sig_info, verify_params_types)?;
 
     let is_verified = verify_input_params(
+        sig_info,
         &sdk_messages,
         sender,
         sent_funds,
@@ -730,6 +765,7 @@ fn verify_callback_sig_impl(
 
 #[allow(clippy::too_many_arguments)]
 fn verify_input_params(
+    sig_info: &SigInfo,
     sdk_messages: &[DirectSdkMsg],
     sender: &CanonicalAddr,
     sent_funds: &[Coin],
@@ -766,22 +802,21 @@ fn verify_input_params(
         }
     };
 
-    #[cfg(all(feature = "light-client-validation", not(feature = "test")))]
+    #[cfg(all(feature = "light-client-validation", not(feature = "go-tests")))]
     {
         info!("Verifying message in signed block...");
-        if !check_msg_in_current_block(&sent_wasm_input.to_vec()) {
+        if !check_tx_in_current_block(sig_info.sign_bytes.as_slice()) {
             return Err(EnclaveError::ValidationFailure);
         }
     }
-    #[cfg(all(feature = "light-client-validation", feature = "test"))]
+    #[cfg(all(feature = "light-client-validation", feature = "go-tests"))]
     {
-        // allow skipping light client validation in tests
+        // allow skipping light client validation in go-tests
         // if the env variable SKIP_LIGHT_CLIENT_VALIDATION is set
-        let is_skip_light_client_validation =
-            std::env::var("SKIP_LIGHT_CLIENT_VALIDATION").unwrap_or_default();
-        if is_skip_light_client_validation == "" {
+        let is_skip_light_client_validation = std::env::var("SKIP_LIGHT_CLIENT_VALIDATION");
+        if is_skip_light_client_validation.is_err() {
             info!("Verifying message in signed block...");
-            if !check_msg_in_current_block(&sent_wasm_input.to_vec()) {
+            if !check_tx_in_current_block(sig_info.sign_bytes.as_slice()) {
                 return Err(EnclaveError::ValidationFailure);
             }
         }

cosmwasm/enclaves/shared/contract-engine/src/lib.rs

@@ -30,7 +30,7 @@ mod wasm3;
 
 pub use contract_operations::{handle, init, query};
 #[cfg(feature = "light-client-validation")]
-pub use contract_validation::check_msg_in_current_block;
+pub use contract_validation::{check_cert_in_current_block, check_tx_in_current_block};
 
 #[cfg(feature = "test")]
 pub mod tests {

go-cosmwasm/Cargo.toml

@@ -33,7 +33,7 @@ production = ["cosmwasm-sgx-vm/production"]
 debug-print = ["cosmwasm-sgx-vm/debug-print"]
 # features that do nothing here but are just here for compatability with enclave
 light-client-validation = []
-test = []
+go-tests = []
 random = []
 verify-validator-whitelist = []