Skip to content

Commit

Permalink
[compiler v2] Resource access control (read-write sets) (#10480)
Browse files Browse the repository at this point in the history
* [compiler v2] Resource access control (read-write sets)

This is an e2e implementation of resource access control for Move, with most parts in place:

- Replaces the acquires syntax in a downwards-compatible way
- The extended syntax is only available in compiler v2
- One can now specify `acquires`, `reads`, and `writes`
- One can specify the address of a resource in dependency of parameters
- Multiple levels of wildcards are allowed, e.g. `acquires *(object::address_of(param))` specifies that all resources at the given address are read or written.
- Implements parsing->expansion->move model->file format generator
- Extends `file_format::FunctionHandle` to carry the new information, introducing bytecode version v7. v7 became the new experimental version only available in test code for now.
- TODO: dynamic runtime checking of resource access. Static analysis is also on the horizon, but not needed for an MVP of this feature.
- TODO: bytecode verification of access specifiers

An AIP for this new feature will be filed soon.

As an example, here is some extract from the tests:

```move
module 0x42::m {

    struct S has store {}
    struct R has store {}
    struct T has store {}
    struct G<T> has store {}

    fun f1() acquires S {
    }

    fun f2() reads S {
    }

    fun f3() writes S {
    }

    fun f4() acquires S(*) {
    }

    fun f_multiple() acquires R reads R writes T, S reads G<u64> {
    }

    fun f5() acquires 0x42::*::* {
    }

    fun f6() acquires 0x42::m::R {
    }

    fun f7() acquires *(*) {
    }

    fun f8() acquires *(0x42) {
    }

    fun f9(a: address) acquires *(a) {
    }

    fun f10(x: u64) acquires *(make_up_address(x)) {
    }

    fun make_up_address(x: u64): address {
        @0x42
    }
}
```

* Addressing reviewer comments

* Addressing reviewer comments #2

* Addressing reviewer comments #3

* Addressing reviewer comments #4

* Reviewer comments #5
  • Loading branch information
wrwg authored Oct 29, 2023
1 parent b55ad61 commit 58271fd
Show file tree
Hide file tree
Showing 80 changed files with 1,915 additions and 180 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ pub enum FeatureFlag {
ConcurrentAssets,
LimitMaxIdentifierLength,
OperatorBeneficiaryChange,
VMBinaryFormatV7,
}

fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
Expand Down Expand Up @@ -185,6 +186,7 @@ impl From<FeatureFlag> for AptosFeatureFlag {
},
FeatureFlag::AptosStdChainIdNatives => AptosFeatureFlag::APTOS_STD_CHAIN_ID_NATIVES,
FeatureFlag::VMBinaryFormatV6 => AptosFeatureFlag::VM_BINARY_FORMAT_V6,
FeatureFlag::VMBinaryFormatV7 => AptosFeatureFlag::VM_BINARY_FORMAT_V7,
FeatureFlag::MultiEd25519PkValidateV2Natives => {
AptosFeatureFlag::MULTI_ED25519_PK_VALIDATE_V2_NATIVES
},
Expand Down Expand Up @@ -252,6 +254,7 @@ impl From<AptosFeatureFlag> for FeatureFlag {
},
AptosFeatureFlag::APTOS_STD_CHAIN_ID_NATIVES => FeatureFlag::AptosStdChainIdNatives,
AptosFeatureFlag::VM_BINARY_FORMAT_V6 => FeatureFlag::VMBinaryFormatV6,
AptosFeatureFlag::VM_BINARY_FORMAT_V7 => FeatureFlag::VMBinaryFormatV7,
AptosFeatureFlag::MULTI_ED25519_PK_VALIDATE_V2_NATIVES => {
FeatureFlag::MultiEd25519PkValidateV2Natives
},
Expand Down
14 changes: 4 additions & 10 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use crate::{
counters::*,
data_cache::{AsMoveResolver, StorageAdapter},
errors::expect_only_successful_execution,
move_vm_ext::{AptosMoveResolver, RespawnedSession, SessionExt, SessionId},
move_vm_ext::{
get_max_binary_format_version, AptosMoveResolver, RespawnedSession, SessionExt, SessionId,
},
sharded_block_executor::{executor_client::ExecutorClient, ShardedBlockExecutor},
system_module_names::*,
transaction_metadata::TransactionMetadata,
Expand Down Expand Up @@ -855,15 +857,7 @@ impl AptosVM {

/// Deserialize a module bundle.
fn deserialize_module_bundle(&self, modules: &ModuleBundle) -> VMResult<Vec<CompiledModule>> {
let max_version = if self
.0
.get_features()
.is_enabled(FeatureFlag::VM_BINARY_FORMAT_V6)
{
6
} else {
5
};
let max_version = get_max_binary_format_version(self.0.get_features(), None);
let max_identifier_size = if self
.0
.get_features()
Expand Down
5 changes: 3 additions & 2 deletions aptos-move/aptos-vm/src/data_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl<'e, E: ExecutorView> StorageAdapter<'e, E> {
features: &Features,
maybe_resource_group_view: Option<&'e dyn ResourceGroupView>,
) -> Self {
let max_binary_version = get_max_binary_format_version(features, gas_feature_version);
let max_binary_version = get_max_binary_format_version(features, Some(gas_feature_version));
let max_identifier_size = get_max_identifier_size(features);
let resource_group_adapter = ResourceGroupAdapter::new(
maybe_resource_group_view,
Expand Down Expand Up @@ -332,7 +332,8 @@ impl<S: StateView> AsMoveResolver<S> for S {
let config_view = ConfigAdapter(self);
let (_, gas_feature_version) = gas_config(&config_view);
let features = Features::fetch_config(&config_view).unwrap_or_default();
let max_binary_version = get_max_binary_format_version(&features, gas_feature_version);
let max_binary_version =
get_max_binary_format_version(&features, Some(gas_feature_version));
let resource_group_adapter = ResourceGroupAdapter::new(None, self, gas_feature_version);
let max_identifier_size = get_max_identifier_size(&features);
StorageAdapter::new(
Expand Down
21 changes: 16 additions & 5 deletions aptos-move/aptos-vm/src/move_vm_ext/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use aptos_types::on_chain_config::{FeatureFlag, Features, TimedFeatureFlag, Time
use move_binary_format::{
deserializer::DeserializerConfig,
errors::VMResult,
file_format_common,
file_format_common::{IDENTIFIER_SIZE_MAX, LEGACY_IDENTIFIER_SIZE_MAX},
};
use move_bytecode_verifier::VerifierConfig;
Expand All @@ -32,11 +33,21 @@ pub struct MoveVmExt {
features: Arc<Features>,
}

pub fn get_max_binary_format_version(features: &Features, gas_feature_version: u64) -> u32 {
if features.is_enabled(FeatureFlag::VM_BINARY_FORMAT_V6) && gas_feature_version >= 5 {
6
pub fn get_max_binary_format_version(
features: &Features,
gas_feature_version_opt: Option<u64>,
) -> u32 {
// For historical reasons, we support still < gas version 5, but if a new caller don't specify
// the gas version, we default to 5, which was introduced in late '22.
let gas_feature_version = gas_feature_version_opt.unwrap_or(5);
if gas_feature_version < 5 {
file_format_common::VERSION_5
} else if features.is_enabled(FeatureFlag::VM_BINARY_FORMAT_V7) {
file_format_common::VERSION_7
} else if features.is_enabled(FeatureFlag::VM_BINARY_FORMAT_V6) {
file_format_common::VERSION_6
} else {
5
file_format_common::VERSION_5
}
}

Expand Down Expand Up @@ -66,7 +77,7 @@ impl MoveVmExt {
// Therefore it depends on a new version of the gas schedule and cannot be allowed if
// the gas schedule hasn't been updated yet.
let max_binary_format_version =
get_max_binary_format_version(&features, gas_feature_version);
get_max_binary_format_version(&features, Some(gas_feature_version));

let max_identifier_size = get_max_identifier_size(&features);

Expand Down
1 change: 1 addition & 0 deletions aptos-move/e2e-move-tests/src/tests/access_path_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ fn access_path_panic() {
parameters: SignatureIndex(0),
return_: SignatureIndex(0),
type_parameters: vec![],
access_specifiers: None,
}],
field_handles: vec![],
friend_decls: vec![],
Expand Down
4 changes: 4 additions & 0 deletions aptos-move/e2e-testsuite/src/tests/scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ fn script_none_existing_module_dep() {
parameters: SignatureIndex(0),
return_: SignatureIndex(0),
type_parameters: vec![],
access_specifiers: None,
};
script.function_handles.push(fun_handle);

Expand Down Expand Up @@ -177,6 +178,7 @@ fn script_non_existing_function_dep() {
parameters: SignatureIndex(0),
return_: SignatureIndex(0),
type_parameters: vec![],
access_specifiers: None,
};
script.function_handles.push(fun_handle);

Expand Down Expand Up @@ -257,6 +259,7 @@ fn script_bad_sig_function_dep() {
parameters: SignatureIndex(0),
return_: SignatureIndex(0),
type_parameters: vec![],
access_specifiers: None,
};
script.function_handles.push(fun_handle);

Expand Down Expand Up @@ -465,6 +468,7 @@ fn forbid_script_emitting_events() {
type_parameters: vec![
AbilitySet::singleton(Ability::Store) | AbilitySet::singleton(Ability::Drop),
],
access_specifiers: None,
});
script.module_handles.push(ModuleHandle {
address: AddressIdentifierIndex(0),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ fuzz_target!(|code_unit: CodeUnit| {
parameters: SignatureIndex(0),
return_: SignatureIndex(1),
type_parameters: vec![],
access_specifiers: None,
};

module.function_handles.push(fun_handle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ fuzz_target!(|mix: Mixed| {
parameters: SignatureIndex(0),
return_: SignatureIndex(1),
type_parameters: vec![],
access_specifiers: None,
};

module.function_handles.push(fun_handle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

use move_binary_format::{
access::ModuleAccess,
deserializer::DeserializerConfig,
file_format::{
empty_module, AbilitySet, CompiledModule, FunctionHandle, IdentifierIndex, Signature,
SignatureIndex, SignatureToken,
},
file_format_common::{IDENTIFIER_SIZE_MAX, VERSION_MAX},
};
use move_core_types::identifier::Identifier;
use proptest::prelude::*;
Expand All @@ -16,10 +18,13 @@ proptest! {
#[test]
fn serializer_roundtrip(module in CompiledModule::valid_strategy(20)) {
let mut serialized = Vec::with_capacity(2048);
module.serialize(&mut serialized).expect("serialization should work");
module.serialize_for_version(Some(VERSION_MAX), &mut serialized).expect("serialization should work");

let deserialized_module = CompiledModule::deserialize(&serialized)
.expect("deserialization should work");

let deserialized_module = CompiledModule::deserialize_with_config(
&serialized,
&DeserializerConfig::new(VERSION_MAX, IDENTIFIER_SIZE_MAX)
).expect("deserialization should work");

prop_assert_eq!(module, deserialized_module);
}
Expand All @@ -34,7 +39,7 @@ proptest! {
#[test]
fn garbage_inputs(module in any_with::<CompiledModule>(16)) {
let mut serialized = Vec::with_capacity(65536);
module.serialize(&mut serialized).expect("serialization should work");
module.serialize_for_version(Some(VERSION_MAX), &mut serialized).expect("serialization should work");

let deserialized_module = CompiledModule::deserialize_no_check_bounds(&serialized)
.expect("deserialization should work");
Expand Down Expand Up @@ -66,14 +71,18 @@ fn simple_generic_module_round_trip() {
parameters: sig_t1_idx,
return_: sig_unit_idx,
type_parameters: vec![AbilitySet::EMPTY],
access_specifiers: None,
});

let mut serialized = Vec::with_capacity(2048);
m.serialize(&mut serialized)
m.serialize_for_version(Some(VERSION_MAX), &mut serialized)
.expect("serialization should work");

let deserialized_m =
CompiledModule::deserialize(&serialized).expect("deserialization should work");
let deserialized_m = CompiledModule::deserialize_with_config(
&serialized,
&DeserializerConfig::new(VERSION_MAX, IDENTIFIER_SIZE_MAX),
)
.expect("deserialization should work");

assert_eq!(m, deserialized_m);
}
10 changes: 10 additions & 0 deletions third_party/move/move-binary-format/src/binary_views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,16 @@ impl<'a> BinaryIndexedView<'a> {
)
}

pub fn safe_module_id_for_handle(&self, module_handle: &ModuleHandle) -> Option<ModuleId> {
self.address_identifiers()
.get(module_handle.address.0 as usize)
.and_then(|a| {
self.identifiers()
.get(module_handle.name.0 as usize)
.map(|id| ModuleId::new(*a, id.to_owned()))
})
}

pub fn self_id(&self) -> Option<ModuleId> {
match self {
BinaryIndexedView::Module(m) => Some(m.self_id()),
Expand Down
Loading

0 comments on commit 58271fd

Please sign in to comment.