Skip to content

lore-server: dial the auth service on a scheme tonic understands - #230

Open
nollbit wants to merge 1 commit into
EpicGames:mainfrom
goalsgame:fix/auth-endpoint-scheme
Open

nollbit wants to merge 1 commit into
EpicGames:mainfrom
goalsgame:fix/auth-endpoint-scheme

Conversation

@nollbit

@nollbit nollbit commented Oct 1, 2026

Copy link
Copy Markdown

The bug

[environment.endpoint] auth_url carries the configured scheme — ucs-auth://, oidc:// — which is not a scheme tonic can dial. Both server auth clients pass it to Endpoint::from_shared unchanged and gate TLS on a literal prefix:

// lore-server/src/authnz/auth.rs  (and rebac.rs)
if auth_url.starts_with("https://") {
    endpoint = endpoint.tls_config(...)
}

So an auth service configured as anything other than https:// is dialled in plaintext. The HTTP/2 preface arrives at a TLS listener, which closes the connection, and the call returns transport error with a broken pipe.

Only calls that reach the auth service online are affected. Where a token's claims answer the question locally, nothing dials out — which is why this can sit unnoticed for a long time: on a deployment whose tokens carry a resources claim, most traffic never touches it. The first thing to fail is whatever has no repository id to short-circuit on, which for us was RepositoryList.

The fix

lore_transport::auth::ucs_auth::grpc_endpoint already performs exactly this rewrite for the client side, loopback-http exemption included. The client used it; the server did not. This makes it pub and calls it from authnz::auth and authnz::rebac rather than restating the rule in two more places.

Endpoint construction moves into a function so the rewrite can be tested without standing up a listener.

Scope

Present in v0.8.6, v0.9.0, v0.10.0 and current main. Not a recent regression.

Verification

Reproduced against a real TLS auth listener, before and after:

Dialled as Result
plaintext HTTP/2 preface peer closes the connection — reproduces the failure
TLS, valid bearer token grpc-status: 0 and the expected response

cargo build -p lore-server, cargo test -p lore-server, cargo clippy --all-targets and cargo +nightly fmt --check are clean on top of main.

Tests added: ucs_auth_scheme_dials_https (both modules), https_auth_url_is_unchanged, loopback_http_stays_plaintext.

One note on severity

Against a TLS auth service this fails closed — the handshake fails and nothing is transmitted, which is how we found it. Against an auth service that accepts h2c, the same path would send authorization: Bearer <token> over an unencrypted channel. That is the reason grpc_endpoint upgrades non-loopback http:// to https:// in the first place; the server side simply was not using it.

I raised this with your security team rather than assume it was purely a functional bug, and opened this PR on the basis that the failure is fail-closed in the configuration that actually ships. Happy to pull it if you would rather handle it privately.

Alternatives

Reusing grpc_endpoint keeps one definition of the rule. Duplicating the scheme handling into the server, or normalising auth_url once at config load, would also work — the latter is arguably cleaner if you would rather the server never carry a non-dialable URL at all. Glad to rework it whichever way you prefer.

`[environment.endpoint] auth_url` carries the configured scheme --
`ucs-auth://`, `oidc://` -- which is not one tonic can dial. Both server
auth clients passed it to `Endpoint::from_shared` unchanged and gated
TLS on `starts_with("https://")`, so any auth service not literally
configured as `https://` was dialled in plaintext: the HTTP/2 preface
arrives at a TLS listener, which closes the connection, and the call
comes back as `transport error` with a broken pipe.

`lore_transport::auth::ucs_auth::grpc_endpoint` already does this
rewrite for the client side, loopback-http exemption included. Make it
public and use it from `authnz::auth` and `authnz::rebac` rather than
repeating the rule.

Only calls that reach the auth service online are affected; where a
token's claims answer the question locally, nothing dials out. The
endpoint construction moves into a function so the scheme rewrite can
be tested without a listener.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Johan Mjönes <johan@playgoals.com>
@github-actions github-actions Bot added area:server Server, provider integrations, telemetry area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals labels Oct 1, 2026
@nollbit
nollbit marked this pull request as ready for review October 1, 2026 10:12

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals area:server Server, provider integrations, telemetry

Development

Successfully merging this pull request may close these issues.

1 participant