'alg' is an optional field in the JWK spec, but it is required by this library

[jpda]

Microsoft's identity platform uses the kty parameter for the encryption family, alg is omitted. alg is optional per rfc7517, however this library seems to require it. Seems since the alg variable is already set to "RS256," and kty is in the allJwkFields array and required per spec, that removing alg from the required list may be all that's required.

The allJwkFields array includes alg.

https://github.com/awslabs/aws-jwt-verify/blob/f0531cd190f4cf9213e058b55d725a346775964d/src/jwk.ts#L21

A missing alg throws an error here.

https://github.com/awslabs/aws-jwt-verify/blob/f0531cd190f4cf9213e058b55d725a346775964d/src/jwk.ts#L99-L103

example key data can be found here

replicated below (snipped the cert data and removed some keys, as there are normally ~6-8 keys, for brevity)

{
  "keys": [
    {
      "kty": "RSA",
      "use": "sig",
      "kid": "nOo3ZDrODXEK1jKWhXslHR_KXEg",
      "x5t": "nOo3ZDrODXEK1jKWhXslHR_KXEg",
      "n": "oaLLT9hkc.../snip",
      "e": "AQAB",
      "x5c": [
      "MIIDBTCCAe2g.../snip"
      ],
      "issuer": "https://login.microsoftonline.com/{tenantid}/v2.0"
    },
    {
      "kty": "RSA",
      "use": "sig",
      "kid": "l3sQ-50cCH4xBVZLHTGwnSR7680",
      "x5t": "l3sQ-50cCH4xBVZLHTGwnSR7680",
      "n": "sfsXMXWuO-.../snip",
      "e": "AQAB",
      "x5c": [
        "MIIDBTCCA.../snip"
      ],
      "issuer": "https://login.microsoftonline.com/{tenantid}/v2.0"
    }
  ]
}

[ottokruse]

Thanks for the feedback, and the suggestion! We'll look into it and get back to you.

[ottokruse]

Hi John,

Is there a particular reason Microsoft didn't add "alg" to the JWK? (Honest question, for more context. See you're a PM there. AWS Cognito and Auth0 are examples of IDPs that do have alg on JWKs)

"kty" "RSA" means the JWK can be used with either rs256, rs384 or rs512, so it leaves the question, which of these will the JWT be signed with then? Of course there's the JWT header alg claim, but that can't be trusted by itself, as malicious actors can control it. (Auth0's blog has a nice article on this, you probably know it)

Having alg on the JWK is nice, to cross validate with the alg in the JWT header (which our lib does, using the JWT header kid to figure out the right JWK––we're considering relaxing that, and match only kty if the JWK does not have an alg).

(Note: currently our lib is uses RS256 only and always, trying JWT's with other algs does not work, but one day we might add more algorithm support, and then it becomes paramount to do the algorithm selection right)

And a request, is it easy for you to provide here a sample test JWT signed with the one of the JWKs in https://login.microsoftonline.com/common/discovery/v2.0/keys? That'd be great for testing, to make sure our code changes work for Microsoft JWTs. (Tried these Microsoft sample JWTs but they give an invalid signature)

Thank you

[ottokruse]

For reference, same issue in another library (that was resolved): https://github.com/jpadilla/pyjwt/issues/603

[cmatskas]

Sorry for following up on this but was this issue closed as "not implemented" or has there been an updated release with a fix? What more info do you need from us (MS) to make the fix? Token examples below (I would prioritize v2 vs v1) Azure Active Directory v1 token Azure Active Directory v2 token

[leelalagudu]

Hi,

We have re-considered our approach for this part of the solution to both ensure that the JWK and JWT's are validated correctly (i.e. supporting kty as a mandatory parameter in JWK , while treating alg as an optional parameter in JWK) as well as to accommodate the current algorithm supportability of the solution, which is RS256 only.

• Consider alg as an optional field on the JWK while validating JWK
• Evaluate if kty on the JWK is set to RSA
• If JWK has alg set, then validate that alg set in JWT header matches the alg element of JWK
• Validate that JWT alg header is set to RS256

We are working on the PR currently and should have an update on the repo by end of next week. Thank you for passing the tokens, it would help us with our local testing.

Thank you.

[cmatskas]

@leelalagudu this is awesome news! Thanks for the update :)

I look forward to taking the library for a spin when the PR is merged :)

Thanks again

[ottokruse]

Should work now for Microsoft JWTs too 🎉

Please let us know any and all feedback

[cmatskas]

Tested it successfully and it’s all working as expected! Many thanks for your work on this!