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.

cli/src/cli.rs

@@ -292,7 +292,7 @@ pub enum CliCommand {
         seed: Option<String>,
         identity_account: SignerIndex,
         authorized_voter: Option<Pubkey>,
-        authorized_withdrawer: Option<Pubkey>,
+        authorized_withdrawer: Pubkey,
         commission: u8,
         memo: Option<String>,
     },
@@ -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,
         };

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")
+                    Arg::with_name("allow_unsafe_authorized_withdrawer")
-                        .long("authorized-withdrawer")
+                        .long("allow-unsafe-authorized-withdrawer")
-                        .value_name("WITHDRAWER_PUBKEY"),
+                        .takes_value(false)
-                        "Public key of the authorized withdrawer [default: validator identity pubkey]. "),
+                        .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<String>,
     identity_account: SignerIndex,
     authorized_voter: &Option<Pubkey>,
-    authorized_withdrawer: &Option<Pubkey>,
+    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",
+            &identity_keypair_file,
-            &authed.to_string(),
+            "--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,
                 },

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,
     };

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,
     };

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

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
+Because theft of a authorized withdrawer keypair can give complete control over
-the withdraw authority keypair in an offline/cold wallet, as it is
+the operation of a validator to an attacker, is is advised to keep the withdraw
-not needed to sign frequent transactions.
+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
+The authorized withdrawer must be set when the vote account is created.  It must
-`--authorized-withdrawer` option. If this is not provided, the validator
+not be set to a keypair that is the same as either the validator identity
-identity will be set as the withdraw authority by default.
+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
 

multinode-demo/validator.sh

@@ -78,6 +78,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")
@@ -203,6 +206,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
@@ -300,7 +306,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"
 
@@ -315,6 +321,13 @@ rpc_url=$($solana_gossip $maybe_allow_private_addr rpc-url --timeout 180 --entry
 
 [[ -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"