From 7d0f85a4684e1e1d0c4c320b7f233c2d741f8b3c Mon Sep 17 00:00:00 2001 From: Lior Bondarevski Date: Mon, 29 Aug 2022 19:16:28 +0300 Subject: [PATCH 1/4] IBC ack and timeout should be encrypted --- .../shared/contract-engine/src/message.rs | 148 +++++++----------- .../shared/cosmwasm-types/v1.0/src/ibc.rs | 46 ------ 2 files changed, 53 insertions(+), 141 deletions(-) diff --git a/cosmwasm/enclaves/shared/contract-engine/src/message.rs b/cosmwasm/enclaves/shared/contract-engine/src/message.rs index 56fbf4749..fc484982a 100644 --- a/cosmwasm/enclaves/shared/contract-engine/src/message.rs +++ b/cosmwasm/enclaves/shared/contract-engine/src/message.rs @@ -1,10 +1,9 @@ use log::{trace, warn}; -use serde::de::DeserializeOwned; use serde::Serialize; use cosmos_proto::tx::signing::SignMode; use cw_types_v010::encoding::Binary; -use cw_types_v1::ibc::{IbcPacketAckMsg, IbcPacketReceiveMsg, IbcPacketTimeoutMsg, IbcPacketTrait}; +use cw_types_v1::ibc::IbcPacketReceiveMsg; use cw_types_v1::results::{DecryptedReply, Event, Reply, SubMsgResponse, SubMsgResult}; use enclave_cosmos_types::types::{HandleType, SigInfo}; use enclave_ffi_types::EnclaveError; @@ -91,81 +90,6 @@ pub fn try_get_decrypted_secret_msg(message: &[u8]) -> Option( - _t: T, - message: &[u8], - function_name: &str, -) -> Result -where - T: IbcPacketTrait + Serialize + DeserializeOwned + core::fmt::Debug, -{ - let mut parsed_encrypted_ibc_packet: T = - serde_json::from_slice(&message.to_vec()).map_err(|err| { - warn!( - "{} msg got an error while trying to deserialize msg input bytes into json {:?}: {}", - function_name, - String::from_utf8_lossy(&message), - err - ); - EnclaveError::FailedToDeserialize - })?; - - let tmp_secret_data = get_secret_msg(parsed_encrypted_ibc_packet.get_packet().as_slice()); - let mut was_msg_encrypted = false; - let mut orig_secret_msg = tmp_secret_data; - - match orig_secret_msg.decrypt() { - Ok(decrypted_msg) => { - // IBC packet was encrypted - - trace!( - "{} data before decryption: {:?}", - function_name, - base64::encode(&message) - ); - - parsed_encrypted_ibc_packet.set_packet(decrypted_msg.as_slice().into()); - was_msg_encrypted = true; - } - Err(_) => { - // assume data is not encrypted - - trace!( - "{} data was plaintext: {:?}", - function_name, - base64::encode(&message) - ); - } - } - - let ack = parsed_encrypted_ibc_packet.get_ack(); - let tmp_secret_ack = get_secret_msg(ack.unwrap_or(Binary::from(vec![].as_slice())).as_slice()); - - match tmp_secret_ack.decrypt() { - Ok(ack_data) => { - parsed_encrypted_ibc_packet.set_ack(ack_data.as_slice().into()); - was_msg_encrypted = true; - - orig_secret_msg = tmp_secret_ack; - } - Err(_) => {} - } - - Ok(ParsedMessage { - should_validate_sig_info: false, - was_msg_encrypted, - secret_msg: orig_secret_msg, - decrypted_msg: serde_json::to_vec(&parsed_encrypted_ibc_packet).map_err(|err| { - warn!( - "got an error while trying to serialize {} msg into bytes {:?}: {}", - function_name, parsed_encrypted_ibc_packet, err - ); - EnclaveError::FailedToSerialize - })?, - contract_hash_for_validation: None, - }) -} - // Parse the message that was passed to handle (Based on the assumption that it might be a reply or IBC as well) pub fn parse_message( message: &[u8], @@ -481,7 +405,9 @@ pub fn parse_message( } HandleType::HANDLE_TYPE_IBC_CHANNEL_OPEN | HandleType::HANDLE_TYPE_IBC_CHANNEL_CONNECT - | HandleType::HANDLE_TYPE_IBC_CHANNEL_CLOSE => { + | HandleType::HANDLE_TYPE_IBC_CHANNEL_CLOSE + | HandleType::HANDLE_TYPE_IBC_PACKET_ACK + | HandleType::HANDLE_TYPE_IBC_PACKET_TIMEOUT => { trace!( "parsing {} msg (Should always be plaintext): {:?}", HandleType::to_export_name(&handle_type), @@ -506,23 +432,55 @@ pub fn parse_message( } HandleType::HANDLE_TYPE_IBC_PACKET_RECEIVE => { // TODO: Maybe mark whether the message was encrypted or not. - parse_ibc_packet( - IbcPacketReceiveMsg::default(), - message, - "ibc_packet_receive", - ) - } - HandleType::HANDLE_TYPE_IBC_PACKET_ACK => { - // TODO: Maybe mark whether the message was encrypted or not. - parse_ibc_packet(IbcPacketAckMsg::default(), message, "ibc_packet_receive") - } - HandleType::HANDLE_TYPE_IBC_PACKET_TIMEOUT => { - // TODO: Maybe mark whether the message was encrypted or not. - parse_ibc_packet( - IbcPacketTimeoutMsg::default(), - message, - "ibc_packet_timeout", - ) + let mut parsed_encrypted_ibc_packet: IbcPacketReceiveMsg = + serde_json::from_slice(&message.to_vec()).map_err(|err| { + warn!( + "Got an error while trying to deserialize input bytes msg into IbcPacketReceiveMsg message {:?}: {}", + String::from_utf8_lossy(&message), + err + ); + EnclaveError::FailedToDeserialize + })?; + + let tmp_secret_data = + get_secret_msg(parsed_encrypted_ibc_packet.packet.data.as_slice()); + let mut was_msg_encrypted = false; + let orig_secret_msg = tmp_secret_data; + + match orig_secret_msg.decrypt() { + Ok(decrypted_msg) => { + // IBC packet was encrypted + + trace!( + "ibc_packet_receive data before decryption: {:?}", + base64::encode(&message) + ); + + parsed_encrypted_ibc_packet.packet.data = decrypted_msg.as_slice().into(); + was_msg_encrypted = true; + } + Err(_) => { + // assume data is not encrypted + + trace!( + "ibc_packet_receive data was plaintext: {:?}", + base64::encode(&message) + ); + } + } + Ok(ParsedMessage { + should_validate_sig_info: false, + was_msg_encrypted, + secret_msg: orig_secret_msg, + decrypted_msg: serde_json::to_vec(&parsed_encrypted_ibc_packet).map_err(|err| { + warn!( + "got an error while trying to serialize IbcPacketReceive msg into bytes {:?}: {}", + parsed_encrypted_ibc_packet, err + ); + EnclaveError::FailedToSerialize + })?, + contract_hash_for_validation: None, + }) } }; } diff --git a/cosmwasm/enclaves/shared/cosmwasm-types/v1.0/src/ibc.rs b/cosmwasm/enclaves/shared/cosmwasm-types/v1.0/src/ibc.rs index f598d984c..6c620e915 100644 --- a/cosmwasm/enclaves/shared/cosmwasm-types/v1.0/src/ibc.rs +++ b/cosmwasm/enclaves/shared/cosmwasm-types/v1.0/src/ibc.rs @@ -179,52 +179,6 @@ pub struct IbcPacketReceiveMsg { pub relayer: Addr, } -pub trait IbcPacketTrait { - type Data; - fn get_packet(&self) -> &Self::Data; - fn set_packet(&mut self, data: Self::Data); - fn get_ack(&self) -> Option; - fn set_ack(&mut self, data: Self::Data); -} - -macro_rules! impl_IbcPacketTrait { - ($($t:ty),+) => { - $(impl IbcPacketTrait for $t { - type Data = Binary; - fn get_packet(&self) -> &Self::Data { - &self.packet.data - } - fn set_packet(&mut self, data: Self::Data) { - self.packet.data = data; - } - fn get_ack(&self) -> Option { - return None; - } - fn set_ack(&mut self, _: Self::Data) { - // Nothing to do here - } - })* - } -} - -impl_IbcPacketTrait! {IbcPacketReceiveMsg, IbcPacketTimeoutMsg} - -impl IbcPacketTrait for IbcPacketAckMsg { - type Data = Binary; - fn get_packet(&self) -> &Self::Data { - &self.original_packet.data - } - fn set_packet(&mut self, data: Self::Data) { - self.original_packet.data = data; - } - fn get_ack(&self) -> Option { - Some(self.acknowledgement.data.clone()) - } - fn set_ack(&mut self, data: Self::Data) { - self.acknowledgement.data = data; - } -} - impl IbcPacketReceiveMsg { pub fn default() -> Self { Self { From b058d3ce0d9dc6ff42fd43687aaa58674812f610 Mon Sep 17 00:00:00 2001 From: Lior Bondarevski Date: Mon, 29 Aug 2022 19:16:40 +0300 Subject: [PATCH 2/4] IBC ack tests --- x/compute/internal/keeper/ibc_test.go | 144 +++++++++++++++++- .../keeper/testdata/ibc/src/contract.rs | 26 +++- 2 files changed, 157 insertions(+), 13 deletions(-) diff --git a/x/compute/internal/keeper/ibc_test.go b/x/compute/internal/keeper/ibc_test.go index 10ceb1325..ae042da74 100644 --- a/x/compute/internal/keeper/ibc_test.go +++ b/x/compute/internal/keeper/ibc_test.go @@ -1,6 +1,7 @@ package keeper import ( + "encoding/binary" "encoding/hex" "fmt" "math" @@ -246,7 +247,7 @@ func ibcPacketReceiveHelper( func ibcPacketAckHelper( t *testing.T, keeper Keeper, ctx sdk.Context, contractAddr sdk.AccAddress, creatorPrivKey crypto.PrivKey, gas uint64, originalPacket v1types.IBCPacket, ack []byte, -) cosmwasm.StdError { +) (sdk.Context, []ContractEvent, cosmwasm.StdError) { // create new ctx with the same storage and a gas limit // this is to reset the event manager, so we won't get // events from past calls @@ -271,16 +272,19 @@ func ibcPacketAckHelper( require.NotZero(t, gasMeter.GetWasmCounter(), err) if err != nil { - return cosmwasm.StdError{GenericErr: &cosmwasm.GenericErr{Msg: err.Error()}} + return ctx, nil, cosmwasm.StdError{GenericErr: &cosmwasm.GenericErr{Msg: err.Error()}} } - return cosmwasm.StdError{} + // wasmEvents comes from all the callbacks as well + wasmEvents := tryDecryptWasmEvents(ctx, []byte{}, true) + + return ctx, wasmEvents, cosmwasm.StdError{} } func ibcPacketTimeoutHelper( t *testing.T, keeper Keeper, ctx sdk.Context, contractAddr sdk.AccAddress, creatorPrivKey crypto.PrivKey, gas uint64, originalPacket v1types.IBCPacket, -) cosmwasm.StdError { +) (sdk.Context, []ContractEvent, cosmwasm.StdError) { // create new ctx with the same storage and a gas limit // this is to reset the event manager, so we won't get // events from past calls @@ -302,10 +306,13 @@ func ibcPacketTimeoutHelper( require.NotZero(t, gasMeter.GetWasmCounter(), err) if err != nil { - return cosmwasm.StdError{GenericErr: &cosmwasm.GenericErr{Msg: err.Error()}} + return ctx, nil, cosmwasm.StdError{GenericErr: &cosmwasm.GenericErr{Msg: err.Error()}} } - return cosmwasm.StdError{} + // wasmEvents comes from all the callbacks as well + wasmEvents := tryDecryptWasmEvents(ctx, []byte{}, true) + + return ctx, wasmEvents, cosmwasm.StdError{} } func TestIBCChannelOpen(t *testing.T) { @@ -773,3 +780,128 @@ func TestIBCPacketReceive(t *testing.T) { } } } + +func TestIBCPacketAck(t *testing.T) { + ctx, keeper, codeID, _, walletA, privKeyA, _, _ := setupTest(t, "./testdata/ibc/contract.wasm", sdk.NewCoins()) + + _, _, contractAddress, _, err := initHelper(t, keeper, ctx, codeID, walletA, privKeyA, `{"init":{}}`, true, true, defaultGasForTests) + require.Empty(t, err) + + for _, test := range []struct { + description string + sequence uint64 + output string + isSuccess bool + hasAttributes bool + hasEvents bool + }{ + { + description: "Default", + sequence: 0, + output: "8", + isSuccess: true, + hasAttributes: false, + hasEvents: false, + }, + { + description: "SubmessageNoReply", + sequence: 1, + output: "14", + isSuccess: true, + hasAttributes: false, + hasEvents: false, + }, + { + description: "SubmessageWithReply", + sequence: 2, + output: "21", + isSuccess: true, + hasAttributes: false, + hasEvents: false, + }, + { + description: "Attributes", + sequence: 3, + output: "11", + isSuccess: true, + hasAttributes: true, + hasEvents: false, + }, + { + description: "Events", + sequence: 4, + output: "12", + isSuccess: true, + hasAttributes: false, + hasEvents: true, + }, + { + description: "Error", + sequence: 5, + output: "", + isSuccess: false, + hasAttributes: false, + hasEvents: false, + }, + } { + t.Run(test.description, func(t *testing.T) { + ibcPacket := createIBCPacket(createIBCEndpoint(PortIDForContract(contractAddress), "channel.1"), + createIBCEndpoint(PortIDForContract(contractAddress), "channel.0"), + test.sequence, + createIBCTimeout(math.MaxUint64), + []byte{}, + ) + ack := make([]byte, 8) + binary.LittleEndian.PutUint64(ack, uint64(test.sequence)) + + ctx, events, err := ibcPacketAckHelper(t, keeper, ctx, contractAddress, privKeyA, defaultGasForTests, ibcPacket, ack) + + if !test.isSuccess { + require.Contains(t, fmt.Sprintf("%+v", err), "Intentional") + } else { + require.Empty(t, err) + if test.hasAttributes { + require.Equal(t, + []ContractEvent{ + { + {Key: "contract_address", Value: contractAddress.String()}, + {Key: "attr1", Value: "😗"}, + }, + }, + events, + ) + } + + if test.hasEvents { + hadCyber1 := false + evts := ctx.EventManager().Events() + for _, e := range evts { + if e.Type == "wasm-cyber1" { + require.False(t, hadCyber1) + attrs, err := parseAndDecryptAttributes(e.Attributes, []byte{}, false) + require.Empty(t, err) + + require.Equal(t, + []v010cosmwasm.LogAttribute{ + {Key: "contract_address", Value: contractAddress.String()}, + {Key: "attr1", Value: "🤯"}, + }, + attrs, + ) + + hadCyber1 = true + } + } + + require.True(t, hadCyber1) + } + + queryRes, err := queryHelper(t, keeper, ctx, contractAddress, `{"q":{}}`, true, true, math.MaxUint64) + + require.Empty(t, err) + + require.Equal(t, test.output, queryRes) + } + }) + } +} diff --git a/x/compute/internal/keeper/testdata/ibc/src/contract.rs b/x/compute/internal/keeper/testdata/ibc/src/contract.rs index 9b66e4936..cb5016848 100644 --- a/x/compute/internal/keeper/testdata/ibc/src/contract.rs +++ b/x/compute/internal/keeper/testdata/ibc/src/contract.rs @@ -3,8 +3,8 @@ use crate::state::{count, count_read}; use cosmwasm_std::{ entry_point, to_binary, Binary, CosmosMsg, Deps, DepsMut, Env, Event, Ibc3ChannelOpenResponse, IbcBasicResponse, IbcChannelCloseMsg, IbcChannelConnectMsg, IbcChannelOpenMsg, - IbcChannelOpenResponse, IbcPacketReceiveMsg, IbcReceiveResponse, MessageInfo, Reply, ReplyOn, - Response, StdError, StdResult, SubMsg, SubMsgResult, WasmMsg, + IbcChannelOpenResponse, IbcPacketAckMsg, IbcPacketReceiveMsg, IbcReceiveResponse, MessageInfo, + Reply, ReplyOn, Response, StdError, StdResult, SubMsg, SubMsgResult, WasmMsg, }; pub const IBC_APP_VERSION: &str = "ibc-v1"; @@ -177,8 +177,6 @@ pub fn ibc_channel_connect( } #[entry_point] -/// On closed channel, we take all tokens from reflect contract to this contract. -/// We also delete the channel entry from accounts. pub fn ibc_channel_close( deps: DepsMut, env: Env, @@ -202,9 +200,6 @@ pub fn ibc_channel_close( } #[entry_point] -/// we look for a the proper reflect contract to relay to and send the message -/// We cannot return any meaningful response value as we do not know the response value -/// of execution. We just return ok if we dispatched, error if we failed to dispatch pub fn ibc_packet_receive( deps: DepsMut, env: Env, @@ -221,3 +216,20 @@ pub fn ibc_packet_receive( resp } + +#[entry_point] +pub fn ibc_packet_ack( + deps: DepsMut, + env: Env, + msg: IbcPacketAckMsg, +) -> StdResult { + let mut ack = [0u8; 8]; + ack.copy_from_slice(&msg.acknowledgement.data.as_slice()[0..8]); + + if u64::from_le_bytes(ack) != msg.original_packet.sequence { + return Err(StdError::generic_err("Wrong ack")); + } + + count(deps.storage).save(&(msg.original_packet.sequence + 8))?; + get_resp_based_on_num(env, msg.original_packet.sequence) +} From 5b97e6fef205809dca814dd4158cefbcab61bff8 Mon Sep 17 00:00:00 2001 From: Lior Bondarevski Date: Tue, 30 Aug 2022 11:11:24 +0300 Subject: [PATCH 3/4] Encrypt the output of every reply (Should be changed) --- .../src/contract_operations.rs | 3 +- .../enclaves/shared/contract-engine/src/io.rs | 4 +- .../shared/contract-engine/src/message.rs | 9 +- x/compute/internal/keeper/ibc_test.go | 123 ++++++++++++++++++ .../internal/keeper/secret_contracts_test.go | 3 - x/compute/internal/keeper/test_common.go | 3 + .../keeper/testdata/ibc/src/contract.rs | 17 ++- 7 files changed, 152 insertions(+), 10 deletions(-) diff --git a/cosmwasm/enclaves/shared/contract-engine/src/contract_operations.rs b/cosmwasm/enclaves/shared/contract-engine/src/contract_operations.rs index 274d3782a..195bf917c 100644 --- a/cosmwasm/enclaves/shared/contract-engine/src/contract_operations.rs +++ b/cosmwasm/enclaves/shared/contract-engine/src/contract_operations.rs @@ -186,6 +186,7 @@ pub fn handle( let ParsedMessage { should_validate_sig_info, was_msg_encrypted, + should_encrypt_output, secret_msg, decrypted_msg, contract_hash_for_validation, @@ -262,7 +263,7 @@ pub fn handle( secret_msg.nonce, secret_msg.user_public_key ); - if was_msg_encrypted { + if should_encrypt_output { output = encrypt_output( output, &secret_msg, diff --git a/cosmwasm/enclaves/shared/contract-engine/src/io.rs b/cosmwasm/enclaves/shared/contract-engine/src/io.rs index b5177c8cf..ac2666ee8 100644 --- a/cosmwasm/enclaves/shared/contract-engine/src/io.rs +++ b/cosmwasm/enclaves/shared/contract-engine/src/io.rs @@ -411,8 +411,8 @@ pub fn encrypt_output( // More info in: https://github.com/CosmWasm/cosmwasm/blob/v1.0.0/packages/std/src/results/submessages.rs#L192-L198 let encryption_key = calc_encryption_key(&secret_msg.nonce, &secret_msg.user_public_key); trace!( - "Output before encryption: {:?}", - String::from_utf8_lossy(&output) + "Output before encryption: {:?} {:?} {:?}", + String::from_utf8_lossy(&output), secret_msg.nonce, secret_msg.user_public_key ); let mut output: RawWasmOutput = serde_json::from_slice(&output).map_err(|err| { diff --git a/cosmwasm/enclaves/shared/contract-engine/src/message.rs b/cosmwasm/enclaves/shared/contract-engine/src/message.rs index fc484982a..4e959b088 100644 --- a/cosmwasm/enclaves/shared/contract-engine/src/message.rs +++ b/cosmwasm/enclaves/shared/contract-engine/src/message.rs @@ -1,5 +1,4 @@ use log::{trace, warn}; -use serde::Serialize; use cosmos_proto::tx::signing::SignMode; use cw_types_v010::encoding::Binary; @@ -15,6 +14,7 @@ const HEX_ENCODED_HASH_SIZE: usize = 64; pub struct ParsedMessage { pub should_validate_sig_info: bool, pub was_msg_encrypted: bool, + pub should_encrypt_output: bool, pub secret_msg: SecretMessage, pub decrypted_msg: Vec, pub contract_hash_for_validation: Option>, @@ -107,6 +107,7 @@ pub fn parse_message( Ok(ParsedMessage { should_validate_sig_info: true, was_msg_encrypted: true, + should_encrypt_output: true, secret_msg: decrypted_secret_msg.secret_msg, decrypted_msg: decrypted_secret_msg.decrypted_msg, contract_hash_for_validation: None, @@ -124,6 +125,7 @@ pub fn parse_message( Ok(ParsedMessage { should_validate_sig_info: false, was_msg_encrypted: false, + should_encrypt_output: false, secret_msg, decrypted_msg, contract_hash_for_validation: None, @@ -197,6 +199,7 @@ pub fn parse_message( return Ok(ParsedMessage { should_validate_sig_info: false, was_msg_encrypted: false, + should_encrypt_output: true, // When replies are plaintext it doesn't mean we can't encrypt them secret_msg: reply_secret_msg, decrypted_msg: serialized_reply, contract_hash_for_validation: None, @@ -301,6 +304,7 @@ pub fn parse_message( Ok(ParsedMessage { should_validate_sig_info: true, was_msg_encrypted: true, + should_encrypt_output: true, secret_msg: reply_secret_msg, decrypted_msg: decrypted_reply_as_vec, contract_hash_for_validation: Some( @@ -394,6 +398,7 @@ pub fn parse_message( Ok(ParsedMessage { should_validate_sig_info: true, was_msg_encrypted: true, + should_encrypt_output: true, secret_msg: reply_secret_msg, decrypted_msg: decrypted_reply_as_vec, contract_hash_for_validation: Some( @@ -425,6 +430,7 @@ pub fn parse_message( Ok(ParsedMessage { should_validate_sig_info: false, was_msg_encrypted: false, + should_encrypt_output: false, secret_msg: scrt_msg, decrypted_msg, contract_hash_for_validation: None, @@ -471,6 +477,7 @@ pub fn parse_message( Ok(ParsedMessage { should_validate_sig_info: false, was_msg_encrypted, + should_encrypt_output: was_msg_encrypted, secret_msg: orig_secret_msg, decrypted_msg: serde_json::to_vec(&parsed_encrypted_ibc_packet).map_err(|err| { warn!( diff --git a/x/compute/internal/keeper/ibc_test.go b/x/compute/internal/keeper/ibc_test.go index ae042da74..8d224b0d5 100644 --- a/x/compute/internal/keeper/ibc_test.go +++ b/x/compute/internal/keeper/ibc_test.go @@ -905,3 +905,126 @@ func TestIBCPacketAck(t *testing.T) { }) } } + +func TestIBCPacketTimeout(t *testing.T) { + ctx, keeper, codeID, _, walletA, privKeyA, _, _ := setupTest(t, "./testdata/ibc/contract.wasm", sdk.NewCoins()) + + _, _, contractAddress, _, err := initHelper(t, keeper, ctx, codeID, walletA, privKeyA, `{"init":{}}`, true, true, defaultGasForTests) + require.Empty(t, err) + + for _, test := range []struct { + description string + sequence uint64 + output string + isSuccess bool + hasAttributes bool + hasEvents bool + }{ + { + description: "Default", + sequence: 0, + output: "9", + isSuccess: true, + hasAttributes: false, + hasEvents: false, + }, + { + description: "SubmessageNoReply", + sequence: 1, + output: "15", + isSuccess: true, + hasAttributes: false, + hasEvents: false, + }, + { + description: "SubmessageWithReply", + sequence: 2, + output: "22", + isSuccess: true, + hasAttributes: false, + hasEvents: false, + }, + { + description: "Attributes", + sequence: 3, + output: "12", + isSuccess: true, + hasAttributes: true, + hasEvents: false, + }, + { + description: "Events", + sequence: 4, + output: "13", + isSuccess: true, + hasAttributes: false, + hasEvents: true, + }, + { + description: "Error", + sequence: 5, + output: "", + isSuccess: false, + hasAttributes: false, + hasEvents: false, + }, + } { + t.Run(test.description, func(t *testing.T) { + ibcPacket := createIBCPacket(createIBCEndpoint(PortIDForContract(contractAddress), "channel.1"), + createIBCEndpoint(PortIDForContract(contractAddress), "channel.0"), + test.sequence, + createIBCTimeout(math.MaxUint64), + []byte{}, + ) + + ctx, events, err := ibcPacketTimeoutHelper(t, keeper, ctx, contractAddress, privKeyA, defaultGasForTests, ibcPacket) + + if !test.isSuccess { + require.Contains(t, fmt.Sprintf("%+v", err), "Intentional") + } else { + require.Empty(t, err) + if test.hasAttributes { + require.Equal(t, + []ContractEvent{ + { + {Key: "contract_address", Value: contractAddress.String()}, + {Key: "attr1", Value: "😗"}, + }, + }, + events, + ) + } + + if test.hasEvents { + hadCyber1 := false + evts := ctx.EventManager().Events() + for _, e := range evts { + if e.Type == "wasm-cyber1" { + require.False(t, hadCyber1) + attrs, err := parseAndDecryptAttributes(e.Attributes, []byte{}, false) + require.Empty(t, err) + + require.Equal(t, + []v010cosmwasm.LogAttribute{ + {Key: "contract_address", Value: contractAddress.String()}, + {Key: "attr1", Value: "🤯"}, + }, + attrs, + ) + + hadCyber1 = true + } + } + + require.True(t, hadCyber1) + } + + queryRes, err := queryHelper(t, keeper, ctx, contractAddress, `{"q":{}}`, true, true, math.MaxUint64) + + require.Empty(t, err) + + require.Equal(t, test.output, queryRes) + } + }) + } +} diff --git a/x/compute/internal/keeper/secret_contracts_test.go b/x/compute/internal/keeper/secret_contracts_test.go index 50c07f3c9..2ab742628 100644 --- a/x/compute/internal/keeper/secret_contracts_test.go +++ b/x/compute/internal/keeper/secret_contracts_test.go @@ -232,11 +232,8 @@ func tryDecryptWasmEvents(ctx sdk.Context, nonce []byte, shouldSkipAttributes .. if !shouldSkip && newLog.Key != "contract_address" { // key - fmt.Printf("LIORRR before %+v nonce %+v", newLog, nonce) newAttr, err := decryptAttribute(newLog, nonce) - fmt.Printf("LIORRR after %+v", newAttr) if err != nil { - fmt.Printf("LIORRR here %+v", err) continue } diff --git a/x/compute/internal/keeper/test_common.go b/x/compute/internal/keeper/test_common.go index b5f67681b..7c34ba7e8 100644 --- a/x/compute/internal/keeper/test_common.go +++ b/x/compute/internal/keeper/test_common.go @@ -369,6 +369,9 @@ func CreateTestInput(t *testing.T, isCheckTx bool, supportedFeatures string, enc ) memKeys := sdk.NewMemoryStoreKeys(capabilitytypes.MemStoreKey) + for _, v := range memKeys { + ms.MountStoreWithDB(v, sdk.StoreTypeMemory, db) + } upgradeKeeper := upgradekeeper.NewKeeper( map[int64]bool{}, diff --git a/x/compute/internal/keeper/testdata/ibc/src/contract.rs b/x/compute/internal/keeper/testdata/ibc/src/contract.rs index cb5016848..c8cd046b8 100644 --- a/x/compute/internal/keeper/testdata/ibc/src/contract.rs +++ b/x/compute/internal/keeper/testdata/ibc/src/contract.rs @@ -3,8 +3,9 @@ use crate::state::{count, count_read}; use cosmwasm_std::{ entry_point, to_binary, Binary, CosmosMsg, Deps, DepsMut, Env, Event, Ibc3ChannelOpenResponse, IbcBasicResponse, IbcChannelCloseMsg, IbcChannelConnectMsg, IbcChannelOpenMsg, - IbcChannelOpenResponse, IbcPacketAckMsg, IbcPacketReceiveMsg, IbcReceiveResponse, MessageInfo, - Reply, ReplyOn, Response, StdError, StdResult, SubMsg, SubMsgResult, WasmMsg, + IbcChannelOpenResponse, IbcPacketAckMsg, IbcPacketReceiveMsg, IbcPacketTimeoutMsg, + IbcReceiveResponse, MessageInfo, Reply, ReplyOn, Response, StdError, StdResult, SubMsg, + SubMsgResult, WasmMsg, }; pub const IBC_APP_VERSION: &str = "ibc-v1"; @@ -43,7 +44,7 @@ pub fn reply(deps: DepsMut, _env: Env, reply: Reply) -> StdResult { (1, SubMsgResult::Err(_)) => Err(StdError::generic_err("Failed to inc")), (1, SubMsgResult::Ok(_)) => { increment(deps, 6)?; - Ok(Response::default()) + Ok(Response::new().set_data(to_binary(&"out2".to_string())?)) } _ => Err(StdError::generic_err("invalid reply id or result")), } @@ -233,3 +234,13 @@ pub fn ibc_packet_ack( count(deps.storage).save(&(msg.original_packet.sequence + 8))?; get_resp_based_on_num(env, msg.original_packet.sequence) } + +// #[entry_point] +// pub fn ibc_packet_timeout( +// deps: DepsMut, +// env: Env, +// msg: IbcPacketTimeoutMsg, +// ) -> StdResult { +// count(deps.storage).save(&(msg.packet.sequence + 9))?; +// get_resp_based_on_num(env, msg.packet.sequence) +// } From 7d65399ed9904314dbabc9ea8e86a552483b6b91 Mon Sep 17 00:00:00 2001 From: Lior Bondarevski Date: Tue, 30 Aug 2022 15:22:56 +0300 Subject: [PATCH 4/4] Encrypt plaintext replies only if the original message was encrypted --- .../enclaves/shared/contract-engine/src/io.rs | 11 ++- .../shared/contract-engine/src/message.rs | 2 +- .../v1.0/src/results/submessages.rs | 10 +++ go-cosmwasm/types/v1/subcall.go | 14 ++-- x/compute/internal/keeper/ibc_test.go | 72 +++++++++++++++++ x/compute/internal/keeper/msg_dispatcher.go | 5 +- x/compute/internal/keeper/test_common.go | 2 +- .../keeper/testdata/ibc/src/contract.rs | 78 +++++++++++++++++-- 8 files changed, 178 insertions(+), 16 deletions(-) diff --git a/cosmwasm/enclaves/shared/contract-engine/src/io.rs b/cosmwasm/enclaves/shared/contract-engine/src/io.rs index ac2666ee8..0818c2ee2 100644 --- a/cosmwasm/enclaves/shared/contract-engine/src/io.rs +++ b/cosmwasm/enclaves/shared/contract-engine/src/io.rs @@ -412,7 +412,9 @@ pub fn encrypt_output( let encryption_key = calc_encryption_key(&secret_msg.nonce, &secret_msg.user_public_key); trace!( "Output before encryption: {:?} {:?} {:?}", - String::from_utf8_lossy(&output), secret_msg.nonce, secret_msg.user_public_key + String::from_utf8_lossy(&output), + secret_msg.nonce, + secret_msg.user_public_key ); let mut output: RawWasmOutput = serde_json::from_slice(&output).map_err(|err| { @@ -449,6 +451,7 @@ pub fn encrypt_output( let reply = Reply { id: msg_id.unwrap(), result: SubMsgResult::Err(encrypted_err), + was_msg_encrypted: true, }; let reply_as_vec = serde_json::to_vec(&reply).map_err(|err| { warn!( @@ -537,6 +540,7 @@ pub fn encrypt_output( events, data: ok.data.clone(), }), + was_msg_encrypted: true, }; let reply_as_vec = serde_json::to_vec(&reply).map_err(|err| { @@ -580,6 +584,8 @@ pub fn encrypt_output( // We don't encrypt it here to remain with the same type (u64) sub_msg.id = 0; } + + sub_msg.was_msg_encrypted = true; } // v1: The attributes that will be emitted as part of a "wasm" event. @@ -649,6 +655,7 @@ pub fn encrypt_output( events, data: ok.data.clone(), }), + was_msg_encrypted: true, }; let reply_as_vec = serde_json::to_vec(&reply).map_err(|err| { @@ -689,6 +696,8 @@ pub fn encrypt_output( // We don't encrypt it here to remain with the same type (u64) sub_msg.id = 0; } + + sub_msg.was_msg_encrypted = true; } // v1: The attributes that will be emitted as part of a "wasm" event. diff --git a/cosmwasm/enclaves/shared/contract-engine/src/message.rs b/cosmwasm/enclaves/shared/contract-engine/src/message.rs index 4e959b088..d5bbf98fb 100644 --- a/cosmwasm/enclaves/shared/contract-engine/src/message.rs +++ b/cosmwasm/enclaves/shared/contract-engine/src/message.rs @@ -199,7 +199,7 @@ pub fn parse_message( return Ok(ParsedMessage { should_validate_sig_info: false, was_msg_encrypted: false, - should_encrypt_output: true, // When replies are plaintext it doesn't mean we can't encrypt them + should_encrypt_output: reply.was_msg_encrypted, secret_msg: reply_secret_msg, decrypted_msg: serialized_reply, contract_hash_for_validation: None, diff --git a/cosmwasm/enclaves/shared/cosmwasm-types/v1.0/src/results/submessages.rs b/cosmwasm/enclaves/shared/cosmwasm-types/v1.0/src/results/submessages.rs index 78e0c00bf..4a0f08412 100644 --- a/cosmwasm/enclaves/shared/cosmwasm-types/v1.0/src/results/submessages.rs +++ b/cosmwasm/enclaves/shared/cosmwasm-types/v1.0/src/results/submessages.rs @@ -21,6 +21,10 @@ pub enum ReplyOn { Never, } +fn bool_false() -> bool { + false +} + /// A submessage that will guarantee a `reply` call on success or error, depending on /// the `reply_on` setting. If you do not need to process the result, use regular messages instead. /// @@ -38,6 +42,11 @@ where pub msg: CosmosMsg, pub gas_limit: Option, pub reply_on: ReplyOn, + // An indication that will be passed to the reply that will indicate wether the original message, + // which is the one who create the submessages, was encrypted or not. + // Plaintext replies will be encrypted only if the original message was. + #[serde(default = "bool_false")] + pub was_msg_encrypted: bool, } /// The information we get back from a successful sub message execution, @@ -65,6 +74,7 @@ pub struct Reply { /// Use this to identify which submessage triggered the `reply`. pub id: Binary, pub result: SubMsgResult, + pub was_msg_encrypted: bool, } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] pub struct DecryptedReply { diff --git a/go-cosmwasm/types/v1/subcall.go b/go-cosmwasm/types/v1/subcall.go index 0be755b38..7732e600a 100644 --- a/go-cosmwasm/types/v1/subcall.go +++ b/go-cosmwasm/types/v1/subcall.go @@ -66,15 +66,17 @@ type SubMsgResult struct { // SubMsg wraps a CosmosMsg with some metadata for handling replies (ID) and optionally // limiting the gas usage (GasLimit) type SubMsg struct { - ID uint64 `json:"id"` - Msg CosmosMsg `json:"msg"` - GasLimit *uint64 `json:"gas_limit,omitempty"` - ReplyOn replyOn `json:"reply_on"` + ID uint64 `json:"id"` + Msg CosmosMsg `json:"msg"` + GasLimit *uint64 `json:"gas_limit,omitempty"` + ReplyOn replyOn `json:"reply_on"` + WasMsgEncrypted bool `json:"was_msg_encrypted"` } type Reply struct { - ID []byte `json:"id"` - Result SubMsgResult `json:"result"` + ID []byte `json:"id"` + Result SubMsgResult `json:"result"` + WasMsgEncrypted bool `json:"was_msg_encrypted"` } // SubcallResult is the raw response we return from the sdk -> reply after executing a SubMsg. diff --git a/x/compute/internal/keeper/ibc_test.go b/x/compute/internal/keeper/ibc_test.go index 8d224b0d5..13548318a 100644 --- a/x/compute/internal/keeper/ibc_test.go +++ b/x/compute/internal/keeper/ibc_test.go @@ -3,8 +3,10 @@ package keeper import ( "encoding/binary" "encoding/hex" + "encoding/json" "fmt" "math" + "os" "testing" crypto "github.com/cosmos/cosmos-sdk/crypto/types" @@ -719,6 +721,14 @@ func TestIBCPacketReceive(t *testing.T) { hasAttributes: false, hasEvents: false, }, + { + description: "SubmessageWithReplyThatCallsToSubmessage", + sequence: 6, + output: "35", + isSuccess: true, + hasAttributes: false, + hasEvents: false, + }, } { t.Run(fmt.Sprintf("%s-Encryption:%t", test.description, isEncrypted), func(t *testing.T) { ibcPacket := createIBCPacket(createIBCEndpoint(PortIDForContract(contractAddress), "channel.1"), @@ -781,6 +791,68 @@ func TestIBCPacketReceive(t *testing.T) { } } +type ContractInfo struct { + Address string `json:"address"` + Hash string `json:"hash"` +} + +func TestIBCPacketReceiveCallsV010Contract(t *testing.T) { + ctx, keeper, codeID, _, walletA, privKeyA, _, _ := setupTest(t, "./testdata/ibc/contract.wasm", sdk.NewCoins()) + + wasmCode, err := os.ReadFile("./testdata/test-contract/contract.wasm") + require.NoError(t, err) + + v010CodeID, err := keeper.Create(ctx, walletA, wasmCode, "", "") + require.NoError(t, err) + + codeInfo, err := keeper.GetCodeInfo(ctx, v010CodeID) + require.NoError(t, err) + v010CodeHash := hex.EncodeToString(codeInfo.CodeHash) + + _, _, contractAddress, _, err := initHelper(t, keeper, ctx, codeID, walletA, privKeyA, `{"init":{}}`, true, true, defaultGasForTests) + require.Empty(t, err) + _, _, v010ContractAddress, _, err := initHelper(t, keeper, ctx, v010CodeID, walletA, privKeyA, `{"counter":{"counter":10}}`, true, false, defaultGasForTests) + require.Empty(t, err) + contractInfo := ContractInfo{ + Address: v010ContractAddress.String(), + Hash: v010CodeHash, + } + + contractInfoBz, err := json.Marshal(contractInfo) + require.NoError(t, err) + + ibcPacket := createIBCPacket(createIBCEndpoint(PortIDForContract(contractAddress), "channel.1"), + createIBCEndpoint(PortIDForContract(contractAddress), "channel.0"), + 7, + createIBCTimeout(math.MaxUint64), + contractInfoBz, + ) + + expected_v010_result := uint32(15) + + for _, isEncrypted := range []bool{true, true} { + t.Run(fmt.Sprintf("Encryption:%t", isEncrypted), func(t *testing.T) { + ctx, _, _, data, err := ibcPacketReceiveHelper(t, keeper, ctx, contractAddress, privKeyA, isEncrypted, defaultGasForTests, ibcPacket) + require.Empty(t, err) + require.Equal(t, "\"out\"", string(data)) + + queryRes, err := queryHelper(t, keeper, ctx, contractAddress, `{"q":{}}`, true, true, math.MaxUint64) + + require.Empty(t, err) + require.Equal(t, "20", queryRes) + + queryRes, qErr := queryHelper(t, keeper, ctx, v010ContractAddress, `{"get":{}}`, true, false, math.MaxUint64) + require.Empty(t, qErr) + + var resp v1QueryResponse + e := json.Unmarshal([]byte(queryRes), &resp) + require.NoError(t, e) + require.Equal(t, expected_v010_result, resp.Get.Count) + expected_v010_result += 5 + }) + } +} + func TestIBCPacketAck(t *testing.T) { ctx, keeper, codeID, _, walletA, privKeyA, _, _ := setupTest(t, "./testdata/ibc/contract.wasm", sdk.NewCoins()) diff --git a/x/compute/internal/keeper/msg_dispatcher.go b/x/compute/internal/keeper/msg_dispatcher.go index 92792f986..234c90532 100644 --- a/x/compute/internal/keeper/msg_dispatcher.go +++ b/x/compute/internal/keeper/msg_dispatcher.go @@ -274,8 +274,9 @@ func (d MessageDispatcher) DispatchSubmessages(ctx sdk.Context, contractAddr sdk msg_id := []byte(fmt.Sprint(msg.ID)) // now handle the reply, we use the parent context, and abort on error reply := v1wasmTypes.Reply{ - ID: msg_id, - Result: result, + ID: msg_id, + Result: result, + WasMsgEncrypted: msg.WasMsgEncrypted, } // we can ignore any result returned as there is nothing to do with the data diff --git a/x/compute/internal/keeper/test_common.go b/x/compute/internal/keeper/test_common.go index 7c34ba7e8..249e2a37d 100644 --- a/x/compute/internal/keeper/test_common.go +++ b/x/compute/internal/keeper/test_common.go @@ -364,7 +364,7 @@ func CreateTestInput(t *testing.T, isCheckTx bool, supportedFeatures string, enc authtypes.StoreKey, banktypes.StoreKey, stakingtypes.StoreKey, minttypes.StoreKey, distrtypes.StoreKey, slashingtypes.StoreKey, govtypes.StoreKey, paramstypes.StoreKey, ibchost.StoreKey, upgradetypes.StoreKey, - evidencetypes.StoreKey, ibctransfertypes.StoreKey, capabilitytypes.StoreKey, + evidencetypes.StoreKey, ibctransfertypes.StoreKey, capabilitytypes.StoreKey, "compute", feegrant.StoreKey, authzkeeper.StoreKey, icahosttypes.StoreKey, ) diff --git a/x/compute/internal/keeper/testdata/ibc/src/contract.rs b/x/compute/internal/keeper/testdata/ibc/src/contract.rs index c8cd046b8..fa827d2e6 100644 --- a/x/compute/internal/keeper/testdata/ibc/src/contract.rs +++ b/x/compute/internal/keeper/testdata/ibc/src/contract.rs @@ -7,6 +7,8 @@ use cosmwasm_std::{ IbcReceiveResponse, MessageInfo, Reply, ReplyOn, Response, StdError, StdResult, SubMsg, SubMsgResult, WasmMsg, }; +use serde::{Deserialize, Serialize}; +use serde_json_wasm as serde_json; pub const IBC_APP_VERSION: &str = "ibc-v1"; @@ -39,12 +41,28 @@ pub fn query(deps: Deps, _env: Env, _msg: QueryMsg) -> StdResult { } #[entry_point] -pub fn reply(deps: DepsMut, _env: Env, reply: Reply) -> StdResult { +pub fn reply(deps: DepsMut, env: Env, reply: Reply) -> StdResult { match (reply.id, reply.result) { (1, SubMsgResult::Err(_)) => Err(StdError::generic_err("Failed to inc")), (1, SubMsgResult::Ok(_)) => { increment(deps, 6)?; - Ok(Response::new().set_data(to_binary(&"out2".to_string())?)) + Ok(Response::new().set_data(to_binary(&"out".to_string())?)) + } + (2, SubMsgResult::Err(_)) => Err(StdError::generic_err("Failed to inc")), + (2, SubMsgResult::Ok(_)) => { + increment(deps, 6)?; + Ok(Response::new().add_submessage(SubMsg { + id: 1, + msg: CosmosMsg::Wasm(WasmMsg::Execute { + code_hash: env.contract.code_hash, + contract_addr: env.contract.address.into_string(), + msg: Binary::from("{\"increment\":{\"addition\":5}}".as_bytes().to_vec()), + funds: vec![], + }) + .into(), + reply_on: ReplyOn::Always, + gas_limit: None, + })) } _ => Err(StdError::generic_err("invalid reply id or result")), } @@ -115,7 +133,24 @@ pub fn get_resp_based_on_num(env: Env, num: u64) -> StdResult } } -pub fn get_recv_resp_based_on_num(env: Env, num: u64) -> StdResult { +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] +pub struct ContractInfo { + pub address: String, + pub hash: String, +} + +pub fn is_reply_on(num: u64) -> bool { + match num { + 2 | 6 => true, + _ => false, + } +} + +pub fn get_recv_resp_based_on_num( + env: Env, + num: u64, + data: Binary, +) -> StdResult { match num { 0 => Ok(IbcReceiveResponse::default()), 1 => Ok(IbcReceiveResponse::new().add_submessage(SubMsg { @@ -146,6 +181,37 @@ pub fn get_recv_resp_based_on_num(env: Env, num: u64) -> StdResult Ok(IbcReceiveResponse::new() .add_event(Event::new("cyber1".to_string()).add_attribute("attr1", "🤯"))), 5 => Err(StdError::generic_err("Intentional")), + 6 => Ok(IbcReceiveResponse::new().add_submessage(SubMsg { + id: 2, + msg: CosmosMsg::Wasm(WasmMsg::Execute { + code_hash: env.contract.code_hash, + contract_addr: env.contract.address.into_string(), + msg: Binary::from("{\"increment\":{\"addition\":5}}".as_bytes().to_vec()), + funds: vec![], + }) + .into(), + reply_on: ReplyOn::Always, + gas_limit: None, + })), + 7 => { + let contract_info: ContractInfo = serde_json::from_slice(data.as_slice()).unwrap(); + Ok(IbcReceiveResponse::new().add_submessage(SubMsg { + id: 1, + msg: CosmosMsg::Wasm(WasmMsg::Execute { + code_hash: contract_info.hash, + contract_addr: contract_info.address, + msg: Binary::from( + "{\"increment_from_v1\":{\"addition\":5}}" + .as_bytes() + .to_vec(), + ), + funds: vec![], + }) + .into(), + reply_on: ReplyOn::Always, + gas_limit: None, + })) + } _ => Err(StdError::generic_err("Unsupported channel connect type")), } } @@ -207,10 +273,12 @@ pub fn ibc_packet_receive( msg: IbcPacketReceiveMsg, ) -> StdResult { count(deps.storage).save(&(msg.packet.sequence + 7))?; - let mut resp = get_recv_resp_based_on_num(env, msg.packet.sequence); + let mut resp = get_recv_resp_based_on_num(env, msg.packet.sequence, msg.packet.data); match &mut resp { Ok(r) => { - r.acknowledgement = to_binary(&"out".to_string())?; + if !is_reply_on(msg.packet.sequence) { + r.acknowledgement = to_binary(&"out".to_string())?; + } } Err(_) => {} }