Build weak-RSA test fixtures without jwk.Import - #227
Merged
Conversation
Unblocks the jwx renovate PRs (#225 to v3.2.0, #226 to v4), which fail on four tests for the same reason: jwk.Import: key validation failed: jwk.RSAPublicKey: rsa modulus too small: got 1024 bits, need at least 2048 jwx 3.2.0 added an RSA modulus floor to Validate() AND began calling Validate() from Parse/ParseKey as well as Import; 3.0.13 has neither. So the shared helper mintRSABits(t, kid, 1024) can no longer construct the weak fixture, and the tests die at setup without reaching their assertions. Nothing about the package is broken -- the failures are entirely in test scaffolding. The floor still earns its place, but only on one path. A KeyProvider hands over raw crypto keys that never go through jwk, so this package's check is the only one there. On the JWKS path jwx now rejects a short key during Parse, before the filter runs -- and because key-set parsing is strict by default, one weak key fails the WHOLE set. Worth stating plainly, since it is a behaviour change arriving with the bump rather than with any code: an IdP publishing one sub-2048-bit key alongside good ones goes from "that key is filtered out" to "every token gets 503 keys_unavailable". So the coverage is moved to where it is still reachable rather than deleted: - mintRawRSA builds the key and signs a token without touching jwk, for the two tests that genuinely need a weak key. - TestWeakRSAKeyFromProviderRejected uses it, and is tightened while here: it asserted only that the reason was not ReasonSignature, where it can now assert ReasonKeyUnsupported and the cause text. - TestKeyUnsupportedDetailIsLogSafe moves to the provider path, keeping its "the modulus must never appear" assertion meaningful -- a weak RSA key is the thing that could leak one. - TestKeyUnsupportedVsUnknownKID switches its unusable key to `use: enc`, which is equally permanent and survives jwx validation. Its subject is the unusable-vs-absent distinction, not the floor specifically. - TestWeakRSAKeyRejected is deleted rather than rewritten: no public jwx API can build the fixture, and the other ineligibility causes on that path are already covered by TestKeyUsageFilter and TestKeyOpsFilter. A comment records why the gap is deliberate so it is not "fixed" later. Verified green both ways: on 3.0.13 as vendored here, and on 3.2.0 in a scratch copy with the bump applied, which is the combination CI is currently failing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rdimitrov
approved these changes
Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unblocks the jwx renovate PRs — #225 (v3.2.0) and #226 (v4) — which both fail on
the same four tests, for a reason that has nothing to do with the package:
jwx 3.2.0 added an RSA modulus floor to
Validate()and began callingValidate()fromParse/ParseKeyas well asImport. 3.0.13 has neither. Sothe shared helper
mintRSABits(t, kid, 1024)can no longer construct the weakfixture, and
TestWeakRSAKeyRejected,TestWeakRSAKeyFromProviderRejected,TestKeyUnsupportedVsUnknownKIDandTestKeyUnsupportedDetailIsLogSafeall dieat setup without ever reaching their assertions. Test scaffolding only — no
production behaviour is broken.
Test-only change: one file,
authn/validate_test.go.The floor still earns its place, but only on one path
A
KeyProviderhands over raw crypto keys that never go throughjwk, so ourcheck is the only one there. On the JWKS path jwx now rejects a short key during
Parse, before our filter runs. So rather than delete the coverage, it moves towhere it is still reachable:
mintRawRSAbuilds the key and signs a token without touchingjwk, for thetwo tests that genuinely need a weak key.
TestWeakRSAKeyFromProviderRejecteduses it, and is tightened while here: itasserted only that the reason was not
ReasonSignature, where it can nowassert
ReasonKeyUnsupportedand the cause text.TestKeyUnsupportedDetailIsLogSafemoves to the provider path, which keeps its"the modulus must never appear" assertion meaningful — a weak RSA key is the
thing that could leak one.
TestKeyUnsupportedVsUnknownKIDswitches its unusable key touse: enc,equally permanent and survives jwx validation. That test's subject is the
unusable-vs-absent distinction, not the floor specifically.
TestWeakRSAKeyRejectedis deleted rather than rewritten: no public jwx APIcan build the fixture under 3.2, and the other ineligibility causes on that
path are already covered by
TestKeyUsageFilterandTestKeyOpsFilter. Acomment records why the gap is deliberate, so it does not get "fixed" later.
keyTypeMatchesAlgkeeps its floor as defence-in-depth for consumers on olderjwx — it is unreachable under 3.2, not wrong.
One thing to decide before #225 merges — not fixed here
Key-set parsing is strict by default, so under 3.2.0 a JWKS containing one
sub-2048-bit RSA key fails to parse entirely:
Lookupfails and every token gets503
keys_unavailable, including ones signed by perfectly good keys in the sameset. On 3.0.13 that weak key parses and we filter it out as a single ineligible
candidate, and the good keys keep working.
That is a production behaviour change arriving with a dependency bump rather than
with anyone's code, so it deserves to be a decision.
jwk.WithStrictKeySetParsing(false)would degrade it back to "one unusable key", which matches this package's own
per-key filtering — and the resulting
UnsupportedKeyplaceholder would befiltered by
keyEligibleanyway. The catch is that it is typedGlobalParseOptionand applied viajwk.Configure, so if it cannot be threadedper-parse into the cache, a library probably should not be setting it
process-wide. There is also
jwk.WithMinRSAModulusBitsif the floor itself everneeds tuning.
Deliberately left out of this PR so the CI unblock is not entangled with a
policy call.
Test plan
Verified green both ways, since passing on only one is the whole problem:
go test -race ./authn/on 3.0.13 as pinned onmain.go test -race ./authn/on 3.2.0, applied in a scratch copy — the combinationUpdate module github.com/lestrrat-go/jwx/v3 to v3.2.0 #225 and Update module github.com/lestrrat-go/jwx/v3 to v4 #226 currently fail on.
The rest of the suite is unchanged by this PR and left to CI.