wso2 / product-is

Welcome to the WSO2 Identity Server source code! For info on working with the WSO2 Identity Server repository and contributing code, click the link below.
http://wso2.github.io/
Apache License 2.0
735 stars 714 forks source link

`javax.xml.parsers.DocumentBuilderFactory` property is not honoured and leads to `DOM Element node adoption failed` error #20441

Open vfraga opened 2 months ago

vfraga commented 2 months ago

Describe the issue: When marshalling (serialising) an OpenSAML XMLObject using the SSOUtils::marshall(XMLObject) method [1], the Identity Server tries to set the javax.xml.parsers.DocumentBuilderFactory property at runtime before starting the operations using the OpenSAML/Shibboleth helper classes to get the equivalent SAML XML String from the Java OpenSAML XMLObject. However, the AbstractXMLObjectMarshaller::marshall(XMLObject) method relies on a parser pool to create a new document to act as a root document:

@Nonnull
public Element marshall(@Nonnull XMLObject xmlObject) throws MarshallingException {
    try {
        Document document = XMLObjectProviderRegistrySupport.getParserPool().newDocument();
        return this.marshall(xmlObject, document);
    } catch (XMLParserException var3) {
        XMLParserException e = var3;
        throw new MarshallingException("Unable to create Document to place marshalled elements in", e);
    }
}

This document is of class com.sun.org.apache.xerces.internal.dom.DocumentImpl, instead of the expected org.apache.xerces.dom.DeferredDocumentImpl, because the parser pool was already initialised before the javax.xml.parsers.DocumentBuilderFactory property was set at runtime at the start of the SSOUtils::marshall(XMLObject) method. Supposedly, when the SAMLInitializer::doBootstrap() method was called.

This causes an error when trying to unmarshall (deserialise) an SAML XML String into a OpenSAML Java XMLObject and then marshall (serialise) it again back into a SAML XML String:

Caused by: java.lang.RuntimeException: DOM Element node adoption failed. This is most likely caused by the Element and Document being produced by different DOM implementations.
    at net.shibboleth.utilities.java.support.xml.ElementSupport.adoptElement(ElementSupport.java:59)
    at org.opensaml.core.xml.io.AbstractXMLObjectMarshaller.marshall(AbstractXMLObjectMarshaller.java:100)
    at org.opensaml.saml.common.AbstractSAMLObjectMarshaller.marshall(AbstractSAMLObjectMarshaller.java:57)
    at org.opensaml.core.xml.io.AbstractXMLObjectMarshaller.marshall(AbstractXMLObjectMarshaller.java:75)
    at org.wso2.carbon.identity.application.authenticator.samlsso.util.SSOUtils.marshall(SSOUtils.java:377)
    ... 73 more

Related code:

public static void adoptElement(@Nonnull Document adopter, @Nonnull Element adoptee) {
    Constraint.isNotNull(adoptee, "Adoptee Element may not be null");
    Constraint.isNotNull(adopter, "Adopter Element may not be null");
    if (!adoptee.getOwnerDocument().isSameNode(adopter) && adopter.adoptNode(adoptee) == null) {
        throw new RuntimeException("DOM Element node adoption failed. This is most likely caused by the Element and Document being produced by different DOM implementations.");
    }
}

Example situation:

String decodedResponse = new String(org.apache.commons.codec.binary.Base64.decodeBase64(request.getParameter(
                SSOConstants.HTTP_POST_PARAM_SAML2_RESP).getBytes()));
XMLObject samlObject = SSOUtils.unmarshall(decodedResponse);

//Prints 'org.apache.xerces.dom.DeferredDocumentImpl'
System.out.println(((Response) samlObject).getAssertions().get(0).getDOM().getOwnerDocument().getClass().getCanonicalName());  

//Prints 'com.sun.org.apache.xerces.internal.dom.DocumentImpl'
System.out.println(XMLObjectProviderRegistrySupport.getParserPool().newDocument().getClass().getCanonicalName());

How to reproduce:

  1. Set up one Identity Server instance as a SAML Service Provider and use it as a SAML IdP for Federated Authentication in a second Identity Server instance.
  2. Set up an example service provider, and change the 'Local & Outbound Authentication' settings to use the new SAML IdP as the Federated Authenticator.
  3. Clone and compile the custom SAML SSO Manager [2], and add the configuration below to the deployment.toml file of the second instance:
    [authentication.authenticator.saml.parameters]
    SAML2SSOManager = "org.wso2.support.sample.manager.SAML2SSOManager"
    • This custom SAML SSO Manager has a code line that will try to marshall the obtained assertion [3].
  4. Go through the normal login flow in the example application to trigger the error.

Expected behavior: It should be possible to unmarshall and marshall the XMLObject in sequence, as they should've been the same DOM Implementation.

Environment information:

[1] https://github.com/wso2-extensions/identity-outbound-auth-samlsso/blob/v5.8.1/components/org.wso2.carbon.identity.application.authenticator.samlsso/src/main/java/org/wso2/carbon/identity/application/authenticator/samlsso/util/SSOUtils.java#L369 [2] https://github.com/vfraga/test_custom_sso_manager [3] https://github.com/vfraga/test_custom_sso_manager/blob/main/src/main/java/org/wso2/support/sample/manager/SAML2SSOManager.java#L568

vfraga commented 2 months ago

It should be possible to address this issue by changing the SAMLSSOManager::doBootstrap() method to set the javax.xml.parsers.DocumentBuilderFactory property before calling SAMLInitializer::doBootstrap(), as in the example [1]. Additionally, setting the property directly in the wso2server.sh startup script also addressed the issue.

[1] https://github.com/vfraga/test_custom_sso_manager/blob/main/src/main/java/org/wso2/support/sample/manager/SAML2SSOManager.java#L138

AnjanaSamindraPerera commented 1 month ago

@vfraga and I tried to reproduce this locally and couldn't. We should try a different way to produce this from the code [1].

[1] https://github.com/vfraga/test_custom_sso_manager