Skip to content

Add length check for JWK key import#428

Open
devgianlu wants to merge 1 commit intow3c:mainfrom
devgianlu:jwk-import-len
Open

Add length check for JWK key import#428
devgianlu wants to merge 1 commit intow3c:mainfrom
devgianlu:jwk-import-len

Conversation

@devgianlu
Copy link
Member

@devgianlu devgianlu commented Feb 13, 2026

The PR adds length validation of JWK fields provided during key import to match WPT tests.

See WICG/webcrypto-secure-curves#33 (comment)


Clone of WICG/webcrypto-secure-curves#38


Preview | Diff

@devgianlu
Copy link
Member Author

If there's interest in merging this I can join the WG.

@devgianlu
Copy link
Member Author

@twiss I am now a memeber of the WG, so this requires only one other review I guess.

Copy link
Member

@twiss twiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Sorry, I started writing a review of this at some point but never got around to submitting it. See below.

Comment on lines +10835 to +10840
<li>
<p>
Let |jwkPublic| be the {{JsonWebKey/x}} field of |jwk| interpreted
according to Section 2 of [[RFC8037]].
</p>
</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this text is a bit in an awkward middle state where it has a lot of detailed steps, but still doesn't quite state exactly what to do; particularly this step is a bit handwavy in referring to RFC8037. I'm not opposed to being handwavy, but then I'd like to be more succinct as well. The step before this could also be argued to cover this already. Perhaps we can just make that one a bit more explicit by stating something like

                                  If |jwk| does not meet the requirements of
                                  the JWK private key format described in Section 2
                                  of [[RFC8037]], or the encoded key material does not
                                  represent an Ed25519 key of the correct length,
                                  then [= exception/throw =] a {{DataError}}.

and call it a day? (And similarly below.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, we might want to say something similar for the spki and pkcs8 formats.
If we have to add (more) detailed parsing steps for all of them, I think it'll get a bit unwieldy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's a bit handwavy, but the spec is generally pretty explicit about the length of things, and I feel like it would a still ambigous when the JWK contains both public and private key material. For example, if the JWK is a private key you need to check both the public and private keys indipendently. So does the WPT test (see Bad key length: importKey(jwk).

Perhaps we can change interpreted to decoded: Let jwkPublic be the x field of jwk decoded according to Section 2 of [RFC8037].

There is no need to have length checks for SPKI and PKCS8 becuse the length is already encoded in the format so parsing will fail for invalid length data. Raw has already a very specific length check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants