Issue with JWK Validation: Leading Zero in Coordinates

[Useserall]

We have encountered a problem where loading JSON Web Keys (JWKs) results in the following error message:

"failed to validate JSON Web Key: failed to validate JWK: marshaled JWK does not match original JWK"

The JWK is set as follows:

                "kty": "EC",
                "crv": "P-256",
                "alg": "ES256"

Upon investigation, we found that the problem lies in one of the key coordinates starting with a leading zero. For example:
The x coordinate starts with "ALTu..." After the coordinate is changed with the following function

jwkset/marshal.go

Line 485 in b0b8e8b

func base64urlTrailingPadding(s string) ([]byte, error) {

the result looks like:
[0 180 238...]

However, after calling Set.Bytes() on this value, the leading zero disappears..

jwkset/marshal.go

Line 231 in b0b8e8b

X: new(big.Int).SetBytes(x),

When converting the result back to bytes we see the following result:
[180 238...]

This discrepancy leads to the original error message because the deepEqual check here no longer validates correctly.

jwkset/jwk.go

Line 311 in b0b8e8b

ok := reflect.DeepEqual(j.marshal, marshalled)

We recommend addressing this issue by ensuring consistent handling of leading zeros in key coordinates during JWK validation.

We hope to hear soon from you!

Kind regards,
Hauke

[MicahParks]

Hello, thank you for opening this issue.

Are you able to share where you got this JWK from? If it's a public vendor such as an AWS product, Okta, Auth0, etc, I would like to send them an email regarding RFC compliance.

The reason this happened is because the JWK being used is non-RFC compliant regarding padding with leading zeros. Please see this note in the README.md and these lines of RFC 7518:

The octet sequence MUST utilize the minimum number of octets needed to represent the value.

I know saying "the JWK is non-RFC compliant" isn't a satisfying answer. I plan on adding a work-around for non-RFC compliant JWK with leading padding similar to the trailing padding issue. Unfortunately, this is unlikely to happen today, but is likely to happen before the week is over.

Edit:
This is incorrect. This JWK parameter is not a Base64urlUInt, it's a base64url encoding.

[Useserall]

Hello @MicahParks and thank you for your quick reply!

Yes I can share that - the JWK comes from backstage: https://github.com/backstage/backstage
I asked our developers if they knew if a third party library was being used and they replied that internally it appears to be using: https://github.com/panva/jose in version 4.11.1.

If I hear something different or have any updates I will come back to you on this.

I completely agree and would equally ask them to change their method of creating JWKs as they are not RFC compliant.

That would be great, thank you very much!

Kind regards,
Hauke

[raskad]

Hi @MicahParks,

I took a look at the RFC 7518. I might be missing something since I did not read it fully, but to me the section on the "x" and "y" parameters sounds like they should be represented as a fixed length base64url encoded string:

6.2.1.2. "x" (X Coordinate) Parameter

The "x" (x coordinate) parameter contains the x coordinate for the
Elliptic Curve point. It is represented as the base64url encoding of
the octet string representation of the coordinate, as defined in
Section 2.3.5 of SEC1 [SEC1]. The length of this octet string MUST
be the full size of a coordinate for the curve specified in the "crv"
parameter. For example, if the value of "crv" is "P-521", the octet
string must be 66 octets long.

As far as I can see the Base64urlUInt is only used in the parameters "n", "e", "d", "p", "q", "dp", "dq", "qi", the "r" (Prime Factor), "d" (Factor CRT Exponent) and "t" (Factor CRT Coefficient).

Again I may be totally wrong here because I only looked for that term.

[MicahParks]

@raskad thank you for politely pointing that out.

It does look like these JWK parameters are defined by base64url encoding as defined by RFC 7518 Section 1.1, not Base64urlUInt, which is defined differently in Section 2.

I'm going to mark this issue as bug.

[MicahParks]

It's a shame that the RFC did not define some of these JWK parameters as Base64urlUInt or some other type like Base64urlInt for values that could be negative.

Looking at *ecdsa.PublicKey for example, the X, and Y fields are represented as *big.Int and the most natural way to turn that into the JWK parameter using Golang is always going to strip and leading zeros, unless the leading zeros are added manually.

The new validation method I'm working on will be comparing the affected JWK parameters *big.Int types. This technically would be the wrong way to compare Base64url Encoding, but I think it's safe to assume this comparison will be correct for certain JWK parameters, such as "x" and "y", also there's doesn't seem to be any other way to compare values since it's impossible to know how many leading zeros are added when JWKs come from other projects.

[raskad]

The new validation method I'm working on will be comparing the affected JWK parameters *big.Int types.

That was also my first idea when I took a look at the code.

also there's doesn't seem to be any other way to compare values since it's impossible to know how many leading zeros are added when JWKs come from other projects.

From my reading of the descriptions of "x" and "y", in theory they must have a fixed length corresponding to the crv right? The length of this octet string MUST be the full size of a coordinate for the curve specified in the "crv" parameter. I'm not 100% read up on the code, but maybe there could be a check when parsing the jwk to verify that "x" and "y" have the specified length and then later you could add the leading zeroes up to that point? But I'm not sure if that actually improves anything.

[MicahParks]

@raskad I think the line you pointed out from RFC 7518 Section 6.2.1.2 is likely something I overlooked when implemented the affected portion of JWK.

The length of this octet string MUST be the full size of a coordinate for the curve specified in the "crv" parameter.

If I am reading this correctly and remember what exists in the code base today. I also need to check how JWK parameters that are marked as Base64url Encoding are treated. I am concerned some/all are being treated as Base64urlUInt when JSON marshaling or unmarshaling, they could be in a different format and I'm not sure the Golang methods being used are appropriate. .Bytes and .SetBytes

I am going to sign off for the night, but plan on looking at this further tomorrow.

If you want to take a peek at the work-in-progress branch, you can find that here: jwk-validation-rework. I am not attached to this WIP branch. Suggestions or pull requests that take a different approach are welcome.

[MicahParks]

Addressing a subset of the questions posed by this issue is this PR: #19

The PR is meant to comply with the octet length requirements for ECDSA JWK parameters as discussed in this issue.

I encourage any interested parties to review, comment, and emoji react on the PR.

I am still investigating Base64urlUInt vs Base64url Encoding and if this project is in full RFC compliance regarding that.

[MicahParks]

Please see the newly release version v0.5.13. This should resolve the original issue. Thank you all again for opening this issue! If the issue has been solved to your satisfaction, please close the issue. I'll close it sometime in the future if there is no activity.

I've made two new issues relevant to this discussion.

1. Rework validation: Change how JWK validation works #20
2. Investigating Base64urlUInt vs Base64url Encoding and if this project is in full RFC compliance: Investigate if this project is in full RFC compliance for JWK parameters using Base64urlUInt vs Base64url Encoding #21

[Useserall]

Hi again @MicahParks,

thanks for your update and the new version, we appreciate you work!
Is it possible that you update the dependency on this project also in your keyfunc project?

That would help us too. Thanks and kind regards!

[MicahParks]

@Useserall please see keyfunc v3.2.6.

Edit, actually use 3.2.7. I forgot to run go mod tidy.

[MicahParks]

With the newest release, v0.5.15, ECDSA keys produced by this project will have the proper padding in the relevant JWK parameters. This project will also work with all keys regardless of leading padding on ECDSA JWK parameters.