This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/WSS-697 in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
commit 757a2b5306a587c411e31c31286074ec29a423e4 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Fri Oct 30 05:54:54 2026 +0000 WSS-697 - Don't override an existing OpenSAML configuration in OpenSAMLUtil.initSamlEngine --- .../org/apache/wss4j/common/saml/OpenSAMLUtil.java | 83 +++++++-- .../apache/wss4j/common/saml/OpenSAMLUtilTest.java | 206 ++++++++++++++++++++- 2 files changed, 273 insertions(+), 16 deletions(-) diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLUtil.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLUtil.java index 368d3bac4..dbca47ede 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLUtil.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLUtil.java @@ -19,6 +19,11 @@ package org.apache.wss4j.common.saml; +import java.util.HashMap; +import java.util.Map; +import java.util.ServiceConfigurationError; + +import javax.xml.XMLConstants; import javax.xml.namespace.QName; import net.shibboleth.shared.xml.impl.BasicParserPool; @@ -26,7 +31,6 @@ import net.shibboleth.shared.xml.ParserPool; import org.apache.wss4j.common.crypto.WSProviderConfig; import org.apache.wss4j.common.ext.WSSecurityException; -import org.opensaml.core.config.Configuration; import org.opensaml.core.config.ConfigurationService; import org.opensaml.core.config.provider.MapBasedConfiguration; import org.opensaml.core.xml.XMLObject; @@ -60,6 +64,9 @@ public final class OpenSAMLUtil { private static final org.slf4j.Logger LOG = org.slf4j.LoggerFactory.getLogger(OpenSAMLUtil.class); + private static final String DEFER_NODE_EXPANSION_FEATURE = + "http://apache.org/xml/features/dom/defer-node-expansion";; + private static XMLObjectProviderRegistry providerRegistry; private static XMLObjectBuilderFactory builderFactory; private static MarshallerFactory marshallerFactory; @@ -82,31 +89,42 @@ public final class OpenSAMLUtil { LOG.debug("Initializing the opensaml2 library..."); WSProviderConfig.init(); - Configuration configuration = new MapBasedConfiguration(); - ConfigurationService.setConfiguration(configuration); + try { + XMLObjectProviderRegistry existingRegistry = getExistingProviderRegistry(); + if (existingRegistry != null) { + // OpenSAML has already been initialized (e.g. via InitializationService), so keep the + // existing configuration, including the parser pools + LOG.debug("Using the existing OpenSAML configuration"); + providerRegistry = existingRegistry; + } else { + ConfigurationService.setConfiguration(new MapBasedConfiguration()); - providerRegistry = new XMLObjectProviderRegistry(); - configuration.register(XMLObjectProviderRegistry.class, providerRegistry, - ConfigurationService.DEFAULT_PARTITION_NAME); + providerRegistry = new XMLObjectProviderRegistry(); + ConfigurationService.register(XMLObjectProviderRegistry.class, providerRegistry); + } - try { + // Always load the object providers, as an existing initialization may be partial (e.g. in OSGi, + // InitializationService only finds the initializers that are visible to the context classloader) OpenSAMLBootstrap.bootstrap(includeXacml); - SAMLConfiguration samlConfiguration = new SAMLConfiguration(); - - configuration.register(SAMLConfiguration.class, samlConfiguration, - ConfigurationService.DEFAULT_PARTITION_NAME); + if (ConfigurationService.get(SAMLConfiguration.class) == null) { + ConfigurationService.register(SAMLConfiguration.class, new SAMLConfiguration()); + } builderFactory = XMLObjectProviderRegistrySupport.getBuilderFactory(); marshallerFactory = XMLObjectProviderRegistrySupport.getMarshallerFactory(); unmarshallerFactory = XMLObjectProviderRegistrySupport.getUnmarshallerFactory(); try { - configureParserPool(); + if (providerRegistry.getParserPool() == null) { + configureParserPool(); + } // used by org.opensaml.saml.saml2.encryption.Decrypter - configuration.register(DecryptionParserPool.class, new DecryptionParserPool(getParserPool()), - ConfigurationService.DEFAULT_PARTITION_NAME); + if (ConfigurationService.get(DecryptionParserPool.class) == null) { + ConfigurationService.register(DecryptionParserPool.class, + new DecryptionParserPool(createDecryptionParserPool())); + } } catch (Throwable t) { LOG.warn("Unable to bootstrap the parser pool part of the opensaml library " + "- some SAML operations may fail", t); @@ -120,6 +138,17 @@ public final class OpenSAMLUtil { } } + private static XMLObjectProviderRegistry getExistingProviderRegistry() { + try { + return ConfigurationService.get(XMLObjectProviderRegistry.class); + } catch (ServiceConfigurationError e) { + // The lookup of a Configuration implementation via the ServiceLoader failed (e.g. due to + // classloader visibility in OSGi), so treat OpenSAML as not initialized + LOG.debug("Unable to look up an existing OpenSAML configuration", e); + return null; + } + } + private static void configureParserPool() throws Throwable { BasicParserPool pp = new BasicParserPool(); pp.setMaxPoolSize(50); @@ -127,6 +156,32 @@ public final class OpenSAMLUtil { providerRegistry.setParserPool(pp); } + /** + * Create a ParserPool for decryption, configured in the same way as OpenSAML's + * DecryptionParserPoolInitializer. + */ + private static ParserPool createDecryptionParserPool() { + try { + BasicParserPool pp = new BasicParserPool(); + pp.setMaxPoolSize(50); + + // Start with the default (hardened) features, and add the decryption-specific feature. The + // hardening features are set explicitly so that they cannot be lost when the features are replaced. + Map<String, Boolean> features = new HashMap<>(pp.getBuilderFeatures()); + features.put(XMLConstants.FEATURE_SECURE_PROCESSING, Boolean.TRUE); + features.put("http://apache.org/xml/features/disallow-doctype-decl";, Boolean.TRUE); + features.put(DEFER_NODE_EXPANSION_FEATURE, Boolean.FALSE); + pp.setBuilderFeatures(features); + + pp.initialize(); + return pp; + } catch (Throwable t) { + // e.g. the XML parser does not support the feature - fall back to the default parser pool + LOG.warn("Unable to create a decryption-specific parser pool, using the default parser pool instead", t); + return getParserPool(); + } + } + /** * Get the configured ParserPool. * diff --git a/ws-security-common/src/test/java/org/apache/wss4j/common/saml/OpenSAMLUtilTest.java b/ws-security-common/src/test/java/org/apache/wss4j/common/saml/OpenSAMLUtilTest.java index 8cc20b258..3ddbb065e 100644 --- a/ws-security-common/src/test/java/org/apache/wss4j/common/saml/OpenSAMLUtilTest.java +++ b/ws-security-common/src/test/java/org/apache/wss4j/common/saml/OpenSAMLUtilTest.java @@ -19,19 +19,221 @@ package org.apache.wss4j.common.saml; +import java.io.ByteArrayInputStream; +import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; +import java.util.ServiceLoader; + +import javax.xml.namespace.QName; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.validation.Schema; + +import net.shibboleth.shared.xml.ParserPool; +import net.shibboleth.shared.xml.impl.BasicParserPool; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.opensaml.core.config.ConfigurationService; +import org.opensaml.core.config.Initializer; +import org.opensaml.core.config.provider.MapBasedConfiguration; +import org.opensaml.core.xml.config.XMLObjectProviderRegistry; +import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; +import org.opensaml.saml.saml2.core.Assertion; +import org.opensaml.xmlsec.config.DecryptionParserPool; +import org.opensaml.xmlsec.signature.X509Certificate; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** - * Some tests for the Merlin Crypto provider + * Some tests for OpenSAMLUtil */ public class OpenSAMLUtilTest { + @BeforeEach + public void resetOpenSAML() throws Exception { + // Start each test with neither OpenSAML nor WSS4J initialized + ConfigurationService.setConfiguration(new MapBasedConfiguration()); + Field initialized = OpenSAMLUtil.class.getDeclaredField("samlEngineInitialized"); + initialized.setAccessible(true); + initialized.setBoolean(null, false); + } + @Test public void testInitSamlEngine() throws Exception { OpenSAMLUtil.initSamlEngine(); // https://issues.apache.org/jira/browse/WSS-678 OpenSAML Decrypter initialization failed new org.opensaml.saml.saml2.encryption.Decrypter(null, null, null); + + // The decryption parser pool is distinct from the general parser pool + DecryptionParserPool decryptionParserPool = ConfigurationService.get(DecryptionParserPool.class); + assertNotNull(decryptionParserPool); + assertNotSame(OpenSAMLUtil.getParserPool(), decryptionParserPool.getParserPool()); + assertRejectsDoctype(decryptionParserPool.getParserPool()); + + assertWss4jConfig(); + } + + // https://issues.apache.org/jira/browse/WSS-697 + @Test + public void testExistingOpenSAMLConfigurationIsPreserved() throws Exception { + // Initialize OpenSAML in the same way as InitializationService.initialize(), except for the + // metrics initializer, as metrics-core is excluded from the WSS4J dependencies + for (Initializer initializer : ServiceLoader.load(Initializer.class)) { + if (!initializer.getClass().getName().endsWith(".MetricRegistryInitializer")) { + initializer.init(); + } + } + + ParserPool parserPool = XMLObjectProviderRegistrySupport.getParserPool(); + DecryptionParserPool decryptionParserPool = ConfigurationService.get(DecryptionParserPool.class); + assertNotNull(parserPool); + assertNotNull(decryptionParserPool); + + OpenSAMLUtil.initSamlEngine(); + + assertSame(parserPool, XMLObjectProviderRegistrySupport.getParserPool()); + assertSame(parserPool, OpenSAMLUtil.getParserPool()); + assertSame(decryptionParserPool, ConfigurationService.get(DecryptionParserPool.class)); + + new org.opensaml.saml.saml2.encryption.Decrypter(null, null, null); + + assertWss4jConfig(); + } + + // e.g. in OSGi, InitializationService may only find some of the initializers + @Test + public void testPartialOpenSAMLConfiguration() throws Exception { + XMLObjectProviderRegistry registry = new XMLObjectProviderRegistry(); + ConfigurationService.register(XMLObjectProviderRegistry.class, registry); + BasicParserPool parserPool = new BasicParserPool(); + parserPool.initialize(); + registry.setParserPool(parserPool); + + OpenSAMLUtil.initSamlEngine(); + + assertSame(registry, ConfigurationService.get(XMLObjectProviderRegistry.class)); + assertSame(parserPool, OpenSAMLUtil.getParserPool()); + assertNotNull(XMLObjectProviderRegistrySupport.getBuilderFactory().getBuilder(Assertion.DEFAULT_ELEMENT_NAME)); + assertNotNull(ConfigurationService.get(DecryptionParserPool.class)); + assertWss4jConfig(); + } + + @Test + public void testDecryptionParserPoolFallback() throws Exception { + String factoryProperty = DocumentBuilderFactory.class.getName(); + String oldFactory = System.getProperty(factoryProperty); + System.setProperty(factoryProperty, NoDeferNodeExpansionDocumentBuilderFactory.class.getName()); + try { + OpenSAMLUtil.initSamlEngine(); + } finally { + if (oldFactory == null) { + System.clearProperty(factoryProperty); + } else { + System.setProperty(factoryProperty, oldFactory); + } + } + + // The XML parser does not support the decryption-specific feature, so the default parser pool is used + DecryptionParserPool decryptionParserPool = ConfigurationService.get(DecryptionParserPool.class); + assertNotNull(decryptionParserPool); + assertSame(OpenSAMLUtil.getParserPool(), decryptionParserPool.getParserPool()); + assertRejectsDoctype(decryptionParserPool.getParserPool()); + + new org.opensaml.saml.saml2.encryption.Decrypter(null, null, null); + } + + private static void assertRejectsDoctype(ParserPool parserPool) { + String xxe = "<!DOCTYPE root [<!ENTITY xxe SYSTEM \"file:///etc/passwd\">]><root>&xxe;</root>"; + assertThrows(Exception.class, + () -> parserPool.parse(new ByteArrayInputStream(xxe.getBytes(StandardCharsets.UTF_8)))); + } + + private static void assertWss4jConfig() { + assertTrue(XMLObjectProviderRegistrySupport.getUnmarshallerFactory() + .getUnmarshaller(X509Certificate.DEFAULT_ELEMENT_NAME) instanceof WSS4JXSBase64BinaryUnmarshaller); + assertNotNull(XMLObjectProviderRegistrySupport.getBuilderFactory() + .getBuilder(new QName("urn:oasis:xacml:2.0:saml:assertion:schema:os", "XACMLAuthzDecisionStatement"))); + } + + /** + * A DocumentBuilderFactory that does not support the "defer-node-expansion" feature + */ + public static class NoDeferNodeExpansionDocumentBuilderFactory extends DocumentBuilderFactory { + private final DocumentBuilderFactory delegate = DocumentBuilderFactory.newDefaultInstance(); + + @Override + public void setFeature(String name, boolean value) throws ParserConfigurationException { + if (name.endsWith("defer-node-expansion")) { + throw new ParserConfigurationException("Feature not supported: " + name); + } + delegate.setFeature(name, value); + } + + @Override + public boolean getFeature(String name) throws ParserConfigurationException { + return delegate.getFeature(name); + } + + @Override + public DocumentBuilder newDocumentBuilder() throws ParserConfigurationException { + return delegate.newDocumentBuilder(); + } + + @Override + public void setAttribute(String name, Object value) { + delegate.setAttribute(name, value); + } + + @Override + public Object getAttribute(String name) { + return delegate.getAttribute(name); + } + + @Override + public void setNamespaceAware(boolean awareness) { + delegate.setNamespaceAware(awareness); + } + + @Override + public void setValidating(boolean validating) { + delegate.setValidating(validating); + } + + @Override + public void setIgnoringElementContentWhitespace(boolean whitespace) { + delegate.setIgnoringElementContentWhitespace(whitespace); + } + + @Override + public void setExpandEntityReferences(boolean expandEntityRef) { + delegate.setExpandEntityReferences(expandEntityRef); + } + + @Override + public void setIgnoringComments(boolean ignoreComments) { + delegate.setIgnoringComments(ignoreComments); + } + + @Override + public void setCoalescing(boolean coalescing) { + delegate.setCoalescing(coalescing); + } + + @Override + public void setSchema(Schema schema) { + delegate.setSchema(schema); + } + + @Override + public void setXIncludeAware(boolean state) { + delegate.setXIncludeAware(state); + } } -} \ No newline at end of file +}
