Skip to content

Commit

Permalink
runtime-sdk/modules/evm: Do not zeroize query caller in new contracts
Browse files Browse the repository at this point in the history
  • Loading branch information
kostko committed Sep 14, 2023
1 parent 7cfff62 commit 92ce19b
Show file tree
Hide file tree
Showing 12 changed files with 280 additions and 5 deletions.
9 changes: 9 additions & 0 deletions runtime-sdk/modules/evm/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Check warning on line 559 in runtime-sdk/modules/evm/src/backend.rs

View check run for this annotation

Codecov / codecov/patch

runtime-sdk/modules/evm/src/backend.rs#L559

Added line #L559 was not covered by tests
},
);
}

fn transfer(&mut self, transfer: Transfer) -> Result<(), ExitError> {
Expand Down
14 changes: 12 additions & 2 deletions runtime-sdk/modules/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use oasis_runtime_sdk::{
},
};

use types::{H160, H256, U256};
use types::{FeatureMask, H160, H256, U256};

pub mod mock;
#[cfg(test)]
Expand Down Expand Up @@ -596,6 +596,7 @@ impl<Cfg: Config> Module<Cfg> {
if !Cfg::CONFIDENTIAL {
return Ok((call, callformat::Metadata::Empty));
}

if let Ok(types::SignedCallDataPack {
data,
leash,
Expand All @@ -615,13 +616,22 @@ impl<Cfg: Config> Module<Cfg> {
));
}

// Determine the caller based on per-contract features.
let caller = if state::get_metadata(&call.address)
.has_features(FeatureMask::QUERIES_NO_CALLER_ZEROIZE)

Check warning on line 621 in runtime-sdk/modules/evm/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

runtime-sdk/modules/evm/src/lib.rs#L621

Added line #L621 was not covered by tests
{
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
},
Expand Down
43 changes: 42 additions & 1 deletion runtime-sdk/modules/evm/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ use uint::hex::FromHex;

use oasis_runtime_sdk::{
dispatcher,
error::RuntimeError,
testing::mock::{CallOptions, Signer},
types::address::SignatureAddressSpec,
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);
Expand Down Expand Up @@ -64,6 +68,43 @@ 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<C>(
&self,
ctx: &mut C,
address: H160,
name: &str,
param_types: &[ethabi::ParamType],
params: &[ethabi::Token],
) -> Result<Vec<u8>, RuntimeError>
where
C: BatchContext,
{
let data = [
ethabi::short_signature(name, param_types).to_vec(),
ethabi::encode(params),
]
.concat();

self.query(
ctx,

Check warning on line 96 in runtime-sdk/modules/evm/src/mock.rs

View check run for this annotation

Codecov / codecov/patch

runtime-sdk/modules/evm/src/mock.rs#L96

Added line #L96 was not covered by tests
"evm.SimulateCall",
types::SimulateCallQuery {
gas_price: 0.into(),
gas_limit: 10_000_000,

Check warning on line 100 in runtime-sdk/modules/evm/src/mock.rs

View check run for this annotation

Codecov / codecov/patch

runtime-sdk/modules/evm/src/mock.rs#L100

Added line #L100 was not covered by tests
caller: self.address(),
address,

Check warning on line 102 in runtime-sdk/modules/evm/src/mock.rs

View check run for this annotation

Codecov / codecov/patch

runtime-sdk/modules/evm/src/mock.rs#L102

Added line #L102 was not covered by tests
value: 0.into(),
data,
},
)
}
}

impl std::ops::Deref for EvmSigner {
Expand Down
25 changes: 24 additions & 1 deletion runtime-sdk/modules/evm/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u8>).
pub const CODES: &[u8] = &[0x01];
Expand All @@ -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";
Expand Down Expand Up @@ -118,3 +123,21 @@ pub fn block_hashes<'a, S: Store + 'a>(state: S) -> TypedStore<impl Store + 'a>
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()
})
}
61 changes: 61 additions & 0 deletions runtime-sdk/modules/evm/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use oasis_runtime_sdk::{
use crate::{
derive_caller,
mock::{decode_reverted, decode_reverted_raw, load_contract_bytecode, EvmSigner},
state,
types::{self, H160},
Config, Genesis, Module as EVMModule,
};
Expand Down Expand Up @@ -792,6 +793,66 @@ fn test_c10l_evm_runtime() {
do_test_evm_runtime::<ConfidentialEVMConfig>();
}

#[test]
fn test_c10l_queries() {
let mut mock = mock::Mock::default();
let mut ctx = mock.create_ctx_for_runtime::<EVMRuntime<ConfidentialEVMConfig>>(
context::Mode::ExecuteTx,
true,
);
let mut signer = EvmSigner::new(0, keys::dave::sigspec());

EVMRuntime::<ConfidentialEVMConfig>::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<u8> = cbor::from_value(result).unwrap();
let contract_address = H160::from_slice(&result);

let mut ctx = mock
.create_ctx_for_runtime::<EVMRuntime<ConfidentialEVMConfig>>(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)"
);

// 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();
Expand Down
61 changes: 61 additions & 0 deletions runtime-sdk/modules/evm/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Check warning on line 119 in runtime-sdk/modules/evm/src/types.rs

View check run for this annotation

Codecov / codecov/patch

runtime-sdk/modules/evm/src/types.rs#L118-L119

Added lines #L118 - L119 were not covered by tests
}
}

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.
Expand Down Expand Up @@ -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));
}
}
21 changes: 20 additions & 1 deletion runtime-sdk/src/testing/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<C, B>(&mut self, ctx: &mut C, method: &str, body: B) -> dispatcher::DispatchResult
where
Expand Down Expand Up @@ -259,4 +266,16 @@ impl Signer {

result
}

/// Dispatch a query to the given method.
pub fn query<C, A, R>(&self, ctx: &mut C, method: &str, args: A) -> Result<R, RuntimeError>
where
C: BatchContext,
A: cbor::Encode,
R: cbor::Decode,
{
let result =
dispatcher::Dispatcher::<C::Runtime>::dispatch_query(ctx, method, cbor::to_vec(args))?;

Check warning on line 278 in runtime-sdk/src/testing/mock.rs

View check run for this annotation

Codecov / codecov/patch

runtime-sdk/src/testing/mock.rs#L278

Added line #L278 was not covered by tests
Ok(cbor::from_slice(&result).expect("result should decode correctly"))
}
}
6 changes: 6 additions & 0 deletions tests/e2e/contracts/query/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
contract = query.sol
abi = query.abi
hex = query.hex

include ../contracts.mk

1 change: 1 addition & 0 deletions tests/e2e/contracts/query/query.abi
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"inputs":[],"name":"test","outputs":[{"internalType":"address","name":"","type":"address"}],"stateMutability":"view","type":"function"}]
36 changes: 36 additions & 0 deletions tests/e2e/contracts/query/query.go
Original file line number Diff line number Diff line change
@@ -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
}()
1 change: 1 addition & 0 deletions tests/e2e/contracts/query/query.hex
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
6080604052348015600e575f80fd5b50607380601a5f395ff3fe6080604052348015600e575f80fd5b50600436106026575f3560e01c8063f8a8fd6d14602a575b5f80fd5b6040805133815290519081900360200190f3fea264697066735822122069b1e1d9de371cfb51fb1b110da57496602eb6086d8ec235c6da9ff09739c12764736f6c63430008150033
Loading

0 comments on commit 92ce19b

Please sign in to comment.