Skip to content

Commit

Permalink
fix(authentication): jwt assertions audience too strict (#125)
Browse files Browse the repository at this point in the history
This fixes an issue where all JWT assertions are strictly checked against the Token URL. RFC7523 Section 3 states that the JWT must contain an 'aud' claim that identifies the authorization server and that the token endpoint URL may be used, not that it must be used. RFC9126 clarifies this that it should be the issuer value, and that both the token endpoint URL and pushed authorization request endpoint URL must also be accepted. This fix facilitate this.
  • Loading branch information
james-d-elliott authored Sep 1, 2024
1 parent acbadb6 commit 1d32ff6
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 32 deletions.
49 changes: 44 additions & 5 deletions client_authentication_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type DefaultClientAuthenticationStrategy struct {
}
Config interface {
JWKSFetcherStrategyProvider
TokenURLProvider
AllowedJWTAssertionAudiencesProvider
}
}

Expand Down Expand Up @@ -277,15 +277,15 @@ func (s *DefaultClientAuthenticationStrategy) doAuthenticateAssertionJWTBearer(c
}

func (s *DefaultClientAuthenticationStrategy) doAuthenticateAssertionParseAssertionJWTBearer(ctx context.Context, client Client, assertion *ClientAssertion, resolver EndpointClientAuthHandler) (method, kid, alg string, token *xjwt.Token, claims *xjwt.RegisteredClaims, err error) {
var tokenURI string
audience := s.Config.GetAllowedJWTAssertionAudiences(ctx)

if tokenURI = s.Config.GetTokenURL(ctx); tokenURI == "" {
return "", "", "", nil, nil, errorsx.WithStack(ErrMisconfiguration.WithHint("The authorization server does not support OAuth 2.0 JWT Profile Client Authentication RFC7523 or OpenID Connect 1.0 specific authentication methods.").WithDebug("The authorization server Token URL was empty but it's required to validate the RFC7523 audience claim."))
if len(audience) == 0 {
return "", "", "", nil, nil, errorsx.WithStack(ErrMisconfiguration.WithHint("The authorization server does not support OAuth 2.0 JWT Profile Client Authentication RFC7523 or OpenID Connect 1.0 specific authentication methods.").WithDebug("The authorization server could not determine any safe value for it's audience but it's required to validate the RFC7523 client assertions."))
}

opts := []xjwt.ParserOption{
xjwt.WithStrictDecoding(),
xjwt.WithAudience(tokenURI), // Satisfies RFC7523 Section 3 Point 3.
//xjwt.WithAudience(tokenURI), // Satisfies RFC7523 Section 3 Point 3.
xjwt.WithExpirationRequired(), // Satisfies RFC7523 Section 3 Point 4.
xjwt.WithIssuedAt(), // Satisfies RFC7523 Section 3 Point 6.
}
Expand Down Expand Up @@ -320,9 +320,48 @@ func (s *DefaultClientAuthenticationStrategy) doAuthenticateAssertionParseAssert
return "", "", "", nil, nil, resolveJWTErrorToRFCError(err)
}

// Satisfies RFC7523 Section 3 Point 3.
if err = s.doAuthenticateAssertionJWTBearerClaimAudience(ctx, audience, claims); err != nil {
return "", "", "", nil, nil, err
}

return method, kid, alg, token, claims, nil
}

func (s *DefaultClientAuthenticationStrategy) doAuthenticateAssertionJWTBearerClaimAudience(ctx context.Context, audience []string, claims *xjwt.RegisteredClaims) (err error) {
if len(claims.Audience) == 0 {
return errorsx.WithStack(
ErrInvalidClient.
WithHint("Unable to verify the integrity of the 'client_assertion' value. It may have been used before it was issued, may have been used before it's allowed to be used, may have been used after it's expired, or otherwise doesn't meet a particular validation constraint.").
WithDebug("Unable to validate the 'aud' claim of the 'client_assertion' as it was empty."),
)
}

validAudience := false

var aud, unverified string

verification:
for _, unverified = range claims.Audience {
for _, aud = range audience {
if subtle.ConstantTimeCompare([]byte(aud), []byte(unverified)) == 1 {
validAudience = true
break verification
}
}
}

if !validAudience {
return errorsx.WithStack(
ErrInvalidClient.
WithHint("Unable to verify the integrity of the 'client_assertion' value. It may have been used before it was issued, may have been used before it's allowed to be used, may have been used after it's expired, or otherwise doesn't meet a particular validation constraint.").
WithDebugf("Unable to validate the 'aud' claim of the 'client_assertion' value '%s' as it doesn't match any of the expected values '%s'.", strings.Join(claims.Audience, "', '"), strings.Join(audience, "', '")),
)
}

return nil
}

func (s *DefaultClientAuthenticationStrategy) doAuthenticateAssertionParseAssertionJWTBearerFindKey(ctx context.Context, header map[string]any, client AuthenticationMethodClient, handler EndpointClientAuthHandler) (key any, err error) {
var kid, alg, method string

Expand Down
12 changes: 6 additions & 6 deletions client_authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ func TestAuthenticateClient(t *testing.T) {
}, keyRSA, "kid-foo")}, "client_assertion_type": []string{consts.ClientAssertionTypeJWTBearer}},
r: new(http.Request),
expectErr: ErrInvalidClient,
err: "Client authentication failed (e.g., unknown client, no client authentication included, or unsupported authentication method). Unable to decode 'client_assertion' value for an unknown reason. token has invalid claims: token has invalid audience",
err: "Client authentication failed (e.g., unknown client, no client authentication included, or unsupported authentication method). Unable to verify the integrity of the 'client_assertion' value. It may have been used before it was issued, may have been used before it's allowed to be used, may have been used after it's expired, or otherwise doesn't meet a particular validation constraint. Unable to validate the 'aud' claim of the 'client_assertion' value 'token-url-1', 'token-url-2' as it doesn't match any of the expected values 'token-url'.",
},
{
name: "ShouldPassWithProperAssertionWhenJWKsAreSetWithinTheClient",
Expand Down Expand Up @@ -697,9 +697,9 @@ func TestAuthenticateClient(t *testing.T) {
provider := &Fosite{
Store: storage.NewMemoryStore(),
Config: &Config{
JWKSFetcherStrategy: NewDefaultJWKSFetcherStrategy(),
TokenURL: "token-url",
HTTPClient: retryablehttp.NewClient(),
JWKSFetcherStrategy: NewDefaultJWKSFetcherStrategy(),
AllowedJWTAssertionAudiences: []string{"token-url"},
HTTPClient: retryablehttp.NewClient(),
},
}

Expand Down Expand Up @@ -764,8 +764,8 @@ func TestAuthenticateClientTwice(t *testing.T) {
provider := &Fosite{
Store: store,
Config: &Config{
JWKSFetcherStrategy: NewDefaultJWKSFetcherStrategy(),
TokenURL: "token-url",
JWKSFetcherStrategy: NewDefaultJWKSFetcherStrategy(),
AllowedJWTAssertionAudiences: []string{"token-url"},
},
}

Expand Down
8 changes: 5 additions & 3 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,11 @@ type FormPostResponseProvider interface {
GetFormPostResponseWriter(ctx context.Context) FormPostResponseWriter
}

type TokenURLProvider interface {
// GetTokenURL returns the token URL.
GetTokenURL(ctx context.Context) (url string)
// AllowedJWTAssertionAudiencesProvider is a provider used in contexts where the permitted audiences for a JWT assertion
// is required to validate a request.
type AllowedJWTAssertionAudiencesProvider interface {
// GetAllowedJWTAssertionAudiences returns the permitted audience list for JWT Assertions.
GetAllowedJWTAssertionAudiences(ctx context.Context) (audiences []string)
}

// AuthorizeEndpointHandlersProvider returns the provider for configuring the authorize endpoint handlers.
Expand Down
14 changes: 7 additions & 7 deletions config_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ type Config struct {
// AllowedPromptValues sets which OpenID Connect prompt values the server supports. Defaults to []string{"login", "none", "consent", "select_account"}.
AllowedPromptValues []string

// TokenURL is the the URL of the Authorization Server's Token Endpoint. If the authorization server is intended
// to be compatible with the private_key_jwt client authentication method (see http://openid.net/specs/openid-connect-core-1_0.html#CodeFlowAuth),
// this value MUST be set.
TokenURL string
// AllowedJWTAssertionAudiences is a list of permitted client assertion audiences. If the authorization server is
// intended to be compatible with the client_secret_jwt or private_key_jwt client authentication methods
// (see http://openid.net/specs/openid-connect-core-1_0.html#CodeFlowAuth), this value MUST be set.
AllowedJWTAssertionAudiences []string

// JWKSFetcherStrategy is responsible for fetching JSON Web Keys from remote URLs. This is required when the private_key_jwt
// client authentication method is used. Defaults to oauth2.DefaultJWKSFetcherStrategy.
Expand Down Expand Up @@ -277,8 +277,8 @@ func (c *Config) GetHTTPClient(ctx context.Context) *retryablehttp.Client {
return c.HTTPClient
}

func (c *Config) GetTokenURL(ctx context.Context) string {
return c.TokenURL
func (c *Config) GetAllowedJWTAssertionAudiences(ctx context.Context) []string {
return c.AllowedJWTAssertionAudiences
}

func (c *Config) GetFormPostHTMLTemplate(ctx context.Context) *template.Template {
Expand Down Expand Up @@ -652,7 +652,7 @@ var (
_ MessageCatalogProvider = (*Config)(nil)
_ FormPostHTMLTemplateProvider = (*Config)(nil)
_ FormPostResponseProvider = (*Config)(nil)
_ TokenURLProvider = (*Config)(nil)
_ AllowedJWTAssertionAudiencesProvider = (*Config)(nil)
_ HTTPClientProvider = (*Config)(nil)
_ HMACHashingProvider = (*Config)(nil)
_ AuthorizeEndpointHandlersProvider = (*Config)(nil)
Expand Down
2 changes: 1 addition & 1 deletion fosite.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ type Configurator interface {
MessageCatalogProvider
FormPostHTMLTemplateProvider
FormPostResponseProvider
TokenURLProvider
AllowedJWTAssertionAudiencesProvider
AuthorizeEndpointHandlersProvider
TokenEndpointHandlersProvider
TokenIntrospectionHandlersProvider
Expand Down
28 changes: 25 additions & 3 deletions handler/rfc7523/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package rfc7523

import (
"context"
"crypto/subtle"
"strings"
"time"

"github.com/go-jose/go-jose/v4"
Expand All @@ -21,7 +23,7 @@ type Handler struct {

Config interface {
oauth2.AccessTokenLifespanProvider
oauth2.TokenURLProvider
oauth2.AllowedJWTAssertionAudiencesProvider
oauth2.GrantTypeJWTBearerCanSkipClientAuthProvider
oauth2.GrantTypeJWTBearerIDOptionalProvider
oauth2.GrantTypeJWTBearerIssuedDateOptionalProvider
Expand Down Expand Up @@ -229,17 +231,37 @@ func (c *Handler) findPublicKeyForToken(ctx context.Context, token *jwt.JSONWebT
//
//nolint:gocyclo,unparam
func (c *Handler) validateTokenClaims(ctx context.Context, claims jwt.Claims, key *jose.JSONWebKey) error {
audiences := c.Config.GetAllowedJWTAssertionAudiences(ctx)

if len(audiences) == 0 {
return errorsx.WithStack(oauth2.ErrServerError.
WithHint("The JWT in 'assertion' request parameter can't be validated as the authorization server isn't configured to accept JWT assertions."),
)
}

if len(claims.Audience) == 0 {
return errorsx.WithStack(oauth2.ErrInvalidGrant.
WithHint("The JWT in 'assertion' request parameter MUST contain an 'aud' (audience) claim."),
)
}

if !claims.Audience.Contains(c.Config.GetTokenURL(ctx)) {
validAudience := false

verify:
for _, unverified := range claims.Audience {
for _, aud := range audiences {
if subtle.ConstantTimeCompare([]byte(aud), []byte(unverified)) == 1 {
validAudience = true
break verify
}
}
}

if !validAudience {
return errorsx.WithStack(oauth2.ErrInvalidGrant.
WithHintf(
"The JWT in 'assertion' request parameter MUST contain an 'aud' (audience) claim containing a value '%s' that identifies the authorization server as an intended audience.",
c.Config.GetTokenURL(ctx),
strings.Join(audiences, "', '"),
),
)
}
Expand Down
7 changes: 4 additions & 3 deletions handler/rfc7523/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
mrand "math/rand"
"net/url"
"strconv"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -74,7 +75,7 @@ func (s *AuthorizeJWTGrantRequestHandlerTestSuite) SetupTest() {
Config: &oauth2.Config{
ScopeStrategy: oauth2.HierarchicScopeStrategy,
AudienceMatchingStrategy: oauth2.DefaultAudienceMatchingStrategy,
TokenURL: "https://www.example.com/token",
AllowedJWTAssertionAudiences: []string{"https://www.example.com/token"},
GrantTypeJWTBearerCanSkipClientAuth: false,
GrantTypeJWTBearerIDOptional: false,
GrantTypeJWTBearerIssuedDateOptional: false,
Expand Down Expand Up @@ -333,7 +334,7 @@ func (s *AuthorizeJWTGrantRequestHandlerTestSuite) TestNotValidAudienceInAsserti
s.Equal(
fmt.Sprintf(
"The JWT in 'assertion' request parameter MUST contain an 'aud' (audience) claim containing a value '%s' that identifies the authorization server as an intended audience.",
s.handler.Config.GetTokenURL(ctx),
strings.Join(s.handler.Config.GetAllowedJWTAssertionAudiences(ctx), "', '"),
),
oauth2.ErrorToRFC6749Error(err).HintField,
)
Expand Down Expand Up @@ -851,7 +852,7 @@ func (s *AuthorizeJWTGrantPopulateTokenEndpointTestSuite) SetupTest() {
Config: &oauth2.Config{
ScopeStrategy: oauth2.HierarchicScopeStrategy,
AudienceMatchingStrategy: oauth2.DefaultAudienceMatchingStrategy,
TokenURL: "https://www.example.com/token",
AllowedJWTAssertionAudiences: []string{"https://www.example.com/token"},
GrantTypeJWTBearerCanSkipClientAuth: false,
GrantTypeJWTBearerIDOptional: false,
GrantTypeJWTBearerIssuedDateOptional: false,
Expand Down
2 changes: 1 addition & 1 deletion integration/authorize_jwt_bearer_required_iat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestAuthorizeJWTBearerRequiredIATSuite(t *testing.T) {
GrantTypeJWTBearerCanSkipClientAuth: true,
GrantTypeJWTBearerIDOptional: true,
GrantTypeJWTBearerIssuedDateOptional: false,
TokenURL: tokenURL,
AllowedJWTAssertionAudiences: []string{tokenURL},
},
store,
jwtStrategy,
Expand Down
2 changes: 1 addition & 1 deletion integration/authorize_jwt_bearer_required_jti_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestAuthorizeJWTBearerRequiredJtiSuite(t *testing.T) {
GrantTypeJWTBearerCanSkipClientAuth: true,
GrantTypeJWTBearerIDOptional: false,
GrantTypeJWTBearerIssuedDateOptional: true,
TokenURL: tokenURL,
AllowedJWTAssertionAudiences: []string{tokenURL},
},
store,
jwtStrategy,
Expand Down
2 changes: 1 addition & 1 deletion integration/authorize_jwt_bearer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func TestAuthorizeJWTBearerSuite(t *testing.T) {
GrantTypeJWTBearerIDOptional: true,
GrantTypeJWTBearerIssuedDateOptional: true,
GrantTypeJWTBearerMaxDuration: 24 * time.Hour,
TokenURL: tokenURL,
AllowedJWTAssertionAudiences: []string{tokenURL},
},
store,
jwtStrategy,
Expand Down
2 changes: 1 addition & 1 deletion integration/introspect_jwt_bearer_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func TestIntrospectJWTBearerTokenSuite(t *testing.T) {
GrantTypeJWTBearerIDOptional: true,
GrantTypeJWTBearerIssuedDateOptional: true,
AccessTokenLifespan: time.Hour,
TokenURL: tokenURL,
AllowedJWTAssertionAudiences: []string{tokenURL},
},
store,
jwtStrategy,
Expand Down

0 comments on commit 1d32ff6

Please sign in to comment.