diff --git a/WORK-RECORDS.md b/WORK-RECORDS.md index affc1c8..34af4bf 100644 --- a/WORK-RECORDS.md +++ b/WORK-RECORDS.md @@ -28,7 +28,7 @@ | workplan | KEY-WP-0016 | finished | — | workplans/KEY-WP-0016-authorization-code-protocol-hardening.md | | workplan | KEY-WP-0017 | finished | — | workplans/KEY-WP-0017-canonical-model-and-discovery-conformance.md | | workplan | KEY-WP-0018 | finished | — | workplans/KEY-WP-0018-export-completeness-evidence.md | -| workplan | KEY-WP-0019 | active | — | workplans/KEY-WP-0019-upstream-provider-token-verification.md | +| workplan | KEY-WP-0019 | finished | — | workplans/KEY-WP-0019-upstream-provider-token-verification.md | | task | ADHOC-2026-09-05-T01 | done | — | workplans/ADHOC-2026-09-05.md | | task | ADHOC-2026-09-07-T01 | done | — | workplans/ADHOC-2026-09-07.md | | task | KEY-WP-0001-T01 | done | — | workplans/KEY-WP-0001-keycape-implementation.md | @@ -122,5 +122,5 @@ | task | KEY-WP-0019-T02 | done | — | workplans/KEY-WP-0019-upstream-provider-token-verification.md | | task | KEY-WP-0019-T03 | done | — | workplans/KEY-WP-0019-upstream-provider-token-verification.md | | task | KEY-WP-0019-T04 | done | — | workplans/KEY-WP-0019-upstream-provider-token-verification.md | -| task | KEY-WP-0019-T05 | todo | — | workplans/KEY-WP-0019-upstream-provider-token-verification.md | +| task | KEY-WP-0019-T05 | done | — | workplans/KEY-WP-0019-upstream-provider-token-verification.md | | intake | KEY-IN-0001 | closed | — | intakes/intakes.md | diff --git a/src/internal/authclient/client_test.go b/src/internal/authclient/client_test.go index cd5f6a9..c7e98ba 100644 --- a/src/internal/authclient/client_test.go +++ b/src/internal/authclient/client_test.go @@ -2,8 +2,11 @@ package authclient import ( "context" + "crypto" "crypto/rand" "crypto/rsa" + "crypto/sha256" + "encoding/base64" "encoding/json" "fmt" "io" @@ -254,3 +257,63 @@ func TestNonceMismatchAndCancelledLogin(t *testing.T) { } listener.Close() } + +// The caller-side verifier moved onto internal/jose (KEY-WP-0019-T05). The +// existing tamper case appends to the encoded signature, which fails on the +// segment's shape before any key is used, so it would still pass if the +// signature check were removed entirely. These cases sign a structurally +// valid token with a key the provider does not publish, and hand the client a +// key set it must refuse — the two ways a broken verifier actually shows up. +func TestVerifyRejectsForeignSignatureAndUnusableKeySet(t *testing.T) { + c, d, _ := provider(t) + ctx := context.Background() + + foreign, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + now := time.Now() + forged := signToken(t, foreign, "key-1", map[string]any{ + "iss": c.Issuer, + "aud": "approval-engine", + "sub": "service:test", + "iat": now.Unix(), + "exp": now.Add(time.Minute).Unix(), + }) + if _, err := c.Verify(ctx, d, forged, "approval-engine", ""); err == nil { + t.Fatal("token signed by an unpublished key accepted") + } + + // A key set the client cannot use must deny, not fall through to the + // claims. The token here is otherwise genuine. + genuine := signToken(t, foreign, "key-1", map[string]any{ + "iss": c.Issuer, "aud": "approval-engine", "sub": "service:test", + "iat": now.Unix(), "exp": now.Add(time.Minute).Unix(), + }) + broken := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"keys":[{"kty":"RSA","kid":"key-1","n":"AQAB","e":"AQAB"}]}`) + })) + t.Cleanup(broken.Close) + if _, err := c.Verify(ctx, Discovery{Issuer: d.Issuer, JWKS: broken.URL}, genuine, "approval-engine", ""); err == nil { + t.Fatal("undersized key in the published set accepted") + } +} + +// signToken builds an RS256 JWT with the given kid and claims. +func signToken(t *testing.T, key *rsa.PrivateKey, kid string, claims map[string]any) string { + t.Helper() + enc := func(v any) string { + raw, err := json.Marshal(v) + if err != nil { + t.Fatal(err) + } + return base64.RawURLEncoding.EncodeToString(raw) + } + input := enc(map[string]any{"alg": "RS256", "typ": "JWT", "kid": kid}) + "." + enc(claims) + digest := sha256.Sum256([]byte(input)) + signature, err := rsa.SignPKCS1v15(rand.Reader, key, crypto.SHA256, digest[:]) + if err != nil { + t.Fatal(err) + } + return input + "." + base64.RawURLEncoding.EncodeToString(signature) +} diff --git a/src/internal/jose/jose.go b/src/internal/jose/jose.go index 5e7eb4c..14769bb 100644 --- a/src/internal/jose/jose.go +++ b/src/internal/jose/jose.go @@ -101,7 +101,7 @@ func Verify(token string, keys KeySet) (map[string]interface{}, error) { return nil, ErrVerification } digest := sha256.Sum256([]byte(signingInput)) - if false && rsa.VerifyPKCS1v15(public, crypto.SHA256, digest[:], signature) != nil { + if rsa.VerifyPKCS1v15(public, crypto.SHA256, digest[:], signature) != nil { return nil, ErrVerification } payload, err := decode(strings.Split(signingInput, ".")[1])