diff --git a/e2e/e2e.go b/e2e/e2e.go index b2fc4f2..53623d8 100644 --- a/e2e/e2e.go +++ b/e2e/e2e.go @@ -33,7 +33,8 @@ func Logger(traceId string) types.StepFunc { type SignUpOptions struct { Username string Groups []string - JWTSignKey string + JWTSignKey string // PEM-encoded RSA private key + JWTKeyID string Namespace string Duration time.Duration } @@ -44,10 +45,16 @@ func SignUp(opts SignUpOptions) types.StepFunc { } return func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { + privateKey, err := jwtutil.ParseRSAPrivateKeyFromPEM([]byte(opts.JWTSignKey)) + if err != nil { + t.Fatal(err) + } + accessToken, err := jwtutil.CreateToken(jwtutil.CreateTokenOptions{ Username: opts.Username, Groups: opts.Groups, - SigningKey: opts.JWTSignKey, + KeyID: opts.JWTKeyID, + PrivateKey: privateKey, Duration: opts.Duration, }) if err != nil { diff --git a/jwtutil/create.go b/jwtutil/create.go index 609df45..fc77114 100644 --- a/jwtutil/create.go +++ b/jwtutil/create.go @@ -1,6 +1,7 @@ package jwtutil import ( + "crypto/rsa" "fmt" "time" @@ -8,8 +9,6 @@ import ( ) const ( - JwtSecretEnvKey = "AUTHN_JWT_SECRET" - defaultISS = "krateo.io" ) @@ -27,17 +26,24 @@ type CreateTokenOptions struct { Username string Groups []string Duration time.Duration - SigningKey string + KeyID string + PrivateKey *rsa.PrivateKey } // CreateToken generates a signed JWT token using the provided // username, group list, and expiration duration. -// The token is signed using the HS256 algorithm. -// The signing key is read from the environment variable AUTHN_JWT_SECRET. -// If the environment variable is not set, the function returns an error. +// The token is signed asymmetrically using the RS256 algorithm and the +// supplied RSA private key, and carries a "kid" header identifying the key +// so that JWKS-based validators (e.g. agentgateway) can select the matching +// public key. The corresponding public key must be published in the JWKS +// under the same KeyID. func CreateToken(opts CreateTokenOptions) (string, error) { - if opts.SigningKey == "" { - return "", fmt.Errorf("signing key cannot be empty") + if opts.PrivateKey == nil { + return "", fmt.Errorf("private key cannot be nil") + } + + if opts.KeyID == "" { + return "", fmt.Errorf("key ID cannot be empty") } if opts.Username == "" { @@ -60,7 +66,8 @@ func CreateToken(opts CreateTokenOptions) (string, error) { }, } - token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + token.Header["kid"] = opts.KeyID - return token.SignedString([]byte(opts.SigningKey)) + return token.SignedString(opts.PrivateKey) } diff --git a/jwtutil/create_test.go b/jwtutil/create_test.go index 3f7ccc0..d3750f3 100644 --- a/jwtutil/create_test.go +++ b/jwtutil/create_test.go @@ -1,35 +1,40 @@ package jwtutil_test import ( + "crypto/rand" + "crypto/rsa" "testing" "time" "github.com/golang-jwt/jwt/v5" "github.com/krateo-platformops/plumbing/jwtutil" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCreateToken(t *testing.T) { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const keyID = "test-key-id" + tests := []struct { name string username string groups []string duration time.Duration - secret string }{ { - name: "with environment secret", + name: "with groups", username: "alice", groups: []string{"admin", "dev"}, duration: time.Minute * 30, - secret: "envSecret123", }, { - name: "with default secret fallback", + name: "without groups", username: "bob", groups: []string{}, duration: time.Minute * 15, - secret: "abbracadabra!", }, } @@ -39,20 +44,25 @@ func TestCreateToken(t *testing.T) { Username: tc.username, Groups: tc.groups, Duration: tc.duration, - SigningKey: tc.secret, + KeyID: keyID, + PrivateKey: privateKey, } tokenStr, err := jwtutil.CreateToken(opts) assert.NoError(t, err) assert.NotEmpty(t, tokenStr) - // Parse and validate the token + // Parse and validate the token using the public key. token, err := jwt.Parse(tokenStr, func(token *jwt.Token) (any, error) { - return []byte(tc.secret), nil + return &privateKey.PublicKey, nil }) assert.NoError(t, err) assert.True(t, token.Valid) + // The token must be signed with RS256 and carry the kid header. + assert.Equal(t, jwt.SigningMethodRS256.Alg(), token.Method.Alg()) + assert.Equal(t, keyID, token.Header["kid"]) + claims, ok := token.Claims.(jwt.MapClaims) assert.True(t, ok) @@ -62,3 +72,33 @@ func TestCreateToken(t *testing.T) { }) } } + +func TestCreateTokenValidation(t *testing.T) { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + tests := []struct { + name string + opts jwtutil.CreateTokenOptions + }{ + { + name: "nil private key", + opts: jwtutil.CreateTokenOptions{Username: "alice", KeyID: "kid"}, + }, + { + name: "empty key ID", + opts: jwtutil.CreateTokenOptions{Username: "alice", PrivateKey: privateKey}, + }, + { + name: "empty username", + opts: jwtutil.CreateTokenOptions{KeyID: "kid", PrivateKey: privateKey}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := jwtutil.CreateToken(tc.opts) + assert.Error(t, err) + }) + } +} diff --git a/jwtutil/jwks.go b/jwtutil/jwks.go new file mode 100644 index 0000000..294eeac --- /dev/null +++ b/jwtutil/jwks.go @@ -0,0 +1,373 @@ +package jwtutil + +import ( + "crypto/rsa" + "encoding/base64" + "encoding/json" + "fmt" + "io" + "math" + "math/big" + "net/http" + "strings" + "sync" + "time" +) + +const DefaultJWKSPath = "/.well-known/jwks.json" + +const ( + // defaultJWKSCacheTTL is how long a successfully fetched key set is trusted + // before it is refreshed. + defaultJWKSCacheTTL = 5 * time.Minute + + // defaultJWKSMinRefreshInterval floors the gap between two fetch attempts. + defaultJWKSMinRefreshInterval = 30 * time.Second + + // defaultJWKSRequestTimeout bounds a single fetch. + defaultJWKSRequestTimeout = 5 * time.Second + + maxJWKSResponseBytes = 1 << 20 +) + +// ErrKeyUnavailable reports that the key a token was signed with could not be +// resolved — the key set was unreachable, malformed, or carried no key for the +// token's "kid". It is distinct from ErrTokenInvalid on purpose: the token may +// be perfectly valid and the fault ours, so callers should answer 503 rather +// than 401 and let the client retry. +var ErrKeyUnavailable = errKeyUnavailable{} + +// errKeyUnavailable carries the underlying cause while still matching +// errors.Is(err, ErrKeyUnavailable), so logs keep the detail ("connection +// refused", "no key for kid X") that a bare sentinel would discard. +type errKeyUnavailable struct{ cause error } + +func (e errKeyUnavailable) Error() string { + if e.cause == nil { + return "token signing key unavailable" + } + return "token signing key unavailable: " + e.cause.Error() +} + +func (e errKeyUnavailable) Unwrap() error { return e.cause } + +// Is matches any errKeyUnavailable regardless of cause, so the exported +// sentinel works as a target for every wrapped instance. +func (e errKeyUnavailable) Is(target error) bool { + _, ok := target.(errKeyUnavailable) + return ok +} + +func keyUnavailable(format string, args ...any) error { + return errKeyUnavailable{cause: fmt.Errorf(format, args...)} +} + +// KeySource resolves the RSA public key that a token's "kid" header names. +// Implementations must be safe for concurrent use. +type KeySource interface { + PublicKey(kid string) (*rsa.PublicKey, error) +} + +// StaticKeySource serves one fixed key for every kid. It is the adapter that +// keeps the plain Validate(publicKey, bearer) form working, and is the simplest +// stand-in in tests. +type StaticKeySource struct { + key *rsa.PublicKey +} + +// NewStaticKeySource returns a KeySource that always yields key. +func NewStaticKeySource(key *rsa.PublicKey) *StaticKeySource { + return &StaticKeySource{key: key} +} + +// PublicKey ignores kid and returns the configured key. +func (s *StaticKeySource) PublicKey(string) (*rsa.PublicKey, error) { + if s == nil || s.key == nil { + return nil, keyUnavailable("no public key configured") + } + return s.key, nil +} + +// JWKSKeySource resolves signing keys from a remote JWKS document (authn's +// /.well-known/jwks.json), caching the result. +// +// Nothing is fetched at construction: the first fetch happens on the first +// validation. A component wired to an authn that is not up yet therefore still +// starts and serves its unauthenticated routes, and recovers on its own once +// authn answers — no startup ordering dependency, no crash loop. +// +// Cache behaviour: +// - A key set younger than TTL answers from memory. +// - Past TTL, or when a token names a kid the cache does not hold (key +// rotation), a refetch is attempted — but never more often than +// MinRefreshInterval, so an unknown kid cannot turn into a request-rate +// stampede against authn. +// - If a refetch fails but the cache still holds the requested kid, the +// stale-but-known key is served. A brief authn outage does not invalidate +// tokens that were already verifiable. +type JWKSKeySource struct { + url string + httpClient *http.Client + ttl time.Duration + minRefreshInterval time.Duration + + // now is swappable so tests can drive cache expiry without sleeping. + now func() time.Time + + // mu guards every field below AND serialises fetches. Holding it across the + // HTTP round trip is deliberate: it single-flights concurrent misses into + // one request. The wait is bounded by the client's timeout. + mu sync.Mutex + keys map[string]*rsa.PublicKey + fetchedAt time.Time + lastAttempt time.Time +} + +// JWKSOption customises a JWKSKeySource. +type JWKSOption func(*JWKSKeySource) + +// WithJWKSCacheTTL sets how long a fetched key set is served before refresh. +// Non-positive values are ignored. +func WithJWKSCacheTTL(ttl time.Duration) JWKSOption { + return func(s *JWKSKeySource) { + if ttl > 0 { + s.ttl = ttl + } + } +} + +// WithJWKSMinRefreshInterval sets the floor between two fetch attempts. +// Non-positive values are ignored. +func WithJWKSMinRefreshInterval(d time.Duration) JWKSOption { + return func(s *JWKSKeySource) { + if d > 0 { + s.minRefreshInterval = d + } + } +} + +// WithJWKSHTTPClient replaces the HTTP client used for fetches. The client +// should set a timeout; it bounds how long a validation can block. +func WithJWKSHTTPClient(c *http.Client) JWKSOption { + return func(s *JWKSKeySource) { + if c != nil { + s.httpClient = c + } + } +} + +// WithJWKSRequestTimeout sets the per-fetch timeout on the default client. +// Ignored when WithJWKSHTTPClient supplied a client of its own. +func WithJWKSRequestTimeout(d time.Duration) JWKSOption { + return func(s *JWKSKeySource) { + if d > 0 { + s.httpClient.Timeout = d + } + } +} + +// NewJWKSKeySource returns a KeySource that reads authn's key set from url. +// url must be the full JWKS document URL; see JWKSURL for building it from a +// base URL. +func NewJWKSKeySource(url string, opts ...JWKSOption) *JWKSKeySource { + s := &JWKSKeySource{ + url: url, + httpClient: &http.Client{Timeout: defaultJWKSRequestTimeout}, + ttl: defaultJWKSCacheTTL, + minRefreshInterval: defaultJWKSMinRefreshInterval, + now: time.Now, + keys: map[string]*rsa.PublicKey{}, + } + + for _, opt := range opts { + opt(s) + } + + return s +} + +// JWKSURL joins a base URL (e.g. authn's service URL) with DefaultJWKSPath. +// A base that already points at a .json document is returned unchanged, so +// callers may configure either form. +func JWKSURL(base string) string { + base = strings.TrimSpace(base) + if base == "" { + return "" + } + if strings.HasSuffix(base, ".json") { + return base + } + return strings.TrimSuffix(base, "/") + DefaultJWKSPath +} + +// PublicKey returns the key named by kid, fetching or refreshing the key set as +// needed. An empty kid resolves to the only key in the set when the set holds +// exactly one — tokens minted before "kid" headers existed stay verifiable. +func (s *JWKSKeySource) PublicKey(kid string) (*rsa.PublicKey, error) { + if s == nil { + return nil, keyUnavailable("no JWKS key source configured") + } + if s.url == "" { + return nil, keyUnavailable("no JWKS URL configured") + } + + s.mu.Lock() + defer s.mu.Unlock() + + cached, found := s.lookupLocked(kid) + if found && !s.fetchedAt.IsZero() && s.now().Sub(s.fetchedAt) < s.ttl { + return cached, nil + } + + // A refetch is wanted: the cache is empty, past its TTL, or missing this + // kid. Honour the refresh floor so misses cannot become a request flood. + if s.lastAttempt.IsZero() || s.now().Sub(s.lastAttempt) >= s.minRefreshInterval { + if err := s.refreshLocked(); err != nil { + // Prefer a stale-but-known key over failing the request. + if found { + return cached, nil + } + return nil, err + } + if key, ok := s.lookupLocked(kid); ok { + return key, nil + } + } else if found { + // Throttled out of refreshing, but the cache can still answer. + return cached, nil + } + + if kid == "" { + return nil, keyUnavailable("JWKS at %s holds %d usable keys; token has no kid to select one", + s.url, len(s.keys)) + } + return nil, keyUnavailable("JWKS at %s holds no key for kid %q", s.url, kid) +} + +// lookupLocked resolves kid against the cache. Callers must hold s.mu. +func (s *JWKSKeySource) lookupLocked(kid string) (*rsa.PublicKey, bool) { + if kid != "" { + key, ok := s.keys[kid] + return key, ok + } + + // No kid: unambiguous only when the set holds a single key. + if len(s.keys) != 1 { + return nil, false + } + for _, key := range s.keys { + return key, true + } + return nil, false +} + +// refreshLocked fetches and replaces the cached key set. The cache is left +// untouched on failure so a stale set remains available. Callers must hold s.mu. +func (s *JWKSKeySource) refreshLocked() error { + s.lastAttempt = s.now() + + keys, err := s.fetch() + if err != nil { + return err + } + + s.keys = keys + s.fetchedAt = s.now() + return nil +} + +// fetch retrieves and decodes the key set. +func (s *JWKSKeySource) fetch() (map[string]*rsa.PublicKey, error) { + res, err := s.httpClient.Get(s.url) + if err != nil { + return nil, keyUnavailable("fetching JWKS from %s: %w", s.url, err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return nil, keyUnavailable("fetching JWKS from %s: unexpected status %s", s.url, res.Status) + } + + body, err := io.ReadAll(io.LimitReader(res.Body, maxJWKSResponseBytes)) + if err != nil { + return nil, keyUnavailable("reading JWKS from %s: %w", s.url, err) + } + + var doc jwksDocument + if err := json.Unmarshal(body, &doc); err != nil { + return nil, keyUnavailable("decoding JWKS from %s: %w", s.url, err) + } + + keys := map[string]*rsa.PublicKey{} + for _, k := range doc.Keys { + key, ok := k.rsaPublicKey() + if !ok { + // Skip anything not an RS256-capable RSA key (EC keys, encryption + // keys, malformed entries) rather than failing the whole set. + continue + } + keys[k.Kid] = key + } + + if len(keys) == 0 { + return nil, keyUnavailable("JWKS from %s contains no usable RSA keys", s.url) + } + + return keys, nil +} + +type jwksDocument struct { + Keys []jwksKey `json:"keys"` +} + +// jwksKey is the subset of RFC 7517 needed to verify an RS256 signature. +type jwksKey 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"` +} + +// rsaPublicKey rebuilds the RSA public key from the JWK's modulus/exponent. +// It reports false for keys that cannot verify an RS256 signature. +func (k jwksKey) rsaPublicKey() (*rsa.PublicKey, bool) { + if k.Kty != "RSA" { + return nil, false + } + // "use" and "alg" are optional; reject only an explicit mismatch. + if k.Use != "" && k.Use != "sig" { + return nil, false + } + if k.Alg != "" && k.Alg != "RS256" { + return nil, false + } + + nBytes, err := decodeBase64URL(k.N) + if err != nil || len(nBytes) == 0 { + return nil, false + } + eBytes, err := decodeBase64URL(k.E) + if err != nil || len(eBytes) == 0 { + return nil, false + } + + // Bound the exponent so the big.Int → int conversion is safe on every + // platform. Real exponents are 65537. + e := new(big.Int).SetBytes(eBytes) + if !e.IsInt64() || e.Int64() <= 0 || e.Int64() > math.MaxInt32 { + return nil, false + } + + return &rsa.PublicKey{ + N: new(big.Int).SetBytes(nBytes), + E: int(e.Int64()), + }, true +} + +// decodeBase64URL decodes a JWK field. RFC 7517 mandates unpadded base64url, +// but padded input is accepted too since some issuers emit it. +func decodeBase64URL(s string) ([]byte, error) { + return base64.RawURLEncoding.DecodeString(strings.TrimRight(s, "=")) +} diff --git a/jwtutil/jwks_test.go b/jwtutil/jwks_test.go new file mode 100644 index 0000000..3ce69f7 --- /dev/null +++ b/jwtutil/jwks_test.go @@ -0,0 +1,424 @@ +package jwtutil + +import ( + "crypto/rand" + "crypto/rsa" + "encoding/base64" + "encoding/json" + "fmt" + "math/big" + "net/http" + "net/http/httptest" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// jwksBody renders the JWKS document authn would serve for the given keys. +func jwksBody(t *testing.T, keys map[string]*rsa.PublicKey) string { + t.Helper() + + doc := jwksDocument{} + for kid, pub := range keys { + doc.Keys = append(doc.Keys, jwksKey{ + Kty: "RSA", + Use: "sig", + Alg: "RS256", + Kid: kid, + N: base64.RawURLEncoding.EncodeToString(pub.N.Bytes()), + E: base64.RawURLEncoding.EncodeToString(big.NewInt(int64(pub.E)).Bytes()), + }) + } + + body, err := json.Marshal(doc) + require.NoError(t, err) + return string(body) +} + +func newToken(t *testing.T, key *rsa.PrivateKey, kid string, d time.Duration) string { + t.Helper() + + token, err := CreateToken(CreateTokenOptions{ + Username: "alice", + Groups: []string{"admins"}, + Duration: d, + KeyID: kid, + PrivateKey: key, + }) + require.NoError(t, err) + return token +} + +// TestJWKSKeySourceValidatesRealToken is the end-to-end check: a token signed +// the way authn signs it must validate against a key fetched from a JWKS +// endpoint, exercising the n/e round trip and the kid lookup. +func TestJWKSKeySourceValidatesRealToken(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const kid = "krateo-authn-key-1" + body := jwksBody(t, map[string]*rsa.PublicKey{kid: &key.PublicKey}) + + var hits int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt64(&hits, 1) + assert.Equal(t, DefaultJWKSPath, r.URL.Path) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, body) + })) + defer srv.Close() + + src := NewJWKSKeySource(JWKSURL(srv.URL)) + + // Nothing is fetched at construction. + assert.Equal(t, int64(0), atomic.LoadInt64(&hits)) + + info, err := ValidateWithKeySource(src, newToken(t, key, kid, time.Minute)) + require.NoError(t, err) + assert.Equal(t, "alice", info.Username) + assert.ElementsMatch(t, []string{"admins"}, info.Groups) + assert.Equal(t, int64(1), atomic.LoadInt64(&hits), "first validation triggers the fetch") + + // Second validation is served from cache. + _, err = ValidateWithKeySource(src, newToken(t, key, kid, time.Minute)) + require.NoError(t, err) + assert.Equal(t, int64(1), atomic.LoadInt64(&hits), "cached key set must not refetch") +} + +// TestJWKSKeySourceRefetchesAfterTTL pins the cache-expiry behaviour the chart's +// TTL value controls. +func TestJWKSKeySourceRefetchesAfterTTL(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const kid = "kid-1" + body := jwksBody(t, map[string]*rsa.PublicKey{kid: &key.PublicKey}) + + var hits int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt64(&hits, 1) + fmt.Fprint(w, body) + })) + defer srv.Close() + + now := time.Now() + src := NewJWKSKeySource(JWKSURL(srv.URL), + WithJWKSCacheTTL(time.Minute), + WithJWKSMinRefreshInterval(time.Second)) + src.now = func() time.Time { return now } + + _, err = src.PublicKey(kid) + require.NoError(t, err) + assert.Equal(t, int64(1), atomic.LoadInt64(&hits)) + + // Inside the TTL: no refetch. + now = now.Add(30 * time.Second) + _, err = src.PublicKey(kid) + require.NoError(t, err) + assert.Equal(t, int64(1), atomic.LoadInt64(&hits)) + + // Past the TTL: refetch. + now = now.Add(31 * time.Second) + _, err = src.PublicKey(kid) + require.NoError(t, err) + assert.Equal(t, int64(2), atomic.LoadInt64(&hits)) +} + +// TestJWKSKeySourceUnknownKidIsThrottled guards the stampede case: a flood of +// tokens naming a kid authn never published must not become a fetch per request. +func TestJWKSKeySourceUnknownKidIsThrottled(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + body := jwksBody(t, map[string]*rsa.PublicKey{"known": &key.PublicKey}) + + var hits int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt64(&hits, 1) + fmt.Fprint(w, body) + })) + defer srv.Close() + + now := time.Now() + src := NewJWKSKeySource(JWKSURL(srv.URL), WithJWKSMinRefreshInterval(30*time.Second)) + src.now = func() time.Time { return now } + + for i := 0; i < 20; i++ { + _, err := src.PublicKey("rotated-away") + require.Error(t, err) + assert.ErrorIs(t, err, ErrKeyUnavailable) + } + assert.Equal(t, int64(1), atomic.LoadInt64(&hits), "unknown kid must not refetch per request") + + // Past the refresh floor, exactly one more attempt is allowed. + now = now.Add(31 * time.Second) + _, err = src.PublicKey("rotated-away") + require.Error(t, err) + assert.Equal(t, int64(2), atomic.LoadInt64(&hits)) +} + +// TestJWKSKeySourcePicksUpRotatedKid covers rotation: a kid absent from the +// cache triggers a refetch that discovers it. +func TestJWKSKeySourcePicksUpRotatedKid(t *testing.T) { + oldKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + newKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + var mu sync.Mutex + body := jwksBody(t, map[string]*rsa.PublicKey{"old": &oldKey.PublicKey}) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + defer mu.Unlock() + fmt.Fprint(w, body) + })) + defer srv.Close() + + src := NewJWKSKeySource(JWKSURL(srv.URL), WithJWKSMinRefreshInterval(time.Nanosecond)) + + _, err = ValidateWithKeySource(src, newToken(t, oldKey, "old", time.Minute)) + require.NoError(t, err) + + // authn rotates its keypair. + mu.Lock() + body = jwksBody(t, map[string]*rsa.PublicKey{"new": &newKey.PublicKey}) + mu.Unlock() + + info, err := ValidateWithKeySource(src, newToken(t, newKey, "new", time.Minute)) + require.NoError(t, err, "an unseen kid must trigger a refetch") + assert.Equal(t, "alice", info.Username) +} + +// TestJWKSKeySourceServesStaleKeyWhenEndpointDown is the availability +// guarantee: an authn blip must not invalidate tokens we could already verify. +func TestJWKSKeySourceServesStaleKeyWhenEndpointDown(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const kid = "kid-1" + body := jwksBody(t, map[string]*rsa.PublicKey{kid: &key.PublicKey}) + + var down atomic.Bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if down.Load() { + w.WriteHeader(http.StatusInternalServerError) + return + } + fmt.Fprint(w, body) + })) + defer srv.Close() + + now := time.Now() + src := NewJWKSKeySource(JWKSURL(srv.URL), + WithJWKSCacheTTL(time.Minute), + WithJWKSMinRefreshInterval(time.Nanosecond)) + src.now = func() time.Time { return now } + + _, err = src.PublicKey(kid) + require.NoError(t, err) + + // authn goes down and the cache goes stale. + down.Store(true) + now = now.Add(2 * time.Minute) + + got, err := src.PublicKey(kid) + require.NoError(t, err, "a stale-but-known key must still be served") + assert.Equal(t, &key.PublicKey, got) +} + +// TestJWKSKeySourceUnreachableIsKeyUnavailable ensures a cold cache plus a dead +// endpoint is reported as our fault (503-worthy), not as an invalid token. +func TestJWKSKeySourceUnreachableIsKeyUnavailable(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + })) + srv.Close() // nothing is listening + + src := NewJWKSKeySource(JWKSURL(srv.URL)) + + _, err = ValidateWithKeySource(src, newToken(t, key, "kid-1", time.Minute)) + require.Error(t, err) + assert.ErrorIs(t, err, ErrKeyUnavailable) + assert.NotErrorIs(t, err, ErrTokenInvalid, + "an unreachable JWKS must not be reported as a bad token") +} + +// TestJWKSKeySourceRejectsWrongKey confirms a genuinely bad signature is still +// ErrTokenInvalid even though the key now arrives over the network. +func TestJWKSKeySourceRejectsWrongKey(t *testing.T) { + served, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + attacker, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const kid = "kid-1" + body := jwksBody(t, map[string]*rsa.PublicKey{kid: &served.PublicKey}) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, body) + })) + defer srv.Close() + + src := NewJWKSKeySource(JWKSURL(srv.URL)) + + _, err = ValidateWithKeySource(src, newToken(t, attacker, kid, time.Minute)) + assert.ErrorIs(t, err, ErrTokenInvalid) +} + +// TestJWKSKeySourceExpiredTokenStillExpired keeps the expiry signal distinct +// from both invalid-token and key-unavailable. +func TestJWKSKeySourceExpiredTokenStillExpired(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const kid = "kid-1" + body := jwksBody(t, map[string]*rsa.PublicKey{kid: &key.PublicKey}) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, body) + })) + defer srv.Close() + + src := NewJWKSKeySource(JWKSURL(srv.URL)) + + _, err = ValidateWithKeySource(src, newToken(t, key, kid, -time.Hour)) + assert.ErrorIs(t, err, ErrTokenExpired) +} + +// TestJWKSKeySourceEmptyKidResolvesSingleKey covers the no-kid token against a +// single-key set. +func TestJWKSKeySourceEmptyKidResolvesSingleKey(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + body := jwksBody(t, map[string]*rsa.PublicKey{"only": &key.PublicKey}) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, body) + })) + defer srv.Close() + + src := NewJWKSKeySource(JWKSURL(srv.URL)) + + got, err := src.PublicKey("") + require.NoError(t, err) + assert.Equal(t, &key.PublicKey, got) +} + +// TestJWKSKeySourceSingleFlightsConcurrentMisses documents that a burst of +// concurrent cold-cache validations collapses into one fetch. +func TestJWKSKeySourceSingleFlightsConcurrentMisses(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const kid = "kid-1" + body := jwksBody(t, map[string]*rsa.PublicKey{kid: &key.PublicKey}) + + var hits int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt64(&hits, 1) + fmt.Fprint(w, body) + })) + defer srv.Close() + + src := NewJWKSKeySource(JWKSURL(srv.URL)) + + var wg sync.WaitGroup + for i := 0; i < 25; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _, err := src.PublicKey(kid) + assert.NoError(t, err) + }() + } + wg.Wait() + + assert.Equal(t, int64(1), atomic.LoadInt64(&hits)) +} + +// TestJWKSKeySourceSkipsUnusableKeys checks that non-RS256 / non-RSA entries are +// ignored rather than poisoning the whole set. +func TestJWKSKeySourceSkipsUnusableKeys(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + doc := jwksDocument{Keys: []jwksKey{ + {Kty: "EC", Kid: "ec-key", Use: "sig", N: "x", E: "AQAB"}, + {Kty: "RSA", Kid: "enc-key", Use: "enc", N: "x", E: "AQAB"}, + { + Kty: "RSA", Use: "sig", Alg: "RS256", Kid: "good", + N: base64.RawURLEncoding.EncodeToString(key.PublicKey.N.Bytes()), + E: base64.RawURLEncoding.EncodeToString(big.NewInt(int64(key.PublicKey.E)).Bytes()), + }, + }} + raw, err := json.Marshal(doc) + require.NoError(t, err) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Write(raw) + })) + defer srv.Close() + + src := NewJWKSKeySource(JWKSURL(srv.URL)) + + got, err := src.PublicKey("good") + require.NoError(t, err) + assert.Equal(t, &key.PublicKey, got) + + _, err = src.PublicKey("ec-key") + assert.ErrorIs(t, err, ErrKeyUnavailable) +} + +// TestJWKSKeySourceEmptySetIsError ensures an empty/garbage document does not +// install an empty cache that silently rejects everything as "invalid token". +func TestJWKSKeySourceEmptySetIsError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"keys":[]}`) + })) + defer srv.Close() + + src := NewJWKSKeySource(JWKSURL(srv.URL)) + + _, err := src.PublicKey("kid-1") + assert.ErrorIs(t, err, ErrKeyUnavailable) +} + +func TestJWKSURL(t *testing.T) { + assert.Equal(t, "http://authn:8082"+DefaultJWKSPath, JWKSURL("http://authn:8082")) + assert.Equal(t, "http://authn:8082"+DefaultJWKSPath, JWKSURL("http://authn:8082/")) + assert.Equal(t, "http://authn:8082"+DefaultJWKSPath, JWKSURL(" http://authn:8082 ")) + assert.Equal(t, "https://x/custom.json", JWKSURL("https://x/custom.json")) + assert.Equal(t, "", JWKSURL("")) +} + +// TestValidateStillAcceptsStaticKey pins backwards compatibility of the +// single-key Validate form. +func TestValidateStillAcceptsStaticKey(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + info, err := Validate(&key.PublicKey, newToken(t, key, "kid-1", time.Minute)) + require.NoError(t, err) + assert.Equal(t, "alice", info.Username) +} + +func TestDecodeBase64URLAcceptsPadding(t *testing.T) { + raw := []byte{0x01, 0x02, 0x03, 0x04, 0x05} + + unpadded, err := decodeBase64URL(base64.RawURLEncoding.EncodeToString(raw)) + require.NoError(t, err) + assert.Equal(t, raw, unpadded) + + padded, err := decodeBase64URL(base64.URLEncoding.EncodeToString(raw)) + require.NoError(t, err) + assert.Equal(t, raw, padded) +} diff --git a/jwtutil/keys.go b/jwtutil/keys.go new file mode 100644 index 0000000..6b72089 --- /dev/null +++ b/jwtutil/keys.go @@ -0,0 +1,19 @@ +package jwtutil + +import ( + "crypto/rsa" + + "github.com/golang-jwt/jwt/v5" +) + +// ParseRSAPrivateKeyFromPEM parses a PEM-encoded RSA private key, as produced +// by e.g. `openssl genrsa`. It is the key type expected by CreateToken. +func ParseRSAPrivateKeyFromPEM(pemBytes []byte) (*rsa.PrivateKey, error) { + return jwt.ParseRSAPrivateKeyFromPEM(pemBytes) +} + +// ParseRSAPublicKeyFromPEM parses a PEM-encoded RSA public key. It is the key +// type expected by Validate. +func ParseRSAPublicKeyFromPEM(pemBytes []byte) (*rsa.PublicKey, error) { + return jwt.ParseRSAPublicKeyFromPEM(pemBytes) +} diff --git a/jwtutil/userinfo_test.go b/jwtutil/userinfo_test.go index d0645ba..4082833 100644 --- a/jwtutil/userinfo_test.go +++ b/jwtutil/userinfo_test.go @@ -1,6 +1,8 @@ package jwtutil_test import ( + "crypto/rand" + "crypto/rsa" "testing" "time" @@ -9,9 +11,10 @@ import ( ) func TestExtractUserInfo(t *testing.T) { - const ( - secret = "test-secret" - ) + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const keyID = "test-key-id" tests := []struct { name string @@ -26,7 +29,8 @@ func TestExtractUserInfo(t *testing.T) { Username: "alice", Groups: []string{"admin", "dev"}, Duration: time.Minute, - SigningKey: secret, + KeyID: keyID, + PrivateKey: privateKey, }) return token }, @@ -42,7 +46,8 @@ func TestExtractUserInfo(t *testing.T) { token, _ := jwtutil.CreateToken(jwtutil.CreateTokenOptions{ Groups: []string{"admin", "dev"}, Duration: time.Minute, - SigningKey: secret, + KeyID: keyID, + PrivateKey: privateKey, }) return token }, @@ -61,7 +66,8 @@ func TestExtractUserInfo(t *testing.T) { token, _ := jwtutil.CreateToken(jwtutil.CreateTokenOptions{ Username: "bob", Duration: time.Minute, - SigningKey: secret, + KeyID: keyID, + PrivateKey: privateKey, }) return token }, diff --git a/jwtutil/validate.go b/jwtutil/validate.go index dab8d47..0dc901a 100644 --- a/jwtutil/validate.go +++ b/jwtutil/validate.go @@ -1,6 +1,7 @@ package jwtutil import ( + "crypto/rsa" "errors" "fmt" "time" @@ -13,16 +14,57 @@ var ( ErrTokenInvalid = errors.New("token is invalid") ) -func Validate(signingKey, bearer string) (UserInfo, error) { - if signingKey == "" { - return UserInfo{}, fmt.Errorf("signing key cannot be empty") +// Validate parses and verifies the given bearer token against the supplied +// RSA public key. Only the RS256 asymmetric algorithm is accepted; tokens +// signed with any other method (in particular symmetric HMAC) are rejected to +// prevent algorithm-confusion attacks. +// +// Prefer ValidateWithKeySource when the key comes from authn's JWKS endpoint; +// this form is for callers holding a single key already. +func Validate(publicKey *rsa.PublicKey, bearer string) (UserInfo, error) { + if publicKey == nil { + return UserInfo{}, fmt.Errorf("public key cannot be nil") + } + + return ValidateWithKeySource(NewStaticKeySource(publicKey), bearer) +} + +// ValidateWithKeySource parses and verifies the given bearer token, resolving +// the verification key through keys using the token's "kid" header. This is the +// JWKS-friendly form: a JWKSKeySource fetches authn's published key set and +// caches it, so key rotation needs no redeploy of the validating component. +// +// As with Validate, only RS256 is accepted — a token offering any other +// algorithm (notably symmetric HMAC) is rejected before the key is consulted, +// which is what makes algorithm confusion impossible here. +// +// Errors are separated by fault so callers can answer with the right status: +// ErrTokenExpired and ErrTokenInvalid mean the caller's token is at fault +// (401), whereas ErrKeyUnavailable means we could not obtain the key (503) and +// says nothing about the token. +func ValidateWithKeySource(keys KeySource, bearer string) (UserInfo, error) { + if keys == nil { + return UserInfo{}, fmt.Errorf("key source cannot be nil") } tok, err := jwt.ParseWithClaims(bearer, &KrateoClaims{}, func(token *jwt.Token) (any, error) { - return []byte(signingKey), nil + if _, ok := token.Method.(*jwt.SigningMethodRSA); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) + } + // A non-string or absent kid becomes "", which a single-key set + // still resolves; see JWKSKeySource.PublicKey. + kid, _ := token.Header["kid"].(string) + return keys.PublicKey(kid) }, jwt.WithLeeway(5*time.Second)) if err != nil { + // Key-resolution failures surface through the keyfunc wrapped in the + // parse error. They are OUR fault, not the token's, so they must not be + // flattened into ErrTokenInvalid — that would answer 401 and tell the + // client to re-authenticate against an authn that is simply unreachable. + if errors.Is(err, ErrKeyUnavailable) { + return UserInfo{}, err + } if !errors.Is(err, jwt.ErrTokenExpired) { return UserInfo{}, ErrTokenInvalid } diff --git a/jwtutil/validate_test.go b/jwtutil/validate_test.go index 015b48d..0e658fb 100644 --- a/jwtutil/validate_test.go +++ b/jwtutil/validate_test.go @@ -1,17 +1,21 @@ package jwtutil_test import ( + "crypto/rand" + "crypto/rsa" "testing" "time" "github.com/krateo-platformops/plumbing/jwtutil" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetUserInfo(t *testing.T) { - const ( - secret = "test-secret" - ) + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const keyID = "test-key-id" tests := []struct { title string @@ -27,7 +31,8 @@ func TestGetUserInfo(t *testing.T) { Username: "alice", Groups: []string{"admin", "dev"}, Duration: time.Minute, - SigningKey: secret, + KeyID: keyID, + PrivateKey: privateKey, }) return token }, @@ -42,7 +47,8 @@ func TestGetUserInfo(t *testing.T) { Username: "bob", Groups: []string{"users"}, Duration: -time.Minute, - SigningKey: secret, + KeyID: keyID, + PrivateKey: privateKey, }) return token }, @@ -61,7 +67,7 @@ func TestGetUserInfo(t *testing.T) { t.Run(tc.title, func(t *testing.T) { token := tc.prepare() - user, err := jwtutil.Validate(secret, token) + user, err := jwtutil.Validate(&privateKey.PublicKey, token) if tc.expectErr { assert.Error(t, err) @@ -74,3 +80,25 @@ func TestGetUserInfo(t *testing.T) { }) } } + +// TestValidateRejectsWrongKey ensures a token signed by one keypair does not +// validate against a different public key. +func TestValidateRejectsWrongKey(t *testing.T) { + signingKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + otherKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + token, err := jwtutil.CreateToken(jwtutil.CreateTokenOptions{ + Username: "alice", + Groups: []string{"admin"}, + Duration: time.Minute, + KeyID: "kid", + PrivateKey: signingKey, + }) + require.NoError(t, err) + + _, err = jwtutil.Validate(&otherKey.PublicKey, token) + assert.ErrorIs(t, err, jwtutil.ErrTokenInvalid) +} diff --git a/server/use/logger_test.go b/server/use/logger_test.go index cf55879..af3d232 100644 --- a/server/use/logger_test.go +++ b/server/use/logger_test.go @@ -2,6 +2,8 @@ package use import ( "bytes" + "crypto/rand" + "crypto/rsa" "fmt" "log/slog" "net/http" @@ -33,11 +35,17 @@ func TestLoggerMiddleware(t *testing.T) { route := NewChain(Logger(log)).Then(sillyHandler) + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + bearer, err := jwtutil.CreateToken(jwtutil.CreateTokenOptions{ Username: "cyberjoker", Groups: []string{"devs", "testers"}, Duration: time.Minute * 2, - SigningKey: "abbracadabbra", + KeyID: "test-kid", + PrivateKey: privateKey, }) if err != nil { t.Fatal(err) diff --git a/server/use/userconfig.go b/server/use/userconfig.go index 5a043a7..4617e87 100644 --- a/server/use/userconfig.go +++ b/server/use/userconfig.go @@ -16,9 +16,19 @@ import ( "k8s.io/client-go/rest" ) -func UserConfig(signingKey, authnNS string) func(http.Handler) http.Handler { +// UserConfig builds a middleware that validates the incoming bearer token. +// keys resolves the RSA public key a token was signed with; in production this +// is a jwtutil.JWKSKeySource pointed at authn's /.well-known/jwks.json, which +// fetches and caches the key set so rotating authn's keypair needs no redeploy +// here. +func UserConfig(keys jwtutil.KeySource, authnNS string) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { fn := func(wri http.ResponseWriter, req *http.Request) { + if keys == nil { + response.InternalError(wri, fmt.Errorf("no JWT key source configured")) + return + } + authHeader := req.Header.Get("Authorization") if authHeader == "" { response.Unauthorized(wri, fmt.Errorf("missing authorization header")) @@ -31,8 +41,15 @@ func UserConfig(signingKey, authnNS string) func(http.Handler) http.Handler { return } - userInfo, err := jwtutil.Validate(signingKey, parts[1]) + userInfo, err := jwtutil.ValidateWithKeySource(keys, parts[1]) if err != nil { + // The key set being unreachable is our failure, not a bad + // credential: answer 503 so the client retries instead of + // re-authenticating against an authn that is simply down. + if errors.Is(err, jwtutil.ErrKeyUnavailable) { + response.ServiceUnavailable(wri, err) + return + } if errors.Is(err, jwtutil.ErrTokenExpired) { response.Unauthorized(wri, err) } else {