From 0be3b6e3bab7dd03f4a6c3584d7f897fdcfdd456 Mon Sep 17 00:00:00 2001 From: norwnd <112318969+norwnd@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:21:17 +0000 Subject: [PATCH] cli: program upgrade with offline signing (--sign-only mode) (#33860) * cli: program upgrade with offline signing (--sign-only mode) * ditch universal NullSigner approach as overly complex * rearrange some code to make it more coherent, fix typos * apply clippy suggestions to simplify code * review pass * adjust brokens (due to rebase onto refactored structures) doc links * fix linter complaint in doc formatting * remove rebase artifacts --------- Co-authored-by: norwnd --- Cargo.lock | 1 + cli/Cargo.toml | 1 + cli/src/program.rs | 171 ++++++++++++++- cli/tests/program.rs | 252 +++++++++++++++++++++- docs/src/cli/examples/deploy-a-program.md | 49 +++++ 5 files changed, 468 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b78ec8d53..8453cfea9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5751,6 +5751,7 @@ dependencies = [ "solana_rbpf", "spl-memo", "tempfile", + "test-case", "thiserror", "tiny-bip39", ] diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 01d773ff9..b9170ac79 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -59,6 +59,7 @@ assert_matches = { workspace = true } solana-streamer = { workspace = true } solana-test-validator = { workspace = true } tempfile = { workspace = true } +test-case = { workspace = true } [[bin]] name = "solana" diff --git a/cli/src/program.rs b/cli/src/program.rs index 745bcec11..04863c108 100644 --- a/cli/src/program.rs +++ b/cli/src/program.rs @@ -18,11 +18,13 @@ use { input_parsers::*, input_validators::*, keypair::*, + offline::{OfflineArgs, DUMP_TRANSACTION_MESSAGE, SIGN_ONLY_ARG}, }, solana_cli_output::{ - CliProgram, CliProgramAccountType, CliProgramAuthority, CliProgramBuffer, CliProgramId, - CliUpgradeableBuffer, CliUpgradeableBuffers, CliUpgradeableProgram, - CliUpgradeableProgramClosed, CliUpgradeableProgramExtended, CliUpgradeablePrograms, + return_signers_with_config, CliProgram, CliProgramAccountType, CliProgramAuthority, + CliProgramBuffer, CliProgramId, CliUpgradeableBuffer, CliUpgradeableBuffers, + CliUpgradeableProgram, CliUpgradeableProgramClosed, CliUpgradeableProgramExtended, + CliUpgradeablePrograms, ReturnSignersConfig, }, solana_client::{ connection_cache::ConnectionCache, @@ -40,6 +42,7 @@ use { config::{RpcAccountInfoConfig, RpcProgramAccountsConfig, RpcSendTransactionConfig}, filter::{Memcmp, RpcFilterType}, }, + solana_rpc_client_nonce_utils::blockhash_query::BlockhashQuery, solana_sdk::{ account::Account, account_utils::StateMut, @@ -88,6 +91,15 @@ pub enum ProgramCliCommand { allow_excessive_balance: bool, skip_fee_check: bool, }, + Upgrade { + fee_payer_signer_index: SignerIndex, + program_pubkey: Pubkey, + buffer_pubkey: Pubkey, + upgrade_authority_signer_index: SignerIndex, + sign_only: bool, + dump_transaction_message: bool, + blockhash_query: BlockhashQuery, + }, WriteBuffer { program_location: String, fee_payer_signer_index: SignerIndex, @@ -220,6 +232,36 @@ impl ProgramSubCommands for App<'_, '_> { ), ), ) + .subcommand( + SubCommand::with_name("upgrade") + .about("Upgrade an upgradeable program") + .arg(pubkey!( + Arg::with_name("buffer") + .index(1) + .required(true) + .value_name("BUFFER_PUBKEY"), + "Intermediate buffer account with new program data" + )) + .arg(pubkey!( + Arg::with_name("program_id") + .index(2) + .required(true) + .value_name("PROGRAM_ID"), + "Executable program's address (pubkey)" + )) + .arg(fee_payer_arg()) + .arg( + Arg::with_name("upgrade_authority") + .long("upgrade-authority") + .value_name("UPGRADE_AUTHORITY_SIGNER") + .takes_value(true) + .validator(is_valid_signer) + .help( + "Upgrade authority [default: the default configured keypair]", + ), + ) + .offline_args(), + ) .subcommand( SubCommand::with_name("write-buffer") .about("Writes a program into a buffer account") @@ -571,6 +613,47 @@ pub fn parse_program_subcommand( signers: signer_info.signers, } } + ("upgrade", Some(matches)) => { + let sign_only = matches.is_present(SIGN_ONLY_ARG.name); + let dump_transaction_message = matches.is_present(DUMP_TRANSACTION_MESSAGE.name); + let blockhash_query = BlockhashQuery::new_from_matches(matches); + + let buffer_pubkey = pubkey_of_signer(matches, "buffer", wallet_manager) + .unwrap() + .unwrap(); + let program_pubkey = pubkey_of_signer(matches, "program_id", wallet_manager) + .unwrap() + .unwrap(); + + let (fee_payer, fee_payer_pubkey) = + signer_of(matches, FEE_PAYER_ARG.name, wallet_manager)?; + + let mut bulk_signers = vec![ + fee_payer, // if None, default signer will be supplied + ]; + + let (upgrade_authority, upgrade_authority_pubkey) = + signer_of(matches, "upgrade_authority", wallet_manager)?; + bulk_signers.push(upgrade_authority); + + let signer_info = + default_signer.generate_unique_signers(bulk_signers, matches, wallet_manager)?; + + CliCommandInfo { + command: CliCommand::Program(ProgramCliCommand::Upgrade { + fee_payer_signer_index: signer_info.index_of(fee_payer_pubkey).unwrap(), + program_pubkey, + buffer_pubkey, + upgrade_authority_signer_index: signer_info + .index_of(upgrade_authority_pubkey) + .unwrap(), + sign_only, + dump_transaction_message, + blockhash_query, + }), + signers: signer_info.signers, + } + } ("write-buffer", Some(matches)) => { let (fee_payer, fee_payer_pubkey) = signer_of(matches, FEE_PAYER_ARG.name, wallet_manager)?; @@ -816,6 +899,25 @@ pub fn process_program_subcommand( *allow_excessive_balance, *skip_fee_check, ), + ProgramCliCommand::Upgrade { + fee_payer_signer_index, + program_pubkey, + buffer_pubkey, + upgrade_authority_signer_index, + sign_only, + dump_transaction_message, + blockhash_query, + } => process_program_upgrade( + rpc_client, + config, + *fee_payer_signer_index, + *program_pubkey, + *buffer_pubkey, + *upgrade_authority_signer_index, + *sign_only, + *dump_transaction_message, + blockhash_query, + ), ProgramCliCommand::WriteBuffer { program_location, fee_payer_signer_index, @@ -1175,6 +1277,69 @@ fn fetch_buffer_len( } } +/// Upgrade existing program using upgradeable loader +#[allow(clippy::too_many_arguments)] +fn process_program_upgrade( + rpc_client: Arc, + config: &CliConfig, + fee_payer_signer_index: SignerIndex, + program_id: Pubkey, + buffer_pubkey: Pubkey, + upgrade_authority_signer_index: SignerIndex, + sign_only: bool, + dump_transaction_message: bool, + blockhash_query: &BlockhashQuery, +) -> ProcessResult { + let fee_payer_signer = config.signers[fee_payer_signer_index]; + let upgrade_authority_signer = config.signers[upgrade_authority_signer_index]; + + let blockhash = blockhash_query.get_blockhash(&rpc_client, config.commitment)?; + let message = Message::new_with_blockhash( + &[bpf_loader_upgradeable::upgrade( + &program_id, + &buffer_pubkey, + &upgrade_authority_signer.pubkey(), + &fee_payer_signer.pubkey(), + )], + Some(&fee_payer_signer.pubkey()), + &blockhash, + ); + + if sign_only { + let mut tx = Transaction::new_unsigned(message); + let signers = &[fee_payer_signer, upgrade_authority_signer]; + // Using try_partial_sign here because fee_payer_signer might not be the fee payer we + // end up using for this transaction (it might be NullSigner in `--sign-only` mode). + tx.try_partial_sign(signers, blockhash)?; + return_signers_with_config( + &tx, + &config.output_format, + &ReturnSignersConfig { + dump_transaction_message, + }, + ) + } else { + let fee = rpc_client.get_fee_for_message(&message)?; + check_account_for_spend_and_fee_with_commitment( + &rpc_client, + &fee_payer_signer.pubkey(), + 0, + fee, + config.commitment, + )?; + let mut tx = Transaction::new_unsigned(message); + let signers = &[fee_payer_signer, upgrade_authority_signer]; + tx.try_sign(signers, blockhash)?; + rpc_client + .send_and_confirm_transaction_with_spinner(&tx) + .map_err(|e| format!("Upgrading program failed: {e}"))?; + let program_id = CliProgramId { + program_id: program_id.to_string(), + }; + Ok(config.output_format.formatted_string(&program_id)) + } +} + fn process_write_buffer( rpc_client: Arc, config: &CliConfig, diff --git a/cli/tests/program.rs b/cli/tests/program.rs index 7e8d409e1..153cb0c11 100644 --- a/cli/tests/program.rs +++ b/cli/tests/program.rs @@ -1,4 +1,6 @@ #![allow(clippy::arithmetic_side_effects)] +// REMOVE once https://github.com/rust-lang/rust-clippy/issues/11153 is fixed +#![allow(clippy::items_after_test_module)] use { serde_json::Value, @@ -7,19 +9,27 @@ use { program::{ProgramCliCommand, CLOSE_PROGRAM_WARNING}, test_utils::wait_n_slots, }, - solana_cli_output::OutputFormat, + solana_cli_output::{parse_sign_only_reply_string, OutputFormat}, solana_faucet::faucet::run_local_faucet, solana_rpc_client::rpc_client::RpcClient, + solana_rpc_client_nonce_utils::blockhash_query::BlockhashQuery, solana_sdk::{ account_utils::StateMut, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, commitment_config::CommitmentConfig, pubkey::Pubkey, - signature::{Keypair, Signer}, + signature::{Keypair, NullSigner, Signer}, }, solana_streamer::socket::SocketAddrSpace, solana_test_validator::TestValidator, - std::{env, fs::File, io::Read, path::PathBuf, str::FromStr}, + std::{ + env, + fs::File, + io::Read, + path::{Path, PathBuf}, + str::FromStr, + }, + test_case::test_case, }; #[test] @@ -1411,6 +1421,198 @@ fn test_cli_program_mismatch_buffer_authority() { process_command(&config).unwrap(); } +// Assume fee payer will be either online signer or offline signer (could be completely +// separate signer too, but that option is unlikely to be chosen often, so don't bother +// testing for it), we want to test for most common choices. +#[test_case(true; "offline signer will be fee payer")] +#[test_case(false; "online signer will be fee payer")] +fn test_cli_program_deploy_with_offline_signing(use_offline_signer_as_fee_payer: bool) { + solana_logger::setup(); + + let mut noop_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + noop_path.push("tests"); + noop_path.push("fixtures"); + noop_path.push("noop"); + noop_path.set_extension("so"); + + let mut noop_large_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + noop_large_path.push("tests"); + noop_large_path.push("fixtures"); + noop_large_path.push("noop_large"); + noop_large_path.set_extension("so"); + + let mint_keypair = Keypair::new(); + let mint_pubkey = mint_keypair.pubkey(); + let faucet_addr = run_local_faucet(mint_keypair, None); + let test_validator = + TestValidator::with_no_fees(mint_pubkey, Some(faucet_addr), SocketAddrSpace::Unspecified); + + let rpc_client = + RpcClient::new_with_commitment(test_validator.rpc_url(), CommitmentConfig::processed()); + + let blockhash = rpc_client.get_latest_blockhash().unwrap(); + + let mut file = File::open(noop_large_path.to_str().unwrap()).unwrap(); + let mut large_program_data = Vec::new(); + file.read_to_end(&mut large_program_data).unwrap(); + let max_program_data_len = large_program_data.len(); + let minimum_balance_for_large_buffer = rpc_client + .get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_programdata( + max_program_data_len, + )) + .unwrap(); + + let mut config = CliConfig::recent_for_tests(); + config.json_rpc_url = test_validator.rpc_url(); + + let online_signer = Keypair::new(); + let online_signer_identity = NullSigner::new(&online_signer.pubkey()); + let offline_signer = Keypair::new(); + let buffer_signer = Keypair::new(); + // Typically, keypair for program signer should be different from online signer or + // offline signer keypairs. + let program_signer = Keypair::new(); + + config.command = CliCommand::Airdrop { + pubkey: None, + lamports: 100 * minimum_balance_for_large_buffer, // gotta be enough for this test + }; + config.signers = vec![&online_signer]; + process_command(&config).unwrap(); + config.command = CliCommand::Airdrop { + pubkey: None, + lamports: 100 * minimum_balance_for_large_buffer, // gotta be enough for this test + }; + config.signers = vec![&offline_signer]; + process_command(&config).unwrap(); + + // Deploy upgradeable program with authority set to offline signer + config.signers = vec![&online_signer, &offline_signer, &program_signer]; + config.command = CliCommand::Program(ProgramCliCommand::Deploy { + program_location: Some(noop_path.to_str().unwrap().to_string()), + fee_payer_signer_index: 0, + program_signer_index: Some(2), + program_pubkey: Some(program_signer.pubkey()), + buffer_signer_index: None, + buffer_pubkey: None, + allow_excessive_balance: false, + upgrade_authority_signer_index: 1, // must be offline signer for security reasons + is_final: false, + max_len: Some(max_program_data_len), // allows for larger program size with future upgrades + skip_fee_check: false, + }); + config.output_format = OutputFormat::JsonCompact; + process_command(&config).unwrap(); + + // Prepare buffer to upgrade deployed program to a larger program + create_buffer_with_offline_authority( + &rpc_client, + &noop_large_path, + &mut config, + &online_signer, + &offline_signer, + &buffer_signer, + ); + + // Offline sign-only with signature over "wrong" message (with different buffer) + config.signers = vec![&offline_signer]; + let fee_payer_signer_index = if use_offline_signer_as_fee_payer { + 0 // offline signer + } else { + config.signers.push(&online_signer_identity); // can't (and won't) provide signature in --sign-only mode + 1 // online signer + }; + config.command = CliCommand::Program(ProgramCliCommand::Upgrade { + fee_payer_signer_index, + program_pubkey: program_signer.pubkey(), + buffer_pubkey: program_signer.pubkey(), // will ensure offline signature applies to wrong(different) message + upgrade_authority_signer_index: 0, + sign_only: true, + dump_transaction_message: false, + blockhash_query: BlockhashQuery::new(Some(blockhash), true, None), + }); + config.output_format = OutputFormat::JsonCompact; + let sig_response = process_command(&config).unwrap(); + let sign_only = parse_sign_only_reply_string(&sig_response); + let offline_pre_signer = sign_only.presigner_of(&offline_signer.pubkey()).unwrap(); + // Attempt to deploy from buffer using signature over wrong(different) message (should fail) + config.signers = vec![&offline_pre_signer, &program_signer]; + let fee_payer_signer_index = if use_offline_signer_as_fee_payer { + 0 // offline signer + } else { + config.signers.push(&online_signer); // can provide signature when not in --sign-only mode + 2 // online signer + }; + config.command = CliCommand::Program(ProgramCliCommand::Upgrade { + fee_payer_signer_index, + program_pubkey: program_signer.pubkey(), + buffer_pubkey: buffer_signer.pubkey(), + upgrade_authority_signer_index: 0, + sign_only: false, + dump_transaction_message: false, + blockhash_query: BlockhashQuery::new(Some(blockhash), true, None), + }); + config.output_format = OutputFormat::JsonCompact; + let error = process_command(&config).unwrap_err(); + assert_eq!(error.to_string(), "presigner error"); + + // Offline sign-only with online signer as fee payer (correct signature for program upgrade) + config.signers = vec![&offline_signer]; + let fee_payer_signer_index = if use_offline_signer_as_fee_payer { + 0 // offline signer + } else { + config.signers.push(&online_signer_identity); // can't (and won't) provide signature in --sign-only mode + 1 // online signer + }; + config.command = CliCommand::Program(ProgramCliCommand::Upgrade { + fee_payer_signer_index, + program_pubkey: program_signer.pubkey(), + buffer_pubkey: buffer_signer.pubkey(), + upgrade_authority_signer_index: 0, + sign_only: true, + dump_transaction_message: false, + blockhash_query: BlockhashQuery::new(Some(blockhash), true, None), + }); + config.output_format = OutputFormat::JsonCompact; + let sig_response = process_command(&config).unwrap(); + let sign_only = parse_sign_only_reply_string(&sig_response); + let offline_pre_signer = sign_only.presigner_of(&offline_signer.pubkey()).unwrap(); + // Attempt to deploy from buffer using signature over correct message (should succeed) + config.signers = vec![&offline_pre_signer, &program_signer]; + let fee_payer_signer_index = if use_offline_signer_as_fee_payer { + 0 // offline signer + } else { + config.signers.push(&online_signer); // can provide signature when not in --sign-only mode + 2 // online signer + }; + config.command = CliCommand::Program(ProgramCliCommand::Upgrade { + fee_payer_signer_index, + program_pubkey: program_signer.pubkey(), + buffer_pubkey: buffer_signer.pubkey(), + upgrade_authority_signer_index: 0, + sign_only: false, + dump_transaction_message: false, + blockhash_query: BlockhashQuery::new(Some(blockhash), true, None), + }); + config.output_format = OutputFormat::JsonCompact; + process_command(&config).unwrap(); + let (programdata_pubkey, _) = Pubkey::find_program_address( + &[program_signer.pubkey().as_ref()], + &bpf_loader_upgradeable::id(), + ); + let programdata_account = rpc_client.get_account(&programdata_pubkey).unwrap(); + assert_eq!( + programdata_account.lamports, + minimum_balance_for_large_buffer + ); + assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id()); + assert!(!programdata_account.executable); + assert_eq!( + programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..], + large_program_data[..] + ); +} + #[test] fn test_cli_program_show() { solana_logger::setup(); @@ -1678,3 +1880,47 @@ fn test_cli_program_dump() { assert_eq!(program_data[i], out_data[i]); } } + +fn create_buffer_with_offline_authority<'a>( + rpc_client: &RpcClient, + program_path: &Path, + config: &mut CliConfig<'a>, + online_signer: &'a Keypair, + offline_signer: &'a Keypair, + buffer_signer: &'a Keypair, +) { + // Write a buffer + config.signers = vec![online_signer, buffer_signer]; + config.command = CliCommand::Program(ProgramCliCommand::WriteBuffer { + program_location: program_path.to_str().unwrap().to_string(), + fee_payer_signer_index: 0, + buffer_signer_index: Some(1), + buffer_pubkey: Some(buffer_signer.pubkey()), + buffer_authority_signer_index: 0, + max_len: None, + skip_fee_check: false, + }); + process_command(config).unwrap(); + let buffer_account = rpc_client.get_account(&buffer_signer.pubkey()).unwrap(); + if let UpgradeableLoaderState::Buffer { authority_address } = buffer_account.state().unwrap() { + assert_eq!(authority_address, Some(online_signer.pubkey())); + } else { + panic!("not a buffer account"); + } + + // Set buffer authority to offline signer + config.signers = vec![online_signer]; + config.command = CliCommand::Program(ProgramCliCommand::SetBufferAuthority { + buffer_pubkey: buffer_signer.pubkey(), + buffer_authority_index: Some(0), + new_buffer_authority: offline_signer.pubkey(), + }); + config.output_format = OutputFormat::JsonCompact; + process_command(config).unwrap(); + let buffer_account = rpc_client.get_account(&buffer_signer.pubkey()).unwrap(); + if let UpgradeableLoaderState::Buffer { authority_address } = buffer_account.state().unwrap() { + assert_eq!(authority_address, Some(offline_signer.pubkey())); + } else { + panic!("not a buffer account"); + } +} diff --git a/docs/src/cli/examples/deploy-a-program.md b/docs/src/cli/examples/deploy-a-program.md index 1025ca6ea..b7a62837f 100644 --- a/docs/src/cli/examples/deploy-a-program.md +++ b/docs/src/cli/examples/deploy-a-program.md @@ -319,3 +319,52 @@ account and the buffer account is set to zero. The lamports from the buffer account are refunded to a spill account. Buffers also support `show` and `dump` just like programs do. + +### Upgrading program using offline signer as authority + +Some security models require separating the signing process from the transaction broadcast, such that the signing keys can be completely disconnected from any network, also known as [offline signing](offline-signing.md). + +This section describes how a program developer can use offline signing to upgrade their program, unlike the [previous section](deploy-a-program.md#redeploy-a-program), which assumes the machine is connected to the internet, aka online signing. + +Note that only the `upgrade` command can be performed in offline mode. The initial program deployment **must** be performed from an online machine, and only subsequent program upgrades can leverage offline signing. + +Assuming your program has been deployed and its upgrade authority has been changed to an +offline signer, +a typical setup would consist of 2 different signers: +- online signer (fee payer for uploading program buffer and upgrading program) +- offline signer (program upgrade authority) + +The general process is as follows: +1. (online) create buffer and write new program to it +2. (online) set buffer authority to offline signer +3. (optional, online) verify the buffer's on-chain contents +4. (offline) sign a transaction to upgrade the program +5. (online) use this signature to broadcast the upgrade transaction + +```bash +# (1) (use online machine) create buffer +solana program write-buffer + +# (2) (use online machine) set buffer authority to offline signer +solana program set-buffer-authority --new-buffer-authority +``` + +(3) (optional) You may verify that the uploaded program matches the built binary. See +[dumping a program to a file](deploy-a-program.md#dumping-a-program-to-a-file) for more information and details. + +```bash +# (4) (use offline machine) get a signature for your intent to upgrade program +solana program upgrade --sign-only --fee-payer --upgrade-authority --blockhash + +# (5) (use online machine) use this signature to build and broadcast the upgrade transaction on-chain +solana program upgrade --fee-payer --upgrade-authority --blockhash --signer : +``` +Note: +- typically, the output of the previous command(s) will contain some values useful in subsequent commands, e.g. + `--program-id`, `--buffer`, `--signer` +- you need to specify matching (or corresponding) values for params with same names (`--fee-payer`, `--program-id`, + `--upgrade-authority`, `--buffer`, `--blockhash`) in offline/online modes +- you should pre-fill every value except for `blockhash` ahead of time, and once you are ready to act - you'll need to + look up a recent `blockhash` and paste it in to generate the offline transaction signature. The `blockhash` expires + after ~60 seconds. If you didn't make it in time - just get another fresh hash and repeat until you succeed, or + consider using [durable transaction nonces](durable-nonce.md).