diff --git a/actors/account/src/lib.rs b/actors/account/src/lib.rs index 25a630dcc..2f27a5208 100644 --- a/actors/account/src/lib.rs +++ b/actors/account/src/lib.rs @@ -12,7 +12,7 @@ use num_traits::FromPrimitive; use fil_actors_runtime::builtin::singletons::SYSTEM_ACTOR_ADDR; use fil_actors_runtime::runtime::{ActorCode, Runtime}; -use fil_actors_runtime::{actor_error, ActorError}; +use fil_actors_runtime::{actor_error, restrict_internal_api, ActorError}; use fil_actors_runtime::{cbor, ActorDowncast}; use crate::types::AuthenticateMessageParams; @@ -33,6 +33,7 @@ pub enum Method { Constructor = METHOD_CONSTRUCTOR, PubkeyAddress = 2, AuthenticateMessage = 3, + AuthenticateMessageExported = frc42_dispatch::method_hash!("AuthenticateMessage"), UniversalReceiverHook = frc42_dispatch::method_hash!("Receive"), } @@ -109,6 +110,8 @@ impl ActorCode for Actor { where RT: Runtime, { + restrict_internal_api(rt, method)?; + match FromPrimitive::from_u64(method) { Some(Method::Constructor) => { Self::constructor(rt, cbor::deserialize_params(params)?)?; @@ -118,7 +121,7 @@ impl ActorCode for Actor { let addr = Self::pubkey_address(rt)?; Ok(RawBytes::serialize(addr)?) } - Some(Method::AuthenticateMessage) => { + Some(Method::AuthenticateMessage) | Some(Method::AuthenticateMessageExported) => { Self::authenticate_message(rt, cbor::deserialize_params(params)?)?; Ok(RawBytes::default()) } diff --git a/actors/account/tests/account_actor_test.rs b/actors/account/tests/account_actor_test.rs index a437ca7c5..4f9bdf3a0 100644 --- a/actors/account/tests/account_actor_test.rs +++ b/actors/account/tests/account_actor_test.rs @@ -16,12 +16,8 @@ use fil_actors_runtime::test_utils::*; #[test] fn construction() { fn construct(addr: Address, exit_code: ExitCode) { - let mut rt = MockRuntime { - receiver: Address::new_id(100), - caller: SYSTEM_ACTOR_ADDR, - caller_type: *SYSTEM_ACTOR_CODE_ID, - ..Default::default() - }; + let mut rt = MockRuntime { receiver: Address::new_id(100), ..Default::default() }; + rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR); rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]); if exit_code.is_success() { @@ -59,12 +55,8 @@ fn construction() { #[test] fn token_receiver() { - let mut rt = MockRuntime { - receiver: Address::new_id(100), - caller: SYSTEM_ACTOR_ADDR, - caller_type: *SYSTEM_ACTOR_CODE_ID, - ..Default::default() - }; + let mut rt = MockRuntime { receiver: Address::new_id(100), ..Default::default() }; + rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR); rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]); let param = Address::new_secp256k1(&[2; fvm_shared::address::SECP_PUB_LEN]).unwrap(); @@ -74,6 +66,7 @@ fn token_receiver() { ) .unwrap(); + rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000)); rt.expect_validate_caller_any(); let ret = rt.call::( Method::UniversalReceiverHook as MethodNum, @@ -83,25 +76,15 @@ fn token_receiver() { assert_eq!(RawBytes::default(), ret.unwrap()); } -fn check_state(rt: &MockRuntime) { - let test_address = Address::new_id(1000); - let (_, acc) = check_state_invariants(&rt.get_state(), &test_address); - acc.assert_empty(); -} - #[test] fn authenticate_message() { - let mut rt = MockRuntime { - receiver: Address::new_id(100), - caller: SYSTEM_ACTOR_ADDR, - caller_type: *SYSTEM_ACTOR_CODE_ID, - ..Default::default() - }; + let mut rt = MockRuntime { receiver: Address::new_id(100), ..Default::default() }; + rt.set_caller(*SYSTEM_ACTOR_CODE_ID, SYSTEM_ACTOR_ADDR); let addr = Address::new_secp256k1(&[2; fvm_shared::address::SECP_PUB_LEN]).unwrap(); rt.expect_validate_caller_addr(vec![SYSTEM_ACTOR_ADDR]); - - rt.call::(1, &RawBytes::serialize(addr).unwrap()).unwrap(); + rt.call::(Method::Constructor as MethodNum, &RawBytes::serialize(addr).unwrap()) + .unwrap(); let state: State = rt.get_state(); assert_eq!(state.address, addr); @@ -110,6 +93,7 @@ fn authenticate_message() { RawBytes::serialize(AuthenticateMessageParams { signature: vec![], message: vec![] }) .unwrap(); + // Valid signature rt.expect_validate_caller_any(); rt.expect_verify_signature(ExpectedVerifySig { sig: Signature::new_secp256k1(vec![]), @@ -117,8 +101,13 @@ fn authenticate_message() { plaintext: vec![], result: Ok(()), }); - assert_eq!(RawBytes::default(), rt.call::(3, ¶ms).unwrap()); + assert_eq!( + RawBytes::default(), + rt.call::(Method::AuthenticateMessage as MethodNum, ¶ms).unwrap() + ); + rt.verify(); + // Invalid signature rt.expect_validate_caller_any(); rt.expect_verify_signature(ExpectedVerifySig { sig: Signature::new_secp256k1(vec![]), @@ -126,10 +115,34 @@ fn authenticate_message() { plaintext: vec![], result: Err(anyhow!("bad signature")), }); - assert_eq!( + expect_abort_contains_message( ExitCode::USR_ILLEGAL_ARGUMENT, - rt.call::(3, ¶ms).unwrap_err().exit_code() + "bad signature", + rt.call::(Method::AuthenticateMessage as MethodNum, ¶ms), ); - rt.verify(); + + // Invalid caller of internal method number + rt.set_caller(make_identity_cid(b"1234"), Address::new_id(1000)); + expect_abort_contains_message( + ExitCode::USR_FORBIDDEN, + "must be built-in", + rt.call::(Method::AuthenticateMessage as MethodNum, ¶ms), + ); + + // Ok to call exported method number + rt.expect_validate_caller_any(); + rt.expect_verify_signature(ExpectedVerifySig { + sig: Signature::new_secp256k1(vec![]), + signer: addr, + plaintext: vec![], + result: Ok(()), + }); + rt.call::(Method::AuthenticateMessageExported as MethodNum, ¶ms).unwrap(); +} + +fn check_state(rt: &MockRuntime) { + let test_address = Address::new_id(1000); + let (_, acc) = check_state_invariants(&rt.get_state(), &test_address); + acc.assert_empty(); } diff --git a/runtime/src/builtin/shared.rs b/runtime/src/builtin/shared.rs index 5abc54a2c..aea1180c0 100644 --- a/runtime/src/builtin/shared.rs +++ b/runtime/src/builtin/shared.rs @@ -3,8 +3,8 @@ use crate::{actor_error, ActorContext, ActorError}; use fvm_shared::address::Address; -use fvm_shared::ActorID; use fvm_shared::METHOD_SEND; +use fvm_shared::{ActorID, MethodNum}; use crate::runtime::builtins::Type; use crate::runtime::Runtime; @@ -38,3 +38,37 @@ pub fn resolve_to_actor_id( Err(actor_error!(illegal_argument, "failed to resolve or initialize address {}", address)) } + +// The lowest FRC-42 method number. +pub const FIRST_EXPORTED_METHOD_NUMBER: MethodNum = 1 << 24; + +// Checks whether the caller is allowed to invoke some method number. +// All method numbers below the FRC-42 range are restricted to built-in actors +// (including the account and multisig actors). +// Methods may subsequently enforce tighter restrictions. +pub fn restrict_internal_api(rt: &mut RT, method: MethodNum) -> Result<(), ActorError> +where + RT: Runtime, +{ + if method >= FIRST_EXPORTED_METHOD_NUMBER { + return Ok(()); + } + let caller = rt.message().caller(); + let code_cid = rt.get_actor_code_cid(&caller.id().unwrap()); + match code_cid { + None => { + return Err( + actor_error!(forbidden; "no code for caller {} of method {}", caller, method), + ) + } + Some(code_cid) => { + let builtin_type = rt.resolve_builtin_actor_type(&code_cid); + if builtin_type.is_none() { + return Err( + actor_error!(forbidden; "caller {} of method {} must be built-in", caller, method), + ); + } + } + } + Ok(()) +} diff --git a/runtime/src/test_utils.rs b/runtime/src/test_utils.rs index 033b30681..60370daea 100644 --- a/runtime/src/test_utils.rs +++ b/runtime/src/test_utils.rs @@ -42,18 +42,18 @@ use crate::runtime::{ use crate::{actor_error, ActorError}; lazy_static::lazy_static! { - pub static ref SYSTEM_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/system"); - pub static ref INIT_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/init"); - pub static ref CRON_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/cron"); - pub static ref ACCOUNT_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/account"); - pub static ref POWER_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/storagepower"); - pub static ref MINER_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/storageminer"); - pub static ref MARKET_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/storagemarket"); - pub static ref PAYCH_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/paymentchannel"); - pub static ref MULTISIG_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/multisig"); - pub static ref REWARD_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/reward"); - pub static ref VERIFREG_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/verifiedregistry"); - pub static ref DATACAP_TOKEN_ACTOR_CODE_ID: Cid = make_builtin(b"fil/test/datacap"); + pub static ref SYSTEM_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/system"); + pub static ref INIT_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/init"); + pub static ref CRON_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/cron"); + pub static ref ACCOUNT_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/account"); + pub static ref POWER_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/storagepower"); + pub static ref MINER_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/storageminer"); + pub static ref MARKET_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/storagemarket"); + pub static ref PAYCH_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/paymentchannel"); + pub static ref MULTISIG_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/multisig"); + pub static ref REWARD_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/reward"); + pub static ref VERIFREG_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/verifiedregistry"); + pub static ref DATACAP_TOKEN_ACTOR_CODE_ID: Cid = make_identity_cid(b"fil/test/datacap"); pub static ref ACTOR_TYPES: BTreeMap = { let mut map = BTreeMap::new(); map.insert(*SYSTEM_ACTOR_CODE_ID, Type::System); @@ -99,7 +99,7 @@ lazy_static::lazy_static! { const IPLD_RAW: u64 = 0x55; /// Returns an identity CID for bz. -pub fn make_builtin(bz: &[u8]) -> Cid { +pub fn make_identity_cid(bz: &[u8]) -> Cid { Cid::new_v1(IPLD_RAW, OtherMultihash::wrap(0, bz).expect("name too long")) } diff --git a/test_vm/tests/test_vm_test.rs b/test_vm/tests/test_vm_test.rs index 1e17fe49c..760f4340b 100644 --- a/test_vm/tests/test_vm_test.rs +++ b/test_vm/tests/test_vm_test.rs @@ -1,5 +1,7 @@ use fil_actor_account::State as AccountState; -use fil_actors_runtime::test_utils::{make_builtin, ACCOUNT_ACTOR_CODE_ID, PAYCH_ACTOR_CODE_ID}; +use fil_actors_runtime::test_utils::{ + make_identity_cid, ACCOUNT_ACTOR_CODE_ID, PAYCH_ACTOR_CODE_ID, +}; use fvm_ipld_blockstore::MemoryBlockstore; use fvm_ipld_encoding::RawBytes; use fvm_shared::address::Address; @@ -18,14 +20,19 @@ fn state_control() { let addr2 = Address::new_id(2222); // set actor - let a1 = - actor(*ACCOUNT_ACTOR_CODE_ID, make_builtin(b"a1-head"), 42, TokenAmount::from_atto(10u8)); + let a1 = actor( + *ACCOUNT_ACTOR_CODE_ID, + make_identity_cid(b"a1-head"), + 42, + TokenAmount::from_atto(10u8), + ); v.set_actor(addr1, a1.clone()); let out = v.get_actor(addr1).unwrap(); assert_eq!(out, a1); let check = v.checkpoint(); - let a2 = actor(*PAYCH_ACTOR_CODE_ID, make_builtin(b"a2-head"), 88, TokenAmount::from_atto(1u8)); + let a2 = + actor(*PAYCH_ACTOR_CODE_ID, make_identity_cid(b"a2-head"), 88, TokenAmount::from_atto(1u8)); v.set_actor(addr2, a2.clone()); assert_eq!(v.get_actor(addr2).unwrap(), a2); // rollback removes a2 but not a1