This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
The following commit(s) were added to refs/heads/master by this push:
new 8c4821133 WSS-697 - Don't override an existing OpenSAML configuration
in OpenSAMLUtil.initSamlEngine (#765)
8c4821133 is described below
commit 8c4821133d33f796742ac85517cc9e3739f3e98c
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Fri Oct 30 06:15:04 2026 +0000
WSS-697 - Don't override an existing OpenSAML configuration in
OpenSAMLUtil.initSamlEngine (#765)
---
.../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
+}
