[Backport 2.x] Backport OpenSAML 4.3.0 to 2.x

build.gradle

@@ -27,7 +27,7 @@ buildscript {
         common_utils_version = System.getProperty("common_utils.version", '2.9.0.0-SNAPSHOT')
         kafka_version  = '3.5.1'
         apache_cxf_version = '4.0.3'
-        open_saml_version = '3.4.5'
+        open_saml_version = '4.3.0'
         one_login_java_saml = '2.9.0'
         jjwt_version = '0.12.3'
         guava_version = '32.1.3-jre'
@@ -609,7 +609,7 @@ dependencies {
     testImplementation 'org.apache.camel:camel-xmlsecurity:3.21.2'
 
     //OpenSAML
-    implementation 'net.shibboleth.utilities:java-support:7.5.1'
+    implementation 'net.shibboleth.utilities:java-support:8.4.0'
     implementation "com.onelogin:java-saml:${one_login_java_saml}"
     implementation "com.onelogin:java-saml-core:${one_login_java_saml}"
     implementation "org.opensaml:opensaml-core:${open_saml_version}"

plugin-security.policy

@@ -34,6 +34,8 @@ grant {
   permission javax.security.auth.AuthPermission "modifyPrivateCredentials";
   permission javax.security.auth.AuthPermission "doAs";
   permission javax.security.auth.kerberos.ServicePermission "*","accept";
+
+  //SAML and internal plugin policy
   permission java.util.PropertyPermission "*","read,write";
 
   //Enable when we switch to UnboundID LDAP SDK
@@ -59,7 +61,6 @@ grant {
   permission java.security.SecurityPermission "putProviderProperty.BC";
   permission java.security.SecurityPermission "insertProvider.BC";
   permission java.security.SecurityPermission "removeProviderProperty.BC";
-  permission java.util.PropertyPermission "jdk.tls.rejectClientInitiatedRenegotiation", "write";
   permission java.security.SecurityPermission "getProperty.org.bouncycastle.rsa.max_size";
   permission java.security.SecurityPermission "getProperty.org.bouncycastle.rsa.max_mr_tests";
 
@@ -74,6 +75,7 @@ grant {
 
   //Enable this permission to debug unauthorized de-serialization attempt
   //permission java.io.SerializablePermission "enableSubstitution";
+
 };
 
 grant codeBase "${codebase.netty-common}" {

src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java

@@ -20,6 +20,7 @@
 import java.security.PrivilegedExceptionAction;
 import java.util.Map;
 import java.util.Optional;
+import java.util.ServiceLoader;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import javax.xml.parsers.ParserConfigurationException;
@@ -42,6 +43,7 @@
 import org.opensearch.security.filter.SecurityRequest;
 import org.opensearch.security.filter.SecurityRequestChannelUnsupported;
 import org.opensearch.security.filter.SecurityResponse;
+import org.opensearch.security.opensaml.integration.SecurityXMLObjectProviderInitializer;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.support.PemKeyReader;
 import org.opensearch.security.user.AuthCredentials;
@@ -61,9 +63,11 @@
 import net.shibboleth.utilities.java.support.xml.BasicParserPool;
 import org.opensaml.core.config.InitializationException;
 import org.opensaml.core.config.InitializationService;
+import org.opensaml.core.config.Initializer;
 import org.opensaml.saml.metadata.resolver.MetadataResolver;
 import org.opensaml.saml.metadata.resolver.impl.AbstractMetadataResolver;
 import org.opensaml.saml.metadata.resolver.impl.DOMMetadataResolver;
+import org.opensaml.xmlsec.config.impl.XMLObjectProviderInitializer;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.xml.sax.SAXException;
@@ -111,12 +115,12 @@ public HTTPSamlAuthenticator(final Settings settings, final Path configPath) {
             spSignaturePrivateKey = getSpSignaturePrivateKey(settings, configPath);
             useForceAuthn = settings.getAsBoolean("sp.forceAuthn", null);
 
-            if (rolesKey == null || rolesKey.length() == 0) {
+            if (rolesKey == null || rolesKey.isEmpty()) {
                 log.warn("roles_key is not configured, will only extract subject from SAML");
                 rolesKey = null;
             }
 
-            if (subjectKey == null || subjectKey.length() == 0) {
+            if (subjectKey == null || subjectKey.isEmpty()) {
                 // If subjectKey == null, get subject from the NameID element.
                 // Thus, this is a valid configuration.
                 subjectKey = null;
@@ -287,35 +291,40 @@ static void ensureOpenSamlInitialization() {
         }
 
         try {
-            AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
-                @Override
-                public Void run() throws InitializationException {
-
-                    Thread thread = Thread.currentThread();
-                    ClassLoader originalClassLoader = thread.getContextClassLoader();
-
-                    try {
-
-                        thread.setContextClassLoader(InitializationService.class.getClassLoader());
-
-                        InitializationService.initialize();
-
-                        new org.opensaml.saml.config.impl.XMLObjectProviderInitializer().init();
-                        new org.opensaml.saml.config.impl.SAMLConfigurationInitializer().init();
-                        new org.opensaml.xmlsec.config.impl.XMLObjectProviderInitializer().init();
-                    } finally {
-                        thread.setContextClassLoader(originalClassLoader);
-                    }
-
-                    openSamlInitialized = true;
-                    return null;
+            AccessController.doPrivileged((PrivilegedExceptionAction<Void>) () -> {
+                Thread thread = Thread.currentThread();
+                ClassLoader originalClassLoader = thread.getContextClassLoader();
+                try {
+                    thread.setContextClassLoader(InitializationService.class.getClassLoader());
+                    initializeOpenSAMLConfiguration();
+                } catch (InitializationException e) {
+                    throw new RuntimeException(e.getCause());
+                } finally {
+                    thread.setContextClassLoader(originalClassLoader);
                 }
+
+                openSamlInitialized = true;
+                return null;
             });
         } catch (PrivilegedActionException e) {
             throw new RuntimeException(e.getCause());
         }
     }
 
+    private static void initializeOpenSAMLConfiguration() throws InitializationException {
+        log.info("Initializing OpenSAML using the Java Services API");
+
+        final ServiceLoader<Initializer> serviceLoader = ServiceLoader.load(Initializer.class);
+        for (Initializer initializer : serviceLoader) {
+            if (initializer instanceof XMLObjectProviderInitializer) {
+                // replace initialization of X509 builders which support Cleaner with our own solution
+                new SecurityXMLObjectProviderInitializer().init();
+            } else {
+                initializer.init();
+            }
+        }
+    }
+
     @SuppressWarnings("removal")
     private MetadataResolver createMetadataResolver(final Settings settings, final Path configPath) throws Exception {
         final AbstractMetadataResolver metadataResolver;
@@ -349,12 +358,9 @@ private MetadataResolver createMetadataResolver(final Settings settings, final P
         }
 
         try {
-            AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
-                @Override
-                public Void run() throws ComponentInitializationException {
-                    metadataResolver.initialize();
-                    return null;
-                }
+            AccessController.doPrivileged((PrivilegedExceptionAction<Void>) () -> {
+                metadataResolver.initialize();
+                return null;
             });
         } catch (PrivilegedActionException e) {
             if (e.getCause() instanceof ComponentInitializationException) {

src/main/java/com/amazon/dlic/auth/http/saml/Saml2SettingsProvider.java

@@ -14,6 +14,7 @@
 import java.security.AccessController;
 import java.security.PrivateKey;
 import java.security.PrivilegedAction;
+import java.time.Instant;
 import java.util.AbstractMap;
 import java.util.Collection;
 import java.util.HashMap;
@@ -33,7 +34,6 @@
 import com.onelogin.saml2.settings.SettingsBuilder;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 import net.shibboleth.utilities.java.support.resolver.ResolverException;
-import org.joda.time.DateTime;
 import org.opensaml.core.criterion.EntityIdCriterion;
 import org.opensaml.saml.metadata.resolver.MetadataResolver;
 import org.opensaml.saml.metadata.resolver.RefreshableMetadataResolver;
@@ -54,7 +54,7 @@ public class Saml2SettingsProvider {
     private final String idpEntityId;
     private final PrivateKey spSignaturePrivateKey;
     private Saml2Settings cachedSaml2Settings;
-    private DateTime metadataUpdateTime;
+    private Instant metadataUpdateTime;
 
     Saml2SettingsProvider(Settings opensearchSettings, MetadataResolver metadataResolver, PrivateKey spSignaturePrivateKey) {
         this.opensearchSettings = opensearchSettings;
@@ -107,7 +107,7 @@ Saml2Settings get() throws SamlConfigException {
     }
 
     Saml2Settings getCached() throws SamlConfigException {
-        DateTime tempLastUpdate = null;
+        Instant tempLastUpdate = null;
 
         if (this.metadataResolver instanceof RefreshableMetadataResolver && this.isUpdateRequired()) {
             this.cachedSaml2Settings = null;

src/main/java/com/amazon/dlic/auth/http/saml/SamlHTTPMetadataResolver.java

@@ -15,6 +15,7 @@
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
+import java.time.Duration;
 
 import org.apache.http.client.HttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
@@ -31,21 +32,16 @@ public class SamlHTTPMetadataResolver extends HTTPMetadataResolver {
 
     SamlHTTPMetadataResolver(String idpMetadataUrl, Settings opensearchSettings, Path configPath) throws Exception {
         super(createHttpClient(opensearchSettings, configPath), idpMetadataUrl);
-        setMinRefreshDelay(opensearchSettings.getAsLong("idp.min_refresh_delay", 60L * 1000L));
-        setMaxRefreshDelay(opensearchSettings.getAsLong("idp.max_refresh_delay", 14400000L));
+        setMinRefreshDelay(Duration.ofMillis(opensearchSettings.getAsLong("idp.min_refresh_delay", 60L * 1000L)));
+        setMaxRefreshDelay(Duration.ofMillis(opensearchSettings.getAsLong("idp.max_refresh_delay", 14400000L)));
         setRefreshDelayFactor(opensearchSettings.getAsFloat("idp.refresh_delay_factor", 0.75f));
     }
 
     @Override
     @SuppressWarnings("removal")
     protected byte[] fetchMetadata() throws ResolverException {
         try {
-            return AccessController.doPrivileged(new PrivilegedExceptionAction<byte[]>() {
-                @Override
-                public byte[] run() throws ResolverException {
-                    return SamlHTTPMetadataResolver.super.fetchMetadata();
-                }
-            });
+            return AccessController.doPrivileged((PrivilegedExceptionAction<byte[]>) () -> SamlHTTPMetadataResolver.super.fetchMetadata());
         } catch (PrivilegedActionException e) {
 
             if (e.getCause() instanceof ResolverException) {

src/main/java/org/opensearch/security/opensaml/integration/CleanerFactory.java

@@ -0,0 +1,43 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.security.opensaml.integration;
+
+import java.lang.ref.Cleaner;
+import java.util.concurrent.ThreadFactory;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import org.opensearch.common.util.concurrent.OpenSearchExecutors;
+
+/**
+ * The class was adapted from {@link net.shibboleth.utilities.java.support.primitive.CleanerSupport}.
+ * The main reason is that it is only one way to set Cleaner.create()
+ * together with cleaners daemon thread factory which is required for OpenSearch
+ */
+public class CleanerFactory {
+
+    protected final static Logger log = LogManager.getLogger(SecurityXMLObjectProviderInitializer.class);
+
+    private static final ThreadFactory cleanersThreadFactory = OpenSearchExecutors.daemonThreadFactory("cleaners");
+
+    /** Constructor. */
+    private CleanerFactory() {}
+
+    public static Cleaner create(final Class<?> requester) {
+        // Current approach here is to create a new Cleaner on each call. A given class requester/owner
+        // is assumed to call only once and store in static storage.
+        log.debug("Creating new java.lang.ref.Cleaner instance requested by class:  {}", requester.getName());
+        return Cleaner.create(cleanersThreadFactory);
+    }
+
+}

src/main/java/org/opensearch/security/opensaml/integration/SecurityX509CRLBuilder.java

@@ -0,0 +1,23 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.security.opensaml.integration;
+
+import org.opensaml.xmlsec.signature.X509CRL;
+import org.opensaml.xmlsec.signature.impl.X509CRLBuilder;
+
+public class SecurityX509CRLBuilder extends X509CRLBuilder {
+
+    public X509CRL buildObject(final String namespaceURI, final String localName, final String namespacePrefix) {
+        return new SecurityX509CRLImpl(namespaceURI, localName, namespacePrefix);
+    }
+
+}

src/main/java/org/opensearch/security/opensaml/integration/SecurityX509CRLImpl.java

@@ -0,0 +1,86 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.security.opensaml.integration;
+
+import java.lang.ref.Cleaner;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import javax.annotation.Nonnull;
+
+import net.shibboleth.utilities.java.support.collection.IndexingObjectStore;
+import org.opensaml.core.xml.AbstractXMLObject;
+import org.opensaml.core.xml.XMLObject;
+import org.opensaml.xmlsec.signature.X509CRL;
+
+/**
+ * The class was adapted from {@link org.opensaml.xmlsec.signature.impl.X509CRLImpl}.
+ * The main reason is that it is only one way to set up {@link CleanerFactory}
+ * together with cleaners daemon thread factory which is required for OpenSearch
+ */
+public class SecurityX509CRLImpl extends AbstractXMLObject implements X509CRL {
+
+    private static final IndexingObjectStore<String> B64_CRL_STORE = new IndexingObjectStore<>();
+
+    private static final Cleaner CLEANER = CleanerFactory.create(SecurityX509CRLImpl.class);
+
+    private Cleaner.Cleanable cleanable;
+
+    private String b64CRLIndex;
+
+    protected SecurityX509CRLImpl(final String namespaceURI, final String elementLocalName, final String namespacePrefix) {
+        super(namespaceURI, elementLocalName, namespacePrefix);
+    }
+
+    public String getValue() {
+        return B64_CRL_STORE.get(b64CRLIndex);
+    }
+
+    public void setValue(final String newValue) {
+        // Dump our cached DOM if the new value really is new
+        final String currentCRL = B64_CRL_STORE.get(b64CRLIndex);
+        final String newCRL = prepareForAssignment(currentCRL, newValue);
+
+        // This is a new value, remove the old one, add the new one
+        if (!Objects.equals(currentCRL, newCRL)) {
+            if (cleanable != null) {
+                cleanable.clean();
+                cleanable = null;
+            }
+            b64CRLIndex = B64_CRL_STORE.put(newCRL);
+            if (b64CRLIndex != null) {
+                cleanable = CLEANER.register(this, new SecurityX509CRLImpl.CleanerState(b64CRLIndex));
+            }
+        }
+    }
+
+    @Override
+    public List<XMLObject> getOrderedChildren() {
+        return Collections.emptyList();
+    }
+
+    static class CleanerState implements Runnable {
+
+        /** The index to remove from the store. */
+        private String index;
+
+        public CleanerState(@Nonnull final String idx) {
+            index = idx;
+        }
+
+        /** {@inheritDoc} */
+        public void run() {
+            SecurityX509CRLImpl.B64_CRL_STORE.remove(index);
+        }
+
+    }
+}