Skip to content

Commit

Permalink
Keypair: implement clone()
Browse files Browse the repository at this point in the history
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 dalek-cryptography/ed25519-dalek#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.
  • Loading branch information
ckamm committed Jun 28, 2022
1 parent 662818e commit 1f6958c
Showing 1 changed file with 20 additions and 0 deletions.
20 changes: 20 additions & 0 deletions sdk/src/signer/keypair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.clone(),
})
}
}

/// Reads a JSON-encoded `Keypair` from a `Reader` implementor
pub fn read_keypair<R: Read>(reader: &mut R) -> Result<Keypair, Box<dyn error::Error>> {
let bytes: Vec<u8> = serde_json::from_reader(reader)?;
Expand Down

0 comments on commit 1f6958c

Please sign in to comment.