Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ func validateConfig(cfg Config) error {
if cfg.Limits.MaxHTTPResponseBytes <= 0 {
return fmt.Errorf("client: config: limits.max_http_response_bytes must be positive")
}
if cfg.Limits.MaxJOSECompactBytes <= 0 {
return fmt.Errorf("client: config: limits.max_jose_compact_bytes must be positive")
}

if cfg.Profile == ProfileFAPISecurityWithMessageSigning {
if !cfg.Algorithms.RequestObject.IsValid() {
Expand Down
34 changes: 18 additions & 16 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ func validConfig(t *testing.T) client.Config {
MaxClockSkew: 5 * time.Second,
HTTPTimeout: 5 * time.Second,
MaxHTTPResponseBytes: 1 << 16,
MaxJOSECompactBytes: 16 * 1024,
},
}
}
Expand Down Expand Up @@ -211,22 +212,23 @@ func TestNewAcceptsValidConfig(t *testing.T) {

func TestNewRejectsInvalidConfig(t *testing.T) {
cases := map[string]func(*client.Config){
"zero issuer": func(c *client.Config) { c.Issuer = fapi.URL{} },
"empty client id": func(c *client.Config) { c.ClientID = "" },
"empty redirect uri": func(c *client.Config) { c.RedirectURI = "" },
"zero authorization ep": func(c *client.Config) { c.Endpoints.Authorization = fapi.URL{} },
"zero token ep": func(c *client.Config) { c.Endpoints.Token = fapi.URL{} },
"zero par ep": func(c *client.Config) { c.Endpoints.PushedAuthorizationRequest = fapi.URL{} },
"invalid profile": func(c *client.Config) { c.Profile = 0 },
"invalid client auth alg": func(c *client.Config) { c.Algorithms.ClientAuthentication = 0 },
"invalid dpop alg": func(c *client.Config) { c.Algorithms.DPoP = 0 },
"invalid id token alg": func(c *client.Config) { c.Algorithms.IDToken = 0 },
"zero assertion lifetime": func(c *client.Config) { c.Limits.ClientAssertionLifetime = 0 },
"zero session lifetime": func(c *client.Config) { c.Limits.SessionLifetime = 0 },
"zero max id token life": func(c *client.Config) { c.Limits.MaxIDTokenLifetime = 0 },
"negative clock skew": func(c *client.Config) { c.Limits.MaxClockSkew = -time.Second },
"zero http timeout": func(c *client.Config) { c.Limits.HTTPTimeout = 0 },
"zero max response bytes": func(c *client.Config) { c.Limits.MaxHTTPResponseBytes = 0 },
"zero issuer": func(c *client.Config) { c.Issuer = fapi.URL{} },
"empty client id": func(c *client.Config) { c.ClientID = "" },
"empty redirect uri": func(c *client.Config) { c.RedirectURI = "" },
"zero authorization ep": func(c *client.Config) { c.Endpoints.Authorization = fapi.URL{} },
"zero token ep": func(c *client.Config) { c.Endpoints.Token = fapi.URL{} },
"zero par ep": func(c *client.Config) { c.Endpoints.PushedAuthorizationRequest = fapi.URL{} },
"invalid profile": func(c *client.Config) { c.Profile = 0 },
"invalid client auth alg": func(c *client.Config) { c.Algorithms.ClientAuthentication = 0 },
"invalid dpop alg": func(c *client.Config) { c.Algorithms.DPoP = 0 },
"invalid id token alg": func(c *client.Config) { c.Algorithms.IDToken = 0 },
"zero assertion lifetime": func(c *client.Config) { c.Limits.ClientAssertionLifetime = 0 },
"zero session lifetime": func(c *client.Config) { c.Limits.SessionLifetime = 0 },
"zero max id token life": func(c *client.Config) { c.Limits.MaxIDTokenLifetime = 0 },
"negative clock skew": func(c *client.Config) { c.Limits.MaxClockSkew = -time.Second },
"zero http timeout": func(c *client.Config) { c.Limits.HTTPTimeout = 0 },
"zero max response bytes": func(c *client.Config) { c.Limits.MaxHTTPResponseBytes = 0 },
"zero max jose compact bytes": func(c *client.Config) { c.Limits.MaxJOSECompactBytes = 0 },
"id_token key management set without content encryption": func(c *client.Config) {
c.Algorithms.IDTokenKeyManagement = fapi.RSAOAEP256
},
Expand Down
11 changes: 11 additions & 0 deletions client/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,17 @@ type Limits struct {
// MaxHTTPResponseBytes bounds how much of a PAR or token-endpoint
// response body this client reads before failing.
MaxHTTPResponseBytes int64

// MaxJOSECompactBytes bounds how large a JOSE compact serialization
// (JWS or JWE) this client will parse for an ID token or a UserInfo
// response — the two artifacts whose size scales with however many
// scopes/claims this deployment's issuer was asked to grant, so a
// fixed size can fit one user's response and not another's. This is
// deliberately narrower than MaxHTTPResponseBytes, which still
// bounds the outer HTTP read first: a DPoP proof, client assertion
// or request object has no such variability and is not affected by
// this field at all — those stay on jose.DefaultMaxCompactBytes.
MaxJOSECompactBytes int
}

// Config is this client's immutable configuration. It is copied by New;
Expand Down
8 changes: 8 additions & 0 deletions client/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ const (
ErrorInvalidResponse ErrorCode = "invalid_response"
ErrorAuthorizationDenied ErrorCode = "authorization_denied"
ErrorInternal ErrorCode = "internal"

// ErrorResponseTooLarge indicates an ID token or UserInfo response
// exceeded Config.Limits.MaxJOSECompactBytes — distinct from
// ErrorInvalidResponse because the artifact wasn't malformed, it was
// simply larger than configured; PublicDescription names which
// artifact, and the wrapped cause (Unwrap) carries the observed and
// allowed byte counts for logs.
ErrorResponseTooLarge ErrorCode = "response_too_large"
)

// Error is the error type every public Client method returns. Code and
Expand Down
19 changes: 14 additions & 5 deletions client/exchange_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"crypto"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/url"
Expand All @@ -14,6 +15,7 @@ import (
fapi "github.com/idfoundry/fapigo"
"github.com/idfoundry/fapigo/internal/clientassertion"
"github.com/idfoundry/fapigo/internal/dpop"
"github.com/idfoundry/fapigo/internal/jose"
"github.com/idfoundry/fapigo/internal/jwe"
"github.com/idfoundry/fapigo/internal/par"
"github.com/idfoundry/fapigo/internal/token"
Expand Down Expand Up @@ -298,12 +300,16 @@ func (c *Client) validateIDToken(ctx context.Context, raw, nonce string) (token.
func (c *Client) decryptIDToken(ctx context.Context, raw string) (string, *Error) {
unwrapper := decrypterUnwrapper{decrypter: c.deps.Decryption, purpose: keys.IDTokenDecryption}
result, err := jwe.Decrypt(ctx, jwe.DecryptRequest{
Algorithm: c.cfg.Algorithms.IDTokenKeyManagement,
Encryption: c.cfg.Algorithms.IDTokenContentEncryption,
RecipientKey: unwrapper,
Compact: raw,
Algorithm: c.cfg.Algorithms.IDTokenKeyManagement,
Encryption: c.cfg.Algorithms.IDTokenContentEncryption,
RecipientKey: unwrapper,
Compact: raw,
MaxCompactBytes: c.cfg.Limits.MaxJOSECompactBytes,
})
if err != nil {
if errors.Is(err, jwe.ErrTooLarge) {
return "", newError(ErrorResponseTooLarge, "ID token exceeds the configured size limit", err)
}
return "", newError(ErrorInvalidResponse, "ID token decryption failed", err)
}
// RFC 7519 §5.2 requires a producer of a nested JWT to set cty to
Expand Down Expand Up @@ -340,8 +346,11 @@ func isAcceptableNestedJWTContentType(cty string) bool {
// either one that arrived that way directly, or the inner JWT
// decryptIDToken recovered from an encrypted one.
func (c *Client) validateSignedIDToken(ctx context.Context, raw, nonce string) (token.ValidatedIDToken, *Error) {
parsed, err := token.ParseIDToken(raw)
parsed, err := token.ParseIDTokenMax(raw, c.cfg.Limits.MaxJOSECompactBytes)
if err != nil {
if errors.Is(err, jose.ErrTooLarge) {
return token.ValidatedIDToken{}, newError(ErrorResponseTooLarge, "ID token exceeds the configured size limit", err)
}
return token.ValidatedIDToken{}, newError(ErrorInvalidResponse, "malformed ID token", err)
}

Expand Down
4 changes: 2 additions & 2 deletions client/idtoken_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func idTokenTestClient(t *testing.T, now time.Time, algorithms Algorithms, decry
Issuer: issuer,
ClientID: fapi.ClientID(idTokenTestClientID),
Algorithms: algorithms,
Limits: Limits{MaxIDTokenLifetime: 5 * time.Minute, MaxClockSkew: 5 * time.Second},
Limits: Limits{MaxIDTokenLifetime: 5 * time.Minute, MaxClockSkew: 5 * time.Second, MaxJOSECompactBytes: 16 * 1024},
},
deps: Dependencies{
IssuerKeys: fakeIDTokenIssuerKeys{pub: &idKey.PublicKey},
Expand All @@ -100,7 +100,7 @@ func idTokenTestClientWithIssuerKeys(t *testing.T, now time.Time, algorithms Alg
Issuer: issuer,
ClientID: fapi.ClientID(idTokenTestClientID),
Algorithms: algorithms,
Limits: Limits{MaxIDTokenLifetime: 5 * time.Minute, MaxClockSkew: 5 * time.Second},
Limits: Limits{MaxIDTokenLifetime: 5 * time.Minute, MaxClockSkew: 5 * time.Second, MaxJOSECompactBytes: 16 * 1024},
},
deps: Dependencies{
IssuerKeys: issuerKeys,
Expand Down
6 changes: 5 additions & 1 deletion client/issuerjws.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package client

import (
"context"
"errors"

"github.com/idfoundry/fapigo/internal/jose"
"github.com/idfoundry/fapigo/keys"
Expand All @@ -21,8 +22,11 @@ import (
// caller still owns checking whatever claims that payload carries (iss,
// aud, sub, expiry, ...) against its own policy.
func (c *Client) VerifyIssuerJWS(ctx context.Context, compactJWS string) ([]byte, error) {
parsed, err := jose.ParseCompact(compactJWS)
parsed, err := jose.ParseCompactMax(compactJWS, c.cfg.Limits.MaxJOSECompactBytes)
if err != nil {
if errors.Is(err, jose.ErrTooLarge) {
return nil, newError(ErrorResponseTooLarge, "JWS exceeds the configured size limit", err)
}
return nil, newError(ErrorInvalidResponse, "malformed JWS", err)
}

Expand Down
1 change: 1 addition & 0 deletions client/issuerjws_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func issuerJWSTestClient(t *testing.T, issuerKeys keys.IssuerKeySource) *Client
Issuer: issuer,
ClientID: fapi.ClientID(idTokenTestClientID),
Algorithms: Algorithms{UserInfo: fapi.ES256},
Limits: Limits{MaxJOSECompactBytes: 16 * 1024},
},
deps: Dependencies{IssuerKeys: issuerKeys},
}
Expand Down
11 changes: 11 additions & 0 deletions client/presets.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"time"

fapi "github.com/idfoundry/fapigo"
"github.com/idfoundry/fapigo/internal/jose"
)

// RecommendedLimits returns a Limits value grounded the same way
Expand Down Expand Up @@ -62,6 +63,16 @@ func RecommendedLimits() Limits {
// small JSON documents; 1 MiB is generous headroom without
// being an effectively unbounded read.
MaxHTTPResponseBytes: 1 << 20,

// Not spec-mandated — the same 16 KiB this module's own JOSE
// package uses as its default ceiling for a fixed-shape
// artifact (jose.DefaultMaxCompactBytes), carried over here as
// a starting point for an ID token or UserInfo response too. A
// deployment whose issuer is configured to return many claims
// (verified/assured identity claims, in particular, can run
// well past this) should raise it explicitly rather than rely
// on this default.
MaxJOSECompactBytes: jose.DefaultMaxCompactBytes,
}
}

Expand Down
13 changes: 9 additions & 4 deletions client/userinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package client
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"mime"
Expand Down Expand Up @@ -149,12 +150,16 @@ func (c *Client) decodeUserInfoJWT(ctx context.Context, raw string, expectEncryp
func (c *Client) decryptUserInfoJWE(ctx context.Context, raw string) (string, *Error) {
unwrapper := decrypterUnwrapper{decrypter: c.deps.Decryption, purpose: keys.UserInfoDecryption}
result, err := jwe.Decrypt(ctx, jwe.DecryptRequest{
Algorithm: c.cfg.Algorithms.UserInfoKeyManagement,
Encryption: c.cfg.Algorithms.UserInfoContentEncryption,
RecipientKey: unwrapper,
Compact: raw,
Algorithm: c.cfg.Algorithms.UserInfoKeyManagement,
Encryption: c.cfg.Algorithms.UserInfoContentEncryption,
RecipientKey: unwrapper,
Compact: raw,
MaxCompactBytes: c.cfg.Limits.MaxJOSECompactBytes,
})
if err != nil {
if errors.Is(err, jwe.ErrTooLarge) {
return "", newError(ErrorResponseTooLarge, "UserInfo response exceeds the configured size limit", err)
}
return "", newError(ErrorInvalidResponse, "UserInfo response decryption failed", err)
}
if !isAcceptableNestedJWTContentType(result.Header.ContentType) {
Expand Down
31 changes: 31 additions & 0 deletions client/userinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"crypto/elliptic"
"crypto/rand"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"testing"
Expand Down Expand Up @@ -126,6 +127,36 @@ func TestFetchUserInfoVerifiesSignedJWTResponse(t *testing.T) {
}
}

func TestFetchUserInfoRejectsSignedJWTOverConfiguredSizeLimit(t *testing.T) {
userInfoKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
t.Fatalf("generate userinfo key: %v", err)
}
// A large claim value stands in for a claims-heavy response (many
// granted scopes/claims) that legitimately exceeds a small
// configured MaxJOSECompactBytes, distinct from a malformed one.
bigValue := make([]byte, 4096)
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
compact := signUserInfoJWS(t, userInfoKey, map[string]any{"sub": userInfoTestSubject, "big": string(bigValue)})
w.Header().Set("Content-Type", "application/jwt")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(compact))
}))
defer ts.Close()

c := newUserInfoTestClient(t, ts, &userInfoKey.PublicKey, func(cfg *client.Config) {
cfg.Limits.MaxJOSECompactBytes = 1024
}, nil)
_, err = c.FetchUserInfo(context.Background(), userInfoTestTokens())
var cerr *client.Error
if !errors.As(err, &cerr) {
t.Fatalf("FetchUserInfo error = %v, want *client.Error", err)
}
if cerr.Code() != client.ErrorResponseTooLarge {
t.Fatalf("FetchUserInfo error code = %q, want %q", cerr.Code(), client.ErrorResponseTooLarge)
}
}

func TestFetchUserInfoRejectsSignedJWTWithWrongKey(t *testing.T) {
signingKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions cmd/conformance-as/smoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ func newSmokeHarness(t *testing.T, format AccessTokenFormat) *smokeHarness {
MaxClockSkew: 10 * time.Second,
HTTPTimeout: 10 * time.Second,
MaxHTTPResponseBytes: 1 << 16,
MaxJOSECompactBytes: 16 * 1024,
},
}
clientDeps := client.Dependencies{
Expand Down
1 change: 1 addition & 0 deletions cmd/conformance-client/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@
// to usefully treat as fatal. The suite's own graded result — fetched
// separately, after a grace period, back in run — is what actually
// matters.
func runModule(ctx context.Context, d moduleDriver, testName string) string {

Check failure on line 232 in cmd/conformance-client/main.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=IDFoundry_FAPIgo&issues=AaA-NZBfgbujzTJ_qb1G&open=AaA-NZBfgbujzTJ_qb1G&pullRequest=162
rawHTTP, apiBase, planID, clientID, redirectURI, keyMgr, profile := d.HTTP, d.APIBase, d.PlanID, d.ClientID, d.RedirectURI, d.Keys, d.Profile
module, err := createModuleInstance(rawHTTP, apiBase, planID, testName)
if err != nil {
Expand Down Expand Up @@ -285,6 +285,7 @@
MaxClockSkew: 15 * time.Second,
HTTPTimeout: fetchTimeout,
MaxHTTPResponseBytes: 1 << 20,
MaxJOSECompactBytes: 16 * 1024,
}
if profile.signRequestObject {
// This driver's key manager only ever generates ES256 keys, so it
Expand Down
1 change: 1 addition & 0 deletions fapitest/discover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ func TestDiscoverEndToEnd(t *testing.T) {
ClientAssertionLifetime: time.Minute, SessionLifetime: 5 * time.Minute,
MaxIDTokenLifetime: 5 * time.Minute, MaxClockSkew: 5 * time.Second,
HTTPTimeout: 5 * time.Second, MaxHTTPResponseBytes: 1 << 16,
MaxJOSECompactBytes: 16 * 1024,
},
}
clientDeps := client.Dependencies{
Expand Down
1 change: 1 addition & 0 deletions fapitest/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ func New(t *testing.T, cfg Config) *Harness {
MaxClockSkew: 5 * time.Second,
HTTPTimeout: 5 * time.Second,
MaxHTTPResponseBytes: 1 << 16,
MaxJOSECompactBytes: 16 * 1024,
},
}
if cfg.EncryptIDTokens {
Expand Down
25 changes: 18 additions & 7 deletions internal/jose/compact.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ import (
fapi "github.com/idfoundry/fapigo"
)

// maxCompactBytes bounds how large a compact serialization this package
// will attempt to parse, to avoid doing unbounded work on attacker-
// supplied input before any signature has been checked.
const maxCompactBytes = 16 * 1024
// DefaultMaxCompactBytes bounds how large a compact serialization
// ParseCompact will attempt to parse, to avoid doing unbounded work on
// attacker-supplied input before any signature has been checked. It's
// sized for a JWS whose claims are fixed in shape and count (a DPoP
// proof, a client assertion, a request object) — a caller whose
// accepted input can legitimately scale beyond that (an issuer
// response shaped by however many scopes/claims a deployment grants)
// should call ParseCompactMax with its own configured ceiling instead.
const DefaultMaxCompactBytes = 16 * 1024

// Sign produces a JWS compact serialization:
// BASE64URL(header) || "." || BASE64URL(payload) || "." || BASE64URL(signature).
Expand Down Expand Up @@ -75,10 +80,16 @@ type Compact struct {
}

// ParseCompact splits and decodes a compact JWS without verifying its
// signature.
// signature, rejecting one longer than DefaultMaxCompactBytes.
func ParseCompact(s string) (Compact, error) {
if len(s) > maxCompactBytes {
return Compact{}, ErrTooLarge
return ParseCompactMax(s, DefaultMaxCompactBytes)
}

// ParseCompactMax is ParseCompact with an explicit size ceiling, in
// bytes, instead of DefaultMaxCompactBytes.
func ParseCompactMax(s string, maxBytes int) (Compact, error) {
if len(s) > maxBytes {
return Compact{}, fmt.Errorf("%w: %d bytes exceeds the %d byte limit", ErrTooLarge, len(s), maxBytes)
}
parts := strings.Split(s, ".")
if len(parts) != 3 {
Expand Down
13 changes: 12 additions & 1 deletion internal/jose/compact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"crypto/rand"
"crypto/rsa"
"encoding/base64"
"errors"
"strings"
"testing"

Expand Down Expand Up @@ -247,8 +248,18 @@ func TestParseCompactRejectsMalformed(t *testing.T) {
}

func TestParseCompactRejectsOversized(t *testing.T) {
huge := strings.Repeat("a", maxCompactBytes+1)
huge := strings.Repeat("a", DefaultMaxCompactBytes+1)
if _, err := ParseCompact(huge); err == nil {
t.Fatalf("ParseCompact(oversized) = nil error, want error")
}
if _, err := ParseCompact(huge); !errors.Is(err, ErrTooLarge) {
t.Fatalf("ParseCompact(oversized) error = %v, want ErrTooLarge", err)
}
}

func TestParseCompactMaxRespectsExplicitLimit(t *testing.T) {
tooLarge := strings.Repeat("a", 101)
if _, err := ParseCompactMax(tooLarge, 100); !errors.Is(err, ErrTooLarge) {
t.Fatalf("ParseCompactMax(101 bytes, max 100) error = %v, want ErrTooLarge", err)
}
}
4 changes: 4 additions & 0 deletions internal/jwe/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,8 @@ var (
// undifferentiated: which specific step failed is not safe to expose
// to a caller processing untrusted input.
ErrDecryptionFailed = errors.New("jwe: decryption failed")

// ErrTooLarge indicates a compact serialization larger than
// DecryptRequest.MaxCompactBytes.
ErrTooLarge = errors.New("jwe: compact serialization exceeds maximum size")
)
Loading
Loading