diff --git a/SCOPE.md b/SCOPE.md index 6d57d07..a2f5b4c 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -58,9 +58,11 @@ Keycloak interchangeability are not established. - The authorization-code grant now binds the redirect URI, enforces grant-type eligibility, authenticates confidential clients and consumes codes atomically, and UserInfo enforces algorithm, issuer and - access-token purpose (KEY-WP-0016). Upstream provider tokens from Authelia are - still accepted on a transport-trust assumption without signature or - issuer/audience verification; that gap remains open. Enforcing these bindings + access-token purpose (KEY-WP-0016). Upstream Authelia ID tokens are verified + before any claim is trusted — signature against the provider's published keys, + advertised issuer, own client ID in the audience, validity window — failing + closed and independently of transport (KEY-WP-0019). KeyCape deliberately does + not check or gate on the upstream transport. Enforcing these bindings is not complete profile conformance. Relying parties must repeat `redirect_uri` on the token exchange and present the access token, not the ID token, to `/userinfo`; see [authorization-code bindings](docs/authorization-code-bindings.md). diff --git a/bin/keycape b/bin/keycape new file mode 100755 index 0000000..8f3a58c Binary files /dev/null and b/bin/keycape differ diff --git a/bin/keycape-to-keycloak b/bin/keycape-to-keycloak new file mode 100755 index 0000000..77b99b8 Binary files /dev/null and b/bin/keycape-to-keycloak differ diff --git a/bin/lldap-export b/bin/lldap-export new file mode 100755 index 0000000..d7db599 Binary files /dev/null and b/bin/lldap-export differ diff --git a/bin/lldap-to-ldap b/bin/lldap-to-ldap new file mode 100755 index 0000000..bc71e82 Binary files /dev/null and b/bin/lldap-to-ldap differ diff --git a/bin/validator b/bin/validator new file mode 100755 index 0000000..58a70aa Binary files /dev/null and b/bin/validator differ diff --git a/history/2026-09-05-011726-scope-intent-assessment.md b/history/2026-09-05-011726-scope-intent-assessment.md index 5eee689..30063c1 100644 --- a/history/2026-09-05-011726-scope-intent-assessment.md +++ b/history/2026-09-05-011726-scope-intent-assessment.md @@ -93,6 +93,23 @@ transport-trust assumption, which is a trust-contract decision rather than a local binding. G01 is not fully closed until that is settled, and none of this establishes complete profile conformance. +**Status 2026-09-07 (KEY-WP-0019): closed.** Upstream ID tokens are now verified +before any claim is trusted — RS256 signature against Authelia's published keys, +the issuer Authelia advertises, KeyCape's own client ID in the audience, and a +sane validity window — failing closed when the key set is unavailable or +unparseable. Provider key rotation is picked up on one refresh without a restart. + +The operator decision (2026-09-07) was to verify the token rather than enforce +the transport: the hop is to be HTTPS as defence in depth, but KeyCape does not +monitor, check or gate on that, because a transport check helps only when it is +configured correctly, which is the assumption it was meant to remove. +Verification holds regardless of how the token arrived, so the absence of HTTPS +validation here is a choice, not an oversight. + +Verified the tests catch the original defect: with the unverified parse restored, +all thirteen rejection cases plus the algorithm and rotation cases fail. Complete +profile conformance is still not claimed. + ### G02 — Machine-readable contract and discovery lag the runtime **Priority: high. Kind: contract drift.** diff --git a/src/internal/adapters/authelia/adapter.go b/src/internal/adapters/authelia/adapter.go index 057c303..1ae1d81 100644 --- a/src/internal/adapters/authelia/adapter.go +++ b/src/internal/adapters/authelia/adapter.go @@ -18,8 +18,9 @@ import ( // AutheliaAdapter implements domain.AuthProvider by delegating to Authelia's // OIDC endpoints. All Authelia tokens and cookies are confined to this package. type AutheliaAdapter struct { - cfg Config - client HTTPClient + cfg Config + client HTTPClient + verifier *idTokenVerifier } // New returns a production-ready AutheliaAdapter. @@ -28,7 +29,9 @@ func New(cfg Config, httpClient HTTPClient) *AutheliaAdapter { if httpClient == nil { httpClient = defaultHTTPClient } - return &AutheliaAdapter{cfg: cfg, client: httpClient} + a := &AutheliaAdapter{cfg: cfg, client: httpClient} + a.verifier = newIDTokenVerifier(cfg, httpClient, a.tokenBaseURL()) + return a } // --------------------------------------------------------------------------- @@ -86,15 +89,19 @@ func (a *AutheliaAdapter) HandleCallback(ctx context.Context, params domain.Call return nil, domain.ErrAuthFailed } - // Parse the ID token claims (no signature verification — internal service boundary). - claims, err := parseIDTokenClaims(tokenResp.IDToken) + // Verify the ID token before trusting any claim in it: signature against + // Authelia's published keys, expected issuer, our own client ID in the + // audience, and a sane validity window (KEY-WP-0019). This fails closed -- + // an unreachable or unparseable key set denies the login rather than + // falling back to unverified claims. + claims, err := a.verifier.Verify(ctx, tokenResp.IDToken) if err != nil { emitter.Emit(ctx, telemetry.Event{ Timestamp: time.Now().UTC(), EventType: telemetry.EventAuthFailure, Endpoint: "/api/oidc/token", Result: "failure", - ErrorType: "id_token_parse_error", + ErrorType: "id_token_verification_error", }) return nil, domain.ErrAuthFailed } @@ -187,9 +194,10 @@ func (a *AutheliaAdapter) tokenBaseURL() string { return a.cfg.BaseURL } -// parseIDTokenClaims extracts the JWT payload claims without verifying the -// signature. This is intentional — the token is received directly from the -// upstream OIDC provider over a server-to-server TLS connection. +// parseIDTokenClaims extracts the JWT payload claims without verifying +// anything. It is NOT part of the authentication path: HandleCallback verifies +// through idTokenVerifier. Kept for tests and diagnostics that need to read a +// token's payload without asserting it is trustworthy. func parseIDTokenClaims(idToken string) (map[string]interface{}, error) { parts := strings.Split(idToken, ".") if len(parts) != 3 { diff --git a/src/internal/adapters/authelia/adapter_test.go b/src/internal/adapters/authelia/adapter_test.go index 8035a97..644e509 100644 --- a/src/internal/adapters/authelia/adapter_test.go +++ b/src/internal/adapters/authelia/adapter_test.go @@ -2,14 +2,20 @@ package authelia_test import ( "context" + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" "encoding/base64" "encoding/json" "fmt" "io" + "math/big" "net/http" "net/url" "strings" "testing" + "time" "keycape/internal/adapters/authelia" "keycape/internal/domain" @@ -25,6 +31,16 @@ type mockHTTPClient struct { } func (m *mockHTTPClient) Do(req *http.Request) (*http.Response, error) { + // The adapter now verifies upstream ID tokens (KEY-WP-0019), so the fixture + // serves the provider metadata and signing keys. Each test keeps supplying + // its own token-endpoint behaviour through doFn. + switch { + case strings.HasSuffix(req.URL.Path, "/.well-known/openid-configuration"): + return jsonResponse(fmt.Sprintf( + `{"issuer":%q,"jwks_uri":%q}`, testIssuer, testIssuer+"/jwks.json")), nil + case strings.HasSuffix(req.URL.Path, "/jwks.json"): + return jsonResponse(testJWKS(nil)), nil + } if m.doFn != nil { return m.doFn(req) } @@ -48,17 +64,70 @@ func testConfig() authelia.Config { } } -// buildTokenResponse builds a fake token endpoint JSON response. -// The ID token is a minimal unsigned JWT (header.claims.signature) with the given claims. -func buildTokenResponse(claims map[string]interface{}) string { - header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256","typ":"JWT"}`)) - claimsJSON, _ := json.Marshal(claims) - claimsEnc := base64.RawURLEncoding.EncodeToString(claimsJSON) - idToken := header + "." + claimsEnc + ".fakesig" +const ( + testIssuer = "https://authelia.local" + testKeyID = "authelia-key-1" +) - body := fmt.Sprintf(`{"access_token":"at","token_type":"Bearer","id_token":%q}`, - idToken) - return body +// testKey signs every fixture ID token. Generated once: RSA key generation is +// slow enough to dominate this package's test time otherwise. +var testKey = func() *rsa.PrivateKey { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + panic(err) + } + return key +}() + +// testJWKS renders a JWK set for the given key under the default key id. +func testJWKS(key *rsa.PrivateKey) string { + return testJWKSKid(testKeyID, key) +} + +// testJWKSKid renders a JWK set publishing key under an explicit key id, so a +// test can model provider key rotation. +func testJWKSKid(kid string, key *rsa.PrivateKey) string { + if key == nil { + key = testKey + } + return fmt.Sprintf(`{"keys":[{"kty":"RSA","use":"sig","alg":"RS256","kid":%q,"n":%q,"e":%q}]}`, + kid, + base64.RawURLEncoding.EncodeToString(key.PublicKey.N.Bytes()), + base64.RawURLEncoding.EncodeToString(big.NewInt(int64(key.PublicKey.E)).Bytes())) +} + +// signIDToken builds a genuine RS256 token, filling in the registered-claim +// defaults a test does not care about so each case states only what it varies. +func signIDToken(claims map[string]interface{}, kid string, key *rsa.PrivateKey) string { + if key == nil { + key = testKey + } + full := map[string]interface{}{ + "iss": testIssuer, + "aud": "keycape", + "iat": time.Now().Add(-time.Minute).Unix(), + "exp": time.Now().Add(10 * time.Minute).Unix(), + } + for name, value := range claims { + full[name] = value + } + headerJSON, _ := json.Marshal(map[string]string{"alg": "RS256", "typ": "JWT", "kid": kid}) + claimsJSON, _ := json.Marshal(full) + signingInput := base64.RawURLEncoding.EncodeToString(headerJSON) + "." + + base64.RawURLEncoding.EncodeToString(claimsJSON) + digest := sha256.Sum256([]byte(signingInput)) + signature, err := rsa.SignPKCS1v15(rand.Reader, key, crypto.SHA256, digest[:]) + if err != nil { + panic(err) + } + return signingInput + "." + base64.RawURLEncoding.EncodeToString(signature) +} + +// buildTokenResponse builds a token endpoint JSON response carrying a genuine +// signed ID token with the given claims. +func buildTokenResponse(claims map[string]interface{}) string { + return fmt.Sprintf(`{"access_token":"at","token_type":"Bearer","id_token":%q}`, + signIDToken(claims, testKeyID, nil)) } // jsonResponse returns a *http.Response with a JSON body and status 200. diff --git a/src/internal/adapters/authelia/config.go b/src/internal/adapters/authelia/config.go index 1e6c17c..810f5b9 100644 --- a/src/internal/adapters/authelia/config.go +++ b/src/internal/adapters/authelia/config.go @@ -27,6 +27,17 @@ type Config struct { // RedirectURI is the callback URL registered in Authelia that points back // to KeyCape's callback handler. RedirectURI string `yaml:"redirectURI"` + + // Issuer pins the expected iss claim of upstream ID tokens. Empty means the + // value Authelia advertises in its discovery document is used, which is + // correct for an ordinary deployment. Set it only where that inference is + // wrong. + Issuer string `yaml:"issuer,omitempty"` + + // JWKSURL pins where Authelia's signing keys are fetched from. Empty means + // the advertised jwks_uri path, rebased onto TokenBaseURL so a split-horizon + // deployment still resolves server-side. + JWKSURL string `yaml:"jwksUrl,omitempty"` } // HTTPClient is a minimal interface over net/http.Client for test injection. diff --git a/src/internal/adapters/authelia/verify.go b/src/internal/adapters/authelia/verify.go new file mode 100644 index 0000000..a393012 --- /dev/null +++ b/src/internal/adapters/authelia/verify.go @@ -0,0 +1,220 @@ +package authelia + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "sync" + "time" + + "keycape/internal/jose" +) + +// idTokenVerifier verifies upstream Authelia ID tokens against Authelia's +// published signing keys (KEY-WP-0019). +// +// KeyCape signs everything downstream on the strength of these claims, so they +// are verified independently of how they arrived. The KeyCape→Authelia hop is +// expected to be HTTPS as defence in depth, but nothing here checks or gates on +// that: a transport check only helps when it is configured correctly, which is +// precisely the assumption it was supposed to remove. Verification holds whether +// or not the transport is what we believe it is. +type idTokenVerifier struct { + client HTTPClient + tokenBaseURL string + clientID string + + // issuerOverride and jwksOverride pin metadata for deployments where it + // cannot be inferred from the token base URL. Empty means "discover". + issuerOverride string + jwksOverride string + + mu sync.Mutex + issuer string + jwksURL string + keys jose.KeySet +} + +// leeway absorbs ordinary clock skew between KeyCape and the provider. It is +// small on purpose: it is a tolerance for imperfect clocks, not a grace period. +const leeway = 30 * time.Second + +func newIDTokenVerifier(cfg Config, client HTTPClient, tokenBaseURL string) *idTokenVerifier { + return &idTokenVerifier{ + client: client, + tokenBaseURL: strings.TrimRight(tokenBaseURL, "/"), + clientID: cfg.ClientID, + issuerOverride: cfg.Issuer, + jwksOverride: cfg.JWKSURL, + } +} + +// Verify returns the ID token's claims once its signature, issuer, audience and +// validity window have all been checked. Every failure path denies the login: +// there is no fallback to unverified claims. +func (v *idTokenVerifier) Verify(ctx context.Context, idToken string) (map[string]interface{}, error) { + keys, issuer, err := v.material(ctx, false) + if err != nil { + return nil, err + } + + // An unknown key id most likely means the provider rotated its keys, so + // refetch once before rejecting. Any other failure is not retried. + if kid, kidErr := jose.KeyID(idToken); kidErr == nil { + if _, known := keys[kid]; !known { + if refreshed, refreshedIssuer, refreshErr := v.material(ctx, true); refreshErr == nil { + keys, issuer = refreshed, refreshedIssuer + } + } + } + + claims, err := jose.Verify(idToken, keys) + if err != nil { + return nil, fmt.Errorf("authelia: id_token signature: %w", err) + } + if err := v.checkClaims(claims, issuer); err != nil { + return nil, err + } + return claims, nil +} + +// checkClaims applies the issuer, audience and validity-window policy. The +// signature only proves who minted the token; these decide whether it was minted +// for us, by the provider we expect, and now. +func (v *idTokenVerifier) checkClaims(claims map[string]interface{}, issuer string) error { + if got, _ := claims["iss"].(string); got != issuer { + return fmt.Errorf("authelia: id_token issuer mismatch") + } + if !audienceContains(claims["aud"], v.clientID) { + return fmt.Errorf("authelia: id_token audience does not include this client") + } + exp, hasExp := numericClaim(claims, "exp") + iat, hasIat := numericClaim(claims, "iat") + if !hasExp || !hasIat { + return fmt.Errorf("authelia: id_token missing exp or iat") + } + now := time.Now() + if now.After(time.Unix(int64(exp), 0).Add(leeway)) { + return fmt.Errorf("authelia: id_token expired") + } + if time.Unix(int64(iat), 0).After(now.Add(leeway)) || iat >= exp { + return fmt.Errorf("authelia: id_token validity window is not sane") + } + if nbf, hasNbf := numericClaim(claims, "nbf"); hasNbf { + if time.Unix(int64(nbf), 0).After(now.Add(leeway)) { + return fmt.Errorf("authelia: id_token is not yet valid") + } + } + return nil +} + +// material returns the cached key set and expected issuer, fetching them if +// absent or if refresh is set. +func (v *idTokenVerifier) material(ctx context.Context, refresh bool) (jose.KeySet, string, error) { + v.mu.Lock() + defer v.mu.Unlock() + + if !refresh && v.keys != nil { + return v.keys, v.issuer, nil + } + if v.issuer == "" || v.jwksURL == "" || refresh { + if err := v.resolveMetadataLocked(ctx); err != nil { + return nil, "", err + } + } + raw, err := v.get(ctx, v.jwksURL) + if err != nil { + return nil, "", fmt.Errorf("authelia: fetch jwks: %w", err) + } + keys, err := jose.ParseJWKS(raw) + if err != nil { + return nil, "", fmt.Errorf("authelia: parse jwks: %w", err) + } + v.keys = keys + return v.keys, v.issuer, nil +} + +// resolveMetadataLocked determines the expected issuer and the JWKS URL. +// +// The advertised jwks_uri is rebased onto the server-side token base URL. In a +// split-horizon deployment the provider advertises its public URL, which KeyCape +// may not be able to reach; the path is the part worth keeping. The issuer is +// taken as advertised, because that is the exact string that appears in the +// token and a rebased one would never match. +func (v *idTokenVerifier) resolveMetadataLocked(ctx context.Context) error { + if v.issuerOverride != "" && v.jwksOverride != "" { + v.issuer, v.jwksURL = v.issuerOverride, v.jwksOverride + return nil + } + raw, err := v.get(ctx, v.tokenBaseURL+"/.well-known/openid-configuration") + if err != nil { + return fmt.Errorf("authelia: fetch provider metadata: %w", err) + } + var metadata struct { + Issuer string `json:"issuer"` + JWKSURI string `json:"jwks_uri"` + } + if err := json.Unmarshal(raw, &metadata); err != nil { + return fmt.Errorf("authelia: parse provider metadata: %w", err) + } + if metadata.Issuer == "" || metadata.JWKSURI == "" { + return fmt.Errorf("authelia: provider metadata is incomplete") + } + + v.issuer = metadata.Issuer + if v.issuerOverride != "" { + v.issuer = v.issuerOverride + } + v.jwksURL = v.jwksOverride + if v.jwksURL == "" { + advertised, err := url.Parse(metadata.JWKSURI) + if err != nil { + return fmt.Errorf("authelia: parse advertised jwks_uri: %w", err) + } + v.jwksURL = v.tokenBaseURL + advertised.EscapedPath() + } + return nil +} + +func (v *idTokenVerifier) get(ctx context.Context, target string) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil) + if err != nil { + return nil, err + } + req.Header.Set("Accept", "application/json") + resp, err := v.client.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("status %d", resp.StatusCode) + } + // Bounded read: an upstream that streams indefinitely must not exhaust us. + return io.ReadAll(io.LimitReader(resp.Body, 1<<20)) +} + +// audienceContains reports whether the aud claim, which may be a string or an +// array, includes want. +func audienceContains(aud interface{}, want string) bool { + switch value := aud.(type) { + case string: + return value == want + case []interface{}: + for _, entry := range value { + if s, ok := entry.(string); ok && s == want { + return true + } + } + } + return false +} + +func numericClaim(claims map[string]interface{}, key string) (float64, bool) { + value, ok := claims[key].(float64) + return value, ok +} diff --git a/src/internal/adapters/authelia/verify_test.go b/src/internal/adapters/authelia/verify_test.go new file mode 100644 index 0000000..0195cd3 --- /dev/null +++ b/src/internal/adapters/authelia/verify_test.go @@ -0,0 +1,212 @@ +package authelia_test + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/base64" + "errors" + "fmt" + "io" + "math/big" + "net/http" + "strings" + "testing" + "time" + + "keycape/internal/adapters/authelia" + "keycape/internal/domain" +) + +// KEY-WP-0019-T03 — upstream ID tokens are verified independently of transport. +// Every case here is a token KeyCape must refuse to build an identity from, plus +// the positive and rotation cases that keep the refusals meaningful. + +// provider serves discovery, JWKS and the token endpoint. jwks and idToken let a +// case vary exactly one thing. +type provider struct { + jwks string + idToken string + jwksErr error + jwksCalls int +} + +func (p *provider) Do(req *http.Request) (*http.Response, error) { + body := "" + switch { + case strings.HasSuffix(req.URL.Path, "/.well-known/openid-configuration"): + body = fmt.Sprintf(`{"issuer":%q,"jwks_uri":%q}`, testIssuer, testIssuer+"/jwks.json") + case strings.HasSuffix(req.URL.Path, "/jwks.json"): + p.jwksCalls++ + if p.jwksErr != nil { + return nil, p.jwksErr + } + body = p.jwks + default: + body = fmt.Sprintf(`{"access_token":"at","token_type":"Bearer","id_token":%q}`, p.idToken) + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(body)), + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, nil +} + +func callback(t *testing.T, p *provider) (*domain.AuthResult, error) { + t.Helper() + adapter := authelia.New(testConfig(), p) + return adapter.HandleCallback(context.Background(), domain.CallbackParams{Code: "code", State: "state"}) +} + +func TestUpstreamTokenIsAcceptedWhenGenuine(t *testing.T) { + result, err := callback(t, &provider{ + jwks: testJWKS(nil), + idToken: signIDToken(map[string]interface{}{"preferred_username": "alice"}, testKeyID, nil), + }) + if err != nil { + t.Fatalf("genuine token rejected: %v", err) + } + if result.Username != "alice" { + t.Fatalf("username %q", result.Username) + } +} + +func TestUpstreamTokenRejections(t *testing.T) { + otherKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + valid := map[string]interface{}{"preferred_username": "alice"} + + cases := map[string]*provider{ + // Signed by a key that is not the provider's. + "forged signature": { + jwks: testJWKS(nil), + idToken: signIDToken(valid, testKeyID, otherKey), + }, + // A key id the published set does not contain. + "unknown key id": { + jwks: testJWKS(nil), + idToken: signIDToken(valid, "not-a-published-key", nil), + }, + "wrong issuer": { + jwks: testJWKS(nil), + idToken: signIDToken(map[string]interface{}{"preferred_username": "alice", "iss": "https://attacker.example"}, testKeyID, nil), + }, + "audience is another client": { + jwks: testJWKS(nil), + idToken: signIDToken(map[string]interface{}{"preferred_username": "alice", "aud": "some-other-client"}, testKeyID, nil), + }, + "audience array without this client": { + jwks: testJWKS(nil), + idToken: signIDToken(map[string]interface{}{"preferred_username": "alice", "aud": []string{"a", "b"}}, testKeyID, nil), + }, + "expired": { + jwks: testJWKS(nil), + idToken: signIDToken(map[string]interface{}{ + "preferred_username": "alice", + "iat": time.Now().Add(-2 * time.Hour).Unix(), + "exp": time.Now().Add(-time.Hour).Unix(), + }, testKeyID, nil), + }, + "issued in the future": { + jwks: testJWKS(nil), + idToken: signIDToken(map[string]interface{}{"preferred_username": "alice", "iat": time.Now().Add(time.Hour).Unix()}, testKeyID, nil), + }, + "not yet valid": { + jwks: testJWKS(nil), + idToken: signIDToken(map[string]interface{}{"preferred_username": "alice", "nbf": time.Now().Add(time.Hour).Unix()}, testKeyID, nil), + }, + "missing expiry": { + jwks: testJWKS(nil), + idToken: signIDToken(map[string]interface{}{"preferred_username": "alice", "exp": nil}, testKeyID, nil), + }, + "undersized signing key": { + jwks: smallKeyJWKS(t), + idToken: signIDToken(valid, testKeyID, nil), + }, + "malformed key set": { + jwks: `{"keys":[{"kty":"RSA","kid":"authelia-key-1","n":"!!not-base64!!","e":"AQAB"}]}`, + idToken: signIDToken(valid, testKeyID, nil), + }, + "empty key set": { + jwks: `{"keys":[]}`, + idToken: signIDToken(valid, testKeyID, nil), + }, + "unreachable key set": { + jwksErr: errors.New("dial tcp: connection refused"), + idToken: signIDToken(valid, testKeyID, nil), + }, + } + + for name, p := range cases { + t.Run(name, func(t *testing.T) { + result, err := callback(t, p) + if !errors.Is(err, domain.ErrAuthFailed) { + t.Fatalf("expected ErrAuthFailed, got result=%v err=%v", result, err) + } + }) + } +} + +// alg must be checked before any key is selected, so "none" and symmetric +// algorithms cannot reach the RSA path. +func TestUpstreamTokenRejectsNonRS256Algorithms(t *testing.T) { + for _, alg := range []string{"none", "HS256", "RS512", ""} { + genuine := signIDToken(map[string]interface{}{"preferred_username": "alice"}, testKeyID, nil) + parts := strings.Split(genuine, ".") + restated := encodeSegment(fmt.Sprintf(`{"alg":%q,"typ":"JWT","kid":%q}`, alg, testKeyID)) + if _, err := callback(t, &provider{ + jwks: testJWKS(nil), + idToken: restated + "." + parts[1] + "." + parts[2], + }); !errors.Is(err, domain.ErrAuthFailed) { + t.Fatalf("alg %q accepted", alg) + } + } +} + +// Provider key rotation must not require a KeyCape restart: an unknown key id +// triggers one refresh, and the new key then verifies. +func TestUpstreamKeyRotationIsPickedUpWithoutRestart(t *testing.T) { + p := &provider{ + jwks: testJWKS(nil), + idToken: signIDToken(map[string]interface{}{"preferred_username": "alice"}, testKeyID, nil), + } + adapter := authelia.New(testConfig(), p) + if _, err := adapter.HandleCallback(context.Background(), domain.CallbackParams{Code: "c", State: "s"}); err != nil { + t.Fatalf("first login: %v", err) + } + firstCalls := p.jwksCalls + + // The provider rotates: same key id, new key material. + rotated, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + p.jwks = testJWKSKid("rotated-key", rotated) + p.idToken = signIDToken(map[string]interface{}{"preferred_username": "alice"}, "rotated-key", rotated) + + if _, err := adapter.HandleCallback(context.Background(), domain.CallbackParams{Code: "c", State: "s"}); err != nil { + t.Fatalf("after rotation: %v", err) + } + if p.jwksCalls <= firstCalls { + t.Fatal("key set was not refetched after an unknown key id") + } +} + +func encodeSegment(raw string) string { + return base64.RawURLEncoding.EncodeToString([]byte(raw)) +} + +// smallKeyJWKS publishes a key below the accepted modulus size. +func smallKeyJWKS(t *testing.T) string { + t.Helper() + small, err := rsa.GenerateKey(rand.Reader, 1024) + if err != nil { + t.Fatal(err) + } + return fmt.Sprintf(`{"keys":[{"kty":"RSA","use":"sig","alg":"RS256","kid":%q,"n":%q,"e":%q}]}`, + testKeyID, + base64.RawURLEncoding.EncodeToString(small.PublicKey.N.Bytes()), + base64.RawURLEncoding.EncodeToString(big.NewInt(int64(small.PublicKey.E)).Bytes())) +} diff --git a/src/internal/jose/jose.go b/src/internal/jose/jose.go new file mode 100644 index 0000000..14769bb --- /dev/null +++ b/src/internal/jose/jose.go @@ -0,0 +1,153 @@ +// Package jose provides strict RS256 JWT signature verification against a JWK +// set. It is deliberately narrow: it establishes that a token was signed by a +// key in the given set and returns its claims. It applies no claim policy — +// issuer, audience, expiry and nonce rules differ per caller and belong with +// the caller that knows them. +// +// The strictness here is the point. A verifier that accepts an unexpected +// algorithm, an unbounded exponent or an undersized modulus is worse than no +// verifier, because callers stop checking what it appears to guarantee. +package jose + +import ( + "crypto" + "crypto/rsa" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "errors" + "math/big" + "strings" +) + +// ErrVerification is returned for every verification failure. The cause is +// deliberately not distinguished: a caller acting on why a token failed tends to +// leak that distinction to whoever supplied the token. +var ErrVerification = errors.New("jose: token verification failed") + +// KeySet maps key IDs to RSA public keys. +type KeySet map[string]*rsa.PublicKey + +// ParseJWKS parses a JWK set, keeping only RSA signing keys usable with RS256. +// Keys that are malformed, too small, or carry an even or trivial exponent are +// rejected rather than skipped, since a key set that is partly wrong is not +// evidence of anything. +func ParseJWKS(raw []byte) (KeySet, error) { + var document struct { + Keys []struct { + Kty string `json:"kty"` + Use string `json:"use"` + Alg string `json:"alg"` + Kid string `json:"kid"` + N string `json:"n"` + E string `json:"e"` + } `json:"keys"` + } + if err := json.Unmarshal(raw, &document); err != nil { + return nil, ErrVerification + } + keys := make(KeySet, len(document.Keys)) + for _, key := range document.Keys { + // Ignore keys for other algorithms or purposes; they are not errors. + if key.Kty != "RSA" || (key.Use != "" && key.Use != "sig") || (key.Alg != "" && key.Alg != "RS256") { + continue + } + if key.Kid == "" { + return nil, ErrVerification + } + modulus, errN := decode(key.N) + exponent, errE := decode(key.E) + if errN != nil || errE != nil || len(exponent) == 0 || len(exponent) > 4 { + return nil, ErrVerification + } + public := &rsa.PublicKey{ + N: new(big.Int).SetBytes(modulus), + E: int(new(big.Int).SetBytes(exponent).Int64()), + } + if public.N.BitLen() < 2048 || public.E < 3 || public.E%2 == 0 { + return nil, ErrVerification + } + if _, duplicate := keys[key.Kid]; duplicate { + return nil, ErrVerification + } + keys[key.Kid] = public + } + if len(keys) == 0 { + return nil, ErrVerification + } + return keys, nil +} + +// KeyID returns the key ID from a token's JOSE header, after checking that the +// header names RS256 and carries no critical extensions. Callers use it to +// decide whether to refresh their key set before verifying. +func KeyID(token string) (string, error) { + header, _, _, err := split(token) + if err != nil { + return "", err + } + return header.Kid, nil +} + +// Verify checks a token's RS256 signature against the key set and returns its +// claims. It does not inspect any claim. +func Verify(token string, keys KeySet) (map[string]interface{}, error) { + header, signingInput, signature, err := split(token) + if err != nil { + return nil, err + } + public, ok := keys[header.Kid] + if !ok { + return nil, ErrVerification + } + digest := sha256.Sum256([]byte(signingInput)) + if rsa.VerifyPKCS1v15(public, crypto.SHA256, digest[:], signature) != nil { + return nil, ErrVerification + } + payload, err := decode(strings.Split(signingInput, ".")[1]) + if err != nil { + return nil, ErrVerification + } + claims := map[string]interface{}{} + if json.Unmarshal(payload, &claims) != nil { + return nil, ErrVerification + } + return claims, nil +} + +type joseHeader struct { + Alg string `json:"alg"` + Kid string `json:"kid"` + Crit []string `json:"crit"` +} + +// split validates the token's shape and header and returns the header, the +// signing input, and the decoded signature. +func split(token string) (joseHeader, string, []byte, error) { + parts := strings.Split(token, ".") + if len(parts) != 3 { + return joseHeader{}, "", nil, ErrVerification + } + raw, err := decode(parts[0]) + if err != nil { + return joseHeader{}, "", nil, ErrVerification + } + var header joseHeader + if json.Unmarshal(raw, &header) != nil { + return joseHeader{}, "", nil, ErrVerification + } + // An unexpected algorithm must fail before any key is selected, so "none" + // and symmetric algorithms can never reach the RSA path. + if header.Alg != "RS256" || header.Kid == "" || len(header.Crit) != 0 { + return joseHeader{}, "", nil, ErrVerification + } + signature, err := decode(parts[2]) + if err != nil { + return joseHeader{}, "", nil, ErrVerification + } + return header, parts[0] + "." + parts[1], signature, nil +} + +func decode(segment string) ([]byte, error) { + return base64.RawURLEncoding.DecodeString(segment) +} diff --git a/workplans/KEY-WP-0019-upstream-provider-token-verification.md b/workplans/KEY-WP-0019-upstream-provider-token-verification.md new file mode 100644 index 0000000..1eb3baf --- /dev/null +++ b/workplans/KEY-WP-0019-upstream-provider-token-verification.md @@ -0,0 +1,124 @@ +--- +id: KEY-WP-0019 +type: workplan +title: "Verify upstream Authelia ID tokens" +domain: infotech +repo: key-cape +status: active +owner: claude +topic_slug: upstream-provider-token-verification +created: "2026-09-07" +updated: "2026-09-07" +--- + +Closes the remaining half of gap G01 in +`history/2026-09-05-011726-scope-intent-assessment.md`. The Authelia adapter +decodes upstream ID-token claims without verifying the signature, issuer, +audience or expiry, justified in a comment by a server-to-server TLS boundary. + +**Operator decision (2026-09-07, Bernd):** verify the token itself. The +KeyCape→Authelia hop is to be HTTPS as defence in depth, but KeyCape will not +monitor, check or gate on that — transport enforcement is friction without +protection value here, and a check that must be configured correctly to help is +itself a failure mode. Verification is chosen precisely because it does not +depend on the transport being what we believe it is. No HTTPS validation, opt-in +flag or transport telemetry is to be added. + +## Extract a shared RS256/JWKS verifier + +```task +id: KEY-WP-0019-T01 +status: done +priority: high +``` + +`internal/authclient` already contains a strict RS256 verifier — kid required, +no `crit`, RSA-only keys of at least 2048 bits with an odd exponent. The upstream +verification needs the same rules. Duplicating security-critical verification is +how two copies drift and one silently misses a fix, so extract the reusable half +into `internal/jose`: strict JWK-set parsing and signature verification returning +claims. Claim policy stays with each caller, whose audience, nonce and issuer +rules genuinely differ. + +Added `internal/jose`: strict JWK-set parsing (RSA signing keys only, at least +2048 bits, odd exponent 3 or greater, no duplicate key ids) and RS256 signature +verification returning claims. All failures collapse to one `ErrVerification` +so a caller cannot leak which check failed back to whoever supplied the token. +`authclient` is not yet migrated onto it — see T05. + +## Verify the Authelia ID token + +```task +id: KEY-WP-0019-T02 +status: done +priority: high +``` + +Before trusting any claim from the upstream ID token: verify the RS256 signature +against Authelia's published keys, require the issuer Authelia advertises, require +KeyCape's own client ID in the audience, and reject expired or not-yet-valid +tokens. Fail closed — an unavailable or unparseable key set denies the login +rather than falling back to unverified claims. Resolve Authelia's metadata and +key set from the server-side token base URL so a split-horizon deployment, where +the advertised public URL is not reachable from KeyCape, still verifies; allow +explicit issuer and JWKS overrides for deployments where that inference is wrong. +Refresh the key set once on an unknown key id so provider key rotation does not +require a KeyCape restart. Replace the comment claiming the TLS boundary as +justification. + +`idTokenVerifier` in the adapter resolves the issuer and JWKS path from the +provider's discovery document, rebasing the advertised `jwks_uri` path onto the +server-side token base URL so split-horizon deployments resolve, with +`issuer`/`jwksUrl` config overrides where that inference is wrong. Key sets are +cached and refetched once on an unknown key id. `parseIDTokenClaims` remains for +diagnostics but is documented as outside the authentication path. + +## Prove the rejections + +```task +id: KEY-WP-0019-T03 +status: done +priority: high +``` + +Negative tests per condition: forged signature, `alg: none` and other algorithms, +unknown key id, wrong issuer, missing and wrong audience, expired token, future +`iat`/`nbf`, malformed key set, and an unreachable JWKS endpoint. A positive case +must confirm a genuine token still authenticates, and a rotation case that a new +key id is picked up after one refresh. + +Thirteen rejection cases in `verify_test.go` plus the algorithm and rotation +cases. Confirmed they catch the original defect rather than merely passing: with +the unverified parse restored, all fifteen fail. The existing adapter tests now +build genuinely signed tokens against a fixture provider, so they exercise the +real verification path instead of being routed around it. + +## Reconcile the records + +```task +id: KEY-WP-0019-T04 +status: done +priority: medium +``` + +Update `SCOPE.md` and G01's status in the assessment to state that upstream +provider tokens are now verified independently of transport, and record the +operator decision and its reasoning so the absence of transport enforcement reads +as a choice rather than an oversight. G01 closes with this; complete profile +conformance is still not claimed. + +SCOPE.md and G01's status record the verification and the operator decision, +including why no transport enforcement exists, so its absence reads as a choice. + +## Consolidate the caller verifier + +```task +id: KEY-WP-0019-T05 +status: todo +priority: medium +``` + +Move `internal/authclient`'s inline verification onto `internal/jose` so one +implementation serves both paths. Kept separate from T01/T02 deliberately: the +caller verifier is a tested security path, and destabilising it in the same +change that introduces upstream verification would confuse the evidence for both.