From f555ce4bb18c9d253da2bb8b0651dfdfb25a06c8 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Thu, 13 Aug 2026 14:46:49 +0200 Subject: [PATCH 1/2] REF-29: Restrict the algorithms accepted when decrypting assertions 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. --- .../oio/saml/service/AssertionService.java | 26 ++++++++++++- .../saml/service/AssertionServiceTest.java | 38 +++++++++++++++++++ .../java/dk/gov/oio/saml/util/IdpUtil.java | 26 +++++++++++-- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/AssertionService.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/AssertionService.java index dd7786b..5302832 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/service/AssertionService.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/service/AssertionService.java @@ -1,6 +1,9 @@ package dk.gov.oio.saml.service; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.List; import org.slf4j.Logger; @@ -15,11 +18,13 @@ import org.opensaml.security.x509.BasicX509Credential; import org.opensaml.xmlsec.encryption.support.ChainingEncryptedKeyResolver; import org.opensaml.xmlsec.encryption.support.DecryptionException; +import org.opensaml.xmlsec.encryption.support.EncryptionConstants; import org.opensaml.xmlsec.encryption.support.EncryptedKeyResolver; import org.opensaml.xmlsec.encryption.support.InlineEncryptedKeyResolver; import org.opensaml.xmlsec.encryption.support.SimpleRetrievalMethodEncryptedKeyResolver; import org.opensaml.xmlsec.keyinfo.KeyInfoCredentialResolver; import org.opensaml.xmlsec.keyinfo.impl.StaticKeyInfoCredentialResolver; +import org.opensaml.xmlsec.signature.support.SignatureConstants; import dk.gov.oio.saml.util.ExternalException; import dk.gov.oio.saml.util.InternalException; @@ -27,6 +32,25 @@ public class AssertionService { private static final Logger log = LoggerFactory.getLogger(AssertionService.class); + // Key transport and block encryption algorithms allowed by [OIO-ALG-01], identical in the OIOSAML Web + // SSO profiles 3.0.3 and 4.0.0. Anything else, RSA-1.5 in particular, is rejected before decryption. + // The digest and mask generation entries are not encryption algorithms: they are the parameters + // RSA-OAEP is used with, and the Decrypter checks them against the same list. + private static final Collection ALLOWED_ENCRYPTION_ALGORITHMS = Collections.unmodifiableList(Arrays.asList( + EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP, + EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP11, + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES128, + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256, + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES128_GCM, + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES192_GCM, + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256_GCM, + SignatureConstants.ALGO_ID_DIGEST_SHA1, + EncryptionConstants.ALGO_ID_DIGEST_SHA256, + EncryptionConstants.ALGO_ID_DIGEST_SHA512, + EncryptionConstants.ALGO_ID_MGF1_SHA1, + EncryptionConstants.ALGO_ID_MGF1_SHA256, + EncryptionConstants.ALGO_ID_MGF1_SHA512)); + public Assertion getAssertion(Response response) throws InternalException, ExternalException { if (response.getEncryptedAssertions().size() > 0) { EncryptedAssertion encryptedAssertion = response.getEncryptedAssertions().get(0); @@ -64,7 +88,7 @@ private Assertion decryptAssertion(EncryptedAssertion encryptedAssertion) throws ChainingEncryptedKeyResolver kekResolver = new ChainingEncryptedKeyResolver(encryptedKeyResolvers); - Decrypter decrypter = new Decrypter(null, keyResolver, kekResolver); + Decrypter decrypter = new Decrypter(null, keyResolver, kekResolver, ALLOWED_ENCRYPTION_ALGORITHMS, null); decrypter.setRootInNewDocument(true); return decrypter.decrypt(encryptedAssertion); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/AssertionServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/AssertionServiceTest.java index 1646811..1491cd4 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/AssertionServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/AssertionServiceTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.opensaml.saml.saml2.core.Assertion; +import org.opensaml.xmlsec.encryption.support.EncryptionConstants; import dk.gov.oio.saml.util.ExternalException; import dk.gov.oio.saml.util.IdpUtil; @@ -37,6 +38,43 @@ public void testGetPlaintextAssertion() throws Exception { Assertions.assertNotNull(assertion); } + @DisplayName("Test retrieving Assertion encrypted with AES-GCM") + @Test + public void testGetAssertionEncryptedWithGcm() throws Exception { + String nameID = "https://data.gov.dk/model/core/edi/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7"; + + AssertionService assertionService = new AssertionService(); + Assertion assertion = assertionService.getAssertion(IdpUtil.createResponse(true, true, true, nameID, TestConstants.SP_ENTITY_ID, TestConstants.SP_ASSERTION_CONSUMER_URL, UUID.randomUUID().toString(), + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256_GCM, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP)); + + Assertions.assertNotNull(assertion); + Assertions.assertEquals(nameID, assertion.getSubject().getNameID().getValue()); + } + + @DisplayName("Test that an Assertion with a key transport algorithm outside the profile is rejected") + @Test + public void testRejectAssertionWithDisallowedKeyTransportAlgorithm() throws Exception { + AssertionService assertionService = new AssertionService(); + + // RSA 1.5 key transport is not one of the algorithms allowed by [OIO-ALG-01] + Assertions.assertThrows(ExternalException.class, () -> { + assertionService.getAssertion(IdpUtil.createResponse(true, true, true, "NAMEID", TestConstants.SP_ENTITY_ID, TestConstants.SP_ASSERTION_CONSUMER_URL, UUID.randomUUID().toString(), + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSA15)); + }); + } + + @DisplayName("Test that an Assertion with a block encryption algorithm outside the profile is rejected") + @Test + public void testRejectAssertionWithDisallowedBlockEncryptionAlgorithm() throws Exception { + AssertionService assertionService = new AssertionService(); + + // Triple DES block encryption is not one of the algorithms allowed by [OIO-ALG-01] + Assertions.assertThrows(ExternalException.class, () -> { + assertionService.getAssertion(IdpUtil.createResponse(true, true, true, "NAMEID", TestConstants.SP_ENTITY_ID, TestConstants.SP_ASSERTION_CONSUMER_URL, UUID.randomUUID().toString(), + EncryptionConstants.ALGO_ID_BLOCKCIPHER_TRIPLEDES, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP)); + }); + } + @DisplayName("Test retrieving badly formatted plaintext Assertion") @Test public void testGetBadlyEncryptedAssertion() throws Exception { diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java b/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java index 427dcb7..ee11307 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java @@ -89,6 +89,24 @@ public static Response createResponse( String recipientEntityId, String assertionConsumerUrl, String inResponseToId) throws Exception { + return createResponse(encrypted, validCert, validSignature, subjectNameID, recipientEntityId, assertionConsumerUrl, inResponseToId, + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP); + } + + /** + * @param dataAlgorithm algorithm the assertion itself is encrypted with + * @param keyTransportAlgorithm algorithm the encryption key is wrapped with + */ + public static Response createResponse( + boolean encrypted, + boolean validCert, + boolean validSignature, + String subjectNameID, + String recipientEntityId, + String assertionConsumerUrl, + String inResponseToId, + String dataAlgorithm, + String keyTransportAlgorithm) throws Exception { DateTime issueInstant = new DateTime(); @@ -113,7 +131,7 @@ public static Response createResponse( Assertion assertion = createAssertion(issueInstant, subjectNameID, recipientEntityId, assertionConsumerUrl); SignAssertion(assertion, validSignature); if (encrypted) { - EncryptedAssertion encryptedAssertion = encryptAssertion(assertion, validCert); + EncryptedAssertion encryptedAssertion = encryptAssertion(assertion, validCert, dataAlgorithm, keyTransportAlgorithm); response.getEncryptedAssertions().add(encryptedAssertion); } else { @@ -235,17 +253,17 @@ public static LogoutRequest createLogoutRequest(String nameID, String nameIDForm return outgoingLR; } - private static EncryptedAssertion encryptAssertion(Assertion assertion, boolean validCert) throws Exception { + private static EncryptedAssertion encryptAssertion(Assertion assertion, boolean validCert, String dataAlgorithm, String keyTransportAlgorithm) throws Exception { X509Certificate certificate = getSPCertificate(validCert); Credential keyEncryptionCredential = new BasicX509Credential(certificate); DataEncryptionParameters encParams = new DataEncryptionParameters(); - encParams.setAlgorithm(EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256); + encParams.setAlgorithm(dataAlgorithm); KeyEncryptionParameters kekParams = new KeyEncryptionParameters(); kekParams.setEncryptionCredential(keyEncryptionCredential); - kekParams.setAlgorithm(EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP); + kekParams.setAlgorithm(keyTransportAlgorithm); Encrypter samlEncrypter = new Encrypter(encParams, kekParams); samlEncrypter.setKeyPlacement(Encrypter.KeyPlacement.PEER); From a489cd3a1624624df80e071ce55cde7d3dd25e22 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Wed, 26 Aug 2026 18:20:38 +0200 Subject: [PATCH 2/2] Fixed merge conflicts. --- .../java/dk/gov/oio/saml/service/AssertionServiceTest.java | 6 +++--- oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/AssertionServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/AssertionServiceTest.java index 1491cd4..81bfa39 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/AssertionServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/AssertionServiceTest.java @@ -45,7 +45,7 @@ public void testGetAssertionEncryptedWithGcm() throws Exception { AssertionService assertionService = new AssertionService(); Assertion assertion = assertionService.getAssertion(IdpUtil.createResponse(true, true, true, nameID, TestConstants.SP_ENTITY_ID, TestConstants.SP_ASSERTION_CONSUMER_URL, UUID.randomUUID().toString(), - EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256_GCM, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP)); + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256_GCM, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP, TestConstants.SPEC_VERSION_OIOSAML_30)); Assertions.assertNotNull(assertion); Assertions.assertEquals(nameID, assertion.getSubject().getNameID().getValue()); @@ -59,7 +59,7 @@ public void testRejectAssertionWithDisallowedKeyTransportAlgorithm() throws Exce // RSA 1.5 key transport is not one of the algorithms allowed by [OIO-ALG-01] Assertions.assertThrows(ExternalException.class, () -> { assertionService.getAssertion(IdpUtil.createResponse(true, true, true, "NAMEID", TestConstants.SP_ENTITY_ID, TestConstants.SP_ASSERTION_CONSUMER_URL, UUID.randomUUID().toString(), - EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSA15)); + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSA15, TestConstants.SPEC_VERSION_OIOSAML_30)); }); } @@ -71,7 +71,7 @@ public void testRejectAssertionWithDisallowedBlockEncryptionAlgorithm() throws E // Triple DES block encryption is not one of the algorithms allowed by [OIO-ALG-01] Assertions.assertThrows(ExternalException.class, () -> { assertionService.getAssertion(IdpUtil.createResponse(true, true, true, "NAMEID", TestConstants.SP_ENTITY_ID, TestConstants.SP_ASSERTION_CONSUMER_URL, UUID.randomUUID().toString(), - EncryptionConstants.ALGO_ID_BLOCKCIPHER_TRIPLEDES, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP)); + EncryptionConstants.ALGO_ID_BLOCKCIPHER_TRIPLEDES, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP, TestConstants.SPEC_VERSION_OIOSAML_30)); }); } diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java b/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java index 1fb7b2c..ca55c94 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java @@ -79,7 +79,7 @@ public static MessageContext createMessageWithAssertion( String inResponseToId, String specVersion) throws Exception { // Create proxy Response - Response response = createResponse(encrypted, validCert, validSignature, subjectNameID, recipientEntityId, assertionConsumerUrl, inResponseToId, specVersion); + Response response = createResponse(encrypted, validCert, validSignature, subjectNameID, recipientEntityId, assertionConsumerUrl, inResponseToId, EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP, specVersion); // Build Proxy MessageContext and add response MessageContext messageContext = new MessageContext<>(); @@ -110,7 +110,7 @@ public static Response createResponse( String assertionConsumerUrl, String inResponseToId) throws Exception { return createResponse(encrypted, validCert, validSignature, subjectNameID, recipientEntityId, assertionConsumerUrl, inResponseToId, - EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP, TestConstants.SPEC_VERSION); + EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256, EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP, TestConstants.SPEC_VERSION_OIOSAML_30); } /** @@ -126,7 +126,7 @@ public static Response createResponse( String assertionConsumerUrl, String inResponseToId, String dataAlgorithm, - String keyTransportAlgorithm + String keyTransportAlgorithm, String specVersion) throws Exception { DateTime issueInstant = new DateTime();