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 2816faf7b Remove Bouncy Castle from key identifier parsing (#647)
2816faf7b is described below
commit 2816faf7b272ffbb2cc71e42072fe1b66c5f2ef4
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
+}
