From 3b6a4da17d71341a2f85817d171caa7188ed80e4 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Thu, 29 Jan 2026 18:17:07 +0000 Subject: [PATCH] Complete FIPS architecture review cleanup - Rename fips-links.md to fips-transports.md, update all references - Update README with all 6 design documents in organized sections - Reorganize architecture review: remove verbose resolved items, consolidate deferred items, focus on actionable issues - Clarify Transport/Link/Peer lifecycle in architecture doc: - Transports static after startup - Links on-demand, driven by peer lifecycle - Connectionless transports (UDP) immediate established - Connection-oriented (Tor) require link setup before auth - Add Resource Limits configuration section (max_peers, max_transports, max_pending_auth, max_pending_lookups, memory_budget) - Close timer management as non-issue (tokio handles scale) - Defer init/shutdown to future iteration Review status: 6 resolved, 5 deferred, 4 low-priority open --- docs/design/README.md | 23 +- docs/design/fips-architecture-review.md | 330 ++++-------------- docs/design/fips-architecture.md | 55 ++- docs/design/fips-design.md | 2 +- docs/design/fips-routing.md | 2 +- .../{fips-links.md => fips-transports.md} | 0 6 files changed, 133 insertions(+), 279 deletions(-) rename docs/design/{fips-links.md => fips-transports.md} (100%) diff --git a/docs/design/README.md b/docs/design/README.md index 431455a..1903783 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -1,10 +1,19 @@ # FIPS Design Documents -Protocol design specifications and analysis. +Protocol design specifications and analysis for the Federated Interoperable Peering System. -| Document | Description | -|--------------------------------------------------------|-------------------------------------------------| -| [fips-design.md](fips-design.md) | Full protocol design specification | -| [fips-links.md](fips-links.md) | Link protocol requirements and characteristics | -| [fips-routing.md](fips-routing.md) | Routing, Bloom filters, discovery, sessions | -| [spanning-tree-dynamics.md](spanning-tree-dynamics.md) | Spanning tree protocol dynamics study | +## Protocol Design + +| Document | Description | +| ------------------------------------------------------ | -------------------------------------------------------------------------------------- | +| [fips-design.md](fips-design.md) | Core protocol specification: goals, architecture, identity, addressing, spanning tree | +| [fips-routing.md](fips-routing.md) | Routing architecture: Bloom filters, discovery protocol, session establishment | +| [fips-transports.md](fips-transports.md) | Transport protocol characteristics: UDP, Ethernet, Tor, radio, and other link types | +| [spanning-tree-dynamics.md](spanning-tree-dynamics.md) | Detailed study of spanning tree gossip protocol behavior and convergence | + +## Implementation + +| Document | Description | +| ---------------------------------------------------------- | --------------------------------------------------------------------------------------- | +| [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 | diff --git a/docs/design/fips-architecture-review.md b/docs/design/fips-architecture-review.md index 4ca1ae0..5161629 100644 --- a/docs/design/fips-architecture-review.md +++ b/docs/design/fips-architecture-review.md @@ -1,193 +1,32 @@ # FIPS Architecture Document Review -**Date**: 2025-01-29 **Document reviewed**: fips-architecture.md +**Last updated**: 2025-01-29 -This file captures critique and open issues identified during architecture review. -Items should be addressed before implementation begins or marked as intentionally -deferred. +This document tracks open issues and deferred items for the FIPS architecture. +Issues are organized by priority. Resolved items have been archived. --- -## High Priority (Implementation Blockers) +## Summary -### 1. Coordinate Ordering Inconsistency +| # | Issue | Priority | Status | Notes | +| -- | ---------------------- | -------- | -------- | -------------------- | +| 11 | Config validation | Low | OPEN | Add validation rules | +| 12 | Default rationales | Low | OPEN | Document reasoning | +| 14 | Gateway details | Low | OPEN | Incomplete spec | +| 15 | DiscoveredPeer hint | Low | OPEN | Type mismatch | +| 2 | Auth protocol ref | High | DEFERRED | Wire protocol work | +| 3 | Concurrency model | High | DEFERRED | Future iteration | +| 4 | Keepalive format | High | DEFERRED | Wire protocol work | +| 7 | Error handling | Medium | DEFERRED | Future iteration | +| 10 | Init/shutdown | Medium | DEFERRED | Future iteration | -**Issue**: Architecture says `[self, parent, ..., root]` but spanning-tree-dynamics.md -sometimes uses `[root, ..., self]` ordering. - -**Location**: fips-architecture.md line 165, spanning-tree-dynamics.md line 346-349 - -**Resolution needed**: Standardize on one ordering across all documents. - -**Status**: RESOLVED - -**Resolution**: Standardized on `[self, parent, ..., root]` (node→root) ordering. -Updated all coordinate/ancestry references in spanning-tree-dynamics.md. +**Resolved (archived):** 1, 5, 6, 8, 9, 13 --- -### 2. Authentication Protocol Not Referenced - -**Issue**: Architecture references "FIPS auth handshake" but doesn't define it or -reference where it's defined. - -**Location**: fips-architecture.md line 285 - -**Resolution needed**: Add reference to fips-design.md lines 115-205, or include -summary in architecture document. - -**Status**: DEFERRED (wire protocol) - -**Note**: Will be addressed when wire protocol section is developed. Not a blocker -for current architecture iteration. - ---- - -### 3. Concurrency Model Unspecified - -**Issue**: Document doesn't specify whether implementation should be: - -- Single-threaded async -- Multi-threaded with message passing -- How state machines are driven (polling, callbacks, async/await) -- Whether transports run in separate threads/tasks - -**Location**: Entire document - -**Resolution needed**: Add "Concurrency Model" section specifying the expected -runtime architecture. - -**Status**: DEFERRED (future iteration) - -**Note**: Architecture document is a work-in-progress. Concurrency model section -will be added in a future design iteration. - ---- - -### 4. Keepalive Message Format Unspecified - -**Issue**: Configuration specifies `peer.keepalive.interval` but no message type -is defined for keepalives. RTT measurement mechanism is unclear. - -**Location**: fips-architecture.md line 762 - -**Resolution needed**: Clarify whether Dummy (0x00) message from fips-design.md -is used, and how RTT is measured (request/response probing or passive observation). - -**Status**: DEFERRED (wire protocol) - -**Note**: Will be addressed when wire protocol section is developed. - ---- - -### 5. Cache Naming Confusion - -**Issue**: Multiple cache references that may or may not be the same thing: - -- `coord_cache: CoordCache` on Node (line 35) -- `coord_cache: HashMap` on TreeState (line 167) -- `discovery.cache.max_entries` configuration (line 745) -- `session.cache.max_entries` configuration (line 751) - -**Resolution needed**: Clarify whether these are the same cache or different caches. -If different, explain the distinction. - -**Status**: RESOLVED - -**Resolution**: Removed misplaced `coord_cache` from TreeState (it belongs only on -Node). Added clarifying text in config section explaining that `discovery.cache` -and `session.cache` configure the same underlying `Node.coord_cache` but are grouped -by purpose (endpoint discovery vs transit routing). - ---- - -## Medium Priority (Design Clarity) - -### 6. Peer vs Link Relationship - -**Issue**: Document states "one-to-one mapping between peers and links" (line 148) -but Links can exist before authentication completes, meaning Links exist without -Peers temporarily. - -**Resolution needed**: Clarify the relationship lifecycle: - -- Link created on connection (before auth) -- Peer created on successful auth -- Peer always references exactly one Link -- Link can exist without Peer (during auth or after auth failure) - -**Status**: OPEN - ---- - -### 7. Error Handling Strategy Absent - -**Issue**: No systematic error handling definitions: - -- Signature verification failure handling -- Malformed packet handling -- Error codes and response formats -- Logging/reporting strategy - -**Resolution needed**: Add "Error Handling" section or reference to protocol spec. - -**Status**: DEFERRED (future iteration) - -**Note**: Error handling section will be added in a future design iteration. - ---- - -### 8. Memory Bounds Missing - -**Issue**: Configuration specifies cache sizes but no limits for: - -- Maximum peers -- Maximum transports -- Maximum pending operations/queues -- Overall memory budget - -**Resolution needed**: Add resource limit configuration or document that these -are implementation-defined. - -**Status**: OPEN - ---- - -### 9. Timer Management at Scale - -**Issue**: Multiple timers per peer (auth timeout, keepalive, reconnect delay, -filter debounce). For 100 peers, this could be hundreds of timers. - -**Resolution needed**: Note that timer wheel or hierarchical timing wheel may -be needed for efficient implementation at scale. - -**Status**: OPEN - ---- - -### 10. Initialization/Shutdown Sequences - -**Issue**: No startup sequence defined: - -- Transport start order -- TUN interface initialization relative to peering -- Behavior when configured peers are unreachable -- When node is "ready" to route - -No shutdown procedure defined: - -- Should nodes announce departure? -- Session termination -- TUN interface cleanup - -**Resolution needed**: Add "Initialization" and "Shutdown" sections. - -**Status**: OPEN - ---- - -## Lower Priority (Polish) +## Open Issues - Low Priority ### 11. Configuration Validation Rules @@ -197,7 +36,7 @@ No shutdown procedure defined: - What if `peer.keepalive.timeout` < `peer.keepalive.interval`? - What if `timeout.adaptive.min` > `timeout.adaptive.max`? -**Status**: OPEN +**Resolution needed**: Add validation rules or defer to implementation. --- @@ -209,29 +48,7 @@ No shutdown procedure defined: - `tree.parent.hold_time = 10s`: Why 10 seconds? - `filter.stale.threshold = 300s`: Based on what analysis? -**Status**: OPEN - ---- - -### 13. Terminology Inconsistency Across Documents - -**Issue**: "Link" and "Transport" meanings differ between documents: - -- fips-links.md uses "link" to mean underlying transport protocol -- fips-architecture.md uses "Link" to mean connection over Transport -- fips-design.md has `FipsLink` trait which is really a transport interface - -**Resolution needed**: Standardize terminology or add glossary. - -**Status**: RESOLVED - -**Resolution**: Updated fips-design.md and fips-links.md to use consistent terminology: - -- Transport = interface/medium (UDP socket, Ethernet NIC, Tor client) -- Link = connection instance to a specific peer over a transport -- Renamed `trait FipsLink` section to reference architecture doc -- Renamed `trait LinkDriver` section to reference architecture doc -- Updated section headers and examples throughout both documents +**Resolution needed**: Add brief rationale comments or design rationale document. --- @@ -243,7 +60,8 @@ No shutdown procedure defined: - How gateway prefixes are advertised in Bloom filters - Configuration for gateway mode -**Status**: OPEN +**Resolution needed**: Expand gateway section or reference future gateway +design document. --- @@ -255,77 +73,65 @@ The discover() return type doesn't show the hint field. **Resolution needed**: Align trait signature with event structure. -**Status**: OPEN +--- + +## Deferred Items + +### Wire Protocol (Future Work) + +These items will be addressed when the wire protocol section is developed: + +| # | Issue | Description | +| --- | ----------------------- | ----------------------------------------------------------------------------------------- | +| 2 | Auth protocol reference | Architecture references "FIPS auth handshake" but doesn't define or reference fips-design | +| 4 | Keepalive format | `peer.keepalive.interval` configured but no message type defined; RTT measurement unclear | + +### Future Architecture Iterations + +These items are deferred to future design iterations: + +| # | Issue | Description | +| --- | ----------------- | -------------------------------------------------------------------------------------------- | +| 3 | Concurrency model | No specification for threading model, state machine driving, or transport isolation | +| 7 | Error handling | No systematic error handling definitions for signature failures, malformed packets, or codes | +| 10 | Init/shutdown | Startup sequence, transport ordering, shutdown procedure, departure announcement | --- -## Edge Cases Not Addressed +## Edge Cases for Future Consideration -### Root Node Failure +### Network Dynamics -- How long until new root is elected? -- How are in-flight sessions affected? -- Any proactive root backup mechanism? +- **Root node failure**: Election timing, in-flight session impact, proactive backup +- **Rapid peer churn**: 500ms debounce may cause filter oscillation +- **Network partition healing**: Session conflicts, stale cache recovery -### Rapid Peer Churn +### Transport Edge Cases -- Debounce (500ms) may not be sufficient -- Could lead to Bloom filter oscillation or announcement storms +- **Startup timing**: Tor takes 30s-2min while UDP is instant; node status during window +- **MTU mismatch**: Path includes links with different MTUs (Ethernet 1500 vs LoRa 222) +- **Clock skew**: TreeAnnounce timestamps; 5-minute tolerance from fips-design.md -### Network Partition Healing +### Security Considerations -- What happens to sessions that existed in both partitions? -- How do routers with stale cache entries recover? +- **Memory exhaustion**: Many fake peers +- **CPU exhaustion**: Signature verification flooding +- **Bandwidth exhaustion**: Bloom filter spam -### Transport Startup Timing +### Configuration Edge Cases -- If Tor takes 180s but UDP is instant, what's node status during window? -- Are Tor-only peers unreachable during Tor bootstrap? - -### MTU Mismatch Across Path - -- Path includes links with different MTUs (Ethernet 1500 vs LoRa 222) -- No path MTU discovery or fragmentation strategy defined - -### Clock Skew - -- TreeAnnounce timestamps with significant clock skew -- 5-minute tolerance in auth (fips-design.md) should be reflected here - -### Resource Exhaustion Attacks - -- Memory exhaustion via many fake peers -- CPU exhaustion via signature verification flooding -- Bandwidth exhaustion via Bloom filter spam - -### Conflicting Peer Configurations - -- Two configured peers with same npub but different addresses -- Discovered peer conflicts with configured peer - -### TUN Interface Unavailable - -- TUN creation fails (permissions, kernel module) -- Can node run in "relay-only" mode without TUN? +- **Conflicting peers**: Same npub with different addresses; discovered vs configured conflict +- **TUN unavailable**: Permissions, kernel module; relay-only mode possibility --- -## Resolution Tracking +## Resolved Items (Archived) -| # | Issue | Priority | Status | Resolution | -|----|------------------------|----------|----------|-------------------------------| -| 1 | Coordinate ordering | High | RESOLVED | node→root standardized | -| 2 | Auth protocol ref | High | DEFERRED | wire protocol (future) | -| 3 | Concurrency model | High | DEFERRED | future iteration | -| 4 | Keepalive format | High | DEFERRED | wire protocol (future) | -| 5 | Cache naming | High | RESOLVED | clarified in architecture | -| 6 | Peer/Link relationship | Medium | OPEN | | -| 7 | Error handling | Medium | DEFERRED | future iteration | -| 8 | Memory bounds | Medium | OPEN | | -| 9 | Timer management | Medium | OPEN | | -| 10 | Init/shutdown | Medium | OPEN | | -| 11 | Config validation | Low | OPEN | | -| 12 | Default rationales | Low | OPEN | | -| 13 | Terminology | Low | RESOLVED | fips-design, fips-links updated| -| 14 | Gateway details | Low | OPEN | | -| 15 | DiscoveredPeer hint | Low | OPEN | | +| # | Issue | Resolution | +| --- | ------------------- | -------------------------------------------------------------------------------------- | +| 1 | Coordinate ordering | Standardized on `[self, parent, ..., root]` (node→root) in all documents | +| 5 | Cache naming | Removed misplaced `coord_cache` from TreeState; clarified config sections | +| 6 | Peer/Link lifecycle | Clarified: transports static, links on-demand driven by peer lifecycle | +| 8 | Memory bounds | Added Resource Limits config: max_peers, max_transports, pending limits, memory budget | +| 9 | Timer management | Non-issue: async runtimes (tokio) handle hundreds of timers efficiently at this scale | +| 13 | Terminology | Standardized Transport/Link terminology in fips-design.md and fips-transports.md | diff --git a/docs/design/fips-architecture.md b/docs/design/fips-architecture.md index 9dd5417..aece3c3 100644 --- a/docs/design/fips-architecture.md +++ b/docs/design/fips-architecture.md @@ -55,7 +55,8 @@ Transport addresses and FIPS identity are fully decoupled. ### Transport -A physical or logical interface over which links can be established. +A physical or logical interface over which links can be established. Transports +are statically configured and exist for the lifetime of the node after startup. ``` Transport (trait) @@ -86,7 +87,9 @@ internally. The FIPS routing layer sees only FIPS packets. ### Link -A communication channel to a specific remote endpoint over a transport. +A communication channel to a specific remote endpoint over a transport. Links are +created on demand when connecting to a peer and torn down when the peer connection +terminates. Link lifecycle is driven by the peer lifecycle. ``` Link @@ -100,10 +103,12 @@ Link ``` For connectionless transports (UDP, Ethernet, WiFi), links are lightweight—just -`(transport_id, remote_addr)` with implicit "always connected" state. +`(transport_id, remote_addr)` with implicit "established" state (no connection +setup required). For connection-oriented transports (Tor), links track real connection state and -hold I/O handles. +hold I/O handles. The link must complete transport-layer connection setup before +FIPS session establishment can proceed. **Link statistics (measured):** @@ -145,9 +150,28 @@ Peer └── link_stats: LinkStats ``` -There is a one-to-one mapping between peers and links. If the same remote node -is reachable via multiple transports, that would be multiple Peer entries (though -for initial implementation, we assume single transport per peer). +**Peer/Link Lifecycle:** + +Links and peers have a one-to-one mapping with coupled lifecycles: + +1. **Outbound connection**: Desire to connect to a peer triggers link creation + over the appropriate transport. For connection-oriented transports (Tor), the + link goes through connection setup; for connectionless (UDP), it immediately + becomes established. Once the link is ready, FIPS authentication proceeds. + +2. **Inbound connection**: Incoming data on a transport creates a link, then + authentication creates the peer. + +3. **Peer references link**: An authenticated peer always references exactly one + active link. + +4. **Termination**: When a peer connection terminates, the associated link is + torn down. For connectionless transports this is trivial cleanup; for + connection-oriented transports this closes the underlying connection. + +If the same remote node is reachable via multiple transports, that would be +multiple Peer entries (though for initial implementation, we assume single +transport per peer). --- @@ -719,6 +743,21 @@ Discovery is per-transport: | `node.tun.device` | string | "fips0" | TUN device name | | `node.tun.mtu` | u16 | 1280 | TUN interface MTU | +### Resource Limits + +| Parameter | Type | Default | Description | +|-----------|------|---------|-------------| +| `limits.max_peers` | u32 | 128 | Maximum concurrent authenticated peers | +| `limits.max_transports` | u8 | 8 | Maximum configured transports | +| `limits.max_pending_auth` | u32 | 32 | Maximum connections awaiting authentication | +| `limits.max_pending_lookups` | u32 | 1000 | Maximum in-flight discovery lookups | +| `limits.memory_budget` | u64 | 0 | Soft memory limit in bytes (0 = unlimited) | + +Limits are enforced at connection time. Exceeding `max_pending_auth` rejects new +inbound connections; exceeding `max_peers` prevents new outbound connections. +The `memory_budget` is advisory—implementations should shed load when approaching +the limit but need not enforce it strictly. + ### Spanning Tree | Parameter | Type | Default | Description | @@ -849,6 +888,6 @@ configure the same underlying cache but are grouped by purpose. ## References - [fips-design.md](fips-design.md) — Overall FIPS protocol design -- [fips-links.md](fips-links.md) — Transport protocols and characteristics +- [fips-transports.md](fips-transports.md) — Transport protocol characteristics - [fips-routing.md](fips-routing.md) — Routing, Bloom filters, discovery - [spanning-tree-dynamics.md](spanning-tree-dynamics.md) — Tree protocol dynamics diff --git a/docs/design/fips-design.md b/docs/design/fips-design.md index 4886091..260ed14 100644 --- a/docs/design/fips-design.md +++ b/docs/design/fips-design.md @@ -488,7 +488,7 @@ Links are connection instances to specific peers over a transport. > **Note**: The Transport trait definition and detailed transport specifications > are in [fips-architecture.md](fips-architecture.md). This section provides a -> conceptual overview. See [fips-links.md](fips-links.md) for transport +> conceptual overview. See [fips-transports.md](fips-transports.md) for transport > characteristics and requirements. ### Transport Interface Concept diff --git a/docs/design/fips-routing.md b/docs/design/fips-routing.md index d8ee1e6..9968061 100644 --- a/docs/design/fips-routing.md +++ b/docs/design/fips-routing.md @@ -604,5 +604,5 @@ When nodes join/leave: ## References - [fips-design.md](fips-design.md) — Overall FIPS architecture -- [fips-links.md](fips-links.md) — Link layer requirements +- [fips-transports.md](fips-transports.md) — Transport protocol characteristics - [spanning-tree-dynamics.md](spanning-tree-dynamics.md) — Tree protocol details diff --git a/docs/design/fips-links.md b/docs/design/fips-transports.md similarity index 100% rename from docs/design/fips-links.md rename to docs/design/fips-transports.md