mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 19:18:25 +00:00
Give the rekey static mismatch a counter of its own
A forged rekey msg2 drives the static-mismatch reject arm, which abandons the rekey cycle. Repeated, it denies key rotation indefinitely, and the reject was charged to the undifferentiated BadState bucket, so the attack looked like ordinary handshake noise. The arm now records its own reject kind and its own counter. No behaviour changes: the abandon, the index free and the peer-map removal are untouched. This is the observability half only. The denial itself needs the continuity check hoisted ahead of handshake consumption, which is a separate change. Note what this does not achieve on its own. Nothing outside the tests reads these counters: HandshakeStats::snapshot has no callers, and the control socket's status response carries no handshake reject block. The new counter discriminates in-process and is testable, and an operator still cannot see it. Plumbing the handshake counters to an operator-readable surface would move all three, not just this one, and belongs in its own change. The BadState doc enumeration is updated as the issue asks. It remains incomplete on four further counts, which are left listed in the report rather than silently corrected here.
This commit is contained in:
@@ -721,7 +721,7 @@ impl Node {
|
||||
let _ = self.index_allocator.free(idx);
|
||||
}
|
||||
self.stats_mut().record_reject(RejectReason::Handshake(
|
||||
HandshakeReject::BadState,
|
||||
HandshakeReject::RekeyStaticMismatch,
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
+17
-2
@@ -154,7 +154,9 @@ pub enum DiscoveryReject {
|
||||
/// `UnknownConnection` covers lookup-miss sites where an inbound message
|
||||
/// arrived for a connection identifier we don't recognise (no pending
|
||||
/// outbound for the receiver_idx in msg2; duplicate msg1 with no stored
|
||||
/// msg2 to resend).
|
||||
/// msg2 to resend). `RekeyStaticMismatch` is carved out of the
|
||||
/// `BadState` bulk so the one attacker-driven arm in the cluster has a
|
||||
/// counter of its own.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
|
||||
#[non_exhaustive]
|
||||
pub enum HandshakeReject {
|
||||
@@ -162,7 +164,10 @@ pub enum HandshakeReject {
|
||||
/// failed, identity could not be learned, index allocator returned an
|
||||
/// error, msg2/msg3 send failed, promote_connection returned an error,
|
||||
/// ACL gate rejected the peer, or the admission gate fired
|
||||
/// (max_peers / accept_connections). Tracked via
|
||||
/// (max_peers / accept_connections). The rekey static-key continuity
|
||||
/// gate also lived here until it was given its own
|
||||
/// [`RekeyStaticMismatch`](HandshakeReject::RekeyStaticMismatch)
|
||||
/// variant. Tracked via
|
||||
/// [`HandshakeStats::bad_state`](crate::node::stats::HandshakeStats).
|
||||
BadState,
|
||||
/// Inbound handshake message arrived but the connection identifier
|
||||
@@ -172,6 +177,16 @@ pub enum HandshakeReject {
|
||||
/// no rekey-responder state). Tracked via
|
||||
/// [`HandshakeStats::unknown_connection`](crate::node::stats::HandshakeStats).
|
||||
UnknownConnection,
|
||||
/// Initiator-side rekey msg2 completed the Noise read, but the static
|
||||
/// key it revealed is not the key the established session is bound to
|
||||
/// — the msg2 did not come from the peer we are rekeying with. The
|
||||
/// rekey cycle is abandoned and the working session is left untouched.
|
||||
/// Unlike the rest of the cluster this arm cannot be reached by
|
||||
/// routine handshake noise: the rekey msg1 header travels in the
|
||||
/// clear, so a sustained rate here means an on-path attacker is
|
||||
/// forging rekey msg2 and suppressing key rotation. Tracked via
|
||||
/// [`HandshakeStats::rekey_static_mismatch`](crate::node::stats::HandshakeStats).
|
||||
RekeyStaticMismatch,
|
||||
}
|
||||
|
||||
/// FSP session rejection reasons.
|
||||
|
||||
@@ -143,6 +143,12 @@ pub struct HandshakeStats {
|
||||
/// resend, msg3 for an unknown pending-inbound index without a
|
||||
/// matching rekey-responder slot.
|
||||
pub unknown_connection: u64,
|
||||
/// Initiator-side rekey msg2 read cleanly but revealed a static key
|
||||
/// other than the one the established session is bound to, so the
|
||||
/// rekey cycle was abandoned and the working session kept. A
|
||||
/// sustained rate here is an on-path attacker forging rekey msg2 and
|
||||
/// suppressing key rotation, not routine handshake noise.
|
||||
pub rekey_static_mismatch: u64,
|
||||
}
|
||||
|
||||
impl HandshakeStats {
|
||||
@@ -150,6 +156,7 @@ impl HandshakeStats {
|
||||
HandshakeStatsSnapshot {
|
||||
bad_state: self.bad_state,
|
||||
unknown_connection: self.unknown_connection,
|
||||
rekey_static_mismatch: self.rekey_static_mismatch,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -157,6 +164,7 @@ impl HandshakeStats {
|
||||
match reason {
|
||||
HandshakeReject::BadState => self.bad_state += 1,
|
||||
HandshakeReject::UnknownConnection => self.unknown_connection += 1,
|
||||
HandshakeReject::RekeyStaticMismatch => self.rekey_static_mismatch += 1,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -401,6 +409,7 @@ pub struct SessionStatsSnapshot {
|
||||
pub struct HandshakeStatsSnapshot {
|
||||
pub bad_state: u64,
|
||||
pub unknown_connection: u64,
|
||||
pub rekey_static_mismatch: u64,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Default, Serialize)]
|
||||
@@ -574,6 +583,16 @@ mod tests {
|
||||
assert_eq!(stats.bad_state, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn handshake_stats_record_reject_rekey_static_mismatch() {
|
||||
let mut stats = HandshakeStats::default();
|
||||
stats.record_reject(HandshakeReject::RekeyStaticMismatch);
|
||||
stats.record_reject(HandshakeReject::RekeyStaticMismatch);
|
||||
assert_eq!(stats.rekey_static_mismatch, 2);
|
||||
assert_eq!(stats.bad_state, 0);
|
||||
assert_eq!(stats.snapshot().rekey_static_mismatch, 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn node_stats_record_reject_dispatches_to_handshake() {
|
||||
let mut stats = NodeStats::new();
|
||||
|
||||
@@ -3044,6 +3044,96 @@ async fn test_rekey_msg2_foreign_static_rejected() {
|
||||
stop_hs(&mut attacker).await;
|
||||
}
|
||||
|
||||
/// The forged rekey msg2 is charged to `rekey_static_mismatch`, not to the
|
||||
/// undifferentiated `bad_state` bucket.
|
||||
///
|
||||
/// This counter is the only machine-readable indicator that the continuity
|
||||
/// gate is firing. `bad_state` is shared with header parse failures, ACL
|
||||
/// denials, allocator pressure and admission drops, so an operator alerting on
|
||||
/// it cannot tell an on-path attacker forging rekey msg2 from routine
|
||||
/// handshake noise. The `bad_state == 0` limb is what makes this test
|
||||
/// discriminating: recording the old catch-all variant from the reject arm
|
||||
/// fails both assertions, not neither.
|
||||
#[tokio::test]
|
||||
async fn test_forged_rekey_msg2_is_counted_as_rekey_static_mismatch_not_bad_state() {
|
||||
let mut rekey_config = Config::new();
|
||||
rekey_config.node.rekey.enabled = true;
|
||||
rekey_config.node.rekey.after_secs = 30;
|
||||
|
||||
let mut initiator = make_hs_node(rekey_config).await;
|
||||
let mut responder = make_hs_node(Config::new()).await;
|
||||
let mut attacker = make_hs_node(Config::new()).await;
|
||||
|
||||
let responder_addr =
|
||||
*PeerIdentity::from_pubkey_full(responder.node.identity().pubkey_full()).node_addr();
|
||||
|
||||
let msg3 = drive_to_msg3(&mut initiator, &mut responder, 1000).await;
|
||||
responder.node.handle_msg3(msg3).await;
|
||||
assert_eq!(initiator.node.peer_count(), 1);
|
||||
|
||||
// Counter identity: the established handshake must not itself have
|
||||
// charged either counter, or the post-attack readings prove nothing.
|
||||
assert_eq!(
|
||||
initiator.node.stats().handshake.rekey_static_mismatch,
|
||||
0,
|
||||
"the clean handshake charges no mismatch"
|
||||
);
|
||||
assert_eq!(
|
||||
initiator.node.stats().handshake.bad_state,
|
||||
0,
|
||||
"the clean handshake charges no catch-all reject"
|
||||
);
|
||||
|
||||
initiator
|
||||
.node
|
||||
.get_peer_mut(&responder_addr)
|
||||
.unwrap()
|
||||
.test_backdate_session_established(std::time::Duration::from_secs(120));
|
||||
initiator.node.check_rekey().await;
|
||||
assert!(
|
||||
initiator
|
||||
.node
|
||||
.get_peer(&responder_addr)
|
||||
.unwrap()
|
||||
.rekey_our_index()
|
||||
.is_some(),
|
||||
"the cadence started a rekey, so the reject arm is reachable"
|
||||
);
|
||||
|
||||
// The attacker answers the cleartext rekey msg1 under its own static.
|
||||
let rekey_msg1 = recv_phase(&mut responder.packet_rx, 1, "rekey msg1").await;
|
||||
attacker.node.handle_msg1(rekey_msg1).await;
|
||||
let forged_msg2 = recv_phase(&mut initiator.packet_rx, 2, "forged rekey msg2").await;
|
||||
initiator.node.handle_msg2(forged_msg2).await;
|
||||
|
||||
// Arm identity: the reject really happened, rather than the msg2 being
|
||||
// dropped earlier for some unrelated reason.
|
||||
assert_eq!(initiator.node.peer_count(), 1, "peer set unchanged");
|
||||
assert!(
|
||||
!initiator
|
||||
.node
|
||||
.get_peer(&responder_addr)
|
||||
.unwrap()
|
||||
.rekey_in_progress(),
|
||||
"the rejected rekey cycle is abandoned"
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
initiator.node.stats().handshake.rekey_static_mismatch,
|
||||
1,
|
||||
"the forged rekey msg2 is charged to its own counter"
|
||||
);
|
||||
assert_eq!(
|
||||
initiator.node.stats().handshake.bad_state,
|
||||
0,
|
||||
"and is no longer hidden in the undifferentiated bucket"
|
||||
);
|
||||
|
||||
stop_hs(&mut initiator).await;
|
||||
stop_hs(&mut responder).await;
|
||||
stop_hs(&mut attacker).await;
|
||||
}
|
||||
|
||||
/// The same cadence-driven rekey, answered by the REAL peer, still installs the
|
||||
/// pending session — the gate must be invisible on the legitimate path.
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user