Regression: SAML Authentication fails when EncryptedAssertion is signed but response is not signed

[sarod]

Describe the bug

With spring-security 5.5.0+ authenticating with SAML fails with Saml2AuthenticationException{error=[malformed_response_data] No assertions found in response.} when EncryptedAssertion is signed but response is not signed.

Similar response authenticates properly with spring-security 5.4.6.

To Reproduce
Configure SAML response to Encrypt & Sign Assertion but keep the message/response unsigned.
Try to authenticate with OpenSamlAuthenticationProvider or OpenSaml4AuthenticationProvider.

Alternatively the following test method can be added to OpenSamlAuthenticationProviderTests to reproduce.

	@Test
	public void authenticateWhenEncryptedAssertionWithSignatureAndNoResponseSignatureThenItSucceeds() {
		Response response = response();
		Assertion assertion = TestOpenSamlObjects.signed(assertion(),
				TestSaml2X509Credentials.assertingPartySigningCredential(), RELYING_PARTY_ENTITY_ID);
		EncryptedAssertion encryptedAssertion = TestOpenSamlObjects.encrypted(assertion,
				TestSaml2X509Credentials.assertingPartyEncryptingCredential());
		response.getEncryptedAssertions().add(encryptedAssertion);
		Saml2AuthenticationToken token = token(response, decrypting(verifying(registration())));
		this.provider.authenticate(token);
	}

NOTE: with current main branch it seems that gradle tasks opensaml3Test and opensaml4Test don't seem to run any tests. This may be user error because I'm not a gradle expert.
In any case I add to move all the code & tests back to the main and test sourcesets to be able to run the tests.

Expected behavior
Authentication should proceed without errors.

Analysis

In 5.4.6
The method OpenSamlAuthenticationProvider.decryptAssertions is called even if Response is not signed.
This mutates the Response.getAssertions() list by adding the decrypted assertions.
Then OpenSamlAuthenticationProvider#validateAssertions checks that some assertion exist with

List<Assertion> assertions = response.getAssertions();
if (assertions.isEmpty()) {
    throw createAuthenticationException(Saml2ErrorCodes.MALFORMED_RESPONSE_DATA,
            "No assertions found in response.", null);
}

In 5.5
In 5.5 the decryption logic has been moved to responseElementsDecrypter but the decryption is only called if response is signed.

boolean responseSigned = response.isSigned();
if (responseSigned) {
    this.responseElementsDecrypter.accept(responseToken);
}

So at this stage the Response.getAssertions() contains the decrypted assertions only if the response is signed.

Then the responseSignatureValidator is called which does

if (response.getAssertions().isEmpty()) {
    throw createAuthenticationException(Saml2ErrorCodes.MALFORMED_RESPONSE_DATA,
            "No assertions found in response.", null);
}

And this throws the exception.

Modifying code to always decrypting seems to fix the issue.

[sarod]

Hi @jzheaux,

This regression is pretty critical for us. So I would like to create a pull request to accelerate resolution if that's fine with you.

However the fact that the test cases in the opensaml3Test & opensaml4Test source sets are not run should probably be fixed before that in a separate PR.
The problem is that I have no clue how to fix the gradle build so that tests are run properly.
Do you have any idea of what could be wrong with the test execution?

[jzheaux]

The test execution is related to updating to JUnit 5. I've opened a ticket to address that.

As for whether one can safely decrypt an unsigned response, there has already been some discussion about this, so let's continue the conversation over there. If it becomes apparent that a code change is necessary, we can reopen this ticket.

[jzheaux]

In the meantime, you can decrypt them yourself by customizing the response validator, like so:

OpenSaml4AuthenticationProvider authenticationProvider = new OpenSaml4AuthenticationProvider();
Converter<ResponseToken, ResponseToken> decrypt = (responseToken) -> {
	DecryptionParameters parameters = new DecryptionParameters();
	// ... set parameters as needed
	Decrypter decrypter = new Decrypter(parameters);
	Response response = responseToken.getResponse();
  	EncryptedAssertion encrypted = response.getEncryptedAssertions().get(0);
  	try {
  		Assertion assertion = decrypter.decrypt(encrypted);
  		response.getAssertions().add(assertion);
  	} catch (Exception e) {
  	  	throw new Saml2AuthenticationException(...);
  	}
	return response;
};
authenticationProvider.setResponseValidator(decrypt.andThen(createDefaultResponseValidator()));

[sarod]

@jzheaux it looks like setResponseValidator was introduced in 5.6.0-M1 but 5.6.0 is not released yet and it's not available in 5.5.x right?
So it's looks like there is no workaround to this regression using a released version right?

[fr2lancer]

I found a handy method to enable decryption for this case

OpenSamlDecryptionUtils.decryptAssertionElements

however it is package protected. will it be opened in the future?

[jzheaux]

For those landing on @sarod's comment, the conversation is continued on #9044.