From afb87a386ab24e97e78116ac7688bc52b53bdf6b Mon Sep 17 00:00:00 2001 From: Bryan Ischo Date: Thu, 2 Sep 2021 17:22:33 -0700 Subject: [PATCH] Removed the --authorized-withdrawer argument from create-vote-account The parameter is now a required third argument. This is because authorized withdrawer should never be the same as vote account keypair or validator identity keypair for security reasons. Added a --allow-unsafe-authorized-withdrawer to override this restriction if necessary. (cherry picked from commit e288459cf28693597b9aba7c7b83ca1780b127fc) --- cli/src/cli.rs | 8 +-- cli/src/vote.rs | 62 ++++++++++++++----- cli/tests/stake.rs | 3 +- cli/tests/vote.rs | 2 +- docs/src/running-validator/validator-start.md | 21 ++++++- docs/src/running-validator/vote-accounts.md | 32 +++++----- multinode-demo/validator.sh | 15 ++++- 7 files changed, 106 insertions(+), 37 deletions(-) diff --git a/cli/src/cli.rs b/cli/src/cli.rs index 8a0b58928c..b77321bd15 100644 --- a/cli/src/cli.rs +++ b/cli/src/cli.rs @@ -292,7 +292,7 @@ pub enum CliCommand { seed: Option, identity_account: SignerIndex, authorized_voter: Option, - authorized_withdrawer: Option, + authorized_withdrawer: Pubkey, commission: u8, memo: Option, }, @@ -1379,7 +1379,7 @@ pub fn process_command(config: &CliConfig) -> ProcessResult { seed, *identity_account, authorized_voter, - authorized_withdrawer, + *authorized_withdrawer, *commission, memo.as_ref(), ), @@ -1943,7 +1943,7 @@ mod tests { seed: None, identity_account: 2, authorized_voter: Some(bob_pubkey), - authorized_withdrawer: Some(bob_pubkey), + authorized_withdrawer: bob_pubkey, commission: 0, memo: None, }; @@ -2144,7 +2144,7 @@ mod tests { seed: None, identity_account: 2, authorized_voter: Some(bob_pubkey), - authorized_withdrawer: Some(bob_pubkey), + authorized_withdrawer: bob_pubkey, commission: 0, memo: None, }; diff --git a/cli/src/vote.rs b/cli/src/vote.rs index ee81929d07..264d96bc7f 100644 --- a/cli/src/vote.rs +++ b/cli/src/vote.rs @@ -55,6 +55,15 @@ impl VoteSubCommands for App<'_, '_> { .validator(is_valid_signer) .help("Keypair of validator that will vote with this account"), ) + .arg( + pubkey!(Arg::with_name("authorized_withdrawer") + .index(3) + .value_name("WITHDRAWER_PUBKEY") + .takes_value(true) + .required(true) + .long("authorized-withdrawer"), + "Public key of the authorized withdrawer") + ) .arg( Arg::with_name("commission") .long("commission") @@ -70,10 +79,12 @@ impl VoteSubCommands for App<'_, '_> { "Public key of the authorized voter [default: validator identity pubkey]. "), ) .arg( - pubkey!(Arg::with_name("authorized_withdrawer") - .long("authorized-withdrawer") - .value_name("WITHDRAWER_PUBKEY"), - "Public key of the authorized withdrawer [default: validator identity pubkey]. "), + Arg::with_name("allow_unsafe_authorized_withdrawer") + .long("allow-unsafe-authorized-withdrawer") + .takes_value(false) + .help("Allow an authorized withdrawer pubkey to be identical to the validator identity \ + account pubkey or vote account pubkey, which is normally an unsafe \ + configuration and should be avoided."), ) .arg( Arg::with_name("seed") @@ -340,9 +351,28 @@ pub fn parse_create_vote_account( signer_of(matches, "identity_account", wallet_manager)?; let commission = value_t_or_exit!(matches, "commission", u8); let authorized_voter = pubkey_of_signer(matches, "authorized_voter", wallet_manager)?; - let authorized_withdrawer = pubkey_of_signer(matches, "authorized_withdrawer", wallet_manager)?; + let authorized_withdrawer = + pubkey_of_signer(matches, "authorized_withdrawer", wallet_manager)?.unwrap(); + let allow_unsafe = matches.is_present("allow_unsafe_authorized_withdrawer"); let memo = matches.value_of(MEMO_ARG.name).map(String::from); + if !allow_unsafe { + if authorized_withdrawer == vote_account_pubkey.unwrap() { + return Err(CliError::BadParameter( + "Authorized withdrawer pubkey is identical to vote \ + account pubkey, an unsafe configuration" + .to_owned(), + )); + } + if authorized_withdrawer == identity_pubkey.unwrap() { + return Err(CliError::BadParameter( + "Authorized withdrawer pubkey is identical to identity \ + account pubkey, an unsafe configuration" + .to_owned(), + )); + } + } + let payer_provided = None; let signer_info = default_signer.generate_unique_signers( vec![payer_provided, vote_account, identity_account], @@ -531,7 +561,7 @@ pub fn process_create_vote_account( seed: &Option, identity_account: SignerIndex, authorized_voter: &Option, - authorized_withdrawer: &Option, + authorized_withdrawer: Pubkey, commission: u8, memo: Option<&String>, ) -> ProcessResult { @@ -563,7 +593,7 @@ pub fn process_create_vote_account( let vote_init = VoteInit { node_pubkey: identity_pubkey, authorized_voter: authorized_voter.unwrap_or(identity_pubkey), - authorized_withdrawer: authorized_withdrawer.unwrap_or(identity_pubkey), + authorized_withdrawer, commission, }; @@ -1025,12 +1055,14 @@ mod tests { // Test CreateVoteAccount SubCommand let (identity_keypair_file, mut tmp_file) = make_tmp_file(); let identity_keypair = Keypair::new(); + let authorized_withdrawer = Keypair::new().pubkey(); write_keypair(&identity_keypair, tmp_file.as_file_mut()).unwrap(); let test_create_vote_account = test_commands.clone().get_matches_from(vec![ "test", "create-vote-account", &keypair_file, &identity_keypair_file, + &authorized_withdrawer.to_string(), "--commission", "10", ]); @@ -1042,7 +1074,7 @@ mod tests { seed: None, identity_account: 2, authorized_voter: None, - authorized_withdrawer: None, + authorized_withdrawer, commission: 10, memo: None, }, @@ -1063,6 +1095,7 @@ mod tests { "create-vote-account", &keypair_file, &identity_keypair_file, + &authorized_withdrawer.to_string(), ]); assert_eq!( parse_command(&test_create_vote_account2, &default_signer, &mut None).unwrap(), @@ -1072,7 +1105,7 @@ mod tests { seed: None, identity_account: 2, authorized_voter: None, - authorized_withdrawer: None, + authorized_withdrawer, commission: 100, memo: None, }, @@ -1095,6 +1128,7 @@ mod tests { "create-vote-account", &keypair_file, &identity_keypair_file, + &authorized_withdrawer.to_string(), "--authorized-voter", &authed.to_string(), ]); @@ -1106,7 +1140,7 @@ mod tests { seed: None, identity_account: 2, authorized_voter: Some(authed), - authorized_withdrawer: None, + authorized_withdrawer, commission: 100, memo: None, }, @@ -1121,14 +1155,14 @@ mod tests { let (keypair_file, mut tmp_file) = make_tmp_file(); let keypair = Keypair::new(); write_keypair(&keypair, tmp_file.as_file_mut()).unwrap(); - // test init with an authed withdrawer + // succeed even though withdrawer unsafe (because forcefully allowed) let test_create_vote_account4 = test_commands.clone().get_matches_from(vec![ "test", "create-vote-account", &keypair_file, &identity_keypair_file, - "--authorized-withdrawer", - &authed.to_string(), + &identity_keypair_file, + "--allow-unsafe-authorized-withdrawer", ]); assert_eq!( parse_command(&test_create_vote_account4, &default_signer, &mut None).unwrap(), @@ -1138,7 +1172,7 @@ mod tests { seed: None, identity_account: 2, authorized_voter: None, - authorized_withdrawer: Some(authed), + authorized_withdrawer: identity_keypair.pubkey(), commission: 100, memo: None, }, diff --git a/cli/tests/stake.rs b/cli/tests/stake.rs index 5e41030a53..57d96dae37 100644 --- a/cli/tests/stake.rs +++ b/cli/tests/stake.rs @@ -30,6 +30,7 @@ use solana_streamer::socket::SocketAddrSpace; fn test_stake_delegation_force() { let mint_keypair = Keypair::new(); let mint_pubkey = mint_keypair.pubkey(); + let authorized_withdrawer = Keypair::new().pubkey(); let faucet_addr = run_local_faucet(mint_keypair, None); let test_validator = TestValidator::with_no_fees(mint_pubkey, Some(faucet_addr), SocketAddrSpace::Unspecified); @@ -53,7 +54,7 @@ fn test_stake_delegation_force() { seed: None, identity_account: 0, authorized_voter: None, - authorized_withdrawer: None, + authorized_withdrawer, commission: 0, memo: None, }; diff --git a/cli/tests/vote.rs b/cli/tests/vote.rs index 6026bdde35..49336d9cb2 100644 --- a/cli/tests/vote.rs +++ b/cli/tests/vote.rs @@ -45,7 +45,7 @@ fn test_vote_authorize_and_withdraw() { seed: None, identity_account: 0, authorized_voter: None, - authorized_withdrawer: Some(config.signers[0].pubkey()), + authorized_withdrawer: config.signers[0].pubkey(), commission: 0, memo: None, }; diff --git a/docs/src/running-validator/validator-start.md b/docs/src/running-validator/validator-start.md index b25df95738..3aafa06687 100644 --- a/docs/src/running-validator/validator-start.md +++ b/docs/src/running-validator/validator-start.md @@ -239,6 +239,23 @@ solana balance --lamports Read more about the [difference between SOL and lamports here](../introduction.md#what-are-sols). +## Create Authorized Withdrawer Account + +If you haven't already done so, create an authorized-withdrawer keypair to be used +as the ultimate authority over your validator. This keypair will have the +authority to withdraw from your vote account, and will have the additional +authority to change all other aspects of your vote account. Needless to say, +this is a very important keypair as anyone who possesses it can make any +changes to your vote account, including taking ownership of it permanently. +So it is very important to keep your authorized-withdrawer keypair in a safe +location. It does not need to be stored on your validator, and should not be +stored anywhere from where it could be accessed by unauthorized parties. To +create your authorized-withdrawer keypair: + +```bash +solana-keygen new -o ~/authorized-withdrawer-keypair.json +``` + ## Create Vote Account If you haven’t already done so, create a vote-account keypair and create the @@ -253,9 +270,11 @@ The following command can be used to create your vote account on the blockchain with all the default options: ```bash -solana create-vote-account ~/vote-account-keypair.json ~/validator-keypair.json +solana create-vote-account ~/vote-account-keypair.json ~/validator-keypair.json ~/authorized-withdrawer-keypair.json ``` +Remember to move your authorized withdrawer keypair into a very secure location after running the above command. + Read more about [creating and managing a vote account](vote-accounts.md). ## Known validators diff --git a/docs/src/running-validator/vote-accounts.md b/docs/src/running-validator/vote-accounts.md index f948abc95a..7af56cc71d 100644 --- a/docs/src/running-validator/vote-accounts.md +++ b/docs/src/running-validator/vote-accounts.md @@ -20,7 +20,7 @@ of the account. [vote-update-validator](../cli/usage.md#solana-vote-update-validator). - To change the [vote authority](#vote-authority), use [vote-authorize-voter](../cli/usage.md#solana-vote-authorize-voter). -- To change the [withdraw authority](#withdraw-authority), use +- To change the [authorized withdrawer](#authorized-withdrawer), use [vote-authorize-withdrawer](../cli/usage.md#solana-vote-authorize-withdrawer). - To change the [commission](#commission), use [vote-update-commission](../cli/usage.md#solana-vote-update-commission). @@ -95,26 +95,28 @@ multiple times. This allows the validator process to keep voting successfully when the network reaches an epoch boundary at which the validator's vote authority account changes. -### Withdraw Authority +### Authorized Withdrawer -The _withdraw authority_ keypair is used to withdraw funds from a vote account +The _authorized withdrawer_ keypair is used to withdraw funds from a vote account using the [withdraw-from-vote-account](../cli/usage.md#solana-withdraw-from-vote-account) command. Any network rewards a validator earns are deposited into the vote -account and are only retrievable by signing with the withdraw authority keypair. +account and are only retrievable by signing with the authorized withdrawer keypair. -The withdraw authority is also required to sign any transaction to change +The authorized withdrawer is also required to sign any transaction to change a vote account's [commission](#commission), and to change the validator identity on a vote account. -Because the vote account could accrue a significant balance, consider keeping -the withdraw authority keypair in an offline/cold wallet, as it is -not needed to sign frequent transactions. +Because theft of a authorized withdrawer keypair can give complete control over +the operation of a validator to an attacker, is is advised to keep the withdraw +authority keypair in an offline/cold wallet in a secure location. The withdraw +authority keypair is not needed during operation of a validator and should not +stored on the validator itself. -The withdraw authority can be set at vote account creation with the -`--authorized-withdrawer` option. If this is not provided, the validator -identity will be set as the withdraw authority by default. +The authorized withdrawer must be set when the vote account is created. It must +not be set to a keypair that is the same as either the validator identity +keypair or the vote authority keypair. -The withdraw authority can be changed later with the +The authorized withdrawer can be changed later with the [vote-authorize-withdrawer](../cli/usage.md#solana-vote-authorize-withdrawer) command. @@ -155,13 +157,13 @@ with a live validator. ### Vote Account Validator Identity -You will need access to the _withdraw authority_ keypair for the vote account to +You will need access to the _authorized withdrawer_ keypair for the vote account to change the validator identity. The follow steps assume that -`~/withdraw-authority.json` is that keypair. +`~/authorized_withdrawer.json` is that keypair. 1. Create the new validator identity keypair, `solana-keygen new -o ~/new-validator-keypair.json`. 2. Ensure that the new identity account has been funded, `solana transfer ~/new-validator-keypair.json 500`. -3. Run `solana vote-update-validator ~/vote-account-keypair.json ~/new-validator-keypair.json ~/withdraw-authority.json` +3. Run `solana vote-update-validator ~/vote-account-keypair.json ~/new-validator-keypair.json ~/authorized_withdrawer.json` to modify the validator identity in your vote account 4. Restart your validator with the new identity keypair for the `--identity` argument diff --git a/multinode-demo/validator.sh b/multinode-demo/validator.sh index bb596ee67c..ade5eb6fc4 100755 --- a/multinode-demo/validator.sh +++ b/multinode-demo/validator.sh @@ -77,6 +77,9 @@ while [[ -n $1 ]]; do elif [[ $1 = --authorized-voter ]]; then args+=("$1" "$2") shift 2 + elif [[ $1 = --authorized-withdrawer ]]; then + authorized_withdrawer_pubkey=$2 + shift 2 elif [[ $1 = --vote-account ]]; then vote_account=$2 args+=("$1" "$2") @@ -198,6 +201,9 @@ if [[ -n $REQUIRE_KEYPAIRS ]]; then if [[ -z $vote_account ]]; then usage "Error: --vote-account not specified" fi + if [[ -z $authorized_withdrawer_pubkey ]]; then + usage "Error: --authorized_withdrawer not specified" + fi fi if [[ -z "$ledger_dir" ]]; then @@ -295,7 +301,7 @@ setup_validator_accounts() { fi echo "Creating validator vote account" - wallet create-vote-account "$vote_account" "$identity" || return $? + wallet create-vote-account "$vote_account" "$identity" "$authorized_withdrawer" || return $? fi echo "Validator vote account configured" @@ -309,6 +315,13 @@ rpc_url=$($solana_gossip rpc-url --timeout 180 --entrypoint "$gossip_entrypoint" [[ -r "$identity" ]] || $solana_keygen new --no-passphrase -so "$identity" [[ -r "$vote_account" ]] || $solana_keygen new --no-passphrase -so "$vote_account" +if [ -z "$authorized_withdrawer_pubkey" ]; then + authorized_withdrawer_file=$ledger_dir/authorized-withdrawer.json + [[ -r "$authorized_withdrawer_file" ]] || $solana_keygen new --no-passphrase -so "$authorized_withdrawer_file"; + authorized_withdrawer=$authorized_withdrawer_file +else + authorized_withdrawer=$authorized_withdrawer_pubkey +fi setup_validator_accounts "$node_sol"