diff --git a/README.md b/README.md index e72d3d7..7a39736 100644 --- a/README.md +++ b/README.md @@ -127,9 +127,11 @@ upstream ID token, and minted Sith JWT remain server-side. A bounded, single-use transaction binds the exact workspace, state, nonce, redirect URI, issuer, client ID, and verifier. The callback consumes that transaction before redeeming the code, resolves the current membership through forced RLS, and returns only a short-lived `__Host-sith-session` cookie (`Secure`, -`HttpOnly`, `Path=/`, no `Domain`, `SameSite=Strict`). Its short-lived `__Host-sith-oidc-tx` -transaction cookie is `SameSite=Lax` only so a top-level IdP callback can return; it contains an -opaque random binding, not a credential. Restart, expiry, replay, malformed/duplicate provider +`HttpOnly`, `Path=/`, no `Domain`, `SameSite=Lax`). Lax allows that new session to reach the safe +top-level console `GET` whose navigation began at the external IdP, while excluding unsafe +cross-site methods. Its short-lived `__Host-sith-oidc-tx` transaction cookie is also `SameSite=Lax` +so the top-level IdP callback can return; it contains an opaque random binding, not a credential. +Restart, expiry, replay, malformed/duplicate provider JSON, failed PKCE, wrong state/nonce/issuer/audience, or an unavailable provider fails closed. No login artifact, token, or proof is persisted or logged. On success, the callback redirects only to the transaction-bound `GET /v1/workspaces/{workspace}/console` path; it accepts no return URL. diff --git a/charts/sith-hub/README.md b/charts/sith-hub/README.md index d163911..d4accb8 100644 --- a/charts/sith-hub/README.md +++ b/charts/sith-hub/README.md @@ -37,7 +37,7 @@ Service port, credential, or OCM/spoke dependency check. The chart pins workload hardening (UID/GID 65532, read-only root filesystem, RuntimeDefault seccomp, no privilege escalation, and all Linux capabilities dropped). It deliberately does not create a broad egress NetworkPolicy: the database and pinned OCM endpoints are deployment-specific, so operators must place the release in a namespace with an appropriate least-privilege egress policy. KMS provider resources, release-bound image publication, real install/upgrade proof, air-gap bundles, and addon packaging are later E9/E3 slices. -`runtime.browserOIDC` is disabled only when all three of `issuer`, `clientID`, and `redirectURI` are empty. Setting any one enables a fail-closed validation requiring all three. The configured client ID is the exact ID-token audience; the callback must be the same HTTPS URL registered at the issuer and supplied to the Hub. In that mode the Hub mounts `session-private.pem`, checks that it matches `session-public.pem`, completes authorization-code + PKCE (`S256`) server-side, and returns a short-lived `__Host-` `Secure`, `HttpOnly`, strict-same-site session cookie. The chart never renders any of this secret material. The Hub needs narrowly allowlisted egress only to the configured issuer's discovery, JWKS, and token endpoints; the operator's browser navigates to the issuer authorization endpoint. +`runtime.browserOIDC` is disabled only when all three of `issuer`, `clientID`, and `redirectURI` are empty. Setting any one enables a fail-closed validation requiring all three. The configured client ID is the exact ID-token audience; the callback must be the same HTTPS URL registered at the issuer and supplied to the Hub. In that mode the Hub mounts `session-private.pem`, checks that it matches `session-public.pem`, completes authorization-code + PKCE (`S256`) server-side, and returns a short-lived `__Host-`, `Secure`, `HttpOnly`, `SameSite=Lax` session cookie. Lax permits the safe top-level console `GET` after the cross-site IdP callback but does not include the cookie on unsafe cross-site methods. The chart never renders any of this secret material. The Hub needs narrowly allowlisted egress only to the configured issuer's discovery, JWKS, and token endpoints; the operator's browser navigates to the issuer authorization endpoint. `runtime.metrics.listenAddress` is opt-in and must be exactly `127.0.0.1:` or `[::1]:`, with a non-zero port. When set, the chart provides only the corresponding diff --git a/internal/hubserver/browser_oidc.go b/internal/hubserver/browser_oidc.go index 7d4f827..99ef3cb 100644 --- a/internal/hubserver/browser_oidc.go +++ b/internal/hubserver/browser_oidc.go @@ -175,7 +175,8 @@ func (handler *BrowserOIDCHandler) Login(response http.ResponseWriter, request * response.WriteHeader(http.StatusFound) } -// Callback consumes one transaction and sets the final strict host-only session cookie on success. +// Callback consumes one transaction and sets the final host-only session cookie on success. +// Lax is required for the safe top-level redirect whose navigation began at the external IdP. func (handler *BrowserOIDCHandler) Callback(response http.ResponseWriter, request *http.Request) { if !handler.acceptRequest(response, request, false) { return @@ -223,7 +224,7 @@ func (handler *BrowserOIDCHandler) Callback(response http.ResponseWriter, reques remaining := int(session.ExpiresAt.Sub(handler.now()).Seconds()) http.SetCookie(response, &http.Cookie{ Name: browserOIDCSessionCookie, Value: session.AccessToken, Path: "/", Secure: true, HttpOnly: true, - SameSite: http.SameSiteStrictMode, Expires: session.ExpiresAt.UTC(), MaxAge: max(1, remaining), + SameSite: http.SameSiteLaxMode, Expires: session.ExpiresAt.UTC(), MaxAge: max(1, remaining), }) response.Header().Set("Location", browserConsolePath(transaction.workspaceID)) response.WriteHeader(http.StatusSeeOther) diff --git a/internal/hubserver/browser_oidc_test.go b/internal/hubserver/browser_oidc_test.go index cb34be4..9625ac1 100644 --- a/internal/hubserver/browser_oidc_test.go +++ b/internal/hubserver/browser_oidc_test.go @@ -103,7 +103,7 @@ func TestBrowserOIDCHandlerKeepsTokensOutOfBrowserPayloads(t *testing.T) { t.Fatalf("callback status/location/body = %d/%q/%q", callbackResponse.Code, callbackResponse.Header().Get("Location"), callbackResponse.Body.String()) } sessionCookie := requiredBrowserCookie(t, callbackResponse.Result().Cookies(), browserOIDCSessionCookie) - if !sessionCookie.Secure || !sessionCookie.HttpOnly || sessionCookie.Path != "/" || sessionCookie.Domain != "" || sessionCookie.SameSite != http.SameSiteStrictMode || sessionCookie.Value != stub.session.AccessToken { + if !sessionCookie.Secure || !sessionCookie.HttpOnly || sessionCookie.Path != "/" || sessionCookie.Domain != "" || sessionCookie.SameSite != http.SameSiteLaxMode || sessionCookie.Value != stub.session.AccessToken { t.Fatalf("session cookie = %#v", sessionCookie) } if stub.exchangeCalls != 1 || stub.exchange.Code != "provider-code" || stub.exchange.CodeVerifier != verifier || @@ -125,6 +125,32 @@ func TestBrowserOIDCHandlerKeepsTokensOutOfBrowserPayloads(t *testing.T) { } } +func TestBrowserOIDCHandlerRejectsUnsafeMethods(t *testing.T) { + now := time.Date(2026, 7, 22, 18, 0, 0, 0, time.UTC) + stub := &browserOIDCServiceStub{} + handler := newBrowserOIDCTestHandler(t, stub, &now) + + for _, target := range []string{ + "https://hub.sith.test/v1/workspaces/workspace-a/console/login", + "https://hub.sith.test/v1/console/oidc/callback?code=provider-code&state=provider-state", + } { + request := httptest.NewRequest(http.MethodPost, target, nil) + request.RemoteAddr = "192.0.2.21:8443" + response := httptest.NewRecorder() + if request.URL.Path == handler.callbackURL.Path { + handler.Callback(response, request) + } else { + handler.Login(response, request) + } + if response.Code != http.StatusNotFound { + t.Fatalf("POST %s status = %d, want %d", request.URL.Path, response.Code, http.StatusNotFound) + } + } + if stub.authorizeCalls != 0 || stub.exchangeCalls != 0 || stub.sessionCalls != 0 || len(handler.transactions) != 0 { + t.Fatalf("unsafe method calls/transactions = %d/%d/%d/%d", stub.authorizeCalls, stub.exchangeCalls, stub.sessionCalls, len(handler.transactions)) + } +} + func TestBrowserOIDCHandlerCountsEachRequestOnceAtRateLimit(t *testing.T) { now := time.Date(2026, 7, 16, 15, 0, 0, 0, time.UTC) limiter, err := NewAttemptLimiter(AttemptLimiterConfig{