Skip to content

Harden SAML validation and add opt-in security defenses#9

Merged
marevol merged 1 commit into
mainfrom
security-hardening
Jul 5, 2026
Merged

Harden SAML validation and add opt-in security defenses#9
marevol merged 1 commit into
mainfrom
security-hardening

Conversation

@marevol

@marevol marevol commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes two correctness bugs in encrypted-assertion handling and adds several
backward-compatible, opt-in security controls surfaced by a security review
of the SP validation paths.

All changes are additive: no existing public method signature, default
value, or runtime behavior changes. Integrators see no difference unless they
explicitly opt in. Full test suite passes (core 415, toolkit 92).

Bug fixes

  • SamlResponse.decryptAssertion — a guard checked the wrong node list
    (encryptedDataNodes, already proven non-empty) instead of the
    post-decryption AssertionDataNodes. Because Util.decryptElement swallows
    decryption failures, a failed decryption left no saml:Assertion, and the
    code dereferenced null, throwing an uncaught NullPointerException out of
    the SamlResponse constructor
    (which Auth.processResponse does not wrap).
    It now raises a typed ValidationException.
  • SamlResponse.validateTimestamps — it read the raw (pre-decryption)
    document, so for an encrypted assertion the Conditions
    (NotBefore/NotOnOrAfter) live inside the ciphertext, getElementsByTagNameNS
    matched nothing, and the temporal-validity window was silently never
    checked
    . It now reads the decrypted document like every other accessor.

New opt-in security controls (defaults preserve current behavior)

  • Metadata signature validation — wires the already-present (but previously
    unused) Util.validateMetadataSign into new overloads:
    Saml2Settings.validateMetadata(xml, cert[, fingerprint, alg, rejectDeprecatedAlg])
    and IdPMetadataParser.parseXML/parseFileXML/parseRemoteXML(..., X509Certificate).
    Signature validation runs only when a trust anchor is supplied.

    Security note: the trusted certificate must be provisioned
    out-of-band
    (pinned config/keystore), not taken from the same metadata
    document being validated — otherwise a MITM/compromised source can serve a
    self-consistent forged document + cert. This is documented in the new
    Javadoc.

  • Replay protection — new ReplayCache SPI + thread-safe
    InMemoryReplayCache (TTL, no background threads). Enable with
    auth.setReplayCache(new InMemoryReplayCache()) or
    settings.setReplayCache(...). Detects replayed assertion IDs (SSO) and
    logout-message IDs (SLO); IDs are recorded only after a message is otherwise
    fully valid, so invalid messages cannot poison the cache. Default null =
    disabled (unchanged behavior).
  • RSA-OAEP key transport + inbound decrypt allow-list — outbound NameID
    encryption algorithm is now selectable via
    onelogin.saml2.security.nameid_encryption_algorithm (default rsa-1_5,
    unchanged); an optional inbound allow-list
    onelogin.saml2.security.allowed_key_transport_algorithms (default empty =
    permissive) lets deployments reject legacy PKCS#1 v1.5 key transport. A
    rejected algorithm reuses the existing generic decryption-failure path, so it
    is indistinguishable from a wrong-key failure (no new padding-oracle side
    channel).
  • Configuration warningsSaml2Settings.getSecurityWarnings() reports
    weak-but-valid settings (strict off, weak fingerprint algorithm, deprecated
    algorithms not rejected, assertions/messages not required signed) and is
    logged at settings build time. checkSettings() is left unchanged.

Docs

  • Correct the "Security Defaults" note in CLAUDE.md (deprecated algorithms are
    not rejected by default; fingerprint default is sha1) and add production
    recommendations.
  • Harden the RelayState redirect example in README.md and the JSP sample
    (fix a reference-equality guard and add an allow-list warning to prevent an
    open redirect in copied integration code).

Backward compatibility

  • Every new capability is reached through new overloads / new optional
    fields / new methods; existing callers and the ~27 tests that validate the
    same fixture multiple times pass unmodified.
  • New settings default to the current behavior (RSA-1.5 outbound, permissive
    inbound, no replay cache, no metadata signature check).

Testing

  • TDD for every change (failing test confirmed first). New tests cover both bug
    reproductions, metadata signature validation, getSecurityWarnings(),
    RSA-OAEP output + allow-list rejection, InMemoryReplayCache (including a
    concurrency smoke test), and SSO/SLO replay detection.
  • mvn -B test: core 415, toolkit 92, 0 failures/errors.

Fix two correctness bugs in encrypted-assertion handling and add several
backward-compatible, opt-in security controls. All changes are additive:
existing public signatures, default values, and behavior are unchanged, so
integrators see no difference unless they explicitly opt in.

Bug fixes:
- SamlResponse.decryptAssertion: correct a wrong-variable guard that let a
  failed decryption dereference null and throw an uncaught
  NullPointerException out of the constructor; it now raises a typed
  ValidationException.
- SamlResponse.validateTimestamps: read the decrypted document so an
  encrypted assertion's Conditions (NotBefore/NotOnOrAfter) are actually
  validated instead of being silently skipped.

New opt-in controls (defaults preserve current behavior):
- Metadata signature validation: wire the existing Util.validateMetadataSign
  into new Saml2Settings.validateMetadata(...) and IdPMetadataParser.parse*(...)
  overloads that accept a trusted signing certificate (must be provisioned
  out-of-band; documented in Javadoc).
- Replay protection: new ReplayCache SPI plus InMemoryReplayCache; enable via
  Saml2Settings/Auth.setReplayCache(...). Detects replayed assertion and
  logout-message IDs.
- RSA-OAEP key transport for outbound NameID encryption and an inbound decrypt
  key-transport algorithm allow-list, both configurable; defaults preserve
  RSA-1.5 behavior. Algorithm rejection reuses the existing generic
  decryption-failure path to avoid introducing a padding-oracle side channel.
- Saml2Settings.getSecurityWarnings() surfaces weak-configuration warnings and
  is logged at settings build time (checkSettings() is unchanged).

Docs: correct the security-defaults note in CLAUDE.md and harden the
RelayState redirect example in README and the JSP sample.

Adds tests for every change; full suite passes (core 415, toolkit 92).
@marevol marevol merged commit 81bd043 into main Jul 5, 2026
1 check passed
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.

1 participant