[WSS-331] Insufficient checking of SAML Condition NotBefore/NotOnOrAfter validation dates (?)

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.6.5
• Assignee: Colm O hEigeartaigh
• Reporter: Glen Mazza
• Created: 27/Dec/11 19:02
• Updated: 05/Mar/12 13:21
• Resolved: 03/Jan/12 11:21

Description

Hi, the Assertions and Protocols for the OASIS Security Assertion Markup Language (SAML) V2.0 (Mar 2005) - docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf, gives this Schema for saml:Conditions:

<element name="Conditions" type="saml:ConditionsType"/>
<complexType name="ConditionsType">
<choice minOccurs="0" maxOccurs="unbounded">
<element ref="saml:Condition"/>
<element ref="saml:AudienceRestriction"/>
<element ref="saml:OneTimeUse"/>
<element ref="saml:ProxyRestriction"/>
</choice>
<attribute name="NotBefore" type="dateTime" use="optional"/>
<attribute name="NotOnOrAfter" type="dateTime" use="optional"/>
</complexType>

As shown above, NotBefore and NotOnOrAfter are both optional--however, absence of one should not negate checking of the other.

In class org.apache.ws.security.validate.SamlAssertionValidator on TRUNK, I see this code in method validate():

DateTime validFrom = null;
DateTime validTill = null;
if (assertion.getSamlVersion().equals(SAMLVersion.VERSION_20)
&& assertion.getSaml2().getConditions() != null)

{ validFrom = assertion.getSaml2().getConditions().getNotBefore(); validTill = assertion.getSaml2().getConditions().getNotOnOrAfter(); }

else if (assertion.getSamlVersion().equals(SAMLVersion.VERSION_11)
...similar...
}
if (validFrom != null && validTill != null
&& !(validFrom.isBeforeNow() && validTill.isAfterNow()))

{ LOG.debug("SAML Token condition not met"); throw new WSSecurityException(WSSecurityException.FAILURE, "invalidSAMLsecurity"); }

The If block right above will skip checking if either validFrom or validTo is missing, but if just one of the two constraints is present it appears that single constraint should still be checked. Also, the logic above requires both validFrom and validTill to be violated before the WSSecurityException is thrown, but it should be thrown even if just one of the two constraints fail.