From 2276fcf34ab476578ff8653a98f986435f1e135f Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 23 Jul 2020 14:45:31 -0600 Subject: [PATCH] Add OpenSamlInitializationService Closes gh-8772 --- .../core/OpenSamlInitializationService.java | 146 ++++++++++++++++++ .../OpenSamlAuthenticationProvider.java | 6 +- .../OpenSamlAuthenticationRequestFactory.java | 7 +- .../OpenSamlImplementation.java | 87 +---------- .../OpenSamlInitializationServiceTests.java | 47 ++++++ .../authentication/TestOpenSamlObjects.java | 5 + .../OpenSamlActionTestingSupport.java | 20 ++- 7 files changed, 229 insertions(+), 89 deletions(-) create mode 100644 saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/core/OpenSamlInitializationService.java create mode 100644 saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/core/OpenSamlInitializationServiceTests.java diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/core/OpenSamlInitializationService.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/core/OpenSamlInitializationService.java new file mode 100644 index 0000000000..a533a2753e --- /dev/null +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/core/OpenSamlInitializationService.java @@ -0,0 +1,146 @@ +/* + * Copyright 2002-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.saml2.core; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; +import javax.xml.XMLConstants; + +import net.shibboleth.utilities.java.support.xml.BasicParserPool; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.opensaml.core.config.ConfigurationService; +import org.opensaml.core.config.InitializationService; +import org.opensaml.core.xml.config.XMLObjectProviderRegistry; + +import org.springframework.security.saml2.Saml2Exception; + +import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; +import static org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport.setParserPool; + +/** + * An initialization service for initializing OpenSAML. Each Spring Security OpenSAML-based component invokes + * the {@link #initialize()} method at static initialization time. + * + * {@link #initialize()} is idempotent and may be safely called in custom classes that need OpenSAML to be + * initialized in order to function correctly. It's recommended that you call this {@link #initialize()} method + * when using Spring Security and OpenSAML instead of OpenSAML's {@link InitializationService#initialize()}. + * + * The primary purpose of {@link #initialize()} is to prepare OpenSAML's {@link XMLObjectProviderRegistry} + * with some reasonable defaults. Any changes that Spring Security makes to the registry happen in this method. + * + * To override those defaults, call {@link #requireInitialize(Consumer)} and change the registry: + * + * 
+ * 	static {
+ *  	OpenSamlInitializationService.requireInitialize(registry -> {
+ *  	 	registry.setParserPool(...);
+ *  		registry.getBuilderFactory().registerBuilder(...);
+ *  	});
+ *  }
+ *
+ * + * {@link #requireInitialize(Consumer)} may only be called once per application. + * + * If the application already initialized OpenSAML before {@link #requireInitialize(Consumer)} was called, + * then the configuration changes will not be applied and an exception will be thrown. The reason for this is to + * alert you to the fact that there are likely some initialization ordering problems in your application that + * would otherwise lead to an unpredictable state. + * + * If you must change the registry's configuration in multiple places in your application, you are expected + * to handle the initialization ordering issues yourself instead of trying to call {@link #requireInitialize(Consumer)} + * multiple times. + * + * @author Josh Cummings + * @since 5.4 + */ +public class OpenSamlInitializationService { + private static final Log log = LogFactory.getLog(OpenSamlInitializationService.class); + private static final AtomicBoolean initialized = new AtomicBoolean(false); + + /** + * Ready OpenSAML for use and configure it with reasonable defaults. + * + * Initialization is guaranteed to happen only once per application. This method will passively return + * {@code false} if initialization already took place earlier in the application. + * + * @return whether or not initialization was performed. The first thread to initialize OpenSAML will + * return {@code true} while the rest will return {@code false}. + * @throws Saml2Exception if OpenSAML failed to initialize + */ + public static boolean initialize() { + return initialize(registry -> {}); + } + + /** + * Ready OpenSAML for use, configure it with reasonable defaults, and modify the {@link XMLObjectProviderRegistry} + * using the provided {@link Consumer}. + * + * Initialization is guaranteed to happen only once per application. This method will throw an exception + * if initialization already took place earlier in the application. + * + * @param registryConsumer the {@link Consumer} to further configure the {@link XMLObjectProviderRegistry} + * @throws Saml2Exception if initialization already happened previously or if OpenSAML failed to initialize + */ + public static void requireInitialize(Consumer registryConsumer) { + if (!initialize(registryConsumer)) { + throw new Saml2Exception("OpenSAML was already initialized previously"); + } + } + + private static boolean initialize(Consumer registryConsumer) { + if (initialized.compareAndSet(false, true)) { + log.trace("Initializing OpenSAML"); + + try { + InitializationService.initialize(); + } catch (Exception e) { + throw new Saml2Exception(e); + } + + BasicParserPool parserPool = new BasicParserPool(); + parserPool.setMaxPoolSize(50); + + Map parserBuilderFeatures = new HashMap<>(); + parserBuilderFeatures.put("http://apache.org/xml/features/disallow-doctype-decl", TRUE); + parserBuilderFeatures.put(XMLConstants.FEATURE_SECURE_PROCESSING, TRUE); + parserBuilderFeatures.put("http://xml.org/sax/features/external-general-entities", FALSE); + parserBuilderFeatures.put("http://apache.org/xml/features/validation/schema/normalized-value", FALSE); + parserBuilderFeatures.put("http://xml.org/sax/features/external-parameter-entities", FALSE); + parserBuilderFeatures.put("http://apache.org/xml/features/dom/defer-node-expansion", FALSE); + parserPool.setBuilderFeatures(parserBuilderFeatures); + + try { + parserPool.initialize(); + } catch (Exception e) { + throw new Saml2Exception(e); + } + setParserPool(parserPool); + + registryConsumer.accept(ConfigurationService.get(XMLObjectProviderRegistry.class)); + + log.debug("Initialized OpenSAML"); + return true; + } else { + log.debug("Refused to re-initialize OpenSAML"); + return false; + } + } +} diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProvider.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProvider.java index e93f5f89ec..eb8b353cb5 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProvider.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationProvider.java @@ -98,6 +98,7 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper; import org.springframework.security.saml2.Saml2Exception; +import org.springframework.security.saml2.core.OpenSamlInitializationService; import org.springframework.security.saml2.core.Saml2Error; import org.springframework.security.saml2.core.Saml2X509Credential; import org.springframework.util.Assert; @@ -160,6 +161,10 @@ import static org.springframework.util.Assert.notNull; */ public final class OpenSamlAuthenticationProvider implements AuthenticationProvider { + static { + OpenSamlInitializationService.initialize(); + } + private static Log logger = LogFactory.getLog(OpenSamlAuthenticationProvider.class); private final OpenSamlImplementation saml = OpenSamlImplementation.getInstance(); @@ -270,7 +275,6 @@ public final class OpenSamlAuthenticationProvider implements AuthenticationProvi } catch (Saml2Exception x) { throw authException(MALFORMED_RESPONSE_DATA, x.getMessage(), x); } - } private void process(Saml2AuthenticationToken token, Response response) { diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactory.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactory.java index ee7cb1bff0..71d9a18733 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactory.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactory.java @@ -17,6 +17,7 @@ package org.springframework.security.saml2.provider.service.authentication; import java.security.PrivateKey; +import java.security.cert.X509Certificate; import java.time.Clock; import java.time.Instant; import java.util.Collection; @@ -24,7 +25,6 @@ import java.util.Map; import java.util.UUID; import java.util.function.Consumer; import java.util.function.Function; -import java.security.cert.X509Certificate; import org.joda.time.DateTime; import org.opensaml.core.xml.io.MarshallingException; @@ -43,6 +43,7 @@ import org.opensaml.xmlsec.signature.support.SignatureSupport; import org.springframework.core.convert.converter.Converter; import org.springframework.security.saml2.Saml2Exception; +import org.springframework.security.saml2.core.OpenSamlInitializationService; import org.springframework.security.saml2.core.Saml2X509Credential; import org.springframework.security.saml2.provider.service.authentication.Saml2RedirectAuthenticationRequest.Builder; import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration; @@ -56,6 +57,10 @@ import static org.springframework.security.saml2.provider.service.authentication * @since 5.2 */ public class OpenSamlAuthenticationRequestFactory implements Saml2AuthenticationRequestFactory { + static { + OpenSamlInitializationService.initialize(); + } + private Clock clock = Clock.systemUTC(); private final OpenSamlImplementation saml = OpenSamlImplementation.getInstance(); diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlImplementation.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlImplementation.java index 55d867e754..04a929044e 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlImplementation.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlImplementation.java @@ -20,22 +20,14 @@ import java.io.ByteArrayInputStream; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Collection; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; -import javax.xml.XMLConstants; import javax.xml.namespace.QName; -import net.shibboleth.utilities.java.support.component.ComponentInitializationException; -import net.shibboleth.utilities.java.support.xml.BasicParserPool; import net.shibboleth.utilities.java.support.xml.SerializeSupport; import net.shibboleth.utilities.java.support.xml.XMLParserException; -import org.opensaml.core.config.ConfigurationService; -import org.opensaml.core.config.InitializationException; -import org.opensaml.core.config.InitializationService; import org.opensaml.core.xml.XMLObject; import org.opensaml.core.xml.XMLObjectBuilderFactory; -import org.opensaml.core.xml.config.XMLObjectProviderRegistry; import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; import org.opensaml.core.xml.io.MarshallerFactory; import org.opensaml.core.xml.io.MarshallingException; @@ -62,24 +54,27 @@ import org.w3c.dom.Document; import org.w3c.dom.Element; import org.springframework.security.saml2.Saml2Exception; +import org.springframework.security.saml2.core.OpenSamlInitializationService; import org.springframework.security.saml2.core.Saml2X509Credential; import org.springframework.util.Assert; import org.springframework.web.util.UriUtils; -import static java.lang.Boolean.FALSE; -import static java.lang.Boolean.TRUE; import static java.util.Arrays.asList; +import static org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport.getParserPool; import static org.springframework.util.StringUtils.hasText; /** * @since 5.2 */ final class OpenSamlImplementation { + static { + OpenSamlInitializationService.initialize(); + } + private static OpenSamlImplementation instance = new OpenSamlImplementation(); private static XMLObjectBuilderFactory xmlObjectBuilderFactory = XMLObjectProviderRegistrySupport.getBuilderFactory(); - private final BasicParserPool parserPool = new BasicParserPool(); private final EncryptedKeyResolver encryptedKeyResolver = new ChainingEncryptedKeyResolver( asList( new InlineEncryptedKeyResolver(), @@ -88,74 +83,6 @@ final class OpenSamlImplementation { ) ); - private OpenSamlImplementation() { - bootstrap(); - } - - /* - * ============================================================== - * PRIVATE METHODS - * ============================================================== - */ - private void bootstrap() { - // configure default values - // maxPoolSize = 5; - this.parserPool.setMaxPoolSize(50); - // coalescing = true; - this.parserPool.setCoalescing(true); - // expandEntityReferences = false; - this.parserPool.setExpandEntityReferences(false); - // ignoreComments = true; - this.parserPool.setIgnoreComments(true); - // ignoreElementContentWhitespace = true; - this.parserPool.setIgnoreElementContentWhitespace(true); - // namespaceAware = true; - this.parserPool.setNamespaceAware(true); - // schema = null; - this.parserPool.setSchema(null); - // dtdValidating = false; - this.parserPool.setDTDValidating(false); - // xincludeAware = false; - this.parserPool.setXincludeAware(false); - - Map builderAttributes = new HashMap<>(); - this.parserPool.setBuilderAttributes(builderAttributes); - - Map parserBuilderFeatures = new HashMap<>(); - parserBuilderFeatures.put("http://apache.org/xml/features/disallow-doctype-decl", TRUE); - parserBuilderFeatures.put(XMLConstants.FEATURE_SECURE_PROCESSING, TRUE); - parserBuilderFeatures.put("http://xml.org/sax/features/external-general-entities", FALSE); - parserBuilderFeatures.put("http://apache.org/xml/features/validation/schema/normalized-value", FALSE); - parserBuilderFeatures.put("http://xml.org/sax/features/external-parameter-entities", FALSE); - parserBuilderFeatures.put("http://apache.org/xml/features/dom/defer-node-expansion", FALSE); - this.parserPool.setBuilderFeatures(parserBuilderFeatures); - - try { - this.parserPool.initialize(); - } - catch (ComponentInitializationException x) { - throw new Saml2Exception("Unable to initialize OpenSaml v3 ParserPool", x); - } - - try { - InitializationService.initialize(); - } - catch (InitializationException e) { - throw new Saml2Exception("Unable to initialize OpenSaml v3", e); - } - - XMLObjectProviderRegistry registry; - synchronized (ConfigurationService.class) { - registry = ConfigurationService.get(XMLObjectProviderRegistry.class); - if (registry == null) { - registry = new XMLObjectProviderRegistry(); - ConfigurationService.register(XMLObjectProviderRegistry.class, registry); - } - } - - registry.setParserPool(this.parserPool); - } - /* * ============================================================== * PUBLIC METHODS @@ -259,7 +186,7 @@ final class OpenSamlImplementation { private XMLObject parse(byte[] xml) { try { - Document document = this.parserPool.parse(new ByteArrayInputStream(xml)); + Document document = getParserPool().parse(new ByteArrayInputStream(xml)); Element element = document.getDocumentElement(); return getUnmarshallerFactory().getUnmarshaller(element).unmarshall(element); } diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/core/OpenSamlInitializationServiceTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/core/OpenSamlInitializationServiceTests.java new file mode 100644 index 0000000000..93569bd6a6 --- /dev/null +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/core/OpenSamlInitializationServiceTests.java @@ -0,0 +1,47 @@ +/* + * Copyright 2002-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.saml2.core; + +import org.junit.Test; +import org.opensaml.core.config.ConfigurationService; +import org.opensaml.core.xml.config.XMLObjectProviderRegistry; + +import org.springframework.security.saml2.Saml2Exception; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +/** + * Tests for {@link OpenSamlInitializationService} + * + * @author Josh Cummings + */ +public class OpenSamlInitializationServiceTests { + + @Test + public void initializeWhenInvokedMultipleTimesThenInitializesOnce() { + OpenSamlInitializationService.initialize(); + XMLObjectProviderRegistry registry = ConfigurationService.get(XMLObjectProviderRegistry.class); + assertThat(registry.getParserPool()).isNotNull(); + registry.setParserPool(null); + OpenSamlInitializationService.initialize(); + assertThat(registry.getParserPool()).isNull(); + assertThatCode(() -> OpenSamlInitializationService.requireInitialize(r -> {})) + .isInstanceOf(Saml2Exception.class) + .hasMessageContaining("OpenSAML was already initialized previously"); + } +} diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/TestOpenSamlObjects.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/TestOpenSamlObjects.java index 878b52a134..17a66e5857 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/TestOpenSamlObjects.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/TestOpenSamlObjects.java @@ -74,9 +74,14 @@ import org.opensaml.xmlsec.signature.support.SignatureException; import org.opensaml.xmlsec.signature.support.SignatureSupport; import org.springframework.security.saml2.Saml2Exception; +import org.springframework.security.saml2.core.OpenSamlInitializationService; import org.springframework.security.saml2.core.Saml2X509Credential; final class TestOpenSamlObjects { + static { + OpenSamlInitializationService.initialize(); + } + private static OpenSamlImplementation saml = OpenSamlImplementation.getInstance(); private static String USERNAME = "test@saml.user"; diff --git a/samples/boot/saml2login/src/integration-test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlActionTestingSupport.java b/samples/boot/saml2login/src/integration-test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlActionTestingSupport.java index 09806bef97..bb94c9c78d 100644 --- a/samples/boot/saml2login/src/integration-test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlActionTestingSupport.java +++ b/samples/boot/saml2login/src/integration-test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlActionTestingSupport.java @@ -16,6 +16,13 @@ package org.springframework.security.saml2.provider.service.authentication; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.cert.X509Certificate; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.crypto.SecretKey; + import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty; import org.apache.xml.security.algorithms.JCEMapper; import org.apache.xml.security.encryption.XMLCipherParameters; @@ -54,14 +61,9 @@ import org.opensaml.security.credential.CredentialSupport; import org.opensaml.xmlsec.encryption.support.DataEncryptionParameters; import org.opensaml.xmlsec.encryption.support.EncryptionException; import org.opensaml.xmlsec.encryption.support.KeyEncryptionParameters; -import org.springframework.security.saml2.Saml2Exception; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import javax.crypto.SecretKey; -import java.security.NoSuchAlgorithmException; -import java.security.NoSuchProviderException; -import java.security.cert.X509Certificate; +import org.springframework.security.saml2.Saml2Exception; +import org.springframework.security.saml2.core.OpenSamlInitializationService; import static java.util.Arrays.asList; import static org.opensaml.security.crypto.KeySupport.generateKey; @@ -73,6 +75,10 @@ import static org.opensaml.security.crypto.KeySupport.generateKey; */ public class OpenSamlActionTestingSupport { + static { + OpenSamlInitializationService.initialize(); + } + /** ID used for all generated {@link Response} objects. */ final static String REQUEST_ID = "request";