Clear every copy of private key material the crate can reach

The earlier change cleared the symmetric keys. This one covers the rest, and
covers it by enumerating where key bytes actually live rather than by pattern,
because three successive passes each cleared one place and missed another.

The handshake state and the identity now clear their keypairs on drop. That
matters more than the stack copies already handled: the ephemeral key was
being wiped in two short-lived locals and then stored in a field that outlived
both. These types are ordinary structs, so they can carry a drop even though
the keypair inside them cannot.

Also cleared: the temporary each of the fourteen elliptic-curve calls makes
from a keypair, the by-value keypair parameters, the identity generation and
parsing paths, the encoded secret strings, and the private key as it passes
through configuration. The config file text is treated as secret for as long
as it is held, since the key can be written straight into it.

Two places assign over the configured key rather than dropping the struct that
holds it. Assignment frees the old string without running the drop, so both
now clear it first.

What is deliberately not cleared, and why: the hash and key-derivation states,
and the cached cipher keys inside ring. None of those crates offers a clearing
route at the versions we pin, which I checked in their sources rather than
assuming, and reaching for unsafe here was not worth it for a residue that
needs local memory access to read.

One limitation is worth stating plainly. These key types are copyable, so the
compiler may duplicate them where we cannot see. This clears the copies the
crate owns, not every copy that ever existed. The drops also have no test:
reading a dropped struct's bytes means reading freed memory.
This commit is contained in:
Johnathan Corgan
2026-08-16 16:34:40 +00:00
parent ae787b9bbd
commit ee1c624cef
15 changed files with 405 additions and 84 deletions
+11 -2
View File
@@ -10,6 +10,7 @@ use fips::{Config, Node};
use std::path::PathBuf;
use tracing::{debug, error, info, warn};
use tracing_subscriber::{EnvFilter, fmt};
use zeroize::Zeroize;
/// FIPS mesh network daemon
#[derive(Parser, Debug)]
@@ -126,7 +127,7 @@ async fn run_daemon(
fips::node::warn_on_legacy_config_paths();
// Identity provisioning: config nsec > key file > generate ephemeral
let resolved = match resolve_identity(&config, &loaded_paths) {
let mut resolved = match resolve_identity(&config, &loaded_paths) {
Ok(r) => r,
Err(e) => {
error!("Failed to resolve identity: {}", e);
@@ -146,7 +147,15 @@ async fn run_daemon(
// Create node with resolved identity
let mut config = config;
config.node.identity.nsec = Some(resolved.nsec);
// Take the nsec rather than move it: `ResolvedIdentity` clears its copy
// on drop, so it cannot be left partially moved. Clear whatever the field
// already held first — assigning over it drops the old `String` in place,
// which does not run `Drop for IdentityConfig`, so a key that came from
// the config file would be freed uncleared.
if let Some(mut old) = config.node.identity.nsec.take() {
old.zeroize();
}
config.node.identity.nsec = Some(std::mem::take(&mut resolved.nsec));
debug!("Creating node");
let mut node = match Node::new(config) {
Ok(node) => node,
+10 -2
View File
@@ -15,6 +15,7 @@ use std::io::{BufRead, BufReader, Write};
use std::net::{Ipv6Addr, SocketAddrV6};
use std::path::{Path, PathBuf};
use std::time::Duration;
use zeroize::Zeroizing;
/// FIPS control client
#[derive(Parser, Debug)]
@@ -382,11 +383,18 @@ fn main() {
// Commands that don't require a running daemon
if let Commands::Keygen { dir, force, stdout } = &cli.command {
let identity = Identity::generate();
let nsec = encode_nsec(&identity.keypair().secret_key());
// `keypair()` and `secret_key()` each hand back a whole private key
// rather than a handle, so both temporaries are bound and erased. The
// nsec is that same key in another encoding, so it is guarded too.
let mut our_keypair = identity.keypair();
let mut secret_key = our_keypair.secret_key();
let nsec = Zeroizing::new(encode_nsec(&secret_key));
secret_key.non_secure_erase();
our_keypair.non_secure_erase();
let npub = identity.npub();
if *stdout {
println!("{nsec}");
println!("{}", nsec.as_str());
println!("{npub}");
return;
}
+79 -13
View File
@@ -32,6 +32,7 @@ use crate::{Identity, IdentityError};
use serde::{Deserialize, Serialize};
use std::path::{Path, PathBuf};
use thiserror::Error;
use zeroize::{Zeroize, Zeroizing};
#[cfg(target_os = "linux")]
pub use gateway::{ConntrackConfig, GatewayConfig, GatewayDnsConfig, PortForward, Proto};
@@ -217,11 +218,16 @@ pub fn default_gateway_path() -> PathBuf {
}
/// Read a bare bech32 nsec from a key file.
///
/// The file contents are the private key, and trimming copies it into a
/// second string, so the read buffer is cleared on every exit path rather
/// than dropped as it stands. The returned nsec is the caller's.
pub fn read_key_file(path: &Path) -> Result<String, ConfigError> {
let contents = std::fs::read_to_string(path).map_err(|e| ConfigError::ReadFile {
path: path.to_path_buf(),
source: e,
})?;
let contents = Zeroizing::new(contents);
let nsec = contents.trim().to_string();
if nsec.is_empty() {
return Err(ConfigError::EmptyKeyFile {
@@ -442,7 +448,9 @@ pub fn resolve_identity(
if config.node.identity.persistent {
// Persistent mode: load existing key file or generate-and-persist
if key_path.exists() {
let nsec = read_key_file(&key_path)?;
// Held in a guard, not a bare `String`: if the parse below fails,
// the `?` returns and a bare local would be freed uncleared.
let nsec = Zeroizing::new(read_key_file(&key_path)?);
let identity = Identity::from_secret_str(&nsec)?;
warn_unmanaged_key_file(&key_path);
if let Err(e) = write_pub_file(&pub_path, &identity.npub()) {
@@ -453,7 +461,7 @@ pub fn resolve_identity(
);
}
return Ok(ResolvedIdentity {
nsec,
nsec: nsec.to_string(),
source: IdentitySource::KeyFile(key_path),
});
}
@@ -467,7 +475,8 @@ pub fn resolve_identity(
Path::new(SYSTEM_CONFIG_DIR),
Path::new(LEGACY_SYSTEM_CONFIG_DIR),
) {
let nsec = read_key_file(&legacy)?;
// Guarded for the same reason as the current-path read above.
let nsec = Zeroizing::new(read_key_file(&legacy)?);
let identity = Identity::from_secret_str(&nsec)?;
tracing::warn!(
legacy = %legacy.display(),
@@ -484,14 +493,20 @@ pub fn resolve_identity(
);
}
return Ok(ResolvedIdentity {
nsec,
nsec: nsec.to_string(),
source: IdentitySource::KeyFile(legacy),
});
}
// No key file anywhere — generate and persist
let identity = Identity::generate();
let nsec = encode_nsec(&identity.keypair().secret_key());
// `keypair()` and `secret_key()` each hand back a whole private key
// rather than a handle, so both temporaries are bound and erased.
let mut our_keypair = identity.keypair();
let mut secret_key = our_keypair.secret_key();
let nsec = encode_nsec(&secret_key);
secret_key.non_secure_erase();
our_keypair.non_secure_erase();
let npub = identity.npub();
if let Some(parent) = key_path.parent() {
@@ -529,7 +544,13 @@ pub fn resolve_identity(
// Ephemeral mode (default): fresh keypair every start, write key files
// for operator visibility
let identity = Identity::generate();
let nsec = encode_nsec(&identity.keypair().secret_key());
// `keypair()` and `secret_key()` each hand back a whole private key
// rather than a handle, so both temporaries are bound and erased.
let mut our_keypair = identity.keypair();
let mut secret_key = our_keypair.secret_key();
let nsec = encode_nsec(&secret_key);
secret_key.non_secure_erase();
our_keypair.non_secure_erase();
let npub = identity.npub();
if let Some(parent) = key_path.parent() {
@@ -571,6 +592,14 @@ pub fn resolve_identity(
}
/// Result of identity resolution.
///
/// `nsec` is the node's private key in plaintext. Every local that carries it
/// through [`resolve_identity`] either moves into this struct or is held in a
/// guard that clears it, so this is where the surviving string lives and where
/// clearing it belongs.
/// A caller that wants the value out should take it with [`Option::take`] or
/// `std::mem::take` rather than moving the field, which the `Drop` below
/// forbids.
pub struct ResolvedIdentity {
/// The nsec string (bech32 or hex) for creating an Identity.
pub nsec: String,
@@ -578,6 +607,14 @@ pub struct ResolvedIdentity {
pub source: IdentitySource,
}
impl Drop for ResolvedIdentity {
/// Clear the plaintext private key rather than dropping the allocation
/// with the key still in it.
fn drop(&mut self) {
self.nsec.zeroize();
}
}
/// Where a resolved identity originated.
pub enum IdentitySource {
/// From explicit nsec in config file.
@@ -639,6 +676,20 @@ pub struct IdentityConfig {
pub persistent: bool,
}
impl Drop for IdentityConfig {
/// Clear the plaintext private key.
///
/// This field holds the node's private key for the whole process
/// lifetime, which is the longest any secret lives in this crate, so
/// leaving the allocation to be freed with the key still in it is the
/// largest residue the crate can reach. A caller that needs the value out
/// should take it with [`Option::take`]; moving the field is what the
/// `Drop` forbids.
fn drop(&mut self) {
self.nsec.zeroize();
}
}
/// Root configuration structure.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct Config {
@@ -709,10 +760,16 @@ impl Config {
/// Load configuration from a single file.
pub fn load_file(path: &Path) -> Result<Self, ConfigError> {
let contents = std::fs::read_to_string(path).map_err(|e| ConfigError::ReadFile {
path: path.to_path_buf(),
source: e,
})?;
// The config file is the highest-priority home of a plaintext key:
// `node.identity.nsec` is read straight out of it, so the whole file
// text is treated as secret for as long as it is held.
let contents =
Zeroizing::new(
std::fs::read_to_string(path).map_err(|e| ConfigError::ReadFile {
path: path.to_path_buf(),
source: e,
})?,
);
serde_yaml::from_str(&contents).map_err(|e| ConfigError::ParseYaml {
path: path.to_path_buf(),
@@ -755,10 +812,19 @@ impl Config {
/// Merge another configuration into this one.
///
/// Values from `other` override values in `self` when present.
pub fn merge(&mut self, other: Config) {
// Merge node.identity section
pub fn merge(&mut self, mut other: Config) {
// Merge node.identity section. The nsec is taken rather than moved
// out of `other.node.identity`, which clears its private key on drop
// and so cannot be left partially moved.
if other.node.identity.nsec.is_some() {
self.node.identity.nsec = other.node.identity.nsec;
// Clear whatever this field already held before overwriting it.
// Assigning over the field drops the old `String` in place, which
// does not run `Drop for IdentityConfig` and would free a
// plaintext key uncleared when two config files both carry one.
if let Some(mut old) = self.node.identity.nsec.take() {
old.zeroize();
}
self.node.identity.nsec = other.node.identity.nsec.take();
}
if other.node.identity.persistent {
self.node.identity.persistent = true;
+10 -1
View File
@@ -15,6 +15,7 @@ use serde::Serialize;
use tokio::sync::{Mutex, Notify, RwLock, broadcast, mpsc, oneshot};
use tokio::task::JoinHandle;
use tracing::{debug, info, trace, warn};
use zeroize::{Zeroize, Zeroizing};
use super::failure_state::FailureState;
use super::offer_admission::{AdmissionReject, OfferAdmission};
@@ -273,7 +274,15 @@ impl NostrDiscovery {
return Err(BootstrapError::Disabled);
}
let keys = nostr::Keys::parse(&hex::encode(identity.keypair().secret_bytes()))
// Three copies of the private key are made to reach `Keys::parse`:
// the keypair, its raw bytes, and the hex string. Each is bound and
// cleared here; `nostr::Keys` clears its own on drop.
let mut our_keypair = identity.keypair();
let mut secret_bytes = our_keypair.secret_bytes();
let secret_hex = Zeroizing::new(hex::encode(secret_bytes));
secret_bytes.zeroize();
our_keypair.non_secure_erase();
let keys = nostr::Keys::parse(secret_hex.as_str())
.map_err(|e| BootstrapError::Nostr(e.to_string()))?;
let client = Client::builder()
.signer(keys.clone())
+17 -3
View File
@@ -2,6 +2,7 @@
use bech32::{Bech32, Hrp};
use secp256k1::{SecretKey, XOnlyPublicKey};
use zeroize::{Zeroize, Zeroizing};
use super::IdentityError;
@@ -33,14 +34,25 @@ pub fn decode_npub(npub: &str) -> Result<XOnlyPublicKey, IdentityError> {
}
/// Encode a secret key as a bech32 nsec string (NIP-19).
///
/// The returned string is the private key in another encoding, so it is the
/// caller's to clear. What this function clears is the raw byte copy
/// `secret_bytes` hands back, which would otherwise sit in an unnamed
/// temporary until the end of the statement.
pub fn encode_nsec(secret_key: &SecretKey) -> String {
bech32::encode::<Bech32>(NSEC_HRP, &secret_key.secret_bytes())
.expect("nsec encoding cannot fail")
let mut secret_bytes = secret_key.secret_bytes();
let nsec =
bech32::encode::<Bech32>(NSEC_HRP, &secret_bytes).expect("nsec encoding cannot fail");
secret_bytes.zeroize();
nsec
}
/// Decode an nsec string to a secret key.
pub fn decode_nsec(nsec: &str) -> Result<SecretKey, IdentityError> {
let (hrp, data) = bech32::decode(nsec)?;
// `data` is the raw private key. The guard clears it on every exit path,
// including the two length and prefix rejections below.
let data = Zeroizing::new(data);
if hrp != NSEC_HRP {
return Err(IdentityError::InvalidNsecPrefix(hrp.to_string()));
@@ -59,7 +71,9 @@ pub fn decode_secret(s: &str) -> Result<SecretKey, IdentityError> {
if s.starts_with("nsec1") {
decode_nsec(s)
} else {
let bytes = hex::decode(s)?;
// `bytes` is the raw private key; the guard clears it on both the
// length rejection and the normal return.
let bytes = Zeroizing::new(hex::decode(s)?);
if bytes.len() != 32 {
return Err(IdentityError::InvalidNsecLength(bytes.len()));
}
+77 -12
View File
@@ -2,6 +2,7 @@
use secp256k1::{Keypair, PublicKey, SecretKey, XOnlyPublicKey};
use std::fmt;
use zeroize::Zeroize;
use super::auth::{AuthResponse, auth_challenge_digest};
use super::encoding::{decode_secret, encode_npub};
@@ -11,6 +12,13 @@ use super::{FipsAddress, IdentityError, NodeAddr, sha256};
///
/// The identity holds the secp256k1 keypair and provides methods for signing
/// and verifying protocol messages.
///
/// The keypair is the node's long-term private key. It is erased when the
/// identity is dropped, and every constructor below erases the intermediate
/// secret it built the identity from. All of that clears the copies this
/// crate owns, not every copy that ever existed: `secp256k1` names its erase
/// non-secure because the compiler may duplicate or move the bytes to places
/// no code here can name.
#[derive(Clone)]
pub struct Identity {
keypair: Keypair,
@@ -23,39 +31,51 @@ impl Identity {
pub fn generate() -> Self {
let mut secret_bytes = [0u8; 32];
rand::Rng::fill_bytes(&mut rand::rng(), &mut secret_bytes);
let secret_key =
let mut secret_key =
SecretKey::from_slice(&secret_bytes).expect("32 random bytes is a valid secret key");
Self::from_secret_key(secret_key)
let identity = Self::from_secret_key(secret_key);
secret_bytes.zeroize();
secret_key.non_secure_erase();
identity
}
/// Create an identity from an existing keypair.
pub fn from_keypair(keypair: Keypair) -> Self {
pub fn from_keypair(mut keypair: Keypair) -> Self {
let (pubkey, _parity) = keypair.x_only_public_key();
let node_addr = NodeAddr::from_pubkey(&pubkey);
let address = FipsAddress::from_node_addr(&node_addr);
Self {
let identity = Self {
keypair,
node_addr,
address,
}
};
keypair.non_secure_erase();
identity
}
/// Create an identity from a secret key.
pub fn from_secret_key(secret_key: SecretKey) -> Self {
let keypair = Keypair::from_secret_key(&super::SECP, &secret_key);
Self::from_keypair(keypair)
pub fn from_secret_key(mut secret_key: SecretKey) -> Self {
let mut keypair = Keypair::from_secret_key(&super::SECP, &secret_key);
let identity = Self::from_keypair(keypair);
keypair.non_secure_erase();
secret_key.non_secure_erase();
identity
}
/// Create an identity from secret key bytes.
pub fn from_secret_bytes(bytes: &[u8; 32]) -> Result<Self, IdentityError> {
let secret_key = SecretKey::from_slice(bytes)?;
Ok(Self::from_secret_key(secret_key))
let mut secret_key = SecretKey::from_slice(bytes)?;
let identity = Self::from_secret_key(secret_key);
secret_key.non_secure_erase();
Ok(identity)
}
/// Create an identity from an nsec string (bech32) or hex-encoded secret.
pub fn from_secret_str(s: &str) -> Result<Self, IdentityError> {
let secret_key = decode_secret(s)?;
Ok(Self::from_secret_key(secret_key))
let mut secret_key = decode_secret(s)?;
let identity = Self::from_secret_key(secret_key);
secret_key.non_secure_erase();
Ok(identity)
}
/// Return the underlying keypair.
@@ -110,6 +130,51 @@ impl Identity {
}
}
impl Drop for Identity {
/// Erase the long-term private key this identity owns.
///
/// `Keypair` is `Copy` and so cannot clear itself on drop; `Identity` is
/// not, so it does it for the copy it holds. See the type's own
/// documentation for what that does and does not reach.
fn drop(&mut self) {
self.keypair.non_secure_erase();
}
}
/// A `Keypair` copy that is erased when it goes out of scope.
///
/// `Keypair` is `Copy` and so cannot clear itself on drop. A frame that holds
/// a copy of the node's long-term private key across several exit paths —
/// early error returns, `?`, a normal return — would otherwise need an erase
/// written at each one, and a missed path is invisible. Holding the copy here
/// instead makes the clearing structural.
///
/// This clears the copy this guard owns, not every copy that ever existed:
/// `secp256k1` names its erase non-secure because the compiler may duplicate
/// or move the bytes to places no code here can name.
pub(crate) struct ErasingKeypair(Keypair);
impl ErasingKeypair {
/// Take a copy of `source` into the guard and erase `source` in place, so
/// the caller's own binding does not outlive the move.
pub(crate) fn take(source: &mut Keypair) -> Self {
let guarded = Self(*source);
source.non_secure_erase();
guarded
}
/// Borrow the guarded keypair.
pub(crate) fn get(&self) -> &Keypair {
&self.0
}
}
impl Drop for ErasingKeypair {
fn drop(&mut self) {
self.0.non_secure_erase();
}
}
impl fmt::Debug for Identity {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Identity")
+1
View File
@@ -20,6 +20,7 @@ use thiserror::Error;
pub use address::FipsAddress;
pub use auth::{AuthChallenge, AuthResponse};
pub use encoding::{decode_npub, decode_nsec, decode_secret, encode_npub, encode_nsec};
pub(crate) use local::ErasingKeypair;
pub use local::Identity;
pub use node_addr::NodeAddr;
pub use peer::PeerIdentity;
+7 -3
View File
@@ -226,14 +226,18 @@ impl Node {
packet.timestamp_ms,
);
let our_keypair = self.identity().keypair();
// This frame's own copy of the node's long-term private key; the
// handshake state keeps its own and clears that on drop.
let mut our_keypair = self.identity().keypair();
let noise_msg1 = &packet.data[header.noise_msg1_offset..];
let msg2_response = match conn.receive_handshake_init(
let init_result = conn.receive_handshake_init(
our_keypair,
self.startup_epoch(),
noise_msg1,
packet.timestamp_ms,
) {
);
our_keypair.non_secure_erase();
let msg2_response = match init_result {
Ok(m) => m,
Err(e) => {
debug!(
+8 -2
View File
@@ -194,8 +194,11 @@ impl Node {
};
// Create IK initiator handshake directly (no PeerConnection)
let our_keypair = self.identity().keypair();
// This frame's own copy of the node's long-term private key; the
// handshake state keeps its own and clears that on drop.
let mut our_keypair = self.identity().keypair();
let mut hs = HandshakeState::new_initiator(our_keypair, peer_pubkey);
our_keypair.non_secure_erase();
hs.set_local_epoch(self.startup_epoch());
let noise_msg1 = match hs.write_message_1() {
@@ -605,8 +608,11 @@ impl Node {
let dest_pubkey = *entry.remote_pubkey();
// Create Noise XK initiator handshake
let our_keypair = self.identity().keypair();
// This frame's own copy of the node's long-term private key; the
// handshake state keeps its own and clears that on drop.
let mut our_keypair = self.identity().keypair();
let mut handshake = HandshakeState::new_xk_initiator(our_keypair, dest_pubkey);
our_keypair.non_secure_erase();
handshake.set_local_epoch(self.startup_epoch());
let msg1 = match handshake.write_xk_message_1() {
+17 -4
View File
@@ -626,8 +626,11 @@ impl Node {
.record_reject(RejectReason::Session(SessionReject::RekeyPending));
return;
}
let our_keypair = self.identity().keypair();
// This frame's own copy of the node's long-term private key;
// the handshake state keeps its own and clears that on drop.
let mut our_keypair = self.identity().keypair();
let mut handshake = HandshakeState::new_xk_responder(our_keypair);
our_keypair.non_secure_erase();
handshake.set_local_epoch(self.startup_epoch());
if let Err(e) = handshake.read_xk_message_1(&setup.handshake_payload) {
@@ -681,8 +684,11 @@ impl Node {
}
// Create XK responder handshake and process msg1
let our_keypair = self.identity().keypair();
// This frame's own copy of the node's long-term private key; the
// handshake state keeps its own and clears that on drop.
let mut our_keypair = self.identity().keypair();
let mut handshake = HandshakeState::new_xk_responder(our_keypair);
our_keypair.non_secure_erase();
handshake.set_local_epoch(self.startup_epoch());
if let Err(e) = handshake.read_xk_message_1(&setup.handshake_payload) {
@@ -720,7 +726,11 @@ impl Node {
// Store session entry in AwaitingMsg3 state with ack payload for potential resend.
// Use a dummy pubkey since we don't know the initiator's identity yet.
// We use our own pubkey as placeholder; it will be replaced in handle_session_msg3.
let placeholder_pubkey = self.identity().keypair().public_key();
// `keypair()` hands back a copy of the long-term private key, so the
// temporary is bound and erased rather than left to the statement end.
let mut our_keypair = self.identity().keypair();
let placeholder_pubkey = our_keypair.public_key();
our_keypair.non_secure_erase();
let now_ms = Self::now_ms();
let resend_interval = self.config().node.rate_limit.handshake_resend_interval_ms;
let mut entry = SessionEntry::new(
@@ -1728,8 +1738,11 @@ impl Node {
}
// Create Noise XK initiator handshake
let our_keypair = self.identity().keypair();
// This frame's own copy of the node's long-term private key; the
// handshake state keeps its own and clears that on drop.
let mut our_keypair = self.identity().keypair();
let mut handshake = HandshakeState::new_xk_initiator(our_keypair, dest_pubkey);
our_keypair.non_secure_erase();
handshake.set_local_epoch(self.startup_epoch());
let msg1 = handshake
.write_xk_message_1()
+16 -12
View File
@@ -489,18 +489,22 @@ impl Node {
};
// Start the Noise handshake and get message 1
let our_keypair = self.identity().keypair();
let noise_msg1 =
match connection.start_handshake(our_keypair, self.startup_epoch(), current_time_ms) {
Ok(msg) => msg,
Err(e) => {
// Clean up the index and link
let _ = self.index_allocator.free(our_index);
self.links.remove(&link_id);
self.addr_to_link.remove(&(transport_id, remote_addr));
return Err(NodeError::HandshakeFailed(e.to_string()));
}
};
// This frame's own copy of the node's long-term private key; the
// handshake state keeps its own and clears that on drop.
let mut our_keypair = self.identity().keypair();
let start_result =
connection.start_handshake(our_keypair, self.startup_epoch(), current_time_ms);
our_keypair.non_secure_erase();
let noise_msg1 = match start_result {
Ok(msg) => msg,
Err(e) => {
// Clean up the index and link
let _ = self.index_allocator.free(our_index);
self.links.remove(&link_id);
self.addr_to_link.remove(&(transport_id, remote_addr));
return Err(NodeError::HandshakeFailed(e.to_string()));
}
};
// Set index and transport info on the connection
connection.set_our_index(our_index);
+116 -21
View File
@@ -179,7 +179,7 @@ impl HandshakeState {
///
/// The initiator knows the responder's static key and will send first.
/// Used by FMP (link layer).
pub fn new_initiator(static_keypair: Keypair, remote_static: PublicKey) -> Self {
pub fn new_initiator(mut static_keypair: Keypair, remote_static: PublicKey) -> Self {
let secp = Secp256k1::new();
let mut state = Self {
pattern: NoisePattern::Ik,
@@ -201,6 +201,10 @@ impl HandshakeState {
let normalized = Self::normalize_for_premessage(&remote_static);
state.symmetric.mix_hash(&normalized);
// The struct now holds its own copy of the long-term private key and
// clears it on drop, so this frame's parameter copy is erased here.
static_keypair.non_secure_erase();
state
}
@@ -208,7 +212,7 @@ impl HandshakeState {
///
/// The responder does NOT know the initiator's static key - it will be
/// learned from message 1. Used by FMP (link layer).
pub fn new_responder(static_keypair: Keypair) -> Self {
pub fn new_responder(mut static_keypair: Keypair) -> Self {
let secp = Secp256k1::new();
let mut state = Self {
pattern: NoisePattern::Ik,
@@ -229,6 +233,10 @@ impl HandshakeState {
let normalized = Self::normalize_for_premessage(&state.static_keypair.public_key());
state.symmetric.mix_hash(&normalized);
// The struct now holds its own copy of the long-term private key and
// clears it on drop, so this frame's parameter copy is erased here.
static_keypair.non_secure_erase();
state
}
@@ -236,7 +244,7 @@ impl HandshakeState {
///
/// The initiator knows the responder's static key. XK defers the
/// initiator's static key reveal to msg3. Used by FSP (session layer).
pub fn new_xk_initiator(static_keypair: Keypair, remote_static: PublicKey) -> Self {
pub fn new_xk_initiator(mut static_keypair: Keypair, remote_static: PublicKey) -> Self {
let secp = Secp256k1::new();
let mut state = Self {
pattern: NoisePattern::Xk,
@@ -256,6 +264,10 @@ impl HandshakeState {
let normalized = Self::normalize_for_premessage(&remote_static);
state.symmetric.mix_hash(&normalized);
// The struct now holds its own copy of the long-term private key and
// clears it on drop, so this frame's parameter copy is erased here.
static_keypair.non_secure_erase();
state
}
@@ -263,7 +275,7 @@ impl HandshakeState {
///
/// The responder does NOT know the initiator's static key - it will be
/// learned from message 3. Used by FSP (session layer).
pub fn new_xk_responder(static_keypair: Keypair) -> Self {
pub fn new_xk_responder(mut static_keypair: Keypair) -> Self {
let secp = Secp256k1::new();
let mut state = Self {
pattern: NoisePattern::Xk,
@@ -283,6 +295,10 @@ impl HandshakeState {
let normalized = Self::normalize_for_premessage(&state.static_keypair.public_key());
state.symmetric.mix_hash(&normalized);
// The struct now holds its own copy of the long-term private key and
// clears it on drop, so this frame's parameter copy is erased here.
static_keypair.non_secure_erase();
state
}
@@ -322,9 +338,15 @@ impl HandshakeState {
let mut secret_bytes = [0u8; 32];
rng.fill_bytes(&mut secret_bytes);
let secret_key =
let mut secret_key =
SecretKey::from_slice(&secret_bytes).expect("32 random bytes is valid secret key");
self.ephemeral_keypair = Some(Keypair::from_secret_key(&self.secp, &secret_key));
secret_bytes.zeroize();
// The erase is called non-secure because the private key may sit in
// further copies this function cannot name. It still clears the copy
// this frame owns; the keypair the key was just stored in is cleared
// by `Drop for HandshakeState`.
secret_key.non_secure_erase();
}
/// Perform ECDH between our secret and their public key.
@@ -335,15 +357,26 @@ impl HandshakeState {
/// may have the wrong parity for the responder's static key. Since P and
/// -P produce ECDH result points with the same x-coordinate, hashing
/// only x ensures both sides derive the same shared secret.
///
/// `our_secret` is borrowed, so this frame makes no copy of it. Every
/// caller below binds the value `Keypair::secret_key` hands back and
/// erases that binding once the DH is done, because the returned
/// `SecretKey` is a whole private key rather than a handle to one. Those
/// erases clear the copies this crate owns; `secp256k1` names its erase
/// non-secure because the compiler may hold further copies that no code
/// here can name.
fn ecdh(&self, our_secret: &SecretKey, their_public: &PublicKey) -> [u8; 32] {
// Get raw (x, y) coordinates (64 bytes) without any hashing
let point = shared_secret_point(their_public, our_secret);
let mut point = shared_secret_point(their_public, our_secret);
// Hash only the x-coordinate (first 32 bytes), ignoring y/parity
let mut hasher = Sha256::new();
hasher.update(&point[..32]);
let hash = hasher.finalize();
let mut hash = hasher.finalize();
let mut result = [0u8; 32];
result.copy_from_slice(&hash);
hash.as_mut_slice().zeroize();
point.zeroize();
// `result` is moved out, so clearing it belongs to the caller.
result
}
@@ -388,8 +421,11 @@ impl HandshakeState {
self.symmetric.mix_hash(&e_pub);
// -> es: DH(e, rs), mix into key
let es = self.ecdh(&ephemeral.secret_key(), &remote_static);
let mut sk = ephemeral.secret_key();
let mut es = self.ecdh(&sk, &remote_static);
self.symmetric.mix_key(&es);
es.zeroize();
sk.non_secure_erase();
// -> s: encrypt our static and send
let our_static = self.static_keypair.public_key().serialize();
@@ -397,8 +433,11 @@ impl HandshakeState {
message.extend_from_slice(&encrypted_static);
// -> ss: DH(s, rs), mix into key
let ss = self.ecdh(&self.static_keypair.secret_key(), &remote_static);
let mut sk = self.static_keypair.secret_key();
let mut ss = self.ecdh(&sk, &remote_static);
self.symmetric.mix_key(&ss);
ss.zeroize();
sk.non_secure_erase();
// -> epoch: encrypt startup epoch for restart detection
let encrypted_epoch = self.symmetric.encrypt_and_hash(&epoch)?;
@@ -441,8 +480,11 @@ impl HandshakeState {
// -> es: DH(s, re), mix into key
// (responder uses their static with initiator's ephemeral)
let es = self.ecdh(&self.static_keypair.secret_key(), &re);
let mut sk = self.static_keypair.secret_key();
let mut es = self.ecdh(&sk, &re);
self.symmetric.mix_key(&es);
es.zeroize();
sk.non_secure_erase();
// -> s: decrypt initiator's static
let encrypted_static_end = PUBKEY_SIZE + PUBKEY_SIZE + super::TAG_SIZE;
@@ -453,8 +495,11 @@ impl HandshakeState {
self.remote_static = Some(rs);
// -> ss: DH(s, rs), mix into key
let ss = self.ecdh(&self.static_keypair.secret_key(), &rs);
let mut sk = self.static_keypair.secret_key();
let mut ss = self.ecdh(&sk, &rs);
self.symmetric.mix_key(&ss);
ss.zeroize();
sk.non_secure_erase();
// -> epoch: decrypt initiator's startup epoch
let encrypted_epoch = &message[encrypted_static_end..];
@@ -508,12 +553,18 @@ impl HandshakeState {
self.symmetric.mix_hash(&e_pub);
// <- ee: DH(e, re), mix into key
let ee = self.ecdh(&ephemeral.secret_key(), &re);
let mut sk = ephemeral.secret_key();
let mut ee = self.ecdh(&sk, &re);
self.symmetric.mix_key(&ee);
ee.zeroize();
sk.non_secure_erase();
// <- se: DH(s, re), mix into key
let se = self.ecdh(&self.static_keypair.secret_key(), &re);
let mut sk = self.static_keypair.secret_key();
let mut se = self.ecdh(&sk, &re);
self.symmetric.mix_key(&se);
se.zeroize();
sk.non_secure_erase();
// <- epoch: encrypt startup epoch for restart detection
let encrypted_epoch = self.symmetric.encrypt_and_hash(&epoch)?;
@@ -556,14 +607,20 @@ impl HandshakeState {
// <- ee: DH(e, re), mix into key
let ephemeral = self.ephemeral_keypair.as_ref().unwrap();
let ee = self.ecdh(&ephemeral.secret_key(), &re);
let mut sk = ephemeral.secret_key();
let mut ee = self.ecdh(&sk, &re);
self.symmetric.mix_key(&ee);
ee.zeroize();
sk.non_secure_erase();
// <- se: DH(e, rs), mix into key
// (initiator uses their ephemeral with responder's static)
let rs = self.remote_static.expect("initiator has remote static");
let se = self.ecdh(&ephemeral.secret_key(), &rs);
let mut sk = ephemeral.secret_key();
let mut se = self.ecdh(&sk, &rs);
self.symmetric.mix_key(&se);
se.zeroize();
sk.non_secure_erase();
// <- epoch: decrypt responder's startup epoch
let encrypted_epoch = &message[PUBKEY_SIZE..];
@@ -620,8 +677,11 @@ impl HandshakeState {
self.symmetric.mix_hash(&e_pub);
// -> es: DH(e, rs), mix into key
let es = self.ecdh(&ephemeral.secret_key(), &remote_static);
let mut sk = ephemeral.secret_key();
let mut es = self.ecdh(&sk, &remote_static);
self.symmetric.mix_key(&es);
es.zeroize();
sk.non_secure_erase();
self.progress = HandshakeProgress::Message1Done;
@@ -660,8 +720,11 @@ impl HandshakeState {
// -> es: DH(s, re), mix into key
// (responder uses their static with initiator's ephemeral)
let es = self.ecdh(&self.static_keypair.secret_key(), &re);
let mut sk = self.static_keypair.secret_key();
let mut es = self.ecdh(&sk, &re);
self.symmetric.mix_key(&es);
es.zeroize();
sk.non_secure_erase();
self.progress = HandshakeProgress::Message1Done;
@@ -707,8 +770,11 @@ impl HandshakeState {
self.symmetric.mix_hash(&e_pub);
// <- ee: DH(e, re), mix into key
let ee = self.ecdh(&ephemeral.secret_key(), &re);
let mut sk = ephemeral.secret_key();
let mut ee = self.ecdh(&sk, &re);
self.symmetric.mix_key(&ee);
ee.zeroize();
sk.non_secure_erase();
// <- epoch: encrypt startup epoch for restart detection
let encrypted_epoch = self.symmetric.encrypt_and_hash(&epoch)?;
@@ -752,8 +818,11 @@ impl HandshakeState {
// <- ee: DH(e, re), mix into key
let ephemeral = self.ephemeral_keypair.as_ref().unwrap();
let ee = self.ecdh(&ephemeral.secret_key(), &re);
let mut sk = ephemeral.secret_key();
let mut ee = self.ecdh(&sk, &re);
self.symmetric.mix_key(&ee);
ee.zeroize();
sk.non_secure_erase();
// <- epoch: decrypt responder's startup epoch
let encrypted_epoch = &message[PUBKEY_SIZE..];
@@ -839,8 +908,11 @@ impl HandshakeState {
message.extend_from_slice(&encrypted_static);
// -> se: DH(s, re), mix into key
let se = self.ecdh(&self.static_keypair.secret_key(), &re);
let mut sk = self.static_keypair.secret_key();
let mut se = self.ecdh(&sk, &re);
self.symmetric.mix_key(&se);
se.zeroize();
sk.non_secure_erase();
// -> epoch: encrypt startup epoch for restart detection
let encrypted_epoch = self.symmetric.encrypt_and_hash(&epoch)?;
@@ -890,8 +962,11 @@ impl HandshakeState {
.ephemeral_keypair
.as_ref()
.expect("should have ephemeral after msg2");
let se = self.ecdh(&ephemeral.secret_key(), &rs);
let mut sk = ephemeral.secret_key();
let mut se = self.ecdh(&sk, &rs);
self.symmetric.mix_key(&se);
se.zeroize();
sk.non_secure_erase();
// -> epoch: decrypt initiator's startup epoch
let encrypted_epoch = &message[encrypted_static_end..];
@@ -943,6 +1018,26 @@ impl HandshakeState {
}
}
impl Drop for HandshakeState {
/// Erase the two private keys this state holds.
///
/// `static_keypair` is the node's long-term private key and
/// `ephemeral_keypair` is the handshake's own. Both live here for the
/// whole handshake, which is longer than in any other frame, so this is
/// where clearing them matters most. `Keypair` is `Copy` and so cannot
/// clear itself on drop; `HandshakeState` is not, so it does it for both.
///
/// This clears the copies this crate owns, not every copy that ever
/// existed: `secp256k1` names its erase non-secure because the compiler
/// may duplicate or move the bytes to places no code here can name.
fn drop(&mut self) {
self.static_keypair.non_secure_erase();
if let Some(ephemeral) = self.ephemeral_keypair.as_mut() {
ephemeral.non_secure_erase();
}
}
}
impl fmt::Debug for HandshakeState {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("HandshakeState")
+13 -4
View File
@@ -229,14 +229,19 @@ impl Clone for CipherState {
impl CipherState {
/// Create a new cipher state with the given key.
pub(crate) fn new(key: [u8; 32]) -> Self {
///
/// The parameter is this frame's own copy of live key material, so it is
/// cleared before returning. The caller's copy stays the caller's to clear.
pub(crate) fn new(mut key: [u8; 32]) -> Self {
let cipher = Self::build_cipher(&key);
Self {
let state = Self {
key,
cipher,
nonce: 0,
has_key: true,
}
};
key.zeroize();
state
}
/// Create an empty cipher state (no key yet).
@@ -250,11 +255,15 @@ impl CipherState {
}
/// Initialize with a key.
pub(super) fn initialize_key(&mut self, key: [u8; 32]) {
///
/// The parameter is this frame's own copy of live key material, so it is
/// cleared before returning. The caller's copy stays the caller's to clear.
pub(super) fn initialize_key(&mut self, mut key: [u8; 32]) {
self.key = key;
self.cipher = Self::build_cipher(&key);
self.nonce = 0;
self.has_key = true;
key.zeroize();
}
/// Build a ring `LessSafeKey` from raw key bytes. Centralized so the
+5 -1
View File
@@ -212,7 +212,11 @@ fn cipher_state_drop_impl_clears_the_retained_key() {
assert_zeroize_on_drop::<CipherState>();
let mut cipher = CipherState::new([7u8; 32]);
assert_eq!(cipher.key_bytes(), [7u8; 32]);
// `key_bytes` hands back a copy of live key material, so the observation
// is bound and cleared rather than left in an unnamed temporary.
let mut observed = cipher.key_bytes();
assert_eq!(observed, [7u8; 32]);
observed.zeroize();
assert!(cipher.has_key());
cipher.zeroize();
+18 -4
View File
@@ -5,6 +5,7 @@
//! ActivePeer upon successful authentication.
use crate::PeerIdentity;
use crate::identity::ErasingKeypair;
use crate::noise::{self, NoiseError, NoiseSession};
use crate::transport::{LinkDirection, LinkId, LinkStats, TransportAddr, TransportId};
use crate::utils::index::SessionIndex;
@@ -402,10 +403,15 @@ impl PeerConnection {
/// The epoch is our startup epoch, encrypted into msg1 for restart detection.
pub fn start_handshake(
&mut self,
our_keypair: Keypair,
mut our_keypair: Keypair,
epoch: [u8; 8],
current_time_ms: u64,
) -> Result<Vec<u8>, NoiseError> {
// The parameter is this frame's own copy of the node's long-term
// private key, and the state checks below return before it is used.
// The guard clears it on every exit path.
let our_keypair = ErasingKeypair::take(&mut our_keypair);
if self.direction != LinkDirection::Outbound {
return Err(NoiseError::WrongState {
expected: "outbound connection".to_string(),
@@ -426,7 +432,9 @@ impl PeerConnection {
.expect("outbound must have expected identity")
.pubkey_full();
let mut hs = noise::HandshakeState::new_initiator(our_keypair, remote_static);
let mut kp = *our_keypair.get();
let mut hs = noise::HandshakeState::new_initiator(kp, remote_static);
kp.non_secure_erase();
hs.set_local_epoch(epoch);
let msg1 = hs.write_message_1()?;
@@ -443,11 +451,15 @@ impl PeerConnection {
/// The epoch is our startup epoch, encrypted into msg2 for restart detection.
pub fn receive_handshake_init(
&mut self,
our_keypair: Keypair,
mut our_keypair: Keypair,
epoch: [u8; 8],
message: &[u8],
current_time_ms: u64,
) -> Result<Vec<u8>, NoiseError> {
// Same as `start_handshake`: the parameter copy outlives two early
// returns, so the guard owns it rather than an erase per exit path.
let our_keypair = ErasingKeypair::take(&mut our_keypair);
if self.direction != LinkDirection::Inbound {
return Err(NoiseError::WrongState {
expected: "inbound connection".to_string(),
@@ -462,7 +474,9 @@ impl PeerConnection {
});
}
let mut hs = noise::HandshakeState::new_responder(our_keypair);
let mut kp = *our_keypair.get();
let mut hs = noise::HandshakeState::new_responder(kp);
kp.non_secure_erase();
hs.set_local_epoch(epoch);
// Process message 1 (this reveals the initiator's identity and epoch)