Skip to content
This repository has been archived by the owner on Nov 29, 2022. It is now read-only.

SES-38: WebSSOProfileConsumerImpl fails if there is more than one AudienceRestriction #44

Closed
spring-projects-issues opened this issue Mar 4, 2010 · 5 comments
Labels
in: core An issue in spring-security-saml-core status: invalid An issue that we don't feel is valid type: bug A general bug type: jira An issue that was migrated from JIRA

Comments

@spring-projects-issues
Copy link

Phil Varner (Migrated from SES-38) said:

In WebSSOProfileConsumerImpl#verifyAssertionConditions, if the local entity is not in the first AudienceRestriction, an exception is thrown. The relevant code is:

340 audience:
341 for (AudienceRestriction rest : conditions.getAudienceRestrictions()) {
342 if (rest.getAudiences().size() == 0) {
343 log.debug("No audit audience specified for the assertion");
344 throw new SAMLException("SAML response is invalid");
345 }
346 for (Audience aud : rest.getAudiences()) {
347 if (context.getLocalEntityId().equals(aud.getAudienceURI())) {
348 continue audience;
==> this should return rather than continue.

349 }
350 }

==> If it's not in the first AR, we get here and throw an exception

351 log.debug("Our entity is not the intended audience of the assertion");
352 throw new SAMLException("SAML response is not intended for this entity");
353 }

@spring-projects-issues
Copy link
Author

Phil Varner said:

Patch:

Index: saml2-core/src/main/java/org/springframework/security/saml/websso/WebSSOProfileConsumerImpl.java

--- saml2-core/src/main/java/org/springframework/security/saml/websso/WebSSOProfileConsumerImpl.java (revision 74)
+++ saml2-core/src/main/java/org/springframework/security/saml/websso/WebSSOProfileConsumerImpl.java (working copy)
@@ -337,27 +337,24 @@
throw new SAMLException("SAML response is not valid");
}

  •    audience:
    
    • for (AudienceRestriction rest : conditions.getAudienceRestrictions()) {
    •    if (rest.getAudiences().size() == 0) {
             log.debug("No audit audience specified for the assertion");
             throw new SAMLException("SAML response is invalid");
         }
      
    •    for (Audience aud : rest.getAudiences()) {
             if (context.getLocalEntityId().equals(aud.getAudienceURI())) {
      
  •                continue audience;
    
  •                return;  // found!
             }
         }
    
  •        log.debug("Our entity is not the intended audience of the assertion");
    
  •        throw new SAMLException("SAML response is not intended for this entity");
     }
    
  •    /** ? BUG
    
  •     if (conditions.getConditions().size() > 0) {
    
  •     log.debug("Assertion contain not understood conditions");
    
  •     throw new SAMLException("SAML response is not valid");
    
  •     }
    
  •     */
    
  •    log.debug("Our entity " + context.getLocalEntityId() + " is not the intended audience of the assertion");
    
  •    throw new SAMLException("SAML response is not intended for this entity");
    

    }

    /**

@spring-projects-issues
Copy link
Author

Vladimir Schäfer said:

Hi, I'm just checking this bug and the original implementation seems to be correct. In SAML2-core, 922:

"Note that multiple elements MAY be included in a single assertion, and each
MUST be evaluated independently. The effect of this requirement and the preceding definition is that
within a given condition, the audiences form a disjunction (an "OR") while multiple conditions form a
conjunction (an "AND")."

The original code failed when any of the AudienceRestrictions didn't contain the current SP, as defined.

Is it possible that your assertions is not formatted in accordance with the specification? If this is the case I'd recommend to override the WebSSOProfileConsumerImpl#verifyAssertionConditions method in your custom implementation to solve the problem.

@spring-projects-issues
Copy link
Author

Phil Varner said:

Thanks Vladimir. When you say "the original code failed", you mean that it correctly returned an error?

@spring-projects-issues
Copy link
Author

Vladimir Schäfer said:

Yes, that's what I meant.

@spring-projects-issues
Copy link
Author

Phil Varner said:

It looks like Siteminder might not implement the spec correctly. I'm working with a client using Siteminder, and the Assertions contain two separate ARs with one correctly containing the entityID and the other with the Siteminder name for the IdP-SP configuration.

This document on WebEx / Siteminder integration has an example AuthnRequest in the appendix:
http://developer.webex.com/c/document_library/get_file?folderId=22041&name=DLFE-902.pdf
which contains two identical ARs, which I assume is because one was generated based on the SP entityID and the other was the Siteminder config name. In the case that these aren't the same, it seems to me it's always a spec violation, since the Siteminder name is never going to have any meaning to the SP.

This zip has a doc:http://developer.webex.com/c/document_library/get_file?folderId=22041&name=DLFE-1803.zip
describing the config of Siteminder. It looks like the on page 18 in the "SSO" tab, the "Audience" field is always included as an AR, even if it's the same as the entityID in the SP metadata.

The IDPM I'm working with told me this is how he always configures it, and that Google Apps and SFDC work with it.

Maybe there should be a "relaxed" mode by which the semantics of matches at least one AR passes?

@spring-projects-issues spring-projects-issues added in: core An issue in spring-security-saml-core Resolved type: bug A general bug status: invalid An issue that we don't feel is valid type: jira An issue that was migrated from JIRA labels Feb 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
in: core An issue in spring-security-saml-core status: invalid An issue that we don't feel is valid type: bug A general bug type: jira An issue that was migrated from JIRA
Development

No branches or pull requests

1 participant