search

javaee / metro-wsit

https://javaee.github.io/metro-wsit/
Other
9 stars 24 forks source link

ImportSamlAssertionFilter - ClassCastException in SAML Token Handling #1107

Closed glassfishrobot closed 15 years ago

glassfishrobot commented 15 years ago

When I have a soap request with text elements in der security header before the saml token (this is a valid request I think) I became a ClassCastException. This is very hard and I think we must check the XML element form the parser befor we can cast it to XML element. The attachted code shows a possible patch for this problem.

In the class com.sun.xml.wss.impl.filter.ImportSamlAssertionFilter line 88 we shold have a code like this: ... Object obj = iter.next(); if (obj instanceof Element) { elem = (Element) obj; ...

Then the problem is fixed, we use this patch by putting the patched class in the local classpath.

/*

/*

package com.sun.xml.wss.impl.filter;

import com.sun.xml.wss.impl.misc.DefaultSecurityEnvironmentImpl; import com.sun.xml.wss.saml.Assertion; import com.sun.xml.wss.saml.AssertionUtil;

import java.util.Iterator; import java.util.HashMap; import java.util.logging.Level; import javax.xml.soap.SOAPElement; import org.w3c.dom.Element;

import com.sun.xml.wss.impl.MessageConstants; import com.sun.xml.wss.impl.SecurableSoapMessage; import com.sun.xml.wss.XWSSecurityException; import com.sun.xml.wss.impl.FilterProcessingContext; import com.sun.xml.wss.logging.LogDomainConstants; import com.sun.xml.wss.core.SecurityHeader; import com.sun.xml.wss.impl.policy.mls.AuthenticationTokenPolicy;

import java.util.logging.Logger; import org.w3c.dom.NodeList; import com.sun.xml.wss.impl.policy.mls.MessagePolicy;

/**

protected static final Logger log = Logger.getLogger( LogDomainConstants.FILTER_DOMAIN,LogDomainConstants.FILTER_DOMAIN_BUNDLE);

public static void process(FilterProcessingContext context) throws XWSSecurityException {

SecurableSoapMessage secureMessage = context.getSecurableSoapMessage(); SecurityHeader wsseSecurity = secureMessage.findSecurityHeader(); Assertion samlAssertion = null; SOAPElement samlElement = null;

if( context.getMode() == FilterProcessingContext.ADHOC || context.getMode() == FilterProcessingContext.DEFAULT || context.getMode() == FilterProcessingContext.WSDL_POLICY) {

NodeList nl = null; Element elem = null;

for(Iterator iter = wsseSecurity.getChildElements(); iter.hasNext(){

Object obj = iter.next(); if (obj instanceof Element) { elem = (Element) obj; if (elem.getAttributeNode("ID") != null )

{ nl = wsseSecurity.getElementsByTagNameNS( MessageConstants.SAML_v2_0_NS, MessageConstants.SAML_ASSERTION_LNAME); break; }

else if(elem.getAttributeNode("AssertionID")!= null)

{ nl = wsseSecurity.getElementsByTagNameNS( MessageConstants.SAML_v1_0_NS, MessageConstants.SAML_ASSERTION_LNAME); break; }

} } // if (wsseSecurity.getChildElements()Attributes().equals("AssertionID"))

{ // nl = wsseSecurity.getElementsByTagNameNS( // MessageConstants.SAML_v1_0_NS, MessageConstants.SAML_ASSERTION_LNAME); // }

else

{ // nl = wsseSecurity.getElementsByTagNameNS( // MessageConstants.SAML_v2_0_NS, MessageConstants.SAML_ASSERTION_LNAME); // }

if (nl == null)

{ throw new XWSSecurityException("SAMLAssertion is null"); }

int nodeListLength = nl.getLength(); int countSamlInsideAdviceElement = 0; for(int i =0; i<nodeListLength; i++){ if(nl.item.getParentNode().getLocalName().equals("Advice"))

{ countSamlInsideAdviceElement++; }

}

//for now we dont allow multiple saml assertions if (nodeListLength == 0)

{ // Log Message throw new XWSSecurityException( "No SAML Assertion found, Reciever requirement not met"); //}

else if ((nodeListLength - countSamlInsideAdviceElement) > 1)

{ // throw new XWSSecurityException( // "More than one SAML Assertion found, Reciever requirement not met"); }

else{ samlElement = (SOAPElement)nl.item(0); try

{ samlAssertion = AssertionUtil.fromElement(samlElement); }

catch(Exception e)

{ log.log(Level.SEVERE,"WSS0418.saml.import.exception"); throw SecurableSoapMessage.newSOAPFaultException( MessageConstants.WSSE_INVALID_SECURITY, "Exception while importing SAML Token", e); }

}

if (context.getMode() == FilterProcessingContext.ADHOC) {

//try to validate against the policy AuthenticationTokenPolicy policy = (AuthenticationTokenPolicy)context.getSecurityPolicy(); AuthenticationTokenPolicy.SAMLAssertionBinding samlPolicy =

(AuthenticationTokenPolicy.SAMLAssertionBinding)policy.getFeatureBinding();

//ensure the authorityId if specified matches if (!"".equals(samlPolicy.getAuthorityIdentifier())) { if (!samlPolicy.getAuthorityIdentifier().equals(samlAssertion.getSamlIssuer()))

{ //log here XWSSecurityException xwse = new XWSSecurityException("Invalid Assertion Issuer, expected " + samlPolicy.getAuthorityIdentifier() + ", found " + (samlAssertion.getSamlIssuer())); throw SecurableSoapMessage.newSOAPFaultException( MessageConstants.WSSE_INVALID_SECURITY_TOKEN, "Received SAML Assertion has invalid Issuer", xwse); }

} }

}else { if (context.getMode() == FilterProcessingContext.POSTHOC)

{ throw new XWSSecurityException( "Internal Error: Called ImportSAMLAssertionFilter in POSTHOC Mode"); }

if (context.getMode() == FilterProcessingContext.WSDL_POLICY)

{ AuthenticationTokenPolicy.SAMLAssertionBinding bind = new AuthenticationTokenPolicy.SAMLAssertionBinding(); ((MessagePolicy)context.getInferredSecurityPolicy()).append(bind); }

try

{ samlAssertion = AssertionUtil.fromElement(wsseSecurity.getCurrentHeaderElement()); }

catch(Exception ex)

{ throw SecurableSoapMessage.newSOAPFaultException( MessageConstants.WSSE_INVALID_SECURITY_TOKEN, "Exception while importing SAML Assertion", ex); }

}

HashMap tokenCache = context.getTokenCache(); //assuming unique IDs tokenCache.put(samlAssertion.getAssertionID(), samlAssertion);

//if (!samlAssertion.isTimeValid())

{ // log.log(Level.SEVERE, "WSS0417.saml.timestamp.invalid"); // throw SecurableSoapMessage.newSOAPFaultException( // MessageConstants.WSSE_FAILED_AUTHENTICATION, // "SAML Condition (notBefore, notOnOrAfter) Validation failed", // new Exception( // "SAML Condition (notBefore, notOnOrAfter) Validation failed")); //}

//ensure it is an SV assertion /*String confirmationMethod = AssertionUtil.getConfirmationMethod(samlElement); if (!MessageConstants.SAML_SENDER_VOUCHES.equals(confirmationMethod))

{ XWSSecurityException xwse = new XWSSecurityException("Invalid ConfirmationMethod " + confirmationMethod); throw SecurableSoapMessage.newSOAPFaultException( MessageConstants.WSSE_INVALID_SECURITY, "Invalid ConfirmationMethod", xwse); }

*/

context.getSecurityEnvironment().validateSAMLAssertion(context.getExtraneousProperties(), samlElement);

context.getSecurityEnvironment().updateOtherPartySubject( DefaultSecurityEnvironmentImpl.getSubject(context), samlAssertion);

}

}

Environment

Operating System: All Platform: All

Affected Versions

[1.4]

glassfishrobot commented 15 years ago

Reported by christianbaranowski@java.net

glassfishrobot commented 15 years ago

christianbaranowski@java.net said: The bug is found in Version 1.4

glassfishrobot commented 15 years ago

achimbitzer@java.net said: adding myself to cc

glassfishrobot commented 15 years ago

jhiller@java.net said: added myself to CC

glassfishrobot commented 15 years ago

sm228678@java.net said: fixed

glassfishrobot commented 15 years ago

achimbitzer@java.net said: Thank You for the (partial) fix. I've tested the fix with the current nightly version (metro 08.04.2009).

Here are my results: The for loop now contains a type check using instanceof to prevent ClassCastExceptions. An XWSSecurityException is thrown if the instanceof check fails:

NodeList nl = null; Element elem = null;

for(Iterator iter = wsseSecurity.getChildElements(); iter.hasNext(){ Object obj = iter.next(); if (obj instanceof Element)

{ nl = ...; }else { throw new XWSSecurityException("SAMLAssertion is not valid"); } }

If there are whitespaces in the security header in front of the SAML assertion element, the following happens: iter.next() returns a Text (not an Element). Therefore the instanceof check fails and the XWSSecurityException is thrown with the error message "SAMLAssertion is not valid". So whitespace still causes an exception / soap fault.

I think the exception message is misleading as there is no invalid SAMLAssertion but just text content inside the security header.

As the security header is an xsd:any, whitespace is explicitly allowed (to be more specific: textual content is explicitly allowed). Therefore no exception should be thrown if iter.next() returns a Text instead of an Element:

NodeList nl = null; Element elem = null;

for(Iterator iter = wsseSecurity.getChildElements(); iter.hasNext(){ Object obj = iter.next(); if (obj instanceof Element) { nl = ...; }

// no else block throwing an Exception }

If no SAML assertion is found inside the for loop, the variable nl remains null. This is handled correctly after the for loop:

if (nl == null)

{ throw new XWSSecurityException("SAMLAssertion is null"); }

Therefore no exception must be thrown inside the foor loop if iter.next() returns no Element. It would be great if You could remove the exception inside the for loop!

glassfishrobot commented 15 years ago

achimbitzer@java.net said: One other thing I've noticed:

iter.next() is now called twice inside the for loop:

86 for (Iterator iter = wsseSecurity.getChildElements(); iter.hasNext() { 87 Object obj = iter.next(); 88 if (obj instanceof Element) { 89 elem = (Element) iter.next(); 90 if (elem.getAttributeNode("ID") != null) { // [...]

The first invocation occurs in line 87, the second invocation occurs in line 89. The result is a java.util.NoSuchElementException.

The variable obj already contains the current element, so this variable should also be used in line 89:

86 for (Iterator iter = wsseSecurity.getChildElements(); iter.hasNext() { 87 Object obj = iter.next(); 88 if (obj instanceof Element) { 89 elem = (Element) obj; // use obj here instead of iter.next() 90 if (elem.getAttributeNode("ID") != null) { // [...]

Could You please change this, too? Thanks in advance

glassfishrobot commented 15 years ago

sm228678@java.net said: changes made to the previous fix. please check it with tomorrow's nightly and let us know.

glassfishrobot commented 15 years ago

achimbitzer@java.net said: thanks a lot, now it works!

glassfishrobot commented 15 years ago

Was assigned to kumarjayanti@java.net

glassfishrobot commented 7 years ago

This issue was imported from java.net JIRA WSIT-1107

glassfishrobot commented 15 years ago

Marked as fixed on Monday, April 13th 2009, 7:26:55 pm