From 914778c8a8396816ca6d3de78a529bb52fc18e20 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 19 Apr 2026 10:43:51 -0400 Subject: [PATCH] refactor(server): remove inbound TLS termination Remove server-side TLS listener support so Fabro only binds plain TCP or Unix sockets, and update docs/tests around proxy-terminated HTTPS. This also drops the removed [server.listen.tls] config shape and the inbound TLS-specific diagnostics, fixtures, and integration coverage. --- Cargo.lock | 8 - docs/administration/deploy-digital-ocean.mdx | 7 +- docs/administration/deploy-fly-io.mdx | 5 +- docs/administration/deploy-railway.mdx | 7 +- docs/administration/deploy-render.mdx | 7 +- docs/administration/deploy-server.mdx | 15 +- docs/administration/security.mdx | 8 +- docs/administration/server-configuration.mdx | 6 +- docs/administration/troubleshooting.mdx | 4 +- docs/api-reference/demo-mode.mdx | 2 +- docs/api-reference/overview.mdx | 18 +- docs/reference/architecture.mdx | 20 +- docs/reference/cli.mdx | 4 +- docs/reference/user-configuration.mdx | 8 +- lib/crates/fabro-config/src/resolve/server.rs | 53 +--- lib/crates/fabro-config/tests/resolve_root.rs | 7 +- .../fabro-config/tests/resolve_server.rs | 18 +- lib/crates/fabro-server/Cargo.toml | 8 - lib/crates/fabro-server/src/diagnostics.rs | 71 ----- lib/crates/fabro-server/src/lib.rs | 1 - lib/crates/fabro-server/src/serve.rs | 41 +-- lib/crates/fabro-server/src/settings_view.rs | 13 +- lib/crates/fabro-server/src/tls.rs | 121 -------- .../fabro-server/tests/fixtures/mtls/ca.crt | 9 - .../tests/fixtures/mtls/client.crt | 9 - .../tests/fixtures/mtls/client.key | 3 - .../fixtures/mtls/jwt-ed25519-private.pem | 3 - .../fixtures/mtls/jwt-ed25519-public.pem | 3 - .../tests/fixtures/mtls/server.crt | 9 - .../tests/fixtures/mtls/server.key | 3 - .../tests/fixtures/mtls/wrong-client.crt | 9 - .../tests/fixtures/mtls/wrong-client.key | 3 - lib/crates/fabro-server/tests/it/api/docs.rs | 37 +++ lib/crates/fabro-server/tests/it/api/mod.rs | 4 +- lib/crates/fabro-server/tests/it/api/runs.rs | 4 - .../fabro-server/tests/it/api/settings.rs | 4 - lib/crates/fabro-server/tests/it/api/tcp.rs | 280 ++++++++++++++++++ lib/crates/fabro-server/tests/it/api/tls.rs | 155 ---------- lib/crates/fabro-types/src/settings/mod.rs | 2 +- .../fabro-types/src/settings/resolved.rs | 12 +- lib/crates/fabro-types/src/settings/server.rs | 30 +- 41 files changed, 406 insertions(+), 625 deletions(-) delete mode 100644 lib/crates/fabro-server/src/tls.rs delete mode 100644 lib/crates/fabro-server/tests/fixtures/mtls/ca.crt delete mode 100644 lib/crates/fabro-server/tests/fixtures/mtls/client.crt delete mode 100644 lib/crates/fabro-server/tests/fixtures/mtls/client.key delete mode 100644 lib/crates/fabro-server/tests/fixtures/mtls/jwt-ed25519-private.pem delete mode 100644 lib/crates/fabro-server/tests/fixtures/mtls/jwt-ed25519-public.pem delete mode 100644 lib/crates/fabro-server/tests/fixtures/mtls/server.crt delete mode 100644 lib/crates/fabro-server/tests/fixtures/mtls/server.key delete mode 100644 lib/crates/fabro-server/tests/fixtures/mtls/wrong-client.crt delete mode 100644 lib/crates/fabro-server/tests/fixtures/mtls/wrong-client.key create mode 100644 lib/crates/fabro-server/tests/it/api/docs.rs create mode 100644 lib/crates/fabro-server/tests/it/api/tcp.rs delete mode 100644 lib/crates/fabro-server/tests/it/api/tls.rs diff --git a/Cargo.lock b/Cargo.lock index a1804ed1b..6f2dfec48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1951,8 +1951,6 @@ dependencies = [ "hmac", "http-body-util", "httpmock", - "hyper", - "hyper-util", "ipnet", "jsonwebtoken", "mime_guess", @@ -1960,9 +1958,6 @@ dependencies = [ "object_store", "rand 0.9.4", "regex", - "rustls", - "rustls-pemfile", - "rustls-pki-types", "semver", "serde", "serde_json", @@ -1971,18 +1966,15 @@ dependencies = [ "tempfile", "thiserror 2.0.18", "tokio", - "tokio-rustls", "tokio-stream", "toml 0.8.23", "toml_edit", "tower", "tower-http", - "tower-service", "tracing", "ulid", "uuid", "walkdir", - "x509-parser", ] [[package]] diff --git a/docs/administration/deploy-digital-ocean.mdx b/docs/administration/deploy-digital-ocean.mdx index b82c862d2..764757212 100644 --- a/docs/administration/deploy-digital-ocean.mdx +++ b/docs/administration/deploy-digital-ocean.mdx @@ -93,8 +93,9 @@ Once `https:///health` returns `ok`, two things to grab: 2. **Point your local CLI at the server** — add the URL to `~/.fabro/settings.toml`: ```toml title="~/.fabro/settings.toml" - [server] - target = "https://fabro.example.com/api/v1" + [cli.target] + type = "http" + url = "https://fabro.example.com/api/v1" ``` Then commands like `fabro model list --server ` will hit your Droplet. @@ -131,6 +132,6 @@ To pin a specific version instead of `:nightly`, edit `docker-compose.yaml` and Auth, dev tokens, submitting runs, and pointing the CLI at your deployment. - Full `settings.toml` reference — TLS, auth methods, concurrency, and more. + Full `settings.toml` reference — reverse-proxy TLS, auth methods, concurrency, and more. diff --git a/docs/administration/deploy-fly-io.mdx b/docs/administration/deploy-fly-io.mdx index cf3c4e61b..51bc1d21f 100644 --- a/docs/administration/deploy-fly-io.mdx +++ b/docs/administration/deploy-fly-io.mdx @@ -75,8 +75,9 @@ Once the deploy is healthy, Fly exposes a `.fly.dev` URL (or your custom do 2. **Point your local CLI at the server** — add the Fly URL to `~/.fabro/settings.toml`: ```toml title="~/.fabro/settings.toml" - [server] - target = "https://.fly.dev/api/v1" + [cli.target] + type = "http" + url = "https://.fly.dev/api/v1" ``` Then commands like `fabro model list --server ` will hit your Fly instance. diff --git a/docs/administration/deploy-railway.mdx b/docs/administration/deploy-railway.mdx index e0560b019..c932246fb 100644 --- a/docs/administration/deploy-railway.mdx +++ b/docs/administration/deploy-railway.mdx @@ -52,8 +52,9 @@ Once the deploy is healthy, Railway exposes a `*.up.railway.app` URL (or your cu 2. **Point your local CLI at the server** — add the Railway URL to `~/.fabro/settings.toml`: ```toml title="~/.fabro/settings.toml" - [server] - target = "https://.up.railway.app/api/v1" + [cli.target] + type = "http" + url = "https://.up.railway.app/api/v1" ``` Then commands like `fabro model list --server ` will hit your Railway instance. @@ -77,6 +78,6 @@ Railway re-pulls the GHCR image on every deploy. `Dockerfile.deploy` references Auth, dev tokens, submitting runs, and pointing the CLI at your deployment. - Full `settings.toml` reference — TLS, auth methods, concurrency, and more. + Full `settings.toml` reference — reverse-proxy TLS, auth methods, concurrency, and more. diff --git a/docs/administration/deploy-render.mdx b/docs/administration/deploy-render.mdx index 2f2bdc1fb..ff8837365 100644 --- a/docs/administration/deploy-render.mdx +++ b/docs/administration/deploy-render.mdx @@ -58,8 +58,9 @@ Once the deploy is healthy, Render exposes a `*.onrender.com` URL (or your custo 2. **Point your local CLI at the server** — add the Render URL to `~/.fabro/settings.toml`: ```toml title="~/.fabro/settings.toml" - [server] - target = "https://.onrender.com/api/v1" + [cli.target] + type = "http" + url = "https://.onrender.com/api/v1" ``` Then commands like `fabro model list --server ` will hit your Render instance. @@ -83,6 +84,6 @@ Render re-pulls the GHCR image on every deploy. `Dockerfile.deploy` references t Auth, dev tokens, submitting runs, and pointing the CLI at your deployment. - Full `settings.toml` reference — TLS, auth methods, concurrency, and more. + Full `settings.toml` reference — reverse-proxy TLS, auth methods, concurrency, and more. diff --git a/docs/administration/deploy-server.mdx b/docs/administration/deploy-server.mdx index 67e9d13dc..d243e9f77 100644 --- a/docs/administration/deploy-server.mdx +++ b/docs/administration/deploy-server.mdx @@ -22,7 +22,7 @@ Both interfaces use the same workflow engine, the same Graphviz files, and the s | **Events** | Printed to stderr | Streamed via SSE | | **Persistence** | Checkpoint files only | Persistent run store + checkpoint files | | **Web UI** | Not available | Full React interface | -| **Authentication** | None | JWT and/or mTLS | +| **Authentication** | None | Dev token and/or GitHub OAuth | ## Starting the server @@ -118,25 +118,26 @@ Send the `X-Fabro-Demo: 1` header on any API request to get static mock data wit The CLI can target a running Fabro server for commands that support a remote API. Configure `~/.fabro/settings.toml`: ```toml title="settings.toml" -[server] -target = "https://fabro.example.com:3000/api/v1" +[cli.target] +type = "http" +url = "https://fabro.example.com/api/v1" ``` Or use the `--server` flag: ```bash -fabro model list --server https://fabro.example.com:3000/api/v1 +fabro model list --server https://fabro.example.com/api/v1 ``` -`fabro model list` and `fabro model test` honor `[server].target` by default unless you explicitly pass `--storage-dir`. `fabro exec` remains a local agent session and only uses the server when you pass `--server`. +`fabro model list` and `fabro model test` honor `[cli.target]` by default unless you explicitly pass `--storage-dir`. `fabro exec` remains a local agent session and only uses the server when you pass `--server`. -See [User Configuration](/reference/user-configuration#server-section) for the full connection options, including mTLS setup. +See [User Configuration](/reference/user-configuration#cli.target-section) for the full connection options, including client certificates for proxy-terminated HTTPS endpoints. ## Next steps - Full settings.toml reference — authentication, TLS, run defaults, and more. + Full settings.toml reference — authentication, reverse-proxy TLS, run defaults, and more. Step-by-step guide for deploying Fabro on Railway. diff --git a/docs/administration/security.mdx b/docs/administration/security.mdx index 24c574921..2a3feec24 100644 --- a/docs/administration/security.mdx +++ b/docs/administration/security.mdx @@ -28,10 +28,10 @@ Fabro is single-tenant software designed for small, trusted teams. The following ### Authentication -- **Enable authentication.** Fabro supports GitHub OAuth and Tailscale header-based auth for the web app. Do not use `insecure_disabled` outside of local development. -- **Configure a username allowlist.** Both GitHub and Tailscale auth support `allowed_usernames` in `settings.toml`. An empty allowlist rejects all requests. -- **Use JWT to connect the web app to the API.** Configure `FABRO_JWT_PRIVATE_KEY` on the web app and `FABRO_JWT_PUBLIC_KEY` on the API server. JWT tokens are Ed25519-signed and short-lived (30 seconds). -- **Use mTLS for machine-to-machine API access.** Configure `[api.tls]` in `settings.toml` with server cert, key, and CA. Set client auth to `Required` for programmatic clients (CI, scripts). +- **Enable authentication.** Fabro supports `dev-token` and GitHub OAuth. Do not disable auth outside of local development or controlled demos. +- **Configure a username allowlist for GitHub OAuth.** `[server.auth.github].allowed_usernames` should contain the exact GitHub users allowed to log in. An empty list rejects everyone. +- **Configure the session secret used by the web flow.** `SESSION_SECRET` should be provisioned with a strong value on long-lived deployments. If you also provision `FABRO_JWT_PRIVATE_KEY` and `FABRO_JWT_PUBLIC_KEY`, treat them as server runtime secrets, but they are not what currently gates browser auth. +- **Terminate HTTPS or mTLS upstream when needed.** Fabro's listener is plain HTTP/Unix only. If CI, scripts, or a browser must connect over HTTPS, terminate TLS at a reverse proxy or load balancer and keep the Fabro listener on a private network. ### Secrets diff --git a/docs/administration/server-configuration.mdx b/docs/administration/server-configuration.mdx index 358cf6b86..30fbe39e9 100644 --- a/docs/administration/server-configuration.mdx +++ b/docs/administration/server-configuration.mdx @@ -22,6 +22,8 @@ Legacy `server.toml`, `user.toml`, and `cli.toml` are ignored with a warning. Re The CLI-only `[cli.*]` sections (including `[cli.target]`) belong in the client machine's `settings.toml`. They tell CLI commands how to reach a server. The server process does not read `[cli.*]` for its own binding or routing. +Fabro does not terminate inbound TLS directly. Bind `[server.listen]` to a Unix socket or plain TCP port, and terminate HTTPS or mTLS at a reverse proxy, load balancer, or platform ingress in front of Fabro. Use `[server.api].url` and `[server.web].url` for those external HTTPS URLs. + ### Full reference ```toml title="settings.toml" @@ -31,10 +33,6 @@ _version = 1 type = "tcp" address = "0.0.0.0:3000" -[server.listen.tls] -cert = "/etc/fabro/tls/cert.pem" -key = "/etc/fabro/tls/key.pem" - [server.api] url = "https://fabro.example.com/api/v1" diff --git a/docs/administration/troubleshooting.mdx b/docs/administration/troubleshooting.mdx index 90d34559d..89142fc64 100644 --- a/docs/administration/troubleshooting.mdx +++ b/docs/administration/troubleshooting.mdx @@ -10,7 +10,7 @@ The `fabro doctor` command validates your installation: ```bash fabro doctor # Local config checks + live server diagnostics fabro doctor --verbose # Show detailed output for each check -fabro doctor --server https://fabro.example.com:3000/api/v1 +fabro doctor --server https://fabro.example.com/api/v1 ``` It checks: @@ -29,7 +29,7 @@ It checks: **Port already in use** — Change the port with `fabro server start --port 3001` or stop the conflicting process. -**SSE streams disconnecting** — If using a reverse proxy, ensure buffering is disabled and the connection timeout is long enough for workflow runs. See the [reverse proxy example](/administration/deployment#binding-and-tls). +**SSE streams disconnecting** — If using a reverse proxy, ensure buffering is disabled and the connection timeout is long enough for workflow runs. See the [DigitalOcean reverse-proxy example](/administration/deploy-digital-ocean). **Run config validation errors** — Use `fabro preflight` to validate without executing: diff --git a/docs/api-reference/demo-mode.mdx b/docs/api-reference/demo-mode.mdx index 0af074560..380f0414b 100644 --- a/docs/api-reference/demo-mode.mdx +++ b/docs/api-reference/demo-mode.mdx @@ -13,7 +13,7 @@ Demo mode is activated **per-request** by sending a header: X-Fabro-Demo: 1 ``` -When the server receives this header, it routes the request to a parallel set of demo handlers that return static JSON instead of hitting the real backend. Authentication is bypassed — no JWT or mTLS credentials are needed. +When the server receives this header, it routes the request to a parallel set of demo handlers that return static JSON instead of hitting the real backend. Authentication is bypassed — no credentials are needed. Requests **without** the header are routed to the real API as usual, so demo and production traffic coexist on the same server. diff --git a/docs/api-reference/overview.mdx b/docs/api-reference/overview.mdx index 7c8a4e359..bbb30e5e1 100644 --- a/docs/api-reference/overview.mdx +++ b/docs/api-reference/overview.mdx @@ -11,13 +11,13 @@ The Fabro API is a REST API for managing workflow runs, interactive sessions, an ## Base URL -The versioned API is served by `fabro server start`, which defaults to: +By default, `fabro server start` listens on the Unix socket `~/.fabro/fabro.sock`. If you bind Fabro to TCP instead, the versioned API is served at a URL like: ``` http://localhost:3000/api/v1 ``` -The base URL is configurable via `settings.toml`: +The advertised public API URL is configurable via `settings.toml`: ```toml title="settings.toml" [server.api] @@ -63,19 +63,9 @@ When `"github"` is enabled, browser users can sign in through GitHub OAuth. Succ When `[server.web].enabled = true`, the server requires `SESSION_SECRET` and issues a private `__fabro_session` cookie after successful login. The cookie is session transport only; the underlying bootstrap method remains `dev-token` or `github`, and that provenance is preserved in run metadata. -### HTTPS +### HTTPS and Reverse Proxies -If you want HTTPS on the listener, configure shared TLS on `[server.listen.tls]`: - -```toml title="settings.toml" -[server.listen] -type = "tcp" -address = "0.0.0.0:3000" - -[server.listen.tls] -cert = "/etc/fabro/tls/cert.pem" -key = "/etc/fabro/tls/key.pem" -``` +Fabro's listener is plain HTTP (or a Unix socket) only. If you want a public HTTPS endpoint, terminate TLS at a reverse proxy, load balancer, or platform ingress and point it at Fabro's internal listener. ## Errors diff --git a/docs/reference/architecture.mdx b/docs/reference/architecture.mdx index df9e90456..ff8015e71 100644 --- a/docs/reference/architecture.mdx +++ b/docs/reference/architecture.mdx @@ -28,7 +28,7 @@ CLI mode is ideal for: fabro server start ``` -`fabro server start` starts an HTTP server (default `127.0.0.1:3000`) with persistent run storage. Runs are submitted via the REST API and executed asynchronously. +`fabro server start` starts an HTTP server, binding to `~/.fabro/fabro.sock` by default (or plain TCP when configured), with persistent run storage. Runs are submitted via the REST API and executed asynchronously. Public HTTPS, when needed, is terminated upstream by a reverse proxy or platform ingress. ### Configuration @@ -38,11 +38,11 @@ Key server config options: | Setting | Description | |---|---| -| `api.host` / `api.port` | Bind address (default `127.0.0.1:3000`) | -| `api.authentication_strategies` | Auth methods: `jwt`, `mtls`, or both | -| `api.tls` | Optional HTTPS with cert/key/CA paths | -| `max_concurrent_runs` | Scheduler concurrency limit (default 5) | -| `[llm]`, `[sandbox]`, `[vars]` | Defaults applied to every run (overridable per-run) | +| `server.listen` | Bind transport: Unix socket or plain TCP listener | +| `server.api.url` / `server.web.url` | External HTTPS URLs advertised to clients | +| `server.auth.methods` | Bootstrap auth methods: `dev-token`, `github`, or both | +| `server.scheduler.max_concurrent_runs` | Scheduler concurrency limit (default 5) | +| `[run.*]` | Defaults applied to every run (overridable per-run) | ### Run lifecycle @@ -61,10 +61,10 @@ In API mode, human-in-the-loop questions are served over HTTP instead of termina ### Authentication -API mode supports two authentication strategies, configurable in `settings.toml`: +API mode supports two bootstrap auth methods, configurable in `settings.toml`: -- **JWT** — EdDSA-signed tokens (used by the web UI) -- **mTLS** — Mutual TLS with client certificates (used for service-to-service communication) +- **`dev-token`** — Bearer token access for operators and automation +- **`github`** — GitHub OAuth for browser users, resulting in a session cookie ### Demo mode @@ -75,7 +75,7 @@ Demo mode is per-request: send the `X-Fabro-Demo: 1` HTTP header to get static m The web UI is a React app (`apps/fabro-web`) that connects to the API server. Start it alongside `fabro server start`: ```bash -fabro server start # API on port 3000 +fabro server start # API on ~/.fabro/fabro.sock by default cd apps/fabro-web && bun run dev # rebuilds web assets on change; refresh the browser ``` diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 417a8ac9e..79da428d5 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -41,7 +41,7 @@ name = "claude-sonnet-4-5" [cli.target] type = "http" -url = "https://fabro.example.com:3000/api/v1" +url = "https://fabro.example.com/api/v1" ``` `[cli.exec]` config applies to `fabro exec`. `[run.model]` sets the default workflow model/provider for commands like `fabro run` and `fabro preflight`. `[cli.target]` stores connection info for commands that can target a remote Fabro server. @@ -767,7 +767,7 @@ Check environment and integration health. `fabro doctor` always performs live se ```bash fabro doctor fabro doctor -v -fabro doctor --server https://fabro.example.com:3000/api/v1 +fabro doctor --server https://fabro.example.com/api/v1 ``` | Flag | Description | diff --git a/docs/reference/user-configuration.mdx b/docs/reference/user-configuration.mdx index 56c4f913d..c9c29f9bf 100644 --- a/docs/reference/user-configuration.mdx +++ b/docs/reference/user-configuration.mdx @@ -61,7 +61,7 @@ _version = 1 [cli.target] type = "http" -url = "https://fabro.example.com:3000/api/v1" +url = "https://fabro.example.com/api/v1" [cli.target.tls] cert = "~/.fabro/tls/client.crt" @@ -245,7 +245,7 @@ Connection info for commands that target a remote Fabro server. ```toml title="settings.toml" [cli.target] type = "http" -url = "https://fabro.example.com:3000/api/v1" +url = "https://fabro.example.com/api/v1" ``` | Key | Description | @@ -257,14 +257,14 @@ url = "https://fabro.example.com:3000/api/v1" `fabro model` uses `[cli.target]` by default when no explicit `--storage-dir` is passed. An explicit `--server` flag overrides the configured target: ```bash -fabro model list --server https://fabro.example.com:3000/api/v1 +fabro model list --server https://fabro.example.com/api/v1 ``` `fabro exec` does not automatically use `[cli.target]`. It only routes model traffic through a Fabro server when you pass `--server` for that invocation. ### `[cli.target.tls]` section -Optional mTLS configuration for authenticating with an HTTP target. When present, the CLI presents a client certificate during the TLS handshake. +Optional client-certificate configuration for authenticating with an HTTP target. When present, the CLI presents a client certificate during the TLS handshake with your external HTTPS endpoint or reverse proxy. ```toml title="settings.toml" [cli.target.tls] diff --git a/lib/crates/fabro-config/src/resolve/server.rs b/lib/crates/fabro-config/src/resolve/server.rs index 5d4f5a439..6716440fc 100644 --- a/lib/crates/fabro-config/src/resolve/server.rs +++ b/lib/crates/fabro-config/src/resolve/server.rs @@ -7,10 +7,9 @@ use fabro_types::settings::server::{ ServerAuthMethod, ServerAuthSettings, ServerIntegrationsLayer, ServerIntegrationsSettings, ServerIpAllowlistLayer, ServerIpAllowlistOverrideLayer, ServerIpAllowlistOverrideSettings, ServerIpAllowlistSettings, ServerLayer, ServerListenLayer, ServerListenSettings, - ServerListenTlsLayer, ServerLoggingSettings, ServerSchedulerSettings, ServerSettings, - ServerSlateDbLayer, ServerSlateDbSettings, ServerStorageLayer, ServerStorageSettings, - ServerWebLayer, ServerWebSettings, SlackIntegrationSettings, TeamsIntegrationSettings, - TlsConfig, + ServerLoggingSettings, ServerSchedulerSettings, ServerSettings, ServerSlateDbLayer, + ServerSlateDbSettings, ServerStorageLayer, ServerStorageSettings, ServerWebLayer, + ServerWebSettings, SlackIntegrationSettings, TeamsIntegrationSettings, }; use fabro_util::Home; @@ -18,7 +17,7 @@ use super::{ResolveError, default_interp, parse_socket_addr, require_interp}; pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> ServerSettings { let storage = resolve_storage(layer.storage.as_ref()); - let (listen, _valid_tls) = resolve_listen(layer.listen.as_ref(), errors); + let listen = resolve_listen(layer.listen.as_ref(), errors); let web = resolve_web(layer.api.as_ref(), layer.web.as_ref()); let auth = resolve_auth(layer.auth.as_ref(), errors); let ip_allowlist = resolve_ip_allowlist(layer.ip_allowlist.as_ref(), errors); @@ -65,49 +64,27 @@ fn resolve_storage(layer: Option<&ServerStorageLayer>) -> ServerStorageSettings fn resolve_listen( layer: Option<&ServerListenLayer>, errors: &mut Vec, -) -> (ServerListenSettings, bool) { +) -> ServerListenSettings { match layer { - None => ( - ServerListenSettings::Unix { - path: default_interp(Home::from_env().socket_path()), - }, - false, - ), - Some(ServerListenLayer::Unix { path }) => ( - ServerListenSettings::Unix { - path: path - .clone() - .unwrap_or_else(|| default_interp(Home::from_env().socket_path())), - }, - false, - ), - Some(ServerListenLayer::Tcp { address, tls }) => { + None => ServerListenSettings::Unix { + path: default_interp(Home::from_env().socket_path()), + }, + Some(ServerListenLayer::Unix { path }) => ServerListenSettings::Unix { + path: path + .clone() + .unwrap_or_else(|| default_interp(Home::from_env().socket_path())), + }, + Some(ServerListenLayer::Tcp { address }) => { let address = parse_socket_addr( &require_interp(address.as_ref(), "server.listen.address", errors), "server.listen.address", errors, ); - let (tls, valid_tls) = resolve_tls(tls.as_ref(), errors); - (ServerListenSettings::Tcp { address, tls }, valid_tls) + ServerListenSettings::Tcp { address } } } } -fn resolve_tls( - layer: Option<&ServerListenTlsLayer>, - errors: &mut Vec, -) -> (Option, bool) { - let Some(layer) = layer else { - return (None, false); - }; - - let cert = require_interp(layer.cert.as_ref(), "server.listen.tls.cert", errors); - let key = require_interp(layer.key.as_ref(), "server.listen.tls.key", errors); - let valid = layer.cert.is_some() && layer.key.is_some(); - - (Some(TlsConfig { cert, key }), valid) -} - fn resolve_web(_api: Option<&ServerApiLayer>, layer: Option<&ServerWebLayer>) -> ServerWebSettings { let layer = layer.expect("defaults.toml should provide server.web defaults"); diff --git a/lib/crates/fabro-config/tests/resolve_root.rs b/lib/crates/fabro-config/tests/resolve_root.rs index a613265a3..0048eb4f3 100644 --- a/lib/crates/fabro-config/tests/resolve_root.rs +++ b/lib/crates/fabro-config/tests/resolve_root.rs @@ -27,10 +27,7 @@ _version = 1 [server.listen] type = "tcp" -address = "127.0.0.1:3000" - -[server.listen.tls] -cert = "/tmp/server.pem" +address = "not-a-socket-addr" [server.auth] methods = ["github"] @@ -50,7 +47,7 @@ provider = "not-a-provider" .collect::>() .join("\n"); - assert!(rendered.contains("server.listen.tls.key")); + assert!(rendered.contains("server.listen.address")); assert!(rendered.contains("server.auth.github.allowed_usernames")); assert!(rendered.contains("run.sandbox.provider")); } diff --git a/lib/crates/fabro-config/tests/resolve_server.rs b/lib/crates/fabro-config/tests/resolve_server.rs index 673cd3a25..811d8bb63 100644 --- a/lib/crates/fabro-config/tests/resolve_server.rs +++ b/lib/crates/fabro-config/tests/resolve_server.rs @@ -65,8 +65,8 @@ fn resolves_server_defaults_from_empty_settings() { } #[test] -fn reports_tls_shape_errors() { - let file = parse( +fn parsing_rejects_inbound_listener_tls_configuration() { + let err = fabro_config::parse_settings_layer( r#" _version = 1 @@ -76,19 +76,11 @@ address = "127.0.0.1:32276" [server.listen.tls] cert = "/etc/fabro/server.pem" - "#, - ); + ) + .expect_err("listener TLS should be rejected at parse time"); - let errors = fabro_config::resolve_server_from_file(&file) - .expect_err("incomplete tls config should fail"); - let rendered = errors - .iter() - .map(ToString::to_string) - .collect::>() - .join("\n"); - - assert!(rendered.contains("server.listen.tls.key")); + assert!(err.to_string().contains("unknown field `tls`")); } #[test] diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index c5d10f4a7..cd00f2832 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -47,14 +47,6 @@ tokio-stream = { workspace = true, features = ["sync"] } base64.workspace = true jsonwebtoken.workspace = true tokio.workspace = true -tokio-rustls = "0.26" -rustls = { version = "0.23", default-features = false, features = ["std", "ring"] } -rustls-pemfile = "2" -rustls-pki-types = "1" -hyper = "1" -hyper-util = { version = "0.1", features = ["tokio", "server-auto", "http1", "http2"] } -tower-service = "0.3" -x509-parser = "0.16" serde.workspace = true serde_json.workspace = true serde_yaml = "0.9" diff --git a/lib/crates/fabro-server/src/diagnostics.rs b/lib/crates/fabro-server/src/diagnostics.rs index 4682119ec..6123adc5c 100644 --- a/lib/crates/fabro-server/src/diagnostics.rs +++ b/lib/crates/fabro-server/src/diagnostics.rs @@ -1,4 +1,3 @@ -use std::path::PathBuf; use std::time::Duration; use base64::Engine as _; @@ -47,37 +46,6 @@ fn decode_pem_value(name: &str, value: &str) -> Result { String::from_utf8(bytes).map_err(|e| format!("{name} base64 decoded to invalid UTF-8: {e}")) } -fn validate_tls_cert(pem: &str, now_epoch: i64) -> Result { - let mut reader = std::io::Cursor::new(pem.as_bytes()); - let certs: Vec<_> = rustls_pemfile::certs(&mut reader) - .collect::, _>>() - .map_err(|e| format!("failed to parse certificate PEM: {e}"))?; - if certs.is_empty() { - return Err("no certificates found in PEM".to_string()); - } - let (_, parsed) = x509_parser::parse_x509_certificate(&certs[0]) - .map_err(|e| format!("failed to parse X.509 certificate: {e}"))?; - let not_after = parsed.validity().not_after.timestamp(); - if not_after <= now_epoch { - return Err("certificate has expired".to_string()); - } - let cn = parsed - .subject() - .iter_common_name() - .next() - .and_then(|cn| cn.as_str().ok()) - .unwrap_or("(no CN)"); - Ok(format!("CN={cn}, valid")) -} - -fn validate_tls_private_key(pem: &str) -> Result<(), String> { - let mut reader = std::io::Cursor::new(pem.as_bytes()); - rustls_pemfile::private_key(&mut reader) - .map_err(|e| format!("failed to parse private key PEM: {e}"))? - .ok_or_else(|| "no private key found in PEM".to_string())?; - Ok(()) -} - fn validate_session_secret(value: &str) -> Result<(), String> { session_secret::validate_session_secret(value) } @@ -492,11 +460,6 @@ async fn check_brave_search(state: &AppState) -> CheckResult { } fn check_crypto(state: &AppState) -> CheckResult { - let settings_file = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); let resolved_server_settings = state.server_settings(); let mut details = Vec::new(); @@ -559,40 +522,6 @@ fn check_crypto(state: &AppState) -> CheckResult { } } - if let Some(listen) = settings_file - .server - .as_ref() - .and_then(|s| s.listen.as_ref()) - { - use fabro_types::settings::server::ServerListenLayer; - - if let ServerListenLayer::Tcp { tls: Some(tls), .. } = listen { - let read = |raw: Option, label: &str| -> Result { - let Some(path_str) = raw else { - return Err(format!("server.listen.tls.{label} is not configured")); - }; - let path = PathBuf::from(&path_str); - let expanded = fabro_config::expand_tilde(&path); - std::fs::read_to_string(&expanded) - .map_err(|e| format!("{}: {e}", expanded.display())) - }; - match ( - read(tls.cert.as_ref().map(InterpString::as_source), "cert"), - read(tls.key.as_ref().map(InterpString::as_source), "key"), - ) { - (Ok(cert_pem), Ok(key_pem)) => { - if let Err(err) = validate_tls_cert(&cert_pem, chrono::Utc::now().timestamp()) { - errors.push(err); - } - if let Err(err) = validate_tls_private_key(&key_pem) { - errors.push(err); - } - } - _ => errors.push("failed to read TLS files".to_string()), - } - } - } - if errors.is_empty() { CheckResult { name: "Crypto".to_string(), diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index a1547dc86..b13afd51a 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -19,7 +19,6 @@ pub mod server; mod server_secrets; mod settings_view; pub mod static_files; -pub mod tls; pub mod web_auth; pub use error::{ApiError, Error, Result}; diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 49917ce95..363cb6e2e 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -33,7 +33,6 @@ use crate::server::{ reconcile_incomplete_runs_on_startup, shutdown_active_workers, spawn_scheduler, }; use crate::server_secrets::ServerSecrets; -use crate::tls::{build_rustls_config, serve_tls_with_shutdown}; const TEST_IN_MEMORY_STORE_ENV: &str = "FABRO_TEST_IN_MEMORY_STORE"; pub const DEFAULT_TCP_PORT: u16 = 32276; @@ -246,7 +245,6 @@ fn bind_override_layer(bind: BindRequest) -> SettingsLayer { }, BindRequest::Tcp(address) => ServerListenLayer::Tcp { address: Some(InterpString::parse(&address.to_string())), - tls: None, }, BindRequest::TcpHost(_) => { unreachable!("host-only bind requests are handled before building a settings override") @@ -515,12 +513,6 @@ where } }); - // Branch: TLS, plain TCP, or Unix socket - let tls_settings = match &resolved_server_settings.listen { - ServerListenSettings::Tcp { tls, .. } => tls.clone(), - ServerListenSettings::Unix { .. } => None, - }; - let bound_listener = bind_listener(&bind_request).await?; let bind_addr = bound_listener.bind.clone(); if bound_listener.used_random_port_fallback { @@ -563,38 +555,19 @@ where match bound_listener.listener { BoundListener::Unix(listener) => { - if tls_settings.is_some() { - warn!("TLS is configured but not supported on Unix sockets; ignoring TLS settings"); - } announce_server_ready(&bind_addr, styles); axum::serve(listener, router) .with_graceful_shutdown(wait_for_shutdown(shutdown_rx.clone())) .await?; } BoundListener::Tcp(listener) => { - if let Some(ref tls_settings) = tls_settings { - let rustls_config = build_rustls_config(tls_settings)?; - let tls_acceptor = tokio_rustls::TlsAcceptor::from(rustls_config); - - info!("TLS enabled"); - announce_server_ready(&bind_addr, styles); - - serve_tls_with_shutdown( - listener, - tls_acceptor, - router, - wait_for_shutdown(shutdown_rx.clone()), - ) - .await?; - } else { - announce_server_ready(&bind_addr, styles); - axum::serve( - listener, - router.into_make_service_with_connect_info::(), - ) - .with_graceful_shutdown(wait_for_shutdown(shutdown_rx.clone())) - .await?; - } + announce_server_ready(&bind_addr, styles); + axum::serve( + listener, + router.into_make_service_with_connect_info::(), + ) + .with_graceful_shutdown(wait_for_shutdown(shutdown_rx.clone())) + .await?; } } diff --git a/lib/crates/fabro-server/src/settings_view.rs b/lib/crates/fabro-server/src/settings_view.rs index a91a5ea30..f620891e1 100644 --- a/lib/crates/fabro-server/src/settings_view.rs +++ b/lib/crates/fabro-server/src/settings_view.rs @@ -10,9 +10,8 @@ //! //! Per the requirements doc, only the transport bind needs redaction now: //! -//! - `server.listen` — the whole subtree. Bind address reveals network -//! topology; `[server.listen.tls]` cert/key paths reveal the host filesystem -//! layout. +//! - `server.listen` — the whole subtree. Bind addresses and socket paths +//! reveal network topology and host filesystem layout. //! //! ## Why that's all //! @@ -132,10 +131,6 @@ _version = 1 [server.listen] type = "tcp" address = "127.0.0.1:32276" - -[server.listen.tls] -cert = "/etc/fabro/tls/cert.pem" -key = "/etc/fabro/tls/key.pem" "#, ); let redacted = redact_for_api(&settings); @@ -249,10 +244,6 @@ _version = 1 type = "tcp" address = "127.0.0.1:32276" -[server.listen.tls] -cert = "/etc/fabro/tls/cert.pem" -key = "/etc/fabro/tls/key.pem" - [server.auth] methods = ["github", "dev-token"] diff --git a/lib/crates/fabro-server/src/tls.rs b/lib/crates/fabro-server/src/tls.rs deleted file mode 100644 index 07da9e745..000000000 --- a/lib/crates/fabro-server/src/tls.rs +++ /dev/null @@ -1,121 +0,0 @@ -use std::future::Future; -use std::path::Path; -use std::pin::Pin; -use std::sync::Arc; - -use anyhow::Context; -use axum::extract::ConnectInfo; -use fabro_types::settings::{InterpString, TlsConfig}; -use rustls::ServerConfig; -use rustls_pki_types::{CertificateDer, PrivateKeyDer}; -use tokio::net::TcpListener; -use tracing::error; - -/// Build a rustls `ServerConfig` from the `[server.listen.tls]` configuration. -pub fn build_rustls_config(tls_settings: &TlsConfig) -> anyhow::Result> { - let cert = resolve_path(&tls_settings.cert)?; - let key_path = resolve_path(&tls_settings.key)?; - - let certs = load_certs(&cert); - let key = load_private_key(&key_path); - - let config = ServerConfig::builder() - .with_no_client_auth() - .with_single_cert(certs, key) - .expect("invalid server certificate or key"); - - Ok(Arc::new(config)) -} - -pub async fn serve_tls( - listener: TcpListener, - tls_acceptor: tokio_rustls::TlsAcceptor, - router: axum::Router, -) -> anyhow::Result<()> { - serve_tls_with_shutdown(listener, tls_acceptor, router, std::future::pending()).await -} - -/// Serve requests over TLS until the supplied shutdown future resolves. -pub async fn serve_tls_with_shutdown( - listener: TcpListener, - tls_acceptor: tokio_rustls::TlsAcceptor, - router: axum::Router, - shutdown: F, -) -> anyhow::Result<()> -where - F: Future + Send, -{ - use hyper::body::Incoming; - use hyper::service::service_fn; - use hyper_util::rt::{TokioExecutor, TokioIo}; - use hyper_util::server::conn::auto::Builder; - use tower_service::Service; - - let builder = Builder::new(TokioExecutor::new()); - let mut shutdown = Pin::from(Box::new(shutdown)); - - loop { - let accepted = tokio::select! { - () = &mut shutdown => return Ok(()), - accepted = listener.accept() => accepted?, - }; - let (tcp_stream, remote_addr) = accepted; - - let tls_acceptor = tls_acceptor.clone(); - let router = router.clone(); - let builder = builder.clone(); - - tokio::spawn(async move { - let tls_stream = match tls_acceptor.accept(tcp_stream).await { - Ok(s) => s, - Err(e) => { - error!(%remote_addr, "TLS handshake failed: {e}"); - return; - } - }; - - let io = TokioIo::new(tls_stream); - - let service = service_fn(move |mut req: hyper::Request| { - let mut router = router.clone(); - async move { - req.extensions_mut().insert(ConnectInfo(remote_addr)); - router.call(req).await - } - }); - - if let Err(e) = builder.serve_connection(io, service).await { - error!(%remote_addr, "connection error: {e}"); - } - }); - } -} - -pub use fabro_config::expand_tilde; - -fn resolve_path(value: &InterpString) -> anyhow::Result { - let resolved = value - .resolve(|name| std::env::var(name).ok()) - .with_context(|| format!("failed to resolve {}", value.as_source()))?; - Ok(expand_tilde(Path::new(&resolved.value))) -} - -fn load_certs(path: &Path) -> Vec> { - let path = expand_tilde(path); - let file = std::fs::File::open(&path) - .unwrap_or_else(|e| panic!("failed to open certificate file {}: {e}", path.display())); - let mut reader = std::io::BufReader::new(file); - rustls_pemfile::certs(&mut reader) - .collect::, _>>() - .unwrap_or_else(|e| panic!("failed to parse certificates from {}: {e}", path.display())) -} - -fn load_private_key(path: &Path) -> PrivateKeyDer<'static> { - let path = expand_tilde(path); - let file = std::fs::File::open(&path) - .unwrap_or_else(|e| panic!("failed to open private key file {}: {e}", path.display())); - let mut reader = std::io::BufReader::new(file); - rustls_pemfile::private_key(&mut reader) - .unwrap_or_else(|e| panic!("failed to parse private key from {}: {e}", path.display())) - .unwrap_or_else(|| panic!("no private key found in {}", path.display())) -} diff --git a/lib/crates/fabro-server/tests/fixtures/mtls/ca.crt b/lib/crates/fabro-server/tests/fixtures/mtls/ca.crt deleted file mode 100644 index 25f16654c..000000000 --- a/lib/crates/fabro-server/tests/fixtures/mtls/ca.crt +++ /dev/null @@ -1,9 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIBRjCB+aADAgECAhRKD83+hLEUl2GQUrsSgNaDBjPrpDAFBgMrZXAwETEPMA0G -A1UEAwwGVGVzdENBMB4XDTI2MDQwNTE2MjEzM1oXDTM2MDQwMjE2MjEzM1owETEP -MA0GA1UEAwwGVGVzdENBMCowBQYDK2VwAyEA3vVnIRyxAa9q+qtf0OPWoOUKff1D -Pq5LpXPUTh1nrJejYzBhMB0GA1UdDgQWBBT88UyTLCWai4vJtkS5K0zutivZOTAf -BgNVHSMEGDAWgBT88UyTLCWai4vJtkS5K0zutivZOTAPBgNVHRMBAf8EBTADAQH/ -MA4GA1UdDwEB/wQEAwIBBjAFBgMrZXADQQBUmXc96ILueacLnf7kSJS35wiCl044 -Js8vwgQuTkJ9SDhuCOt88E4b9vZMhx2kOBLiwTyTdOILhVECPE9FZicD ------END CERTIFICATE----- diff --git a/lib/crates/fabro-server/tests/fixtures/mtls/client.crt b/lib/crates/fabro-server/tests/fixtures/mtls/client.crt deleted file mode 100644 index 3f6dcd30f..000000000 --- a/lib/crates/fabro-server/tests/fixtures/mtls/client.crt +++ /dev/null @@ -1,9 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIBMjCB5aADAgECAhRjMLlP+97gUZFyv5k1WdriOASrLDAFBgMrZXAwETEPMA0G -A1UEAwwGVGVzdENBMB4XDTI2MDQwNTE2MjEzM1oXDTM2MDQwMjE2MjEzM1owEzER -MA8GA1UEAwwIdGVzdHVzZXIwKjAFBgMrZXADIQCYYub304Ilt7lkzkN5plpIlGCo -xR8wL18Xob7/hW2SWqNNMEswCQYDVR0TBAIwADAdBgNVHQ4EFgQUL4ve1GH0sFJ+ -33TwQP1R6oactHYwHwYDVR0jBBgwFoAU/PFMkywlmouLybZEuStM7rYr2TkwBQYD -K2VwA0EAEpBsV5kpyuEF3t5GzuxELDJgtVxGLpZD5PsPqj+wxv5j6TeOwCE/LRRV -JsKYJt3SMdqySx84dfscPD9c5HMPCw== ------END CERTIFICATE----- diff --git a/lib/crates/fabro-server/tests/fixtures/mtls/client.key b/lib/crates/fabro-server/tests/fixtures/mtls/client.key deleted file mode 100644 index ca97bb3ba..000000000 --- a/lib/crates/fabro-server/tests/fixtures/mtls/client.key +++ /dev/null @@ -1,3 +0,0 @@ ------BEGIN PRIVATE KEY----- -MC4CAQAwBQYDK2VwBCIEIME1COxOi67I+kdoIH+ms4c0zKA8D7M8SkeJyjC89+pj ------END PRIVATE KEY----- diff --git a/lib/crates/fabro-server/tests/fixtures/mtls/jwt-ed25519-private.pem b/lib/crates/fabro-server/tests/fixtures/mtls/jwt-ed25519-private.pem deleted file mode 100644 index b847be8a6..000000000 --- a/lib/crates/fabro-server/tests/fixtures/mtls/jwt-ed25519-private.pem +++ /dev/null @@ -1,3 +0,0 @@ ------BEGIN PRIVATE KEY----- -MC4CAQAwBQYDK2VwBCIEIMr+udNo63lm79G+2xETGqoQsMJUvpbTUFhXdgKNI10C ------END PRIVATE KEY----- diff --git a/lib/crates/fabro-server/tests/fixtures/mtls/jwt-ed25519-public.pem b/lib/crates/fabro-server/tests/fixtures/mtls/jwt-ed25519-public.pem deleted file mode 100644 index b1abb273e..000000000 --- a/lib/crates/fabro-server/tests/fixtures/mtls/jwt-ed25519-public.pem +++ /dev/null @@ -1,3 +0,0 @@ ------BEGIN PUBLIC KEY----- -MCowBQYDK2VwAyEA93ZZOd4zYtjwgdzSw+brqyWM9USG5INKCGWUEHRVRBw= ------END PUBLIC KEY----- diff --git a/lib/crates/fabro-server/tests/fixtures/mtls/server.crt b/lib/crates/fabro-server/tests/fixtures/mtls/server.crt deleted file mode 100644 index bca953a06..000000000 --- a/lib/crates/fabro-server/tests/fixtures/mtls/server.crt +++ /dev/null @@ -1,9 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIBOTCB7KADAgECAhRjMLlP+97gUZFyv5k1WdriOASrKzAFBgMrZXAwETEPMA0G -A1UEAwwGVGVzdENBMB4XDTI2MDQwNTE2MjEzM1oXDTM2MDQwMjE2MjEzM1owFDES -MBAGA1UEAwwJbG9jYWxob3N0MCowBQYDK2VwAyEAn8X6FEFjCq5MKfiSVNKjRY5p -TKdDrASo29olFWz8qy+jUzBRMA8GA1UdEQQIMAaHBH8AAAEwHQYDVR0OBBYEFL7n -tv01dMhzLJ0dzTo7tEAXrYjuMB8GA1UdIwQYMBaAFPzxTJMsJZqLi8m2RLkrTO62 -K9k5MAUGAytlcANBAGpMB98RKLprVHiagV1Myj08TK2Lz4+K+Hs2fhUVMgRP9JXV -vf8tC77XV/fH9wIKeaPvsupFO73AdD0BQZb8bQ0= ------END CERTIFICATE----- diff --git a/lib/crates/fabro-server/tests/fixtures/mtls/server.key b/lib/crates/fabro-server/tests/fixtures/mtls/server.key deleted file mode 100644 index 67ff8e49a..000000000 --- a/lib/crates/fabro-server/tests/fixtures/mtls/server.key +++ /dev/null @@ -1,3 +0,0 @@ ------BEGIN PRIVATE KEY----- -MC4CAQAwBQYDK2VwBCIEIAX0EXHZDH5uU2h5ctNqHfa9hMtO9tfoM1kCRL9JHGtA ------END PRIVATE KEY----- diff --git a/lib/crates/fabro-server/tests/fixtures/mtls/wrong-client.crt b/lib/crates/fabro-server/tests/fixtures/mtls/wrong-client.crt deleted file mode 100644 index 17b0ab0aa..000000000 --- a/lib/crates/fabro-server/tests/fixtures/mtls/wrong-client.crt +++ /dev/null @@ -1,9 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIBMzCB5qADAgECAhRT8XV12gL48jHEcgn7qTk7b5ocbzAFBgMrZXAwEjEQMA4G -A1UEAwwHV3JvbmdDQTAeFw0yNjA0MDUxNjIxMzNaFw0zNjA0MDIxNjIxMzNaMBMx -ETAPBgNVBAMMCGludHJ1ZGVyMCowBQYDK2VwAyEADrp6dr+UfNhzR6guiNU5ns0c -Y97Ari4gVZnh8DE1MB6jTTBLMAkGA1UdEwQCMAAwHQYDVR0OBBYEFIF2t8T34ktN -Y276k4702JltR0iEMB8GA1UdIwQYMBaAFNFUQIdydtMb4t9g8+0s9DHh0pYIMAUG -AytlcANBAFO7sA+Po2qFaTRSdpxuAQIbywHiF92uyombcfQkQPgbVbAA3oH9gh32 -4uG4c1OCE+w1AI1f2/EpC4zZRPZVAwI= ------END CERTIFICATE----- diff --git a/lib/crates/fabro-server/tests/fixtures/mtls/wrong-client.key b/lib/crates/fabro-server/tests/fixtures/mtls/wrong-client.key deleted file mode 100644 index 80c20f2da..000000000 --- a/lib/crates/fabro-server/tests/fixtures/mtls/wrong-client.key +++ /dev/null @@ -1,3 +0,0 @@ ------BEGIN PRIVATE KEY----- -MC4CAQAwBQYDK2VwBCIEIAU8EOnIZ26wKCqJ/WTcoCBHETbSYsILQ9zxddB92JVQ ------END PRIVATE KEY----- diff --git a/lib/crates/fabro-server/tests/it/api/docs.rs b/lib/crates/fabro-server/tests/it/api/docs.rs new file mode 100644 index 000000000..d43c6a6c4 --- /dev/null +++ b/lib/crates/fabro-server/tests/it/api/docs.rs @@ -0,0 +1,37 @@ +use std::path::PathBuf; + +fn read_doc(relative_path: &str) -> String { + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../../../") + .join(relative_path); + std::fs::read_to_string(&path) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", path.display())) +} + +#[test] +fn active_server_docs_describe_the_unix_socket_default() { + let architecture = read_doc("docs/reference/architecture.mdx"); + assert!( + architecture.contains("~/.fabro/fabro.sock"), + "architecture doc should mention the default Unix socket bind" + ); + + let api_overview = read_doc("docs/api-reference/overview.mdx"); + assert!( + api_overview.contains("~/.fabro/fabro.sock"), + "API overview should mention the default Unix socket bind" + ); +} + +#[test] +fn security_doc_does_not_require_jwt_keys_for_the_current_web_flow() { + let security = read_doc("docs/administration/security.mdx"); + assert!( + security.contains("SESSION_SECRET"), + "security doc should still mention the session secret" + ); + assert!( + !security.contains("`FABRO_JWT_PRIVATE_KEY`, `FABRO_JWT_PUBLIC_KEY`, and `SESSION_SECRET`"), + "security doc should not describe JWT keys as required for the current web flow" + ); +} diff --git a/lib/crates/fabro-server/tests/it/api/mod.rs b/lib/crates/fabro-server/tests/it/api/mod.rs index 26702579b..d924a389f 100644 --- a/lib/crates/fabro-server/tests/it/api/mod.rs +++ b/lib/crates/fabro-server/tests/it/api/mod.rs @@ -1,6 +1,6 @@ +mod docs; mod routing; mod runs; mod settings; mod system; -#[cfg(target_os = "linux")] -mod tls; +mod tcp; diff --git a/lib/crates/fabro-server/tests/it/api/runs.rs b/lib/crates/fabro-server/tests/it/api/runs.rs index 0097a3c2b..ff6edafba 100644 --- a/lib/crates/fabro-server/tests/it/api/runs.rs +++ b/lib/crates/fabro-server/tests/it/api/runs.rs @@ -21,10 +21,6 @@ _version = 1 type = "tcp" address = "127.0.0.1:32276" -[server.listen.tls] -cert = "/etc/fabro/tls/cert.pem" -key = "/etc/fabro/tls/key.pem" - [server.auth] methods = ["dev-token", "github"] diff --git a/lib/crates/fabro-server/tests/it/api/settings.rs b/lib/crates/fabro-server/tests/it/api/settings.rs index fd9525e3d..c82772f04 100644 --- a/lib/crates/fabro-server/tests/it/api/settings.rs +++ b/lib/crates/fabro-server/tests/it/api/settings.rs @@ -19,10 +19,6 @@ _version = 1 type = "tcp" address = "127.0.0.1:32276" -[server.listen.tls] -cert = "/etc/fabro/tls/cert.pem" -key = "/etc/fabro/tls/key.pem" - [server.storage] root = "/srv/fabro" diff --git a/lib/crates/fabro-server/tests/it/api/tcp.rs b/lib/crates/fabro-server/tests/it/api/tcp.rs new file mode 100644 index 000000000..a78e6eaa7 --- /dev/null +++ b/lib/crates/fabro-server/tests/it/api/tcp.rs @@ -0,0 +1,280 @@ +use std::net::SocketAddr; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::time::Duration; +#[cfg(unix)] +use std::time::{SystemTime, UNIX_EPOCH}; + +use fabro_server::bind::Bind; +use fabro_server::ip_allowlist::{IpAllowlist, IpAllowlistConfig}; +use fabro_server::jwt_auth::{AuthMode, ConfiguredAuth}; +use fabro_server::serve::{ServeArgs, serve_command}; +use fabro_server::server::{ + RouterOptions, build_router, build_router_with_options, create_app_state, +}; +use fabro_types::settings::ServerAuthMethod; +use fabro_util::terminal::Styles; +use tempfile::TempDir; +use tokio::net::TcpListener; +use tokio::task::JoinHandle; +use tokio::time::sleep; + +use crate::helpers::api; + +const TEST_DEV_TOKEN: &str = + "fabro_dev_abababababababababababababababababababababababababababababababab"; + +async fn start_tcp_server(auth_mode: AuthMode) -> SocketAddr { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + let state = create_app_state(); + let router = build_router(state, auth_mode); + + tokio::spawn(async move { + let _ = axum::serve( + listener, + router.into_make_service_with_connect_info::(), + ) + .await; + }); + + addr +} + +async fn start_tcp_server_with_allowlist( + auth_mode: AuthMode, + ip_allowlist: Arc, +) -> SocketAddr { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + let state = create_app_state(); + let router = + build_router_with_options(state, auth_mode, ip_allowlist, RouterOptions::default()); + + tokio::spawn(async move { + let _ = axum::serve( + listener, + router.into_make_service_with_connect_info::(), + ) + .await; + }); + + addr +} + +fn build_client() -> fabro_http::HttpClient { + fabro_http::test_http_client().unwrap() +} + +#[cfg(unix)] +fn build_unix_client(path: &Path) -> fabro_http::HttpClient { + fabro_http::HttpClientBuilder::new() + .unix_socket(path) + .no_proxy() + .build() + .unwrap() +} + +fn write_test_config(tempdir: &TempDir, settings: &str) -> PathBuf { + let config_path = tempdir.path().join("settings.toml"); + std::fs::write(&config_path, settings).unwrap(); + std::fs::write( + tempdir.path().join("server.env"), + format!("FABRO_DEV_TOKEN={TEST_DEV_TOKEN}\n"), + ) + .unwrap(); + config_path +} + +async fn spawn_served_listener( + settings: impl AsRef, +) -> (JoinHandle>, Bind, TempDir) { + let tempdir = tempfile::tempdir().unwrap(); + let config_path = write_test_config(&tempdir, settings.as_ref()); + let styles: &'static Styles = Box::leak(Box::new(Styles::new(false))); + let (tx, rx) = tokio::sync::oneshot::channel(); + let mut tx = Some(tx); + let storage_dir = tempdir.path().to_path_buf(); + + let handle = tokio::spawn(async move { + Box::pin(serve_command( + ServeArgs { + bind: None, + web: false, + no_web: true, + model: None, + provider: None, + sandbox: None, + max_concurrent_runs: None, + config: Some(config_path), + #[cfg(debug_assertions)] + watch_web: false, + }, + styles, + Some(storage_dir), + move |bind| { + let sender = tx.take().expect("server should only report readiness once"); + sender.send(bind.clone()).ok(); + Ok(()) + }, + )) + .await + }); + + let bind = rx.await.expect("server should report its bind address"); + (handle, bind, tempdir) +} + +async fn wait_for_tcp_health(addr: SocketAddr) { + let client = build_client(); + let url = format!("http://127.0.0.1:{}/health", addr.port()); + + for _ in 0..50 { + if let Ok(response) = client.get(&url).send().await { + if response.status() == 200 { + return; + } + } + sleep(Duration::from_millis(10)).await; + } + + panic!("timed out waiting for TCP health endpoint at {url}"); +} + +#[cfg(unix)] +async fn wait_for_unix_health(path: &Path) { + let client = build_unix_client(path); + + for _ in 0..50 { + if let Ok(response) = client.get("http://fabro/health").send().await { + if response.status() == 200 { + return; + } + } + sleep(Duration::from_millis(10)).await; + } + + panic!( + "timed out waiting for Unix socket health endpoint at {}", + path.display() + ); +} + +#[tokio::test] +async fn tcp_accepts_plain_http_requests() { + let (handle, bind, _tempdir) = spawn_served_listener( + r#" +_version = 1 + +[server.listen] +type = "tcp" +address = "127.0.0.1:0" + +[server.auth] +methods = ["dev-token"] +"#, + ) + .await; + let addr = match bind { + Bind::Tcp(addr) => addr, + Bind::Unix(path) => panic!("expected TCP bind, got unix socket at {}", path.display()), + }; + wait_for_tcp_health(addr).await; + + let client = build_client(); + + let response = client + .get(format!("http://127.0.0.1:{}{}", addr.port(), api("/runs"))) + .bearer_auth(TEST_DEV_TOKEN) + .send() + .await + .expect("plain HTTP request should succeed"); + + assert_eq!(response.status(), 200); + handle.abort(); +} + +#[tokio::test] +async fn tcp_dev_token_auth_uses_bearer_auth() { + let auth_mode = AuthMode::Enabled(ConfiguredAuth { + methods: vec![ServerAuthMethod::DevToken], + dev_token: Some(TEST_DEV_TOKEN.to_string()), + }); + let addr = start_tcp_server(auth_mode).await; + let client = build_client(); + let url = format!("http://127.0.0.1:{}{}", addr.port(), api("/runs")); + + let unauthorized = client.get(&url).send().await.unwrap(); + assert_eq!(unauthorized.status(), 401); + + let authorized = client + .get(url) + .bearer_auth(TEST_DEV_TOKEN) + .send() + .await + .unwrap(); + assert_eq!(authorized.status(), 200); +} + +#[cfg(unix)] +#[tokio::test] +async fn unix_socket_accepts_plain_http_requests() { + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let socket_path = std::env::temp_dir().join(format!("fabro-server-it-{unique}.sock")); + let (handle, bind, _tempdir) = spawn_served_listener(format!( + r#" +_version = 1 + +[server.listen] +type = "unix" +path = "{}" + +[server.auth] +methods = ["dev-token"] +"#, + socket_path.display() + )) + .await; + let path = match bind { + Bind::Unix(path) => path, + Bind::Tcp(addr) => panic!("expected Unix bind, got TCP address {addr}"), + }; + wait_for_unix_health(&path).await; + + let response = build_unix_client(&path) + .get(format!("http://fabro{}", api("/runs"))) + .bearer_auth(TEST_DEV_TOKEN) + .send() + .await + .expect("Unix-socket HTTP request should succeed"); + + assert_eq!(response.status(), 200); + handle.abort(); + std::fs::remove_file(&path).ok(); +} + +#[tokio::test] +async fn tcp_ip_allowlist_uses_connect_info() { + let addr = start_tcp_server_with_allowlist( + AuthMode::Disabled, + Arc::new(IpAllowlistConfig { + allowlist: IpAllowlist::new(vec!["10.0.0.0/8".parse().unwrap()]), + trusted_proxy_count: 0, + }), + ) + .await; + let client = build_client(); + + let response = client + .get(format!("http://127.0.0.1:{}{}", addr.port(), api("/runs"))) + .send() + .await + .unwrap(); + + assert_eq!(response.status(), 403); +} diff --git a/lib/crates/fabro-server/tests/it/api/tls.rs b/lib/crates/fabro-server/tests/it/api/tls.rs deleted file mode 100644 index 8b719eeef..000000000 --- a/lib/crates/fabro-server/tests/it/api/tls.rs +++ /dev/null @@ -1,155 +0,0 @@ -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use fabro_server::ip_allowlist::{IpAllowlist, IpAllowlistConfig}; -use fabro_server::jwt_auth::{AuthMode, ConfiguredAuth}; -use fabro_server::server::{ - RouterOptions, build_router, build_router_with_options, create_app_state, -}; -use fabro_server::tls::build_rustls_config; -use fabro_types::settings::{InterpString, ServerAuthMethod, TlsConfig}; -use tokio::net::TcpListener; - -use crate::helpers::api; - -fn fixture_path(name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures/mtls") - .join(name) -} - -struct PkiPaths { - ca_cert: PathBuf, - server_cert: PathBuf, - server_key: PathBuf, -} - -fn fixture_pki() -> PkiPaths { - PkiPaths { - ca_cert: fixture_path("ca.crt"), - server_cert: fixture_path("server.crt"), - server_key: fixture_path("server.key"), - } -} - -async fn start_tls_server(tls_settings: &TlsConfig, auth_mode: AuthMode) -> std::net::SocketAddr { - let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let addr = listener.local_addr().unwrap(); - - let rustls_config = build_rustls_config(tls_settings).unwrap(); - let tls_acceptor = tokio_rustls::TlsAcceptor::from(rustls_config); - - let state = create_app_state(); - let router = build_router(state, auth_mode); - - tokio::spawn(async move { - let _ = fabro_server::tls::serve_tls(listener, tls_acceptor, router).await; - }); - - addr -} - -async fn start_tls_server_with_allowlist( - tls_settings: &TlsConfig, - auth_mode: AuthMode, - ip_allowlist: Arc, -) -> std::net::SocketAddr { - let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let addr = listener.local_addr().unwrap(); - - let rustls_config = build_rustls_config(tls_settings).unwrap(); - let tls_acceptor = tokio_rustls::TlsAcceptor::from(rustls_config); - - let state = create_app_state(); - let router = - build_router_with_options(state, auth_mode, ip_allowlist, RouterOptions::default()); - - tokio::spawn(async move { - let _ = fabro_server::tls::serve_tls(listener, tls_acceptor, router).await; - }); - - addr -} - -fn build_client(ca_cert_path: &Path) -> fabro_http::HttpClient { - let ca_pem = std::fs::read(ca_cert_path).unwrap(); - let ca_cert = fabro_http::tls::Certificate::from_pem(&ca_pem).unwrap(); - - fabro_http::HttpClientBuilder::new() - .add_root_certificate(ca_cert) - .no_proxy() - .use_rustls_tls() - .build() - .unwrap() -} - -fn install_crypto_provider() { - let _ = rustls::crypto::ring::default_provider().install_default(); -} - -fn tls_settings(pki: &PkiPaths) -> TlsConfig { - TlsConfig { - cert: InterpString::parse(&pki.server_cert.to_string_lossy()), - key: InterpString::parse(&pki.server_key.to_string_lossy()), - } -} - -#[tokio::test] -async fn tls_accepts_requests_without_client_cert() { - install_crypto_provider(); - let pki = fixture_pki(); - let addr = start_tls_server(&tls_settings(&pki), AuthMode::Disabled).await; - let client = build_client(&pki.ca_cert); - - let response = client - .get(format!("https://127.0.0.1:{}{}", addr.port(), api("/runs"))) - .send() - .await - .expect("request over TLS should succeed without a client certificate"); - - assert_eq!(response.status(), 200); -} - -#[tokio::test] -async fn tls_dev_token_auth_does_not_require_client_cert() { - install_crypto_provider(); - let pki = fixture_pki(); - let dev_token = "fabro_dev_abababababababababababababababababababababababababababababababab"; - let auth_mode = AuthMode::Enabled(ConfiguredAuth { - methods: vec![ServerAuthMethod::DevToken], - dev_token: Some(dev_token.to_string()), - }); - let addr = start_tls_server(&tls_settings(&pki), auth_mode).await; - let client = build_client(&pki.ca_cert); - let url = format!("https://127.0.0.1:{}{}", addr.port(), api("/runs")); - - let unauthorized = client.get(&url).send().await.unwrap(); - assert_eq!(unauthorized.status(), 401); - - let authorized = client.get(url).bearer_auth(dev_token).send().await.unwrap(); - assert_eq!(authorized.status(), 200); -} - -#[tokio::test] -async fn tls_ip_allowlist_uses_connect_info() { - install_crypto_provider(); - let pki = fixture_pki(); - let addr = start_tls_server_with_allowlist( - &tls_settings(&pki), - AuthMode::Disabled, - Arc::new(IpAllowlistConfig { - allowlist: IpAllowlist::new(vec!["10.0.0.0/8".parse().unwrap()]), - trusted_proxy_count: 0, - }), - ) - .await; - let client = build_client(&pki.ca_cert); - - let response = client - .get(format!("https://127.0.0.1:{}{}", addr.port(), api("/runs"))) - .send() - .await - .unwrap(); - - assert_eq!(response.status(), 403); -} diff --git a/lib/crates/fabro-types/src/settings/mod.rs b/lib/crates/fabro-types/src/settings/mod.rs index 3f839aa72..5a034b8d9 100644 --- a/lib/crates/fabro-types/src/settings/mod.rs +++ b/lib/crates/fabro-types/src/settings/mod.rs @@ -52,7 +52,7 @@ pub use server::{ ServerIpAllowlistLayer, ServerIpAllowlistOverrideLayer, ServerIpAllowlistOverrideSettings, ServerIpAllowlistSettings, ServerLayer, ServerListenSettings, ServerLoggingSettings, ServerSchedulerSettings, ServerSettings, ServerSlateDbSettings, ServerStorageSettings, - ServerWebSettings, SlackIntegrationSettings, TeamsIntegrationSettings, TlsConfig, + ServerWebSettings, SlackIntegrationSettings, TeamsIntegrationSettings, }; pub use size::{ParseSizeError, Size}; pub use splice_array::{SPLICE_MARKER, SpliceArray, SpliceArrayError}; diff --git a/lib/crates/fabro-types/src/settings/resolved.rs b/lib/crates/fabro-types/src/settings/resolved.rs index 79a4825ff..16ad7a310 100644 --- a/lib/crates/fabro-types/src/settings/resolved.rs +++ b/lib/crates/fabro-types/src/settings/resolved.rs @@ -29,7 +29,7 @@ mod tests { DockerfileSource, McpServerSettings, McpTransport, RunAgentSettings, RunGoal, RunSettings, }; use crate::settings::server::{ - ObjectStoreSettings, ServerListenSettings, ServerSettings, ServerSlateDbSettings, TlsConfig, + ObjectStoreSettings, ServerListenSettings, ServerSettings, ServerSlateDbSettings, }; #[test] @@ -119,19 +119,11 @@ mod tests { assert_eq!( serde_json::to_value(ServerListenSettings::Tcp { address: "127.0.0.1:8080".parse().unwrap(), - tls: Some(TlsConfig { - cert: InterpString::parse("/tmp/server.crt"), - key: InterpString::parse("/tmp/server.key"), - }), }) .unwrap(), json!({ "type": "tcp", - "address": "127.0.0.1:8080", - "tls": { - "cert": "/tmp/server.crt", - "key": "/tmp/server.key" - } + "address": "127.0.0.1:8080" }) ); diff --git a/lib/crates/fabro-types/src/settings/server.rs b/lib/crates/fabro-types/src/settings/server.rs index ba7895e25..b9280eef7 100644 --- a/lib/crates/fabro-types/src/settings/server.rs +++ b/lib/crates/fabro-types/src/settings/server.rs @@ -37,7 +37,6 @@ pub enum ServerListenSettings { Tcp { #[serde(serialize_with = "serialize_socket_addr")] address: SocketAddr, - tls: Option, }, Unix { path: InterpString, @@ -52,21 +51,6 @@ impl Default for ServerListenSettings { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] -pub struct TlsConfig { - pub cert: InterpString, - pub key: InterpString, -} - -impl Default for TlsConfig { - fn default() -> Self { - Self { - cert: InterpString::parse(""), - key: InterpString::parse(""), - } - } -} - #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)] pub struct ServerApiSettings { pub url: Option, @@ -307,16 +291,13 @@ pub struct ServerLayer { pub integrations: Option, } -/// `[server.listen]` — shared bind transport. TLS lives under -/// `[server.listen.tls]`. +/// `[server.listen]` — shared bind transport. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields, tag = "type", rename_all = "lowercase")] pub enum ServerListenLayer { Tcp { #[serde(default)] address: Option, - #[serde(default)] - tls: Option, }, Unix { #[serde(default)] @@ -324,15 +305,6 @@ pub enum ServerListenLayer { }, } -#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct ServerListenTlsLayer { - #[serde(default, skip_serializing_if = "Option::is_none")] - pub cert: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub key: Option, -} - /// `[server.api]` — API surface settings. /// /// `url` is an optional public URL; it is **not** derived from `server.listen`.