Session 43: CLI config option and state machine design

Add command-line argument parsing with clap:
- -c/--config option to specify config file path
- Overrides default search path when provided
- Proper error handling for missing/invalid files

Improve logging:
- Peer connection log now uses separate log entries per field
- Better readability with aligned timestamps

Fix ICMPv6 error handling:
- Add multicast destination filter to should_send_icmp_error()
- Router Solicitation packets (ff02::2) now silently dropped
- Add test case for multicast destination

Add phase-based state machine design document:
- Document pattern where lifecycle phases use distinct structs in enum
- PeerSlot::Connecting(PeerConnection) -> PeerSlot::Active(ActivePeer)
- Benefits: type safety, memory efficiency, security
- Describes timeout handling and lookup table requirements

Cargo.lock

@@ -11,6 +11,56 @@ dependencies = [
  "memchr",
 ]
 
+[[package]]
+name = "anstream"
+version = "0.6.21"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "43d5b281e737544384e969a5ccad3f1cdd24b48086a0fc1b2a5262a26b8f4f4a"
+dependencies = [
+ "anstyle",
+ "anstyle-parse",
+ "anstyle-query",
+ "anstyle-wincon",
+ "colorchoice",
+ "is_terminal_polyfill",
+ "utf8parse",
+]
+
+[[package]]
+name = "anstyle"
+version = "1.0.13"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5192cca8006f1fd4f7237516f40fa183bb07f8fbdfedaa0036de5ea9b0b45e78"
+
+[[package]]
+name = "anstyle-parse"
+version = "0.2.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4e7644824f0aa2c7b9384579234ef10eb7efb6a0deb83f9630a49594dd9c15c2"
+dependencies = [
+ "utf8parse",
+]
+
+[[package]]
+name = "anstyle-query"
+version = "1.1.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc"
+dependencies = [
+ "windows-sys 0.61.2",
+]
+
+[[package]]
+name = "anstyle-wincon"
+version = "3.0.11"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d"
+dependencies = [
+ "anstyle",
+ "once_cell_polyfill",
+ "windows-sys 0.61.2",
+]
+
 [[package]]
 name = "anyhow"
 version = "1.0.100"
@@ -151,6 +201,52 @@ version = "0.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724"
 
+[[package]]
+name = "clap"
+version = "4.5.56"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a75ca66430e33a14957acc24c5077b503e7d374151b2b4b3a10c83b4ceb4be0e"
+dependencies = [
+ "clap_builder",
+ "clap_derive",
+]
+
+[[package]]
+name = "clap_builder"
+version = "4.5.56"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "793207c7fa6300a0608d1080b858e5fdbe713cdc1c8db9fb17777d8a13e63df0"
+dependencies = [
+ "anstream",
+ "anstyle",
+ "clap_lex",
+ "strsim",
+]
+
+[[package]]
+name = "clap_derive"
+version = "4.5.55"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a92793da1a46a5f2a02a6f4c46c6496b28c43638adea8306fcb0caa1634f24e5"
+dependencies = [
+ "heck",
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "clap_lex"
+version = "0.7.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c3e64b0cc0439b12df2fa678eae89a1c56a529fd067a9115f7827f1fffd22b32"
+
+[[package]]
+name = "colorchoice"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b05b61dc5112cbb17e4b6cd61790d9845d13888356391624cbe7e41efeac1e75"
+
 [[package]]
 name = "concurrent-queue"
 version = "2.5.0"
@@ -270,6 +366,7 @@ name = "fips"
 version = "0.1.0"
 dependencies = [
  "bech32",
+ "clap",
  "dirs",
  "futures",
  "hex",
@@ -426,6 +523,12 @@ version = "0.16.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100"
 
+[[package]]
+name = "heck"
+version = "0.5.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea"
+
 [[package]]
 name = "hex"
 version = "0.4.3"
@@ -457,6 +560,12 @@ version = "2.11.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "469fb0b9cefa57e3ef31275ee7cacb78f2fdca44e4765491884a2b119d4eb130"
 
+[[package]]
+name = "is_terminal_polyfill"
+version = "1.70.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695"
+
 [[package]]
 name = "itoa"
 version = "1.0.17"
@@ -635,6 +744,12 @@ version = "1.21.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d"
 
+[[package]]
+name = "once_cell_polyfill"
+version = "1.70.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe"
+
 [[package]]
 name = "option-ext"
 version = "0.2.0"
@@ -925,6 +1040,12 @@ dependencies = [
  "windows-sys 0.60.2",
 ]
 
+[[package]]
+name = "strsim"
+version = "0.11.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f"
+
 [[package]]
 name = "syn"
 version = "2.0.114"
@@ -1138,6 +1259,12 @@ version = "0.2.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861"
 
+[[package]]
+name = "utf8parse"
+version = "0.2.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821"
+
 [[package]]
 name = "valuable"
 version = "0.1.1"

Cargo.toml

@@ -13,6 +13,7 @@ serde = { version = "1.0", features = ["derive"] }
 serde_yaml = "0.9"
 dirs = "6.0"
 hex = "0.4"
+clap = { version = "4.5", features = ["derive"] }
 tracing = "0.1"
 tracing-subscriber = { version = "0.3", features = ["env-filter"] }
 tun = { version = "0.7", features = ["async"] }

docs/design/README.md

@@ -18,3 +18,5 @@ Protocol design specifications and analysis for the Federated Interoperable Peer
 | [fips-architecture.md](fips-architecture.md)               | Software architecture: entities, state machines, transport abstractions, configuration  |
 | [fips-architecture-review.md](fips-architecture-review.md) | Architecture review issues and resolution status                                        |
 | [fips-tun-driver.md](fips-tun-driver.md)                   | TUN interface driver: reader/writer threads, ICMPv6, packet flow                        |
+| [fips-state-machines.md](fips-state-machines.md)           | Phase-based state machine pattern: peer lifecycle, transitions, timeout handling        |
+| [fips-protocol-flow.md](fips-protocol-flow.md)             | Protocol message flow: packet channel, event loop, dispatching                          |

docs/design/fips-state-machines.md

@@ -0,0 +1,360 @@
+# FIPS State Machine Design
+
+This document describes the phase-based state machine pattern used throughout
+FIPS, where different lifecycle phases are represented by distinct struct types
+wrapped in an enum rather than a single struct with a state field.
+
+## Pattern Overview
+
+### Traditional Approach (Single Struct + State Enum)
+
+```rust
+enum PeerState {
+    Connecting,
+    Authenticating,
+    Active,
+    Disconnected,
+}
+
+struct Peer {
+    identity: PeerIdentity,
+    state: PeerState,
+    // Fields needed by ALL states
+    ephemeral_key: Option<Keypair>,      // Only used during auth
+    session_keys: Option<SessionKeys>,    // Only valid when Active
+    tree_coords: Option<TreeCoordinate>,  // Only valid when Active
+    // ...
+}
+
+impl Peer {
+    fn handle_packet(&mut self, packet: &[u8]) {
+        match self.state {
+            PeerState::Connecting => { /* must check we're not Active */ }
+            PeerState::Active => { /* must check session_keys.is_some() */ }
+            // ...
+        }
+    }
+}
+```
+
+**Problems:**
+
+- Fields that only apply to certain states are `Option<T>` or uninitialized
+- Methods must check state before operating (runtime errors possible)
+- Auth-phase secrets (ephemeral keys) persist in memory after auth completes
+- Single struct grows to accommodate all phases
+
+### Phase-Based Approach (Enum of Structs)
+
+```rust
+/// What the Node stores per peer slot
+enum PeerSlot {
+    Connecting(PeerConnection),
+    Active(ActivePeer),
+}
+
+/// Handles authentication handshake only
+struct PeerConnection {
+    identity: PeerIdentity,
+    link_id: LinkId,
+    direction: Direction,
+    // Handshake-specific state
+    ephemeral_keypair: Keypair,
+    remote_ephemeral: Option<PublicKey>,
+    handshake_hash: [u8; 32],
+    attempts: u32,
+    last_sent: Instant,
+}
+
+/// Fully authenticated peer
+struct ActivePeer {
+    identity: PeerIdentity,
+    link_id: LinkId,
+    session: SessionKeys,
+    // Routing state
+    declaration: Option<ParentDeclaration>,
+    coords: Option<TreeCoordinate>,
+    inbound_filter: Option<BloomFilter>,
+    last_seen: Instant,
+}
+```
+
+**Benefits:**
+
+- Each struct only contains fields relevant to that phase
+- Methods can't be called in wrong state (compile-time safety)
+- Ephemeral keys automatically dropped when `PeerConnection` → `ActivePeer`
+- Each phase struct is smaller, simpler, independently testable
+
+## Transition Pattern
+
+Phase transitions return the new phase, consuming the old:
+
+```rust
+impl PeerConnection {
+    /// Handle incoming packet during handshake
+    fn handle_packet(self, packet: &[u8]) -> ConnectionResult {
+        // Parse and validate...
+        match self.state {
+            HandshakeState::WaitingForResponse => {
+                // Verify response, derive session keys
+                let session = self.derive_session_keys(&response);
+                ConnectionResult::Authenticated {
+                    peer: ActivePeer {
+                        identity: self.identity,
+                        link_id: self.link_id,
+                        session,
+                        declaration: None,
+                        coords: None,
+                        inbound_filter: None,
+                        last_seen: Instant::now(),
+                    },
+                }
+            }
+            // ...
+        }
+    }
+}
+
+enum ConnectionResult {
+    /// Stay in connecting phase, send this response
+    Continue(Vec<u8>),
+    /// Auth complete, here's the active peer
+    Authenticated { peer: ActivePeer },
+    /// Auth failed
+    Failed(String),
+}
+```
+
+The Node's event loop handles the transition:
+
+```rust
+fn handle_packet(&mut self, link_id: LinkId, packet: &[u8]) {
+    let slot = self.peers.get_mut(&link_id);
+
+    match slot {
+        PeerSlot::Connecting(conn) => {
+            // Note: take() to move ownership for transition
+            let conn = std::mem::take(conn);
+            match conn.handle_packet(packet) {
+                ConnectionResult::Continue(response) => {
+                    *slot = PeerSlot::Connecting(conn);
+                    self.send(link_id, response);
+                }
+                ConnectionResult::Authenticated { peer } => {
+                    *slot = PeerSlot::Active(peer);
+                    self.on_peer_active(link_id);
+                }
+                ConnectionResult::Failed(reason) => {
+                    self.peers.remove(&link_id);
+                    warn!(%reason, "Peer auth failed");
+                }
+            }
+        }
+        PeerSlot::Active(peer) => {
+            let actions = peer.handle_packet(packet);
+            self.execute(actions);
+        }
+    }
+}
+```
+
+## Timeout Handling
+
+Each phase struct tracks its own timing. The Node's event loop periodically
+scans for timeouts:
+
+```rust
+impl PeerConnection {
+    fn check_timeout(&mut self, now: Instant) -> TimeoutResult {
+        if now.duration_since(self.last_sent) < HANDSHAKE_TIMEOUT {
+            return TimeoutResult::Ok;
+        }
+
+        self.attempts += 1;
+        if self.attempts > MAX_HANDSHAKE_ATTEMPTS {
+            return TimeoutResult::GiveUp;
+        }
+
+        self.last_sent = now;
+        TimeoutResult::Retry(self.build_retry_packet())
+    }
+}
+
+enum TimeoutResult {
+    Ok,
+    Retry(Vec<u8>),
+    GiveUp,
+}
+```
+
+Node event loop (simple periodic scan):
+
+```rust
+loop {
+    select! {
+        packet = packet_rx.recv() => { /* dispatch */ }
+
+        _ = interval.tick() => {
+            let now = Instant::now();
+            let mut to_remove = vec![];
+
+            for (id, slot) in &mut self.peers {
+                if let PeerSlot::Connecting(conn) = slot {
+                    match conn.check_timeout(now) {
+                        TimeoutResult::Retry(packet) => {
+                            self.send(conn.link_id, packet);
+                        }
+                        TimeoutResult::GiveUp => {
+                            to_remove.push(*id);
+                        }
+                        TimeoutResult::Ok => {}
+                    }
+                }
+            }
+
+            for id in to_remove {
+                self.peers.remove(&id);
+            }
+        }
+    }
+}
+```
+
+## Application in FIPS
+
+### Peer Lifecycle
+
+```text
+PeerSlot::Connecting(PeerConnection)
+    │
+    │ AuthInit/AuthResponse exchange
+    │ Noise KK handshake
+    ▼
+PeerSlot::Active(ActivePeer)
+    │
+    │ Link failure / explicit disconnect
+    ▼
+[removed from peers map]
+```
+
+**PeerConnection** contains:
+
+- Noise handshake state (ephemeral keys, handshake hash)
+- Retry tracking (attempts, last_sent)
+- Direction (Inbound vs Outbound)
+
+**ActivePeer** contains:
+
+- Session keys (for encrypt/decrypt)
+- Tree position (declaration, coordinates)
+- Bloom filter (what's reachable through this peer)
+- Statistics (last_seen, link_stats)
+
+### Link Lifecycle (Connection-Oriented Transports)
+
+For transports like Tor that require connection setup:
+
+```rust
+enum LinkSlot {
+    Connecting(LinkConnection),
+    Established(EstablishedLink),
+}
+
+struct LinkConnection {
+    transport_id: TransportId,
+    remote_addr: TransportAddr,
+    connect_started: Instant,
+    // Tor circuit build state, etc.
+}
+
+struct EstablishedLink {
+    transport_id: TransportId,
+    remote_addr: TransportAddr,
+    // I/O handles
+    writer: TorWriter,
+    // Stats
+    established_at: Instant,
+}
+```
+
+For connectionless transports (UDP), links are immediately "established" -
+no `LinkConnection` phase needed.
+
+### Node Lifecycle
+
+```rust
+enum NodePhase {
+    Created(CreatedNode),
+    Starting(StartingNode),
+    Running(RunningNode),
+    Stopping(StoppingNode),
+}
+```
+
+Currently the Node uses a simpler `NodeState` enum because startup/shutdown
+are brief and don't need complex per-phase logic. Phase-based approach would
+be useful if startup involved multi-step async operations with retries.
+
+### Transport Lifecycle
+
+```rust
+enum TransportPhase {
+    Configured(ConfiguredTransport),
+    Starting(StartingTransport),
+    Up(UpTransport),
+    Failed(FailedTransport),
+}
+```
+
+Again, currently simpler because transport startup is straightforward.
+Would be valuable for transports with complex initialization (Tor bootstrap).
+
+## When to Use This Pattern
+
+**Use phase-based structs when:**
+
+- Different phases have different fields (auth secrets vs session keys)
+- Phase-specific logic is complex enough to benefit from isolation
+- Security-sensitive data should be dropped after phase completion
+- You want compile-time enforcement of valid operations per phase
+
+**Use simple state enum when:**
+
+- All phases share the same fields
+- Phase transitions are simple (just flip a flag)
+- The struct is small and phase logic is trivial
+
+## Lookup Tables
+
+When using `PeerSlot` enum, need reverse lookups for packet dispatch:
+
+```rust
+struct Node {
+    // Primary storage
+    peers: HashMap<NodeId, PeerSlot>,
+
+    // Reverse lookup: (transport, remote_addr) → NodeId
+    // Needed because ReceivedPacket has addr, not NodeId
+    addr_to_peer: HashMap<(TransportId, TransportAddr), NodeId>,
+}
+```
+
+For inbound connections from unknown addresses:
+
+1. Parse packet → extract sender's identity (in AuthInit)
+2. Create new PeerConnection
+3. Add to `peers` and `addr_to_peer`
+
+## Summary
+
+The phase-based state machine pattern provides:
+
+1. **Type safety** - Can't call auth methods on active peer
+2. **Memory efficiency** - Phase-specific data dropped on transition
+3. **Clarity** - Each struct is focused and comprehensible
+4. **Security** - Ephemeral keys don't linger after auth
+5. **Testability** - Each phase testable in isolation
+
+The cost is slightly more complex transition handling in the event loop,
+but this is offset by simpler per-phase logic.

src/bin/fips.rs

@@ -2,10 +2,21 @@
 //!
 //! Loads configuration and creates the top-level node instance.
 
+use clap::Parser;
 use fips::{Config, Node, TunState};
+use std::path::PathBuf;
 use tracing::{error, info, warn, Level};
 use tracing_subscriber::{fmt, EnvFilter};
 
+/// FIPS mesh network daemon
+#[derive(Parser, Debug)]
+#[command(name = "fips", version, about)]
+struct Args {
+    /// Path to configuration file (overrides default search paths)
+    #[arg(short, long, value_name = "FILE")]
+    config: Option<PathBuf>,
+}
+
 #[tokio::main(flavor = "current_thread")]
 async fn main() {
     // Initialize logging
@@ -18,15 +29,29 @@ async fn main() {
         .with_target(true)
         .init();
 
+    let args = Args::parse();
+
     info!("FIPS starting");
 
     // Load configuration
     info!("Loading configuration");
-    let (config, loaded_paths) = match Config::load() {
+    let (config, loaded_paths) = if let Some(config_path) = &args.config {
-        Ok(result) => result,
+        // Explicit config file specified - load only that file
-        Err(e) => {
+        match Config::load_file(config_path) {
-            error!("Failed to load configuration: {}", e);
+            Ok(config) => (config, vec![config_path.clone()]),
-            std::process::exit(1);
+            Err(e) => {
+                error!("Failed to load configuration from {}: {}", config_path.display(), e);
+                std::process::exit(1);
+            }
+        }
+    } else {
+        // Use default search paths
+        match Config::load() {
+            Ok(result) => result,
+            Err(e) => {
+                error!("Failed to load configuration: {}", e);
+                std::process::exit(1);
+            }
         }
     };
 

src/icmp.rs

@@ -66,6 +66,7 @@ const MAX_ORIGINAL_PACKET: usize = MIN_IPV6_MTU - IPV6_HEADER_LEN - ICMPV6_HEADE
 /// - Not IPv6
 /// - An ICMPv6 error message itself
 /// - Has a multicast source address
+/// - Has a multicast destination address
 /// - Has an unspecified source address (::)
 pub fn should_send_icmp_error(packet: &[u8]) -> bool {
     // Must have at least an IPv6 header
@@ -92,6 +93,15 @@ pub fn should_send_icmp_error(packet: &[u8]) -> bool {
         return false;
     }
 
+    // Extract destination address
+    let dst = Ipv6Addr::from(<[u8; 16]>::try_from(&packet[24..40]).unwrap());
+
+    // Don't send errors for multicast destination (first byte 0xff)
+    // e.g., ff02::2 (all-routers) from Router Solicitation
+    if dst.octets()[0] == 0xff {
+        return false;
+    }
+
     // Don't send errors for ICMPv6 error messages (types 0-127)
     let next_header = packet[6];
     if next_header == IPPROTO_ICMPV6 && packet.len() > IPV6_HEADER_LEN {
@@ -292,6 +302,15 @@ mod tests {
         assert!(!should_send_icmp_error(&packet));
     }
 
+    #[test]
+    fn test_should_not_send_error_multicast_destination() {
+        let src = "fe80::1".parse().unwrap();
+        let dst = "ff02::2".parse().unwrap(); // all-routers multicast
+        let packet = make_ipv6_packet(src, dst, 17, &[0u8; 8]);
+
+        assert!(!should_send_icmp_error(&packet));
+    }
+
     #[test]
     fn test_should_not_send_error_for_icmp_error() {
         let src = "fd00::1".parse().unwrap();

src/node.rs

@@ -424,15 +424,12 @@ impl Node {
                 .map(|a| format!(" ({})", a))
                 .unwrap_or_default();
 
-            info!(
+            info!("Peer connection initiated{}", alias_display);
-                npub = %peer_config.npub,
+            info!("  npub: {}", peer_config.npub);
-                node_id = %peer_node_id,
+            info!("  node_id: {}", peer_node_id);
-                transport = %addr.transport,
+            info!("  transport: {}", addr.transport);
-                addr = %addr.addr,
+            info!("  addr: {}", addr.addr);
-                link_id = %link_id,
+            info!("  link_id: {}", link_id);
-                "Peer connection initiated{}",
-                alias_display
-            );
 
             self.peers.insert(peer_node_id, peer);