Skip to content

Commit

Permalink
runtime-sdk/modules/evm: Revert "drop signed queries in new contracts"
Browse files Browse the repository at this point in the history
Given further thought this seems too much like a foot gun.
  • Loading branch information
kostko committed Sep 18, 2023
1 parent b9bea98 commit ae0dd9c
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 128 deletions.
9 changes: 0 additions & 9 deletions runtime-sdk/modules/evm/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
13 changes: 2 additions & 11 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::{FeatureMask, H160, H256, U256};
use types::{H160, H256, U256};

#[cfg(any(test, feature = "test"))]
pub mod mock;
Expand Down Expand Up @@ -617,22 +617,13 @@ 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)
{
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
},
Expand Down
25 changes: 1 addition & 24 deletions runtime-sdk/modules/evm/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u8>).
pub const CODES: &[u8] = &[0x01];
Expand All @@ -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";
Expand Down Expand Up @@ -123,21 +118,3 @@ 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()
})
}
24 changes: 1 addition & 23 deletions runtime-sdk/modules/evm/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
}
Expand Down
61 changes: 0 additions & 61 deletions runtime-sdk/modules/evm/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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));
}
}

0 comments on commit ae0dd9c

Please sign in to comment.