Add SetAuthorityChecked instruction to bpf loader (#28424)

* SetAuthorityChecked

* restore old logic for loader

* add more upgrade authority checked test cases

* setBufferAuthority checked tests

* format

* add set_buffer_authority_checked instruction to sdk

* Update transaction-status/src/parse_bpf_loader.rs

Co-authored-by: Justin Starry <justin.m.starry@gmail.com>

* add is_set_authority_checked function

* fix set_buffer_authority_checked sdk instruction

* feature gate setAuthorityChecked

* add bpf loader tests for setAuthorityChecked ixs

* test that you can set to same authority

* allow set_authority_checked to be called via cpi (if feature is enabled)

* fix ci

* fmt

Co-authored-by: Justin Starry <justin.m.starry@gmail.com>
Co-authored-by: Justin Starry <justin@solana.com>
This commit is contained in:
0xripleys 2022-11-01 09:34:04 +01:00 committed by GitHub
parent f352934fca
commit 5de4dd8f9b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 680 additions and 2 deletions

View File

@ -48,6 +48,7 @@ use {
cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts,
check_slice_translation_size, disable_deploy_of_alloc_free_syscall,
disable_deprecated_loader, enable_bpf_loader_extend_program_ix,
enable_bpf_loader_set_authority_checked_ix,
error_on_syscall_bpf_function_hash_collisions, limit_max_instruction_trace_length,
reject_callx_r10, FeatureSet,
},
@ -1015,6 +1016,79 @@ fn process_loader_upgradeable_instruction(
ic_logger_msg!(log_collector, "New authority {:?}", new_authority);
}
UpgradeableLoaderInstruction::SetAuthorityChecked => {
if !invoke_context
.feature_set
.is_active(&enable_bpf_loader_set_authority_checked_ix::id())
{
return Err(InstructionError::InvalidInstructionData);
}
instruction_context.check_number_of_instruction_accounts(3)?;
let mut account =
instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
let present_authority_key = transaction_context.get_key_of_account_at_index(
instruction_context.get_index_of_instruction_account_in_transaction(1)?,
)?;
let new_authority_key = transaction_context.get_key_of_account_at_index(
instruction_context.get_index_of_instruction_account_in_transaction(2)?,
)?;
match account.get_state()? {
UpgradeableLoaderState::Buffer { authority_address } => {
if authority_address.is_none() {
ic_logger_msg!(log_collector, "Buffer is immutable");
return Err(InstructionError::Immutable);
}
if authority_address != Some(*present_authority_key) {
ic_logger_msg!(log_collector, "Incorrect buffer authority provided");
return Err(InstructionError::IncorrectAuthority);
}
if !instruction_context.is_instruction_account_signer(1)? {
ic_logger_msg!(log_collector, "Buffer authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
if !instruction_context.is_instruction_account_signer(2)? {
ic_logger_msg!(log_collector, "New authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
account.set_state(&UpgradeableLoaderState::Buffer {
authority_address: Some(*new_authority_key),
})?;
}
UpgradeableLoaderState::ProgramData {
slot,
upgrade_authority_address,
} => {
if upgrade_authority_address.is_none() {
ic_logger_msg!(log_collector, "Program not upgradeable");
return Err(InstructionError::Immutable);
}
if upgrade_authority_address != Some(*present_authority_key) {
ic_logger_msg!(log_collector, "Incorrect upgrade authority provided");
return Err(InstructionError::IncorrectAuthority);
}
if !instruction_context.is_instruction_account_signer(1)? {
ic_logger_msg!(log_collector, "Upgrade authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
if !instruction_context.is_instruction_account_signer(2)? {
ic_logger_msg!(log_collector, "New authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
account.set_state(&UpgradeableLoaderState::ProgramData {
slot,
upgrade_authority_address: Some(*new_authority_key),
})?;
}
_ => {
ic_logger_msg!(log_collector, "Account does not support authorities");
return Err(InstructionError::InvalidArgument);
}
}
ic_logger_msg!(log_collector, "New authority {:?}", new_authority_key);
}
UpgradeableLoaderInstruction::Close => {
instruction_context.check_number_of_instruction_accounts(2)?;
if instruction_context.get_index_of_instruction_account_in_transaction(0)?
@ -2927,6 +3001,251 @@ mod tests {
);
}
#[test]
fn test_bpf_loader_upgradeable_set_upgrade_authority_checked() {
let instruction =
bincode::serialize(&UpgradeableLoaderInstruction::SetAuthorityChecked).unwrap();
let loader_id = bpf_loader_upgradeable::id();
let slot = 0;
let upgrade_authority_address = Pubkey::new_unique();
let upgrade_authority_account = AccountSharedData::new(1, 0, &Pubkey::new_unique());
let new_upgrade_authority_address = Pubkey::new_unique();
let new_upgrade_authority_account = AccountSharedData::new(1, 0, &Pubkey::new_unique());
let program_address = Pubkey::new_unique();
let (programdata_address, _) = Pubkey::find_program_address(
&[program_address.as_ref()],
&bpf_loader_upgradeable::id(),
);
let mut programdata_account = AccountSharedData::new(
1,
UpgradeableLoaderState::size_of_programdata(0),
&bpf_loader_upgradeable::id(),
);
programdata_account
.set_state(&UpgradeableLoaderState::ProgramData {
slot,
upgrade_authority_address: Some(upgrade_authority_address),
})
.unwrap();
let programdata_meta = AccountMeta {
pubkey: programdata_address,
is_signer: false,
is_writable: true,
};
let upgrade_authority_meta = AccountMeta {
pubkey: upgrade_authority_address,
is_signer: true,
is_writable: false,
};
let new_upgrade_authority_meta = AccountMeta {
pubkey: new_upgrade_authority_address,
is_signer: true,
is_writable: false,
};
// Case: Set to new authority
let accounts = process_instruction(
&loader_id,
&[],
&instruction,
vec![
(programdata_address, programdata_account.clone()),
(upgrade_authority_address, upgrade_authority_account.clone()),
(
new_upgrade_authority_address,
new_upgrade_authority_account.clone(),
),
],
vec![
programdata_meta.clone(),
upgrade_authority_meta.clone(),
new_upgrade_authority_meta.clone(),
],
Ok(()),
);
let state: UpgradeableLoaderState = accounts.first().unwrap().state().unwrap();
assert_eq!(
state,
UpgradeableLoaderState::ProgramData {
slot,
upgrade_authority_address: Some(new_upgrade_authority_address),
}
);
// Case: set to same authority
process_instruction(
&loader_id,
&[],
&instruction,
vec![
(programdata_address, programdata_account.clone()),
(upgrade_authority_address, upgrade_authority_account.clone()),
],
vec![
programdata_meta.clone(),
upgrade_authority_meta.clone(),
upgrade_authority_meta.clone(),
],
Ok(()),
);
// Case: present authority not in instruction
process_instruction(
&loader_id,
&[],
&instruction,
vec![
(programdata_address, programdata_account.clone()),
(upgrade_authority_address, upgrade_authority_account.clone()),
(
new_upgrade_authority_address,
new_upgrade_authority_account.clone(),
),
],
vec![programdata_meta.clone(), new_upgrade_authority_meta.clone()],
Err(InstructionError::NotEnoughAccountKeys),
);
// Case: new authority not in instruction
process_instruction(
&loader_id,
&[],
&instruction,
vec![
(programdata_address, programdata_account.clone()),
(upgrade_authority_address, upgrade_authority_account.clone()),
(
new_upgrade_authority_address,
new_upgrade_authority_account.clone(),
),
],
vec![programdata_meta.clone(), upgrade_authority_meta.clone()],
Err(InstructionError::NotEnoughAccountKeys),
);
// Case: present authority did not sign
process_instruction(
&loader_id,
&[],
&instruction,
vec![
(programdata_address, programdata_account.clone()),
(upgrade_authority_address, upgrade_authority_account.clone()),
(
new_upgrade_authority_address,
new_upgrade_authority_account.clone(),
),
],
vec![
programdata_meta.clone(),
AccountMeta {
pubkey: upgrade_authority_address,
is_signer: false,
is_writable: false,
},
new_upgrade_authority_meta.clone(),
],
Err(InstructionError::MissingRequiredSignature),
);
// Case: New authority did not sign
process_instruction(
&loader_id,
&[],
&instruction,
vec![
(programdata_address, programdata_account.clone()),
(upgrade_authority_address, upgrade_authority_account.clone()),
(
new_upgrade_authority_address,
new_upgrade_authority_account.clone(),
),
],
vec![
programdata_meta.clone(),
upgrade_authority_meta.clone(),
AccountMeta {
pubkey: new_upgrade_authority_address,
is_signer: false,
is_writable: false,
},
],
Err(InstructionError::MissingRequiredSignature),
);
// Case: wrong present authority
let invalid_upgrade_authority_address = Pubkey::new_unique();
process_instruction(
&loader_id,
&[],
&instruction,
vec![
(programdata_address, programdata_account.clone()),
(
invalid_upgrade_authority_address,
upgrade_authority_account.clone(),
),
(new_upgrade_authority_address, new_upgrade_authority_account),
],
vec![
programdata_meta.clone(),
AccountMeta {
pubkey: invalid_upgrade_authority_address,
is_signer: true,
is_writable: false,
},
new_upgrade_authority_meta.clone(),
],
Err(InstructionError::IncorrectAuthority),
);
// Case: programdata is immutable
programdata_account
.set_state(&UpgradeableLoaderState::ProgramData {
slot,
upgrade_authority_address: None,
})
.unwrap();
process_instruction(
&loader_id,
&[],
&instruction,
vec![
(programdata_address, programdata_account.clone()),
(upgrade_authority_address, upgrade_authority_account.clone()),
],
vec![
programdata_meta.clone(),
upgrade_authority_meta.clone(),
new_upgrade_authority_meta.clone(),
],
Err(InstructionError::Immutable),
);
// Case: Not a ProgramData account
programdata_account
.set_state(&UpgradeableLoaderState::Program {
programdata_address: Pubkey::new_unique(),
})
.unwrap();
process_instruction(
&loader_id,
&[],
&instruction,
vec![
(programdata_address, programdata_account.clone()),
(upgrade_authority_address, upgrade_authority_account),
],
vec![
programdata_meta,
upgrade_authority_meta,
new_upgrade_authority_meta,
],
Err(InstructionError::InvalidArgument),
);
}
#[test]
fn test_bpf_loader_upgradeable_set_buffer_authority() {
let instruction = bincode::serialize(&UpgradeableLoaderInstruction::SetAuthority).unwrap();
@ -3099,6 +3418,204 @@ mod tests {
);
}
#[test]
fn test_bpf_loader_upgradeable_set_buffer_authority_checked() {
let instruction =
bincode::serialize(&UpgradeableLoaderInstruction::SetAuthorityChecked).unwrap();
let loader_id = bpf_loader_upgradeable::id();
let invalid_authority_address = Pubkey::new_unique();
let authority_address = Pubkey::new_unique();
let authority_account = AccountSharedData::new(1, 0, &Pubkey::new_unique());
let new_authority_address = Pubkey::new_unique();
let new_authority_account = AccountSharedData::new(1, 0, &Pubkey::new_unique());
let buffer_address = Pubkey::new_unique();
let mut buffer_account =
AccountSharedData::new(1, UpgradeableLoaderState::size_of_buffer(0), &loader_id);
buffer_account
.set_state(&UpgradeableLoaderState::Buffer {
authority_address: Some(authority_address),
})
.unwrap();
let mut transaction_accounts = vec![
(buffer_address, buffer_account.clone()),
(authority_address, authority_account.clone()),
(new_authority_address, new_authority_account.clone()),
];
let buffer_meta = AccountMeta {
pubkey: buffer_address,
is_signer: false,
is_writable: true,
};
let authority_meta = AccountMeta {
pubkey: authority_address,
is_signer: true,
is_writable: false,
};
let new_authority_meta = AccountMeta {
pubkey: new_authority_address,
is_signer: true,
is_writable: false,
};
// Case: Set to new authority
buffer_account
.set_state(&UpgradeableLoaderState::Buffer {
authority_address: Some(authority_address),
})
.unwrap();
let accounts = process_instruction(
&loader_id,
&[],
&instruction,
transaction_accounts.clone(),
vec![
buffer_meta.clone(),
authority_meta.clone(),
new_authority_meta.clone(),
],
Ok(()),
);
let state: UpgradeableLoaderState = accounts.first().unwrap().state().unwrap();
assert_eq!(
state,
UpgradeableLoaderState::Buffer {
authority_address: Some(new_authority_address),
}
);
// Case: set to same authority
process_instruction(
&loader_id,
&[],
&instruction,
transaction_accounts.clone(),
vec![
buffer_meta.clone(),
authority_meta.clone(),
authority_meta.clone(),
],
Ok(()),
);
// Case: Missing current authority
process_instruction(
&loader_id,
&[],
&instruction,
transaction_accounts.clone(),
vec![buffer_meta.clone(), new_authority_meta.clone()],
Err(InstructionError::NotEnoughAccountKeys),
);
// Case: Missing new authority
process_instruction(
&loader_id,
&[],
&instruction,
transaction_accounts.clone(),
vec![buffer_meta.clone(), authority_meta.clone()],
Err(InstructionError::NotEnoughAccountKeys),
);
// Case: wrong present authority
process_instruction(
&loader_id,
&[],
&instruction,
vec![
(buffer_address, buffer_account.clone()),
(invalid_authority_address, authority_account),
(new_authority_address, new_authority_account),
],
vec![
buffer_meta.clone(),
AccountMeta {
pubkey: invalid_authority_address,
is_signer: true,
is_writable: false,
},
new_authority_meta.clone(),
],
Err(InstructionError::IncorrectAuthority),
);
// Case: present authority did not sign
process_instruction(
&loader_id,
&[],
&instruction,
transaction_accounts.clone(),
vec![
buffer_meta.clone(),
AccountMeta {
pubkey: authority_address,
is_signer: false,
is_writable: false,
},
new_authority_meta.clone(),
],
Err(InstructionError::MissingRequiredSignature),
);
// Case: new authority did not sign
process_instruction(
&loader_id,
&[],
&instruction,
transaction_accounts.clone(),
vec![
buffer_meta.clone(),
authority_meta.clone(),
AccountMeta {
pubkey: new_authority_address,
is_signer: false,
is_writable: false,
},
],
Err(InstructionError::MissingRequiredSignature),
);
// Case: Not a Buffer account
transaction_accounts
.get_mut(0)
.unwrap()
.1
.set_state(&UpgradeableLoaderState::Program {
programdata_address: Pubkey::new_unique(),
})
.unwrap();
process_instruction(
&loader_id,
&[],
&instruction,
transaction_accounts.clone(),
vec![
buffer_meta.clone(),
authority_meta.clone(),
new_authority_meta.clone(),
],
Err(InstructionError::InvalidArgument),
);
// Case: Buffer is immutable
transaction_accounts
.get_mut(0)
.unwrap()
.1
.set_state(&UpgradeableLoaderState::Buffer {
authority_address: None,
})
.unwrap();
process_instruction(
&loader_id,
&[],
&instruction,
transaction_accounts.clone(),
vec![buffer_meta, authority_meta, new_authority_meta],
Err(InstructionError::Immutable),
);
}
#[test]
fn test_bpf_loader_upgradeable_close() {
let instruction = bincode::serialize(&UpgradeableLoaderInstruction::Close).unwrap();

View File

@ -1,8 +1,11 @@
use {
super::*,
crate::declare_syscall,
solana_sdk::syscalls::{
MAX_CPI_ACCOUNT_INFOS, MAX_CPI_INSTRUCTION_ACCOUNTS, MAX_CPI_INSTRUCTION_DATA_LEN,
solana_sdk::{
feature_set::enable_bpf_loader_set_authority_checked_ix,
syscalls::{
MAX_CPI_ACCOUNT_INFOS, MAX_CPI_INSTRUCTION_ACCOUNTS, MAX_CPI_INSTRUCTION_DATA_LEN,
},
},
};
@ -833,6 +836,12 @@ fn check_authorized_program(
|| (bpf_loader_upgradeable::check_id(program_id)
&& !(bpf_loader_upgradeable::is_upgrade_instruction(instruction_data)
|| bpf_loader_upgradeable::is_set_authority_instruction(instruction_data)
|| (invoke_context
.feature_set
.is_active(&enable_bpf_loader_set_authority_checked_ix::id())
&& bpf_loader_upgradeable::is_set_authority_checked_instruction(
instruction_data,
))
|| bpf_loader_upgradeable::is_close_instruction(instruction_data)))
|| is_precompile(program_id, |feature_id: &Pubkey| {
invoke_context.feature_set.is_active(feature_id)

View File

@ -231,6 +231,10 @@ pub fn is_close_instruction(instruction_data: &[u8]) -> bool {
!instruction_data.is_empty() && 5 == instruction_data[0]
}
pub fn is_set_authority_checked_instruction(instruction_data: &[u8]) -> bool {
!instruction_data.is_empty() && 7 == instruction_data[0]
}
/// Returns the instructions required to set a buffers's authority.
pub fn set_buffer_authority(
buffer_address: &Pubkey,
@ -248,6 +252,24 @@ pub fn set_buffer_authority(
)
}
/// Returns the instructions required to set a buffers's authority. If using this instruction, the new authority
/// must sign.
pub fn set_buffer_authority_checked(
buffer_address: &Pubkey,
current_authority_address: &Pubkey,
new_authority_address: &Pubkey,
) -> Instruction {
Instruction::new_with_bincode(
id(),
&UpgradeableLoaderInstruction::SetAuthorityChecked,
vec![
AccountMeta::new(*buffer_address, false),
AccountMeta::new_readonly(*current_authority_address, true),
AccountMeta::new_readonly(*new_authority_address, true),
],
)
}
/// Returns the instructions required to set a program's authority.
pub fn set_upgrade_authority(
program_address: &Pubkey,
@ -266,6 +288,27 @@ pub fn set_upgrade_authority(
Instruction::new_with_bincode(id(), &UpgradeableLoaderInstruction::SetAuthority, metas)
}
/// Returns the instructions required to set a program's authority. If using this instruction, the new authority
/// must sign.
pub fn set_upgrade_authority_checked(
program_address: &Pubkey,
current_authority_address: &Pubkey,
new_authority_address: &Pubkey,
) -> Instruction {
let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id());
let metas = vec![
AccountMeta::new(programdata_address, false),
AccountMeta::new_readonly(*current_authority_address, true),
AccountMeta::new_readonly(*new_authority_address, true),
];
Instruction::new_with_bincode(
id(),
&UpgradeableLoaderInstruction::SetAuthorityChecked,
metas,
)
}
/// Returns the instructions required to close a buffer account
pub fn close(
close_address: &Pubkey,
@ -455,6 +498,15 @@ mod tests {
);
}
#[test]
fn test_is_set_authority_checked_instruction() {
assert!(!is_set_authority_checked_instruction(&[]));
assert_is_instruction(
is_set_authority_checked_instruction,
UpgradeableLoaderInstruction::SetAuthorityChecked {},
);
}
#[test]
fn test_is_upgrade_instruction() {
assert!(!is_upgrade_instruction(&[]));

View File

@ -147,4 +147,17 @@ pub enum UpgradeableLoaderInstruction {
/// Number of bytes to extend the program data.
additional_bytes: u32,
},
/// Set a new authority that is allowed to write the buffer or upgrade the
/// program.
///
/// This instruction differs from SetAuthority in that the new authority is a
/// required signer.
///
/// # Account references
/// 0. `[writable]` The Buffer or ProgramData account to change the
/// authority of.
/// 1. `[signer]` The current authority.
/// 2. `[signer]` The new authority.
SetAuthorityChecked,
}

View File

@ -534,6 +534,10 @@ pub mod check_syscall_outputs_do_not_overlap {
solana_sdk::declare_id!("3uRVPBpyEJRo1emLCrq38eLRFGcu6uKSpUXqGvU8T7SZ");
}
pub mod enable_bpf_loader_set_authority_checked_ix {
solana_sdk::declare_id!("5x3825XS7M2A3Ekbn5VGGkvFoAg5qrRWkTrY4bARP1GL");
}
lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -662,6 +666,7 @@ lazy_static! {
(increase_tx_account_lock_limit::id(), "increase tx account lock limit to 128 #27241"),
(limit_max_instruction_trace_length::id(), "limit max instruction trace length #27939"),
(check_syscall_outputs_do_not_overlap::id(), "check syscall outputs do_not overlap #28600"),
(enable_bpf_loader_set_authority_checked_ix::id(), "enable bpf upgradeable loader SetAuthorityChecked instruction #28424"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()

View File

@ -139,6 +139,17 @@ pub fn parse_bpf_upgradeable_loader(
}),
})
}
UpgradeableLoaderInstruction::SetAuthorityChecked => {
check_num_bpf_upgradeable_loader_accounts(&instruction.accounts, 3)?;
Ok(ParsedInstructionEnum {
instruction_type: "setAuthorityChecked".to_string(),
info: json!({
"account": account_keys[instruction.accounts[0] as usize].to_string(),
"authority": account_keys[instruction.accounts[1] as usize].to_string(),
"newAuthority": account_keys[instruction.accounts[2] as usize].to_string(),
}),
})
}
UpgradeableLoaderInstruction::Close => {
check_num_bpf_upgradeable_loader_accounts(&instruction.accounts, 3)?;
Ok(ParsedInstructionEnum {
@ -536,6 +547,39 @@ mod test {
.is_err());
}
#[test]
fn test_parse_bpf_upgradeable_loader_set_buffer_authority_checked_ix() {
let buffer_address = Pubkey::new_unique();
let current_authority_address = Pubkey::new_unique();
let new_authority_address = Pubkey::new_unique();
let instruction = bpf_loader_upgradeable::set_buffer_authority_checked(
&buffer_address,
&current_authority_address,
&new_authority_address,
);
let message = Message::new(&[instruction], None);
assert_eq!(
parse_bpf_upgradeable_loader(
&message.instructions[0],
&AccountKeys::new(&message.account_keys, None)
)
.unwrap(),
ParsedInstructionEnum {
instruction_type: "setAuthorityChecked".to_string(),
info: json!({
"account": buffer_address.to_string(),
"authority": current_authority_address.to_string(),
"newAuthority": new_authority_address.to_string(),
}),
}
);
assert!(parse_bpf_upgradeable_loader(
&message.instructions[0],
&AccountKeys::new(&message.account_keys[0..2], None)
)
.is_err());
}
#[test]
fn test_parse_bpf_upgradeable_loader_set_upgrade_authority_ix() {
let program_address = Pubkey::new_unique();
@ -615,6 +659,44 @@ mod test {
.is_err());
}
#[test]
fn test_parse_bpf_upgradeable_loader_set_upgrade_authority_checked_ix() {
let program_address = Pubkey::new_unique();
let current_authority_address = Pubkey::new_unique();
let new_authority_address = Pubkey::new_unique();
let (programdata_address, _) = Pubkey::find_program_address(
&[program_address.as_ref()],
&bpf_loader_upgradeable::id(),
);
let instruction = bpf_loader_upgradeable::set_upgrade_authority_checked(
&program_address,
&current_authority_address,
&new_authority_address,
);
let message = Message::new(&[instruction], None);
assert_eq!(
parse_bpf_upgradeable_loader(
&message.instructions[0],
&AccountKeys::new(&message.account_keys, None)
)
.unwrap(),
ParsedInstructionEnum {
instruction_type: "setAuthorityChecked".to_string(),
info: json!({
"account": programdata_address.to_string(),
"authority": current_authority_address.to_string(),
"newAuthority": new_authority_address.to_string(),
}),
}
);
assert!(parse_bpf_upgradeable_loader(
&message.instructions[0],
&AccountKeys::new(&message.account_keys[0..2], None)
)
.is_err());
}
#[test]
fn test_parse_bpf_upgradeable_loader_close_buffer_ix() {
let close_address = Pubkey::new_unique();