Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ed25519 PK validation in Move should return None instead of aborting on wrong length #7043

Merged
merged 31 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
e1934c0
Timed feature flag for ed25519 abort
mstraka100 Mar 9, 2023
272bce7
[natives] expose all feature flags inside NativeContext as an extension
alinush Mar 9, 2023
95bde37
started normal feature ed25519 abort
mstraka100 Mar 10, 2023
dd00ce9
merge with alin/native-feature-flag-context
mstraka100 Mar 10, 2023
824795d
ed25519 abort as normal feature flag start
mstraka100 Mar 10, 2023
1e58a1f
more code for adding ed25519 abort feature flag
mstraka100 Mar 10, 2023
fef208a
[natives] expose all feature flags inside NativeContext as an extension
alinush Mar 9, 2023
34e47bc
actually, undo the NativeContext extension approach
alinush Mar 10, 2023
215663d
pass in an Arc<Features> to each SafeNativeContext
alinush Mar 10, 2023
030f587
expose method to access feature flags in SafeNativeContext
alinush Mar 10, 2023
3b17b86
oops, remove last .clone() calls
alinush Mar 10, 2023
d8b033d
merge with alin/native-feature-flag-context
mstraka100 Mar 10, 2023
1a0984f
use new api from alin/native-feature-flag-context
mstraka100 Mar 10, 2023
a46de7b
add move function to access ed25519 abort feature flag
mstraka100 Mar 10, 2023
87dbb68
merge with alin/native-feature-flag-context
mstraka100 Mar 10, 2023
e8982ec
cargo +nightly fmt
mstraka100 Mar 10, 2023
9684d26
merge with main
mstraka100 Mar 16, 2023
617e8b7
rename flag
mstraka100 Mar 16, 2023
c03147a
cargo +nightly fmt
mstraka100 Mar 16, 2023
04aca27
fix flag name
mstraka100 Mar 16, 2023
7e228c8
remove getter functions for flag
mstraka100 Mar 16, 2023
6ad7817
add ED25519 prefix to flag
mstraka100 Mar 16, 2023
03dfecd
fix names
mstraka100 Mar 16, 2023
380e3da
Merge branch 'main' into michael/native_public_key_validate
mstraka100 Mar 16, 2023
e01811d
added missing match clause
mstraka100 Mar 17, 2023
936d627
merge with main
mstraka100 Mar 17, 2023
b38630c
fix merge conflict
mstraka100 Mar 17, 2023
5884c4e
merge conflict
mstraka100 Mar 17, 2023
7527146
Merge branch 'main' into michael/native_public_key_validate
mstraka100 Mar 17, 2023
fb37768
Merge branch 'main' into michael/native_public_key_validate
mstraka100 Mar 20, 2023
d48c4a5
Merge branch 'main' into michael/native_public_key_validate
mstraka100 Mar 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions aptos-move/aptos-release-builder/data/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ feature_flags:
- resource_groups
- multisig_accounts
- delegation_pools
- ed25519_pk_validate_no_abort_on_wrong_length
alinush marked this conversation as resolved.
Show resolved Hide resolved
disabled: []
is_multi_step: true
framework_release_at_end: true
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub enum FeatureFlag {
ResourceGroups,
MultisigAccounts,
DelegationPools,
Ed25519PkValidateNoAbortOnWrongLength,
}

fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
Expand Down Expand Up @@ -128,6 +129,9 @@ impl From<FeatureFlag> for AptosFeatureFlag {
FeatureFlag::ResourceGroups => AptosFeatureFlag::RESOURCE_GROUPS,
FeatureFlag::MultisigAccounts => AptosFeatureFlag::MULTISIG_ACCOUNTS,
FeatureFlag::DelegationPools => AptosFeatureFlag::DELEGATION_POOLS,
FeatureFlag::Ed25519PkValidateNoAbortOnWrongLength => {
AptosFeatureFlag::ED25519PKVALIDATENOABORTONWRONGLENGTH
mstraka100 marked this conversation as resolved.
Show resolved Hide resolved
},
}
}
}
Expand Down
10 changes: 5 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,7 +18,7 @@ use move_table_extension::NativeTableContext;
use move_vm_runtime::{
config::VMConfig, move_vm::MoveVM, native_extensions::NativeContextExtensions,
};
use std::ops::Deref;
use std::{ops::Deref, sync::Arc};

pub struct MoveVmExt {
inner: MoveVM,
Expand All @@ -44,19 +44,19 @@ impl MoveVmExt {
5
};

let treat_friend_as_private = features.is_enabled(FeatureFlag::TREAT_FRIEND_AS_PRIVATE);

Ok(Self {
inner: MoveVM::new_with_config(
aptos_natives(
native_gas_params,
abs_val_size_gas_params,
gas_feature_version,
timed_features.clone(),
Arc::new(features),
),
VMConfig {
verifier: verifier_config(
features.is_enabled(FeatureFlag::TREAT_FRIEND_AS_PRIVATE),
&timed_features,
),
verifier: verifier_config(treat_friend_as_private, &timed_features),
max_binary_format_version,
paranoid_type_checks: crate::AptosVM::get_paranoid_checks(),
},
Expand Down
11 changes: 9 additions & 2 deletions aptos-move/aptos-vm/src/natives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
use aptos_gas::{AbstractValueSizeGasParameters, NativeGasParameters, LATEST_GAS_FEATURE_VERSION};
#[cfg(feature = "testing")]
use aptos_types::chain_id::ChainId;
use aptos_types::{account_config::CORE_CODE_ADDRESS, on_chain_config::TimedFeatures};
use aptos_types::{
account_config::CORE_CODE_ADDRESS,
on_chain_config::{Features, TimedFeatures},
};
use move_vm_runtime::native_functions::NativeFunctionTable;
use std::sync::Arc;
#[cfg(feature = "testing")]
use {
aptos_framework::natives::{
Expand All @@ -27,6 +31,7 @@ pub fn aptos_natives(
abs_val_size_gas_params: AbstractValueSizeGasParameters,
gas_feature_version: u64,
timed_features: TimedFeatures,
features: Arc<Features>,
) -> NativeFunctionTable {
move_stdlib::natives::all_natives(CORE_CODE_ADDRESS, gas_params.move_stdlib)
.into_iter()
Expand All @@ -35,6 +40,7 @@ pub fn aptos_natives(
CORE_CODE_ADDRESS,
gas_params.aptos_framework,
timed_features,
features,
move |val| abs_val_size_gas_params.abstract_value_size(val, gas_feature_version),
))
.chain(move_table_extension::table_natives(
Expand All @@ -61,7 +67,8 @@ pub fn assert_no_test_natives(err_msg: &str) {
NativeGasParameters::zeros(),
AbstractValueSizeGasParameters::zeros(),
LATEST_GAS_FEATURE_VERSION,
TimedFeatures::enable_all()
TimedFeatures::enable_all(),
Arc::new(Features::default())
)
.into_iter()
.all(|(_, module_name, func_name, _)| {
Expand Down
59 changes: 59 additions & 0 deletions aptos-move/framework/move-stdlib/doc/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ the Move stdlib, the Aptos stdlib, and the Aptos framework.
- [Function `multisig_accounts_enabled`](#0x1_features_multisig_accounts_enabled)
- [Function `get_delegation_pools_feature`](#0x1_features_get_delegation_pools_feature)
- [Function `delegation_pools_enabled`](#0x1_features_delegation_pools_enabled)
- [Function `get_pubkey_validate_aborts_wrong_length`](#0x1_features_get_pubkey_validate_aborts_wrong_length)
- [Function `pubkey_validate_aborts_wrong_length_enabled`](#0x1_features_pubkey_validate_aborts_wrong_length_enabled)
- [Function `change_feature_flags`](#0x1_features_change_feature_flags)
- [Function `is_enabled`](#0x1_features_is_enabled)
- [Function `set`](#0x1_features_set)
Expand Down Expand Up @@ -138,6 +140,17 @@ Lifetime: transient



<a name="0x1_features_ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH"></a>

Whether native_public_key_validate aborts when a public key of the wrong length is given
Lifetime: ephemeral


<pre><code><b>const</b> <a href="features.md#0x1_features_ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH">ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH</a>: u64 = 12;
</code></pre>



<a name="0x1_features_EFRAMEWORK_SIGNER_NEEDED"></a>

The provided signer has not a framework address.
Expand Down Expand Up @@ -676,6 +689,52 @@ Lifetime: transient



</details>

<a name="0x1_features_get_pubkey_validate_aborts_wrong_length"></a>

## Function `get_pubkey_validate_aborts_wrong_length`



<pre><code><b>public</b> <b>fun</b> <a href="features.md#0x1_features_get_pubkey_validate_aborts_wrong_length">get_pubkey_validate_aborts_wrong_length</a>(): u64
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>public</b> <b>fun</b> <a href="features.md#0x1_features_get_pubkey_validate_aborts_wrong_length">get_pubkey_validate_aborts_wrong_length</a>(): u64 { <a href="features.md#0x1_features_ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH">ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH</a> }
</code></pre>



</details>

<a name="0x1_features_pubkey_validate_aborts_wrong_length_enabled"></a>

## Function `pubkey_validate_aborts_wrong_length_enabled`



<pre><code><b>public</b> <b>fun</b> <a href="features.md#0x1_features_pubkey_validate_aborts_wrong_length_enabled">pubkey_validate_aborts_wrong_length_enabled</a>(): bool
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>public</b> <b>fun</b> <a href="features.md#0x1_features_pubkey_validate_aborts_wrong_length_enabled">pubkey_validate_aborts_wrong_length_enabled</a>(): bool <b>acquires</b> <a href="features.md#0x1_features_Features">Features</a> {
<a href="features.md#0x1_features_is_enabled">is_enabled</a>(<a href="features.md#0x1_features_ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH">ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH</a>)
}
</code></pre>



</details>

<a name="0x1_features_change_feature_flags"></a>
Expand Down
10 changes: 10 additions & 0 deletions aptos-move/framework/move-stdlib/sources/configs/features.move
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,16 @@ module std::features {
is_enabled(DELEGATION_POOLS)
}

/// Whether native_public_key_validate aborts when a public key of the wrong length is given
/// Lifetime: ephemeral
const ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH: u64 = 12;

public fun get_pubkey_validate_aborts_wrong_length(): u64 { ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH }

public fun pubkey_validate_aborts_wrong_length_enabled(): bool acquires Features {
is_enabled(ED25519_PK_VALIDATE_NO_ABORT_ON_WRONG_LENGTH)
}

// ============================================================================================
// Feature Flag Implementation

Expand Down
7 changes: 5 additions & 2 deletions aptos-move/framework/src/natives/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ use crate::{
},
safely_pop_arg,
};
use aptos_types::on_chain_config::TimedFeatures;
use aptos_types::on_chain_config::{Features, TimedFeatures};
use move_core_types::{account_address::AccountAddress, gas_algebra::InternalGas};
use move_vm_runtime::native_functions::NativeFunction;
use move_vm_types::{loaded_data::runtime_types::Type, values::Value};
use smallvec::{smallvec, SmallVec};
use std::collections::VecDeque;
use std::{collections::VecDeque, sync::Arc};

/***************************************************************************************************
* native fun create_address
Expand Down Expand Up @@ -62,13 +62,15 @@ pub struct GasParameters {
pub fn make_all(
gas_params: GasParameters,
timed_features: TimedFeatures,
features: Arc<Features>,
) -> impl Iterator<Item = (String, NativeFunction)> {
let natives = [
(
"create_address",
make_safe_native(
gas_params.create_address,
timed_features.clone(),
features.clone(),
native_create_address,
),
),
Expand All @@ -79,6 +81,7 @@ pub fn make_all(
make_safe_native(
gas_params.create_signer,
timed_features,
features,
create_signer::native_create_signer,
),
),
Expand Down
28 changes: 22 additions & 6 deletions aptos-move/framework/src/natives/aggregator_natives/aggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ use crate::{
safely_pop_arg,
};
use aptos_aggregator::aggregator_extension::AggregatorID;
use aptos_types::on_chain_config::TimedFeatures;
use aptos_types::on_chain_config::{Features, TimedFeatures};
use move_core_types::gas_algebra::InternalGas;
use move_vm_runtime::native_functions::NativeFunction;
use move_vm_types::{
loaded_data::runtime_types::Type,
values::{Struct, StructRef, Value},
};
use smallvec::{smallvec, SmallVec};
use std::collections::VecDeque;
use std::{collections::VecDeque, sync::Arc};

/***************************************************************************************************
* native fun add(aggregator: &mut Aggregator, value: u128);
Expand Down Expand Up @@ -177,23 +177,39 @@ pub struct GasParameters {
pub fn make_all(
gas_params: GasParameters,
timed_features: TimedFeatures,
features: Arc<Features>,
) -> impl Iterator<Item = (String, NativeFunction)> {
let natives = [
(
"add",
make_safe_native(gas_params.add, timed_features.clone(), native_add),
make_safe_native(
gas_params.add,
timed_features.clone(),
features.clone(),
native_add,
),
),
(
"read",
make_safe_native(gas_params.read, timed_features.clone(), native_read),
make_safe_native(
gas_params.read,
timed_features.clone(),
features.clone(),
native_read,
),
),
(
"sub",
make_safe_native(gas_params.sub, timed_features.clone(), native_sub),
make_safe_native(
gas_params.sub,
timed_features.clone(),
features.clone(),
native_sub,
),
),
(
"destroy",
make_safe_native(gas_params.destroy, timed_features, native_destroy),
make_safe_native(gas_params.destroy, timed_features, features, native_destroy),
),
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,18 @@ use crate::{
};
use aptos_aggregator::aggregator_extension::{extension_error, AggregatorHandle, AggregatorID};
use aptos_crypto::hash::DefaultHasher;
use aptos_types::{account_address::AccountAddress, on_chain_config::TimedFeatures};
use aptos_types::{
account_address::AccountAddress,
on_chain_config::{Features, TimedFeatures},
};
use move_core_types::gas_algebra::InternalGas;
use move_vm_runtime::native_functions::NativeFunction;
use move_vm_types::{
loaded_data::runtime_types::Type,
values::{Struct, StructRef, Value},
};
use smallvec::{smallvec, SmallVec};
use std::collections::VecDeque;
use std::{collections::VecDeque, sync::Arc};

/***************************************************************************************************
* native fun new_aggregator(aggregator_factory: &mut AggregatorFactory, limit: u128): Aggregator;
Expand Down Expand Up @@ -86,12 +89,14 @@ pub struct GasParameters {
pub fn make_all(
gas_params: GasParameters,
timed_features: TimedFeatures,
features: Arc<Features>,
) -> impl Iterator<Item = (String, NativeFunction)> {
let natives = [(
"new_aggregator",
make_safe_native(
gas_params.new_aggregator,
timed_features,
features,
native_new_aggregator,
),
)];
Expand Down
8 changes: 7 additions & 1 deletion aptos-move/framework/src/natives/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use crate::{
};
use anyhow::bail;
use aptos_types::{
on_chain_config::TimedFeatures, transaction::ModuleBundle, vm_status::StatusCode,
on_chain_config::{Features, TimedFeatures},
transaction::ModuleBundle,
vm_status::StatusCode,
};
use better_any::{Tid, TidAble};
use move_binary_format::errors::{PartialVMError, PartialVMResult};
Expand All @@ -29,6 +31,7 @@ use std::{
collections::{btree_map::Entry, BTreeMap, BTreeSet, VecDeque},
fmt,
str::FromStr,
sync::Arc,
};

/// A wrapper around the representation of a Move Option, which is a vector with 0 or 1 element.
Expand Down Expand Up @@ -310,13 +313,15 @@ pub struct GasParameters {
pub fn make_all(
gas_params: GasParameters,
timed_features: TimedFeatures,
features: Arc<Features>,
) -> impl Iterator<Item = (String, NativeFunction)> {
let natives = [
(
"request_publish",
make_safe_native(
gas_params.request_publish.clone(),
timed_features.clone(),
features.clone(),
native_request_publish,
),
),
Expand All @@ -325,6 +330,7 @@ pub fn make_all(
make_safe_native(
gas_params.request_publish,
timed_features,
features,
native_request_publish,
),
),
Expand Down
Loading