This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch 2_4_x-fixes
in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
The following commit(s) were added to refs/heads/2_4_x-fixes by this push:
new 58cb2e501 Bound the encrypted data nesting depth (#713)
58cb2e501 is described below
commit 58cb2e501077d4b015dad742413e3e51c5877eb3
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Mon Sep 21 07:29:30 2026 +0100
Bound the encrypted data nesting depth (#713)
---
.../org/apache/wss4j/dom/handler/RequestData.java | 41 +++++++
.../dom/processor/EncryptedAssertionProcessor.java | 24 +++-
.../dom/processor/EncryptedDataProcessor.java | 16 ++-
.../wss4j/dom/handler/RequestDataNestingTest.java | 134 +++++++++++++++++++++
4 files changed, 208 insertions(+), 7 deletions(-)
diff --git
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/RequestData.java
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/RequestData.java
index 9f9528fe4..6550ab2bd 100644
---
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/RequestData.java
+++
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/RequestData.java
@@ -84,6 +84,7 @@ public class RequestData {
private AlgorithmSuite samlAlgorithmSuite;
private boolean disableBSPEnforcement;
private boolean allowRSA15KeyTransportAlgorithm;
+ private int processorNestingDepth;
private boolean addUsernameTokenNonce;
private boolean addUsernameTokenCreated;
private Certificate[] tlsCerts;
@@ -545,6 +546,46 @@ public class RequestData {
this.disableBSPEnforcement = disableBSPEnforcement;
}
+ /**
+ * The maximum depth to which a processor may hand a token it has just
uncovered to another
+ * processor. Decrypting an EncryptedData whose plaintext is itself a
security structure does
+ * exactly that, and does it recursively, so a sender who nests those
structures is choosing
+ * how much of the receiver's stack to consume. Each level costs very
little to write - every
+ * one of them may point its KeyInfo at a single EncryptedKey, which is
decrypted once and
+ * cached - so the chain has to be bounded.
+ * <p/>
+ * Real messages stay well inside this. An EncryptedAssertion reaches
depth 2: the assertion's
+ * EncryptedData, then the decrypted Assertion handed to the SAML
processor, which dispatches
+ * no further. Re-encrypting an already encrypted assertion reaches 3.
Sibling tokens in the
+ * same header do not accumulate - each is entered and left in turn - so
the bound constrains
+ * nesting only.
+ */
+ public static final int MAXIMUM_PROCESSOR_NESTING_DEPTH = 5;
+
+ /**
+ * Record that processing is about to descend into a token uncovered by
another token, and
+ * refuse to go deeper than {@link #MAXIMUM_PROCESSOR_NESTING_DEPTH}.
Every caller must pair
+ * this with {@link #exitNestedToken()} in a finally block.
+ *
+ * @throws WSSecurityException if the message nests tokens too deeply
+ */
+ public void enterNestedToken() throws WSSecurityException {
+ if (processorNestingDepth >= MAXIMUM_PROCESSOR_NESTING_DEPTH) {
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.INVALID_SECURITY, "empty",
+ new Object[] {"Tokens are nested more than "
+ + MAXIMUM_PROCESSOR_NESTING_DEPTH + " deep"});
+ }
+ processorNestingDepth++;
+ }
+
+ /**
+ * Record that processing has come back out of a nested token.
+ */
+ public void exitNestedToken() {
+ processorNestingDepth--;
+ }
+
public boolean isAllowRSA15KeyTransportAlgorithm() {
return allowRSA15KeyTransportAlgorithm;
}
diff --git
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedAssertionProcessor.java
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedAssertionProcessor.java
index 2faad4365..48d2b9d85 100644
---
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedAssertionProcessor.java
+++
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedAssertionProcessor.java
@@ -70,7 +70,12 @@ public class EncryptedAssertionProcessor implements
Processor {
((Element)currentChild).getLocalName());
Processor proc = request.getWssConfig().getProcessor(el);
if (proc != null) {
-
completeResults.addAll(proc.handleToken((Element)currentChild, request));
+ request.enterNestedToken();
+ try {
+
completeResults.addAll(proc.handleToken((Element)currentChild, request));
+ } finally {
+ request.exitNestedToken();
+ }
}
}
}
@@ -92,8 +97,14 @@ public class EncryptedAssertionProcessor implements
Processor {
Processor proc =
request.getWssConfig().getProcessor(el);
if (proc != null) {
LOG.debug("Processing decrypted element with:
{}", proc.getClass().getName());
- List<WSSecurityEngineResult> results =
proc.handleToken(decryptedElem, request);
- completeResults.addAll(0, results);
+ request.enterNestedToken();
+ try {
+ List<WSSecurityEngineResult> results =
+ proc.handleToken(decryptedElem,
request);
+ completeResults.addAll(0, results);
+ } finally {
+ request.exitNestedToken();
+ }
return completeResults;
}
}
@@ -119,7 +130,12 @@ public class EncryptedAssertionProcessor implements
Processor {
Processor proc = request.getWssConfig().getProcessor(el);
if (proc != null) {
LOG.debug("Processing decrypted element with: {}",
proc.getClass().getName());
- return proc.handleToken(encryptedDataElement, request);
+ request.enterNestedToken();
+ try {
+ return proc.handleToken(encryptedDataElement, request);
+ } finally {
+ request.exitNestedToken();
+ }
}
return Collections.emptyList();
diff --git
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedDataProcessor.java
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedDataProcessor.java
index 44763811e..5d9735c5c 100644
---
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedDataProcessor.java
+++
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedDataProcessor.java
@@ -120,7 +120,12 @@ public class EncryptedDataProcessor implements Processor {
} else if (encryptedKeyElement != null && data.getWssConfig() != null)
{
WSSConfig wssConfig = data.getWssConfig();
Processor encrKeyProc =
wssConfig.getProcessor(WSConstants.ENCRYPTED_KEY);
- encrKeyResults = encrKeyProc.handleToken(encryptedKeyElement,
data);
+ data.enterNestedToken();
+ try {
+ encrKeyResults = encrKeyProc.handleToken(encryptedKeyElement,
data);
+ } finally {
+ data.exitNestedToken();
+ }
byte[] symmKey =
(byte[])encrKeyResults.get(0).get(WSSecurityEngineResult.TAG_SECRET);
key = KeyUtils.prepareSecretKey(symEncAlgo, symmKey);
@@ -191,8 +196,13 @@ public class EncryptedDataProcessor implements Processor {
Processor proc = data.getWssConfig().getProcessor(el);
if (proc != null) {
LOG.debug("Processing decrypted element with: {}",
proc.getClass().getName());
- List<WSSecurityEngineResult> results =
proc.handleToken(decryptedElem, data);
- completeResults.addAll(0, results);
+ data.enterNestedToken();
+ try {
+ List<WSSecurityEngineResult> results =
proc.handleToken(decryptedElem, data);
+ completeResults.addAll(0, results);
+ } finally {
+ data.exitNestedToken();
+ }
return completeResults;
}
}
diff --git
a/ws-security-dom/src/test/java/org/apache/wss4j/dom/handler/RequestDataNestingTest.java
b/ws-security-dom/src/test/java/org/apache/wss4j/dom/handler/RequestDataNestingTest.java
new file mode 100644
index 000000000..3c9349c16
--- /dev/null
+++
b/ws-security-dom/src/test/java/org/apache/wss4j/dom/handler/RequestDataNestingTest.java
@@ -0,0 +1,134 @@
+/**
+ * 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.wss4j.dom.handler;
+
+import org.apache.wss4j.common.ext.WSSecurityException;
+import org.apache.wss4j.dom.engine.WSSConfig;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+/**
+ * A processor that uncovers a token inside the one it is processing hands it
to another processor,
+ * which may do the same again, so how deep a message may nest tokens is how
much of the receiver's
+ * stack the sender gets to spend. RequestData carries that bound for the
message.
+ */
+public class RequestDataNestingTest {
+
+ private static final int LIMIT =
RequestData.MAXIMUM_PROCESSOR_NESTING_DEPTH;
+
+ @BeforeAll
+ public static void setUp() {
+ // Without this the message bundle is not loaded and every
getMessage() returns the
+ // library's "you must initialize" text instead of the message that
was raised.
+ WSSConfig.init();
+ }
+
+ /**
+ * The bound has to leave room for the deepest nesting a real message
performs. An
+ * EncryptedAssertion reaches 2, and re-encrypting an already encrypted
assertion reaches 3.
+ */
+ @Test
+ public void testLimitAccommodatesRealMessages() {
+ assertEquals(true, LIMIT >= 3,
+ "the bound must not reject an encrypted, encrypted assertion: " +
LIMIT);
+ }
+
+ @Test
+ public void testNestingUpToTheLimitIsAllowed() {
+ RequestData data = new RequestData();
+
+ assertDoesNotThrow(() -> {
+ for (int i = 0; i < LIMIT; i++) {
+ data.enterNestedToken();
+ }
+ });
+ }
+
+ @Test
+ public void testNestingBeyondTheLimitIsRejected() throws Exception {
+ RequestData data = new RequestData();
+ for (int i = 0; i < LIMIT; i++) {
+ data.enterNestedToken();
+ }
+
+ WSSecurityException exception =
+ assertThrows(WSSecurityException.class, data::enterNestedToken);
+ assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY,
exception.getErrorCode());
+ }
+
+ /**
+ * Unwinding restores the budget. Were a caller to leave the depth raised
- by not pairing
+ * enterNestedToken with a finally block - a message would grow harder to
process the further
+ * through it the engine got.
+ */
+ @Test
+ public void testLeavingATokenRestoresTheBudget() throws Exception {
+ RequestData data = new RequestData();
+ for (int i = 0; i < LIMIT; i++) {
+ data.enterNestedToken();
+ }
+ for (int i = 0; i < LIMIT; i++) {
+ data.exitNestedToken();
+ }
+
+ assertDoesNotThrow(() -> {
+ for (int i = 0; i < LIMIT; i++) {
+ data.enterNestedToken();
+ }
+ });
+ }
+
+ /**
+ * Tokens that sit side by side are entered and left in turn rather than
nested, so any number
+ * of them may appear in one message. The bound constrains depth, not
count.
+ */
+ @Test
+ public void testSiblingTokensDoNotAccumulate() {
+ RequestData data = new RequestData();
+
+ assertDoesNotThrow(() -> {
+ for (int sibling = 0; sibling < LIMIT * 10; sibling++) {
+ data.enterNestedToken();
+ data.exitNestedToken();
+ }
+ });
+ }
+
+ /**
+ * The exception has to name the limit: "nested too deeply" without a
number tells an operator
+ * facing a rejected message nothing about whether it is their message or
their configuration.
+ */
+ @Test
+ public void testTheRejectionNamesTheLimit() throws Exception {
+ RequestData data = new RequestData();
+ for (int i = 0; i < LIMIT; i++) {
+ data.enterNestedToken();
+ }
+
+ WSSecurityException exception =
+ assertThrows(WSSecurityException.class, data::enterNestedToken);
+ assertEquals(true,
exception.getMessage().contains(String.valueOf(LIMIT)),
+ "the rejection should say what the limit is: " +
exception.getMessage());
+ }
+}
