Keypair: implement clone() (#26248)
* Keypair: implement clone() This was not implemented upstream in ed25519-dalek to force everyone to think twice before creating another copy of a potentially sensitive private key in memory. See https://github.com/dalek-cryptography/ed25519-dalek/issues/76 However, there are now 9 instances of Keypair::from_bytes(&keypair.to_bytes()) in the solana codebase and it would be preferable to have a function. In particular since this also comes up when writing programs and can cause users to either start messing with lifetimes or discover the from_bytes() workaround themselves. This patch opts to not implement the Clone trait. This avoids automatic use in order to preserve some of the original "let developers think twice about this" intention. * Use Keypair::clone
This commit is contained in:
parent
270315a7f6
commit
cf58640937
|
@ -3522,7 +3522,7 @@ pub(crate) mod tests {
|
||||||
hash::{hash, Hash},
|
hash::{hash, Hash},
|
||||||
instruction::InstructionError,
|
instruction::InstructionError,
|
||||||
poh_config::PohConfig,
|
poh_config::PohConfig,
|
||||||
signature::{Keypair, Signer},
|
signature::{Keypair, KeypairInsecureClone, Signer},
|
||||||
system_transaction,
|
system_transaction,
|
||||||
transaction::TransactionError,
|
transaction::TransactionError,
|
||||||
},
|
},
|
||||||
|
@ -3602,7 +3602,7 @@ pub(crate) mod tests {
|
||||||
let my_pubkey = my_keypairs.node_keypair.pubkey();
|
let my_pubkey = my_keypairs.node_keypair.pubkey();
|
||||||
let cluster_info = ClusterInfo::new(
|
let cluster_info = ClusterInfo::new(
|
||||||
Node::new_localhost_with_pubkey(&my_pubkey).info,
|
Node::new_localhost_with_pubkey(&my_pubkey).info,
|
||||||
Arc::new(Keypair::from_bytes(&my_keypairs.node_keypair.to_bytes()).unwrap()),
|
Arc::new(my_keypairs.node_keypair.clone()),
|
||||||
SocketAddrSpace::Unspecified,
|
SocketAddrSpace::Unspecified,
|
||||||
);
|
);
|
||||||
assert_eq!(my_pubkey, cluster_info.id());
|
assert_eq!(my_pubkey, cluster_info.id());
|
||||||
|
|
|
@ -37,7 +37,7 @@ use {
|
||||||
message::Message,
|
message::Message,
|
||||||
poh_config::PohConfig,
|
poh_config::PohConfig,
|
||||||
pubkey::Pubkey,
|
pubkey::Pubkey,
|
||||||
signature::{Keypair, Signer},
|
signature::{Keypair, KeypairInsecureClone, Signer},
|
||||||
stake::{
|
stake::{
|
||||||
config as stake_config, instruction as stake_instruction,
|
config as stake_config, instruction as stake_instruction,
|
||||||
state::{Authorized, Lockup},
|
state::{Authorized, Lockup},
|
||||||
|
@ -55,6 +55,7 @@ use {
|
||||||
collections::HashMap,
|
collections::HashMap,
|
||||||
io::{Error, ErrorKind, Result},
|
io::{Error, ErrorKind, Result},
|
||||||
iter,
|
iter,
|
||||||
|
ops::Deref,
|
||||||
path::{Path, PathBuf},
|
path::{Path, PathBuf},
|
||||||
sync::{Arc, RwLock},
|
sync::{Arc, RwLock},
|
||||||
},
|
},
|
||||||
|
@ -203,10 +204,8 @@ impl LocalCluster {
|
||||||
if *in_genesis {
|
if *in_genesis {
|
||||||
Some((
|
Some((
|
||||||
ValidatorVoteKeypairs {
|
ValidatorVoteKeypairs {
|
||||||
node_keypair: Keypair::from_bytes(&node_keypair.to_bytes())
|
node_keypair: node_keypair.deref().clone(),
|
||||||
.unwrap(),
|
vote_keypair: vote_keypair.deref().clone(),
|
||||||
vote_keypair: Keypair::from_bytes(&vote_keypair.to_bytes())
|
|
||||||
.unwrap(),
|
|
||||||
stake_keypair: Keypair::new(),
|
stake_keypair: Keypair::new(),
|
||||||
},
|
},
|
||||||
stake,
|
stake,
|
||||||
|
@ -265,9 +264,8 @@ impl LocalCluster {
|
||||||
let mut leader_config = safe_clone_config(&config.validator_configs[0]);
|
let mut leader_config = safe_clone_config(&config.validator_configs[0]);
|
||||||
leader_config.rpc_addrs = Some((leader_node.info.rpc, leader_node.info.rpc_pubsub));
|
leader_config.rpc_addrs = Some((leader_node.info.rpc, leader_node.info.rpc_pubsub));
|
||||||
Self::sync_ledger_path_across_nested_config_fields(&mut leader_config, &leader_ledger_path);
|
Self::sync_ledger_path_across_nested_config_fields(&mut leader_config, &leader_ledger_path);
|
||||||
let leader_keypair = Arc::new(Keypair::from_bytes(&leader_keypair.to_bytes()).unwrap());
|
let leader_keypair = Arc::new(leader_keypair.clone());
|
||||||
let leader_vote_keypair =
|
let leader_vote_keypair = Arc::new(leader_vote_keypair.clone());
|
||||||
Arc::new(Keypair::from_bytes(&leader_vote_keypair.to_bytes()).unwrap());
|
|
||||||
|
|
||||||
let leader_server = Validator::new(
|
let leader_server = Validator::new(
|
||||||
leader_node,
|
leader_node,
|
||||||
|
@ -315,7 +313,7 @@ impl LocalCluster {
|
||||||
.map(|keypairs| {
|
.map(|keypairs| {
|
||||||
(
|
(
|
||||||
keypairs.node_keypair.pubkey(),
|
keypairs.node_keypair.pubkey(),
|
||||||
Arc::new(Keypair::from_bytes(&keypairs.vote_keypair.to_bytes()).unwrap()),
|
Arc::new(keypairs.vote_keypair.clone()),
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
.collect();
|
.collect();
|
||||||
|
|
|
@ -29,12 +29,13 @@ use {
|
||||||
clock::{Slot, MAX_PROCESSING_AGE},
|
clock::{Slot, MAX_PROCESSING_AGE},
|
||||||
hash::Hash,
|
hash::Hash,
|
||||||
pubkey::Pubkey,
|
pubkey::Pubkey,
|
||||||
signature::{Keypair, Signer},
|
signature::{KeypairInsecureClone, Signer},
|
||||||
},
|
},
|
||||||
solana_streamer::socket::SocketAddrSpace,
|
solana_streamer::socket::SocketAddrSpace,
|
||||||
solana_vote_program::{vote_state::MAX_LOCKOUT_HISTORY, vote_transaction},
|
solana_vote_program::{vote_state::MAX_LOCKOUT_HISTORY, vote_transaction},
|
||||||
std::{
|
std::{
|
||||||
collections::{BTreeSet, HashSet},
|
collections::{BTreeSet, HashSet},
|
||||||
|
ops::Deref,
|
||||||
path::Path,
|
path::Path,
|
||||||
sync::{
|
sync::{
|
||||||
atomic::{AtomicBool, Ordering},
|
atomic::{AtomicBool, Ordering},
|
||||||
|
@ -462,7 +463,7 @@ fn test_duplicate_shreds_broadcast_leader() {
|
||||||
let (gossip_service, _tcp_listener, cluster_info) = gossip_service::make_gossip_node(
|
let (gossip_service, _tcp_listener, cluster_info) = gossip_service::make_gossip_node(
|
||||||
// Need to use our validator's keypair to gossip EpochSlots and votes for our
|
// Need to use our validator's keypair to gossip EpochSlots and votes for our
|
||||||
// node later.
|
// node later.
|
||||||
Keypair::from_bytes(&node_keypair.to_bytes()).unwrap(),
|
node_keypair.deref().clone(),
|
||||||
Some(&cluster.entry_point_info.gossip),
|
Some(&cluster.entry_point_info.gossip),
|
||||||
&exit,
|
&exit,
|
||||||
None,
|
None,
|
||||||
|
|
|
@ -7,7 +7,7 @@ use {
|
||||||
genesis_config::{ClusterType, GenesisConfig},
|
genesis_config::{ClusterType, GenesisConfig},
|
||||||
pubkey::Pubkey,
|
pubkey::Pubkey,
|
||||||
rent::Rent,
|
rent::Rent,
|
||||||
signature::{Keypair, Signer},
|
signature::{Keypair, KeypairInsecureClone, Signer},
|
||||||
stake::state::StakeState,
|
stake::state::StakeState,
|
||||||
system_program,
|
system_program,
|
||||||
},
|
},
|
||||||
|
@ -109,8 +109,7 @@ pub fn create_genesis_config_with_vote_accounts_and_cluster_type(
|
||||||
assert_eq!(voting_keypairs.len(), stakes.len());
|
assert_eq!(voting_keypairs.len(), stakes.len());
|
||||||
|
|
||||||
let mint_keypair = Keypair::new();
|
let mint_keypair = Keypair::new();
|
||||||
let voting_keypair =
|
let voting_keypair = voting_keypairs[0].borrow().vote_keypair.clone();
|
||||||
Keypair::from_bytes(&voting_keypairs[0].borrow().vote_keypair.to_bytes()).unwrap();
|
|
||||||
|
|
||||||
let validator_pubkey = voting_keypairs[0].borrow().node_keypair.pubkey();
|
let validator_pubkey = voting_keypairs[0].borrow().node_keypair.pubkey();
|
||||||
let genesis_config = create_genesis_config_with_leader_ex(
|
let genesis_config = create_genesis_config_with_leader_ex(
|
||||||
|
|
|
@ -97,6 +97,26 @@ where
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Allows Keypair cloning
|
||||||
|
///
|
||||||
|
/// Note that the `Clone` trait is intentionally unimplemented because making a
|
||||||
|
/// second copy of sensitive secret keys in memory is usually a bad idea.
|
||||||
|
///
|
||||||
|
/// Only use this in tests or when strictly required.
|
||||||
|
pub trait KeypairInsecureClone {
|
||||||
|
fn clone(&self) -> Self;
|
||||||
|
}
|
||||||
|
|
||||||
|
impl KeypairInsecureClone for Keypair {
|
||||||
|
fn clone(&self) -> Self {
|
||||||
|
Self(ed25519_dalek::Keypair {
|
||||||
|
// This will never error since self is a valid keypair
|
||||||
|
secret: ed25519_dalek::SecretKey::from_bytes(self.0.secret.as_bytes()).unwrap(),
|
||||||
|
public: self.0.public,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Reads a JSON-encoded `Keypair` from a `Reader` implementor
|
/// Reads a JSON-encoded `Keypair` from a `Reader` implementor
|
||||||
pub fn read_keypair<R: Read>(reader: &mut R) -> Result<Keypair, Box<dyn error::Error>> {
|
pub fn read_keypair<R: Read>(reader: &mut R) -> Result<Keypair, Box<dyn error::Error>> {
|
||||||
let bytes: Vec<u8> = serde_json::from_reader(reader)?;
|
let bytes: Vec<u8> = serde_json::from_reader(reader)?;
|
||||||
|
|
Loading…
Reference in New Issue