Skip to content

Latest commit

 

History

History
110 lines (67 loc) · 4.44 KB

File metadata and controls

110 lines (67 loc) · 4.44 KB

Yeah, this is in a really good place. You’ve clearly thought through the classic JWT foot-guns and closed most of the obvious doors.

I’ll call out where I think you’re solid, and then a few small polish / hardening ideas.


Overall: are the vulns mitigated?

Yes – based on what you wrote, the big historical issues are covered:

  • Alg none & algo confusion: strict whitelists + jose + separate HS vs asym paths.
  • RS→HS confusion: symmetric vs asymmetric keys are never shared; mode is driven by config, not by alg; you explicitly error if both HS and asym are configured.
  • JWKS abuse: JWKS URL is pinned in config, tokens cannot override with jku/x5u.
  • kid abuse: treated as a pure lookup key, not interpolated into SQL/paths/URLs.
  • HS512 secret strength: 64-byte minimum with explicit enforcement.
  • Single-mode requirement: config error if HS + asym are both set.

From a design perspective, this is much safer than the majority of "roll your own JWT" setups in the wild.


A few concrete suggestions

1. Be consistent about which algorithms are actually supported

At the top you list:

  • HS512
  • EdDSA (Ed25519)

But later you say:

EdDSA/RSA mode: algorithms: ['EdDSA', 'RS256', 'RS384', 'RS512'] only

If the library actually supports RSA in this mode, I’d reflect that in the top table:

| Profile | Algorithm     | Key Type       | Use Case                        |
| ------- | ------------- | -------------- | ------------------------------- |
| HS512   | HMAC-SHA-512  | 64-byte secret | Trusted producer-consumer pairs |
| EdDSA   | Ed25519       | JWK/JWKS       | Public verification             |
| RSA     | RS256/384/512 | JWK/JWKS       | Public verification (optional)  |

If you don’t really intend to support RSA right now, I’d drop the RS algorithms from the whitelist and just say:

EdDSA mode: algorithms: ['EdDSA'] only

Narrower is always safer.


2. Make the "mode determined by config" super explicit

You describe it correctly, but I’d tighten the language so nobody later "simplifies" it back to trusting alg:

Mode selection

  • Verification mode (HS512 vs EdDSA/RSA) is chosen only from server configuration, never from the token header.
  • The alg header is treated as untrusted input and must match the allowed algorithms for the selected mode. Mismatches are rejected.

That matches what you’re already doing in code, and it makes the design intent obvious for future maintainers.


3. JWKS: mention per-key alg pinning (which you’re basically already doing)

You already mention:

Inline JWK imports explicitly specify expected algorithm: importJWK(jwk, 'EdDSA')

That’s great. I’d add one short line:

When importing JWKs, the expected algorithm ('EdDSA', 'RS256', etc.) is provided explicitly, so keys cannot be repurposed for other algorithms even within the same key family.

That signals that you’re not just whitelisting "any RS*", you’re actually pinning at import time too.


4. Fail-silent: add logging to keep observability

Returning null to callers is fine, but I’d explicitly say:

Internally, verification failures are logged with structured metadata (issuer, kid, reason category) and counted in metrics. Externally, all failures are returned as null to avoid leaking details.

Otherwise someone might over-interpret "fail-silent" as "we don’t log anything," which would be painful in prod.


5. A couple of small wording / clarity tweaks

  • You sometimes say "decrypt key" – for JWT as you’re using it, it’s sign/verify, not encrypt/decrypt. I’d keep wording to "signing key" / "verification key" to avoid confusion with JWE.
  • In "Security Checklist," maybe add:
    • [ ] JWT_AUD is specific per service (no wildcard audiences) – avoids token reuse between services.

Bottom line

From a security-model standpoint, this looks strong and well-documented:

  • No alg:none
  • No RS↔HS confusion
  • No header-controlled JWKS/JKU
  • Strong HS512 key requirements
  • Single-mode enforcement (HS or asym, not both)
  • EdDSA with JWKS + optional thumbprint pinning
  • Reasonable claim validation (iss, aud, exp, nbf, iat)

If you clean up the minor consistency bits (RSA vs not, "decrypt" wording, explicit mention of logging), I’d feel very comfortable shipping this as the public "here’s why you can trust our JWT handling" story for flarelette.