diff --git a/controlplane/admin/alb_oidc.go b/controlplane/admin/alb_oidc.go new file mode 100644 index 00000000..aa323c6c --- /dev/null +++ b/controlplane/admin/alb_oidc.go @@ -0,0 +1,221 @@ +//go:build kubernetes + +package admin + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/json" + "encoding/pem" + "fmt" + "io" + "net/http" + "regexp" + "strings" + "sync" + "time" +) + +// ALBOIDCVerifier verifies the X-Amzn-Oidc-Data JWT that an AWS ALB injects +// after a successful Cognito authentication. +// +// The verifier checks the ES256 signature against the ALB's regional public +// key. AWS publishes these keys at a fixed HTTPS endpoint per region. The +// verifier also checks the exp, signer, iss, and client claims. A request +// that never crossed the ALB cannot produce a JWT that passes these checks. +// +// This verification is the trust boundary for operator identity. Earlier +// versions trusted the header without a signature check. That trusted the +// network path instead: any caller that reached the pod directly could forge +// the header. +type ALBOIDCVerifier struct { + region string + issuer string + clientID string + + // keyURL builds the public-key URL for a kid. Tests override it. + keyURL func(kid string) string + // httpClient fetches public keys. Tests override it. + httpClient *http.Client + // now returns the current time. Tests override it. + now func() time.Time + + mu sync.Mutex + keys map[string]*ecdsa.PublicKey +} + +// albKeyIDPattern restricts kid values before they enter a URL path. +// AWS key IDs contain only these characters. +var albKeyIDPattern = regexp.MustCompile(`^[A-Za-z0-9_-]{1,128}$`) + +// NewALBOIDCVerifier returns a verifier for ALB OIDC JWTs. +// +// region is the AWS region of the ALB. The verifier uses it for the public +// key endpoint and the signer claim prefix. issuer is the expected iss claim. +// clientID is the expected client claim. Empty issuer or clientID skips that +// check. Keep both set in production. +func NewALBOIDCVerifier(region, issuer, clientID string) (*ALBOIDCVerifier, error) { + if region == "" { + return nil, fmt.Errorf("ALB OIDC verifier requires an AWS region") + } + v := &ALBOIDCVerifier{ + region: region, + issuer: issuer, + clientID: clientID, + now: time.Now, + } + v.keyURL = func(kid string) string { + return fmt.Sprintf("https://public-keys.auth.elb.%s.amazonaws.com/%s", region, kid) + } + v.httpClient = &http.Client{Timeout: 5 * time.Second} + return v, nil +} + +// Verify checks the signature and claims of an X-Amzn-Oidc-Data JWT. It +// returns the claims on success. +func (v *ALBOIDCVerifier) Verify(token string) (map[string]any, error) { + parts := strings.Split(token, ".") + if len(parts) != 3 { + return nil, errMalformedJWT + } + headerBytes, err := base64.RawURLEncoding.DecodeString(parts[0]) + if err != nil { + headerBytes, err = base64.URLEncoding.DecodeString(parts[0]) + if err != nil { + return nil, errMalformedJWT + } + } + var header struct { + Alg string `json:"alg"` + Kid string `json:"kid"` + } + if err := json.Unmarshal(headerBytes, &header); err != nil { + return nil, errMalformedJWT + } + // The ALB signs with ES256. Reject any other algorithm. This prevents + // algorithm-confusion attacks. + if header.Alg != "ES256" { + return nil, &jwtError{"unexpected JWT algorithm"} + } + if !albKeyIDPattern.MatchString(header.Kid) { + return nil, &jwtError{"invalid JWT key id"} + } + + key, err := v.publicKey(header.Kid) + if err != nil { + return nil, fmt.Errorf("fetch ALB public key: %w", err) + } + + sig, err := base64.RawURLEncoding.DecodeString(parts[2]) + if err != nil { + sig, err = base64.URLEncoding.DecodeString(parts[2]) + if err != nil { + return nil, errMalformedJWT + } + } + digest := sha256.Sum256([]byte(parts[0] + "." + parts[1])) + if !ecdsa.VerifyASN1(key, digest[:], sig) { + return nil, &jwtError{"JWT signature verification failed"} + } + + claims, err := decodeJWTClaims(token) + if err != nil { + return nil, err + } + if err := v.checkClaims(claims); err != nil { + return nil, err + } + return claims, nil +} + +// checkClaims validates the registered claims of a verified JWT. +func (v *ALBOIDCVerifier) checkClaims(claims map[string]any) error { + // The ALB always sets exp. Reject tokens without it. + exp, ok := claims["exp"].(float64) + if !ok { + return &jwtError{"missing exp claim"} + } + // Allow 60 seconds of clock skew between the ALB and this process. + if time.Unix(int64(exp), 0).Add(60 * time.Second).Before(v.now()) { + return &jwtError{"JWT expired"} + } + // The signer claim names the ALB that signed the JWT. It must be an ALB + // in the configured region. This blocks JWTs that a different ALB signed. + signer := stringClaim(claims, "signer") + expectedPrefix := "arn:aws:elasticloadbalancing:" + v.region + ":" + if !strings.HasPrefix(signer, expectedPrefix) { + return &jwtError{"unexpected JWT signer"} + } + if v.issuer != "" && stringClaim(claims, "iss") != v.issuer { + return &jwtError{"unexpected JWT issuer"} + } + if v.clientID != "" && stringClaim(claims, "client") != v.clientID { + return &jwtError{"unexpected JWT client"} + } + return nil +} + +// publicKey returns the cached key for kid. It fetches the key from AWS on +// a cache miss. A forged JWT with an unknown kid causes one fetch per kid. +// The kid pattern and the per-kid cache bound that fetch rate. +func (v *ALBOIDCVerifier) publicKey(kid string) (*ecdsa.PublicKey, error) { + v.mu.Lock() + if v.keys == nil { + v.keys = make(map[string]*ecdsa.PublicKey) + } + if key, ok := v.keys[kid]; ok { + v.mu.Unlock() + return key, nil + } + v.mu.Unlock() + + key, err := v.fetchKey(kid) + if err != nil { + return nil, err + } + + v.mu.Lock() + v.keys[kid] = key + v.mu.Unlock() + return key, nil +} + +// fetchKey downloads the PEM-encoded public key for kid from AWS. +func (v *ALBOIDCVerifier) fetchKey(kid string) (*ecdsa.PublicKey, error) { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, v.keyURL(kid), nil) + if err != nil { + return nil, err + } + resp, err := v.httpClient.Do(req) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("key endpoint returned %s", resp.Status) + } + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return nil, err + } + block, _ := pem.Decode(body) + if block == nil { + return nil, fmt.Errorf("key endpoint returned no PEM block") + } + parsed, err := x509.ParsePKIXPublicKey(block.Bytes) + if err != nil { + return nil, fmt.Errorf("parse public key: %w", err) + } + key, ok := parsed.(*ecdsa.PublicKey) + if !ok { + return nil, fmt.Errorf("public key is not ECDSA") + } + if key.Curve != elliptic.P256() { + return nil, fmt.Errorf("public key is not P-256") + } + return key, nil +} diff --git a/controlplane/admin/alb_oidc_test.go b/controlplane/admin/alb_oidc_test.go new file mode 100644 index 00000000..455c3e2b --- /dev/null +++ b/controlplane/admin/alb_oidc_test.go @@ -0,0 +1,161 @@ +//go:build kubernetes + +package admin + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/json" + "encoding/pem" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" +) + +// testALBSigningKey generates a throwaway P-256 key for signed test JWTs. +func testALBSigningKey(t *testing.T) *ecdsa.PrivateKey { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate test key: %v", err) + } + return key +} + +// testALBVerifier returns a verifier whose key endpoint is a local test +// server that serves the public half of key for any kid. +func testALBVerifier(t *testing.T, key *ecdsa.PrivateKey, issuer, clientID string) *ALBOIDCVerifier { + t.Helper() + der, err := x509.MarshalPKIXPublicKey(&key.PublicKey) + if err != nil { + t.Fatalf("marshal test public key: %v", err) + } + pemBytes := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: der}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(pemBytes) + })) + t.Cleanup(srv.Close) + + v, err := NewALBOIDCVerifier("us-east-1", issuer, clientID) + if err != nil { + t.Fatalf("NewALBOIDCVerifier: %v", err) + } + v.keyURL = func(kid string) string { return srv.URL + "/" + url.PathEscape(kid) } + v.httpClient = srv.Client() + return v +} + +// signedOIDC builds an ES256-signed JWT with the given claims, in the +// X-Amzn-Oidc-Data wire format (header.payload.signature). +func signedOIDC(t *testing.T, key *ecdsa.PrivateKey, claims map[string]any) string { + t.Helper() + headerSeg := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"ES256","kid":"test-kid","typ":"JWT"}`)) + payload, err := json.Marshal(claims) + if err != nil { + t.Fatalf("marshal claims: %v", err) + } + payloadSeg := base64.RawURLEncoding.EncodeToString(payload) + digest := sha256.Sum256([]byte(headerSeg + "." + payloadSeg)) + sig, err := ecdsa.SignASN1(rand.Reader, key, digest[:]) + if err != nil { + t.Fatalf("sign test JWT: %v", err) + } + return headerSeg + "." + payloadSeg + "." + base64.RawURLEncoding.EncodeToString(sig) +} + +// validALBClaims returns claims that pass the verifier's claim checks for a +// verifier built with issuer testALBIssuer and clientID testALBClientID. +// Callers override individual keys to build rejection cases. +const ( + testALBIssuer = "https://cognito-idp.us-east-1.amazonaws.com/us-east-1_TEST" + testALBClientID = "test-client-id" +) + +func validALBClaims(overrides map[string]any) map[string]any { + claims := map[string]any{ + "exp": float64(time.Now().Add(time.Hour).Unix()), + "signer": "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/internal/test/abc", + "iss": testALBIssuer, + "client": testALBClientID, + } + for k, v := range overrides { + claims[k] = v + } + return claims +} + +func TestALBOIDCVerifierAcceptsValidToken(t *testing.T) { + key := testALBSigningKey(t) + v := testALBVerifier(t, key, testALBIssuer, testALBClientID) + claims, err := v.Verify(signedOIDC(t, key, validALBClaims(map[string]any{"email": "a@posthog.com"}))) + if err != nil { + t.Fatalf("Verify: %v", err) + } + if claims["email"] != "a@posthog.com" { + t.Fatalf("email = %v", claims["email"]) + } +} + +func TestALBOIDCVerifierRejections(t *testing.T) { + key := testALBSigningKey(t) + otherKey := testALBSigningKey(t) + v := testALBVerifier(t, key, testALBIssuer, testALBClientID) + + wrongSig := signedOIDC(t, otherKey, validALBClaims(nil)) + + tampered := signedOIDC(t, key, validALBClaims(nil)) + // Replace the payload with attacker claims, keep the original signature. + forgedPayload, _ := json.Marshal(validALBClaims(map[string]any{"email": "admin@posthog.com", "role": "admin"})) + parts := splitForTest(t, tampered) + tampered = parts[0] + "." + base64.RawURLEncoding.EncodeToString(forgedPayload) + "." + parts[2] + + cases := []struct { + name string + token string + }{ + {"signature from wrong key", wrongSig}, + {"tampered payload", tampered}, + {"unsigned legacy token", mkUnsignedOIDC(map[string]any{"email": "a@posthog.com"})}, + {"expired", signedOIDC(t, key, validALBClaims(map[string]any{"exp": float64(time.Now().Add(-time.Hour).Unix())}))}, + {"missing exp", signedOIDC(t, key, map[string]any{"signer": "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/internal/test/abc", "iss": testALBIssuer, "client": testALBClientID})}, + {"signer in another region", signedOIDC(t, key, validALBClaims(map[string]any{"signer": "arn:aws:elasticloadbalancing:eu-central-1:123456789012:loadbalancer/internal/test/abc"}))}, + {"wrong issuer", signedOIDC(t, key, validALBClaims(map[string]any{"iss": "https://cognito-idp.us-east-1.amazonaws.com/us-east-1_OTHER"}))}, + {"wrong client", signedOIDC(t, key, validALBClaims(map[string]any{"client": "other-client"}))}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if _, err := v.Verify(tc.token); err == nil { + t.Fatal("Verify accepted a token it must reject") + } + }) + } +} + +// splitForTest splits a JWT into its three segments. +func splitForTest(t *testing.T, token string) [3]string { + t.Helper() + var out [3]string + start := 0 + for i := 0; i < 2; i++ { + idx := -1 + for j := start; j < len(token); j++ { + if token[j] == '.' { + idx = j + break + } + } + if idx < 0 { + t.Fatalf("token has fewer than 3 segments") + } + out[i] = token[start:idx] + start = idx + 1 + } + out[2] = token[start:] + return out +} diff --git a/controlplane/admin/authz.go b/controlplane/admin/authz.go index b97c88d1..4b4aee8c 100644 --- a/controlplane/admin/authz.go +++ b/controlplane/admin/authz.go @@ -30,13 +30,15 @@ const ( // albOIDCDataHeader is the signed JWT the AWS ALB injects after a // successful Cognito (Google Workspace) authentication. It carries the - // user's claims (email, groups). The ALB strips any client-supplied copy, - // so on an internal-scheme LB reachable only over the tailnet it is the - // trust boundary for operator identity. + // user's claims (email, groups). The ALB strips any client-supplied copy. + // The admin layer verifies the JWT signature before trusting it, because + // the pod is also reachable without crossing the ALB (same-namespace pods, + // kubectl port-forward). + // + // The companion X-Amzn-Oidc-Identity header (a bare email string) is + // deliberately not read: it carries no signature, so it is forgeable by + // any caller that bypasses the ALB. albOIDCDataHeader = "X-Amzn-Oidc-Data" - // albOIDCIdentityHeader carries just the subject/email; used as a fallback - // when the full data JWT is absent. - albOIDCIdentityHeader = "X-Amzn-Oidc-Identity" ) // ssoEmailDomain is the only email domain accepted on the SSO path. SSO emails @@ -74,7 +76,13 @@ func IdentityFromContext(c *gin.Context) *Identity { // path and always maps to admin. Otherwise the ALB-injected Cognito JWT yields // the caller's email, and resolve (the operators-table lookup) maps that email // to a Role. Unauthenticated requests are rejected 401. -func AuthMiddleware(tokens TokenSet, resolve RoleResolver) gin.HandlerFunc { +// +// The verifier checks the ALB OIDC JWT signature. A nil verifier disables the +// SSO path entirely: SSO headers are ignored and only bearer tokens +// authenticate. The control plane fails startup when SSO is configured without +// a verifier, so a nil verifier means the operator deliberately runs without +// SSO. +func AuthMiddleware(tokens TokenSet, resolve RoleResolver, verifier *ALBOIDCVerifier) gin.HandlerFunc { return func(c *gin.Context) { // 1. Internal secret (header or login cookie) -> admin (break-glass). if tokens.Valid(requestAdminToken(c)) { @@ -82,8 +90,12 @@ func AuthMiddleware(tokens TokenSet, resolve RoleResolver) gin.HandlerFunc { c.Next() return } - // 2. ALB/Cognito SSO identity: extract the email, then resolve its role. - email := emailFromOIDC(c) + // 2. ALB/Cognito SSO identity: verify the JWT, extract the email, then + // resolve its role. + email := "" + if verifier != nil { + email = emailFromOIDC(c, verifier) + } if email == "" { c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "not authenticated"}) return @@ -97,33 +109,25 @@ func AuthMiddleware(tokens TokenSet, resolve RoleResolver) gin.HandlerFunc { } } -// emailFromOIDC extracts the caller's email from the ALB OIDC data JWT (falling -// back to the `sub` claim, then to the identity-only header). It returns "" when -// no usable SSO identity is present OR when the email fails domain hardening. +// emailFromOIDC extracts the caller's email from a signature-verified ALB OIDC +// data JWT. It returns "" when no usable SSO identity is present OR when the +// email fails domain hardening. +// +// The X-Amzn-Oidc-Identity header is never consulted. It is an unsigned bare +// email string, so any caller that bypasses the ALB could forge it. // // Domain hardening: only @posthog.com emails are accepted, and a JWT // email_verified claim that is explicitly false rejects the identity. This is // defense in depth on top of the ALB/Cognito allow-list — a stray non-corporate // or unverified identity never becomes a logged-in (even viewer) caller. -// -// The JWT signature is NOT verified here: the request only reaches this pod via -// the internal-scheme ALB (which signs and injects the header and strips -// client-supplied copies) over a tailnet-restricted network. Verifying the -// ALB's regional public key by `kid` is a hardening follow-up tracked in the -// design doc. -func emailFromOIDC(c *gin.Context) string { +func emailFromOIDC(c *gin.Context, verifier *ALBOIDCVerifier) string { raw := c.GetHeader(albOIDCDataHeader) if raw == "" { - // Fallback: identity-only header (email/subject). The data JWT is absent, - // so there is no email_verified claim to consult — domain check still applies. - if email := c.GetHeader(albOIDCIdentityHeader); acceptableSSOEmail(email, true) { - return strings.ToLower(strings.TrimSpace(email)) - } return "" } - claims, err := decodeJWTClaims(raw) + claims, err := verifier.Verify(raw) if err != nil { - slog.Warn("admin: failed to decode ALB OIDC data header", "error", err) + slog.Warn("admin: rejected ALB OIDC data header", "error", err) return "" } email := stringClaim(claims, "email") diff --git a/controlplane/admin/authz_test.go b/controlplane/admin/authz_test.go index 02af5ca0..a59427d3 100644 --- a/controlplane/admin/authz_test.go +++ b/controlplane/admin/authz_test.go @@ -3,6 +3,7 @@ package admin import ( + "crypto/ecdsa" "encoding/base64" "encoding/json" "net/http" @@ -12,14 +13,34 @@ import ( "github.com/gin-gonic/gin" ) -// mkOIDC builds a fake ALB x-amzn-oidc-data JWT (header.payload.sig) with the -// given claims. Only the payload segment is read by the decoder. -func mkOIDC(claims map[string]any) string { +// mkUnsignedOIDC builds a fake ALB x-amzn-oidc-data JWT (header.payload.sig) +// with the given claims. It carries no valid signature. The verifier rejects +// it; tests use it to prove that. +func mkUnsignedOIDC(claims map[string]any) string { payload, _ := json.Marshal(claims) seg := base64.RawURLEncoding.EncodeToString(payload) return "eyJ0eXAiOiJKV1QifQ." + seg + ".sig" } +// testSSO wires a signed-JWT SSO path for middleware tests. +type testSSO struct { + key *ecdsa.PrivateKey + verifier *ALBOIDCVerifier +} + +func newTestSSO(t *testing.T) *testSSO { + t.Helper() + key := testALBSigningKey(t) + return &testSSO{key: key, verifier: testALBVerifier(t, key, testALBIssuer, testALBClientID)} +} + +// oidc returns a signed JWT that the test verifier accepts, with the given +// claim overrides (typically "email"). +func (s *testSSO) oidc(t *testing.T, overrides map[string]any) string { + t.Helper() + return signedOIDC(t, s.key, validALBClaims(overrides)) +} + // roleByEmail builds a fake RoleResolver from an email→role map. Unknown // emails resolve to viewer (the production fail-closed default). func roleByEmail(admins ...string) RoleResolver { @@ -38,7 +59,7 @@ func roleByEmail(admins ...string) RoleResolver { func TestAuthMiddlewareInternalSecretIsAdmin(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), roleByEmail()), func(c *gin.Context) { + r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), roleByEmail(), nil), func(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"role": IdentityFromContext(c).Role}) }) req := httptest.NewRequest(http.MethodGet, "/x", nil) @@ -57,6 +78,7 @@ func TestAuthMiddlewareInternalSecretIsAdmin(t *testing.T) { // production, the operators-table lookup). Unknown emails fail closed to viewer. func TestAuthMiddlewareSSORoleMapping(t *testing.T) { gin.SetMode(gin.TestMode) + sso := newTestSSO(t) resolve := roleByEmail("a@posthog.com") cases := []struct { name string @@ -70,11 +92,11 @@ func TestAuthMiddlewareSSORoleMapping(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { r := gin.New() - r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), resolve), func(c *gin.Context) { + r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), resolve, sso.verifier), func(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"role": IdentityFromContext(c).Role}) }) req := httptest.NewRequest(http.MethodGet, "/x", nil) - req.Header.Set(albOIDCDataHeader, mkOIDC(map[string]any{"email": tc.email})) + req.Header.Set(albOIDCDataHeader, sso.oidc(t, map[string]any{"email": tc.email})) rec := httptest.NewRecorder() r.ServeHTTP(rec, req) if rec.Code != http.StatusOK { @@ -93,6 +115,7 @@ func TestAuthMiddlewareSSORoleMapping(t *testing.T) { // rejected too. func TestAuthMiddlewareDomainHardening(t *testing.T) { gin.SetMode(gin.TestMode) + sso := newTestSSO(t) // Resolver returns admin for everything to prove rejection is upstream of it. resolve := func(string) Role { return RoleAdmin } for _, tc := range []struct { @@ -104,11 +127,11 @@ func TestAuthMiddlewareDomainHardening(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { r := gin.New() - r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), resolve), func(c *gin.Context) { + r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), resolve, sso.verifier), func(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"ok": true}) }) req := httptest.NewRequest(http.MethodGet, "/x", nil) - req.Header.Set(albOIDCDataHeader, mkOIDC(tc.claims)) + req.Header.Set(albOIDCDataHeader, sso.oidc(t, tc.claims)) rec := httptest.NewRecorder() r.ServeHTTP(rec, req) if rec.Code != http.StatusUnauthorized { @@ -118,16 +141,75 @@ func TestAuthMiddlewareDomainHardening(t *testing.T) { } } +// A forged SSO header never authenticates: unsigned JWTs, JWTs signed by the +// wrong key, and the unsigned identity-only header all fail with 401. This is +// the regression test for the header-forgery finding: the pod is reachable +// without crossing the ALB, so the signature is the trust boundary. +func TestAuthMiddlewareRejectsForgedSSO(t *testing.T) { + gin.SetMode(gin.TestMode) + sso := newTestSSO(t) + // Resolver returns admin for everything to prove rejection is upstream of it. + resolve := func(string) Role { return RoleAdmin } + + otherKey := testALBSigningKey(t) + + cases := []struct { + name string + header string + value string + }{ + {"unsigned data JWT", albOIDCDataHeader, mkUnsignedOIDC(map[string]any{"email": "a@posthog.com"})}, + {"wrong-key signature", albOIDCDataHeader, signedOIDC(t, otherKey, validALBClaims(map[string]any{"email": "a@posthog.com"}))}, + {"identity-only header", "X-Amzn-Oidc-Identity", "a@posthog.com"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := gin.New() + r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), resolve, sso.verifier), func(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"ok": true}) + }) + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.Header.Set(tc.header, tc.value) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401", rec.Code) + } + }) + } +} + +// Without a verifier, SSO headers are ignored entirely: only bearer tokens +// authenticate. This is the fail-closed default for deployments without an +// ALB. +func TestAuthMiddlewareWithoutVerifierIgnoresSSO(t *testing.T) { + gin.SetMode(gin.TestMode) + sso := newTestSSO(t) + resolve := func(string) Role { return RoleAdmin } + r := gin.New() + r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), resolve, nil), func(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"ok": true}) + }) + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.Header.Set(albOIDCDataHeader, sso.oidc(t, map[string]any{"email": "a@posthog.com"})) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401", rec.Code) + } +} + // RequireAdmin gates a route regardless of method (used by the audit read). func TestRequireAdmin(t *testing.T) { gin.SetMode(gin.TestMode) + sso := newTestSSO(t) resolve := roleByEmail("a@posthog.com") r := gin.New() - r.GET("/audit", AuthMiddleware(NewTokenSet("secret", nil), resolve), RequireAdmin(), func(c *gin.Context) { + r.GET("/audit", AuthMiddleware(NewTokenSet("secret", nil), resolve, sso.verifier), RequireAdmin(), func(c *gin.Context) { c.Status(http.StatusOK) }) - viewer := mkOIDC(map[string]any{"email": "v@posthog.com"}) - admin := mkOIDC(map[string]any{"email": "a@posthog.com"}) + viewer := sso.oidc(t, map[string]any{"email": "v@posthog.com"}) + admin := sso.oidc(t, map[string]any{"email": "a@posthog.com"}) for _, tc := range []struct { name, oidc string want int @@ -150,7 +232,7 @@ func TestRequireAdmin(t *testing.T) { func TestAuthMiddlewareRejectsUnauthenticated(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), roleByEmail()), func(c *gin.Context) { + r.GET("/x", AuthMiddleware(NewTokenSet("secret", nil), roleByEmail(), nil), func(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"ok": true}) }) req := httptest.NewRequest(http.MethodGet, "/x", nil) @@ -164,11 +246,12 @@ func TestAuthMiddlewareRejectsUnauthenticated(t *testing.T) { // RoleGate: viewers can GET but not mutate; the audit log GET is admin-only. func TestRoleGate(t *testing.T) { gin.SetMode(gin.TestMode) + sso := newTestSSO(t) resolve := roleByEmail("a@posthog.com") build := func() *gin.Engine { r := gin.New() grp := r.Group("/api/v1", - AuthMiddleware(NewTokenSet("secret", nil), resolve), + AuthMiddleware(NewTokenSet("secret", nil), resolve, sso.verifier), RoleGate("/api/v1/audit"), ) grp.GET("/orgs", func(c *gin.Context) { c.Status(http.StatusOK) }) @@ -176,8 +259,8 @@ func TestRoleGate(t *testing.T) { grp.GET("/audit", func(c *gin.Context) { c.Status(http.StatusOK) }) return r } - viewer := mkOIDC(map[string]any{"email": "v@posthog.com"}) - admin := mkOIDC(map[string]any{"email": "a@posthog.com"}) + viewer := sso.oidc(t, map[string]any{"email": "v@posthog.com"}) + admin := sso.oidc(t, map[string]any{"email": "a@posthog.com"}) cases := []struct { name, method, path, oidc string diff --git a/controlplane/admin/dashboard_test.go b/controlplane/admin/dashboard_test.go index ff90c5f8..ac1531b4 100644 --- a/controlplane/admin/dashboard_test.go +++ b/controlplane/admin/dashboard_test.go @@ -226,7 +226,7 @@ func TestLoginWithFallbackTokenWorksEndToEnd(t *testing.T) { RegisterLogin(r, tokens) // A cookie-authenticated request must be accepted by AuthMiddleware (and // mapped to the admin role via the internal-secret path). - r.GET("/api/v1/ping", AuthMiddleware(tokens, nil), func(c *gin.Context) { + r.GET("/api/v1/ping", AuthMiddleware(tokens, nil, nil), func(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"role": IdentityFromContext(c).Role}) }) diff --git a/controlplane/admin_sso.go b/controlplane/admin_sso.go new file mode 100644 index 00000000..00a82ea8 --- /dev/null +++ b/controlplane/admin_sso.go @@ -0,0 +1,40 @@ +//go:build kubernetes + +package controlplane + +import ( + "fmt" + "log/slog" + "os" + + "github.com/posthog/duckgres/controlplane/admin" +) + +// adminSSOVerifierFromEnv builds the ALB OIDC verifier from the environment. +// It returns a nil verifier when DUCKGRES_ADMIN_SSO_ISSUER is unset. A nil +// verifier disables the SSO path: AuthMiddleware then ignores SSO headers and +// accepts only bearer tokens. +func adminSSOVerifierFromEnv(defaultRegion string) (*admin.ALBOIDCVerifier, error) { + issuer := os.Getenv("DUCKGRES_ADMIN_SSO_ISSUER") + if issuer == "" { + slog.Warn("Admin SSO is not configured (DUCKGRES_ADMIN_SSO_ISSUER unset); the admin API accepts only bearer tokens. Set the SSO envs when the admin ingress authenticates through an AWS ALB.") + return nil, nil + } + region := os.Getenv("DUCKGRES_ADMIN_SSO_REGION") + if region == "" { + region = defaultRegion + } + if region == "" { + return nil, fmt.Errorf("DUCKGRES_ADMIN_SSO_ISSUER is set but no AWS region is available: set DUCKGRES_ADMIN_SSO_REGION or DUCKGRES_AWS_REGION") + } + clientID := os.Getenv("DUCKGRES_ADMIN_SSO_CLIENT_ID") + if clientID == "" { + slog.Warn("DUCKGRES_ADMIN_SSO_CLIENT_ID is unset; the admin SSO verifier skips the client claim check. Set it to the Cognito app client ID.") + } + verifier, err := admin.NewALBOIDCVerifier(region, issuer, clientID) + if err != nil { + return nil, err + } + slog.Info("Admin SSO signature verification enabled.", "issuer", issuer, "region", region) + return verifier, nil +} diff --git a/controlplane/multitenant.go b/controlplane/multitenant.go index 31fa4029..71b8c60b 100644 --- a/controlplane/multitenant.go +++ b/controlplane/multitenant.go @@ -627,6 +627,28 @@ func SetupMultiTenant( if err != nil { return nil, nil, nil, nil, nil, nil, fmt.Errorf("init admin audit store: %w", err) } + + // Admin SSO verifier. The admin API trusts the ALB-injected OIDC JWT only + // when its ES256 signature verifies against the ALB's regional AWS public + // key. Without this check, any caller that reaches the pod directly + // (same-namespace pod, kubectl port-forward) can forge the header and + // claim any operator's identity. + // + // Configuration is env-only, matching the other K8s admin knobs: + // DUCKGRES_ADMIN_SSO_ISSUER — expected iss claim (the Cognito user + // pool URL). Set it to enable SSO. + // DUCKGRES_ADMIN_SSO_CLIENT_ID — expected client claim (the Cognito app + // client ID). Recommended. + // DUCKGRES_ADMIN_SSO_REGION — ALB region. Falls back to + // DUCKGRES_AWS_REGION (cfg.K8s.AWSRegion). + // + // When the issuer is unset, the SSO path is off and only bearer tokens + // authenticate. That state is deliberate for local runs, so it logs a + // warning instead of failing startup. + ssoVerifier, err := adminSSOVerifierFromEnv(cfg.K8s.AWSRegion) + if err != nil { + return nil, nil, nil, nil, nil, nil, err + } metricsProxy := admin.NewMetricsProxy(os.Getenv("DUCKGRES_PROMETHEUS_URL")) clusterInfo := &clusterInfoProvider{ router: router, @@ -656,7 +678,7 @@ func SetupMultiTenant( // RoleGate blocks viewer mutations (method-based); the audit-log read // self-gates via RequireAdmin at its route (no brittle path coupling here). api := engine.Group("/api/v1", - admin.AuthMiddleware(adminTokens, resolve), + admin.AuthMiddleware(adminTokens, resolve, ssoVerifier), admin.AuditMiddleware(auditStore), admin.RoleGate(), ) diff --git a/docs/design/admin-ui.md b/docs/design/admin-ui.md index 50658c5a..8fc64f2f 100644 --- a/docs/design/admin-ui.md +++ b/docs/design/admin-ui.md @@ -64,9 +64,13 @@ the proxy is not an open PromQL relay. Org-labelled metrics we expose: `duckgres_scan_*{org}`. Fleet (no org label): worker lifecycle/spawn/reap/queue/cap-drift. ### 3. RBAC + audit — `authz.go`, `audit.go` -- `AuthMiddleware`: decode `x-amzn-oidc-data` (ALB-signed JWT; verify via the ALB public - key endpoint, cache keys — hardening follow-up) → email. Only `@posthog.com` + - `email_verified != false` is accepted, else 401. The role is resolved per-request from +- `AuthMiddleware`: verify `x-amzn-oidc-data` (ALB-signed JWT) via the ALB regional + public-key endpoint (`alb_oidc.go`, keys cached per `kid`) — signature, `exp`, + `signer` (must be an ALB ARN in the configured region), `iss` + (`DUCKGRES_ADMIN_SSO_ISSUER`), `client` (`DUCKGRES_ADMIN_SSO_CLIENT_ID`). The + unsigned `x-amzn-oidc-identity` header is never trusted. Without + `DUCKGRES_ADMIN_SSO_ISSUER` the SSO path is off and only bearer tokens + authenticate. Only `@posthog.com` + `email_verified != false` is accepted, else 401. The role is resolved per-request from the `duckgres_operators` config-schema table (goose migration `000006_create_operators.sql`): an `admin` row → `admin`, else (including no row) → `viewer`. Admins manage operators under **Admin → Operators** diff --git a/server/conn_pg_stat_activity.go b/server/conn_pg_stat_activity.go index 3232875e..dbb8e2a7 100644 --- a/server/conn_pg_stat_activity.go +++ b/server/conn_pg_stat_activity.go @@ -114,16 +114,22 @@ func (c *clientConn) handlePgStatActivityExtended(p *portal) { func (c *clientConn) visiblePgStatActivityConns() []*clientConn { conns := c.server.listConns() - if c.queryAccessPolicy == nil { - return conns - } - visible := make([]*clientConn, 0, len(conns)) for _, conn := range conns { - // Cluster project readers have one distinct username per project. - if conn.orgID == c.orgID && conn.username == c.username { - visible = append(visible, conn) + // The multitenant control plane registers every org's connections in + // one process-wide map. Scope rows to the caller's org so a full-power + // org principal cannot read another org's query text, usernames, or + // client addresses. Standalone mode leaves orgID empty on every + // connection, so this filter passes all rows there. + if conn.orgID != c.orgID { + continue + } + // Project-scoped users see only their own connections. Cluster project + // readers have one distinct username per project. + if c.queryAccessPolicy != nil && conn.username != c.username { + continue } + visible = append(visible, conn) } return visible } diff --git a/server/conn_test.go b/server/conn_test.go index d5a10c56..460084a3 100644 --- a/server/conn_test.go +++ b/server/conn_test.go @@ -3902,6 +3902,45 @@ func TestSetQueryAccessPolicyDisablesPassthrough(t *testing.T) { } } +// An org's full-power principal (nil QueryAccessPolicy) sees its own org's +// connections in pg_stat_activity, never another org's. On the multitenant +// control plane every org shares one process-wide connection registry. +func TestPgStatActivityOrgRootSeesOnlyOwnOrg(t *testing.T) { + srv := &Server{conns: make(map[int32]*clientConn)} + root := &clientConn{server: srv, username: "root", orgID: "org-a", pid: 100} + sameOrg := &clientConn{username: "svc-reporting", orgID: "org-a", pid: 101} + otherOrg := &clientConn{username: "root", orgID: "org-b", pid: 102} + for _, conn := range []*clientConn{root, sameOrg, otherOrg} { + srv.registerConn(conn) + } + + visible := root.visiblePgStatActivityConns() + pids := map[int32]bool{} + for _, conn := range visible { + pids[conn.pid] = true + } + if len(visible) != 2 || !pids[100] || !pids[101] { + t.Fatalf("org root should see exactly its own org's connections, got pids %v", pids) + } + if pids[102] { + t.Fatal("org root saw another org's connection in pg_stat_activity") + } +} + +// Standalone mode leaves orgID empty on every connection. The org filter +// passes all rows there, preserving the single-tenant behavior. +func TestPgStatActivityStandaloneSeesAll(t *testing.T) { + srv := &Server{conns: make(map[int32]*clientConn)} + first := &clientConn{server: srv, username: "postgres", pid: 100} + second := &clientConn{username: "postgres", pid: 101} + for _, conn := range []*clientConn{first, second} { + srv.registerConn(conn) + } + if got := len(first.visiblePgStatActivityConns()); got != 2 { + t.Fatalf("standalone connection should see all %d connections, got %d", 2, got) + } +} + func TestConnectionRegistry(t *testing.T) { srv := &Server{ conns: make(map[int32]*clientConn), diff --git a/tests/mw-dev/e2e/harness.sh b/tests/mw-dev/e2e/harness.sh index 616f29e8..fa99bf08 100755 --- a/tests/mw-dev/e2e/harness.sh +++ b/tests/mw-dev/e2e/harness.sh @@ -3176,25 +3176,22 @@ copy_active_and_survives_idle() { # org password fail "copy_active: streaming COPY (~70s) failed all $attempt attempts with transient stream errors — likely a real re-arm/reaper regression (last: $last)" } -# ---- admin RBAC: SSO viewer is read-only ----------------------------------- -# A forged ALB OIDC header (no internal secret) for an @posthog.com email that -# is NOT in the operators table resolves to the viewer role (fail-closed -# default): it can read but must be blocked from mutations and from the audit -# log. Exercises RoleGate + RequireAdmin against the REAL router (the unit tests -# cover the gate algorithm; this covers the wiring). The JWT is unsigned because -# the CP trusts the ALB-injected header by network position. (Role is no longer -# group-based — it comes from the operators table; an unknown email = viewer.) +# ---- admin SSO: forged headers are rejected --------------------------------- +# The admin API verifies the ALB OIDC JWT signature. An unsigned JWT, and the +# unsigned identity-only header, must both get 401 — the pod is reachable +# without crossing the ALB (same-namespace pods, kubectl port-forward), so the +# signature is the trust boundary. The viewer RoleGate wiring stays covered by +# the unit tests (TestAuthMiddlewareSSORoleMapping, TestRoleGate); the real +# ALB JWT is not available to this harness. admin_rbac_viewer() { # org org="$1" - log "admin RBAC: forged SSO viewer (unknown operator) is read-only (no mutate, no audit)" + log "admin SSO: forged OIDC headers are rejected (401)" payload="$(printf '{"email":"ci-viewer@posthog.com","email_verified":true}' | base64 -w0 | tr '+/' '-_' | tr -d '=')" vh="X-Amzn-Oidc-Data: e30.${payload}.sig" code="$(curl -s -o /dev/null -w '%{http_code}' -H "$vh" "$API/api/v1/orgs")" - [ "$code" = "200" ] || fail "viewer GET /orgs returned $code, want 200 (reads allowed)" - code="$(curl -s -o /dev/null -w '%{http_code}' -H "$vh" "$API/api/v1/audit")" - [ "$code" = "403" ] || fail "viewer GET /audit returned $code, want 403 (audit is admin-only)" - code="$(curl -s -o /dev/null -w '%{http_code}' -X PUT -H "$vh" -H 'Content-Type: application/json' -d '{}' "$API/api/v1/orgs/$org")" - [ "$code" = "403" ] || fail "viewer PUT /orgs/$org returned $code, want 403 (mutations are admin-only)" + [ "$code" = "401" ] || fail "forged X-Amzn-Oidc-Data GET /orgs returned $code, want 401" + code="$(curl -s -o /dev/null -w '%{http_code}' -H "X-Amzn-Oidc-Identity: ci-viewer@posthog.com" "$API/api/v1/orgs")" + [ "$code" = "401" ] || fail "forged X-Amzn-Oidc-Identity GET /orgs returned $code, want 401" } # ---- instance-invalidation guard: no spurious retirement --------------------