diff --git a/docs/ServiceProxy.md b/docs/ServiceProxy.md index 23f3aedcd..e1da27221 100644 --- a/docs/ServiceProxy.md +++ b/docs/ServiceProxy.md @@ -1,6 +1,12 @@ # HTTPS Proxy for App Services -Running app services (`type: service`) bind a port on a compute node and publish `http://:/...`. Set `apps.service_proxy_domain` to republish them over HTTPS at a per-job subdomain instead. +Running app services (`type: service`) bind a port on a compute node and publish `http://:/...`. Set `apps.service_proxy_domain` to republish them over HTTPS at a per-job subdomain instead: + +``` +https://job-12-k7m2qhxr.services.example.org/lab?token=8f2c... +``` + +The `job--` label is signed with `session_secret_key`. Job ids are a small global sequence, so an unsigned `job-12` label would let anyone who can reach the proxy sweep `job-1`..`job-500` and find — and, for any service that does not enforce its own token, reach — every running service on the instance. The MAC is 8 base32 characters (40 bits); guessing it is online-only against a reverse proxy that answers 403, so a sweep of that space takes decades at 10k requests/second. Design rationale: `docs/superpowers/specs/2026-08-28-app-service-https-proxy-design.md`. @@ -21,22 +27,25 @@ Design rationale: `docs/superpowers/specs/2026-08-28-app-service-https-proxy-des ## Fileglancer configuration ```yaml +session_secret_key: "" apps: service_proxy_domain: "services.example.org" ``` -Leave it empty to disable; the direct `http://:` URL is then published unchanged. +Leave `service_proxy_domain` empty to disable; the direct `http://:` URL is then published unchanged. + +`session_secret_key` is required when the proxy domain is set, and the server refuses to start without it. An unset key is generated at random per process, so under `uvicorn --workers N` each worker would sign hostnames with a different key and most proxied requests would be refused. Rotating it invalidates live service URLs, on top of the session revocation rotation already causes. ## Reverse proxy configuration -Fileglancer does not proxy the traffic itself. It exposes `GET /api/apps/resolve`, which reads the `Host` header and answers `204` with `X-Fg-Upstream: :`, or `403`. The reverse proxy resolves each request through it and connects to the upstream directly, so no proxied bytes pass through the application server. +Fileglancer does not proxy the traffic itself. It exposes `GET /api/apps/resolve`, which reads the `Host` header and answers `204` with `X-Fg-Upstream: :` and `X-Fg-Upstream-Scheme: http|https`, or `403`. The reverse proxy resolves each request through it and connects to the upstream directly, so no proxied bytes pass through the application server. Add a server block for the wildcard zone. This assumes a `map $http_upgrade $connection_upgrade` block already exists at the http level: ```nginx server { listen 443 ssl http2; - server_name ~^job-\d+\.services\.example\.org$; + server_name ~^job-\d+-[a-z2-7]+\.services\.example\.org$; ssl_certificate /etc/nginx/certs/services-wildcard.crt; ssl_certificate_key /etc/nginx/certs/services-wildcard.key; @@ -52,11 +61,18 @@ server { location / { auth_request /_fg_resolve; auth_request_set $upstream $upstream_http_x_fg_upstream; + auth_request_set $upscheme $upstream_http_x_fg_upstream_scheme; # Required because proxy_pass targets a variable. Use whatever resolver the # host actually runs; 127.0.0.53 is systemd-resolved's stub. resolver 127.0.0.53 valid=30s; - proxy_pass http://$upstream; + proxy_pass $upscheme://$upstream; + + # An app that terminates TLS does so with a certificate it generated on the + # compute node, so there is no trust anchor to verify against. See "TLS to + # the app host" below. Leave proxy_ssl_name at its default. + proxy_ssl_verify off; + proxy_ssl_server_name on; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; @@ -70,6 +86,17 @@ server { proxy_buffering off; proxy_read_timeout 3600s; } + + # A refused resolution means the service is gone, not that the user did + # something wrong, so serve Fileglancer's explanation instead of nginx's + # stock 403 page. The `=` makes the response carry the page's own 503. + error_page 403 = /_fg_unavailable; + + location = /_fg_unavailable { + internal; + proxy_pass http://127.0.0.1:8989/api/apps/service-unavailable; + proxy_set_header Host $host; + } } ``` @@ -79,15 +106,28 @@ Also add this to the **main** server block, so the resolve endpoint is not reach location = /api/apps/resolve { return 404; } ``` -Four details are load-bearing: +Seven details are load-bearing: - **`internal;`** on the `/_fg_resolve` location makes it reachable only from nginx's own `auth_request` subrequest, never from a client. Together with the `return 404` in the main server block, it is what keeps the unauthenticated resolve endpoint off the network. Do not remove either. - **`proxy_set_header Host $host`** passes the app subdomain through unchanged, so the app sees `Host` and `Origin` as the same value. This is what makes JupyterLab's WebSocket origin check pass without per-app configuration. - **`resolver`** is mandatory. Without it nginx refuses to start when `proxy_pass` targets a variable. - **`proxy_buffering off`** and the long `proxy_read_timeout` suit long-lived WebSocket and streaming sessions, such as the remote desktop app. +- **`proxy_pass $upscheme://$upstream`**, rather than a hardcoded `http://`, is what lets an app that serves HTTPS be reached at all. Sending a plaintext request to a TLS listener does not silently lose encryption — the app rejects it — so hardcoding the scheme breaks those apps outright. +- **`proxy_intercept_errors` must stay off** (its default) for the `error_page 403` above to mean what it says. The 403 it catches is the one nginx generates when `auth_request` is denied; turning interception on would also catch a 403 from the app itself — a JupyterLab token rejection, say — and replace it with the "503 Service Unavailable" page. +- **`/_fg_unavailable` is a prefix location, not a named one**, because nginx refuses a `proxy_pass` with a URI part inside a named location (`proxy_pass cannot have URI part in location given by regular expression, or inside named location`). `internal;` is what keeps it out of the URL space the app sees, so a request for that path gets a 404 rather than the error page. The existing HTTP-to-HTTPS redirect block is typically `default_server` with `server_name _`, in which case it already covers the new subdomains. +## TLS to the app host + +The hop from the browser to the reverse proxy is always HTTPS. The hop from the proxy to the compute node is whatever the service itself published: Fileglancer reports the scheme from the service's own URL file and the proxy dials it with that. An app that fronts itself with a TLS terminator — Caddy, stunnel, its own `--certfile` — is reached over HTTPS; an app that serves plain HTTP is reached over HTTP, and nothing is required of it. + +**This is opportunistic encryption, and it authenticates nothing.** `proxy_ssl_verify` is `off` because the certificates in question are generated on the compute node at launch and signed by nobody: there is no trust anchor to check them against, and the node's name and port change every launch, so there is nothing stable to pin either. What it buys is that the service's token stops crossing the node network in cleartext. What it does not buy is any assurance that the thing answering on that host and port is the service — the upstream is read from a file the user's own job wrote, so the proxy could not make that claim regardless of certificates. + +Leave `proxy_ssl_name` at its default, which is the host from `proxy_pass`. Apps that generate their own certificate name it after the compute node, so overriding this to `$host` (the `job-` subdomain) would send an SNI value no app's certificate carries. + +To see how much traffic is still cleartext, read the `plaintext` figure in the aggregate log line described below. It counts alongside `hit` and `miss` rather than instead of them, so it reads as a fraction of the total. + ## Verification Once DNS and the certificate are in place, launch each service app and confirm it loads and stays connected. WebSocket behavior is the thing to watch: @@ -103,7 +143,7 @@ If an app rejects the proxied origin, fix it in that app's manifest (most server ## Residual risks - Set `apps.service_proxy_upstream_zone` to the DNS zone your compute nodes live in, e.g. `nodes.example.org`. Without it the proxy will dial any host the Fileglancer host can reach, because the upstream comes from a file the user's job wrote. Loopback, the unspecified address, link-local (including cloud instance metadata), multicast and reserved addresses are always refused, since those are what reach the Fileglancer host itself. Private and public node addresses are allowed — an address on a routable interface is already reachable directly by any cluster user. Matching is on whole DNS labels, so a leading dot is optional and a sibling zone like `evil-nodes.example.org` does not qualify. The zone applies to hostnames only: a service that publishes the node's IP instead of its name is still accepted, so setting a zone will not break one. To confine those, set `apps.service_proxy_upstream_networks` to the CIDR networks your nodes occupy, e.g. `10.20.0.0/16`. The two settings divide the space between them — the zone governs upstreams published as names, the networks govern upstreams published as addresses — and each is empty by default, meaning no restriction on that form. Bad CIDR entries are refused at startup rather than at request time, since a typo there would otherwise reject every address upstream while looking configured. -- Per-job subdomains make a running service much cheaper to *find* — `https://job-./` instead of port-scanning compute nodes — and the proxy vhost is reachable from wherever the Fileglancer HTTPS host is. The service's own token is still the only credential, but weigh this before exposing the vhost on a wide network. +- The signed hostname is not a substitute for a service enforcing its own token. It is unguessable, but a hostname leaks where a query string does not: plaintext SNI on the wire, DNS resolvers, and the proxy's own access log. Treat it as what makes enumeration infeasible, and `${FG_SERVICE_TOKEN}` as the credential. An app with no authentication of its own (TensorBoard, for one) is protected only by the label. - The resolve endpoint is called once per proxied HTTP request, so a single page load of an app like JupyterLab generates dozens. Successful resolutions are cached in-process for 10 seconds, which collapses that burst to roughly one database read per service per 10 seconds per worker. Refusals are deliberately not cached, so a service starts resolving the moment it publishes its URL. The endpoint is excluded from the per-request access log for the same reason and reports running totals once a minute instead — grep for `service proxy resolve totals` to see hits, misses and refusals by reason. - That 10-second cache is also the window in which a job that has just stopped can still be proxied. Compute-node ports get recycled, so the window is kept short deliberately; if a port is reused within it, a client can briefly reach the new occupant, which will reject it for lack of that service's own token. -- A service that manages its own URL (`auto_url` unset) should write its URL file exactly once. The cached upstream is refreshed only while someone has the job's detail page open, so a URL that changes mid-run can go stale. +- A service that manages its own URL (`auto_url` unset) should write its URL file exactly once, and should write the scheme it actually serves. Both the upstream and its scheme are taken from that file: publishing `http://` for a listener that speaks TLS, or the reverse, produces a hop that fails rather than one that merely works unencrypted. The cached upstream is refreshed only while someone has the job's detail page open, so a URL that changes mid-run can go stale. diff --git a/docs/config.yaml.template b/docs/config.yaml.template index c06ed1dee..70c0bb759 100644 --- a/docs/config.yaml.template +++ b/docs/config.yaml.template @@ -197,10 +197,11 @@ session_cookie_secure: true # # Wildcard DNS zone serving per-job HTTPS # # subdomains for running services. When set, a # # service is published at -# # https://job-./ instead of its own -# # http://:/ URL. Needs a wildcard -# # certificate for the zone and a reverse proxy -# # resolving upstreams via /api/apps/resolve; +# # https://job--./ instead of its +# # own http://:/ URL. Needs a wildcard +# # certificate for the zone, a reverse proxy +# # resolving upstreams via /api/apps/resolve, and +# # session_secret_key (which signs the hostname); # # see docs/ServiceProxy.md. Empty = direct URL. # service_proxy_upstream_zone: nodes.example.org # # DNS zone the proxy is allowed to dial, i.e. diff --git a/docs/superpowers/specs/2026-09-02-app-service-upstream-tls-options.md b/docs/superpowers/specs/2026-09-02-app-service-upstream-tls-options.md new file mode 100644 index 000000000..e01e7973c --- /dev/null +++ b/docs/superpowers/specs/2026-09-02-app-service-upstream-tls-options.md @@ -0,0 +1,182 @@ +# Authenticating the app-service proxy's upstream hop — options, parked + +Deferred design notes, not a plan. Split out of #440 and +JaneliaSciComp/fileglancer-hub#15, which give the browser HTTPS to nginx and +leave the nginx→app-host hop plain HTTP. + +**Status: parked as too heavy for a mostly trusted intranet.** Recorded so the +reasoning is not re-derived if the deployment ever reaches a less trusted +network — a wider vhost exposure, a cloud burst, or a multi-tenant cluster. +The low-effort step actually taken is opportunistic encryption only (option A +below): the proxy dials whatever scheme the service published, and does not +verify the certificate. + +## Two apps already serve TLS with self-signed certificates + +Found after the options below were first written, and it changes their weighting +rather than any of their mechanics. + +- **`JaneliaSciComp/marimo_ai_sandbox`** — `container/caddy-lib.sh`'s + `caddy_generate_cert` makes an `openssl` self-signed EC P-256 leaf, 10-year + validity, persisted per host under `CERT_DIR` and regenerated only when the + hostname changes. SANs are the node FQDN, the short hostname, `localhost` and + the host's IP addresses. Caddy serves it with `tls $CERT_FILE $KEY_FILE` (not + Caddy's internal-CA issuer, which shells out to `sudo` and hangs on a compute + node). `container/https-wrap.sh:262` publishes + `https://:/?access_token=…`. It bypasses `auto_url` specifically + so Fileglancer does not publish the backend's plain-HTTP port instead of + Caddy's TLS one. +- **`JaneliaSciComp/mhs-spec-review#1`** — Caddy with `tls internal` + (Caddy's own internal CA, `skip_install_trust`) on a fixed port 8443, and + writes `https://$(hostname -f):8443/`. + +Neither certificate is trusted by any system store, so both need option A. +Neither app can be verified under option C without changing how it generates +certificates. + +## The constraint that shapes all of it + +The upstream is a new node and port on every launch, sometimes published as a +bare IP address, and it arrives in a file the user's own job wrote. Nothing +about it is stable enough to name in static configuration. + +## A. Self-signed, unverified — `proxy_ssl_verify off` + +Confidentiality against passive capture on the node network. No authentication +of the upstream. Not a regression, since nothing authenticates the upstream +today either, but it has to be documented as opportunistic encryption rather +than as verified TLS. **This is the option being pursued.** + +It is also the only option the two existing TLS apps work under as written. +Leave `proxy_ssl_name` at its default (the `proxy_pass` host) rather than +setting it to `$host`: both apps' certificates are SAN'd to the node's own +name, which is what the default sends. + +## B. Self-signed with fingerprint pinning + +What would make self-signed meaningful, and nginx cannot do it: +`proxy_ssl_trusted_certificate` is a static path with no variable support, and +there is no upstream-fingerprint directive. + +Workarounds are worse than they sound. Appending each job's certificate to a +trust bundle and reloading nginx per service launch is racy, and the bundle +grows without bound. Moving the hop onto something that can pin per connection +(ghostunnel, Envoy with ext_authz) introduces a new component that carries the +noVNC video stream — the thing #440 went out of its way to avoid. + +Considered and rejected on its own merits, independent of the intranet +judgement. + +## C. Internal CA + +Issue a short-lived leaf per service job with +`SAN = job-.services.int.janelia.org`, which is exactly the value nginx +already has in `$host`: + +```nginx +proxy_ssl_trusted_certificate /etc/nginx/certs/fg-app-ca.pem; +proxy_ssl_verify on; +proxy_ssl_verify_depth 2; +proxy_ssl_server_name on; +proxy_ssl_name $host; +``` + +The verification config is fully static, and the certificate binds to the *job* +rather than to the node. That is what makes it tractable at all when the node +name changes every launch and is sometimes an IP address that name-based +verification could never cover. + +Issuance would fit where the plumbing already is: write `service_tls.{crt,key}` +(0600, user-owned) into the work dir at submit time and export +`FG_SERVICE_TLS_CERT` / `FG_SERVICE_TLS_KEY` from the preamble at +`fileglancer/apps/jobs.py`, next to the `SERVICE_URL_PATH` export. Lifetime = max +walltime plus slack. The CA key stays root-only on the Fileglancer host and off +the shared filesystem, or comes from step-ca / Vault if one is already run. + +Strictly this is "we are our own CA," not per-app self-signed. + +Note the interaction with the two existing TLS apps: `proxy_ssl_name $host` +sends the job subdomain as the SNI and requires it in the leaf's SAN, which +neither app's self-generated certificate has. Adopting C therefore means +converting both apps to consume an issued certificate, not just adding nginx +directives — a migration cost the sketch above hides. + +Parked because it means running a CA — key custody, rotation, an issuance path +in the submit hot path — to defend against an active on-path attacker inside +the cluster network. That is not the threat model here. + +## Non-option, recorded so it is not re-proposed + +Compute nodes are `.int.janelia.org`, so the existing org wildcard +certificate would cover them with no new PKI at all. Rejected: its private key +would have to be readable by every user's job. + +## The app side: getting services to serve TLS + +Support is uneven across the shipped apps. Jupyter has `--certfile` / +`--keyfile`; openvscode-server and websockify vary; TensorBoard has none. +Per-manifest TLS flags therefore mean an open-ended per-app tail — the same trap +the subdomain-over-path-prefix decision in #440 was made to avoid. + +The uniform alternative, if this is ever revisited: keep the app on +`127.0.0.1:$PORT` and put a TLS terminator in the job wrapper, binding the +public port with the issued certificate. One change in the `jobs.py` preamble +instead of N manifest changes, and `auto_url` apps get it for free. It also +closes a hole that exists independent of all this — the raw app port is +reachable by any cluster user today. With ghostunnel, nginx could additionally +present a `proxy_ssl_certificate` that the sidecar requires, making the app +reachable *only* through the proxy. + +This is not hypothetical: both apps above already do exactly this with Caddy, +arrived at independently, and `marimo_ai_sandbox` has already factored the +machinery into a reusable `caddy-lib.sh` shared by its HTTPS and web-terminal +wrappers. If a uniform terminator is ever wanted, that file is the closest +thing to a prototype, and Caddy is the more likely choice than stunnel purely +because two apps already depend on it. + +Parked with C only insofar as *issued* certificates go: the sidecar needs +something to bind with, and without an issuance story it can only bind a +self-signed certificate — which is what these two apps already do for +themselves, so a Fileglancer-provided sidecar would add nothing at option A. + +## What is not parked + +The scheme plumbing: return the upstream scheme instead of discarding it, carry +it in its own `X-Fg-Upstream-Scheme` header (including through the TTL cache, or +cached hits silently downgrade), and let nginx `proxy_pass +$upscheme://$upstream`. An earlier draft gated this on an +`apps.service_proxy_upstream_tls: allow | require` setting; that was withdrawn, +because `require` would refuse every plaintext app and TensorBoard has no TLS +option at all, which left the setting with one legal value. A `plaintext` +counter in the existing once-a-minute resolve totals gives an operator the same +visibility without refusing anyone's launch. + +Because those two apps exist, this is not an enhancement — it is a +**prerequisite for setting `apps.service_proxy_domain` at all.** Discarding the +scheme means nginx does `proxy_pass http://:8443` at a TLS listener, and +Caddy answers a plaintext request to an HTTPS port with a 400. Both apps break +the moment the proxy is enabled. + +Two further per-app breakages, independent of the scheme and belonging to those +repos rather than to Fileglancer: + +- `mhs-spec-review`'s Caddyfile site address is Host-matched + (`{$CADDY_HOSTNAME}:8443, localhost:8443, 127.0.0.1:8443`). nginx passes + `Host $host` through unchanged — load-bearing for Jupyter's WebSocket origin + check — so Caddy sees `job-.services.int.janelia.org`, matches no site + and 404s even once the scheme is right. It needs a catch-all `:8443` site + address. `marimo_ai_sandbox` uses a port-only address and is unaffected. +- That app's GitHub OAuth callback is pinned to + `https://int.janelia.org:8443/callback` with wildcard matching, and its + `url_for(_external=True)` + `ProxyFix` would produce + `https://job-.services.int.janelia.org/callback` behind the proxy — + different port, two extra labels. Whether GitHub's wildcard matching accepts + that needs checking; if not, that app wants a way to opt out of being + republished rather than a fix. + +The second one suggests a gap worth considering separately: there is currently +no way for a manifest to say "do not proxy me." + +Also: hub#15's per-app verification checklist lists only the five shipped +service apps. Both Caddy apps should be on it, since they are the only two that +exercise the TLS path at all. diff --git a/fileglancer/apps/__init__.py b/fileglancer/apps/__init__.py index 228b20267..e36bbce1a 100644 --- a/fileglancer/apps/__init__.py +++ b/fileglancer/apps/__init__.py @@ -53,5 +53,7 @@ record_resolve, reset_resolve_metrics, resolve_counts, + service_host_label, upstream_from_service_url, + upstream_scheme_from_service_url, ) diff --git a/fileglancer/apps/serviceproxy.py b/fileglancer/apps/serviceproxy.py index c8e8338fe..ef733bb66 100644 --- a/fileglancer/apps/serviceproxy.py +++ b/fileglancer/apps/serviceproxy.py @@ -2,11 +2,14 @@ A service job publishes ``http://:`` to its work directory. When a proxy domain is configured, Fileglancer republishes that as -``https://job-.`` and tells the reverse proxy which -upstream the hostname maps to. These functions are the whole translation layer -between the two forms, kept pure so they can be tested without a database. +``https://job--.`` and tells the reverse proxy +which upstream the hostname maps to. These functions are the whole translation +layer between the two forms, kept pure so they can be tested without a database. """ +import base64 +import hashlib +import hmac import ipaddress import re import time @@ -28,45 +31,82 @@ # aim the proxy at the web host itself rather than at a compute node. _LOCAL_NAMES = frozenset({'localhost'}) +# Job ids are a small global sequence, so a bare job- label would let anyone +# who can reach the proxy sweep job-1..job-500 and find every running service on +# the instance — and reach any of them that does not enforce its own token. The +# label therefore carries a MAC over the id, keyed by the server's session +# secret. Signing rather than storing a random key keeps the id in the hostname, +# so resolution stays one indexed read with no extra column, and the MAC is +# checked before the database is touched at all. +# +# Eight base32 characters is 40 bits. Guessing is online-only — a wrong label is +# a 403 from the reverse proxy, and there is nothing to attack offline — so at +# 10k requests/second a sweep of the space takes decades. Rotating +# session_secret_key invalidates live service URLs, which is the same blast +# radius as the session revocation that rotation already causes. +_MAC_CHARS = 8 + + +def _service_mac(job_id: int, secret: str) -> str: + """The MAC for a job's proxy hostname label.""" + digest = hmac.new(secret.encode('utf-8'), + f'job:{job_id}'.encode('utf-8'), + hashlib.sha256).digest() + # base32 rather than hex: same DNS-safe alphabet cost, 25% fewer characters + # per bit. Lowercased because hostnames are compared case-insensitively. + return base64.b32encode(digest).decode('ascii').lower()[:_MAC_CHARS] + + +def service_host_label(job_id: int, secret: str) -> str: + """The leftmost hostname label for a job's proxy URL, e.g. ``job-12-k7m2qhxr``.""" + return f'job-{job_id}-{_service_mac(job_id, secret)}' + def build_proxied_service_url(service_url: Optional[str], job_id: int, - proxy_domain: str) -> Optional[str]: + proxy_domain: str, secret: str) -> Optional[str]: """Rewrite a published service URL to its HTTPS proxy form. Path, query and fragment are carried over verbatim: the query string holds the service's own access token, which remains the only credential. - Returns None when there is nothing to rewrite or no proxy domain is - configured, in which case the caller should publish the URL unchanged. + Returns None when there is nothing to rewrite, no proxy domain is + configured, or no secret is available to sign the hostname with, in which + case the caller should publish the URL unchanged. """ - if not service_url or not proxy_domain: + if not service_url or not proxy_domain or not secret: return None parts = urlsplit(service_url) return urlunsplit(( 'https', - f'job-{job_id}.{proxy_domain}', + f'{service_host_label(job_id, secret)}.{proxy_domain}', parts.path, parts.query, parts.fragment, )) -def job_id_from_host(host: Optional[str], proxy_domain: str) -> Optional[int]: +def job_id_from_host(host: Optional[str], proxy_domain: str, + secret: str) -> Optional[int]: """Extract the job id from a proxy hostname, or None if it isn't one. Matches the whole hostname, so neither a longer suffix - (``job-1.services.example.org.evil``) nor an extra label - (``x.job-1.services.example.org``) is accepted. + (``job-1-k7m2qhxr.services.example.org.evil``) nor an extra label + (``x.job-1-k7m2qhxr.services.example.org``) is accepted, and the label's MAC + must verify against the id it carries. """ - if not host or not proxy_domain: + if not host or not proxy_domain or not secret: return None # $host in nginx normally omits the port, but a client can send one. hostname = host.split(':', 1)[0].strip().lower() match = re.fullmatch( - r'job-(\d{1,9})\.' + re.escape(proxy_domain.lower()), hostname) + r'job-(\d{1,9})-([a-z2-7]{%d})\.' % _MAC_CHARS + + re.escape(proxy_domain.lower()), hostname) if match is None: return None - return int(match.group(1)) + job_id = int(match.group(1)) + if not hmac.compare_digest(match.group(2), _service_mac(job_id, secret)): + return None + return job_id def _is_ip_literal(host: str) -> bool: @@ -214,6 +254,29 @@ def upstream_from_service_url(service_url: Optional[str], return netloc +def upstream_scheme_from_service_url(service_url: Optional[str]) -> str: + """Return the scheme the reverse proxy should dial an upstream with. + + Only meaningful for a URL ``upstream_from_service_url`` has already + accepted; on its own this says nothing about whether the authority is safe + to dial, so the resolve endpoint calls the two in that order. + + Anything that is not exactly ``https`` yields ``http``, including a URL that + fails to parse. That is the safe direction: plaintext to a plaintext + listener is what the proxy has always done, whereas guessing ``https`` at + one turns a working service into a failed handshake. Nothing needs refusing + here either — ``read_service_url_file`` rejects a URL that is not + ``http://`` or ``https://`` before it can reach the database. + """ + if not service_url: + return 'http' + try: + scheme = urlsplit(service_url).scheme + except ValueError: + return 'http' + return 'https' if scheme.lower() == 'https' else 'http' + + # --- Resolution cache and counters --- # # The reverse proxy calls the resolve endpoint once per proxied HTTP request, so @@ -225,6 +288,9 @@ def upstream_from_service_url(service_url: Optional[str], # published its URL yet must stay a miss, or clicking "Open Service" the moment a # service comes up would fail for the whole TTL. # +# An entry holds both the upstream and the scheme to dial it with, since the +# reverse proxy needs both and only one of them is in the hostname. +# # The TTL is deliberately short. It is the window during which a job that has # stopped can still be proxied, and the RUNNING check it bypasses exists because # compute-node ports get recycled. A page load's burst is sub-second, so a few @@ -241,14 +307,20 @@ def upstream_from_service_url(service_url: Optional[str], _resolve_last_logged = 0.0 -def cached_upstream(job_id: int) -> Optional[str]: - """Return a recently resolved upstream for a job, or None to consult the DB.""" +def cached_upstream(job_id: int) -> Optional[tuple]: + """Return a recently resolved ``(upstream, scheme)``, or None to consult the DB.""" return _resolve_cache.get(job_id) -def cache_upstream(job_id: int, upstream: str) -> None: - """Remember a successful resolution for the cache's short TTL.""" - _resolve_cache[job_id] = upstream +def cache_upstream(job_id: int, upstream: str, scheme: str) -> None: + """Remember a successful resolution for the cache's short TTL. + + The scheme is stored with the upstream rather than recomputed per hit. + Hits outnumber misses by design, so a scheme derived on the miss path alone + would leave an HTTPS service being dialed as plaintext for the rest of the + TTL — and flapping between the two as entries expire. + """ + _resolve_cache[job_id] = (upstream, scheme) def record_resolve(outcome: str) -> None: @@ -258,7 +330,9 @@ def record_resolve(outcome: str) -> None: every minute says the same thing as hundreds of individual lines, and says it in a form an operator can actually read. Outcomes are coarse on purpose — 'hit', 'miss', and a refusal reason — so the line stays useful without - naming any specific job. + naming any specific job. 'plaintext' is counted in addition to a hit or a + miss, not instead of one, so it reads as a fraction of the total: how much + of the proxied traffic is still unencrypted on the hop to the app host. """ global _resolve_last_logged _resolve_counts[outcome] += 1 diff --git a/fileglancer/server.py b/fileglancer/server.py index 2a5aacb1f..5644f3f18 100644 --- a/fileglancer/server.py +++ b/fileglancer/server.py @@ -27,7 +27,7 @@ from contextlib import asynccontextmanager from fastapi import FastAPI, HTTPException, Request, Query, Path, Body, Depends from fastapi.middleware.cors import CORSMiddleware -from fastapi.responses import RedirectResponse, Response, JSONResponse, PlainTextResponse, StreamingResponse, FileResponse +from fastapi.responses import RedirectResponse, Response, JSONResponse, PlainTextResponse, StreamingResponse, FileResponse, HTMLResponse from fastapi.exceptions import RequestValidationError, StarletteHTTPException from fastapi.staticfiles import StaticFiles from starlette.middleware.sessions import SessionMiddleware @@ -365,6 +365,54 @@ def _validate_short_name(short_name: str) -> None: raise HTTPException(status_code=400, detail="short_name can only contain letters, numbers, hyphens, and underscores") +# Shown at the URL the user clicked when the service proxy refuses to resolve a +# hostname, which is almost always a service whose job has since stopped. The +# reverse proxy maps its own 403 here (see docs/ServiceProxy.md), so this ships +# with Fileglancer rather than living in each deployment's nginx html directory. +# +# Self-contained on purpose: the proxy's server block routes everything through +# auth_request, so a stylesheet or image referenced from here would be refused +# by the same check that produced this page. No link back to Fileglancer either +# — the app has no configured public base URL, and this page is served on a +# subdomain that cannot be assumed to share one with it. +_SERVICE_UNAVAILABLE_HTML = """ + + + + +Service not running + + + +
+

503 Service Unavailable

+

The app that was served at this address is no longer available. Services + are reachable only while their job is running.

+
    +
  • The job finished, was cancelled, or hit its time limit.
    • +
  • The job is still starting up and hasn't published its address yet — + wait a moment and reload.
    • +
  • The link is old. Each launch gets its own address, so a bookmark from a + previous run will not work.
    • +
+

Open the job in Fileglancer to check its status or launch the + app again.

+
+ + +""" + + def create_app(settings): # Initialize OAuth client for OKTA @@ -2715,10 +2763,30 @@ async def get_job(job_id: int, # The cached value stays raw — it is the proxy's upstream. Only what # goes back to the browser is rewritten. proxied = apps_module.build_proxied_service_url( - service_url, job_id, settings.apps.service_proxy_domain) + service_url, job_id, settings.apps.service_proxy_domain, + settings.session_secret_key) return _convert_job(db_job, service_url=proxied or service_url, files=files, phase=phase) + def _resolve_ok(upstream: str, scheme: str) -> Response: + """Build the 204 the reverse proxy reads, and count a plaintext hop. + + The scheme is its own header rather than a prefix on X-Fg-Upstream so a + reverse proxy configured before this existed keeps working: it ignores + the header it does not know and dials http, exactly as it did before. + Folding the scheme into the existing header would instead have it build + `proxy_pass http://https://host:port`. + + Counting happens here so the cached and uncached paths cannot disagree + about it. + """ + if scheme != 'https': + apps_module.record_resolve("plaintext") + return Response(status_code=204, headers={ + "X-Fg-Upstream": upstream, + "X-Fg-Upstream-Scheme": scheme, + }) + @app.get("/api/apps/resolve", include_in_schema=False) async def resolve_service_upstream(request: Request): """Map a service proxy hostname to its upstream, for the reverse proxy. @@ -2730,8 +2798,10 @@ async def resolve_service_upstream(request: Request): that the job's detail page already shows, and the reverse proxy marks its location `internal` so it is not reachable from outside. - Returns 204 with X-Fg-Upstream on success and 403 for everything else, so - auth_request denies the request. + Returns 204 with X-Fg-Upstream and X-Fg-Upstream-Scheme on success, and + 403 for everything else, so auth_request denies the request. The scheme + is whatever the service published: an app that terminates TLS itself is + dialed over HTTPS instead of being downgraded to cleartext. Successful resolutions are cached for a few seconds, which is also the window in which a job that has just stopped can still be proxied. See @@ -2740,7 +2810,8 @@ async def resolve_service_upstream(request: Request): """ proxy_domain = settings.apps.service_proxy_domain job_id = apps_module.job_id_from_host( - request.headers.get('host'), proxy_domain) + request.headers.get('host'), proxy_domain, + settings.session_secret_key) if job_id is None: apps_module.record_resolve("refused_bad_host") raise HTTPException(status_code=403, detail="Not a service proxy host") @@ -2749,11 +2820,10 @@ async def resolve_service_upstream(request: Request): # cache and keep the database out of the hot path. Only hits are cached; # a service that has not published its URL yet must be able to start # resolving the moment it does. - upstream = apps_module.cached_upstream(job_id) - if upstream is not None: + cached = apps_module.cached_upstream(job_id) + if cached is not None: apps_module.record_resolve("hit") - return Response(status_code=204, - headers={"X-Fg-Upstream": upstream}) + return _resolve_ok(*cached) with db.get_db_session(settings.db_url) as session: db_job = db.get_job_by_id(session, job_id) @@ -2762,8 +2832,9 @@ async def resolve_service_upstream(request: Request): or db_job.status != 'RUNNING'): apps_module.record_resolve("refused_not_running") raise HTTPException(status_code=403, detail="No running service for this host") + service_url = db_job.service_url upstream = apps_module.upstream_from_service_url( - db_job.service_url, + service_url, allowed_zone=settings.apps.service_proxy_upstream_zone, allowed_networks=tuple(settings.apps.service_proxy_upstream_networks)) @@ -2771,9 +2842,26 @@ async def resolve_service_upstream(request: Request): apps_module.record_resolve("refused_no_upstream") raise HTTPException(status_code=403, detail="No usable upstream for this host") - apps_module.cache_upstream(job_id, upstream) + # Read only after the authority has been accepted: the scheme says + # nothing about whether the host is safe to dial. + scheme = apps_module.upstream_scheme_from_service_url(service_url) + apps_module.cache_upstream(job_id, upstream, scheme) apps_module.record_resolve("miss") - return Response(status_code=204, headers={"X-Fg-Upstream": upstream}) + return _resolve_ok(upstream, scheme) + + @app.get("/api/apps/service-unavailable", include_in_schema=False) + async def service_unavailable_page(): + """The page a user sees when a service proxy hostname no longer resolves. + + The reverse proxy renders this in place of its own 403, at the URL the + user clicked. 503 rather than 403 because "not running" is what actually + happened in every case a real user reaches: a refusal from + /api/apps/resolve means the job is gone, still starting, or was never + theirs to reach, and none of those is worth distinguishing to a browser. + Unauthenticated for the same reason the resolve endpoint is — the session + cookie is scoped to the Fileglancer hostname and is never sent here. + """ + return HTMLResponse(_SERVICE_UNAVAILABLE_HTML, status_code=503) @app.patch("/api/jobs/{job_id}", response_model=Job, description="Rename a job") diff --git a/fileglancer/settings.py b/fileglancer/settings.py index 8a98cd05a..bff67106d 100644 --- a/fileglancer/settings.py +++ b/fileglancer/settings.py @@ -285,6 +285,22 @@ def settings_customise_sources( # noqa: PLR0913 file_secret_settings, ) + @model_validator(mode='after') + def validate_service_proxy_needs_a_session_secret(self): + """The service proxy signs its hostnames with session_secret_key. + + An unset key is generated at random per process, so under + `uvicorn --workers N` each worker would sign with a different key and + most proxied requests would be refused. Fail at startup instead, where + the cause is obvious. + """ + if self.apps.service_proxy_domain and not self.session_secret_key: + raise ValueError( + "apps.service_proxy_domain requires session_secret_key to be " + "set: service proxy hostnames are signed with it, and a " + "per-process random key breaks resolution across workers") + return self + @model_validator(mode='after') def set_jira_browse_url(self): if self.jira_browse_url is None: diff --git a/lefthook.yml b/lefthook.yml index 012c34a02..8d6792661 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -6,7 +6,10 @@ pre-push: jobs: - name: Run Prettier check; fix if check fails root: frontend/ - run: pixi run ./prettier-check-and-fix.sh + # Invoked through bash explicitly: on Windows, pixi hands the script to + # the OS to execute and gets `os error 193` (not a valid Win32 + # application), so the hook could never run there. + run: pixi run bash ./prettier-check-and-fix.sh - name: Run ESLint check root: frontend/ diff --git a/tests/test_service_proxy.py b/tests/test_service_proxy.py index 3cdec9436..93a6e4dbe 100644 --- a/tests/test_service_proxy.py +++ b/tests/test_service_proxy.py @@ -23,6 +23,12 @@ OWNER = "alice" PROXY_DOMAIN = "services.example.org" +SECRET = "test-session-secret" + + +def _host(job_id): + """The signed proxy hostname for a job, as the API would publish it.""" + return f"{apps.service_host_label(job_id, SECRET)}.{PROXY_DOMAIN}" @pytest.fixture @@ -54,6 +60,7 @@ def _build(proxy_domain="", upstream_zone=""): db_url=db_url, file_share_mounts=[], cli_mode=True, + session_secret_key=SECRET, apps=AppsSettings(service_proxy_domain=proxy_domain, service_proxy_upstream_zone=upstream_zone), ), db_url @@ -196,11 +203,30 @@ def _seed_running_service_with_url(db_url, url="http://node01:41235/lab?token=ab def test_resolve_returns_upstream(app_factory): app, db_url = app_factory(PROXY_DOMAIN) job_id = _seed_running_service_with_url(db_url) - resp = _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}") + resp = _resolve(app, _host(job_id)) assert resp.status_code == 204 assert resp.headers["x-fg-upstream"] == "node01:41235" +@pytest.mark.parametrize("url,scheme", [ + ("http://node01:41235/lab?token=abc", "http"), + ("https://node01:41235/lab?token=abc", "https"), +]) +def test_resolve_reports_the_published_scheme(app_factory, url, scheme): + """An app that terminates TLS itself must not be dialed as cleartext. + + Two shipped apps front themselves with Caddy and publish an https URL; a + plaintext request at their TLS listener is answered with a 400, so dropping + the scheme here breaks them outright rather than merely leaving the hop + unencrypted.""" + app, db_url = app_factory(PROXY_DOMAIN) + job_id = _seed_running_service_with_url(db_url, url=url) + resp = _resolve(app, _host(job_id)) + assert resp.status_code == 204 + assert resp.headers["x-fg-upstream"] == "node01:41235" + assert resp.headers["x-fg-upstream-scheme"] == scheme + + def test_resolve_rejects_finished_job(app_factory): """Compute-node ports get recycled. A stale subdomain must not be proxied to whatever service now holds that port on that node.""" @@ -212,7 +238,7 @@ def test_resolve_rejects_finished_job(app_factory): session.commit() finally: session.close() - resp = _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}") + resp = _resolve(app, _host(job_id)) assert resp.status_code == 403 assert "x-fg-upstream" not in resp.headers @@ -225,24 +251,26 @@ def test_resolve_rejects_non_service_job(app_factory): set_job_service_url(session, job_id, "http://node01:41235/") finally: session.close() - assert _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}").status_code == 403 + assert _resolve(app, _host(job_id)).status_code == 403 def test_resolve_rejects_job_without_cached_url(app_factory): app, db_url = app_factory(PROXY_DOMAIN) job_id = _seed_service_job(db_url) - assert _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}").status_code == 403 + assert _resolve(app, _host(job_id)).status_code == 403 def test_resolve_rejects_unknown_job(app_factory): app, db_url = app_factory(PROXY_DOMAIN) - assert _resolve(app, f"job-999999.{PROXY_DOMAIN}").status_code == 403 + assert _resolve(app, _host(999999)).status_code == 403 @pytest.mark.parametrize("host", [ "fileglancer.example.org", "job-1.evil.example.org", "job-abc.services.example.org", + "job-1.services.example.org", # unsigned: the old, guessable form + "job-1-aaaaaaaa.services.example.org", # wrong MAC ]) def test_resolve_rejects_bad_hosts(app_factory, host): app, db_url = app_factory(PROXY_DOMAIN) @@ -254,7 +282,19 @@ def test_resolve_disabled_when_no_proxy_domain(app_factory): """With the feature off, the endpoint refuses everything.""" app, db_url = app_factory("") job_id = _seed_running_service_with_url(db_url) - assert _resolve(app, f"job-{job_id}.{PROXY_DOMAIN}").status_code == 403 + assert _resolve(app, _host(job_id)).status_code == 403 + + +def test_service_unavailable_page_is_servable_without_auth(app_factory): + """The reverse proxy renders this in place of its 403, on a subdomain the + session cookie is never sent to, so it must need no user and no assets.""" + app, _ = app_factory(PROXY_DOMAIN) + resp = TestClient(app).get("/api/apps/service-unavailable") + assert resp.status_code == 503 + assert resp.headers["content-type"].startswith("text/html") + assert "503 Service Unavailable" in resp.text + # Self-contained: the proxy would refuse a subresource fetched from here. + assert "src=" not in resp.text and "