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
This commit is contained in:
Johnathan Corgan
2026-01-29 18:17:07 +00:00
parent fc20c47119
commit 3b6a4da17d
6 changed files with 133 additions and 279 deletions
+15 -6
View File
@@ -1,10 +1,19 @@
# FIPS Design Documents
Protocol design specifications and analysis.
Protocol design specifications and analysis for the Federated Interoperable Peering System.
## Protocol Design
| 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 |
| ------------------------------------------------------ | -------------------------------------------------------------------------------------- |
| [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 |
+68 -262
View File
@@ -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<Ipv6Addr, CachedCoords>` 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 |
+47 -8
View File
@@ -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
+1 -1
View File
@@ -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
+1 -1
View File
@@ -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