Skip to content

REF-29: Restrict the algorithms accepted when decrypting assertions - #96

Merged
thomasnymand merged 3 commits into
masterfrom
feature/REF-29-decryption-algorithm-allowlist
Aug 26, 2026
Merged

thomasnymand merged 3 commits into
masterfrom
feature/REF-29-decryption-algorithm-allowlist

Conversation

@thomasnymand

Copy link
Copy Markdown
Collaborator

Limits the assertion consumer to the encryption algorithms the profile allows.

Problem

The Decrypter in AssertionService was built without an algorithm list, so it decrypted whatever the ciphertext named, including RSA-1.5 key transport and block ciphers outside the profile. Decryption runs before signature validation and the endpoint accepts unauthenticated input, so the accepted set should be no wider than the profile requires.

Changes

  • Pass the algorithms of [OIO-ALG-01], identical in the OIOSAML Web SSO profiles 3.0.3 and 4.0.0: RSA-OAEP (both URIs) for key transport, AES-128/256-CBC and AES-128/192/256-GCM for block encryption. Anything else is refused before decryption is attempted; the uniform external error message is unchanged.
  • The list also carries the digest and mask-generation algorithms RSA-OAEP is parameterised with (SHA-1, SHA-256, SHA-512 and the matching MGF1 URIs), because Decrypter.validateAlgorithms checks those against the same list and OpenSAML emits rsa-oaep-mgf1p with a SHA-1 digest by default. They are not usable as encryption algorithms, and OAEP does not rely on collision resistance, so SHA-1 there does not weaken key transport. Leaving them out rejects every IdP that emits default-parameter OAEP.

The set is hard-coded rather than configurable: the profile prescribes it exactly, and CLAUDE.md records that it must not be widened.

Verification

mvn -pl oiosaml test → 115 tests, 1 failure: the pre-existing OIOBPPUtilTest (JDK 26 JAXB incompatibility), which also fails on master.

New tests decrypt an AES-GCM assertion and reject RSA-1.5 key transport and Triple DES block encryption. With AssertionService reverted, both rejection tests fail, i.e. those assertions decrypt. IdpUtil.createResponse gained an overload taking the algorithm pair.

Follow-up (not in this PR)

Inbound signature algorithms are still unconstrained, so SHA-1 signatures are accepted; [OIO-ALG-01] allows only rsa-sha256 / ecdsa-sha256 with SHA-256 digests. Separately, the same key pair is still advertised and used for both signing and encryption.

The Decrypter was built without an algorithm list, so the assertion consumer
decrypted whatever the ciphertext named, including RSA-1.5 key transport and
block ciphers the profile does not allow. Decryption happens before signature
validation and the endpoint takes unauthenticated input, so the accepted set
should be no wider than the profile requires.

Pass the algorithms of [OIO-ALG-01], identical in the OIOSAML Web SSO profiles
3.0.3 and 4.0.0: RSA-OAEP for key transport, AES-128/256-CBC and AES-128/192/256
-GCM for block encryption. Anything else is refused before decryption is
attempted, and the uniform external error message is unchanged.

The list also carries the digest and mask generation algorithms RSA-OAEP is
parameterised with, SHA-1, SHA-256 and SHA-512, because the Decrypter checks
those against the same list. They are not usable as encryption algorithms, and
OAEP does not rely on collision resistance, so accepting SHA-1 there does not
weaken the key transport.

Tests cover decryption with AES-GCM and rejection of RSA-1.5 key transport and
Triple DES block encryption. The test IdP can now encrypt with a given
algorithm pair.
@thomasnymand
thomasnymand requested a review from mthiim August 18, 2026 07:16
# Conflicts:
#	oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java
@thomasnymand
thomasnymand merged commit 4c5216b into master Aug 26, 2026
@thomasnymand
thomasnymand deleted the feature/REF-29-decryption-algorithm-allowlist branch August 26, 2026 16:22
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