diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 312a062..8cf3853 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -1,6 +1,6 @@ --- status: draft -last_updated: 2026-07-12 +last_updated: 2026-07-14 --- # Alknet Architecture @@ -194,7 +194,7 @@ adapter location map is now consistent: all HTTP-backed adapters | [007](decisions/007-bistream-type-definition.md) | BiStream Type Definition | Accepted | | [008](decisions/008-secret-service-integration.md) | Vault Integration Point | Accepted | | [009](decisions/009-one-way-door-decision-framework.md) | One-Way Door Decision Framework | Accepted | -| [010](decisions/010-alpn-router-and-endpoint.md) | ALPN Router and Endpoint | Accepted | +| [010](decisions/010-alpn-router-and-endpoint.md) | ALPN Router and Endpoint | Accepted (Amendment 1 superseded by ADR-083 — TCP+TLS dispatch is first-class via public `dispatch`) | | [011](decisions/011-authcontext-structure.md) | AuthContext Structure and Resolution Flow | Accepted | | [012](decisions/012-call-protocol-stream-model.md) | Call Protocol Stream Model | Accepted | | [013](decisions/013-rust-canonical-implementation.md) | Rust as Canonical Implementation Language | Accepted | @@ -211,7 +211,7 @@ adapter location map is now consistent: all HTTP-backed adapters | [024](decisions/024-operation-registry-layering.md) | Operation Registry Layering | Accepted | | [025](decisions/025-vault-local-only-dispatch.md) | Vault Local-Only Dispatch | Accepted | | [026](decisions/026-vault-key-model-hd-derivation.md) | Vault Key Model — HD Derivation | Accepted | -| [027](decisions/027-tls-identity-redesign-acme-rawkey-decoupling.md) | TLS Identity Redesign — ACME + RawKey Decoupling | Accepted | +| [027](decisions/027-tls-identity-redesign-acme-rawkey-decoupling.md) | TLS Identity Redesign — ACME + RawKey Decoupling | Accepted (§5 amended by ADR-083 — guard moves to shared `dispatch`) | | [028](decisions/028-callclient-peer-scoped-registry-filtering.md) | Peer-Scoped Registry Filtering for CallClient Inbound Dispatch | ~~Accepted~~ → **Superseded** by ADR-029 | | [029](decisions/029-peer-graph-routing-model.md) | Peer-Graph Routing Model for alknet-call Composition | Accepted (Assumption 1's `PeerId` source superseded by ADR-030) | | [030](decisions/030-peerentry-and-identity-id-decoupling.md) | PeerEntry and Identity.id Decoupling | Accepted (supersedes ADR-029 Assumption 1's UUID source) | @@ -266,7 +266,8 @@ adapter location map is now consistent: all HTTP-backed adapters | [079](decisions/079-hub-relay-translate-not-forward.md) | Hub Relay — Translate, Not Transparently Forward | Accepted | | [080](decisions/080-channelclient.md) | ChannelClient — the Client Side of a Channels Connection | Accepted | | [081](decisions/081-channels-subcrate-decomposition.md) | channels Sub-Crate Decomposition | Accepted | -| [082](decisions/082-alknet-tls-extraction.md) | alknet-tls Crate Extraction | Proposed | +| [082](decisions/082-alknet-tls-extraction.md) | alknet-tls Crate Extraction | Proposed (amended — endpoint signature superseded by ADR-083) | +| [083](decisions/083-endpoint-as-accept-loop-runner.md) | Endpoint as Pure Accept-Loop Runner with Public Dispatch | Proposed | ## Open Questions diff --git a/docs/architecture/crates/tls/README.md b/docs/architecture/crates/tls/README.md index c16407f..f38cf91 100644 --- a/docs/architecture/crates/tls/README.md +++ b/docs/architecture/crates/tls/README.md @@ -1,6 +1,6 @@ --- status: draft -last_updated: 2026-07-13 +last_updated: 2026-07-14 --- # alknet-tls @@ -267,26 +267,47 @@ production and `rustls::sign` in the test helper `build_ed25519_spki_der` ### What `AlknetEndpoint` does after the refactor `AlknetEndpoint::new()` currently builds `TlsSetup` internally. After -the refactor, it takes an `Arc`: +the refactor (see [ADR-083](../../decisions/083-endpoint-as-accept-loop-runner.md)), +the endpoint takes **no TLS config at all** — it is a pure accept-loop +runner with a public `dispatch` method: ```rust impl AlknetEndpoint { - pub async fn new( - static_config: &StaticConfig, - tls_config: Arc, // ← new param + pub fn new( handlers: HandlerRegistry, dynamic: Arc>, identity_provider: Arc, - ) -> Result; + drain_timeout: Duration, + ) -> Self; + + pub fn with_quinn(mut self, endpoint: quinn::Endpoint) -> Self; + pub fn with_iroh(mut self, endpoint: iroh::Endpoint) -> Self; + + pub fn dispatch( + &self, + connection: Connection, + alpn: Vec, + fingerprint: Option, + remote_addr: Option, + ); + + pub async fn run(self: Arc); } ``` -The endpoint calls `tls_config.for_quinn()` to get its quinn server -config. The ACME handle is on the `TlsServerConfig`, not the endpoint — -the endpoint doesn't own the ACME task. The assembly layer builds the -`TlsServerConfig` once, passes `Arc::clone()` to the endpoint, and -passes another `Arc::clone()` to the TCP+TLS accept loop (which lives in -the hub or a future `TcpTlsAcceptor`). +The assembly layer builds the `TlsServerConfig`(s), builds the +transports (`for_quinn()` → `quinn::Endpoint::server()`, +`for_tcp_tls()` → `TlsAcceptor`, the `Ed25519SecretKey` → iroh), and +hands the pre-built quinn/iroh endpoints to `AlknetEndpoint` via +builder methods. A hub serving native clients and browsers holds two +`TlsServerConfig`s (raw key + X.509/ACME); the endpoint takes neither — +it takes the already-built transport endpoints. The TCP+TLS accept +loops (one per config) call `endpoint.dispatch(...)` — the same +dispatch path quinn and iroh use. The ACME handle lives on the +`TlsServerConfig`, not the endpoint. + +This resolves the single-`Arc` problem: the endpoint +has no "the TLS config" to take because a hub has two. ### The TCP+TLS accept loop (out of scope for this crate) @@ -336,6 +357,7 @@ All design decisions are documented as ADRs in | ADR | Decision | Summary | |-----|----------|---------| | [082](../../decisions/082-alknet-tls-extraction.md) | alknet-tls crate extraction | Extract TLS setup from alknet-core/endpoint.rs; `TlsServerConfig` shareable across quinn + TCP+TLS + iroh; one ACME state machine | +| [083](../../decisions/083-endpoint-as-accept-loop-runner.md) | Endpoint as accept-loop runner | `AlknetEndpoint` takes no TLS config; assembly layer builds transports from `TlsServerConfig`s; public `dispatch` method; `acme-tls/1` guard moves to shared `dispatch` | ## Open Questions @@ -348,6 +370,17 @@ See [open-questions.md](../../open-questions.md) for full details. on `alknet-tls` — a new dep edge. If it stays, core keeps a narrow `rustls` dep. Decision-ready — the answer depends on whether we want core to be `rustls`-free. +- **OQ-60** (open): Where does transport construction live? The + endpoint-as-accept-loop-runner boundary (ADR-083) commits to + construction being outside the endpoint, but the destination — + assembly layer, `alknet-tls` convenience helpers, or a transport + module/crate — is open. `alknet-tls`'s stated job is "TLS setup, not + transport accept logic," which is an argument against the helper + option. +- **OQ-61** (open): Multi-owner shutdown coordination. The endpoint owns + dispatched handlers (spawned in `dispatch`); the assembly layer owns + spawned accept loops (TCP+TLS). The coordination mechanism (shared + `shutdown_sender`, drain semantics) is unspecified. ## References diff --git a/docs/architecture/decisions/027-tls-identity-redesign-acme-rawkey-decoupling.md b/docs/architecture/decisions/027-tls-identity-redesign-acme-rawkey-decoupling.md index afabec5..f5da2dd 100644 --- a/docs/architecture/decisions/027-tls-identity-redesign-acme-rawkey-decoupling.md +++ b/docs/architecture/decisions/027-tls-identity-redesign-acme-rawkey-decoupling.md @@ -2,7 +2,10 @@ ## Status -Accepted +Accepted (§5 amended by ADR-083 — the `acme-tls/1` guard moves from +`dispatch_quinn` to the shared `dispatch` method, since ACME challenges +arrive over TCP+TLS, not QUIC; the rationale holds, only the location +changes) ## Context @@ -171,11 +174,13 @@ if let Some(TlsIdentity::RawKey(key)) = static_config.tls_identity.as_ref() { `iroh::SecretKey::from_bytes(&[u8; 32])` accepts raw Ed25519 key bytes — no information loss. This conversion is `#[cfg(feature = "iroh")]` only. -### 5. ACME ALPN challenge handling in `dispatch_quinn` +### 5. ACME ALPN challenge handling in `dispatch` (moved from `dispatch_quinn` by ADR-083) -Add an early-return guard in `dispatch_quinn` before the handler lookup: +Add an early-return guard in `dispatch` (the shared dispatch path, +moved from `dispatch_quinn` by ADR-083) before the handler lookup: ```rust +// In the shared `dispatch` method (moved from `dispatch_quinn` by ADR-083): if alpn == b"acme-tls/1" { debug!("acme-tls/1 challenge connection completed at TLS layer; closing"); connection.close(0u32.into(), b"acme done"); @@ -185,7 +190,13 @@ if alpn == b"acme-tls/1" { This avoids the misleading "no handler for ALPN" warning. The challenge is already answered at the TLS layer; the application just closes -gracefully. No `ProtocolHandler` registration for `acme-tls/1`. +gracefully. No `ProtocolHandler` registration for `acme-tls/1`. The +guard is transport-agnostic — it fires for any connection whose TLS +handshake negotiated `acme-tls/1`, regardless of which transport +delivered it. In practice ACME TLS-ALPN-01 challenges arrive over +TCP+TLS (CAs validate via TCP to port 443, not QUIC); advertising +`acme-tls/1` on a QUIC listener that shares the ACME config is +harmless. See ADR-083 for the full rationale. ### 6. Feature-gate ACME behind a new `acme` feature diff --git a/docs/architecture/decisions/082-alknet-tls-extraction.md b/docs/architecture/decisions/082-alknet-tls-extraction.md index ba309b6..c4a646d 100644 --- a/docs/architecture/decisions/082-alknet-tls-extraction.md +++ b/docs/architecture/decisions/082-alknet-tls-extraction.md @@ -2,7 +2,9 @@ ## Status -Proposed +Proposed (amended 2026-07-14: the `AlknetEndpoint::new` signature +referenced here was superseded by ADR-083 — the endpoint takes no TLS +config; the assembly layer builds transports from `TlsServerConfig`s) ## Context @@ -127,7 +129,7 @@ impl TlsServerConfig { | `Ed25519SecretKey` | `alknet-core/config.rs` | Config type — iroh reads it directly | | `AcmeDirectory` | `alknet-core/config.rs` | Config type | | `fingerprint.rs` | `alknet-core` | Shared by server (endpoint) and client (`alknet-call`'s `FingerprintPinVerifier`) — moving it would create a dep edge from `alknet-call` to `alknet-tls`. Production code uses `sha2` + manual DER only; `rustls` is test-only. See OQ-59. | -| `AlknetEndpoint` | `alknet-core` | The endpoint struct stays; it takes `Arc` instead of building it internally | +| `AlknetEndpoint` | `alknet-core` | The endpoint struct stays; it takes no TLS config (see ADR-083) | ### Feature gates @@ -148,25 +150,25 @@ when `acme` is enabled. `rustls-pki-types` is available via `rustls`'s re-export (core lists it directly; the new crate can rely on the re-export or list it directly — implementation detail). -### `AlknetEndpoint` takes `Arc` +### `AlknetEndpoint` takes no TLS config (see ADR-083) -```rust -impl AlknetEndpoint { - pub async fn new( - static_config: &StaticConfig, - tls_config: Arc, // ← new param - handlers: HandlerRegistry, - dynamic: Arc>, - identity_provider: Arc, - ) -> Result; -} -``` +ADR-082's original proposal was that `AlknetEndpoint::new` would take +`Arc`. That does not hold: a hub serving both native +clients (raw key) and browsers (X.509/ACME) holds **two** +`TlsServerConfig`s, and the endpoint has no single "the TLS config" to +take. The endpoint takes no TLS config at all — it is a pure +accept-loop runner with a public `dispatch` method. The assembly layer +builds the `TlsServerConfig`s and the transports, and hands the +pre-built quinn/iroh endpoints to `AlknetEndpoint` via builder methods. +See [ADR-083](083-endpoint-as-accept-loop-runner.md) for the endpoint's +new shape and signature. -The endpoint calls `tls_config.for_quinn()` to get its quinn server -config. The ACME handle lives on the `TlsServerConfig`, not the -endpoint — the endpoint doesn't own the ACME task. The assembly layer -builds the `TlsServerConfig` once, passes `Arc::clone()` to the -endpoint, and passes another `Arc::clone()` to the TCP+TLS accept loop. +`alknet-tls`'s job is to make the cert available to whichever transports +the deployment runs. The endpoint's job is to dispatch. The two are +decoupled — `alknet-tls` provides `TlsServerConfig` and its accessors +(`for_quinn`, `for_tcp_tls`, `rustls_config`); the assembly layer wires +them to transports; the endpoint dispatches connections from those +transports. ### The TCP+TLS accept loop lives outside `alknet-tls` @@ -228,11 +230,12 @@ that compiles and passes type-checks but silently changes TLS behavior: (requires X.509) is the exception, not the constraint. **Negative:** -- `AlknetEndpoint::new` gains a new parameter (`Arc`), - which is a breaking change for existing callers. The assembly layer - (CLI binary, hub construction) must build the `TlsServerConfig` before - constructing the endpoint. This is expected — it's the point of the - extraction. +- `AlknetEndpoint::new` no longer takes a `tls_config` parameter, which + is a breaking change for existing callers. The assembly layer builds + the `TlsServerConfig`s and the transports and hands pre-built + endpoints to `AlknetEndpoint` via builder methods (see + [ADR-083](083-endpoint-as-accept-loop-runner.md)). This is expected — + it is the point of the extraction. - `alknet-core` may keep a narrow `rustls` dep if `fingerprint.rs` stays (OQ-59). If `fingerprint.rs` moves to `alknet-tls`, `alknet-call`'s client-side `FingerprintPinVerifier` gains a dep on `alknet-tls`. The @@ -243,11 +246,12 @@ that compiles and passes type-checks but silently changes TLS behavior: ## Door type **One-way.** Extracting TLS setup into a shareable config is a -structural change: `AlknetEndpoint::new` signature changes, the assembly -layer must build `TlsServerConfig` separately, and the cert-sharing -contract (one config, N transports) becomes the architecture. Reversing -would mean re-welding TLS to the endpoint and losing the multi-transport -cert-reuse capability — the exact capability the hub needs. +structural change: the assembly layer must build `TlsServerConfig` +separately and the cert-sharing contract (one config, N transports) +becomes the architecture. Reversing would mean re-welding TLS to the +endpoint and losing the multi-transport cert-reuse capability — the +exact capability the hub needs. The `AlknetEndpoint::new` signature +change is documented in ADR-083, not here. The `TlsServerConfig` API surface (`new`, `for_quinn`, `for_tcp_tls`, `rustls_config`) is one-way — changing it after consumers exist is a @@ -259,6 +263,9 @@ details that can change without breaking the contract. - ADR-010 Amendment 1 — TCP+TLS dispatch via `from_stream` (the accept loop that consumes `TlsServerConfig::for_tcp_tls()`) +- ADR-083 — endpoint as pure accept-loop runner with public dispatch + (the endpoint takes no TLS config; the assembly layer builds + transports from `TlsServerConfig`s) - ADR-027 — `TlsIdentity` (RawKey / X509 / Acme), RFC 7250, browser limitation - ADR-030 §6 — fingerprint normalization (`ed25519:` across diff --git a/docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md b/docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md new file mode 100644 index 0000000..3582ae8 --- /dev/null +++ b/docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md @@ -0,0 +1,368 @@ +# ADR-083: Endpoint as Pure Accept-Loop Runner with Public Dispatch + +## Status + +Proposed + +## Context + +`AlknetEndpoint` (ADR-010) conflates two concerns: + +1. **Transport construction** — building `quinn::Endpoint` and + `iroh::Endpoint`, reading `static_config.tls_identity`, constructing TLS + internally (`TlsSetup::new`, `build_rustls_server_config`), wrapping + rustls into `quinn::ServerConfig`, binding. +2. **Dispatch** — take a connection, extract its ALPN, look up the + handler, build `AuthContext`, spawn the handler. + +The conflation was tolerable while TLS was welded to quinn (one transport, +one cert path). Two things surfaced it: + +- **ADR-082 (`alknet-tls` extraction)** pulls quinn's TLS construction + out of the endpoint into a shareable `TlsServerConfig`. Once the cert + is built outside the endpoint, the endpoint's transport-construction + role is the odd one out — it's the only thing still reading + `static_config` and building transports. +- **A hub holds two `TlsServerConfig`s**, not one (raw key for native + clients on QUIC + TCP+TLS fallback; X.509/ACME for HTTPS on TCP+TLS). + ADR-082's original signature `AlknetEndpoint::new(..., Arc)` + cannot represent this — a hub has two configs and the endpoint has no + single "the TLS config" to take. The TLS extraction surfaces this + because the endpoint can no longer build the one cert it used to own. + +### The dispatch logic is the endpoint's real value + +Stripped of transport construction, what the endpoint provides is +**dispatch**: take a connection, extract its ALPN, look up the handler, +build `AuthContext`, spawn. This is identical for every transport. The +transport-specific parts are narrow bookends: + +- **ALPN extraction** — quinn: `handshake_data().protocol`; iroh: + `connecting.alpn().await`; TCP+TLS: `tls_stream.alpn()`. +- **Fingerprint extraction** — quinn: `peer_identity()` → + `CertificateDer`; iroh: `remote_id()` → `ed25519:`; TCP+TLS: + `tls_stream.peer_certificates()` → `CertificateDer`. +- **Connection construction** — quinn: `from_quinn_with_alpn`; iroh: + `from_iroh`; TCP+TLS: `from_bidi(tls_stream, alpn, remote_addr)`. +- **No-handler close** — quinn/iroh: `connection.close()`; TCP+TLS: drop + the stream (the `Connection::close` API handles both uniformly — + ADR-065). + +Everything between those bookends — the ACME ALPN guard, the handler +lookup, the `build_auth_context` call, the `tokio::spawn` — is identical. +Exposing a public `dispatch` method that takes the already-extracted +ALPN, fingerprint, and `Connection` lets every transport's accept loop +call the same dispatch path. No duplicated `build_auth_context` or +handler-lookup logic at the assembly layer. + +### TCP+TLS is a first-class transport, not a sibling afterthought + +ADR-010 Amendment 1 made TCP+TLS a "sibling accept loop" outside the +endpoint because the endpoint's dispatch logic was crate-private and +couldn't be shared. The assembly layer had to duplicate +`build_auth_context` and handler lookup, or call `HandlerRegistry::get` +directly. The "sibling" framing was a workaround for the endpoint being +welded to quinn — not a deliberate design. + +With a public `dispatch` method, the TCP+TLS accept loop calls into the +endpoint — the same path quinn and iroh use. Amendment 1's *ownership* +model (the TCP+TLS listener owns its own `TcpListener` and `TlsAcceptor`, +living outside the endpoint struct) survives; its *dispatch* workaround +(duplicated `build_auth_context` and handler-lookup logic) is retired. + +TCP+TLS is the most common transport after raw-key QUIC, not an edge +case: HTTPS for browsers, worker registration over HTTP, raw-key fallback +for native clients when UDP is blocked. + +### The `acme-tls/1` guard + +ADR-027 §5 places the `acme-tls/1` early-return guard in +`dispatch_quinn`. The rationale (no handler for `acme-tls/1`; the +challenge is answered at the TLS layer; close gracefully) is correct. +The **location** is wrong once TCP+TLS exists. + +ACME TLS-ALPN-01 (RFC 8737) challenges are validated by the CA +connecting **over TCP to port 443**. Let's Encrypt's validator is a TCP +TLS client — it does not speak QUIC. In any real hub deployment, the +ACME challenge arrives on the **TCP+TLS** listener (443), not the QUIC +listener (4433). Today the guard fires only because quinn happens to be +the listener that's bound to the ACME port — an artifact of TCP+TLS not +existing yet, not a property of QUIC. + +The guard's job is transport-agnostic: "if the TLS handshake negotiated +`acme-tls/1`, this is a challenge connection — the cert was already +served at the TLS layer via `ResolvesServerCertAcme`, close it, no +handler." `Connection::close` is transport-abstracted (ADR-065), so +the guard works uniformly for quinn, iroh, and TCP+TLS. The right home is +the **shared `dispatch` method**, not `dispatch_quinn`. In practice only +the TCP+TLS listener on 443 receives challenges (CAs validate via TCP); +advertising `acme-tls/1` on a QUIC listener that shares the ACME config +is harmless — no QUIC client negotiates it. + +### `StaticConfig`'s role shifts + +`StaticConfig` today holds `listen_addr`, `tls_identity`, `iroh_relay`, +`drain_timeout`. After this ADR, the endpoint reads only `drain_timeout` +(passed directly to `new`); the assembly layer reads the rest to build +transports. `StaticConfig` becomes the canonical deployment config the +**assembly layer** reads, not "the thing the endpoint takes." It stays +in `alknet-core` (it's a config type) and stays extensible (future +transport config fields land here as the assembly layer needs them). + +This is a conceptual shift from ADR-010's framing ("the CLI binary +constructs a `HandlerRegistry` and passes it, with `StaticConfig`, to +`AlknetEndpoint::new()`"). The endpoint no longer takes `StaticConfig` +at all. The endpoint takes `drain_timeout` and pre-built transport +endpoints; the assembly layer is the `StaticConfig` consumer. + +## Decision + +### The endpoint is a pure accept-loop runner + a public dispatch method + +```rust +pub struct AlknetEndpoint { + quinn: Option, + iroh: Option, + handlers: Arc, // ADR-010 + dynamic: Arc>, // ADR-010, unchanged + identity_provider: Arc, // ADR-010, unchanged + shutdown_tx: watch::Sender, + shutdown_rx: watch::Receiver, + drain_timeout: Duration, +} + +impl AlknetEndpoint { + pub fn new( + handlers: HandlerRegistry, + dynamic: Arc>, + identity_provider: Arc, + drain_timeout: Duration, + ) -> Self; + + pub fn with_quinn(mut self, endpoint: quinn::Endpoint) -> Self; + pub fn with_iroh(mut self, endpoint: iroh::Endpoint) -> Self; + + /// Clone of the shutdown watch sender. The assembly layer uses this + /// to signal its own accept loops (TCP+TLS) to stop accepting — one + /// signal, all loops stop. See OQ-61 for the full coordination model. + pub fn shutdown_sender(&self) -> watch::Sender; + + /// Dispatch a pre-established connection by ALPN. Called by the + /// endpoint's own accept loops (quinn, iroh) after transport- + /// specific extraction, and by external accept loops (TCP+TLS, + /// future SSH, future WebTransport) after their own extraction. + /// + /// Synchronous (non-async): performs the ACME guard, handler + /// lookup, `build_auth_context`, and `tokio::spawn`s the handler. + /// Returns immediately after spawning — the handler runs on its own + /// task. The caller's accept loop is not blocked by handler + /// execution. + /// + /// No-handler match (ALPN not in the registry): closes the + /// connection and logs a warning, per ADR-010's existing dispatch + /// behavior. Does not panic, does not return an error — the + /// connection is dropped, the accept loop continues. + pub fn dispatch( + &self, + connection: Connection, + alpn: Vec, + fingerprint: Option, + remote_addr: Option, + ); + + /// Run the endpoint's owned accept loops. Returns when shutdown is + /// signaled. The caller drives shutdown via `shutdown_sender()`. + pub async fn run(self: Arc); +} +``` + +The consumer pattern: the assembly layer holds `Arc`, +clones the `Arc` for `run` (which consumes one clone), and passes +`&endpoint` to its TCP+TLS accept loops so they can call `dispatch`. +After `run` returns, the assembly layer drops its `Arc`; in-flight +dispatched handlers (spawned by `dispatch` via `tokio::spawn`) are +owned by the endpoint's runtime and drain per `shutdown` / OQ-61. + +`AlknetEndpoint::new` takes **no `StaticConfig`** and **no TLS config**. +The assembly layer (the deployment binary that composes crates — see +ADR-014) reads `StaticConfig`, builds the transports, and hands the +quinn/iroh endpoints to `AlknetEndpoint` via builder methods. The +`handlers`, `dynamic`, and `identity_provider` fields are unchanged +passthroughs from ADR-010 — this ADR does not change auth or dynamic +config; it changes who builds transports and where dispatch lives. + +`build_iroh_endpoint` moves out of `alknet-core/endpoint.rs`. Its +destination is an open question (see OQ-60) — the assembly layer, an +`alknet-tls` convenience helper, or a transport-construction +module/crate. This ADR commits to the **boundary** (construction is +not in the endpoint); the *where* is tracked separately so it doesn't +block the endpoint-shape decision. (`build_quinn_server_config_from_rustls` +is different: it's a thin wrapper that converts a `rustls::ServerConfig` +into a `quinn::ServerConfig`, which is exactly `TlsServerConfig::for_quinn()` +— its destination is `alknet-tls`, decided in ADR-082. Only +`build_iroh_endpoint`, which reads `StaticConfig` and builds an +`iroh::Endpoint` without a rustls config, is genuinely undecided.) + +### `dispatch` is public and transport-agnostic + +The public `dispatch` method takes the already-extracted ALPN, +fingerprint (if available), remote address (if available), and a +`Connection`. It performs the ACME guard, the handler lookup, the +`build_auth_context` call, and the `tokio::spawn`. `build_auth_context` +becomes a private helper called by `dispatch`, not a standalone +function — the assembly layer calls `dispatch`, which calls +`build_auth_context` internally. + +Transport-specific extraction (`extract_quinn_alpn`, +`extract_quinn_client_fingerprint`, `extract_iroh_client_fingerprint`) +stays private in the endpoint — the accept loops call them, then call +`dispatch` with the results. + +### The `acme-tls/1` guard moves to `dispatch` + +```rust +pub fn dispatch(&self, connection: Connection, alpn: Vec, ...) { + if alpn == b"acme-tls/1" { + debug!("acme-tls/1 challenge connection completed at TLS layer; closing"); + connection.close(0, "acme done"); + return; + } + // ... handler lookup, build_auth_context, spawn +} +``` + +The guard is **not** feature-gated on `acme`: if the `acme` feature is +off, no transport advertises `acme-tls/1`, the ALPN never negotiates, the +guard never fires. It is harmless dead code without the feature. + +This supersedes ADR-027 §5's "in `dispatch_quinn`" location. The +rationale (no handler for `acme-tls/1`, silent close) holds; only the +location changes. The `acme-tls/1` ALPN append still happens in +`TlsServerConfig::new`'s ACME branch (ADR-082), not per-transport — every +transport using the ACME `TlsServerConfig` advertises it, but in +practice only the TCP+TLS listener on 443 receives challenges. + +### `StaticConfig` stays in core; the endpoint drops it + +`StaticConfig` remains in `alknet-core/config.rs` as the canonical +deployment config the assembly layer reads. The endpoint no longer takes +`&StaticConfig` — `new()` takes `drain_timeout: Duration` directly. +Future transport config fields (addresses, relay URLs, identity) land in +`StaticConfig` as the assembly layer needs them; the endpoint's `new` +signature does not change when they're added, because the endpoint +doesn't read them. + +### What moves out of `alknet-core/endpoint.rs` + +| Code | Destination | +|------|-------------| +| `build_rustls_server_config()` | `alknet-tls` (ADR-082) | +| `TlsSetup` / ACME state machine | `alknet-tls` (ADR-082) | +| `RawKeyCertResolver` | `alknet-tls` (ADR-082) | +| `Ed25519SigningKey` | `alknet-tls` (ADR-082) | +| `AcceptAnyCertVerifier` | `alknet-tls` (ADR-082) | +| `SelfSignedCert` / `generate_self_signed_cert()` | `alknet-tls` (ADR-082) | +| `load_cert_chain()` / `load_private_key()` | `alknet-tls` (ADR-082) | +| `build_quinn_server_config_from_rustls()` | `alknet-tls` (`for_quinn()`, ADR-082) | +| `build_iroh_endpoint()` | Out of core (destination: OQ-60) | + +### What stays in `alknet-core/endpoint.rs` + +- `AlknetEndpoint` struct (accept-loop runner + public `dispatch`) +- `HandlerRegistry` +- `dispatch` (public — ACME guard, handler lookup, `build_auth_context`, + spawn) +- `dispatch_quinn` / `dispatch_iroh` (private — transport-specific + extraction, then call `dispatch`) +- `run_quinn_accept_loop` / `run_iroh_accept_loop` +- `extract_quinn_alpn` / `extract_quinn_client_fingerprint` / + `extract_iroh_client_fingerprint` +- `build_auth_context` (private helper, called by `dispatch`) + +### ADR-082 amendment + +ADR-082's `AlknetEndpoint::new(..., Arc, ...)` signature +is superseded by this ADR. The endpoint takes no TLS config; the +assembly layer builds transports from `TlsServerConfig`s and hands them +to the endpoint via `with_quinn` / `with_iroh`. ADR-082 should reference +this ADR for the endpoint signature and focus on what `alknet-tls` +provides (`TlsServerConfig` and its accessors). + +## Consequences + +**Positive:** +- The endpoint has one job: dispatch. Transport construction lives + outside it, where the multi-`TlsServerConfig` hub case is natural. +- TCP+TLS dispatch is first-class — same `dispatch` path as quinn/iroh, + no duplicated `build_auth_context` or handler-lookup logic at the + assembly layer. ADR-010 Amendment 1's second-class-dispatch workaround + is retired. +- The `acme-tls/1` guard is transport-agnostic — it works for TCP+TLS + (where challenges actually arrive) and any future transport. ADR-027 + §5's quinn-specific location is corrected. +- `StaticConfig`'s role is clear: it's the assembly-layer config, not + the endpoint's. Adding transport config fields doesn't churn the + endpoint's `new` signature. +- The hub (the first multi-transport consumer) is unblocked: build two + `TlsServerConfig`s, build quinn/iroh/TCP+TLS listeners, hand the + endpoint the quinn/iroh ones, spawn the TCP+TLS loops calling + `endpoint.dispatch`. + +**Negative:** +- `AlknetEndpoint::new` signature changes (breaking). Pre-1.0, in-repo + consumers only — the assembly layer and tests must update. Expected; + this is the point of the refactor. +- `build_iroh_endpoint` leaves core. Its destination is an open question + (OQ-60), not decided here. The boundary (not in the endpoint) is + committed; the *where* is not, so that the endpoint-shape decision + isn't blocked on the transport-construction-location decision. + (`build_quinn_server_config_from_rustls` is decided — it moves to + `alknet-tls` as `for_quinn()` per ADR-082; only `build_iroh_endpoint` + is open.) +- Multi-owner shutdown: the endpoint owns shutdown of *dispatched + handlers* (it spawned them in `dispatch`); the assembly layer owns + shutdown of the *accept loops it spawned* (TCP+TLS listeners). The + coordination mechanism (shared `shutdown_sender`, drain semantics) is + a follow-up design point, not resolved by this ADR — see OQ-61. + +## Door type + +**One-way.** The endpoint's `new` signature, the public `dispatch` +contract, and the "endpoint owns dispatch, not construction" boundary +are structural. Reversing would mean re-welding transport construction +to the endpoint and re-privatizing `dispatch` — breaking every +multi-transport consumer (hub, future HTTP, future SSH). + +The `dispatch` signature (`connection, alpn, fingerprint, remote_addr`) +is one-way — changing it after consumers exist is a rewrite. The +internal implementation (how extraction is factored, how `run` spawns +tasks) is two-way. + +## References + +- ADR-010: ALPN router and endpoint (amended — the endpoint no longer + constructs transports; Amendment 1's second-class-dispatch workaround + is retired by the public `dispatch` method) +- ADR-014: Secret material flow and capability injection (defines the + "assembly layer" term — the deployment binary that composes crates) +- ADR-010 Amendment 1: TCP+TLS as sibling (superseded — Amendment 1's + *ownership* model (sibling listener outside the endpoint) survives; + its *dispatch* workaround (duplicated logic) is retired by this ADR's + public `dispatch`) +- ADR-027 §5: ACME ALPN challenge handling (location amended — guard + moves from `dispatch_quinn` to shared `dispatch`) +- ADR-065: `Connection::from_stream`/`from_bidi` (the primitive that + makes TCP+TLS dispatch possible; `Connection::close` is + transport-abstracted, so the `acme-tls/1` guard works uniformly) +- ADR-082: `alknet-tls` extraction (amended — endpoint takes no + `Arc`; the assembly layer builds transports from + `TlsServerConfig`s) +- ADR-080: `ChannelClient::from_connection` (the transport-agnostic + client pattern this ADR mirrors on the server side) +- `docs/research/alknet-endpoint-refactor/findings.md` — the analysis + that surfaced the conflation and the two-config hub case +- `crates/alknet-core/src/endpoint.rs` — the code being refactored +- OQ-60: Where does transport construction live? (assembly layer, + `alknet-tls` helper, or transport module/crate) +- OQ-61: Multi-owner shutdown coordination (endpoint owns dispatched + handlers; assembly layer owns spawned accept loops) \ No newline at end of file diff --git a/docs/architecture/open-questions.md b/docs/architecture/open-questions.md index de014ce..221c3c8 100644 --- a/docs/architecture/open-questions.md +++ b/docs/architecture/open-questions.md @@ -1,6 +1,6 @@ --- status: draft -last_updated: 2026-07-12 +last_updated: 2026-07-14 --- # Open Questions @@ -77,6 +77,8 @@ Door type is separate from whether a decision is made. A two-way door is a decis | [OQ-14](questions/014-batch-operation-semantics.md) | Batch Operation Semantics | resolved | two | low | | [OQ-55](questions/055-alknetclient-establishment-extraction.md) | AlknetClient / Client Establishment Extraction | deferred(scope) | two | med | | [OQ-59](questions/059-fingerprint-module-location.md) | Should `fingerprint.rs` Stay in Core or Move to `alknet-tls`? | open | two | med | +| [OQ-60](questions/060-transport-construction-location.md) | Where Does Transport Construction Live? | open | one | high | +| [OQ-61](questions/061-multi-owner-shutdown-coordination.md) | Multi-Owner Shutdown Coordination | open | two | med | ### alknet-call diff --git a/docs/architecture/questions/060-transport-construction-location.md b/docs/architecture/questions/060-transport-construction-location.md new file mode 100644 index 0000000..063594c --- /dev/null +++ b/docs/architecture/questions/060-transport-construction-location.md @@ -0,0 +1,50 @@ +# OQ-60: Where Does Transport Construction Live? + +- **Origin**: `docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md` + (the endpoint refactor commits to the boundary — construction is not + in the endpoint — but not the location of `build_iroh_endpoint`). +- **Status**: open +- **Door type**: one-way (where `build_iroh_endpoint` lives determines + who depends on `iroh` for transport construction; moving it later + churns the dep graph and every binary's assembly code) +- **Priority**: high (the hub is the first multi-transport consumer; + its assembly code sets the pattern) +- **Blocked on**: nothing structural. The three options are clear; the + decision is a trade-off, not a missing capability. +- **Scope**: This OQ covers **`build_iroh_endpoint`** — the function + that reads `StaticConfig` and builds an `iroh::Endpoint`. + `build_quinn_server_config_from_rustls` is **decided** (it moves to + `alknet-tls` as `TlsServerConfig::for_quinn()` per ADR-082 — it's a + thin wrapper over a `rustls::ServerConfig`, which `alknet-tls` owns). + Only `build_iroh_endpoint` is genuinely undecided: it reads + `StaticConfig` (not a rustls config), builds an `iroh::Endpoint` from + an `Ed25519SecretKey` + relay URL, and doesn't fit the `alknet-tls` + cert-provider boundary. +- **Resolution**: Not yet decided. The options: + + **Option A: In the assembly layer (the binary).** Each binary reads + `StaticConfig` and hand-assembles quinn/iroh/TCP+TLS. Pro: maximal + flexibility, core stays lean. Con: every binary duplicates the + "build an iroh endpoint from an `Ed25519SecretKey` + relay URL" + boilerplate; a 10-step procedure is copy-pasted per binary. + + **Option B: In `alknet-tls` as convenience helpers.** `alknet-tls` + gains a `build_iroh_endpoint` helper. Pro: one place. Con: `alknet-tls` + then depends on `iroh`, bloats a crate whose stated job is "TLS setup, + not transport endpoint construction" (ADR-082 scopes `alknet-tls` to + cert config and its accessors — `build_iroh_endpoint` doesn't touch + certs, it touches iroh's relay/secret-key APIs). + + **Option C: A new crate or module — `alknet-transport` / an + `alknet-core::transport` module — that owns transport construction.** + Pro: clean separation (TLS = certs, transport = endpoints, endpoint = + dispatch). Con: another layer in the dep graph. + + The ADR-082 boundary ("`alknet-tls` is the cert provider, not the + transport constructor") is a strong argument against Option B. Option C + is the cleanest separation but adds a layer. Option A is simplest but + risks per-binary duplication that drifts. +- **Cross-references**: ADR-083 (endpoint refactor), ADR-082 + (`alknet-tls` — the cert provider boundary; `for_quinn()` is in scope, + `build_iroh_endpoint` is not), ADR-010 (original endpoint design, + where construction was welded to the endpoint) \ No newline at end of file diff --git a/docs/architecture/questions/061-multi-owner-shutdown-coordination.md b/docs/architecture/questions/061-multi-owner-shutdown-coordination.md new file mode 100644 index 0000000..33af08a --- /dev/null +++ b/docs/architecture/questions/061-multi-owner-shutdown-coordination.md @@ -0,0 +1,42 @@ +# OQ-61: Multi-Owner Shutdown Coordination + +- **Origin**: `docs/architecture/decisions/083-endpoint-as-accept-loop-runner.md` + (the endpoint owns dispatched handlers; the assembly layer owns + spawned accept loops — coordination between them on shutdown is + unspecified). +- **Status**: open +- **Door type**: two-way (the coordination mechanism is an + implementation detail; the ownership boundary — endpoint owns + dispatched handlers, assembly layer owns spawned accept loops — is + the one-way part, committed in ADR-083) +- **Priority**: medium (matters for clean hub shutdown; doesn't block + the endpoint-shape decision or the TLS extraction) +- **Blocked on**: nothing structural. The question is which + coordination primitive (shared `shutdown_sender`, separate channel, + drain semantics) — a design choice, not a missing capability. +- **Resolution**: Not yet decided. The boundary is clear: + + - The endpoint owns shutdown of **dispatched handlers** (it spawned + them via `tokio::spawn` in `dispatch`). + - The assembly layer owns shutdown of the **accept loops it spawned** + (the TCP+TLS listeners). + + The open question is the **coordination mechanism**: + + - Does the assembly layer use the endpoint's `shutdown_sender()` to + signal the TCP+TLS loops, or a separate channel? + - Does `endpoint.shutdown()` drain in-flight dispatched handlers + (including those from TCP+TLS loops), or only the quinn/iroh ones + it spawned via `run()`? + - What happens to in-flight `dispatch` calls after the endpoint is + shut down — are they rejected, or do they complete? + + A likely shape: the assembly layer uses the endpoint's + `shutdown_sender()` for all its accept loops (one signal, all loops + stop accepting); the endpoint's `shutdown()` drains all dispatched + handlers regardless of which transport spawned them (the endpoint + owns them all once `dispatch` is called). But this needs to be + written down and verified against the drain semantics. +- **Cross-references**: ADR-083 (endpoint refactor — ownership + boundary), ADR-010 (original shutdown design — single-owner, the + simpler case being generalized) \ No newline at end of file