laantungir/fips
1
0
Fork 0
mirror of https://github.com/jmcorgan/fips.git synced 2026-10-05 11:08:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge the maintenance line's comment sweep and key-material work

Browse Source 
Carries thirteen commits up from maint: the source-comment sweep and its
regression gate, and the v1 exposure remediation (key-material clearing,
frame-length validation, and post-handshake identity confirmation).

Ten paths conflicted. The resolutions:

- Cargo.toml: kept both dependency additions, libm and zeroize.
- src/control/queries.rs: took the corrected prose from maint but kept
  this branch's path to the relocated rx_loop module. Neither side was
  right alone.
- src/control/read_handle.rs: took maint's corrected description of what
  the snapshot dispatch serves, and kept this branch's paragraph on the
  mutating profiling commands, which maint has never carried.
- src/node/handlers/mod.rs: kept this branch's module list, widening
  handshake to pub(in crate::node) so the tests can name the waiver type.
- src/node/dataplane/rx_loop.rs: union of the import blocks.
- src/node/handlers/handshake.rs: kept the WireOutcome binding and dropped
  maint's possible_restart fix-up, which this branch folded away; placed
  the post-handshake identity confirmation above it, which keeps the
  confirmation ahead of the reverse-lookup repair as intended.
- src/nostr/runtime.rs: took this branch's side. The function maint edits
  was relocated to traversal_machine.rs here and its copy already carries
  the same corrected text.
- src/mmp/report.rs: deleted. This branch retired the module; maint's only
  change was removing a stale comment.
- src/node/lifecycle.rs and src/peer/connection.rs: deleted, but their
  key-clearing work was relocated rather than dropped. The erasing guard
  now wraps both handshake entry points in src/peer/machine.rs and the
  outbound dial in src/node/lifecycle/mod.rs, since the files maint had
  changed no longer exist here. Without the relocation the guard type
  would have arrived with no callers at all.

Eight further edits were forced by names that differ on this branch: the
wire module path at seven sites, the connection lookup in the waiver
classifier, and four test helpers the peer-machine work replaced.
This commit is contained in:
Johnathan Corgan
2026-08-16 17:39:30 +00:00
parent 1e95d34152 f7085f2ed7
commit 9e4b5d6a72
48 changed files with 1841 additions and 237 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/ci.yml
+5 -2
View File
@@ -69,6 +69,8 @@ jobs:
run: bash testing/check-image-scoping.sh
- name: Check every action is pinned to a commit SHA
run: bash testing/check-action-pins.sh
- name: Check every source comment resolves in-repo
run: bash testing/check-comment-refs.sh
# Hermetic: synthetic ping functions, no containers, ~45s. Lives beside
# the other two so both runners gate on it identically — putting it in
# only one would create exactly the drift check-ci-parity.sh exists to
@@ -515,8 +517,9 @@ jobs:
# NM+dnsmasq, dns-delegate, no-resolver) across five distros,
# plus end-to-end scenarios that boot a real fips daemon with a
# real TUN and assert `dig @127.0.0.53 AAAA <npub>.fips`
# returns AAAA. Pins the production DNS bind path that
# ISSUE-2026-0002 lived in. Single matrix entry runs all 13
# returns AAAA. Pins the production DNS bind path where a
# loopback-delivered query was once misattributed to the mesh
# interface and dropped. Single matrix entry runs all 13
# scenarios sequentially; ~7-12 min warm, ~12-15 min cold.
- suite: dns-resolver
type: dns-resolver
Cargo.lock
Generated
+15
View File
@@ -1127,6 +1127,7 @@ dependencies = [
"tun",
"windows-service",
"wintun",
"zeroize",
]
[[package]]
@@ -4215,6 +4216,20 @@ name = "zeroize"
version = "1.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e13c156562582aa81c60cb29407084cdb54c4164760106ab78e6c5b0858cf64e"
dependencies = [
"zeroize_derive",
]
[[package]]
name = "zeroize_derive"
version = "1.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3c50655cbb0fe3fc43170059e702f1ce5e19b84cec58dc87b037a09935c2f328"
dependencies = [
"proc-macro2",
"quote",
"syn 2.0.119",
]
[[package]]
name = "zerotrie"
Cargo.toml
+1
View File
@@ -26,6 +26,7 @@ sha2 = "0.10"
hkdf = "0.12"
ring = "0.17"
libm = "0.2"
zeroize = { version = "1.9", features = ["zeroize_derive"] }
rand = "0.10.1"
crossbeam-channel = "0.5"
thiserror = "2.0"
docs/design/fips-session-layer.md
+4 -1
View File
@@ -223,7 +223,10 @@ than network addresses. A session survives:
FSP uses Noise XK for session encryption, distinct from the Noise IK
pattern used at the link layer. The full Noise descriptor is
`Noise_XK_secp256k1_ChaChaPoly_SHA256`.
`Noise_XK_secp256k1_ChaChaPoly_SHA256`, with one deviation recorded in
[the security reference](../reference/security.md): the handshake AEAD
uses an empty associated-data field where standard Noise
`EncryptAndHash` uses the handshake hash `h`.
The XK pattern (pre-message: `← s`):
docs/reference/security.md
+21 -3
View File
@@ -58,16 +58,34 @@ idempotent).
| Curve | secp256k1 | FMP IK, FSP XK, Schnorr signatures |
| Diffie-Hellman | ECDH on secp256k1 (x-only normalized) | Noise IK, Noise XK |
| AEAD | ChaCha20-Poly1305 | FMP link encryption, FSP session encryption |
| Hash | SHA-256 | NodeAddr derivation, Noise transcript |
| Hash | SHA-256 | NodeAddr derivation, Noise key schedule |
| Key derivation | HKDF-SHA256 | Noise key schedule |
| Signatures | secp256k1 Schnorr | TreeAnnounce, LookupResponse proof, Nostr adverts |
| Noise pattern (link) | `Noise_IK_secp256k1_ChaChaPoly_SHA256` | FMP link layer (IK with epoch payload) |
| Noise pattern (session) | `Noise_XK_secp256k1_ChaChaPoly_SHA256` | FSP session layer (XK with epoch payload) |
| Noise pattern (link) | `Noise_IK_secp256k1_ChaChaPoly_SHA256`, with the deviation below | FMP link layer (IK with epoch payload) |
| Noise pattern (session) | `Noise_XK_secp256k1_ChaChaPoly_SHA256`, with the deviation below | FSP session layer (XK with epoch payload) |
These choices align with the Nostr cryptographic stack
(secp256k1 + ChaCha20-Poly1305 + SHA-256) and the NIP-44 encrypted
messaging standard.
### Deviation: Empty Associated Data in the Handshake AEAD
Both Noise patterns above deviate from the standard construction in one
respect. The handshake AEAD uses an empty associated-data field where
standard Noise `EncryptAndHash` uses the handshake hash `h`.
The choice was deliberate. Using secp256k1 rather than 25519 already put the
construction outside standard Noise, so no standard-Noise peer could be
confused with it, and the transcript hash bought no distinguishing value.
That argument is about domain separation, and on those grounds it holds. It
does not cover transcript binding, which is the property actually absent.
Domain separation and DH binding survive through the chaining key `ck`, which
`mix_key` chains from `ck = h`, seeded from the protocol name in
`SymmetricState::initialize` (`src/noise/handshake.rs`). The handshake hash
`h` is maintained at every step and is never fed to the AEAD, so it binds
nothing.
## Rekey Defaults
Both link-layer and session-layer Noise sessions rekey under one of
packaging/common/fips-dns-setup
+1 -2
View File
@@ -117,8 +117,7 @@ EOF
# - The IPV6_PKTINFO ifindex attribution behaviour where Linux reports
# a packet to fips0's own IPv6 address as arriving on fips0 (despite
# loopback delivery), which would otherwise be silently dropped by
# the daemon's mesh-interface filter — see ISSUE-2026-0002 in the
# project tracker.
# the daemon's mesh-interface filter.
#
# This backend is the recommended path on every systemd-resolved host
# that doesn't have native dns-delegate support (systemd >= 258).
src/bin/fips-gateway.rs
+1 -1
View File
@@ -299,7 +299,7 @@ async fn main() {
}
};
// Install inbound port-forward rules (TASK-2026-0061).
// Install inbound port-forward rules.
if let Err(e) = nat_mgr.set_port_forwards(&gw_config.port_forwards) {
error!(error = %e, "Failed to install port-forward rules");
let _ = nat_mgr.cleanup();
src/bin/fips.rs
+11 -2
View File
@@ -10,6 +10,7 @@ use fips::{Config, Node};
use std::path::PathBuf;
use tracing::{debug, error, info};
use tracing_subscriber::{EnvFilter, fmt};
use zeroize::Zeroize;
/// FIPS mesh network daemon
#[derive(Parser, Debug)]
@@ -130,7 +131,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);
@@ -150,7 +151,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,
src/bin/fipsctl.rs
+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)]
@@ -413,11 +414,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;
}
src/config/gateway.rs
+1 -1
View File
@@ -76,7 +76,7 @@ pub struct GatewayConfig {
#[serde(default)]
pub conntrack: ConntrackConfig,
/// Inbound mesh port forwarding rules. See TASK-2026-0061.
/// Inbound mesh port forwarding rules.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub port_forwards: Vec<PortForward>,
}
src/config/mod.rs
+83 -18
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};
@@ -69,8 +70,7 @@ const PUB_FILENAME: &str = "fips.pub";
/// Recognizes IPv4 `127.x.x.x`, IPv6 `::1` (with or without brackets), and
/// the literal string `localhost`. Hostnames are conservatively assumed to
/// be non-loopback. Used by `Config::validate()` to reject misconfigured
/// loopback UDP binds combined with non-loopback peer addresses (see
/// ISSUE-2026-0005).
/// loopback UDP binds combined with non-loopback peer addresses.
fn is_loopback_addr_str(addr: &str) -> bool {
// Bracketed IPv6: `[::1]:port`
if let Some(rest) = addr.strip_prefix('[')
@@ -310,11 +310,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 {
@@ -535,7 +540,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()) {
@@ -546,7 +553,7 @@ pub fn resolve_identity(
);
}
return Ok(ResolvedIdentity {
nsec,
nsec: nsec.to_string(),
source: IdentitySource::KeyFile(key_path),
});
}
@@ -560,7 +567,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(),
@@ -577,14 +585,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() {
@@ -622,7 +636,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() {
@@ -664,6 +684,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,
@@ -671,6 +699,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.
@@ -732,6 +768,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 {
@@ -802,10 +852,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,
})?,
);
let mut config: Config =
serde_yaml::from_str(&contents).map_err(|e| ConfigError::ParseYaml {
@@ -897,10 +953,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;
@@ -1037,9 +1102,9 @@ impl Config {
// Reject loopback UDP bind combined with non-loopback peer addresses.
// Linux pins the source IP to a loopback-bound socket, so packets
// sent from such a socket to external peers are dropped at the
// routing layer with no clear error in the daemon log. See
// ISSUE-2026-0005. Outbound-only mode is exempt because it
// overrides bind_addr to 0.0.0.0:0 (kernel-picked source).
// routing layer with no clear error in the daemon log.
// Outbound-only mode is exempt because it overrides bind_addr to
// 0.0.0.0:0 (kernel-picked source).
for (name, cfg) in self.transports.udp.iter() {
if cfg.outbound_only() {
continue;
src/config/transport.rs
+1 -1
View File
@@ -103,7 +103,7 @@ pub struct UdpConfig {
/// unfamiliar addresses. The Node-level gate at
/// `src/node/handlers/handshake.rs` carves out msg1 from peers
/// already established on this transport (so rekey continues to
/// work) — see ISSUE-2026-0004.
/// work).
#[serde(default, skip_serializing_if = "Option::is_none")]
pub accept_connections: Option<bool>,
}
src/control/mod.rs
+2 -2
View File
@@ -78,8 +78,8 @@ where
match serde_json::from_str::<Request>(line.trim()) {
Ok(request) => {
// First try to serve the request entirely off-loop from the
// read handle. In R0 this always returns None (no query is
// cut over yet); R1+ adds the per-command snapshot branches.
// read handle. It returns None for any command with no snapshot
// branch, and those fall through to the rx_loop path below.
match snapshot_dispatch(&request, &read_handle) {
Some(resp) => resp,
None => {
src/control/queries.rs
+23 -22
View File
@@ -2687,8 +2687,8 @@ mod tests {
assert_snapshot("show_stats_history_all_peers", &render_response(resp));
}
/// The five Category-D queries cut over to off-loop serving in R3. Served
/// via `snapshot_dispatch`; coverage asserted in
/// The five derived/routing/cache queries served off-loop via
/// `snapshot_dispatch`; coverage asserted in
/// `snapshot_dispatch_serves_category_d_queries` below.
const OFF_LOOP_CATEGORY_D: &[&str] = &[
"show_tree",
@@ -2698,7 +2698,7 @@ mod tests {
"show_identity_cache",
];
/// The six Category-E queries cut over to off-loop serving in R4. Served via
/// The six per-entity table queries served off-loop via
/// `snapshot_dispatch`; coverage asserted in
/// `snapshot_dispatch_serves_category_e_queries`.
const OFF_LOOP_CATEGORY_E: &[&str] = &[
@@ -2710,7 +2710,7 @@ mod tests {
"show_mmp",
];
/// Milestone-completion contract: every pure-read `show_*` query is served
/// Contract: every pure-read `show_*` query is served
/// off-loop via `snapshot_dispatch`, and the rx_loop control path carries no
/// `show_*` arm at all — only the mutating COMMAND handlers (`connect` /
/// `disconnect`) reach it. This test enumerates the full read surface and
@@ -2784,8 +2784,8 @@ mod tests {
/// the rx_loop source carries no `queries::dispatch` call and no
/// `starts_with("show_")` routing branch. Reads the committed source of
/// `src/node/dataplane/rx_loop.rs` and asserts both markers are absent. This
/// is the milestone's "remove `show_*` from the data-plane dispatch path"
/// invariant, guarded against regression.
/// guards the "no `show_*` on the data-plane dispatch path" invariant
/// against regression.
#[test]
fn rx_loop_has_no_show_dispatch() {
let src = include_str!("../node/dataplane/rx_loop.rs");
@@ -2850,10 +2850,10 @@ mod tests {
}
}
/// The R1/R2 scalar-and-series queries are served off-loop via
/// The scalar-and-series queries are served off-loop via
/// `snapshot_dispatch`; mutations return `None` and take the rx_loop COMMAND
/// path. (`show_stats_peers` / `show_stats_history_all_peers`, formerly
/// asserted on-loop here, were cut over in R5 — see
/// asserted on-loop here, are now served off-loop too — see
/// `snapshot_dispatch_serves_every_read_query` for the full read surface.)
#[test]
fn snapshot_dispatch_serves_scalar_and_series_queries() {
@@ -2888,7 +2888,7 @@ mod tests {
"show_stats_all_history",
Some(json!({ "window": "10s", "granularity": "1s" })),
),
// R3 Category-D cutover.
// Derived/routing/cache queries, served off-loop.
("show_tree", None),
("show_bloom", None),
("show_cache", None),
@@ -2910,7 +2910,7 @@ mod tests {
}
}
/// R5 cutover + byte-identity: after a `record_stats_history()` tick the
/// Byte-identity: after a `record_stats_history()` tick the
/// off-loop `show_acl` / `show_stats_peers` / `show_stats_history_all_peers`
/// renders each equal their on-loop oracle byte-for-byte, and all three are
/// served off-loop via `snapshot_dispatch`.
@@ -2996,7 +2996,7 @@ mod tests {
assert_eq!(snap.connection_count, node.connection_count());
assert_eq!(snap.estimated_mesh_size, node.estimated_mesh_size());
assert_eq!(snap.effective_ipv6_mtu, node.effective_ipv6_mtu());
// R5: the ACL status projection matches the node's live ACL status.
// The ACL status projection matches the node's live ACL status.
assert_eq!(snap.acl_status, node.peer_acl_status());
// Off-loop render must equal the on-loop render byte-for-byte.
@@ -3008,9 +3008,9 @@ mod tests {
);
}
/// The five Category-D queries are served off-loop via `snapshot_dispatch`
/// (return `Some` with status ok); everything not cut over stays on the
/// rx_loop path (`None`).
/// The five derived/routing/cache queries are served off-loop via
/// `snapshot_dispatch` (return `Some` with status ok); everything not cut
/// over stays on the rx_loop path (`None`).
#[test]
fn snapshot_dispatch_serves_category_d_queries() {
use super::super::protocol::Request;
@@ -3033,7 +3033,7 @@ mod tests {
}
// Mutations take the rx_loop COMMAND path. (Every read query, including
// the per-peer stats-series queries, is served off-loop as of R5.)
// the per-peer stats-series queries, is served off-loop.)
for cmd in ["connect", "disconnect"] {
assert!(
snapshot_dispatch(&req(cmd), &handle).is_none(),
@@ -3043,7 +3043,7 @@ mod tests {
}
/// The tick-published `RoutingSnapshot` reflects node state, and each
/// off-loop Category-D render equals its on-loop render byte-for-byte
/// off-loop routing render equals its on-loop render byte-for-byte
/// (modulo the volatile-key redaction the wire-schema tests already apply).
#[test]
fn routing_snapshot_matches_on_loop_after_tick() {
@@ -3102,10 +3102,11 @@ mod tests {
);
}
// ---- R4 Category-E coverage ------------------------------------------
// ---- per-entity table coverage ---------------------------------------
/// The six Category-E queries are served off-loop via `snapshot_dispatch`
/// (return `Some` with status ok); mutations take the rx_loop COMMAND path.
/// The six per-entity table queries are served off-loop via
/// `snapshot_dispatch` (return `Some` with status ok); mutations take the
/// rx_loop COMMAND path.
#[test]
fn snapshot_dispatch_serves_category_e_queries() {
use super::super::protocol::Request;
@@ -3128,7 +3129,7 @@ mod tests {
}
// Mutations take the rx_loop COMMAND path. (Every read query is served
// off-loop as of R5.)
// off-loop.)
for cmd in ["connect", "disconnect"] {
assert!(
snapshot_dispatch(&req(cmd), &handle).is_none(),
@@ -3138,7 +3139,7 @@ mod tests {
}
/// Freshness + fidelity: after a `record_stats_history()` tick (the entity
/// publisher site) each off-loop Category-E render equals its on-loop render
/// publisher site) each off-loop per-entity render equals its on-loop render
/// byte-for-byte, and the seeded snapshot is empty before the first tick.
#[test]
fn entity_snapshot_matches_on_loop_after_tick() {
@@ -3188,7 +3189,7 @@ mod tests {
);
}
/// Structural sharing (the R4 umbrella mandate): a republish in which only
/// Structural sharing: a republish in which only
/// one row changed re-allocates only that one `Arc<Row>` — every unchanged
/// row is reused by pointer (`Arc::ptr_eq`). Exercises
/// [`reconcile_rows`](super::super::snapshot::reconcile_rows), the
src/control/read_handle.rs
+50 -42
View File
@@ -2,27 +2,33 @@
//! queries can render off the rx_loop hot path instead of round-tripping the
//! mpsc → rx_loop oneshot.
//!
//! This is the stable seam of the control read-isolation milestone
//! (TASK-2026-0152, phase R0). The handle bundles the state that is already
//! independently shareable, and grows one `ArcSwap` snapshot cell per phase as
//! each subsystem's read state is published from its natural mutator:
//! The handle bundles the node state that is independently shareable, plus one
//! `ArcSwap` snapshot cell per read subsystem:
//!
//! - `context` / `metrics` — already `Arc`-shared (refactor steps B/C).
//! - `stats` (R2) — `ArcSwap<StatsSnapshot>`: stats_history dual-ring + the
//! scalar gauges `show_status` needs, published from the tick.
//! - `routing` (R3) — `ArcSwap<RoutingSnapshot>`: tree / bloom / coord /
//! identity, published from their announce / discovery mutators.
//! - `entities` (R4) — `ArcSwap<EntitySnapshot>`: peers / sessions / links /
//! connections / transports, published per-entity with `Vec<Arc<Row>>`
//! - `context` / `metrics` — already `Arc`-shared.
//! - `stats` — `ArcSwap<StatsSnapshot>`: stats_history dual-ring + the scalar
//! gauges `show_status` needs, published from the tick.
//! - `routing` — `ArcSwap<RoutingSnapshot>`: tree / bloom / coord / identity,
//! published from the tick.
//! - `entities` — `ArcSwap<EntitySnapshot>`: peers / sessions / links /
//! connections / transports, published from the tick with `Vec<Arc<Row>>`
//! structural sharing.
//!
//! Publisher placement follows the Q1 rules in
//! `design/fast-path-refactoring-r0-read-handle.md`: every snapshot is
//! published at its state's natural mutation site (on-change), never by the
//! contended rx_loop task it is meant to bypass.
//! Publisher placement: all three snapshot cells are published from the
//! periodic tick, which runs as one arm of the rx_loop's `select!`. Publishing
//! therefore costs the rx_loop; what the handle removes is the read-side round
//! trip out to the rx_loop and back, not the cost of publishing. The
//! `publish_routing_snapshot` and `publish_entities_snapshot` doc comments on
//! `Node` carry the reasoning for the two projections that need coherent
//! `&Node` access across subsystems.
//!
//! R0 ships only the type and the dispatch seam ([`snapshot_dispatch`]); no
//! query reads the handle yet. Cutover begins in R1.
//! A projection is a point-in-time copy, not a live view. The entity tables in
//! particular are mutated on the packet path between ticks, so a reader sees
//! the state as of the last publish.
//!
//! [`snapshot_dispatch`] is the seam: it serves the commands in its match arms
//! directly from the handle and returns `None` for everything else, so the
//! caller falls back to the mpsc → rx_loop path.
use std::sync::Arc;
@@ -37,9 +43,9 @@ use super::snapshot::{EntitySnapshot, RoutingSnapshot, StatsSnapshot};
/// Cloneable read-only view of node state for off-loop control serving.
///
/// All fields are `Arc` / `ArcSwap` handles, so cloning is cheap and a clone
/// can be held by every accepted control connection. Fields are consumed
/// starting R1 as `show_*` queries cut over to off-loop rendering; until then
/// they are wired but unread.
/// can be held by every accepted control connection. The snapshot cells are
/// read by the `*_from_handle` query functions that [`snapshot_dispatch`]
/// routes to.
#[derive(Clone)]
pub(crate) struct ControlReadHandle {
/// Effectively-immutable node context (config, identity, limits).
@@ -47,14 +53,14 @@ pub(crate) struct ControlReadHandle {
/// Metrics registry (counters / gauges) for `show_stats_*`.
metrics: Arc<MetricsRegistry>,
/// stats_history dual-ring read copy + the scalar gauges/counts
/// `show_status` needs, published from the tick (R2, Q1-b).
/// `show_status` needs, published from the tick.
stats: Arc<ArcSwap<StatsSnapshot>>,
/// Category-D derived/routing/cache read view (tree / bloom / coord /
/// identity + F-queue scalars), published from the tick (R3).
/// Derived/routing/cache read view (tree / bloom / coord /
/// identity + F-queue scalars), published from the tick.
routing: Arc<ArcSwap<RoutingSnapshot>>,
/// Category-E per-entity table read view (peers / sessions / links /
/// Per-entity table read view (peers / sessions / links /
/// connections / transports + mmp), published from the tick with
/// `Vec<Arc<Row>>` structural sharing (R4).
/// `Vec<Arc<Row>>` structural sharing.
entities: Arc<ArcSwap<EntitySnapshot>>,
}
@@ -90,19 +96,19 @@ impl ControlReadHandle {
}
/// Load the latest published stats snapshot (the freshest available by
/// construction; no IO_TIMEOUT staleness gate, per Q1-e).
/// construction; no IO_TIMEOUT staleness gate).
pub(crate) fn stats(&self) -> arc_swap::Guard<Arc<StatsSnapshot>> {
self.stats.load()
}
/// Load the latest published Category-D routing snapshot (freshest
/// available by construction; no staleness gate, per Q1-e).
/// Load the latest published routing snapshot (freshest
/// available by construction; no staleness gate).
pub(crate) fn routing(&self) -> arc_swap::Guard<Arc<RoutingSnapshot>> {
self.routing.load()
}
/// Load the latest published Category-E entity snapshot (freshest available
/// by construction; no staleness gate, per Q1-e).
/// Load the latest published entity snapshot (freshest available
/// by construction; no staleness gate).
pub(crate) fn entities(&self) -> arc_swap::Guard<Arc<EntitySnapshot>> {
self.entities.load()
}
@@ -110,14 +116,16 @@ impl ControlReadHandle {
/// Attempt to serve a request entirely from the read handle, off the rx_loop.
///
/// Returns `Some(response)` when the command is a pure-snapshot query that has
/// been cut over to off-loop rendering, or `None` when it must take the
/// mpsc → rx_loop path (parameterized queries, mutations, and any query not
/// yet cut over).
/// Returns `Some(response)` when the command can be rendered from the bundled
/// snapshot cells, or `None` when it must take the mpsc → rx_loop path.
///
/// Cutover queries (R1) read only `NodeContext` / `MetricsRegistry` (the state
/// the read handle already bundles) plus host-OS facts (`/proc`, nftables), so
/// they render entirely in the control task without touching `Node`.
/// The queries served here read any of the cells the handle bundles —
/// `context`, `metrics`, `stats`, `routing`, `entities` — plus host-OS facts
/// gathered in [`super::listening`] and [`super::firewall_state`], so they
/// render in the control task without touching `Node`. Taking a parameter is
/// not what decides it: `show_stats_history` is parameterized and is served
/// here. What falls back is a query needing live `Node` state the snapshot does
/// not carry, and every mutation.
///
/// **It now also carries mutating commands**, namely the `profile_tick_*`
/// family under the `profiling` feature. They are served here rather than on
@@ -158,18 +166,18 @@ pub(crate) fn snapshot_dispatch(request: &Request, handle: &ControlReadHandle) -
)),
"show_stats_list" => Some(Response::ok(queries::show_stats_list())),
"show_metrics" => Some(Response::ok(queries::show_metrics_from_handle(handle))),
// R5: peer-ACL status, served from the tick-published `StatsSnapshot`.
// Peer-ACL status, served from the tick-published `StatsSnapshot`.
// The ACL is an `arc_swap::ArcSwap<PeerAcl>` reloaded only on the tick;
// its status projection is captured at the same tick.
"show_acl" => Some(Response::ok(queries::show_acl_from_handle(handle))),
// R2: served from the tick-published `StatsSnapshot` (rings + scalar
// Served from the tick-published `StatsSnapshot` (rings + scalar
// gauges/counts). `show_status` and the two node-level/per-peer series
// queries carry enough data in the snapshot to render faithfully
// off-loop, including the parameterized series selectors (the snapshot
// holds the full rings, so any metric / window / granularity is
// satisfiable).
//
// R5 closes out the per-peer stats queries: `show_stats_peers` and
// The per-peer stats queries: `show_stats_peers` and
// `show_stats_history_all_peers` now read the snapshot's per-peer
// `peer_meta` (live `is_active`, resolved npub / display name, captured
// at publish time) joined against the `history` rings, so they no longer
@@ -188,7 +196,7 @@ pub(crate) fn snapshot_dispatch(request: &Request, handle: &ControlReadHandle) -
handle,
request.params.as_ref(),
)),
// R3: served from the tick-published `RoutingSnapshot` (tree / bloom /
// Served from the tick-published `RoutingSnapshot` (tree / bloom /
// coord cache / identity cache + F-queue scalars). Display names are
// resolved at publish time, so these render entirely off-loop. The
// counter-family `stats` blocks come from the `MetricsRegistry` (also
@@ -200,7 +208,7 @@ pub(crate) fn snapshot_dispatch(request: &Request, handle: &ControlReadHandle) -
"show_identity_cache" => Some(Response::ok(queries::show_identity_cache_from_handle(
handle,
))),
// R4: served from the tick-published `EntitySnapshot` (per-entity
// Served from the tick-published `EntitySnapshot` (per-entity
// `Vec<Arc<Row>>` tables with structural sharing). Display names,
// tree-relationship flags, and Nostr-traversal state are resolved at
// publish time, so these render entirely off-loop. All six are
src/control/snapshot.rs
+39 -42
View File
@@ -1,8 +1,7 @@
//! Read-side state snapshots published from the node's natural mutators so
//! pure-snapshot `show_*` queries render off the rx_loop hot path.
//!
//! [`StatsSnapshot`] is the R2 reference implementation of the canonical
//! snapshot pattern (see `design/fast-path-refactoring-r0-read-handle.md`): a
//! [`StatsSnapshot`] is the reference implementation of the canonical snapshot pattern: a
//! read-only data bundle published via `ArcSwap` from the tick after
//! `StatsHistory::tick()`. It carries
//!
@@ -13,10 +12,10 @@
//! peer / session / link / connection / transport counts), plus
//! `peer_aliases` (effectively immutable after construction).
//!
//! The snapshot holds *data*, not rendered `Response` envelopes (Q1-d):
//! The snapshot holds *data*, not rendered `Response` envelopes:
//! rendering happens in the control task off the rx_loop. Staleness is bounded
//! by the tick interval and is never staler than the underlying data, which
//! also advances only on the tick (Q1-b).
//! also advances only on the tick.
use std::collections::HashMap;
use std::sync::Arc;
@@ -28,7 +27,7 @@ use crate::node::stats_history::StatsHistory;
use crate::upper::tun::TunState;
/// Read-only snapshot of the stats-history rings plus the scalar gauges and
/// counts `show_status` reports. Published from the tick (Q1-b).
/// counts `show_status` reports. Published from the tick.
#[derive(Clone)]
pub(crate) struct StatsSnapshot {
/// Cloned read copy of the history rings (the dual-ring read side).
@@ -66,14 +65,14 @@ pub(crate) struct StatsSnapshot {
pub peer_aliases: Arc<HashMap<NodeAddr, String>>,
/// Loaded peer-ACL status (`show_acl`). The ACL itself is an
/// `arc_swap::ArcSwap<PeerAcl>` mutated only by the tick's `reload_peer_acl`;
/// the human-readable status is a cheap projection of it (R5).
/// the human-readable status is a cheap projection of it.
pub acl_status: PeerAclStatus,
/// Per-stats-history-peer metadata resolved against the live peer/session
/// tables and host map at publish time (`show_stats_peers` /
/// `show_stats_history_all_peers`), keyed by `NodeAddr`. The lifecycle
/// timestamps and per-peer metric rings stay in `history`; this map carries
/// only the cross-subsystem fields a renderer can't derive from the rings
/// alone (`is_active`, resolved `npub`, resolved `display_name`) (R5).
/// alone (`is_active`, resolved `npub`, resolved `display_name`).
pub peer_meta: Arc<HashMap<NodeAddr, StatsPeerMeta>>,
}
@@ -138,34 +137,34 @@ fn empty_acl_status() -> PeerAclStatus {
}
// =====================================================================
// RoutingSnapshot (R3 — Category-D derived/routing/cache read view)
// RoutingSnapshot (derived/routing/cache read view)
// =====================================================================
/// Read-only snapshot of the Category-D derived/routing/cache subsystems that
/// Read-only snapshot of the derived/routing/cache subsystems that
/// the pure-snapshot `show_tree` / `show_bloom` / `show_cache` / `show_routing`
/// / `show_identity_cache` queries render. Published via `ArcSwap`.
///
/// The R0 stub (`design/fast-path-refactoring-r0-read-handle.md`) names a
/// single combined `ArcSwap<RoutingSnapshot>` for R3. This is that cell: one
/// This is the single combined `ArcSwap<RoutingSnapshot>` cell for the routing
/// subsystems: one
/// cohesive routing view holding the four subsystems (tree / bloom / coord
/// cache / identity cache) plus the F-queue summary scalars.
///
/// **Publisher placement (Q1).** The four subsystems mutate at many scattered
/// **Publisher placement.** The four subsystems mutate at many scattered
/// handler sites (28 `coord_cache_mut` call sites, 16 `tree_state_mut`, ~32
/// identity-cache touches), and every projected row needs a *display name*
/// resolved against the live peer/session tables and host map — Category-E
/// resolved against the live peer/session tables and host map — per-entity
/// state reachable only with `&Node`. Wiring an on-change `publish_*` at each
/// mutation site would be large, error-prone surgery, and each call would still
/// need `&Node` to resolve names across subsystem boundaries. So this snapshot
/// is published from the **tick** (Q1-b acceptable-at-mutator / the documented
/// interim the spec permits, mirroring R2's stats publish): the tick is the one
/// site with coherent `&Node` access to resolve all display names together. A
/// is published from the **tick**, the same placement the stats snapshot
/// above uses: the tick is the one site with coherent `&Node` access to
/// resolve all display names together. A
/// single combined cell is the natural shape because there is exactly one
/// publisher — the multi-mutator "rebuild the whole snapshot N times" hazard
/// that Q1-c warns against does not arise.
/// does not arise.
///
/// The snapshot holds *data* (typed rows + scalars), not rendered `Response`
/// envelopes (Q1-d); rendering happens off the rx_loop in the control task. The
/// envelopes; rendering happens off the rx_loop in the control task. The
/// counter-family `stats` blocks the queries also emit come from the
/// `MetricsRegistry` (already `Arc`-shared in the handle) at render time, not
/// from this snapshot.
@@ -174,9 +173,9 @@ fn empty_acl_status() -> PeerAclStatus {
/// the captured absolute timestamps, so the rendered age stays fresh relative
/// to the read, exactly as the on-loop queries computed it.
///
/// Forward-compat: when step 5 structurally extracts the Category-D subsystems
/// into typed types, these projections become thin views over them without
/// changing the read-handle interface or this publisher placement.
/// Forward-compat: if the derived/routing/cache subsystems are later
/// extracted into typed types, these projections become thin views over them
/// without changing the read-handle interface or this publisher placement.
#[derive(Clone)]
pub(crate) struct RoutingSnapshot {
/// Spanning-tree read view (`show_tree`).
@@ -397,20 +396,18 @@ pub(crate) struct IdentityRow {
}
// =====================================================================
// EntitySnapshot (R4 — Category-E per-entity table read views)
// EntitySnapshot (per-entity table read views)
// =====================================================================
/// Read-only snapshot of the Category-E per-entity tables that the
/// Read-only snapshot of the per-entity tables that the
/// pure-snapshot `show_peers` / `show_sessions` / `show_links` /
/// `show_connections` / `show_transports` / `show_mmp` queries render.
/// Published via `ArcSwap`.
///
/// The R0 stub (`design/fast-path-refactoring-r0-read-handle.md`) pre-scopes
/// R4 as `entities — ArcSwap<EntitySnapshot>: peers / sessions / links /
/// connections / transports, published per-entity with `Vec<Arc<Row>>`
/// structural sharing`. This is that cell.
/// This is the `entities` cell: peers / sessions / links / connections /
/// transports, published per-entity with `Vec<Arc<Row>>` structural sharing.
///
/// **Structural sharing (the umbrella mandate).** Every entity table is a
/// **Structural sharing.** Every entity table is a
/// `Vec<Arc<Row>>`, so a republish in which only one row changed re-allocates
/// only that one `Arc<Row>` — the unchanged rows are reused by pointer from the
/// previous snapshot (`Arc::ptr_eq`-stable). The publisher diffs each freshly
@@ -418,10 +415,11 @@ pub(crate) struct IdentityRow {
/// keeps the old `Arc` when they are equal. A clone of the snapshot for each
/// accepted control connection is then a vector of cheap pointer clones, not a
/// deep table copy. This is what keeps the per-tick publish cost off the hot
/// path at scale, as the umbrella requires for R4.
/// path at scale.
///
/// **Publisher placement (Q1).** Like R3, this is published from the **tick**,
/// not per-mutator. Two reasons, both stronger than for R3:
/// **Publisher placement.** Like the routing snapshot, this is published from
/// the **tick**, not per-mutator. Two reasons, both stronger here than for the
/// routing snapshot:
///
/// 1. Every projected row needs a *display name* resolved against the live
/// peer/session tables and host map (`&Node`), and `show_peers` additionally
@@ -432,23 +430,22 @@ pub(crate) struct IdentityRow {
/// metrics, `last_seen`, noise counters, replay/decrypt counters) are
/// mutated continuously on the **data plane / rx_loop**, not at the discrete
/// peer/session/link lifecycle mutators. Per-lifecycle-mutator publication
/// (Q1-a) would therefore not even capture freshness for those fields; the
/// would therefore not even capture freshness for those fields; the
/// tick is the natural cadence at which this read view advances.
///
/// The diff-and-reuse therefore satisfies the structural-sharing goal the
/// umbrella mandates (only changed rows re-allocate) while keeping a single
/// coherent `&Node` publisher — the "no monolithic per-tick *re-allocation* of
/// every row" warning is honored because unchanged rows are reused, not rebuilt.
/// This is the documented acceptable interim (the spec's tick-publish-with-
/// Arc-reuse fallback), consistent with R3.
/// The diff-and-reuse therefore satisfies the structural-sharing goal (only
/// changed rows re-allocate) while keeping a single coherent `&Node`
/// publisher: there is no monolithic per-tick *re-allocation* of every row,
/// because unchanged rows are reused rather than rebuilt. This is the same
/// tick publish placement the routing snapshot uses, for the same reason.
///
/// The snapshot holds typed rows (Q1-d data, not rendered `Response`
/// The snapshot holds typed rows (data, not rendered `Response`
/// envelopes). Time-relative fields (`idle_ms`) are derived at render time from
/// captured absolute timestamps, so the rendered age stays fresh relative to
/// the read, exactly as the on-loop queries computed it.
///
/// Forward-compat: step 10 later extracts the session table into a typed
/// `(transport_id, our_index)`-indexed type; these projections then become thin
/// Forward-compat: if the session table is later extracted into a typed
/// `(transport_id, our_index)`-indexed type, these projections become thin
/// views over it without changing the read-handle interface or this publisher
/// placement.
#[derive(Clone)]
@@ -736,7 +733,7 @@ pub(crate) struct MmpSessionRow {
/// one, preserving structural sharing: an `Arc<Row>` from `prev` is reused
/// (kept by pointer) whenever a new row matches an old row by identity `key`
/// **and** compares equal by value, so only changed/new rows allocate a fresh
/// `Arc`. This is the `Vec<Arc<Row>>` discipline the R4 umbrella mandates — a
/// `Arc`. This is the `Vec<Arc<Row>>` structural-sharing discipline — a
/// single-row change re-allocates one row, not the whole table, keeping the
/// per-tick publish cost off the hot path at scale.
///
src/gateway/nat.rs
+2 -2
View File
@@ -66,7 +66,7 @@ pub struct NatManager {
lan_interface: String,
/// Active mappings keyed by virtual IP.
mappings: HashMap<Ipv6Addr, NatMapping>,
/// Inbound port-forward rules (TASK-2026-0061).
/// Inbound port-forward rules.
port_forwards: Vec<PortForward>,
}
@@ -235,7 +235,7 @@ impl NatManager {
batch.add(&snat_rule, MsgType::Add);
}
// Inbound port-forward rules (TASK-2026-0061). Each forward is
// Inbound port-forward rules. Each forward is
// one DNAT rule in prerouting keyed on (iif fips0, nfproto ipv6,
// l4proto, th dport). When any forwards are configured, emit a
// single LAN-side masquerade in postrouting so the LAN target
src/identity/encoding.rs
+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()));
}
src/identity/local.rs
+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")
src/identity/mod.rs
+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;
src/node/dataplane/rx_loop.rs
+40 -1
View File
@@ -1,9 +1,11 @@
//! RX event loop and packet dispatch.
use crate::control::{ControlSocket, commands};
use crate::node::reject::{RejectReason, TransportReject};
use crate::node::{Node, NodeError};
use crate::proto::fmp::wire::{
COMMON_PREFIX_SIZE, CommonPrefix, FMP_VERSION, PHASE_ESTABLISHED, PHASE_MSG1, PHASE_MSG2,
expected_payload_len,
};
use crate::transport::ReceivedPacket;
use std::time::Duration;
@@ -448,7 +450,11 @@ impl Node {
/// Process a single received packet.
///
/// Dispatches based on the phase field in the 4-byte common prefix.
async fn process_packet(&mut self, packet: ReceivedPacket) {
///
/// Visible to the rest of `crate::node` so tests can drive a single
/// packet through the dispatch, the same reach `handle_msg1` and
/// `handle_msg2` already have.
pub(in crate::node) async fn process_packet(&mut self, packet: ReceivedPacket) {
if packet.data.len() < COMMON_PREFIX_SIZE {
return; // Drop packets too short for common prefix
}
@@ -499,6 +505,39 @@ impl Node {
return;
}
// Drop a frame whose declared payload length disagrees with the
// frame that arrived, before that field can be used as a parsing
// input.
//
// Every transport's packets converge here, but the two families
// reach this line differently. TCP, Tor and Nym read their frame
// boundary out of this same field, so for them the comparison holds
// by construction and never fires. UDP, Ethernet and BLE deliver one
// whole frame per packet, where the arrived length is known exactly
// and nothing compares the two today. A short read on those
// transports is a truncated frame, which fails the AEAD tag or the
// exact-size handshake parse already; this changes which reason it
// is dropped for, not whether it is dropped.
//
// A `None` means the phase carries no fixed relationship and the
// frame is left alone rather than rejected, so an unrecognised phase
// still reaches the dispatch below and is handled there.
if let Some(expected) = expected_payload_len(prefix.phase, packet.data.len())
&& prefix.payload_len != expected
{
debug!(
phase = prefix.phase,
declared = prefix.payload_len,
expected,
len = packet.data.len(),
transport_id = %packet.transport_id,
"FMP payload_len disagrees with frame length, dropping"
);
self.stats_mut()
.record_reject(RejectReason::Transport(TransportReject::PayloadLenMismatch));
return;
}
match prefix.phase {
PHASE_ESTABLISHED => {
self.handle_encrypted_frame(packet).await;
src/node/encrypt_worker.rs
+4 -2
View File
@@ -93,8 +93,10 @@ use tracing::{debug, trace, warn};
/// inside the worker. That second alloc + ~1.5 KB memcpy per packet at
/// line rate cost ~150 MB/sec of memory bandwidth on the hot worker.)
pub(crate) struct FmpSendJob {
/// Cloned FMP send cipher. `LessSafeKey` is `Clone` (`ring::aead`)
/// — the clone is just a refcount bump on the inner key material.
/// Cloned FMP send cipher. `LessSafeKey` is `Clone` (`ring::aead`), but
/// the clone is not a refcount bump: `ring` stores the ChaCha20 key
/// inline as `[u32; 8]`, so cloning copies the key material outright and
/// leaves a second copy that nothing outside `ring` can clear.
pub cipher: LessSafeKey,
/// Pre-reserved monotonic counter (via `take_send_counter`).
pub counter: u64,
src/node/handlers/handshake.rs
+142 -3
View File
@@ -22,6 +22,28 @@ use crate::utils::index::SessionIndex;
use std::time::Duration;
use tracing::{debug, info, warn};
/// Why an inbound msg1 got past the `accept_connections` gate, and against
/// what identity the post-DH confirmation must check it.
///
/// Three outcomes, not two: an `Option` would conflate "no waiver was needed"
/// with "the waiver was used and nobody owns the matched address", and the
/// second of those is the case that must reject.
#[derive(Debug, PartialEq, Eq)]
pub(in crate::node) enum Msg1Waiver {
/// The transport accepts fresh inbound handshakes (or no transport is
/// registered), so the address carve-out did not admit this msg1 and
/// there is nothing to confirm.
NotNeeded,
/// The carve-out is what admitted this msg1, and the matched address
/// belongs to this identity: either a promoted peer, or a handshake
/// already in flight on the matched link whose identity is expected
/// (outbound dial) or already learned (inbound msg1).
Expect(NodeAddr),
/// The carve-out is what admitted this msg1, and no identity can be
/// attributed to the matched address. Fail closed: reject after the DH.
Unattributed,
}
impl EstablishView for Node {
fn establish_snapshot(&self, peer_addr: &NodeAddr) -> EstablishSnapshot {
let existing = self.peers.get(peer_addr);
@@ -169,6 +191,80 @@ impl Node {
false
}
/// Classify the msg1 waiver for a source that `should_admit_msg1`
/// admitted, so the post-DH confirmation knows whether it has an
/// identity to check against and what to do when it has none.
///
/// `established` is the caller's already-computed
/// `is_established_link_msg1(...)`, so the O(peers) scan is not repeated
/// on the refusal path.
///
/// The two attribution limbs are composed the same way
/// `is_established_link_msg1` composes its own: as an OR, not as an
/// if/else. An `addr_to_link` entry that yields no identity must not
/// short-circuit the address scan, because the two keys can be different
/// forms of the same peer's address (the hostname-vs-numeric case that
/// predicate 2 exists for) and the entry can outlive the link it named.
pub(in crate::node) fn msg1_waiver(
&self,
established: bool,
transport_id: crate::transport::TransportId,
remote_addr: &crate::transport::TransportAddr,
) -> Msg1Waiver {
// The carve-out only admits anything when the gate would otherwise
// refuse, so an accepting transport has nothing to confirm.
if self
.transports
.get(&transport_id)
.is_none_or(|t| t.accept_connections())
{
return Msg1Waiver::NotNeeded;
}
if !established {
// `should_admit_msg1` refused this msg1 and the caller returned,
// so this arm is unreachable from the one call site. Fail closed
// rather than skipping the check, so a second caller cannot
// reintroduce the hole this classifier exists to close.
return Msg1Waiver::Unattributed;
}
// Predicate 1: the reverse-address lookup.
if let Some(&link_id) = self.addr_to_link.get(&(transport_id, remote_addr.clone())) {
if let Some(peer) = self.peers.values().find(|p| p.link_id() == link_id) {
return Msg1Waiver::Expect(*peer.node_addr());
}
// A link with no promoted peer: a dial in progress or an inbound
// handshake in flight. Both register a connection carrying the
// expected (outbound) or learned (inbound) identity.
if let Some(id) = self
.connections()
.find(|(id, _)| **id == link_id)
.and_then(|(_, machine)| machine.conn_expected_identity())
{
return Msg1Waiver::Expect(*id.node_addr());
}
// Deliberately fall through instead of returning. The entry can
// name a link that no longer exists — `remove_link` clears the
// reverse lookup only under the key it rebuilds from the link's
// own remote address, so an entry inserted under a second
// address form for that link survives its removal. Rejecting
// here would refuse a peer predicate 2 can still attribute, and
// would refuse it permanently: this classifier's caller returns
// above the insert that overwrites the stale entry, so nothing
// downstream would ever repair the map.
}
// Predicate 2: the address scan over promoted peers, which always
// yields an identity when it matches.
self.peers
.values()
.find(|p| {
p.transport_id() == Some(transport_id) && p.current_addr() == Some(remote_addr)
})
.map(|p| Msg1Waiver::Expect(*p.node_addr()))
.unwrap_or(Msg1Waiver::Unattributed)
}
/// Returns true if an inbound msg1 should be admitted past the
/// `accept_connections` gate.
///
@@ -250,6 +346,12 @@ impl Node {
return;
}
// Snapshot which identity, if any, the address carve-out attributed
// this source to. Taken here rather than after the DH so the answer
// is the one the gate acted on. On an accepting transport this is one
// map lookup and a return.
let waiver = self.msg1_waiver(established, packet.transport_id, &packet.remote_addr);
// Parse header
let header = match Msg1Header::parse(&packet.data) {
Some(h) => h,
@@ -334,14 +436,18 @@ impl Node {
machine.set_conn_source_addr(packet.remote_addr.clone());
machine.set_leg(HandshakeCrypto::new());
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 machine.receive_handshake_init(
let init_result = machine.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!(
@@ -368,6 +474,39 @@ impl Node {
let peer_node_addr = *peer_identity.node_addr();
// The address carve-out admitted this msg1 past a refusing gate on
// the strength of the source address alone. Now that the DH has
// revealed the initiator's static, confirm it belongs to the party
// that address is attributed to; an off-path party sourcing from an
// established peer's address gets no further than here. Cheap
// rejection is unchanged: a stranger under accept_connections=false
// is still refused above, having paid nothing.
match waiver {
Msg1Waiver::NotNeeded => {}
Msg1Waiver::Expect(expected) if expected == peer_node_addr => {}
Msg1Waiver::Expect(expected) => {
warn!(
expected = %self.peer_display_name(&expected),
actual = %self.peer_display_name(&peer_node_addr),
transport_id = %packet.transport_id,
"Msg1 admitted by the established-address waiver carries a different identity, dropping"
);
self.stats_mut()
.record_reject(RejectReason::Handshake(HandshakeReject::BadState));
return;
}
Msg1Waiver::Unattributed => {
warn!(
actual = %self.peer_display_name(&peer_node_addr),
transport_id = %packet.transport_id,
"Msg1 admitted by the established-address waiver, but no identity owns that address, dropping"
);
self.stats_mut()
.record_reject(RejectReason::Handshake(HandshakeReject::BadState));
return;
}
}
// === PHASE B result ===
// Bundle the Noise wire-step outputs (identity, remote epoch, sender
// index, opaque msg2 payload). The wire step touched no `Node` registry
src/node/handlers/mod.rs
+1 -1
View File
@@ -1,6 +1,6 @@
//! Message handlers: per-message-type behavior on `impl Node`.
mod handshake;
pub(in crate::node) mod handshake;
pub(crate) mod lookup;
mod mmp;
mod rekey;
src/node/handlers/rekey.rs
+8 -2
View File
@@ -298,8 +298,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() {
@@ -680,8 +683,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() {
src/node/handlers/session.rs
+17 -4
View File
@@ -645,8 +645,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) {
@@ -700,8 +703,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) {
@@ -739,7 +745,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(
@@ -2016,8 +2026,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()
src/node/lifecycle/mod.rs
+7 -4
View File
@@ -610,14 +610,17 @@ impl Node {
};
// Start the Noise handshake and get message 1
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 startup_epoch = self.startup_epoch();
let noise_msg1 = match self
let start_result = self
.peer_machines
.get_mut(&link_id)
.expect("dial-time machine carries the connection")
.start_handshake(our_keypair, startup_epoch, current_time_ms)
{
.start_handshake(our_keypair, 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, link, and dial-time machine
src/node/reject.rs
+25 -4
View File
@@ -298,10 +298,10 @@ pub enum ForwardingReject {
/// Transport-layer rejection reasons.
///
/// Currently covers the admission cap-hit path at the TCP and Tor
/// accept loops. Additional transport-side rejection variants
/// (framing errors, connection failures wired through to the node
/// stats path) can be added incrementally.
/// Covers the admission cap-hit path at the TCP and Tor accept loops
/// and the frame-length check at the receive dispatch point. Additional
/// transport-side rejection variants (connection failures wired through
/// to the node stats path) can be added incrementally.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[non_exhaustive]
pub enum TransportReject {
@@ -310,6 +310,13 @@ pub enum TransportReject {
/// (`max_inbound_connections`) was already reached. Tracked via
/// [`TransportStats::inbound_cap_exceeded`](crate::node::stats::TransportStats).
InboundCapExceeded,
/// Inbound FMP frame dropped because the payload length its header
/// declares does not match the frame the transport delivered. This
/// is a framing rejection, not an admission one: it applies to
/// established data frames as well as to handshake frames, and it
/// is decided before the phase dispatch. Tracked via
/// [`TransportStats::payload_len_mismatch`](crate::node::stats::TransportStats).
PayloadLenMismatch,
}
#[cfg(test)]
@@ -403,4 +410,18 @@ mod tests {
RejectReason::Transport(TransportReject::InboundCapExceeded)
));
}
#[test]
fn transport_reject_payload_len_mismatch_round_trips() {
let r = RejectReason::Transport(TransportReject::PayloadLenMismatch);
assert!(matches!(
r,
RejectReason::Transport(TransportReject::PayloadLenMismatch)
));
assert_ne!(
r,
RejectReason::Transport(TransportReject::InboundCapExceeded),
"a framing drop must not compare equal to an admission rejection"
);
}
}
src/node/stats.rs
+32
View File
@@ -202,6 +202,10 @@ impl MmpStats {
/// typed-rejection enum stays the canonical entry point and so a
/// future transport-to-node bridge (event or sampling) has a
/// well-known destination.
///
/// `payload_len_mismatch` is different in kind: it counts a framing
/// drop the node itself performs at the receive dispatch point, so it
/// has a live writer and can be non-zero on a running node.
#[derive(Default)]
pub struct TransportStats {
/// Reserved for node-side inbound-cap-exceeded admission rejection
@@ -209,18 +213,24 @@ pub struct TransportStats {
/// on the transport-level stats (`TcpStats::connections_rejected`,
/// `TorStats::connections_rejected`) directly.
pub inbound_cap_exceeded: u64,
/// Inbound FMP frames dropped because the payload length declared
/// in the common prefix did not match the frame the transport
/// delivered.
pub payload_len_mismatch: u64,
}
impl TransportStats {
pub fn snapshot(&self) -> TransportStatsSnapshot {
TransportStatsSnapshot {
inbound_cap_exceeded: self.inbound_cap_exceeded,
payload_len_mismatch: self.payload_len_mismatch,
}
}
pub(super) fn record_reject(&mut self, reason: TransportReject) {
match reason {
TransportReject::InboundCapExceeded => self.inbound_cap_exceeded += 1,
TransportReject::PayloadLenMismatch => self.payload_len_mismatch += 1,
}
}
}
@@ -396,6 +406,7 @@ pub struct MmpStatsSnapshot {
#[derive(Clone, Debug, Default, Serialize)]
pub struct TransportStatsSnapshot {
pub inbound_cap_exceeded: u64,
pub payload_len_mismatch: u64,
}
#[derive(Clone, Debug, Default, Serialize)]
@@ -577,4 +588,25 @@ mod tests {
stats.record_reject(RejectReason::Transport(TransportReject::InboundCapExceeded));
assert_eq!(stats.transport.inbound_cap_exceeded, 1);
}
/// Records both transport reasons so a swapped or shared arm shows up
/// as a mis-attributed counter rather than as a plausible total.
#[test]
fn transport_stats_record_reject_keeps_the_two_reasons_on_separate_counters() {
let mut s = TransportStats::default();
s.record_reject(TransportReject::PayloadLenMismatch);
s.record_reject(TransportReject::PayloadLenMismatch);
s.record_reject(TransportReject::InboundCapExceeded);
assert_eq!(s.payload_len_mismatch, 2);
assert_eq!(s.inbound_cap_exceeded, 1);
}
#[test]
fn node_stats_record_reject_dispatches_payload_len_mismatch_to_transport() {
let mut stats = NodeStats::new();
stats.record_reject(RejectReason::Transport(TransportReject::PayloadLenMismatch));
assert_eq!(stats.transport.payload_len_mismatch, 1);
assert_eq!(stats.transport.inbound_cap_exceeded, 0);
assert_eq!(stats.handshake.bad_state, 0);
}
}
src/node/tests/handshake.rs
+660 -2
View File
@@ -1129,7 +1129,7 @@ async fn test_should_admit_msg1_rejects_fresh_when_accept_off() {
assert!(!node.should_admit_msg1(transport_id, &addr));
}
/// ISSUE-2026-0004 regression test: `should_admit_msg1` admits rekey/restart
/// Regression test: `should_admit_msg1` admits rekey/restart
/// msg1 from a peer with an existing link even when the transport has
/// accept_connections=false. Without this, the dual-init tie-breaker
/// deadlocks (the larger-NodeAddr side drops the winner's rekey msg1).
@@ -1201,7 +1201,8 @@ async fn test_should_admit_msg1_admits_rekey_when_udp_accept_off() {
}
/// Regression test for the udp.outbound_only rekey loop observed in
/// production 2026-04-30 (parallel to ISSUE-2026-0004).
/// production 2026-04-30 (parallel to the rekey/restart admission case
/// above).
///
/// Production scenario: nomad runs `udp.outbound_only=true` with peer
/// core-vm configured by hostname (`core-vm.tail65015.ts.net:2121`).
@@ -1282,3 +1283,660 @@ async fn test_should_admit_msg1_admits_rekey_when_addr_form_differs() {
assert!(node.is_established_link_msg1(transport_id, &numeric_addr));
assert!(!node.is_established_link_msg1(transport_id, &stranger_addr));
}
// ============================================================================
// Frame-length validation at the dispatch point
// ============================================================================
/// Build a promoted peer and return the node, the peer's address, and the
/// session index inbound frames must name to reach it.
///
/// `handle_encrypted_frame` looks a frame up by `(transport_id, receiver_idx)`
/// in `peers_by_index`, so a frame carrying this index reaches the decrypt and
/// bumps the peer's failure counter. That counter is how the tests below tell
/// "the frame reached its handler" apart from "the frame was dropped before
/// the dispatch": an unknown session is dropped silently and counts nothing.
fn node_with_promoted_peer(transport_id: TransportId) -> (Node, NodeAddr, SessionIndex) {
let mut node = make_node();
let link_id = LinkId::new(1);
let identity = seed_completed_connection(&mut node, link_id, transport_id, 1_000);
let node_addr = *identity.node_addr();
node.promote_connection(link_id, identity, 2_000).unwrap();
let our_index = node
.get_peer(&node_addr)
.and_then(|p| p.our_index())
.expect("promoted peer must have our_index");
(node, node_addr, our_index)
}
/// A well-formed established frame carrying 40 bytes of inner plaintext.
///
/// 16-byte header + 40 + 16-byte tag = 72 bytes on the wire, declaring 40.
/// The ciphertext is filler: these tests are about the length check, and
/// every one of them stops before or at the AEAD.
fn established_frame_declaring_40(receiver_idx: SessionIndex) -> Vec<u8> {
use crate::noise::TAG_SIZE;
use crate::proto::fmp::wire::{build_encrypted, build_established_header};
let header = build_established_header(receiver_idx, 0, 0, 40);
build_encrypted(&header, &[0u8; 40 + TAG_SIZE])
}
#[tokio::test]
async fn an_established_frame_whose_declared_payload_len_disagrees_with_its_length_is_dropped() {
let transport_id = TransportId::new(1);
let (mut node, node_addr, our_index) = node_with_promoted_peer(transport_id);
let mut frame = established_frame_declaring_40(our_index);
// Bytes 2-3 are the little-endian payload_len. 68 is what a validator
// written to the field's looser description would compute for this frame
// (72 on the wire minus the 4-byte common prefix), so it is both a wrong
// value and the specific wrong value worth naming.
frame[2..4].copy_from_slice(&68u16.to_le_bytes());
node.process_packet(ReceivedPacket::new(
transport_id,
TransportAddr::from_string("127.0.0.1:2121"),
frame,
))
.await;
assert_eq!(
node.stats().transport.payload_len_mismatch,
1,
"the frame must be counted as a framing drop"
);
assert_eq!(
node.get_peer(&node_addr)
.expect("peer must survive a dropped frame")
.consecutive_decrypt_failures(),
0,
"the frame must be dropped before the phase dispatch, so the \
encrypted-frame handler never sees it"
);
}
#[tokio::test]
async fn an_established_frame_with_a_correct_payload_len_is_not_dropped() {
let transport_id = TransportId::new(1);
let (mut node, node_addr, our_index) = node_with_promoted_peer(transport_id);
let frame = established_frame_declaring_40(our_index);
node.process_packet(ReceivedPacket::new(
transport_id,
TransportAddr::from_string("127.0.0.1:2121"),
frame,
))
.await;
assert_eq!(
node.stats().transport.payload_len_mismatch,
0,
"a frame whose header agrees with its length must not be dropped"
);
assert_eq!(
node.get_peer(&node_addr)
.expect("peer must survive a failed decrypt below the threshold")
.consecutive_decrypt_failures(),
1,
"the frame must reach the encrypted-frame handler, where the filler \
ciphertext fails the AEAD tag"
);
}
#[tokio::test]
async fn a_msg1_with_a_correct_payload_len_is_not_dropped() {
use crate::proto::fmp::wire::build_msg1;
let mut node = make_node();
let transport_id = TransportId::new(1);
// A real-shaped msg1 with filler Noise bytes: `build_msg1` writes the
// only payload_len the msg1 arm accepts, and the handshake fails one
// step later at the DH, which is the observable that it got there.
let frame = build_msg1(SessionIndex::new(1), &[0u8; 106]);
node.process_packet(ReceivedPacket::new(
transport_id,
TransportAddr::from_string("127.0.0.1:2121"),
frame,
))
.await;
assert_eq!(
node.stats().transport.payload_len_mismatch,
0,
"a msg1 built by the encoder must not be dropped by the length check"
);
assert_eq!(
node.stats().handshake.bad_state,
1,
"the msg1 must reach handle_msg1, which rejects the filler Noise \
payload at the DH"
);
}
// ============================================================================
// Identity confirmation after the DH (the address-keyed msg1 carve-out)
// ============================================================================
/// A node whose only transport refuses fresh inbound handshakes, paired with
/// a second started UDP socket standing in for the far end.
///
/// Returns the node, the far end's address, the far end's receive channel,
/// and the far end's transport, which the caller must keep alive for its
/// receive loop to go on running.
///
/// Both transports are started on purpose. A msg2 the node decides to send
/// then really leaves it and really arrives on the returned channel, which is
/// what makes "no msg2 was sent" an observation about the confirmation rather
/// than a property of the fixture: on an unstarted transport every send
/// fails, and the send-failure arm records the same reject the confirmation
/// records, so the two worlds would be indistinguishable.
async fn node_refusing_inbound(
transport_id: TransportId,
) -> (
Node,
TransportAddr,
crate::transport::PacketRx,
crate::transport::udp::UdpTransport,
) {
use crate::config::UdpConfig;
use crate::transport::udp::UdpTransport;
let mut node = make_node();
let refusing = UdpConfig {
bind_addr: Some("127.0.0.1:0".to_string()),
accept_connections: Some(false),
..Default::default()
};
let (tx, _rx) = packet_channel(64);
let mut udp = UdpTransport::new(transport_id, None, refusing, tx);
udp.start_async().await.expect("node transport must bind");
node.transports
.insert(transport_id, TransportHandle::Udp(udp));
let far_end_config = UdpConfig {
bind_addr: Some("127.0.0.1:0".to_string()),
..Default::default()
};
let (far_tx, far_rx) = packet_channel(64);
let mut far_end = UdpTransport::new(TransportId::new(200), None, far_end_config, far_tx);
far_end.start_async().await.expect("far end must bind");
let far_addr = TransportAddr::from_string(
&far_end
.local_addr()
.expect("a started transport has a local address")
.to_string(),
);
(node, far_addr, far_rx, far_end)
}
/// A real, cryptographically valid msg1 from `initiator` addressed to
/// `responder`'s static key.
///
/// It has to be valid under our static, or `receive_handshake_init` refuses
/// it for the wrong reason and the test passes without ever reaching the
/// confirmation.
fn genuine_msg1(initiator: &Node, responder: &Node) -> Vec<u8> {
use crate::proto::fmp::wire::build_msg1;
let responder_identity = PeerIdentity::from_pubkey_full(responder.identity().pubkey_full());
let mut machine = outbound_leg(LinkId::new(9_999), responder_identity, 1_000);
let noise_msg1 = machine
.start_handshake(
initiator.identity().keypair(),
initiator.startup_epoch(),
1_000,
)
.expect("the initiator side of a real msg1 must build");
build_msg1(SessionIndex::new(7), &noise_msg1)
}
/// The node address a `Node`'s own identity presents to its peers.
fn node_addr_of(node: &Node) -> NodeAddr {
*PeerIdentity::from_pubkey_full(node.identity().pubkey_full()).node_addr()
}
/// The FMP phase byte of a packet, for telling a msg1 from a msg2 on the wire.
fn wire_phase(data: &[u8]) -> Option<u8> {
crate::proto::fmp::wire::CommonPrefix::parse(data).map(|p| p.phase)
}
/// Assert that nothing arrives on `rx` within a window long enough for a
/// localhost datagram to have been delivered had one been sent.
async fn assert_nothing_sent(rx: &mut crate::transport::PacketRx, why: &str) {
let arrival = tokio::time::timeout(std::time::Duration::from_millis(250), rx.recv()).await;
assert!(arrival.is_err(), "{}", why);
}
#[tokio::test]
async fn a_msg1_spoofed_from_an_established_peers_address_is_dropped_after_the_dh_reveals_a_different_identity()
{
use crate::peer::ActivePeer;
let transport_id = TransportId::new(1);
let (mut node, victim_addr, mut far_rx, _far_end) = node_refusing_inbound(transport_id).await;
// The victim: a promoted peer at the address the spoofed msg1 will be
// sourced from, so the carve-out admits the msg1 past the refusing gate.
let victim = make_node();
let victim_identity = PeerIdentity::from_pubkey_full(victim.identity().pubkey_full());
let victim_node_addr = *victim_identity.node_addr();
let victim_link = node.allocate_link_id();
let mut victim_peer = ActivePeer::new(victim_identity, victim_link, 1_000);
victim_peer.set_current_addr(transport_id, victim_addr.clone());
node.peers.insert(victim_node_addr, victim_peer);
node.addr_to_link
.insert((transport_id, victim_addr.clone()), victim_link);
// The off-path party: a genuine msg1 under our static, built with a
// different identity's keypair, sourced from the victim's address.
let attacker = make_node();
let attacker_node_addr = node_addr_of(&attacker);
let wire_msg1 = genuine_msg1(&attacker, &node);
let bad_state_before = node.stats().handshake.bad_state;
node.handle_msg1(ReceivedPacket::with_timestamp(
transport_id,
victim_addr.clone(),
wire_msg1,
2_000,
))
.await;
assert!(
node.get_peer(&attacker_node_addr).is_none(),
"the identity the DH revealed does not own the address the waiver \
matched, so it must not become a peer"
);
assert_eq!(
node.peer_count(),
1,
"only the victim may remain a peer after the spoofed msg1"
);
assert_eq!(
node.connection_count(),
0,
"the rejected msg1 must leave no connection behind"
);
assert_eq!(
node.link_count(),
0,
"the rejected msg1 must leave no link behind"
);
assert_eq!(
node.stats().handshake.bad_state - bad_state_before,
1,
"the drop must be counted"
);
assert_nothing_sent(
&mut far_rx,
"no msg2 may reach the victim's address: the responder answers only \
after the confirmation, and this msg1 must not get that far",
)
.await;
}
#[tokio::test]
async fn a_msg1_admitted_by_a_link_no_identity_owns_is_dropped_after_the_dh() {
let transport_id = TransportId::new(1);
let (mut node, source_addr, mut far_rx, _far_end) = node_refusing_inbound(transport_id).await;
// The fixture `test_should_admit_msg1_admits_rekey_when_accept_off` uses:
// a reverse-address entry with no peer and no connection behind it. It is
// enough to waive the refusing gate, and it attributes the address to
// nobody.
let link_id = node.allocate_link_id();
node.addr_to_link
.insert((transport_id, source_addr.clone()), link_id);
assert!(
node.should_admit_msg1(transport_id, &source_addr),
"the fixture must exercise the carve-out, not the gate"
);
let initiator = make_node();
let initiator_node_addr = node_addr_of(&initiator);
let wire_msg1 = genuine_msg1(&initiator, &node);
let bad_state_before = node.stats().handshake.bad_state;
node.handle_msg1(ReceivedPacket::with_timestamp(
transport_id,
source_addr.clone(),
wire_msg1,
2_000,
))
.await;
assert!(
node.get_peer(&initiator_node_addr).is_none(),
"a msg1 the waiver could attribute to no identity must not promote \
the identity the DH revealed"
);
assert_eq!(node.peer_count(), 0, "no peer may be created");
assert_eq!(
node.connection_count(),
0,
"the rejected msg1 must leave no connection behind"
);
assert_eq!(
node.link_count(),
0,
"the rejected msg1 must leave no link behind"
);
assert_eq!(
node.stats().handshake.bad_state - bad_state_before,
1,
"the drop must be counted"
);
assert_nothing_sent(
&mut far_rx,
"no msg2 may leave the node for an address no identity owns",
)
.await;
}
#[tokio::test]
async fn a_msg1_from_a_link_whose_dial_expects_this_identity_is_admitted() {
let transport_id = TransportId::new(1);
let (mut node, peer_addr, mut far_rx, _far_end) = node_refusing_inbound(transport_id).await;
// UDP is connectionless, so `initiate_connection` runs `start_handshake`
// in the same synchronous stretch: the reverse-address entry and the
// connection carrying the dialled identity land together, and the
// classifier can attribute the address. Do not substitute a
// connection-oriented transport here; that arm defers `start_handshake`
// and is the window the next test is about.
let peer = make_node();
let peer_identity = PeerIdentity::from_pubkey_full(peer.identity().pubkey_full());
node.initiate_connection(transport_id, peer_addr.clone(), peer_identity)
.await
.expect("the dial must register the link and the connection");
// Our own dial's msg1 went out first; drain it so the assertion below is
// about the answer to the crossing msg1.
let ours = tokio::time::timeout(std::time::Duration::from_secs(1), far_rx.recv())
.await
.expect("our dial's msg1 must arrive")
.expect("the far end's channel must be open");
assert_eq!(
wire_phase(&ours.data),
Some(crate::proto::fmp::wire::PHASE_MSG1),
"the dial's own packet is a msg1"
);
let bad_state_before = node.stats().handshake.bad_state;
let wire_msg1 = genuine_msg1(&peer, &node);
node.handle_msg1(ReceivedPacket::with_timestamp(
transport_id,
peer_addr.clone(),
wire_msg1,
2_000,
))
.await;
let answer = tokio::time::timeout(std::time::Duration::from_secs(1), far_rx.recv())
.await
.expect("the crossing msg1 must be answered")
.expect("the far end's channel must be open");
assert_eq!(
wire_phase(&answer.data),
Some(crate::proto::fmp::wire::PHASE_MSG2),
"a simultaneous open with the peer we dialled must still be answered"
);
assert_eq!(
node.stats().handshake.bad_state - bad_state_before,
0,
"a crossing msg1 from the identity the dial expects must not be \
rejected"
);
}
#[tokio::test]
async fn a_crossing_msg1_in_the_connection_oriented_dial_window_is_rejected() {
use crate::config::TcpConfig;
use crate::transport::tcp::TcpTransport;
let mut node = make_node();
let transport_id = TransportId::new(1);
// bind_addr=None makes accept_connections() false, the idiom
// `test_should_admit_msg1_rejects_fresh_when_accept_off` uses.
let cfg = TcpConfig {
bind_addr: None,
..Default::default()
};
let (tx, _rx) = packet_channel(64);
let tcp = TcpTransport::new(transport_id, None, cfg, tx);
node.transports
.insert(transport_id, TransportHandle::Tcp(tcp));
let peer = make_node();
let peer_identity = PeerIdentity::from_pubkey_full(peer.identity().pubkey_full());
let addr = TransportAddr::from_string("10.0.0.2:2121");
// The state `initiate_connection`'s connection-oriented arm leaves behind
// while the transport connect is outstanding: a link, a reverse-address
// entry, a pending connect, and no connection yet. Built directly rather
// than by driving a connect, which would need a reachable peer.
let link_id = node.allocate_link_id();
let link = Link::new(
link_id,
transport_id,
addr.clone(),
LinkDirection::Outbound,
Duration::from_millis(100),
);
node.links.insert(link_id, link);
node.addr_to_link
.insert((transport_id, addr.clone()), link_id);
node.peering.pending_connects.push(PendingConnect {
link_id,
transport_id,
remote_addr: addr.clone(),
peer_identity,
});
let bad_state_before = node.stats().handshake.bad_state;
let wire_msg1 = genuine_msg1(&peer, &node);
node.handle_msg1(ReceivedPacket::with_timestamp(
transport_id,
addr.clone(),
wire_msg1,
2_000,
))
.await;
// This is the assertion that separates the two worlds, and it is the one
// to read first when the test reds. After the change the reject returns
// above every registry mutation, so the dial's own entry is untouched.
// Without it, `handle_msg1` allocates a fresh link id, writes it over
// this entry, and then removes the entry outright when the msg2 send
// fails on the unstarted transport.
assert_eq!(
node.addr_to_link.get(&(transport_id, addr.clone())),
Some(&link_id),
"the dial's reverse-address entry must still name the dial's own link"
);
// The remaining three state the shape of the outcome. They hold either
// way for this fixture, whose unstarted TCP transport cannot send, so
// they are not what makes this test able to fail.
assert_eq!(node.peer_count(), 0, "no peer may be created");
assert_eq!(node.connection_count(), 0, "no connection may be created");
assert_eq!(
node.stats().handshake.bad_state - bad_state_before,
1,
"the drop must be counted"
);
}
#[tokio::test]
async fn msg1_waiver_classifies_all_three_outcomes() {
use crate::config::UdpConfig;
use crate::node::handlers::handshake::Msg1Waiver;
use crate::peer::ActivePeer;
use crate::transport::udp::UdpTransport;
let mut node = make_node();
// A registered accepting transport, not an unregistered id, so the
// NotNeeded assertion exercises the `accept_connections()` limb of the
// guard rather than its absent-transport fallback.
let accepting_id = TransportId::new(1);
let (accept_tx, _accept_rx) = packet_channel(64);
let accepting = UdpTransport::new(
accepting_id,
None,
UdpConfig {
bind_addr: Some("127.0.0.1:0".to_string()),
accept_connections: Some(true),
..Default::default()
},
accept_tx,
);
node.transports
.insert(accepting_id, TransportHandle::Udp(accepting));
let refusing_id = TransportId::new(2);
let (refuse_tx, _refuse_rx) = packet_channel(64);
let refusing = UdpTransport::new(
refusing_id,
None,
UdpConfig {
bind_addr: Some("127.0.0.1:0".to_string()),
accept_connections: Some(false),
..Default::default()
},
refuse_tx,
);
node.transports
.insert(refusing_id, TransportHandle::Udp(refusing));
let classify = |node: &Node, transport_id: TransportId, addr: &TransportAddr| {
node.msg1_waiver(
node.is_established_link_msg1(transport_id, addr),
transport_id,
addr,
)
};
// NotNeeded: the gate would have admitted this msg1 anyway, so nothing
// was waived and there is nothing to confirm.
let fresh = TransportAddr::from_string("10.0.0.2:2121");
assert_eq!(
classify(&node, accepting_id, &fresh),
Msg1Waiver::NotNeeded,
"an accepting transport waives nothing"
);
// Unattributed: the carve-out admits on a bare reverse-address entry
// that names neither a promoted peer nor a carrier.
let bare = TransportAddr::from_string("10.0.0.3:2121");
let bare_link = node.allocate_link_id();
node.addr_to_link
.insert((refusing_id, bare.clone()), bare_link);
assert_eq!(
classify(&node, refusing_id, &bare),
Msg1Waiver::Unattributed,
"a link no identity owns must fail closed"
);
// Expect, by promoted peer. `ActivePeer::new` sets neither transport_id
// nor current_addr, so this peer is reached through the reverse-address
// entry that names its link.
let promoted_addr = TransportAddr::from_string("10.0.0.4:2121");
let promoted_link = node.allocate_link_id();
let promoted = make_peer_identity();
let promoted_node_addr = *promoted.node_addr();
node.peers.insert(
promoted_node_addr,
ActivePeer::new(promoted, promoted_link, 1_000),
);
node.addr_to_link
.insert((refusing_id, promoted_addr.clone()), promoted_link);
assert_eq!(
classify(&node, refusing_id, &promoted_addr),
Msg1Waiver::Expect(promoted_node_addr),
"an address whose link a promoted peer owns is attributed to that peer"
);
// Expect, by carrier: a link with no promoted peer yet, whose connection
// carries the identity the dial expects.
let carrier_addr = TransportAddr::from_string("10.0.0.5:2121");
let carrier_link = node.allocate_link_id();
let dialled = make_peer_identity();
let dialled_node_addr = *dialled.node_addr();
node.seed_handshake_machine(HandshakeSeed::outbound(carrier_link, dialled, 1_000))
.unwrap();
node.addr_to_link
.insert((refusing_id, carrier_addr.clone()), carrier_link);
assert_eq!(
classify(&node, refusing_id, &carrier_addr),
Msg1Waiver::Expect(dialled_node_addr),
"an address whose link carries a dialled identity is attributed to it"
);
}
#[tokio::test]
async fn a_stale_reverse_address_entry_does_not_hide_a_peer_reachable_by_address() {
use crate::config::UdpConfig;
use crate::node::handlers::handshake::Msg1Waiver;
use crate::peer::ActivePeer;
use crate::transport::udp::UdpTransport;
let mut node = make_node();
let transport_id = TransportId::new(1);
let (tx, _rx) = packet_channel(64);
let udp = UdpTransport::new(
transport_id,
None,
UdpConfig {
bind_addr: Some("127.0.0.1:0".to_string()),
accept_connections: Some(false),
..Default::default()
},
tx,
);
node.transports
.insert(transport_id, TransportHandle::Udp(udp));
// A live peer whose current_addr is the numeric form inbound packets
// carry, which is the second predicate's whole reason for existing.
let numeric = TransportAddr::from_string("100.64.0.5:2121");
let peer_link = node.allocate_link_id();
let peer_identity = make_peer_identity();
let peer_node_addr = *peer_identity.node_addr();
let mut peer = ActivePeer::new(peer_identity, peer_link, 1_000);
peer.set_current_addr(transport_id, numeric.clone());
node.peers.insert(peer_node_addr, peer);
// A reverse-address entry at that same numeric address naming a link
// that no longer exists. `remove_link` clears the reverse lookup only
// under the key it rebuilds from the removed link's own remote address,
// so an entry inserted for that link under a second address form outlives
// it, and link ids are never reused.
let dead_link = node.allocate_link_id();
assert_ne!(
dead_link, peer_link,
"the stale entry names a different link"
);
node.addr_to_link
.insert((transport_id, numeric.clone()), dead_link);
assert_eq!(
node.msg1_waiver(
node.is_established_link_msg1(transport_id, &numeric),
transport_id,
&numeric
),
Msg1Waiver::Expect(peer_node_addr),
"the stale entry must not hide the peer the address scan finds: \
classifying this Unattributed would reject the peer's rekey msg1 \
after the DH, and would go on rejecting it, because the reject \
returns above the insert that would overwrite the stale entry"
);
}
src/node/tests/unit.rs
+2 -2
View File
@@ -1682,7 +1682,7 @@ async fn test_initiate_peer_connections_schedules_retry_on_no_transport() {
}
// ============================================================================
// transport_mtu() — ISSUE-2026-0011 regression coverage
// transport_mtu() — minimum-across-transports regression coverage
// ============================================================================
/// Helper: spawn a UdpTransport with the given mtu, started and operational.
@@ -1707,7 +1707,7 @@ async fn make_udp_transport_with_mtu(id: u32, mtu: u16) -> TransportHandle {
async fn test_transport_mtu_returns_min_across_operational() {
// Multiple operational transports with varied MTUs. The picker must
// return the smallest, deterministically, regardless of HashMap
// iteration order. This is the core ISSUE-2026-0011 regression test.
// iteration order. This is the core regression test for that.
let mut node = make_node();
let (packet_tx, packet_rx) = packet_channel(64);
node.supervisor.packet_tx = Some(packet_tx);
src/noise/handshake.rs
+142 -26
View File
@@ -9,6 +9,7 @@ use rand::Rng;
use secp256k1::{Keypair, PublicKey, Secp256k1, SecretKey, ecdh::shared_secret_point};
use sha2::{Digest, Sha256};
use std::fmt;
use zeroize::{Zeroize, ZeroizeOnDrop};
/// Symmetric state during handshake.
///
@@ -17,13 +18,24 @@ use std::fmt;
/// `Clone` exists for [`HandshakeState::try_read_xk_message_2`], which has to
/// put the pre-read state back after a message that mixed material in before
/// failing to authenticate.
#[derive(Clone)]
///
/// `ck` and `h` are cleared on drop, including on the clone above once it
/// goes out of scope. `cipher` is skipped because [`CipherState`] clears its
/// own retained key.
#[derive(Clone, Zeroize, ZeroizeOnDrop)]
struct SymmetricState {
/// Chaining key for key derivation.
ck: [u8; 32],
/// Handshake hash for transcript binding.
/// Running SHA-256 accumulator over the handshake transcript.
///
/// Maintained by `mix_hash` at every step, but never fed to the AEAD:
/// `encrypt_and_hash` passes an empty associated-data field. Nothing in
/// production reads it, so it provides no transcript binding today, and
/// anything built on `handshake_hash()` (channel binding, an exporter,
/// cookie binding) will silently not work until the AAD carries `h`.
h: [u8; 32],
/// Current cipher state for encrypting handshake payloads.
#[zeroize(skip)]
cipher: CipherState,
}
@@ -70,6 +82,9 @@ impl SymmetricState {
let mut key = [0u8; 32];
key.copy_from_slice(&output[32..64]);
self.cipher.initialize_key(key);
key.zeroize();
output.zeroize();
}
/// Encrypt and mix into hash.
@@ -98,10 +113,16 @@ impl SymmetricState {
k1.copy_from_slice(&output[..32]);
k2.copy_from_slice(&output[32..64]);
(CipherState::new(k1), CipherState::new(k2))
let ciphers = (CipherState::new(k1), CipherState::new(k2));
output.zeroize();
k1.zeroize();
k2.zeroize();
ciphers
}
/// Get the handshake hash (for channel binding).
/// Get the handshake hash.
fn handshake_hash(&self) -> [u8; 32] {
self.h
}
@@ -158,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,
@@ -180,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
}
@@ -187,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,
@@ -208,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
}
@@ -215,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,
@@ -235,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
}
@@ -242,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,
@@ -262,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
}
@@ -301,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.
@@ -314,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
}
@@ -367,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();
@@ -376,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)?;
@@ -420,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;
@@ -432,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..];
@@ -487,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)?;
@@ -535,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..];
@@ -599,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;
@@ -639,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;
@@ -686,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)?;
@@ -731,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..];
@@ -818,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)?;
@@ -869,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..];
@@ -916,12 +1012,32 @@ impl HandshakeState {
))
}
/// Get the handshake hash (for channel binding, available after complete).
/// Get the handshake hash (available after complete).
pub fn handshake_hash(&self) -> [u8; 32] {
self.symmetric.handshake_hash()
}
}
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")
src/noise/mod.rs
+40 -12
View File
@@ -42,6 +42,7 @@ mod session;
use ring::aead::{Aad, CHACHA20_POLY1305, LessSafeKey, Nonce, UnboundKey};
use std::fmt;
use thiserror::Error;
use zeroize::{Zeroize, ZeroizeOnDrop};
pub use handshake::HandshakeState;
pub use replay::ReplayWindow;
@@ -181,21 +182,32 @@ impl fmt::Display for HandshakeProgress {
/// AEAD is `ring`'s ChaCha20-Poly1305 (BoringSSL backend), which dispatches
/// to NEON on aarch64 and AVX2/AVX-512 on x86_64. The 32-byte key is
/// retained alongside a cached `LessSafeKey` so the per-packet AEAD skips
/// the keyed-cipher construction (key copy + Poly1305 key derivation).
/// `LessSafeKey` itself doesn't implement `Clone` (deliberate, for safety),
/// so `CipherState`'s manual `Clone` impl rebuilds the keyed AEAD from the
/// retained key bytes — cheap for ChaCha20-Poly1305 since the construction
/// is essentially a key copy plus a constant-time check.
/// rebuilding the keyed cipher. (It does not skip Poly1305 key derivation,
/// which is nonce-dependent and happens per message inside seal and open.)
/// `CipherState`'s manual `Clone` impl rebuilds the keyed AEAD from those
/// retained bytes, and so does `cipher_clone` for each off-task AEAD worker;
/// those two are why the bytes are kept. The rebuild is cheap for
/// ChaCha20-Poly1305: a length check and a conversion of the 32 key bytes
/// into little-endian words.
///
/// Because the key bytes are retained, every clone leaves a second copy in
/// memory; each copy is cleared when it goes out of scope. Only `key` is
/// zeroized. `cipher` is skipped because `LessSafeKey`
/// holds its material behind `ring`'s own API and cannot be cleared from
/// here; `nonce` and `has_key` are skipped because they are not secret.
#[derive(Zeroize, ZeroizeOnDrop)]
pub struct CipherState {
/// Encryption key (32 bytes). Retained so we can rebuild the keyed
/// AEAD on `Clone` and on `initialize_key` (ring's `UnboundKey` /
/// `LessSafeKey` do not implement `Clone`).
/// Encryption key (32 bytes). Retained because `Clone` and
/// `cipher_clone` rebuild the keyed AEAD from these bytes.
key: [u8; 32],
/// Cached keyed AEAD, valid iff `has_key`. None for an un-keyed state.
#[zeroize(skip)]
cipher: Option<LessSafeKey>,
/// Nonce counter (8 bytes used, 4 bytes zero prefix).
#[zeroize(skip)]
pub(super) nonce: u64,
/// Whether this cipher has a valid key.
#[zeroize(skip)]
has_key: bool,
}
@@ -217,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).
@@ -238,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
@@ -395,6 +416,13 @@ impl CipherState {
self.has_key
}
/// Copy out the retained key bytes, so a test can observe zeroization
/// without reading freed memory.
#[cfg(test)]
fn key_bytes(&self) -> [u8; 32] {
self.key
}
/// Clone the underlying keyed AEAD, for off-task AEAD workers.
///
/// Returns `None` if no key. The cloned `LessSafeKey` pairs with
src/noise/session.rs
+2 -2
View File
@@ -14,7 +14,7 @@ pub struct NoiseSession {
send_cipher: CipherState,
/// Cipher for receiving.
recv_cipher: CipherState,
/// Handshake hash for channel binding.
/// Handshake hash.
handshake_hash: [u8; 32],
/// Remote peer's static public key.
remote_static: PublicKey,
@@ -190,7 +190,7 @@ impl NoiseSession {
self.replay_window.reset();
}
/// Get the handshake hash for channel binding.
/// Get the handshake hash.
pub fn handshake_hash(&self) -> &[u8; 32] {
&self.handshake_hash
}
src/noise/tests.rs
+24
View File
@@ -201,6 +201,30 @@ fn test_cipher_state_nonce_sequence() {
assert_eq!(cipher.nonce(), 2);
}
#[test]
fn cipher_state_drop_impl_clears_the_retained_key() {
// Reading the bytes back out of a dropped `CipherState` would mean
// reading freed memory, so the drop behaviour is asserted through two
// observable proxies instead: that `CipherState` implements
// `ZeroizeOnDrop`, and that an explicit `zeroize()` leaves `key`
// all-zero while `has_key` is untouched.
fn assert_zeroize_on_drop<T: zeroize::ZeroizeOnDrop>() {}
assert_zeroize_on_drop::<CipherState>();
let mut cipher = CipherState::new([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();
assert_eq!(cipher.key_bytes(), [0u8; 32]);
assert!(cipher.has_key());
}
#[test]
fn test_session_remote_static() {
let keypair1 = generate_keypair();
src/nostr/runtime.rs
+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::advert::{AdvertMachine, PublishPlan};
use super::failure_state::FailureState;
@@ -286,7 +287,15 @@ impl NostrRendezvous {
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())
src/peer/machine.rs
+18 -4
View File
@@ -56,6 +56,7 @@
#![allow(dead_code)]
use crate::identity::ErasingKeypair;
use crate::noise::{self, NoiseError, NoiseSession};
use crate::proto::fmp::{
ConnAction, ConnSnapshot, ConnectionState, EstablishSnapshot, Fmp, InboundDecision,
@@ -617,10 +618,15 @@ impl PeerMachine {
/// The epoch is our startup epoch, encrypted into msg1 for restart detection.
pub(crate) 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);
let msg1 = {
let direction = self.conn.direction();
let expected_identity = self.conn.expected_identity().copied();
@@ -637,7 +643,9 @@ impl PeerMachine {
.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()?;
@@ -656,11 +664,15 @@ impl PeerMachine {
/// The epoch is our startup epoch, encrypted into msg2 for restart detection.
pub(crate) 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);
let (msg2, learned_identity, remote_epoch) = {
let direction = self.conn.direction();
let leg = self.leg.as_mut().ok_or_else(no_pending_connection)?;
@@ -672,7 +684,9 @@ impl PeerMachine {
});
}
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)
src/proto/fmp/wire.rs
+68
View File
@@ -234,6 +234,38 @@ pub const FLAG_CE: u8 = 0x02;
/// Spin bit for RTT measurement.
pub const FLAG_SP: u8 = 0x04;
// ============================================================================
// Wire Length Validation
// ============================================================================
/// Expected `payload_len` for a packet of `phase` and total wire length
/// `total`, or `None` if the phase carries no fixed relationship.
///
/// Each frame kind states its own relationship; there is no single shared
/// convention. The handshake frames count everything after the 4-byte common
/// prefix. The established frame counts only the inner plaintext, excluding
/// both the 16-byte header and the AEAD tag, which is what
/// [`CommonPrefix::payload_len`] documents.
///
/// The handshake arms are built from the same constants the encoders use; the
/// established arm is not, since no encoder reads `ENCRYPTED_MIN_SIZE`. Neither
/// shape stops a one-sided edit from making the two disagree, which is what the
/// tests below exist to catch.
///
/// A `None` from the established arm means "no fixed relationship", so a caller
/// skips such a frame rather than rejecting it. That is the safe direction: a
/// truncating cast here would produce a false rejection instead.
pub fn expected_payload_len(phase: u8, total: usize) -> Option<u16> {
match phase {
PHASE_MSG1 => Some((MSG1_WIRE_SIZE - COMMON_PREFIX_SIZE) as u16),
PHASE_MSG2 => Some((MSG2_WIRE_SIZE - COMMON_PREFIX_SIZE) as u16),
PHASE_ESTABLISHED => total
.checked_sub(ENCRYPTED_MIN_SIZE)
.and_then(|n| u16::try_from(n).ok()),
_ => None,
}
}
// ============================================================================
// Common Prefix
// ============================================================================
@@ -791,4 +823,40 @@ mod tests {
// payload_len = sender_idx(4) + receiver_idx(4) + noise_msg2(57) = 65
assert_eq!(prefix.payload_len, 65);
}
#[test]
fn expected_payload_len_for_an_established_frame_excludes_header_and_tag() {
let header = build_established_header(SessionIndex::new(7), 0, 0, 40);
let frame = build_encrypted(&header, &[0u8; 40 + TAG_SIZE]);
// 16 header + 40 plaintext + 16 tag = 72 on the wire, declaring 40.
assert_eq!(frame.len(), ESTABLISHED_HEADER_SIZE + 40 + TAG_SIZE);
assert_eq!(
expected_payload_len(PHASE_ESTABLISHED, frame.len()),
Some(40)
);
}
#[test]
fn expected_payload_len_matches_what_build_msg1_and_build_msg2_actually_emit() {
// The Noise buffers are sized with literals rather than
// HANDSHAKE_MSG1_SIZE / HANDSHAKE_MSG2_SIZE, unlike the two tests
// above. With the constant on both sides, changing it would move the
// encoder's payload_len and the validator's expectation together and
// leave this test green. Pinning the input keeps the two sides
// independent so a one-sided change is caught.
let msg1 = build_msg1(SessionIndex::new(1), &[0u8; 106]);
let prefix = CommonPrefix::parse(&msg1).unwrap();
assert_eq!(
expected_payload_len(PHASE_MSG1, msg1.len()),
Some(prefix.payload_len)
);
let msg2 = build_msg2(SessionIndex::new(1), SessionIndex::new(2), &[0u8; 57]);
let prefix = CommonPrefix::parse(&msg2).unwrap();
assert_eq!(
expected_payload_len(PHASE_MSG2, msg2.len()),
Some(prefix.payload_len)
);
}
}
src/proto/link.rs
+3 -3
View File
@@ -21,10 +21,10 @@ pub enum LinkMessageType {
/// Payload is opaque to intermediate nodes (end-to-end encrypted).
SessionDatagram = 0x00,
// MMP reports (0x01-0x02) — content defined in TASK-2026-0006
/// Sender-side MMP report (stub).
// MMP reports (0x01-0x02) — payload is an encoded SenderReport or ReceiverReport
/// Sender-side MMP report.
SenderReport = 0x01,
/// Receiver-side MMP report (stub).
/// Receiver-side MMP report.
ReceiverReport = 0x02,
// Tree protocol (0x10-0x1F)
src/transport/ble/io.rs
+9
View File
@@ -23,6 +23,15 @@ pub trait BleStream: Send + Sync {
/// Receive data from the L2CAP connection.
///
/// Returns the number of bytes read into `buf`.
///
/// A single call must never return bytes drawn from more than one SDU.
/// The receive loop emits what one call returns as one FMP frame. A
/// concatenation is caught by the frame-length check in the node's
/// dispatch only when the leading frame is an established one; where it
/// is a handshake frame, that check passes and the buffer is dropped one
/// step later by that frame kind's exact-size parse. Returning less than
/// a whole SDU is allowed: a truncated frame fails the AEAD tag or the
/// exact-size parse regardless.
fn recv(
&self,
buf: &mut [u8],
src/transport/framing.rs
+22
View File
@@ -229,6 +229,28 @@ mod tests {
frame
}
/// The wire sizes above are written as literals, independently of the FMP
/// wire module, which derives the same values from the Noise message sizes.
/// Keeping them independent is deliberate: this module takes no dependency
/// on `crate::proto::fmp`, and the import below exists only under
/// `cfg(test)`.
/// The cost is that the two can drift. A wrong literal on this side also
/// breaks the TCP integration tests, since they move real frames through
/// this reader; a wrong value on the wire-module side does not reach here
/// at all, and this test is what catches that direction.
#[test]
fn stream_reader_constants_agree_with_the_fmp_wire_module() {
use crate::proto::fmp::wire;
assert_eq!(MSG1_WIRE_SIZE, wire::MSG1_WIRE_SIZE);
assert_eq!(MSG2_WIRE_SIZE, wire::MSG2_WIRE_SIZE);
assert_eq!(PREFIX_SIZE, wire::COMMON_PREFIX_SIZE);
assert_eq!(
ESTABLISHED_REMAINING_HEADER + PREFIX_SIZE,
wire::ESTABLISHED_HEADER_SIZE
);
}
#[tokio::test]
async fn test_read_established_frame() {
let payload_len = 64u16;
src/transport/udp/mod.rs
+2 -2
View File
@@ -415,8 +415,8 @@ impl Transport for UdpTransport {
/// Whether the transport accepts inbound handshake initiations.
/// `outbound_only` mode forces this to false; otherwise reflects the
/// `accept_connections` config field (default: true). Note that the
/// hard gate is at the Node level (see ISSUE-2026-0004 fix in
/// `src/node/handlers/handshake.rs`); this method is what that gate
/// hard gate is at the Node level (in `src/node/handlers/handshake.rs`);
/// this method is what that gate
/// consults for transports that lack runtime-state-based filtering.
fn accept_connections(&self) -> bool {
if self.config.outbound_only() {
testing/chaos/scenarios/bloom-storm.yaml
+1 -1
View File
@@ -72,7 +72,7 @@ netem:
# `interval_secs`, the policies on the two listed edges are swapped.
# This drives n04 to alternate parents between n02 and n03 each
# round. The 4s cadence and 5ms-vs-100ms delta come from the
# original ISSUE-2026-0019 reproduction harness — they are
# original reproduction harness for this flap — they are
# calibrated to produce a parent switch per round under the
# zero-hysteresis FIPS overrides below.
link_swap:
testing/check-comment-refs.sh
Executable
+181
View File
@@ -0,0 +1,181 @@
#!/bin/bash
# ── Source comment reference guard ──────────────────────────────────────────
# Every reference a source comment makes must resolve for a reader who has
# only this repository.
#
# A comment that names a private planning artifact — by identifier, by file
# path, or in programme vocabulary that has no in-repo referent — is dead text
# to everyone outside the workspace that holds the artifact, and it publishes
# the existence and shape of that workspace to everyone else. A hand-run grep
# closes the population that exists on the day it runs; this closes it for
# every commit after.
#
# Three checks, each a `git grep` at HEAD rather than over the working tree,
# because the thing being gated is what is committed:
#
# 1. identifiers, repo-wide. A local item identifier anywhere in the tree.
# Repo-wide because packaging/, testing/ and the workflow files are as
# public as src/ — more so in the packaging case, which ships to users.
# 2. document paths, scoped to src/. A comment under src/ citing an *.md
# path that does not exist in the tree at the checked commit. This is a
# resolvability rule rather than a denylist, so it catches private
# documents nobody has thought of. Scoped to src/ because resolving the
# whole tree's relative documentation links against the repository root
# is a different checker with its own population to triage first.
# 3. programme vocabulary, scoped to src/. Phase, rung, milestone and step
# labels that name a plan a reader cannot open. The pattern is shared
# verbatim with the sweep that produced today's clean tree, so the two
# cannot drift apart.
#
# Known coverage gaps, recorded rather than discovered:
# - a bare "milestone" in some other phrasing (the narrow alternatives keep
# the Tor bootstrap loop in src/transport/tor/mod.rs out of check 3);
# - the CATEGORY_D / CATEGORY_E code identifiers and test names, which
# check 3's case-sensitive Category-[A-Z] deliberately does not match;
# - programme vocabulary, or a document path, outside src/;
# - a reference to a private artifact made in free prose with no marker at
# all, at any scope: no check here matches it;
# - a pathspec typo introduced after this file lands. git grep returns 1
# with empty output for a pathspec that matches nothing, which is
# indistinguishable from health. The non-empty-population assertion below
# narrows that for the src/-scoped checks and closes nothing for check 1.
#
# Exit 0 = every reference resolves. Exit 1 = one or more do not; every hit is
# printed, not just the first. Exit 2 = the check could not look (not in a work
# tree, wrong directory, empty pathspec, or a git-level failure); never treated
# as a pass.
# ─────────────────────────────────────────────────────────────────────────────
set -uo pipefail
# A pathspec is resolved against the current directory even when a rev is
# supplied, and a run from the wrong directory returns rc 1 with empty output
# on every check — the same shape as a clean tree. Assert the directory, then
# assert the post-condition rather than trusting the cd: `cd ""` succeeds and
# does not move, so a one-line form silently leaves the script where it began
# whenever the command substitution comes back empty.
ROOT=$(git rev-parse --show-toplevel 2>/dev/null) \
|| { echo "check-comment-refs: not inside a git work tree" >&2; exit 2; }
[[ -n "$ROOT" ]] \
|| { echo "check-comment-refs: empty work-tree root" >&2; exit 2; }
cd "$ROOT" || exit 2
[[ -z "$(git rev-parse --show-prefix)" ]] \
|| { echo "check-comment-refs: not at the work-tree root" >&2; exit 2; }
# The src/ pathspec must select something. wc -l is deliberate where the rest
# of this script preserves exit statuses: it always exits 0, so the decision is
# made on n and never on a status, and a failing git ls-tree yields n=0 and
# fires the assertion. This covers checks 2 and 3 only; check 1's pathspec is
# "." and stays non-empty from anywhere, so its wrong-directory case is covered
# by the --show-prefix assertion above and its mistyped-pathspec case by
# nothing.
n=$(git ls-tree -r --name-only HEAD -- src/ | wc -l)
(( n > 0 )) || { echo "check-comment-refs: pathspec src/ matched no files" >&2; exit 2; }
# Deliberately a superset of the sweep's own acceptance regex: the year group
# is optional, so the three-digit form is caught as well, and the separator is
# optional so underscores and spaces are caught alongside hyphens. Narrowing
# this is how a whole class goes unguarded while every break-check still
# passes.
ID_PAT='(TASK|ISSUE|IDEA|QUICK|RECUR)[-_ ]?(20[0-9]{2}[-_ ])?[0-9]{3,4}'
# Verbatim from the sweep's enumerating pattern. Do not edit one without the
# other. If check 3's scope is ever widened beyond src/, this text matches
# itself through six of its alternatives and the widening must add
# ':(exclude)testing/check-comment-refs.sh' — never an exclusion of testing/,
# which holds real check-1 hits a directory-wide exclusion would drop.
VOCAB_PAT='\b[RQ][0-9]\b|Category-[A-Z]|\bumbrella\b|refactor steps?|\bpre-scopes\b|R0 stub|read-isolation|cut over yet|Cutover begins|in Step [0-9]|(the|this) milestone|Milestone-'
MD_PAT='[A-Za-z0-9_./-]+\.md\b'
FAILED=0
# ── Check 1: local item identifiers, repo-wide ──────────────────────────────
# The capture preserves git grep's status instead of flattening it with
# `|| true`: a legitimate no-hit run exits 1, but so would a malformed
# pathspec magic, an unresolvable rev or a bad regex exit 128, and collapsing
# both into "clean" is the single likeliest way this script reports green
# while checking nothing. The hit decision is made on the output; the
# could-not-look decision is made on the status.
rc=0
out=$(git grep -nEI "$ID_PAT" HEAD -- .) || rc=$?
if (( rc > 1 )); then
echo "check-comment-refs: check 1 grep failed (rc=$rc)" >&2
exit 2
fi
if [[ -n "$out" ]]; then
printf '%s\n' "$out" \
| sed -E 's|^HEAD:([^:]*):([0-9]+):|\1:\2: names a local work item: |'
FAILED=1
fi
# ── Check 2: document paths under src/ must resolve in-repo ─────────────────
rc=0
md_lines=$(git grep -nI '\.md' HEAD -- src/) || rc=$?
if (( rc > 1 )); then
echo "check-comment-refs: check 2 grep failed (rc=$rc)" >&2
exit 2
fi
# Drop any line carrying a URL before extracting a token from it. Keying the
# skip on the extracted token cannot work: ':' is outside the token character
# class, so the scheme is stripped first and what survives does not begin with
# "http". Inert on today's tree, and kept so a future URL citation does not red.
cited=$(printf '%s\n' "$md_lines" | grep -vE 'https?://')
declare -A CITES=()
while IFS= read -r line; do
[[ -n "$line" ]] || continue
loc=${line#HEAD:}
loc=$(printf '%s' "$loc" | sed -E 's|^([^:]*):([0-9]+):.*|\1:\2|')
content=$(printf '%s' "$line" | sed -E 's|^HEAD:[^:]*:[0-9]+:||')
while IFS= read -r tok; do
[[ -n "$tok" ]] || continue
CITES["$tok"]+="$loc "
done < <(printf '%s\n' "$content" | grep -oE "$MD_PAT")
done <<< "$cited"
if (( ${#CITES[@]} > 0 )); then
while IFS= read -r tok; do
[[ -n "$tok" ]] || continue
lrc=0
lsout=$(git ls-tree HEAD -- "$tok" 2>/dev/null) || lrc=$?
# A '../'-prefixed or absolute token makes git ls-tree exit 128 with
# "is outside repository". That is neither a hit nor clean: the check
# could not look, and reporting it as a hit would red a legitimate
# citation while reporting it as clean would hide one.
if (( lrc != 0 )); then
echo "check-comment-refs: could not resolve '$tok' (git ls-tree rc=$lrc)" >&2
exit 2
fi
if [[ -z "$lsout" ]]; then
for loc in ${CITES["$tok"]}; do
printf '%s: cites %s, which does not exist in this repository\n' \
"$loc" "$tok"
done
FAILED=1
fi
done < <(printf '%s\n' "${!CITES[@]}" | sort)
fi
# ── Check 3: programme vocabulary under src/ ────────────────────────────────
rc=0
out=$(git grep -nEI "$VOCAB_PAT" HEAD -- src/) || rc=$?
if (( rc > 1 )); then
echo "check-comment-refs: check 3 grep failed (rc=$rc)" >&2
exit 2
fi
if [[ -n "$out" ]]; then
printf '%s\n' "$out" \
| sed -E 's|^HEAD:([^:]*):([0-9]+):|\1:\2: names a plan this repository does not carry: |'
FAILED=1
fi
if (( FAILED != 0 )); then
echo "" >&2
echo "check-comment-refs: a source comment references something a reader holding" >&2
echo "only this repository cannot resolve. Rewrite the comment to say what the" >&2
echo "code does, citing nothing outside the tree." >&2
exit 1
fi
exit 0
testing/ci-local.sh
+13
View File
@@ -1267,6 +1267,18 @@ run_action_pins() {
record "action-pins" $rc
}
# Every reference a source comment makes must resolve for a reader who has only
# this repository. A comment naming a private planning artifact is dead text to
# everyone outside the workspace holding it, and publishes that workspace's
# shape to everyone else. A hand-run grep closes the population that exists on
# the day it runs; this closes it for every commit after.
run_comment_refs() {
local rc=0
info "[comment-refs] Checking that every source comment resolves in-repo"
"$SCRIPT_DIR/check-comment-refs.sh" || rc=$?
record "comment-refs" $rc
}
# Every daemon log string a test matches on must still be emitted by src/.
# A stale one does not fail — it stops observing, and an expect-zero assertion
# built on it then passes for the wrong reason.
@@ -1322,6 +1334,7 @@ main() {
run_trailing_log
run_image_scoping
run_action_pins
run_comment_refs
run_wait_converge
if [[ "$TEST_ONLY" == true ]]; then
testing/interop/interop-test.sh
+1 -1
View File
@@ -949,7 +949,7 @@ echo ""
# versions. A mixed-version bloom/tree-encoding divergence shows up as a
# node that never produces an in-band estimate (or returns null). Strict
# band = [0.75N, 1.25N]; polled up to MESH_SIZE_TIMEOUT (the estimate
# converges over minutes — see ISSUE-2026-0046 on its transient jitter).
# converges over minutes and is transiently jittery).
echo "Phase 7: Mesh-size estimate convergence (strict ±25% of true N=$NUM_NODES)"
PASSED=0; FAILED=0
ms_lo="$(awk -v n="$NUM_NODES" 'BEGIN{printf "%.2f", 0.75*n}')"
testing/mesh-lab/compose-trace-nat.yml
+1 -1
View File
@@ -1,6 +1,6 @@
# Compose override that bumps RUST_LOG to trace level on the modules
# relevant to NAT-traversal handshake-completion flake evidence
# collection (ISSUE-2026-0027):
# collection:
#
# - fips::nostr — overlay advert publish/consume
# (where the cross-init race begins)
testing/static/scripts/rekey-test.sh
+1 -1
View File
@@ -176,7 +176,7 @@ RECONVERGE_STALL=10
TIMEOUT=5
CONVERGENCE_PING_TIMEOUT=1
# Strict-ping retry policy for the per-phase ping_all asserts. Under 1%
# i.i.d. packet loss (the lab condition surfaced by ISSUE-2026-0028) a
# i.i.d. packet loss (the lab condition this retry policy is sized for) a
# single-shot ping fails at roughly 2% per directed pair, so a 20-pair
# strict assert misses with probability ~1 - (0.98)^20 ≈ 33%, which is
# below the per-pair loss-math floor but well above the routing-state