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.

Sign up for GitHub

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

Add permission checks to account #15741

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Add permission checks to account #15741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
187 changes: 185 additions & 2 deletions aptos-move/framework/aptos-framework/doc/account.md
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion aptos-move/framework/aptos-framework/doc/multisig_account.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4166,7 +4166,8 @@ Add new owners, remove owners to remove, update signatures required.



<pre><code><b>aborts_if</b> !<b>exists</b>&lt;<a href="account.md#0x1_account_Account">account::Account</a>&gt;(creator);
<pre><code><b>pragma</b> aborts_if_is_partial;
<b>aborts_if</b> !<b>exists</b>&lt;<a href="account.md#0x1_account_Account">account::Account</a>&gt;(creator);
<b>let</b> owner_nonce = <b>global</b>&lt;<a href="account.md#0x1_account_Account">account::Account</a>&gt;(creator).sequence_number;
</code></pre>

Expand Down
8 changes: 5 additions & 3 deletions aptos-move/framework/aptos-framework/doc/resource_account.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ the SignerCapability.


<pre><code><b>pragma</b> verify = <b>true</b>;
<b>pragma</b> aborts_if_is_strict;
<b>pragma</b> aborts_if_is_partial;
</code></pre>


Expand Down Expand Up @@ -547,7 +547,8 @@ the SignerCapability.



<pre><code><b>let</b> resource_addr = <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(resource);
<pre><code><b>pragma</b> aborts_if_is_partial;
<b>let</b> resource_addr = <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(resource);
// This enforces <a id="high-level-req-1" href="#high-level-req">high-level requirement 1</a>:
<b>include</b> <a href="resource_account.md#0x1_resource_account_RotateAccountAuthenticationKeyAndStoreCapabilityAbortsIf">RotateAccountAuthenticationKeyAndStoreCapabilityAbortsIf</a>;
// This enforces <a id="high-level-req-2" href="#high-level-req">high-level requirement 2</a>:
Expand Down Expand Up @@ -618,7 +619,8 @@ the SignerCapability.



<pre><code>// This enforces <a id="high-level-req-6" href="#high-level-req">high-level requirement 6</a>:
<pre><code><b>pragma</b> aborts_if_is_partial;
// This enforces <a id="high-level-req-6" href="#high-level-req">high-level requirement 6</a>:
<b>aborts_if</b> !<b>exists</b>&lt;<a href="resource_account.md#0x1_resource_account_Container">Container</a>&gt;(source_addr);
<b>let</b> resource_addr = <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(resource);
<b>let</b> container = <b>global</b>&lt;<a href="resource_account.md#0x1_resource_account_Container">Container</a>&gt;(source_addr);
Expand Down
44 changes: 31 additions & 13 deletions aptos-move/framework/aptos-framework/doc/staking_contract.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ pool.
- [Function `pending_distribution_counts`](#@Specification_1_pending_distribution_counts)
- [Function `staking_contract_exists`](#@Specification_1_staking_contract_exists)
- [Function `beneficiary_for_operator`](#@Specification_1_beneficiary_for_operator)
- [Function `get_expected_stake_pool_address`](#@Specification_1_get_expected_stake_pool_address)
- [Function `create_staking_contract`](#@Specification_1_create_staking_contract)
- [Function `create_staking_contract_with_coins`](#@Specification_1_create_staking_contract_with_coins)
- [Function `add_stake`](#@Specification_1_add_stake)
Expand Down Expand Up @@ -2958,35 +2959,52 @@ Staking_contract exists the stacker/operator pair.



<a id="@Specification_1_beneficiary_for_operator"></a>

<a id="0x1_staking_contract_spec_staking_contract_exists"></a>
### Function `beneficiary_for_operator`


<pre><code><b>fun</b> <a href="staking_contract.md#0x1_staking_contract_spec_staking_contract_exists">spec_staking_contract_exists</a>(staker: <b>address</b>, operator: <b>address</b>): bool {
<b>if</b> (!<b>exists</b>&lt;<a href="staking_contract.md#0x1_staking_contract_Store">Store</a>&gt;(staker)) {
<b>false</b>
} <b>else</b> {
<b>let</b> store = <b>global</b>&lt;<a href="staking_contract.md#0x1_staking_contract_Store">Store</a>&gt;(staker);
<a href="../../aptos-stdlib/doc/simple_map.md#0x1_simple_map_spec_contains_key">simple_map::spec_contains_key</a>(store.staking_contracts, operator)
}
}
<pre><code>#[view]
<b>public</b> <b>fun</b> <a href="staking_contract.md#0x1_staking_contract_beneficiary_for_operator">beneficiary_for_operator</a>(operator: <b>address</b>): <b>address</b>
</code></pre>



<a id="@Specification_1_beneficiary_for_operator"></a>

### Function `beneficiary_for_operator`
<pre><code><b>pragma</b> verify = <b>false</b>;
</code></pre>



<a id="@Specification_1_get_expected_stake_pool_address"></a>

### Function `get_expected_stake_pool_address`


<pre><code>#[view]
<b>public</b> <b>fun</b> <a href="staking_contract.md#0x1_staking_contract_beneficiary_for_operator">beneficiary_for_operator</a>(operator: <b>address</b>): <b>address</b>
<b>public</b> <b>fun</b> <a href="staking_contract.md#0x1_staking_contract_get_expected_stake_pool_address">get_expected_stake_pool_address</a>(staker: <b>address</b>, operator: <b>address</b>, contract_creation_seed: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u8&gt;): <b>address</b>
</code></pre>




<pre><code><b>pragma</b> verify = <b>false</b>;
<pre><code><b>pragma</b> aborts_if_is_partial;
</code></pre>




<a id="0x1_staking_contract_spec_staking_contract_exists"></a>


<pre><code><b>fun</b> <a href="staking_contract.md#0x1_staking_contract_spec_staking_contract_exists">spec_staking_contract_exists</a>(staker: <b>address</b>, operator: <b>address</b>): bool {
<b>if</b> (!<b>exists</b>&lt;<a href="staking_contract.md#0x1_staking_contract_Store">Store</a>&gt;(staker)) {
<b>false</b>
} <b>else</b> {
<b>let</b> store = <b>global</b>&lt;<a href="staking_contract.md#0x1_staking_contract_Store">Store</a>&gt;(staker);
<a href="../../aptos-stdlib/doc/simple_map.md#0x1_simple_map_spec_contains_key">simple_map::spec_contains_key</a>(store.staking_contracts, operator)
}
}
</code></pre>


Expand Down
167 changes: 167 additions & 0 deletions aptos-move/framework/aptos-framework/sources/account.move
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module aptos_framework::account {
use aptos_framework::create_signer::create_signer;
use aptos_framework::event::{Self, EventHandle};
use aptos_framework::guid;
use aptos_framework::permissioned_signer;
use aptos_framework::system_addresses;
use aptos_std::ed25519;
use aptos_std::from_bcs;
Expand Down Expand Up @@ -179,6 +180,8 @@ module aptos_framework::account {
const ENEW_AUTH_KEY_ALREADY_MAPPED: u64 = 21;
/// The current authentication key and the new authentication key are the same
const ENEW_AUTH_KEY_SAME_AS_CURRENT: u64 = 22;
/// Current permissioned signer cannot perform the privilaged operations.
const ENO_ACCOUNT_PERMISSION: u64 = 23;
Comment on lines +183 to +184
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appears to be a typo in the error description - privilaged should be spelled privileged

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.


/// Explicitly separate the GUID space between Object and Account to prevent accidental overlap.
const MAX_GUID_CREATION_NUM: u64 = 0x4000000000000;
Expand All @@ -187,6 +190,43 @@ module aptos_framework::account {
/// Create signer for testing, independently of an Aptos-style `Account`.
public fun create_signer_for_test(addr: address): signer { create_signer(addr) }

enum AccountPermission has copy, drop, store {
/// Permission to rotate a key.
KeyRotation,
/// Permission to offer another address to act like your address
Offering,
}

/// Permissions
///
inline fun check_rotation_permission(s: &signer) {
assert!(
permissioned_signer::check_permission_exists(s, AccountPermission::KeyRotation {}),
error::permission_denied(ENO_ACCOUNT_PERMISSION),
);
}

inline fun check_offering_permission(s: &signer) {
assert!(
permissioned_signer::check_permission_exists(s, AccountPermission::Offering {}),
error::permission_denied(ENO_ACCOUNT_PERMISSION),
);
}

/// Grant permission to perform key rotations on behalf of the master signer.
///
/// This is **extremely dangerous** and should be granted only when it's absolutely needed.
public fun grant_key_rotation_permission(master: &signer, permissioned_signer: &signer) {
permissioned_signer::authorize_unlimited(master, permissioned_signer, AccountPermission::KeyRotation {})
}

/// Grant permission to use offered address's signer on behalf of the master signer.
///
/// This is **extremely dangerous** and should be granted only when it's absolutely needed.
public fun grant_key_offering_permission(master: &signer, permissioned_signer: &signer) {
permissioned_signer::authorize_unlimited(master, permissioned_signer, AccountPermission::Offering {})
}

/// Only called during genesis to initialize system resources for this module.
public(friend) fun initialize(aptos_framework: &signer) {
system_addresses::assert_aptos_framework(aptos_framework);
Expand Down Expand Up @@ -302,6 +342,7 @@ module aptos_framework::account {
vector::length(&new_auth_key) == 32,
error::invalid_argument(EMALFORMED_AUTHENTICATION_KEY)
);
check_rotation_permission(account);
let account_resource = borrow_global_mut<Account>(addr);
account_resource.authentication_key = new_auth_key;
}
Expand Down Expand Up @@ -357,6 +398,7 @@ module aptos_framework::account {
) acquires Account, OriginatingAddress {
let addr = signer::address_of(account);
assert!(exists_at(addr), error::not_found(EACCOUNT_DOES_NOT_EXIST));
check_rotation_permission(account);
let account_resource = borrow_global_mut<Account>(addr);

// Verify the given `from_public_key_bytes` matches this account's current authentication key.
Expand Down Expand Up @@ -412,6 +454,7 @@ module aptos_framework::account {
new_public_key_bytes: vector<u8>,
cap_update_table: vector<u8>
) acquires Account, OriginatingAddress {
check_rotation_permission(delegate_signer);
assert!(exists_at(rotation_cap_offerer_address), error::not_found(EOFFERER_ADDRESS_DOES_NOT_EXIST));

// Check that there exists a rotation capability offer at the offerer's account resource for the delegate.
Expand Down Expand Up @@ -471,6 +514,7 @@ module aptos_framework::account {
account_public_key_bytes: vector<u8>,
recipient_address: address,
) acquires Account {
check_rotation_permission(account);
let addr = signer::address_of(account);
assert!(exists_at(recipient_address), error::not_found(EACCOUNT_DOES_NOT_EXIST));

Expand Down Expand Up @@ -569,6 +613,7 @@ module aptos_framework::account {
/// Revoke the rotation capability offer given to `to_be_revoked_recipient_address` from `account`
public entry fun revoke_rotation_capability(account: &signer, to_be_revoked_address: address) acquires Account {
assert!(exists_at(to_be_revoked_address), error::not_found(EACCOUNT_DOES_NOT_EXIST));
check_rotation_permission(account);
let addr = signer::address_of(account);
let account_resource = borrow_global<Account>(addr);
assert!(
Expand All @@ -580,6 +625,7 @@ module aptos_framework::account {

/// Revoke any rotation capability offer in the specified account.
public entry fun revoke_any_rotation_capability(account: &signer) acquires Account {
check_rotation_permission(account);
let account_resource = borrow_global_mut<Account>(signer::address_of(account));
option::extract(&mut account_resource.rotation_capability_offer.for);
}
Expand All @@ -600,6 +646,7 @@ module aptos_framework::account {
account_public_key_bytes: vector<u8>,
recipient_address: address
) acquires Account {
check_offering_permission(account);
let source_address = signer::address_of(account);
assert!(exists_at(recipient_address), error::not_found(EACCOUNT_DOES_NOT_EXIST));

Expand Down Expand Up @@ -639,6 +686,7 @@ module aptos_framework::account {
/// has a signer capability offer from `account` but will be revoked in this function).
public entry fun revoke_signer_capability(account: &signer, to_be_revoked_address: address) acquires Account {
assert!(exists_at(to_be_revoked_address), error::not_found(EACCOUNT_DOES_NOT_EXIST));
check_offering_permission(account);
let addr = signer::address_of(account);
let account_resource = borrow_global<Account>(addr);
assert!(
Expand All @@ -650,13 +698,15 @@ module aptos_framework::account {

/// Revoke any signer capability offer in the specified account.
public entry fun revoke_any_signer_capability(account: &signer) acquires Account {
check_offering_permission(account);
let account_resource = borrow_global_mut<Account>(signer::address_of(account));
option::extract(&mut account_resource.signer_capability_offer.for);
}

/// Return an authorized signer of the offerer, if there's an existing signer capability offer for `account`
/// at the offerer's address.
public fun create_authorized_signer(account: &signer, offerer_address: address): signer acquires Account {
check_offering_permission(account);
assert!(exists_at(offerer_address), error::not_found(EOFFERER_ADDRESS_DOES_NOT_EXIST));

// Check if there's an existing signer capability offer from the offerer.
Expand Down Expand Up @@ -1202,6 +1252,123 @@ module aptos_framework::account {
assert!(signer::address_of(&signer) == signer::address_of(&alice), 0);
}

#[test(bob = @0x345)]
public entry fun test_valid_check_signer_capability_and_create_authorized_signer_with_permission(bob: signer) acquires Account {
let (alice_sk, alice_pk) = ed25519::generate_keys();
let alice_pk_bytes = ed25519::validated_public_key_to_bytes(&alice_pk);
let alice = create_account_from_ed25519_public_key(alice_pk_bytes);
let alice_addr = signer::address_of(&alice);

let bob_addr = signer::address_of(&bob);
create_account(bob_addr);

let challenge = SignerCapabilityOfferProofChallengeV2 {
sequence_number: borrow_global<Account>(alice_addr).sequence_number,
source_address: alice_addr,
recipient_address: bob_addr,
};

let alice_signer_capability_offer_sig = ed25519::sign_struct(&alice_sk, challenge);

let alice_permission_handle = permissioned_signer::create_permissioned_handle(&alice);
let alice_permission_signer = permissioned_signer::signer_from_permissioned_handle(&alice_permission_handle);

grant_key_offering_permission(&alice, &alice_permission_signer);

offer_signer_capability(
&alice_permission_signer,
ed25519::signature_to_bytes(&alice_signer_capability_offer_sig),
0,
alice_pk_bytes,
bob_addr
);

assert!(option::contains(&borrow_global<Account>(alice_addr).signer_capability_offer.for, &bob_addr), 0);

let signer = create_authorized_signer(&bob, alice_addr);
assert!(signer::address_of(&signer) == signer::address_of(&alice), 0);

permissioned_signer::destroy_permissioned_handle(alice_permission_handle);
}

#[test(bob = @0x345)]
#[expected_failure(abort_code = 0x50017, location = Self)]
public entry fun test_valid_check_signer_capability_and_create_authorized_signer_with_no_permission(bob: signer) acquires Account {
let (alice_sk, alice_pk) = ed25519::generate_keys();
let alice_pk_bytes = ed25519::validated_public_key_to_bytes(&alice_pk);
let alice = create_account_from_ed25519_public_key(alice_pk_bytes);
let alice_addr = signer::address_of(&alice);

let bob_addr = signer::address_of(&bob);
create_account(bob_addr);

let challenge = SignerCapabilityOfferProofChallengeV2 {
sequence_number: borrow_global<Account>(alice_addr).sequence_number,
source_address: alice_addr,
recipient_address: bob_addr,
};

let alice_signer_capability_offer_sig = ed25519::sign_struct(&alice_sk, challenge);

let alice_permission_handle = permissioned_signer::create_permissioned_handle(&alice);
let alice_permission_signer = permissioned_signer::signer_from_permissioned_handle(&alice_permission_handle);

offer_signer_capability(
&alice_permission_signer,
ed25519::signature_to_bytes(&alice_signer_capability_offer_sig),
0,
alice_pk_bytes,
bob_addr
);

assert!(option::contains(&borrow_global<Account>(alice_addr).signer_capability_offer.for, &bob_addr), 0);

let signer = create_authorized_signer(&bob, alice_addr);
assert!(signer::address_of(&signer) == signer::address_of(&alice), 0);

permissioned_signer::destroy_permissioned_handle(alice_permission_handle);
}

#[test(bob = @0x345)]
#[expected_failure(abort_code = 0x50017, location = Self)]
public entry fun test_valid_check_signer_capability_and_create_authorized_signer_with_wrong_permission(bob: signer) acquires Account {
let (alice_sk, alice_pk) = ed25519::generate_keys();
let alice_pk_bytes = ed25519::validated_public_key_to_bytes(&alice_pk);
let alice = create_account_from_ed25519_public_key(alice_pk_bytes);
let alice_addr = signer::address_of(&alice);

let bob_addr = signer::address_of(&bob);
create_account(bob_addr);

let challenge = SignerCapabilityOfferProofChallengeV2 {
sequence_number: borrow_global<Account>(alice_addr).sequence_number,
source_address: alice_addr,
recipient_address: bob_addr,
};

let alice_signer_capability_offer_sig = ed25519::sign_struct(&alice_sk, challenge);

let alice_permission_handle = permissioned_signer::create_permissioned_handle(&alice);
let alice_permission_signer = permissioned_signer::signer_from_permissioned_handle(&alice_permission_handle);

grant_key_rotation_permission(&alice, &alice_permission_signer);

offer_signer_capability(
&alice_permission_signer,
ed25519::signature_to_bytes(&alice_signer_capability_offer_sig),
0,
alice_pk_bytes,
bob_addr
);

assert!(option::contains(&borrow_global<Account>(alice_addr).signer_capability_offer.for, &bob_addr), 0);

let signer = create_authorized_signer(&bob, alice_addr);
assert!(signer::address_of(&signer) == signer::address_of(&alice), 0);

permissioned_signer::destroy_permissioned_handle(alice_permission_handle);
}

#[test(bob = @0x345)]
public entry fun test_get_signer_cap_and_is_signer_cap(bob: signer) acquires Account {
let (alice_sk, alice_pk) = ed25519::generate_keys();
Expand Down
6 changes: 3 additions & 3 deletions aptos-move/framework/aptos-framework/sources/account.spec.move
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ spec aptos_framework::account {
///

spec module {
pragma verify = true;
pragma aborts_if_is_strict;
pragma verify = false;


}

/// Only the address `@aptos_framework` can call.
Expand Down Expand Up @@ -390,7 +391,6 @@ spec aptos_framework::account {
source_address,
recipient_address,
};

aborts_if !exists<chain_id::ChainId>(@aptos_framework);
aborts_if !exists<Account>(recipient_address);
aborts_if !exists<Account>(source_address);
Expand Down
Loading
Loading