Skip to content

Build weak-RSA test fixtures without jwk.Import - #227

Merged
jhrozek merged 1 commit into
mainfrom
fix-weak-rsa-fixtures
Aug 10, 2026
Merged

Build weak-RSA test fixtures without jwk.Import#227
jhrozek merged 1 commit into
mainfrom
fix-weak-rsa-fixtures

Conversation

@jhrozek

@jhrozek jhrozek commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

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:

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 TestWeakRSAKeyRejected, TestWeakRSAKeyFromProviderRejected,
TestKeyUnsupportedVsUnknownKID and TestKeyUnsupportedDetailIsLogSafe all die
at 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 KeyProvider hands over raw crypto keys that never go through jwk, so our
check 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 to
where it is still reachable:

  • 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, which keeps 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,
    equally permanent and survives jwx validation. That test's 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 under 3.2, 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 does not get "fixed" later.

keyTypeMatchesAlg keeps its floor as defence-in-depth for consumers on older
jwx — 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: Lookup fails and every token gets
503 keys_unavailable, including ones signed by perfectly good keys in the same
set. 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 UnsupportedKey placeholder would be
filtered by keyEligible anyway. The catch is that it is typed
GlobalParseOption and applied via jwk.Configure, so if it cannot be threaded
per-parse into the cache, a library probably should not be setting it
process-wide. There is also jwk.WithMinRSAModulusBits if the floor itself ever
needs 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:

The rest of the suite is unchanged by this PR and left to CI.

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>
@jhrozek
jhrozek merged commit 912df34 into main Aug 10, 2026
5 checks passed
@jhrozek
jhrozek deleted the fix-weak-rsa-fixtures branch August 10, 2026 20:56
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