Option to enforce algorithm matching between JWT and JWK

[adamcfraser]

In an environment where multiple varieties of a key type are supported (e.g. RS256 and RS384), the library doesn't enforce that the algorithm specified in the keyset for the kid specified in the JWT matches the algorithm specified in the JWT.

The go-jose library does ensure that the JWT is verified using the same algorithm family (e.g. using an rsaEncrypterVerifier), but uses the algorithm defined on the JWT.

Example:

1. jwks defines two keys:
   { "kty": "RSA", "use": "sig", "kid": "kid256", "alg": "RS256", "n": "...", "e": "...", }
   { "kty": "RSA", "use": "sig", "kid": "kid384", "alg": "RS384", "n": "...", "e": "...", }

2. Client generates an RS256 JWT using the private key associated with kid384, with header:
   { "kid": "kid384", "typ": "JWT", "alg": "RS256" }

3. This is successfully validated:

• kid384 is matched here
• RSA verifier is used based on the jwk key type, but uses the JWT's algorithm

I'm unclear on the risk (if any) associated with this type of substitution, but it would be useful to have an option to enforce JWT and key algorithm matching when that's the intended usage.

[ericchiang]

I think this would be reasonable to enforce by default. For what it's worth, we do enforce based on what the provider advertises it supports, just not on a per-key basis:

go-oidc/oidc/verify.go

Lines 123 to 133 in fb9e009

	// Verifier returns an IDTokenVerifier that uses the provider's key set to verify JWTs.
	func (p *Provider) Verifier(config *Config) *IDTokenVerifier {
	if len(config.SupportedSigningAlgs) == 0 && len(p.algorithms) > 0 {
	// Make a copy so we don't modify the config values.
	cp := &Config{}
	*cp = *config
	cp.SupportedSigningAlgs = p.algorithms
	config = cp
	}
	return NewVerifier(p.issuer, p.remoteKeySet, config)
	}

[adamcfraser]

Thanks - I'm familiar with the provider-level enforcement, and agree it's sufficient in the majority of use cases.

[ericchiang]

Out of curiosity, have you brought this up with the go-jose package? They should have all the information they need to detect that an signature's alg value doesn't match the key's alg value.

[adamcfraser]

I was on the fence about whether this was a key matching concern versus a verification concern, and landed on key matching. I'm open to directing this to go-jose if you think that makes more sense.

[ericchiang]

Just to try to be very clear about the security concerns here. Say we have Key A that advertises RS256 and Key B that advertises RS384.

• You shouldn't be able to sign with an algorithm other than RS256 or RS384 (e.g. R512) since the provider wouldn't advertise it
• You'd have to be able to forge a signature with RS256 with Key B, but not RS384 with Key B or RS256 with Key A
• Since go-jose does the actual validation, we have to assume it's logic of determining which alg to use matches ours. The more we add here, the more likely it is that go-oidc will do something different

Given that:

• I'm not super concerned about the security implications here
• This check should probably go in go-jose to make sure its performed correctly

[drakkan]

Just to try to be very clear about the security concerns here. Say we have Key A that advertises RS256 and Key B that advertises RS384.

• You shouldn't be able to sign with an algorithm other than RS256 or RS384 (e.g. R512) since the provider wouldn't advertise it
• You'd have to be able to forge a signature with RS256 with Key B, but not RS384 with Key B or RS256 with Key A
• Since go-jose does the actual validation, we have to assume it's logic of determining which alg to use matches ours. The more we add here, the more likely it is that go-oidc will do something different

Given that:

• I'm not super concerned about the security implications here
• This check should probably go in go-jose to make sure its performed correctly

go-jose v2 is not actively developed anymore. Maybe go-oidc should consider migrating to v3 or to a more actively maintained library, such as jwx, the maintainer, @lestrrat is very responsive. I'm not sure if jwx already does this check

[ericchiang]

Thanks! I've updated the import for go-jose #360

Again, I do think this check should be in the go-jose package.