Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(authentication): jwt assertions audience too strict #125

Merged
merged 1 commit into from
Sep 1, 2024
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
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
Loading