mirror of
https://github.com/jmcorgan/fips.git
synced 2026-08-09 00:04:54 +00:00
Fix PMTUD: per-destination path MTU check and ICMPv6 MTU field width
Two bugs prevented Path MTU Discovery from working across heterogeneous links (e.g., ethernet→UDP boundary): 1. ICMPv6 Packet Too Big wrote the MTU as u16 into a 32-bit field (RFC 4443 §3.2), causing the kernel to read an inflated value. Fixed by changing build_packet_too_big() to use u32 and writing all 4 bytes. 2. handle_tun_outbound() only checked the local transport MTU, not the per-destination PathMtuState updated by MtuExceeded signals. Added a PathMtuState check after session lookup so subsequent oversized packets generate ICMPv6 PTB on TUN instead of being forwarded and dropped at the bottleneck hop. Added integration test exercising the full PMTUD loop across a 3-node chain with heterogeneous MTUs: oversized packet → forwarding failure → MtuExceeded signal → PathMtuState update → ICMPv6 PTB on TUN.
This commit is contained in:
+218
-1
@@ -4,7 +4,7 @@ use super::*;
|
||||
use crate::node::session::EndToEndState;
|
||||
use crate::node::tests::spanning_tree::{
|
||||
cleanup_nodes, generate_random_edges, process_available_packets, run_tree_test,
|
||||
verify_tree_convergence, TestNode,
|
||||
run_tree_test_with_mtus, verify_tree_convergence, TestNode,
|
||||
};
|
||||
use crate::protocol::{SessionAck, SessionDatagram};
|
||||
|
||||
@@ -1700,3 +1700,220 @@ async fn test_session_awaiting_msg3_timeout() {
|
||||
node.resend_pending_session_handshakes(after_timeout).await;
|
||||
assert!(!node.sessions.contains_key(&src_addr), "Timed-out AwaitingMsg3 session should be removed");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_tun_outbound_path_mtu_generates_ptb() {
|
||||
// When a session's PathMtuState reports a lower MTU than the local
|
||||
// transport (simulating a bottleneck learned via MtuExceeded signals),
|
||||
// handle_tun_outbound should generate ICMPv6 Packet Too Big for
|
||||
// oversized packets instead of forwarding them.
|
||||
let edges = vec![(0, 1)];
|
||||
let mut nodes = run_tree_test(2, &edges, false).await;
|
||||
verify_tree_convergence(&nodes);
|
||||
populate_all_coord_caches(&mut nodes);
|
||||
|
||||
let node0_addr = *nodes[0].node.node_addr();
|
||||
let node1_addr = *nodes[1].node.node_addr();
|
||||
let node1_pubkey = nodes[1].node.identity().pubkey_full();
|
||||
|
||||
let src_fips = crate::FipsAddress::from_node_addr(&node0_addr);
|
||||
let dst_fips = crate::FipsAddress::from_node_addr(&node1_addr);
|
||||
|
||||
// Establish session (XK: 3 messages — Setup, Ack, Msg3)
|
||||
nodes[0].node.initiate_session(node1_addr, node1_pubkey).await.unwrap();
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
process_available_packets(&mut nodes).await;
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
process_available_packets(&mut nodes).await;
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
process_available_packets(&mut nodes).await;
|
||||
|
||||
assert!(nodes[0].node.get_session(&node1_addr).unwrap().state().is_established());
|
||||
|
||||
// Simulate receipt of MtuExceeded by reducing PathMtuState to a value
|
||||
// lower than the local transport MTU.
|
||||
let local_transport_mtu = nodes[0].node.transport_mtu();
|
||||
let reduced_mtu = local_transport_mtu - 200;
|
||||
{
|
||||
let entry = nodes[0].node.get_session_mut(&node1_addr).unwrap();
|
||||
let mmp = entry.mmp_mut().unwrap();
|
||||
mmp.path_mtu.apply_notification(reduced_mtu, std::time::Instant::now());
|
||||
assert_eq!(mmp.path_mtu.current_mtu(), reduced_mtu);
|
||||
}
|
||||
|
||||
// Install TUN receiver on source node to capture ICMPv6 PTB
|
||||
let (tun_tx, tun_rx) = std::sync::mpsc::channel();
|
||||
nodes[0].node.tun_tx = Some(tun_tx);
|
||||
|
||||
// Build an IPv6 packet that fits local MTU but exceeds path MTU
|
||||
let reduced_ipv6_mtu = crate::upper::icmp::effective_ipv6_mtu(reduced_mtu) as usize;
|
||||
let local_ipv6_mtu = nodes[0].node.effective_ipv6_mtu() as usize;
|
||||
let oversized_payload = vec![0u8; reduced_ipv6_mtu - 39]; // 40-byte hdr + payload > reduced MTU
|
||||
let ipv6_packet = build_ipv6_packet(&src_fips, &dst_fips, &oversized_payload);
|
||||
assert!(ipv6_packet.len() > reduced_ipv6_mtu, "packet must exceed path MTU");
|
||||
assert!(ipv6_packet.len() <= local_ipv6_mtu, "packet must fit local MTU");
|
||||
|
||||
nodes[0].node.handle_tun_outbound(ipv6_packet).await;
|
||||
|
||||
// Verify ICMPv6 Packet Too Big was generated
|
||||
let ptb_messages: Vec<Vec<u8>> = std::iter::from_fn(|| tun_rx.try_recv().ok()).collect();
|
||||
assert_eq!(ptb_messages.len(), 1, "Should generate exactly one ICMPv6 PTB");
|
||||
|
||||
let ptb = &ptb_messages[0];
|
||||
assert_eq!(ptb[0] >> 4, 6, "Should be IPv6");
|
||||
assert_eq!(ptb[6], 58, "Next header should be ICMPv6 (58)");
|
||||
assert_eq!(ptb[40], 2, "ICMPv6 type should be Packet Too Big (2)");
|
||||
assert_eq!(ptb[41], 0, "ICMPv6 code should be 0");
|
||||
|
||||
// Verify reported MTU (32-bit field at ICMPv6 header bytes 4-7)
|
||||
let reported_mtu = u32::from_be_bytes([ptb[44], ptb[45], ptb[46], ptb[47]]);
|
||||
assert_eq!(reported_mtu, reduced_ipv6_mtu as u32, "Reported MTU should match path IPv6 MTU");
|
||||
|
||||
// Verify a packet that fits within path MTU passes through (no PTB)
|
||||
let (tun_tx2, tun_rx2) = std::sync::mpsc::channel();
|
||||
nodes[0].node.tun_tx = Some(tun_tx2);
|
||||
let fitting_payload = vec![0u8; reduced_ipv6_mtu - 41]; // fits within path MTU
|
||||
let fitting_packet = build_ipv6_packet(&src_fips, &dst_fips, &fitting_payload);
|
||||
assert!(fitting_packet.len() <= reduced_ipv6_mtu);
|
||||
|
||||
nodes[0].node.handle_tun_outbound(fitting_packet).await;
|
||||
|
||||
// No PTB should be generated for a fitting packet
|
||||
let ptb_messages2: Vec<Vec<u8>> = std::iter::from_fn(|| tun_rx2.try_recv().ok()).collect();
|
||||
assert_eq!(ptb_messages2.len(), 0, "Should not generate PTB for fitting packet");
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Integration test: Multi-hop PMTUD with heterogeneous MTUs
|
||||
// ============================================================================
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_multihop_pmtud_heterogeneous_mtu() {
|
||||
// Three-node chain: A(1400)—B(800)—C(800)
|
||||
//
|
||||
// Node B has a smaller transport MTU than A. When A sends an IPv6
|
||||
// packet that fits A's local MTU (1294) but whose wire size after
|
||||
// FIPS encapsulation exceeds B's transport MTU (800), B's forwarding
|
||||
// path fails with MtuExceeded and sends an MtuExceeded signal back
|
||||
// to A. A updates PathMtuState, and the next oversized packet
|
||||
// generates ICMPv6 Packet Too Big on TUN.
|
||||
//
|
||||
// This exercises the full PMTUD loop:
|
||||
// 1. Oversized packet forwarded A→B
|
||||
// 2. B→C forward fails (B's transport MTU 800 exceeded)
|
||||
// 3. B sends MtuExceeded signal back to A
|
||||
// 4. A receives signal, updates PathMtuState for C
|
||||
// 5. Next oversized packet → ICMPv6 PTB on TUN
|
||||
let mtus = [1400, 800, 800];
|
||||
let edges = vec![(0, 1), (1, 2)];
|
||||
let mut nodes = run_tree_test_with_mtus(&mtus, &edges).await;
|
||||
verify_tree_convergence(&nodes);
|
||||
populate_all_coord_caches(&mut nodes);
|
||||
|
||||
let node0_addr = *nodes[0].node.node_addr();
|
||||
let node2_addr = *nodes[2].node.node_addr();
|
||||
|
||||
let src_fips = crate::FipsAddress::from_node_addr(&node0_addr);
|
||||
let dst_fips = crate::FipsAddress::from_node_addr(&node2_addr);
|
||||
|
||||
// Register Node 2's identity in Node 0's cache
|
||||
let node2_pubkey = nodes[2].node.identity().pubkey_full();
|
||||
nodes[0].node.register_identity(node2_addr, node2_pubkey);
|
||||
|
||||
// Establish session A→C via B (triggers routing through tree)
|
||||
nodes[0].node.initiate_session(node2_addr, node2_pubkey).await.unwrap();
|
||||
drain_to_quiescence(&mut nodes).await;
|
||||
assert!(
|
||||
nodes[0].node.get_session(&node2_addr).unwrap().state().is_established(),
|
||||
"Session A→C should be established"
|
||||
);
|
||||
|
||||
// Exhaust coord warmup by sending small packets first.
|
||||
// Without piggybacked coords, the wire packet is ~106 + IPv6 bytes,
|
||||
// which fits B's receive buffer (mtu+100=900) for reasonable sizes.
|
||||
// With coords (~66 extra), the wire could exceed B's recv buffer.
|
||||
for _ in 0..5 {
|
||||
let small = build_ipv6_packet(&src_fips, &dst_fips, &[0u8; 10]);
|
||||
nodes[0].node.send_session_data(&node2_addr, &small).await.unwrap();
|
||||
}
|
||||
drain_to_quiescence(&mut nodes).await;
|
||||
|
||||
// Build an IPv6 packet that fits A's local MTU (1294) but whose wire
|
||||
// size (~750 + 106 = ~856 bytes) exceeds B's transport MTU (800).
|
||||
// effective_ipv6_mtu(1400) = 1294, effective_ipv6_mtu(800) = 694
|
||||
let oversized_payload = vec![0xABu8; 750 - 40]; // 710 bytes payload → 750-byte IPv6 packet
|
||||
let ipv6_packet = build_ipv6_packet(&src_fips, &dst_fips, &oversized_payload);
|
||||
assert_eq!(ipv6_packet.len(), 750);
|
||||
let local_effective_mtu = crate::upper::icmp::effective_ipv6_mtu(1400) as usize;
|
||||
assert!(
|
||||
ipv6_packet.len() <= local_effective_mtu,
|
||||
"packet ({}) must fit A's local MTU ({})",
|
||||
ipv6_packet.len(), local_effective_mtu
|
||||
);
|
||||
|
||||
// Send the oversized packet — B should fail to forward and send
|
||||
// MtuExceeded signal back.
|
||||
nodes[0].node.send_session_data(&node2_addr, &ipv6_packet).await.unwrap();
|
||||
drain_to_quiescence(&mut nodes).await;
|
||||
|
||||
// Verify PathMtuState was updated on A
|
||||
let path_mtu = {
|
||||
let entry = nodes[0].node.get_session(&node2_addr).unwrap();
|
||||
let mmp = entry.mmp().expect("session should have MMP state");
|
||||
mmp.path_mtu.current_mtu()
|
||||
};
|
||||
assert!(
|
||||
path_mtu < 1400,
|
||||
"PathMtuState should have decreased from MtuExceeded signal, got {}",
|
||||
path_mtu
|
||||
);
|
||||
|
||||
// Now send ANOTHER oversized packet — this time handle_tun_outbound
|
||||
// should check PathMtuState and generate ICMPv6 PTB on TUN instead
|
||||
// of forwarding.
|
||||
let (tun_tx2, tun_rx2) = std::sync::mpsc::channel();
|
||||
nodes[0].node.tun_tx = Some(tun_tx2);
|
||||
|
||||
nodes[0].node.handle_tun_outbound(ipv6_packet.clone()).await;
|
||||
|
||||
let ptb_messages: Vec<Vec<u8>> = std::iter::from_fn(|| tun_rx2.try_recv().ok()).collect();
|
||||
assert_eq!(
|
||||
ptb_messages.len(), 1,
|
||||
"Should generate ICMPv6 PTB for oversized packet after PathMtuState update"
|
||||
);
|
||||
|
||||
let ptb = &ptb_messages[0];
|
||||
assert_eq!(ptb[0] >> 4, 6, "Should be IPv6");
|
||||
assert_eq!(ptb[6], 58, "Next header should be ICMPv6 (58)");
|
||||
assert_eq!(ptb[40], 2, "ICMPv6 type should be Packet Too Big (2)");
|
||||
assert_eq!(ptb[41], 0, "ICMPv6 code should be 0");
|
||||
|
||||
// Verify reported MTU is the path MTU (not local MTU)
|
||||
let reported_mtu = u32::from_be_bytes([ptb[44], ptb[45], ptb[46], ptb[47]]);
|
||||
let expected_ipv6_mtu = crate::upper::icmp::effective_ipv6_mtu(path_mtu) as u32;
|
||||
assert_eq!(
|
||||
reported_mtu, expected_ipv6_mtu,
|
||||
"ICMPv6 PTB MTU should match path IPv6 MTU (transport MTU {} - overhead)",
|
||||
path_mtu
|
||||
);
|
||||
|
||||
// Verify a fitting packet still passes through without PTB
|
||||
let (tun_tx3, tun_rx3) = std::sync::mpsc::channel();
|
||||
nodes[0].node.tun_tx = Some(tun_tx3);
|
||||
|
||||
let fitting_payload = vec![0xCDu8; 600 - 40]; // 600-byte IPv6 packet, well within 694
|
||||
let fitting_packet = build_ipv6_packet(&src_fips, &dst_fips, &fitting_payload);
|
||||
assert!(fitting_packet.len() <= expected_ipv6_mtu as usize);
|
||||
|
||||
nodes[0].node.handle_tun_outbound(fitting_packet).await;
|
||||
|
||||
let ptb_messages3: Vec<Vec<u8>> = std::iter::from_fn(|| tun_rx3.try_recv().ok()).collect();
|
||||
assert_eq!(
|
||||
ptb_messages3.len(), 0,
|
||||
"Should not generate PTB for packet fitting within path MTU"
|
||||
);
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
@@ -16,6 +16,11 @@ pub(super) struct TestNode {
|
||||
|
||||
/// Create a test node with a live UDP transport on localhost.
|
||||
pub(super) async fn make_test_node() -> TestNode {
|
||||
make_test_node_with_mtu(1280).await
|
||||
}
|
||||
|
||||
/// Create a test node with a specific transport MTU.
|
||||
pub(super) async fn make_test_node_with_mtu(mtu: u16) -> TestNode {
|
||||
use crate::config::UdpConfig;
|
||||
use crate::transport::udp::UdpTransport;
|
||||
|
||||
@@ -24,7 +29,7 @@ pub(super) async fn make_test_node() -> TestNode {
|
||||
|
||||
let udp_config = UdpConfig {
|
||||
bind_addr: Some("127.0.0.1:0".to_string()),
|
||||
mtu: Some(1280),
|
||||
mtu: Some(mtu),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
@@ -596,6 +601,44 @@ pub(super) async fn run_tree_test(
|
||||
nodes
|
||||
}
|
||||
|
||||
/// Like `run_tree_test` but with per-node transport MTUs.
|
||||
///
|
||||
/// `mtus` must have one entry per node. Used for heterogeneous-MTU tests
|
||||
/// where different hops have different link-layer capacities.
|
||||
pub(super) async fn run_tree_test_with_mtus(
|
||||
mtus: &[u16],
|
||||
edges: &[(usize, usize)],
|
||||
) -> Vec<TestNode> {
|
||||
let mut nodes = Vec::new();
|
||||
for &mtu in mtus {
|
||||
nodes.push(make_test_node_with_mtu(mtu).await);
|
||||
}
|
||||
|
||||
for &(i, j) in edges {
|
||||
initiate_handshake(&mut nodes, i, j).await;
|
||||
}
|
||||
|
||||
let total = drain_all_packets(&mut nodes, false).await;
|
||||
assert!(total > 0, "Should have processed at least some packets");
|
||||
|
||||
for &(i, j) in edges {
|
||||
let j_addr = *nodes[j].node.node_addr();
|
||||
let i_addr = *nodes[i].node.node_addr();
|
||||
assert!(
|
||||
nodes[i].node.get_peer(&j_addr).is_some(),
|
||||
"Node {} should have peer {} (node {})",
|
||||
i, j_addr, j
|
||||
);
|
||||
assert!(
|
||||
nodes[j].node.get_peer(&i_addr).is_some(),
|
||||
"Node {} should have peer {} (node {})",
|
||||
j, i_addr, i
|
||||
);
|
||||
}
|
||||
|
||||
nodes
|
||||
}
|
||||
|
||||
/// Clean up transports for all test nodes.
|
||||
pub(super) async fn cleanup_nodes(nodes: &mut [TestNode]) {
|
||||
for tn in nodes.iter_mut() {
|
||||
|
||||
Reference in New Issue
Block a user