This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/keyutils in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
commit d7f727bc487e03f272b2fba11393bdb38f231b51 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Tue Sep 8 10:39:12 2026 +0100 Reject mismatched symmetric key lengths to prevent cross-algorithm key reuse --- .../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");
