You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-commits@axis.apache.org by ro...@apache.org on 2026/10/09 03:41:53 UTC
(axis-axis2-java-rampart) 02/02: RAMPART-459: initialise OpenSAML once instead of on every message
This is an automated email from the ASF dual-hosted git repository.
robertlazarski pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/axis-axis2-java-rampart.git
commit 6d19d990b76b19076d1e31ed680e280b4d367aeb
Author: Robert Lazarski <ro...@gmail.com>
AuthorDate: Thu Oct 8 17:38:31 2026 -1000
RAMPART-459: initialise OpenSAML once instead of on every message
InitializationService.initialize() has no idempotency guard and holds a class
monitor, so calling it per message serialised all inbound traffic. The new
OpenSAMLInitializer runs initSamlEngine() then initialize() exactly once; that
order matters because initSamlEngine() replaces the global Configuration.
Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
---
.../org/apache/rampart/RampartMessageData.java | 44 +++--------
.../rahas/impl/util/OpenSAMLInitializer.java | 73 +++++++++++++++++
.../org/apache/rahas/impl/util/SAML2Utils.java | 4 +-
.../rahas/impl/util/OpenSAMLInitializerTest.java | 91 ++++++++++++++++++++++
4 files changed, 174 insertions(+), 38 deletions(-)
diff --git a/modules/rampart-core/src/main/java/org/apache/rampart/RampartMessageData.java b/modules/rampart-core/src/main/java/org/apache/rampart/RampartMessageData.java
index 66ea13ac..97e6af78 100644
--- a/modules/rampart-core/src/main/java/org/apache/rampart/RampartMessageData.java
+++ b/modules/rampart-core/src/main/java/org/apache/rampart/RampartMessageData.java
@@ -35,6 +35,7 @@ import org.apache.neethi.PolicyEngine;
import org.apache.rahas.RahasConstants;
import org.apache.rahas.SimpleTokenStore;
import org.apache.rahas.TokenStorage;
+import org.apache.rahas.impl.util.OpenSAMLInitializer;
import org.apache.rampart.handler.RampartUsernameTokenValidator;
import org.apache.rampart.handler.WSSHandlerConstants;
import org.apache.rampart.policy.RampartPolicyBuilder;
@@ -236,44 +237,17 @@ public class RampartMessageData {
try {
- // PERFORMANCE vs CORRECTNESS:
- // The WSS4J / Santuario / OpenSAML providers below are one-time, process-wide
- // initialisers. Invoking them in this per-message constructor is intentional but
- // is a correctness-over-performance trade-off:
- // * Correctness: it guarantees the SAML stack is initialised before any
- // assertion is processed. Without it OpenSAMLUtil.unmarshallerFactory could be
- // null (an initialisation-ordering problem surfaced during the OpenSAML 5 /
- // Jakarta migration), causing SAML processing to fail intermittently.
- // * Performance: all of these calls are idempotent guards, so the steady-state
- // cost is only the guard checks rather than real re-initialisation - but they
- // still run on every message, which is wasteful under high throughput.
- // The proper fix is to run this once per application lifecycle (e.g. in the
- // Rampart/Rahas module init), which is tracked separately; do not move it without
- // re-verifying the unmarshallerFactory ordering issue does not return.
- if (log.isDebugEnabled()) {
- log.debug("WSS4J initialization starting");
- }
+ // The WSS4J / Santuario / OpenSAML initialisers below are process-wide. They
+ // stay on the message path so the SAML stack is ready before any assertion is
+ // processed (without it OpenSAMLUtil.unmarshallerFactory could be null), but
+ // every call here must be cheap once initialised: WSSConfig.init() and
+ // Init.init() guard themselves, and OpenSAML goes through OpenSAMLInitializer
+ // because InitializationService.initialize() does not (RAMPART-459). Never
+ // call InitializationService.initialize() directly from this constructor.
WSSConfig.init();
org.apache.xml.security.Init.init();
- if (log.isDebugEnabled()) {
- log.debug("Basic WSS4J initialization complete");
- }
-
- // Initialize WSS4J's OpenSAML integration specifically
try {
- if (log.isDebugEnabled()) {
- log.debug("Starting OpenSAML initialization");
- }
- org.opensaml.core.config.InitializationService.initialize();
-
- // Call WSS4J's OpenSAMLUtil initialization method
- Class<?> openSAMLUtilClass = Class.forName("org.apache.wss4j.common.saml.OpenSAMLUtil");
- java.lang.reflect.Method initMethod = openSAMLUtilClass.getDeclaredMethod("initSamlEngine");
- initMethod.setAccessible(true);
- initMethod.invoke(null);
- if (log.isDebugEnabled()) {
- log.debug("OpenSAMLUtil.initSamlEngine() called successfully");
- }
+ OpenSAMLInitializer.ensureInitialized();
} catch (Exception e) {
log.warn("WSS4J OpenSAML initialization failed: " + e.getMessage(), e);
}
diff --git a/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/OpenSAMLInitializer.java b/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/OpenSAMLInitializer.java
new file mode 100644
index 00000000..2b30ed79
--- /dev/null
+++ b/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/OpenSAMLInitializer.java
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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
+ *
+ * http://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.apache.rahas.impl.util;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.wss4j.common.saml.OpenSAMLUtil;
+import org.opensaml.core.config.InitializationException;
+import org.opensaml.core.config.InitializationService;
+
+/**
+ * One-time, process-wide initialisation of the OpenSAML stack used by WSS4J and Rahas.
+ *
+ * <p>RAMPART-459: OpenSAML's {@link InitializationService#initialize()} is
+ * {@code static synchronized} and has no idempotency guard - every call reloads every
+ * registered module initialiser. Calling it per message serialised all inbound
+ * traffic on that class monitor, SAML policy or not. Callers on the message path must
+ * use {@link #ensureInitialized()} instead of calling it directly.</p>
+ *
+ * <p>Ordering matters and must not be swapped: {@link OpenSAMLUtil#initSamlEngine()}
+ * replaces OpenSAML's global {@code Configuration} with a fresh one holding only
+ * WSS4J's provider registry, so anything initialised before it is discarded.
+ * {@code InitializationService.initialize()} therefore runs second, populating that
+ * configuration with the remaining OpenSAML modules (global security configuration,
+ * xmlsec, parser pool). This is the state the per-message calls used to converge on
+ * from the second message onwards; it is now reached on the first and left alone.</p>
+ */
+public final class OpenSAMLInitializer {
+
+ private static final Log log = LogFactory.getLog(OpenSAMLInitializer.class);
+
+ private static volatile boolean initialized;
+
+ private OpenSAMLInitializer() {
+ }
+
+ /**
+ * Initialises OpenSAML once. After the first successful call this is a single
+ * volatile read. A failed attempt is not recorded, so the next call retries.
+ */
+ public static void ensureInitialized() throws InitializationException {
+ if (initialized) {
+ return;
+ }
+ synchronized (OpenSAMLInitializer.class) {
+ if (initialized) {
+ return;
+ }
+ OpenSAMLUtil.initSamlEngine();
+ InitializationService.initialize();
+ initialized = true;
+ if (log.isDebugEnabled()) {
+ log.debug("OpenSAML initialization complete");
+ }
+ }
+ }
+}
diff --git a/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/SAML2Utils.java b/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/SAML2Utils.java
index 9015ae59..aad9736b 100644
--- a/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/SAML2Utils.java
+++ b/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/SAML2Utils.java
@@ -51,7 +51,6 @@ import org.opensaml.saml.saml2.core.Subject;
import org.opensaml.saml.saml2.core.SubjectConfirmation;
import org.opensaml.saml.saml2.core.SubjectConfirmationData;
import org.opensaml.saml.common.SAMLVersion;
-import org.opensaml.core.config.InitializationService;
import org.opensaml.core.config.InitializationException;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
@@ -109,7 +108,7 @@ public class SAML2Utils {
//build the assertion by unmarhalling the DOM element.
try {
- InitializationService.initialize();
+ OpenSAMLInitializer.ensureInitialized();
String keyInfoElementString = elem.toString();
DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
@@ -124,7 +123,6 @@ public class SAML2Utils {
.unmarshall(element);
}
catch (InitializationException e) {
-//[ERROR] /home/rlapache/axis-axis2-java-rampart/modules/rampart-trust/src/main/java/org/apache/rahas/impl/util/SAML2Utils.java:[123,60] incompatible types: java.lang.String cannot be converted to java.lang.Exception
throw new WSSecurityException(
WSSecurityException.ErrorCode.FAILURE, e, "Failure in bootstrapping");
} catch (UnmarshallingException e) {
diff --git a/modules/rampart-trust/src/test/java/org/apache/rahas/impl/util/OpenSAMLInitializerTest.java b/modules/rampart-trust/src/test/java/org/apache/rahas/impl/util/OpenSAMLInitializerTest.java
new file mode 100644
index 00000000..7ae2e12c
--- /dev/null
+++ b/modules/rampart-trust/src/test/java/org/apache/rahas/impl/util/OpenSAMLInitializerTest.java
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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
+ *
+ * http://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.apache.rahas.impl.util;
+
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CountDownLatch;
+
+import junit.framework.TestCase;
+
+import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
+import org.opensaml.xmlsec.SecurityConfigurationSupport;
+import org.opensaml.xmlsec.SignatureSigningConfiguration;
+
+/**
+ * RAMPART-459: OpenSAML must be initialised once, not on every message, and must end
+ * up in the same state the per-message calls used to converge on.
+ */
+public class OpenSAMLInitializerTest extends TestCase {
+
+ public void testInitialisesWss4jAndOpenSAMLModules() throws Exception {
+ OpenSAMLInitializer.ensureInitialized();
+
+ assertNotNull("unmarshaller factory must be available",
+ XMLObjectProviderRegistrySupport.getUnmarshallerFactory());
+ // Registered by an OpenSAML module initialiser, not by WSS4J; present only if
+ // InitializationService ran after initSamlEngine replaced the configuration.
+ assertNotNull("OpenSAML global security configuration must be registered",
+ SecurityConfigurationSupport.getGlobalSignatureSigningConfiguration());
+ }
+
+ /**
+ * Each InitializationService run registers fresh global configuration objects, so
+ * an unchanged instance across calls shows the module initialisers did not re-run.
+ */
+ public void testRepeatedCallsDoNotReinitialise() throws Exception {
+ OpenSAMLInitializer.ensureInitialized();
+ SignatureSigningConfiguration before =
+ SecurityConfigurationSupport.getGlobalSignatureSigningConfiguration();
+
+ OpenSAMLInitializer.ensureInitialized();
+ OpenSAMLInitializer.ensureInitialized();
+
+ assertSame("OpenSAML must not be re-initialised on later calls", before,
+ SecurityConfigurationSupport.getGlobalSignatureSigningConfiguration());
+ }
+
+ public void testConcurrentCallsSucceed() throws Exception {
+ final int threadCount = 16;
+ final CountDownLatch startGate = new CountDownLatch(1);
+ final CountDownLatch doneGate = new CountDownLatch(threadCount);
+ final List<Throwable> failures = new CopyOnWriteArrayList<Throwable>();
+
+ for (int i = 0; i < threadCount; i++) {
+ new Thread(new Runnable() {
+ public void run() {
+ try {
+ startGate.await();
+ OpenSAMLInitializer.ensureInitialized();
+ } catch (Throwable t) {
+ failures.add(t);
+ } finally {
+ doneGate.countDown();
+ }
+ }
+ }).start();
+ }
+
+ startGate.countDown();
+ doneGate.await();
+
+ assertTrue("concurrent initialisation must not fail: " + failures, failures.isEmpty());
+ assertNotNull(XMLObjectProviderRegistrySupport.getUnmarshallerFactory());
+ }
+}