From 369c8e8bf1d5ee5055d21c7f59b39bf756f20023 Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Mon, 11 Sep 2023 15:05:35 +0200 Subject: [PATCH] runtime-sdk/modules/evm: Do not zeroize query caller in new contracts --- runtime-sdk/modules/evm/src/backend.rs | 9 ++ runtime-sdk/modules/evm/src/lib.rs | 14 ++- runtime-sdk/modules/evm/src/mock.rs | 129 ++++++++++++++++++++++++- runtime-sdk/modules/evm/src/state.rs | 25 ++++- runtime-sdk/modules/evm/src/test.rs | 88 ++++++++++++++++- runtime-sdk/modules/evm/src/types.rs | 61 ++++++++++++ runtime-sdk/src/testing/mock.rs | 21 +++- tests/e2e/contracts/query/Makefile | 6 ++ tests/e2e/contracts/query/query.abi | 1 + tests/e2e/contracts/query/query.go | 36 +++++++ tests/e2e/contracts/query/query.hex | 1 + tests/e2e/contracts/query/query.sol | 7 ++ 12 files changed, 391 insertions(+), 7 deletions(-) create mode 100644 tests/e2e/contracts/query/Makefile create mode 100644 tests/e2e/contracts/query/query.abi create mode 100644 tests/e2e/contracts/query/query.go create mode 100644 tests/e2e/contracts/query/query.hex create mode 100644 tests/e2e/contracts/query/query.sol diff --git a/runtime-sdk/modules/evm/src/backend.rs b/runtime-sdk/modules/evm/src/backend.rs index 0ae4daac80..030d7b282b 100644 --- a/runtime-sdk/modules/evm/src/backend.rs +++ b/runtime-sdk/modules/evm/src/backend.rs @@ -550,6 +550,15 @@ 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 a0970db24b..3740c24e0a 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::{H160, H256, U256}; +use types::{FeatureMask, H160, H256, U256}; pub mod mock; #[cfg(test)] @@ -596,6 +596,7 @@ impl Module { if !Cfg::CONFIDENTIAL { return Ok((call, callformat::Metadata::Empty)); } + if let Ok(types::SignedCallDataPack { data, leash, @@ -615,13 +616,22 @@ 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: Default::default(), // The sender cannot be spoofed. + caller, data, ..call }, diff --git a/runtime-sdk/modules/evm/src/mock.rs b/runtime-sdk/modules/evm/src/mock.rs index bf54ce0e66..e1dfe9799e 100644 --- a/runtime-sdk/modules/evm/src/mock.rs +++ b/runtime-sdk/modules/evm/src/mock.rs @@ -2,13 +2,20 @@ use uint::hex::FromHex; use oasis_runtime_sdk::{ + callformat, + core::common::crypto::mrae::deoxysii, dispatcher, + error::RuntimeError, + module, testing::mock::{CallOptions, Signer}, - types::address::SignatureAddressSpec, + types::{address::SignatureAddressSpec, transaction}, BatchContext, }; -use crate::types::{self, H160}; +use crate::{ + derive_caller, + types::{self, H160}, +}; /// A mock EVM signer for use during tests. pub struct EvmSigner(Signer); @@ -64,6 +71,104 @@ impl EvmSigner { opts, ) } + + /// Ethereum address for this signer. + pub fn address(&self) -> H160 { + derive_caller::from_sigspec(self.sigspec()).expect("caller should be evm-compatible") + } + + /// Dispatch a query to the given EVM contract method. + pub fn query_evm( + &self, + ctx: &mut C, + address: H160, + name: &str, + param_types: &[ethabi::ParamType], + params: &[ethabi::Token], + ) -> Result, RuntimeError> + where + C: BatchContext, + { + self.query_evm_opts(ctx, address, name, param_types, params, Default::default()) + } + + /// Dispatch a query to the given EVM contract method. + pub fn query_evm_opts( + &self, + ctx: &mut C, + address: H160, + name: &str, + param_types: &[ethabi::ParamType], + params: &[ethabi::Token], + opts: QueryOptions, + ) -> Result, RuntimeError> + where + C: BatchContext, + { + let mut data = [ + ethabi::short_signature(name, param_types).to_vec(), + ethabi::encode(params), + ] + .concat(); + + // Handle optional encryption. + let client_keypair = deoxysii::generate_key_pair(); + if opts.encrypt { + data = cbor::to_vec( + callformat::encode_call( + ctx, + transaction::Call { + format: transaction::CallFormat::EncryptedX25519DeoxysII, + method: "".into(), + body: cbor::Value::from(data), + ..Default::default() + }, + &client_keypair, + ) + .unwrap(), + ); + } + + let mut result: Vec = self.query( + ctx, + "evm.SimulateCall", + types::SimulateCallQuery { + gas_price: 0.into(), + gas_limit: opts.gas_limit, + caller: opts.caller.unwrap_or_else(|| self.address()), + address, + value: 0.into(), + data, + }, + )?; + + // Handle optional decryption. + if opts.encrypt { + let call_result: transaction::CallResult = + cbor::from_slice(&result).expect("result from EVM should be properly encoded"); + let call_result = callformat::decode_result( + ctx, + transaction::CallFormat::EncryptedX25519DeoxysII, + call_result, + &client_keypair, + ) + .expect("callformat decoding should succeed"); + + result = match call_result { + module::CallResult::Ok(v) => { + cbor::from_value(v).expect("result from EVM should be correct") + } + module::CallResult::Failed { + module, + code, + message, + } => return Err(RuntimeError::new(&module, code, &message)), + module::CallResult::Aborted(e) => panic!("aborted with error: {e}"), + }; + } + + Ok(result) + } } impl std::ops::Deref for EvmSigner { @@ -80,6 +185,26 @@ impl std::ops::DerefMut for EvmSigner { } } +/// Options for making queries. +pub struct QueryOptions { + /// Whether the call should be encrypted. + pub encrypt: bool, + /// Gas limit. + pub gas_limit: u64, + /// Use specified caller instead of signer. + pub caller: Option, +} + +impl Default for QueryOptions { + fn default() -> Self { + Self { + encrypt: false, + gas_limit: 10_000_000, + caller: None, + } + } +} + /// Load contract bytecode from a hex-encoded string. pub fn load_contract_bytecode(raw: &str) -> Vec { Vec::from_hex(raw.split_whitespace().collect::()) diff --git a/runtime-sdk/modules/evm/src/state.rs b/runtime-sdk/modules/evm/src/state.rs index 41f3a3e0ba..4950aca2d4 100644 --- a/runtime-sdk/modules/evm/src/state.rs +++ b/runtime-sdk/modules/evm/src/state.rs @@ -3,7 +3,10 @@ use oasis_runtime_sdk::{ storage::{ConfidentialStore, CurrentStore, HashedStore, PrefixStore, Store, TypedStore}, }; -use crate::{types::H160, Config}; +use crate::{ + types::{ContractMetadata, H160}, + Config, +}; /// Prefix for Ethereum account code in our storage (maps H160 -> Vec). pub const CODES: &[u8] = &[0x01]; @@ -14,6 +17,8 @@ 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"; @@ -118,3 +123,21 @@ 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 937fab7080..015d7e029b 100644 --- a/runtime-sdk/modules/evm/src/test.rs +++ b/runtime-sdk/modules/evm/src/test.rs @@ -27,7 +27,8 @@ use oasis_runtime_sdk::{ use crate::{ derive_caller, - mock::{decode_reverted, decode_reverted_raw, load_contract_bytecode, EvmSigner}, + mock::{decode_reverted, decode_reverted_raw, load_contract_bytecode, EvmSigner, QueryOptions}, + state, types::{self, H160}, Config, Genesis, Module as EVMModule, }; @@ -792,6 +793,91 @@ fn test_c10l_evm_runtime() { do_test_evm_runtime::(); } +#[test] +fn test_c10l_queries() { + let mut mock = mock::Mock::default(); + let mut ctx = mock.create_ctx_for_runtime::>( + context::Mode::ExecuteTx, + true, + ); + let mut signer = EvmSigner::new(0, keys::dave::sigspec()); + + EVMRuntime::::migrate(&mut ctx); + + static QUERY_CONTRACT_CODE_HEX: &str = + include_str!("../../../../tests/e2e/contracts/query/query.hex"); + + // Create contract. + let dispatch_result = signer.call( + &mut ctx, + "evm.Create", + types::Create { + value: 0.into(), + init_code: load_contract_bytecode(QUERY_CONTRACT_CODE_HEX), + }, + ); + let result = dispatch_result.result.unwrap(); + let result: Vec = cbor::from_value(result).unwrap(); + let contract_address = H160::from_slice(&result); + + let mut ctx = mock + .create_ctx_for_runtime::>(context::Mode::CheckTx, true); + + // Call the `test` method 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, + signer.address().into(), + "msg.signer should be correct (non-zeroized)" + ); + + // Test call with confidential envelope. + let result = signer + .query_evm_opts( + &mut ctx, + contract_address, + "test", + &[], + &[], + QueryOptions { + encrypt: true, + ..Default::default() + }, + ) + .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, + 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"); +} + #[test] fn test_fee_refunds() { let mut mock = mock::Mock::default(); diff --git a/runtime-sdk/modules/evm/src/types.rs b/runtime-sdk/modules/evm/src/types.rs index 6898f66bab..0b80fa82cd 100644 --- a/runtime-sdk/modules/evm/src/types.rs +++ b/runtime-sdk/modules/evm/src/types.rs @@ -102,6 +102,46 @@ 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. @@ -219,3 +259,24 @@ 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)); + } +} diff --git a/runtime-sdk/src/testing/mock.rs b/runtime-sdk/src/testing/mock.rs index 7d39e7f164..072e7f3755 100644 --- a/runtime-sdk/src/testing/mock.rs +++ b/runtime-sdk/src/testing/mock.rs @@ -14,7 +14,9 @@ use oasis_core_runtime::{ use crate::{ context::{BatchContext, Mode, RuntimeBatchContext}, crypto::random::RootRng, - dispatcher, history, + dispatcher, + error::RuntimeError, + history, keymanager::KeyManager, module::MigrationHandler, modules, @@ -212,6 +214,11 @@ impl Signer { Self { nonce, sigspec } } + /// Address specification for this signer. + pub fn sigspec(&self) -> &SignatureAddressSpec { + &self.sigspec + } + /// Dispatch a call to the given method. pub fn call(&mut self, ctx: &mut C, method: &str, body: B) -> dispatcher::DispatchResult where @@ -259,4 +266,16 @@ impl Signer { result } + + /// Dispatch a query to the given method. + pub fn query(&self, ctx: &mut C, method: &str, args: A) -> Result + where + C: BatchContext, + A: cbor::Encode, + R: cbor::Decode, + { + let result = + dispatcher::Dispatcher::::dispatch_query(ctx, method, cbor::to_vec(args))?; + Ok(cbor::from_slice(&result).expect("result should decode correctly")) + } } diff --git a/tests/e2e/contracts/query/Makefile b/tests/e2e/contracts/query/Makefile new file mode 100644 index 0000000000..b97339be1a --- /dev/null +++ b/tests/e2e/contracts/query/Makefile @@ -0,0 +1,6 @@ +contract = query.sol +abi = query.abi +hex = query.hex + +include ../contracts.mk + diff --git a/tests/e2e/contracts/query/query.abi b/tests/e2e/contracts/query/query.abi new file mode 100644 index 0000000000..6d21806f0b --- /dev/null +++ b/tests/e2e/contracts/query/query.abi @@ -0,0 +1 @@ +[{"inputs":[],"name":"test","outputs":[{"internalType":"address","name":"","type":"address"}],"stateMutability":"view","type":"function"}] \ No newline at end of file diff --git a/tests/e2e/contracts/query/query.go b/tests/e2e/contracts/query/query.go new file mode 100644 index 0000000000..a787ef271c --- /dev/null +++ b/tests/e2e/contracts/query/query.go @@ -0,0 +1,36 @@ +package query + +import ( + _ "embed" + "encoding/hex" + "fmt" + "strings" + + ethABI "github.com/ethereum/go-ethereum/accounts/abi" +) + +// CompiledHex is the compiled subcall contract in hex encoding. +// +//go:embed query.hex +var CompiledHex string + +// Compiled is the compiled subcall contract. +var Compiled = func() []byte { + contract, err := hex.DecodeString(strings.TrimSpace(CompiledHex)) + if err != nil { + panic(fmt.Errorf("failed to decode contract: %w", err)) + } + return contract +}() + +//go:embed query.abi +var abiJSON string + +// ABI is the ABI of the subcall contract. +var ABI = func() ethABI.ABI { + abi, err := ethABI.JSON(strings.NewReader(abiJSON)) + if err != nil { + panic(err) + } + return abi +}() diff --git a/tests/e2e/contracts/query/query.hex b/tests/e2e/contracts/query/query.hex new file mode 100644 index 0000000000..c9221a35b9 --- /dev/null +++ b/tests/e2e/contracts/query/query.hex @@ -0,0 +1 @@ +6080604052348015600e575f80fd5b50607380601a5f395ff3fe6080604052348015600e575f80fd5b50600436106026575f3560e01c8063f8a8fd6d14602a575b5f80fd5b6040805133815290519081900360200190f3fea264697066735822122069b1e1d9de371cfb51fb1b110da57496602eb6086d8ec235c6da9ff09739c12764736f6c63430008150033 \ No newline at end of file diff --git a/tests/e2e/contracts/query/query.sol b/tests/e2e/contracts/query/query.sol new file mode 100644 index 0000000000..726652da1c --- /dev/null +++ b/tests/e2e/contracts/query/query.sol @@ -0,0 +1,7 @@ +pragma solidity ^0.8.0; + +contract Test { + function test() public view returns (address) { + return msg.sender; + } +}