From ae0dd9ce92a7711e1296e9acd3dbe082cfca3401 Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Mon, 18 Sep 2023 20:19:40 +0200 Subject: [PATCH] runtime-sdk/modules/evm: Revert "drop signed queries in new contracts" Given further thought this seems too much like a foot gun. --- runtime-sdk/modules/evm/src/backend.rs | 9 ---- runtime-sdk/modules/evm/src/lib.rs | 13 +----- runtime-sdk/modules/evm/src/state.rs | 25 +---------- runtime-sdk/modules/evm/src/test.rs | 24 +--------- runtime-sdk/modules/evm/src/types.rs | 61 -------------------------- 5 files changed, 4 insertions(+), 128 deletions(-) diff --git a/runtime-sdk/modules/evm/src/backend.rs b/runtime-sdk/modules/evm/src/backend.rs index 030d7b282b..0ae4daac80 100644 --- a/runtime-sdk/modules/evm/src/backend.rs +++ b/runtime-sdk/modules/evm/src/backend.rs @@ -550,15 +550,6 @@ impl<'ctx, 'backend, 'config, C: TxContext, Cfg: Config> StackState<'config> let mut store = state::codes(store); store.insert(address, code); }); - - // Set metadata for newly created contracts. - state::set_metadata( - &address.into(), - types::ContractMetadata { - // For all new contracts we don't zeroize the caller in queries. - features: types::FeatureMask::QUERIES_NO_CALLER_ZEROIZE, - }, - ); } fn transfer(&mut self, transfer: Transfer) -> Result<(), ExitError> { diff --git a/runtime-sdk/modules/evm/src/lib.rs b/runtime-sdk/modules/evm/src/lib.rs index 921651efe0..4ad2cb1573 100644 --- a/runtime-sdk/modules/evm/src/lib.rs +++ b/runtime-sdk/modules/evm/src/lib.rs @@ -39,7 +39,7 @@ use oasis_runtime_sdk::{ }, }; -use types::{FeatureMask, H160, H256, U256}; +use types::{H160, H256, U256}; #[cfg(any(test, feature = "test"))] pub mod mock; @@ -617,22 +617,13 @@ impl Module { )); } - // Determine the caller based on per-contract features. - let caller = if state::get_metadata(&call.address) - .has_features(FeatureMask::QUERIES_NO_CALLER_ZEROIZE) - { - call.caller - } else { - Default::default() // Zeroize caller. - }; - // The call is not signed, but it must be encoded as an oasis-sdk call. let tx_call_format = transaction::CallFormat::Plain; // Queries cannot be encrypted. let (data, tx_metadata) = Self::decode_call_data(ctx, call.data, tx_call_format, 0, true)? .expect("processing always proceeds"); Ok(( types::SimulateCallQuery { - caller, + caller: Default::default(), // The sender cannot be spoofed. data, ..call }, diff --git a/runtime-sdk/modules/evm/src/state.rs b/runtime-sdk/modules/evm/src/state.rs index 4950aca2d4..41f3a3e0ba 100644 --- a/runtime-sdk/modules/evm/src/state.rs +++ b/runtime-sdk/modules/evm/src/state.rs @@ -3,10 +3,7 @@ use oasis_runtime_sdk::{ storage::{ConfidentialStore, CurrentStore, HashedStore, PrefixStore, Store, TypedStore}, }; -use crate::{ - types::{ContractMetadata, H160}, - Config, -}; +use crate::{types::H160, Config}; /// Prefix for Ethereum account code in our storage (maps H160 -> Vec). pub const CODES: &[u8] = &[0x01]; @@ -17,8 +14,6 @@ pub const STORAGES: &[u8] = &[0x02]; pub const BLOCK_HASHES: &[u8] = &[0x03]; /// Prefix for Ethereum account storage in our confidential storage (maps H160||H256 -> H256). pub const CONFIDENTIAL_STORAGES: &[u8] = &[0x04]; -/// Prefix for contract metadata (maps H160 -> ContractMetadata). -pub const METADATA: &[u8] = &[0x05]; /// Confidential store key pair ID domain separation context base. pub const CONFIDENTIAL_STORE_KEY_PAIR_ID_CONTEXT_BASE: &[u8] = b"oasis-runtime-sdk/evm: state"; @@ -123,21 +118,3 @@ pub fn block_hashes<'a, S: Store + 'a>(state: S) -> TypedStore let store = PrefixStore::new(state, &crate::MODULE_NAME); TypedStore::new(PrefixStore::new(store, &BLOCK_HASHES)) } - -/// Set contract metadata. -pub fn set_metadata(address: &H160, metadata: ContractMetadata) { - CurrentStore::with(|store| { - let store = PrefixStore::new(store, &crate::MODULE_NAME); - let mut store = TypedStore::new(PrefixStore::new(store, &METADATA)); - store.insert(address, metadata); - }) -} - -/// Get contract metadata. -pub fn get_metadata(address: &H160) -> ContractMetadata { - CurrentStore::with(|store| { - let store = PrefixStore::new(store, &crate::MODULE_NAME); - let store = TypedStore::new(PrefixStore::new(store, &METADATA)); - store.get(address).unwrap_or_default() - }) -} diff --git a/runtime-sdk/modules/evm/src/test.rs b/runtime-sdk/modules/evm/src/test.rs index 015d7e029b..a7dbb7ada3 100644 --- a/runtime-sdk/modules/evm/src/test.rs +++ b/runtime-sdk/modules/evm/src/test.rs @@ -832,11 +832,7 @@ fn test_c10l_queries() { ethabi::decode(&[ParamType::Address], &result).expect("output should be correct"); let test = result.pop().unwrap().into_address().unwrap(); - assert_eq!( - test, - signer.address().into(), - "msg.signer should be correct (non-zeroized)" - ); + assert_eq!(test, Default::default(), "msg.signer should be zeroized"); // Test call with confidential envelope. let result = signer @@ -856,24 +852,6 @@ fn test_c10l_queries() { let mut result = ethabi::decode(&[ParamType::Address], &result).expect("output should be correct"); - let test = result.pop().unwrap().into_address().unwrap(); - assert_eq!( - test, - signer.address().into(), - "msg.signer should be correct (non-zeroized)" - ); - - // Reset the contract metadata to remove the QUERIES_NO_CALLER_ZEROIZE feature. - state::set_metadata(&contract_address, Default::default()); - - // Call the `test` method again on the contract via a query. - let result = signer - .query_evm(&mut ctx, contract_address, "test", &[], &[]) - .expect("query should succeed"); - - let mut result = - ethabi::decode(&[ParamType::Address], &result).expect("output should be correct"); - let test = result.pop().unwrap().into_address().unwrap(); assert_eq!(test, Default::default(), "msg.signer should be zeroized"); } diff --git a/runtime-sdk/modules/evm/src/types.rs b/runtime-sdk/modules/evm/src/types.rs index 0b80fa82cd..6898f66bab 100644 --- a/runtime-sdk/modules/evm/src/types.rs +++ b/runtime-sdk/modules/evm/src/types.rs @@ -102,46 +102,6 @@ pub struct Leash { pub block_range: u64, } -/// Features affecting a contract's behavior. -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash, cbor::Encode, cbor::Decode)] -#[cbor(transparent)] -pub struct FeatureMask(pub u32); - -impl FeatureMask { - /// Caller should never be zeroized in queries. - pub const QUERIES_NO_CALLER_ZEROIZE: FeatureMask = FeatureMask(1 << 0); -} - -impl std::ops::BitOr for FeatureMask { - type Output = Self; - - fn bitor(self, rhs: Self) -> Self::Output { - Self(self.0 | rhs.0) - } -} - -impl std::ops::BitAnd for FeatureMask { - type Output = Self; - - fn bitand(self, rhs: Self) -> Self::Output { - Self(self.0 & rhs.0) - } -} - -/// Contract metadata. -#[derive(Clone, Debug, Default, cbor::Encode, cbor::Decode)] -pub struct ContractMetadata { - /// Features supported by the contract. - pub features: FeatureMask, -} - -impl ContractMetadata { - /// Check whether the contract has the specified features. - pub fn has_features(&self, mask: FeatureMask) -> bool { - self.features & mask == mask - } -} - // The rest of the file contains wrappers for primitive_types::{H160, H256, U256}, // so that we can implement cbor::{Encode, Decode} for them, ugh. // Remove this once oasis-cbor#8 is implemented. @@ -259,24 +219,3 @@ mod eth { impl_upstream_conversions!(H160, H256, U256); } pub use eth::{H160, H256, U256}; - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn test_contract_metadata() { - let meta = ContractMetadata::default(); - assert!(!meta.has_features(FeatureMask::QUERIES_NO_CALLER_ZEROIZE)); - - let meta = ContractMetadata { - features: FeatureMask::QUERIES_NO_CALLER_ZEROIZE, - }; - assert!(meta.has_features(FeatureMask::QUERIES_NO_CALLER_ZEROIZE)); - - let meta = ContractMetadata { - features: FeatureMask(0xff), - }; - assert!(meta.has_features(FeatureMask::QUERIES_NO_CALLER_ZEROIZE)); - } -}