This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch 3_0_x-fixes in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
commit bc11caf06cbb790867336ed945b2bcd5284c322d Author: araiprof <[email protected]> AuthorDate: Mon Sep 21 16:29:17 2026 +0200 Remove Bouncy Castle from key identifier parsing (#647) --- .../wss4j/common/crypto/BouncyCastleUtils.java | 76 +++++++++++++++++----- .../org/apache/wss4j/common/crypto/DERDecoder.java | 73 +++++++++++++++++---- .../common/crypto/AuthorityKeyIdentifierTest.java | 54 ++++++++++++++- 3 files changed, 173 insertions(+), 30 deletions(-) diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/BouncyCastleUtils.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/BouncyCastleUtils.java index 47556d9df..521a47702 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/BouncyCastleUtils.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/BouncyCastleUtils.java @@ -21,11 +21,12 @@ package org.apache.wss4j.common.crypto; import java.security.cert.X509Certificate; -import org.bouncycastle.asn1.ASN1OctetString; -import org.bouncycastle.asn1.x509.AuthorityKeyIdentifier; -import org.bouncycastle.asn1.x509.SubjectKeyIdentifier; +import org.apache.wss4j.common.ext.WSSecurityException; public final class BouncyCastleUtils { + private static final byte TYPE_CONTEXT_SPECIFIC_0 = (byte)0x80; + private static final byte TYPE_CONTEXT_SPECIFIC_1 = (byte)0xA1; + private static final byte TYPE_CONTEXT_SPECIFIC_2 = (byte)0x82; private BouncyCastleUtils() { // complete @@ -33,27 +34,68 @@ public final class BouncyCastleUtils { public static byte[] getAuthorityKeyIdentifierBytes(X509Certificate cert) { byte[] extensionValue = cert.getExtensionValue("2.5.29.35"); //NOPMD - if (extensionValue != null) { - byte[] octets = ASN1OctetString.getInstance(extensionValue).getOctets(); - AuthorityKeyIdentifier authorityKeyIdentifier = - AuthorityKeyIdentifier.getInstance(octets); - return authorityKeyIdentifier.getKeyIdentifier(); + if (extensionValue == null) { + return new byte[0]; } - return new byte[0]; + return getAuthorityKeyIdentifierBytes(extensionValue); } public static byte[] getSubjectKeyIdentifierBytes(X509Certificate cert) { byte[] extensionValue = cert.getExtensionValue("2.5.29.14"); //NOPMD - if (extensionValue != null) { - byte[] subjectOctets = - ASN1OctetString.getInstance(extensionValue).getOctets(); - SubjectKeyIdentifier subjectKeyIdentifier = - SubjectKeyIdentifier.getInstance(subjectOctets); - return subjectKeyIdentifier.getKeyIdentifier(); + if (extensionValue == null) { + return new byte[0]; } - return new byte[0]; + return getSubjectKeyIdentifierBytes(extensionValue); } -} + static byte[] getAuthorityKeyIdentifierBytes(byte[] extensionValue) { + try { + byte[] extensionBytes = readExtensionValue(extensionValue, DERDecoder.TYPE_SEQUENCE); + if (extensionBytes.length == 0) { + return null; //NOPMD - AuthorityKeyIdentifier#getKeyIdentifier returns null when absent + } + DERDecoder authorityKeyIdentifier = new DERDecoder(extensionBytes); + byte[] keyIdentifier = readOptionalValue(authorityKeyIdentifier, TYPE_CONTEXT_SPECIFIC_0); + readOptionalValue(authorityKeyIdentifier, TYPE_CONTEXT_SPECIFIC_1); + readOptionalValue(authorityKeyIdentifier, TYPE_CONTEXT_SPECIFIC_2); + authorityKeyIdentifier.expectEnd(); + return keyIdentifier; + } catch (WSSecurityException ex) { + throw new IllegalArgumentException("Invalid AuthorityKeyIdentifier extension", ex); + } + } + + static byte[] getSubjectKeyIdentifierBytes(byte[] extensionValue) { + try { + return readExtensionValue(extensionValue, DERDecoder.TYPE_OCTET_STRING); + } catch (WSSecurityException ex) { + throw new IllegalArgumentException("Invalid SubjectKeyIdentifier extension", ex); + } + } + private static byte[] readExtensionValue(byte[] extensionValue, byte extensionType) + throws WSSecurityException { + DERDecoder extension = new DERDecoder(extensionValue); + extension.expect(DERDecoder.TYPE_OCTET_STRING); + int extensionLength = extension.getLength(); + byte[] extensionBytes = extension.getBytes(extensionLength); + extension.expectEnd(); + DERDecoder extensionContents = new DERDecoder(extensionBytes); + extensionContents.expect(extensionType); + int extensionContentsLength = extensionContents.getLength(); + byte[] contents = extensionContents.getBytes(extensionContentsLength); + extensionContents.expectEnd(); + return contents; + } + + private static byte[] readOptionalValue(DERDecoder decoder, byte type) throws WSSecurityException { + if (!decoder.hasRemaining() || !decoder.test(type)) { + return null; //NOPMD - an absent optional value is distinct from an empty value + } + decoder.expect(type); + int length = decoder.getLength(); + return decoder.getBytes(length); + } + +} diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/DERDecoder.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/DERDecoder.java index 78a89267b..8d8045e67 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/DERDecoder.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/DERDecoder.java @@ -77,11 +77,20 @@ public class DERDecoder { pos = 0; } + /** + * Return whether there are bytes left to decode. + * + * @return true if bytes remain. + */ + public boolean hasRemaining() { + return pos < arr.length; + } + /** * Advance the current position by the given number of bytes. * * @param length the number of bytes to skip. - * @throws WSSecurityException if length is negative. + * @throws WSSecurityException if length is negative or exceeds the remaining input. */ public void skip(int length) throws WSSecurityException { if (length < 0) { @@ -91,6 +100,13 @@ public class DERDecoder { new Object[] {"Unsupported DER format"} ); } + if (length > arr.length - pos) { + throw new WSSecurityException( + WSSecurityException.ErrorCode.UNSUPPORTED_SECURITY_TOKEN, + "noSKIHandling", + new Object[] {"Invalid DER format"} + ); + } pos += length; } @@ -170,11 +186,15 @@ public class DERDecoder { new Object[] {"Invalid DER format"} ); } + int firstByte = arr[pos++] & 0xFF; int len; - if ((arr[pos] & 0xFF) <= 0x7F) { - len = arr[pos++]; + if (firstByte <= 0x7F) { + len = firstByte; } else { - int nbytes = arr[pos++] & 0x7F; + int nbytes = firstByte & 0x7F; + if (nbytes == 0) { + return -1; + } if (pos + nbytes > arr.length) { throw new WSSecurityException( WSSecurityException.ErrorCode.UNSUPPORTED_SECURITY_TOKEN, @@ -182,9 +202,25 @@ public class DERDecoder { new Object[] {"Invalid DER format"} ); } + if (nbytes > Integer.BYTES || arr[pos] == 0) { + throw new WSSecurityException( + WSSecurityException.ErrorCode.UNSUPPORTED_SECURITY_TOKEN, + "noSKIHandling", + new Object[] {"Invalid DER format"} + ); + } byte[] lenBytes = new byte[nbytes]; System.arraycopy(arr, pos, lenBytes, 0, lenBytes.length); - len = new BigInteger(1, lenBytes).intValue(); + BigInteger bigIntegerLength = new BigInteger(1, lenBytes); + if (bigIntegerLength.compareTo(BigInteger.valueOf(0x80)) < 0 + || bigIntegerLength.compareTo(BigInteger.valueOf(Integer.MAX_VALUE)) > 0) { + throw new WSSecurityException( + WSSecurityException.ErrorCode.UNSUPPORTED_SECURITY_TOKEN, + "noSKIHandling", + new Object[] {"Invalid DER format"} + ); + } + len = bigIntegerLength.intValue(); pos += nbytes; } return len; @@ -201,18 +237,18 @@ public class DERDecoder { * length is negative. */ public byte[] getBytes(int length) throws WSSecurityException { - if (pos + length > arr.length) { + if (length < 0) { throw new WSSecurityException( WSSecurityException.ErrorCode.UNSUPPORTED_SECURITY_TOKEN, "noSKIHandling", - new Object[] {"Invalid DER format"} - ); - } else if (length < 0) { + new Object[] {"Unsupported DER format"} + ); + } else if (length > arr.length - pos) { throw new WSSecurityException( WSSecurityException.ErrorCode.UNSUPPORTED_SECURITY_TOKEN, "noSKIHandling", - new Object[] {"Unsupported DER format"} - ); + new Object[] {"Invalid DER format"} + ); } byte[] value = new byte[length]; System.arraycopy(arr, pos, value, 0, length); @@ -220,4 +256,19 @@ public class DERDecoder { return value; } + /** + * Confirm that the current position is at the end of the DER value. + * + * @throws WSSecurityException if unconsumed bytes remain. + */ + public void expectEnd() throws WSSecurityException { + if (pos != arr.length) { + throw new WSSecurityException( + WSSecurityException.ErrorCode.UNSUPPORTED_SECURITY_TOKEN, + "noSKIHandling", + new Object[] {"Invalid DER format"} + ); + } + } + } diff --git a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/AuthorityKeyIdentifierTest.java b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/AuthorityKeyIdentifierTest.java index 26eca4b09..ff0f0d340 100644 --- a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/AuthorityKeyIdentifierTest.java +++ b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/AuthorityKeyIdentifierTest.java @@ -28,13 +28,16 @@ import org.apache.wss4j.common.ext.WSSecurityException; import org.apache.wss4j.common.util.Loader; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; /** * This is a test for extracting AuthorityKeyIdentifier/SubjectKeyIdentifier information from - * the certs using BouncyCastle. + * certificates. */ public class AuthorityKeyIdentifierTest { @@ -70,6 +73,53 @@ public class AuthorityKeyIdentifierTest { assertTrue(Arrays.equals(keyIdentifierBytes, subjectKeyIdentifierBytes)); } + @Test + public void testExtractKeyIdentifiersFromDer() { + byte[] expectedKeyIdentifier = {1, 2, 3}; + byte[] authorityKeyIdentifier = {4, 7, 48, 5, (byte)0x80, 3, 1, 2, 3}; + byte[] subjectKeyIdentifier = {4, 5, 4, 3, 1, 2, 3}; + + assertArrayEquals( + expectedKeyIdentifier, + BouncyCastleUtils.getAuthorityKeyIdentifierBytes(authorityKeyIdentifier) + ); + assertArrayEquals( + expectedKeyIdentifier, + BouncyCastleUtils.getSubjectKeyIdentifierBytes(subjectKeyIdentifier) + ); + } + + @Test + public void testAuthorityKeyIdentifierWithoutKeyIdentifier() { + byte[] authorityKeyIdentifier = {4, 5, 48, 3, (byte)0x82, 1, 1}; + byte[] authorityIssuerAndSerial = {4, 10, 48, 8, (byte)0xA1, 3, 48, 1, 0, (byte)0x82, 1, 1}; + byte[] emptyAuthorityKeyIdentifier = {4, 2, 48, 0}; + + assertNull(BouncyCastleUtils.getAuthorityKeyIdentifierBytes(authorityKeyIdentifier)); + assertNull(BouncyCastleUtils.getAuthorityKeyIdentifierBytes(authorityIssuerAndSerial)); + assertNull(BouncyCastleUtils.getAuthorityKeyIdentifierBytes(emptyAuthorityKeyIdentifier)); + } + + @Test + public void testRejectMalformedKeyIdentifiers() { + byte[] truncatedSubjectKeyIdentifier = {4, 5, 4, 3, 1, 2}; + byte[] trailingAuthorityKeyIdentifier = {4, 7, 48, 5, (byte)0x80, 3, 1, 2, 3, 0}; + byte[] indefiniteLengthSubjectKeyIdentifier = {4, (byte)0x80, 4, 0, 0, 0}; + + assertThrows( + IllegalArgumentException.class, + () -> BouncyCastleUtils.getSubjectKeyIdentifierBytes(truncatedSubjectKeyIdentifier) + ); + assertThrows( + IllegalArgumentException.class, + () -> BouncyCastleUtils.getAuthorityKeyIdentifierBytes(trailingAuthorityKeyIdentifier) + ); + assertThrows( + IllegalArgumentException.class, + () -> BouncyCastleUtils.getSubjectKeyIdentifierBytes(indefiniteLengthSubjectKeyIdentifier) + ); + } + @Test public void testMerlinAKI() throws Exception { // Load the keystore @@ -110,4 +160,4 @@ public class AuthorityKeyIdentifierTest { return keyStore; } -} \ No newline at end of file +}
