Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to access encrypted SAML assertions in custom ResponseValidator after upgrade from 6.3 to 6.4 #16367

Open
kimgoetzke opened this issue Jan 6, 2025 · 0 comments
Labels
status: waiting-for-triage An issue we've not yet triaged type: bug A general bug

Comments

@kimgoetzke
Copy link

Describe the bug
In an application where spring-security-saml2-service-provider was upgraded from 6.3.x to 6.4.1, I am experiencing a change in behaviour where I'm struggling to access the list of encrypted assertions in a validator class. I think this may be a bug.

I'm configuring a response validator in my configuration as Spring intends by doing this:

var authProvider = new OpenSaml4AuthenticationProvider();
authProvider.setResponseValidator(responseValidator);

Having received a SAML Response with a single encrypted assertion, in said responseValidator, I'd be able to do something like this:

class SamlResponseValidator implements Converter<ResponseToken, Saml2ResponseValidatorResult> {

  @Override
  public Saml2ResponseValidatorResult convert(ResponseToken responseToken) {
    // Without any other overrides, the below gives you the Spring-decrypted assertion:
    var assertions = responseToken.getResponse().getAssertions();
    
    // In 6.3, the below would _also_ contain the still-encrypted assertions while
    // in 6.4, you'll always just get an empty list, it seems:
    var encryptedAssertions = responseToken.getResponse().getEncryptedAssertions();

    // (...)
  }
}

I can see that this change comes from what is now OpenSaml4Template.OpenSaml4DecryptionConfigurer.decryptResponse(Response response) which will do this:

// ...

int count = 0;
int size = response.getEncryptedAssertions().size();
for (EncryptedAssertion encrypted : response.getEncryptedAssertions()) {
   logger.trace(String.format("Decrypting EncryptedAssertion (%d/%d) in Response [%s]", count, size, response.getID()));
   try {
      Assertion decrypted = this.decrypter.decrypt(encrypted);
      if (decrypted != null) {
         encrypteds.add(encrypted);
         decrypteds.add(decrypted);
      }
      count++;
   } catch (DecryptionException ex) {
      throw new Saml2Exception(ex);
   }
}

response.getEncryptedAssertions().removeAll(encrypteds); // <-- The cause of what I'm seeing!
response.getAssertions().addAll(decrypteds);

// ...

The Javadoc on the method states that:

The methods that follow are adapted from OpenSAML's [classes].

Previously, response.getEncryptedAssertions().removeAll(encrypteds) did not exist.

As a result, I believe there to be two bugs:

  1. responseToken.getResponse().getEncryptedAssertions() no longer contains encrypted assertions when Spring decrypted them for you
  2. int count = 0 in OpenSaml4Template.OpenSaml4DecryptionConfigurer.decryptResponse(Response response) results in incorrect log statements such as Decrypting EncryptedAssertion (0/1) in Response [{id}] when there's only one assertion

To Reproduce

  1. Create a basic application that uses spring-security-saml2-service-provider in which you provide your own response validator with:
var authProvider = new OpenSaml4AuthenticationProvider();
authProvider.setResponseValidator(responseValidator);
  1. Check assertions in the response validator:
class SamlResponseValidator implements Converter<ResponseToken, Saml2ResponseValidatorResult> {

  @Override
  public Saml2ResponseValidatorResult convert(ResponseToken responseToken) {
    var encryptedAssertions = responseToken.getResponse().getEncryptedAssertions();

    // (...)
  }
}
  1. Execute the SAML flow with a SAML response containing an encrypted assertion and assert on/break point on encryptedAssertions
  2. Notice that the value will be 0 instead of 1

Expected behavior

  1. responseToken.getResponse().getEncryptedAssertions() should contain encrypted assertions
  2. The log statement Decrypting EncryptedAssertion (%d/%d) in Response [%s] should log the correct count i.e. 1/1 if one encrypted assertion was found (instead of 0/1)

Sample
Because of 1) how basic this is and 2) this possibly being intentional, I have not included one yet but happy to add it if required.

Before creating this issue, I asked this as a Stack Overflow question but didn't get an answer.

Please tell me if I'm wrong or missing something! If you do agree, I don't mind creating a PR.

@kimgoetzke kimgoetzke added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged type: bug A general bug
Projects
None yet
Development

No branches or pull requests

1 participant