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 cd7de419d Reject mismatched symmetric key lengths to prevent
cross-algorithm key reuse (#677)
cd7de419d is described below
commit cd7de419dcca71276edc6e80f81932690348412b
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Tue Sep 8 12:10:17 2026 +0100
Reject mismatched symmetric key lengths to prevent cross-algorithm key
reuse (#677)
---
.../integration/test/kerberos/KerberosTest.java | 3 +-
.../org/apache/wss4j/common/util/KeyUtils.java | 41 ++++---
.../org/apache/wss4j/common/util/KeyUtilsTest.java | 126 +++++++++++++++++++++
.../wss4j/dom/message/WSSecDerivedKeyBase.java | 2 +-
.../wss4j/dom/processor/EncryptedKeyProcessor.java | 13 +--
.../wss4j/stax/test/SignatureEncryptionTest.java | 3 +
6 files changed, 162 insertions(+), 26 deletions(-)
diff --git
a/integration/src/test/java/org/apache/wss4j/integration/test/kerberos/KerberosTest.java
b/integration/src/test/java/org/apache/wss4j/integration/test/kerberos/KerberosTest.java
index 228db1f52..13d98a83d 100644
---
a/integration/src/test/java/org/apache/wss4j/integration/test/kerberos/KerberosTest.java
+++
b/integration/src/test/java/org/apache/wss4j/integration/test/kerberos/KerberosTest.java
@@ -1122,7 +1122,8 @@ public class KerberosTest {
bst.setID("Id-" + bst.hashCode());
WSSecEncrypt builder = new WSSecEncrypt(secHeader);
- builder.setSymmetricEncAlgorithm(WSConstants.AES_256);
+ // Must match the length of the Kerberos session key issued by the
KDC (aes128)
+ builder.setSymmetricEncAlgorithm(WSConstants.AES_128);
SecretKey secretKey = bst.getSecretKey();
builder.setEncryptSymmKey(false);
builder.setCustomReferenceValue(WSConstants.WSS_GSS_KRB_V5_AP_REQ);
diff --git
a/ws-security-common/src/main/java/org/apache/wss4j/common/util/KeyUtils.java
b/ws-security-common/src/main/java/org/apache/wss4j/common/util/KeyUtils.java
index 59374ace2..dd2eba7bd 100644
---
a/ws-security-common/src/main/java/org/apache/wss4j/common/util/KeyUtils.java
+++
b/ws-security-common/src/main/java/org/apache/wss4j/common/util/KeyUtils.java
@@ -86,8 +86,22 @@ public final class KeyUtils {
/**
* Convert the raw key bytes into a SecretKey object of type algorithm.
+ *
+ * @throws WSSecurityException if the raw key length does not match the
key length required by
+ * the algorithm or exceeds maximum allowed size
*/
- public static SecretKey prepareSecretKey(String algorithm, byte[] rawKey) {
+ public static SecretKey prepareSecretKey(String algorithm, byte[] rawKey)
throws WSSecurityException {
+ if (rawKey == null) {
+ throw new
WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
+ }
+
+ if (rawKey.length > MAX_SYMMETRIC_KEY_SIZE) {
+ // Prevent a possible attack where a huge secret key is specified
+ LOG.warn("The provided key has a length of {} bytes, which exceeds
the maximum allowed length of {} bytes",
+ rawKey.length, MAX_SYMMETRIC_KEY_SIZE);
+ throw new
WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
+ }
+
// Do an additional check on the keysize required by the encryption
algorithm
int size = 0;
try {
@@ -97,22 +111,17 @@ public final class KeyUtils {
LOG.debug(e.getMessage());
}
String keyAlgorithm = JCEMapper.getJCEKeyAlgorithmFromURI(algorithm);
- SecretKeySpec keySpec;
- if (size > 0 && !algorithm.endsWith("gcm") &&
!algorithm.contains("hmac-")) {
- keySpec =
- new SecretKeySpec(
- rawKey, 0, rawKey.length > size ? size : rawKey.length,
keyAlgorithm
- );
- } else if (rawKey.length > MAX_SYMMETRIC_KEY_SIZE) {
- // Prevent a possible attack where a huge secret key is specified
- keySpec =
- new SecretKeySpec(
- rawKey, 0, MAX_SYMMETRIC_KEY_SIZE, keyAlgorithm
- );
- } else {
- keySpec = new SecretKeySpec(rawKey, keyAlgorithm);
+
+ // For fixed-length symmetric ciphers (e.g. AES-CBC, AES-GCM, 3DES,
AES KeyWrap),
+ // strictly verify that the provided key length matches the declared
algorithm's key length.
+ // Refuse to truncate or mismatch key material to prevent
cross-algorithm key-reuse attacks.
+ if (size > 0 && (algorithm == null || !algorithm.contains("hmac-")) &&
rawKey.length != size) {
+ LOG.warn("The provided key has a length of {} bytes, which does
not match the length of"
+ + " {} bytes required by {}", rawKey.length, size, algorithm);
+ throw new
WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
}
- return keySpec;
+
+ return new SecretKeySpec(rawKey, keyAlgorithm);
}
public static KeyGenerator getKeyGenerator(String algorithm) throws
WSSecurityException {
diff --git
a/ws-security-common/src/test/java/org/apache/wss4j/common/util/KeyUtilsTest.java
b/ws-security-common/src/test/java/org/apache/wss4j/common/util/KeyUtilsTest.java
new file mode 100644
index 000000000..b938f2c7a
--- /dev/null
+++
b/ws-security-common/src/test/java/org/apache/wss4j/common/util/KeyUtilsTest.java
@@ -0,0 +1,126 @@
+/**
+ * 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.common.util;
+
+import javax.crypto.SecretKey;
+
+import org.apache.wss4j.common.WSS4JConstants;
+import org.apache.wss4j.common.crypto.WSProviderConfig;
+import org.apache.wss4j.common.ext.WSSecurityException;
+import org.apache.xml.security.signature.XMLSignature;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+class KeyUtilsTest {
+
+ @BeforeAll
+ static void setUp() {
+ WSProviderConfig.setXmlSecIgnoreLineBreak();
+ }
+
+ @Test
+ void rejectsOversizedKeyForFixedLengthEncryptionAlgorithm() {
+ byte[] rawKey = new byte[32];
+
+ WSSecurityException exception =
Assertions.assertThrows(WSSecurityException.class,
+ () -> KeyUtils.prepareSecretKey(WSS4JConstants.AES_128, rawKey));
+
+
Assertions.assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY,
exception.getErrorCode());
+ }
+
+ @Test
+ void rejectsUndersizedKeyForFixedLengthEncryptionAlgorithm() {
+ byte[] rawKey = new byte[8];
+
+ WSSecurityException exception =
Assertions.assertThrows(WSSecurityException.class,
+ () -> KeyUtils.prepareSecretKey(WSS4JConstants.AES_128, rawKey));
+
+
Assertions.assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY,
exception.getErrorCode());
+ }
+
+ @Test
+ void acceptsExactLengthKeyForFixedLengthEncryptionAlgorithm() throws
Exception {
+ byte[] rawKey = new byte[16];
+
+ SecretKey secretKey =
KeyUtils.prepareSecretKey(WSS4JConstants.AES_128, rawKey);
+
+ Assertions.assertArrayEquals(rawKey, secretKey.getEncoded());
+ }
+
+ @Test
+ void rejectsOversizedKeyForGCMAlgorithm() {
+ byte[] rawKey = new byte[32];
+
+ WSSecurityException exception =
Assertions.assertThrows(WSSecurityException.class,
+ () -> KeyUtils.prepareSecretKey(WSS4JConstants.AES_128_GCM,
rawKey));
+
+
Assertions.assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY,
exception.getErrorCode());
+ }
+
+ @Test
+ void rejectsUndersizedKeyForGCMAlgorithm() {
+ byte[] rawKey = new byte[8];
+
+ WSSecurityException exception =
Assertions.assertThrows(WSSecurityException.class,
+ () -> KeyUtils.prepareSecretKey(WSS4JConstants.AES_128_GCM,
rawKey));
+
+
Assertions.assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY,
exception.getErrorCode());
+ }
+
+ @Test
+ void acceptsExactLengthKeyForGCMAlgorithm() throws Exception {
+ byte[] rawKey = new byte[16];
+
+ SecretKey secretKey =
KeyUtils.prepareSecretKey(WSS4JConstants.AES_128_GCM, rawKey);
+
+ Assertions.assertArrayEquals(rawKey, secretKey.getEncoded());
+ }
+
+ @Test
+ void allowsVariableLengthKeyForHMAC() throws Exception {
+ byte[] rawKey64 = new byte[64];
+ byte[] rawKey20 = new byte[20];
+
+ SecretKey secretKey64 =
KeyUtils.prepareSecretKey(XMLSignature.ALGO_ID_MAC_HMAC_SHA256, rawKey64);
+ SecretKey secretKey20 =
KeyUtils.prepareSecretKey(XMLSignature.ALGO_ID_MAC_HMAC_SHA256, rawKey20);
+
+ Assertions.assertArrayEquals(rawKey64, secretKey64.getEncoded());
+ Assertions.assertArrayEquals(rawKey20, secretKey20.getEncoded());
+ }
+
+ @Test
+ void rejectsOversizedKeyForHMAC() {
+ byte[] rawKey = new byte[1025];
+
+ WSSecurityException exception =
Assertions.assertThrows(WSSecurityException.class,
+ () ->
KeyUtils.prepareSecretKey(XMLSignature.ALGO_ID_MAC_HMAC_SHA256, rawKey));
+
+
Assertions.assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY,
exception.getErrorCode());
+ }
+
+ @Test
+ void rejectsNullRawKey() {
+ WSSecurityException exception =
Assertions.assertThrows(WSSecurityException.class,
+ () -> KeyUtils.prepareSecretKey(WSS4JConstants.AES_128, null));
+
+
Assertions.assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY,
exception.getErrorCode());
+ }
+}
\ No newline at end of file
diff --git
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/message/WSSecDerivedKeyBase.java
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/message/WSSecDerivedKeyBase.java
index c59a3e53f..10334dc20 100644
---
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/message/WSSecDerivedKeyBase.java
+++
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/message/WSSecDerivedKeyBase.java
@@ -371,7 +371,7 @@ public abstract class WSSecDerivedKeyBase extends
WSSecSignatureBase {
this.crypto = crypto;
}
- protected SecretKey getDerivedKey(String algorithm) {
+ protected SecretKey getDerivedKey(String algorithm) throws
WSSecurityException {
return KeyUtils.prepareSecretKey(algorithm, derivedKeyBytes);
}
diff --git
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedKeyProcessor.java
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedKeyProcessor.java
index 046a18543..bd5ecc302 100644
---
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedKeyProcessor.java
+++
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/EncryptedKeyProcessor.java
@@ -522,20 +522,17 @@ public class EncryptedKeyProcessor implements Processor {
*/
protected static byte[] getRandomKey(Element refList, WSDocInfo wsDocInfo)
throws WSSecurityException {
try {
- String alg = "AES";
- int size = 16;
+ String algorithmURI = WSConstants.AES_128;
String uri = getFirstDataRefURI(refList);
if (uri != null) {
Element ee =
EncryptionUtils.findEncryptedDataElement(wsDocInfo, uri);
- String algorithmURI = X509Util.getEncAlgo(ee);
- alg = JCEMapper.getJCEKeyAlgorithmFromURI(algorithmURI);
- size = KeyUtils.getKeyLength(algorithmURI);
+ algorithmURI = X509Util.getEncAlgo(ee);
}
- KeyGenerator kgen = KeyGenerator.getInstance(alg);
- kgen.init(size * 8);
- SecretKey k = kgen.generateKey();
+ // The key must have exactly the length required by the algorithm,
otherwise it is
+ // rejected later on, which would reveal that the key decryption
failed.
+ SecretKey k = KeyUtils.getKeyGenerator(algorithmURI).generateKey();
return k.getEncoded();
} catch (Throwable ex) {
// Fallback to just using AES to avoid attacks on EncryptedData
algorithms
diff --git
a/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/SignatureEncryptionTest.java
b/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/SignatureEncryptionTest.java
index a0b01dda0..df955aaac 100644
---
a/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/SignatureEncryptionTest.java
+++
b/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/SignatureEncryptionTest.java
@@ -116,6 +116,7 @@ public class SignatureEncryptionTest extends
AbstractTestBase {
actions.add(WSSConstants.ENCRYPTION);
actions.add(WSSConstants.TIMESTAMP);
securityProperties.setActions(actions);
+
securityProperties.setEncryptionSymAlgorithm(WSSConstants.NS_XENC_AES128);
securityProperties.loadEncryptionKeystore(this.getClass().getClassLoader().getResource("transmitter.jks"),
"default".toCharArray());
securityProperties.setEncryptionUser("receiver");
@@ -191,6 +192,7 @@ public class SignatureEncryptionTest extends
AbstractTestBase {
actions.add(WSSConstants.ENCRYPTION);
actions.add(WSSConstants.TIMESTAMP);
securityProperties.setActions(actions);
+
securityProperties.setEncryptionSymAlgorithm(WSSConstants.NS_XENC_AES128);
securityProperties.loadEncryptionKeystore(this.getClass().getClassLoader().getResource("transmitter.jks"),
"default".toCharArray());
securityProperties.setEncryptionUser("receiver");
@@ -293,6 +295,7 @@ public class SignatureEncryptionTest extends
AbstractTestBase {
actions.add(WSSConstants.SIGNATURE);
actions.add(WSSConstants.TIMESTAMP);
securityProperties.setActions(actions);
+
securityProperties.setEncryptionSymAlgorithm(WSSConstants.NS_XENC_AES128);
securityProperties.loadEncryptionKeystore(this.getClass().getClassLoader().getResource("transmitter.jks"),
"default".toCharArray());
securityProperties.setEncryptionUser("receiver");
