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 32887ed02 Follow-on from last DER patch (#726)
32887ed02 is described below
commit 32887ed02b276f797ad2e5bac8c1bc287d4eec13
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Tue Sep 22 09:28:40 2026 +0100
Follow-on from last DER patch (#726)
---
src/site/asciidoc/migration.adoc | 1 +
src/site/asciidoc/wss4j40.adoc | 67 +++++++++
.../org/apache/wss4j/common/crypto/CryptoBase.java | 34 +++--
.../org/apache/wss4j/common/crypto/Merlin.java | 29 +++-
.../org/apache/wss4j/common/crypto/MerlinAKI.java | 39 ++++-
...CastleUtils.java => X509KeyIdentifierUtil.java} | 60 ++++++--
.../org/apache/wss4j/common/token/DOMX509SKI.java | 17 ++-
.../common/crypto/AuthorityKeyIdentifierTest.java | 20 +--
.../apache/wss4j/common/crypto/DERDecoderTest.java | 166 +++++++++++++++++++++
.../common/crypto/MalformedKeyIdentifierTest.java | 154 +++++++++++++++++++
.../wss4j/common/crypto/NameConstraintsTest.java | 46 ++++++
.../org/apache/wss4j/common/crypto/SKITest.java | 48 ++++++
12 files changed, 642 insertions(+), 39 deletions(-)
diff --git a/src/site/asciidoc/migration.adoc b/src/site/asciidoc/migration.adoc
index 2b1245b3a..bc474ff56 100644
--- a/src/site/asciidoc/migration.adoc
+++ b/src/site/asciidoc/migration.adoc
@@ -21,6 +21,7 @@
Information about migrating to various new versions of WSS4J is provided in
this section.
+include::wss4j40.adoc[]
include::wss4j22.adoc[]
include::wss4j21.adoc[]
include::wss4j20.adoc[]
diff --git a/src/site/asciidoc/wss4j40.adoc b/src/site/asciidoc/wss4j40.adoc
new file mode 100644
index 000000000..9d25ca610
--- /dev/null
+++ b/src/site/asciidoc/wss4j40.adoc
@@ -0,0 +1,67 @@
+//
+// 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.
+//
+
+=== Apache WSS4J 4.0.0 Migration Guide
+
+This section is a migration guide for helping Apache WSS4J 3.0.x users to
migrate
+to the 4.0.x releases.
+
+==== Stricter DER decoding of X.509 key identifiers
+
+The SubjectKeyIdentifier (2.5.29.14) and AuthorityKeyIdentifier (2.5.29.35)
+extensions are no longer decoded with Bouncy Castle. WSS4J now decodes them
+itself, which removes the last compile-time dependency on the optional Bouncy
+Castle artifacts from these code paths.
+
+RFC 5280 requires certificate extensions to be DER encoded, and the WSS4J
decoder
+enforces that. Bouncy Castle accepted the broader BER encoding, so a
certificate
+that is not strictly DER encoded may now be rejected where it previously
worked.
+The encodings that change behaviour are:
+
+* a length in long form where the short form would do (for example `81 03`
+ instead of `03`), or a length carrying a leading zero byte;
+* a SubjectKeyIdentifier encoded as a constructed OCTET STRING (tag `24`) whose
+ segments Bouncy Castle would have concatenated;
+* an AuthorityKeyIdentifier `keyIdentifier` encoded as a constructed
+ `[0]` (tag `A0`) rather than a primitive one (tag `80`);
+* an AuthorityKeyIdentifier SEQUENCE using indefinite length;
+* an AuthorityKeyIdentifier whose fields appear out of order, or which repeats
+ the `[0]` field.
+
+Such a certificate now causes an `IllegalArgumentException` from
+`BouncyCastleUtils.getSubjectKeyIdentifierBytes` and
+`BouncyCastleUtils.getAuthorityKeyIdentifierBytes`, and a `WSSecurityException`
+from `Crypto.getSKIBytesFromCert`. This surfaces when signing with an SKI key
+identifier (`DOMX509SKI`) and when establishing trust with `MerlinAKI`.
+
+If you hit this, the certificate is malformed and should be reissued. There is
no
+configuration option to restore the previous tolerance.
+
+==== Crypto.getSKIBytesFromCert shares the key identifier decoder
+
+`CryptoBase.getSKIBytesFromCert` previously had its own SubjectKeyIdentifier
+decoder, which was more tolerant than the one used to emit an SKI key
identifier.
+Both now use the same decoder, so the identifier WSS4J writes and the one it
+accepts agree. A certificate whose SubjectKeyIdentifier has trailing bytes
after
+the extension value, or whose inner OCTET STRING is not exactly the length the
+wrapper declares, is now rejected with a `WSSecurityException` instead of being
+decoded on a best-effort basis.
+
+The behaviour for well formed certificates is unchanged, as is the SHA-1
fallback
+for certificates that carry no SubjectKeyIdentifier extension.
diff --git
a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java
b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java
index 7279e75f1..3a00688b9 100644
---
a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java
+++
b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java
@@ -208,6 +208,8 @@ public abstract class CryptoBase implements Crypto {
*
* @param cert The certificate to read SKI
* @return The byte array containing the binary SKI data
+ * @throws WSSecurityException if the SubjectKeyIdentifier extension is
present but is not
+ * valid DER, or if no SKI extension is present and SHA-1 is
unavailable
*/
public byte[] getSKIBytesFromCert(X509Certificate cert) throws
WSSecurityException {
//
@@ -232,15 +234,18 @@ public abstract class CryptoBase implements Crypto {
}
//
- // Strip away first (four) bytes from the DerValue (tag and length of
- // ExtensionValue OCTET STRING and KeyIdentifier OCTET STRING)
+ // Strip away the tag and length of the ExtensionValue OCTET STRING
and of the
+ // KeyIdentifier OCTET STRING it wraps. X509KeyIdentifierUtil is the
single decoder for
+ // this extension, so that the identifier we emit and the one we
accept agree.
//
- DERDecoder extVal = new DERDecoder(derEncodedValue);
- extVal.expect(DERDecoder.TYPE_OCTET_STRING); // ExtensionValue OCTET
STRING
- extVal.getLength();
- extVal.expect(DERDecoder.TYPE_OCTET_STRING); // KeyIdentifier OCTET
STRING
- int keyIDLen = extVal.getLength();
- return extVal.getBytes(keyIDLen);
+ try {
+ return
X509KeyIdentifierUtil.getSubjectKeyIdentifierBytes(derEncodedValue);
+ } catch (IllegalArgumentException ex) {
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.UNSUPPORTED_SECURITY_TOKEN, ex,
"noSKIHandling",
+ new Object[] {"Invalid SubjectKeyIdentifier certificate
extension"}
+ );
+ }
}
/**
@@ -387,8 +392,10 @@ public abstract class CryptoBase implements Crypto {
* that it is a single byte comparison, the performance hit is negligible.
*
* @param cert the certificate to extract NameConstraints from
- * @return the NameConstraints, or null if not present
- * @throws WSSecurityException if a processing error occurs decoding the
Octet String
+ * @return the DER-encoded NameConstraints sequence, or an empty array if
the certificate
+ * carries no NameConstraints extension
+ * @throws WSSecurityException if a processing error occurs decoding the
Octet String, or if
+ * the extension is tagged as something other than a SEQUENCE or
an OCTET STRING
*/
protected byte[] getNameConstraints(final X509Certificate cert) throws
WSSecurityException {
byte[] bytes = cert.getExtensionValue(NAME_CONSTRAINTS_OID);
@@ -405,8 +412,11 @@ public abstract class CryptoBase implements Crypto {
case DERDecoder.TYPE_SEQUENCE:
return bytes;
default:
- throw new IllegalArgumentException(
- "Invalid type for NameConstraints; must be Sequence or
OctetString-encoded Sequence");
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.FAILURE, "certpath",
+ new Object[] {"Invalid type for NameConstraints; must be a
SEQUENCE or an "
+ + "OCTET STRING-encoded SEQUENCE"}
+ );
}
}
}
diff --git
a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java
b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java
index 2fa2f204b..e75628382 100644
---
a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java
+++
b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java
@@ -1429,8 +1429,7 @@ public class Merlin extends CryptoBase {
(X509Certificate) keyStore.getCertificate(alias);
if (cert != null) {
if (certProviderHandlesNameConstraints) {
- TrustAnchor anchor = new TrustAnchor(cert,
getNameConstraints(cert));
- set.add(anchor);
+ set.add(createTrustAnchor(cert, alias));
} else {
TrustAnchor anchor = new TrustAnchor(cert, null);
set.add(anchor);
@@ -1439,6 +1438,32 @@ public class Merlin extends CryptoBase {
}
}
+ /**
+ * Build a {@code TrustAnchor} carrying the certificate's NameConstraints.
+ * <p>
+ * {@code getNameConstraints} reports "no constraints" as an empty array,
which
+ * {@code TrustAnchor} rejects; it wants null. It also signals constraints
it cannot decode
+ * with an IllegalArgumentException, which would otherwise escape a method
declared to throw
+ * WSSecurityException.
+ *
+ * @param cert the certificate to anchor trust in
+ * @param alias the keystore alias it was read from, used to identify it
in any error
+ */
+ private TrustAnchor createTrustAnchor(X509Certificate cert, String alias)
throws WSSecurityException {
+ byte[] nameConstraints = getNameConstraints(cert);
+ if (nameConstraints != null && nameConstraints.length == 0) {
+ nameConstraints = null; //NOPMD - TrustAnchor reads null, not
empty, as "unconstrained"
+ }
+ try {
+ return new TrustAnchor(cert, nameConstraints);
+ } catch (IllegalArgumentException ex) {
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.FAILURE, ex, "certpath",
+ new Object[] {"Invalid NameConstraints extension on keystore
entry " + alias}
+ );
+ }
+ }
+
/**
* Get an implementation-specific identifier that corresponds to the
X509Certificate. In
* this case, the identifier is the KeyStore alias.
diff --git
a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java
b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java
index 5e77f8abc..27bf7d7bf 100644
---
a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java
+++
b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java
@@ -124,8 +124,7 @@ public class MerlinAKI extends Merlin {
String issuerString = certs[0].getIssuerX500Principal().getName();
try {
if (certs.length == 1) {
- byte[] keyIdentifierBytes =
- BouncyCastleUtils.getAuthorityKeyIdentifierBytes(certs[0]);
+ byte[] keyIdentifierBytes =
getAuthorityKeyIdentifier(certs[0]);
X509Certificate[] foundCerts =
getX509CertificatesFromKeyIdentifier(keyIdentifierBytes);
// If the certs have not been found, the issuer is not in the
keystore/truststore
@@ -248,7 +247,7 @@ public class MerlinAKI extends Merlin {
if (certs != null && certs.length > 0 && certs[0] instanceof
X509Certificate) {
byte[] subjectKeyIdentifier =
-
BouncyCastleUtils.getSubjectKeyIdentifierBytes((X509Certificate)certs[0]);
+ getSubjectKeyIdentifier((X509Certificate)certs[0],
alias);
if (subjectKeyIdentifier != null
&& Arrays.equals(subjectKeyIdentifier, keyIdentifier))
{
return certs;
@@ -263,4 +262,38 @@ public class MerlinAKI extends Merlin {
return new Certificate[]{};
}
+ /**
+ * Read the AuthorityKeyIdentifier of a received certificate.
+ * <p>
+ * The certificate is attacker supplied, so a malformed extension is
reported as a
+ * WSSecurityException rather than the IllegalArgumentException the
decoder raises. Otherwise
+ * an unchecked exception escapes a method declared to throw
WSSecurityException.
+ */
+ private static byte[] getAuthorityKeyIdentifier(X509Certificate cert)
throws WSSecurityException {
+ try {
+ return X509KeyIdentifierUtil.getAuthorityKeyIdentifierBytes(cert);
+ } catch (IllegalArgumentException ex) {
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.FAILURE, ex, "certpath",
+ new Object[] {"Invalid AuthorityKeyIdentifier certificate
extension"}
+ );
+ }
+ }
+
+ /**
+ * Read the SubjectKeyIdentifier of a certificate held in the keystore or
truststore.
+ * <p>
+ * A certificate we cannot decode cannot match the identifier we are
looking for, so the entry
+ * is skipped rather than failing the whole lookup. Trust still fails if
no other entry
+ * matches.
+ */
+ private static byte[] getSubjectKeyIdentifier(X509Certificate cert, String
alias) {
+ try {
+ return X509KeyIdentifierUtil.getSubjectKeyIdentifierBytes(cert);
+ } catch (IllegalArgumentException ex) {
+ LOG.debug("Skipping alias {} with an invalid SubjectKeyIdentifier
extension", alias, ex);
+ return null; //NOPMD - no usable identifier is distinct from an
empty one
+ }
+ }
+
}
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/X509KeyIdentifierUtil.java
similarity index 59%
rename from
ws-security-common/src/main/java/org/apache/wss4j/common/crypto/BouncyCastleUtils.java
rename to
ws-security-common/src/main/java/org/apache/wss4j/common/crypto/X509KeyIdentifierUtil.java
index 521a47702..68aa4a4e5 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/X509KeyIdentifierUtil.java
@@ -23,15 +23,33 @@ import java.security.cert.X509Certificate;
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;
+/**
+ * Decodes the X.509 key identifier extensions.
+ */
+public final class X509KeyIdentifierUtil {
+ /** AuthorityKeyIdentifier keyIdentifier [0] IMPLICIT KeyIdentifier -
primitive, context-specific 0. */
+ private static final byte TAG_KEY_IDENTIFIER = (byte)0x80;
+ /** AuthorityKeyIdentifier authorityCertIssuer [1] GeneralNames -
constructed, context-specific 1. */
+ private static final byte TAG_AUTHORITY_CERT_ISSUER = (byte)0xA1;
+ /** AuthorityKeyIdentifier authorityCertSerialNumber [2] IMPLICIT
CertificateSerialNumber -
+ * primitive, context-specific 2. */
+ private static final byte TAG_AUTHORITY_CERT_SERIAL_NUMBER = (byte)0x82;
- private BouncyCastleUtils() {
+ private X509KeyIdentifierUtil() {
// complete
}
+ /**
+ * Read the keyIdentifier of the AuthorityKeyIdentifier extension
(2.5.29.35) of the
+ * given certificate.
+ * <p>
+ * X.509 extensions are required to use DER; BER encodings are rejected.
+ *
+ * @param cert the certificate to read the AuthorityKeyIdentifier from.
+ * @return an empty array if the certificate has no AuthorityKeyIdentifier
extension, null if
+ * the extension is present but carries no keyIdentifier,
otherwise the keyIdentifier.
+ * @throws IllegalArgumentException if the extension is present but is not
valid DER.
+ */
public static byte[] getAuthorityKeyIdentifierBytes(X509Certificate cert) {
byte[] extensionValue = cert.getExtensionValue("2.5.29.35"); //NOPMD
if (extensionValue == null) {
@@ -40,6 +58,16 @@ public final class BouncyCastleUtils {
return getAuthorityKeyIdentifierBytes(extensionValue);
}
+ /**
+ * Read the SubjectKeyIdentifier extension (2.5.29.14) of the given
certificate.
+ * <p>
+ * X.509 extensions are required to use DER; BER encodings are rejected.
+ *
+ * @param cert the certificate to read the SubjectKeyIdentifier from.
+ * @return an empty array if the certificate has no SubjectKeyIdentifier
extension, otherwise
+ * the key identifier.
+ * @throws IllegalArgumentException if the extension is present but is not
valid DER.
+ */
public static byte[] getSubjectKeyIdentifierBytes(X509Certificate cert) {
byte[] extensionValue = cert.getExtensionValue("2.5.29.14"); //NOPMD
if (extensionValue == null) {
@@ -55,9 +83,10 @@ public final class BouncyCastleUtils {
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);
+ byte[] keyIdentifier = readOptionalValue(authorityKeyIdentifier,
TAG_KEY_IDENTIFIER);
+ // The remaining fields are not used by WSS4J, but must still be
well formed.
+ skipOptionalValue(authorityKeyIdentifier,
TAG_AUTHORITY_CERT_ISSUER);
+ skipOptionalValue(authorityKeyIdentifier,
TAG_AUTHORITY_CERT_SERIAL_NUMBER);
authorityKeyIdentifier.expectEnd();
return keyIdentifier;
} catch (WSSecurityException ex) {
@@ -90,7 +119,7 @@ public final class BouncyCastleUtils {
}
private static byte[] readOptionalValue(DERDecoder decoder, byte type)
throws WSSecurityException {
- if (!decoder.hasRemaining() || !decoder.test(type)) {
+ if (!startsWith(decoder, type)) {
return null; //NOPMD - an absent optional value is distinct from
an empty value
}
decoder.expect(type);
@@ -98,4 +127,17 @@ public final class BouncyCastleUtils {
return decoder.getBytes(length);
}
+ private static void skipOptionalValue(DERDecoder decoder, byte type)
throws WSSecurityException {
+ if (!startsWith(decoder, type)) {
+ return;
+ }
+ decoder.expect(type);
+ int length = decoder.getLength();
+ decoder.skip(length);
+ }
+
+ private static boolean startsWith(DERDecoder decoder, byte type) throws
WSSecurityException {
+ return decoder.hasRemaining() && decoder.test(type);
+ }
+
}
diff --git
a/ws-security-common/src/main/java/org/apache/wss4j/common/token/DOMX509SKI.java
b/ws-security-common/src/main/java/org/apache/wss4j/common/token/DOMX509SKI.java
index b9bfc2476..0de945499 100644
---
a/ws-security-common/src/main/java/org/apache/wss4j/common/token/DOMX509SKI.java
+++
b/ws-security-common/src/main/java/org/apache/wss4j/common/token/DOMX509SKI.java
@@ -20,7 +20,8 @@
package org.apache.wss4j.common.token;
import org.apache.wss4j.common.WSS4JConstants;
-import org.apache.wss4j.common.crypto.BouncyCastleUtils;
+import org.apache.wss4j.common.crypto.X509KeyIdentifierUtil;
+import org.apache.wss4j.common.ext.WSSecurityException;
import org.apache.wss4j.common.util.DOM2Writer;
import org.w3c.dom.Document;
import org.apache.wss4j.common.util.XMLUtils;
@@ -38,9 +39,19 @@ public final class DOMX509SKI {
/**
* Constructor.
+ *
+ * @throws WSSecurityException if the certificate's SubjectKeyIdentifier
extension is present
+ * but is not valid DER.
*/
- public DOMX509SKI(Document doc, X509Certificate remoteCertificate) {
- skiBytes =
BouncyCastleUtils.getSubjectKeyIdentifierBytes(remoteCertificate);
+ public DOMX509SKI(Document doc, X509Certificate remoteCertificate) throws
WSSecurityException {
+ try {
+ skiBytes =
X509KeyIdentifierUtil.getSubjectKeyIdentifierBytes(remoteCertificate);
+ } catch (IllegalArgumentException ex) {
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.UNSUPPORTED_SECURITY_TOKEN, ex,
"noSKIHandling",
+ new Object[] {"Invalid SubjectKeyIdentifier certificate
extension"}
+ );
+ }
element = doc.createElementNS(WSS4JConstants.SIG_NS, "ds:X509SKI");
element.setTextContent(
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 ff0f0d340..95c6e1a37 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
@@ -55,7 +55,7 @@ public class AuthorityKeyIdentifierTest {
assertNotNull(cert);
// Get AuthorityKeyIdentifier from the cert
- byte[] keyIdentifierBytes =
BouncyCastleUtils.getAuthorityKeyIdentifierBytes(cert);
+ byte[] keyIdentifierBytes =
X509KeyIdentifierUtil.getAuthorityKeyIdentifierBytes(cert);
assertNotNull(keyIdentifierBytes);
// Now load the CA cert
@@ -67,7 +67,7 @@ public class AuthorityKeyIdentifierTest {
// Get SubjectKeyIdentifier from the CA cert
byte[] subjectKeyIdentifierBytes =
- BouncyCastleUtils.getSubjectKeyIdentifierBytes(caCert);
+ X509KeyIdentifierUtil.getSubjectKeyIdentifierBytes(caCert);
assertNotNull(subjectKeyIdentifierBytes);
assertTrue(Arrays.equals(keyIdentifierBytes,
subjectKeyIdentifierBytes));
@@ -81,11 +81,11 @@ public class AuthorityKeyIdentifierTest {
assertArrayEquals(
expectedKeyIdentifier,
-
BouncyCastleUtils.getAuthorityKeyIdentifierBytes(authorityKeyIdentifier)
+
X509KeyIdentifierUtil.getAuthorityKeyIdentifierBytes(authorityKeyIdentifier)
);
assertArrayEquals(
expectedKeyIdentifier,
-
BouncyCastleUtils.getSubjectKeyIdentifierBytes(subjectKeyIdentifier)
+
X509KeyIdentifierUtil.getSubjectKeyIdentifierBytes(subjectKeyIdentifier)
);
}
@@ -95,9 +95,9 @@ public class AuthorityKeyIdentifierTest {
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));
+
assertNull(X509KeyIdentifierUtil.getAuthorityKeyIdentifierBytes(authorityKeyIdentifier));
+
assertNull(X509KeyIdentifierUtil.getAuthorityKeyIdentifierBytes(authorityIssuerAndSerial));
+
assertNull(X509KeyIdentifierUtil.getAuthorityKeyIdentifierBytes(emptyAuthorityKeyIdentifier));
}
@Test
@@ -108,15 +108,15 @@ public class AuthorityKeyIdentifierTest {
assertThrows(
IllegalArgumentException.class,
- () ->
BouncyCastleUtils.getSubjectKeyIdentifierBytes(truncatedSubjectKeyIdentifier)
+ () ->
X509KeyIdentifierUtil.getSubjectKeyIdentifierBytes(truncatedSubjectKeyIdentifier)
);
assertThrows(
IllegalArgumentException.class,
- () ->
BouncyCastleUtils.getAuthorityKeyIdentifierBytes(trailingAuthorityKeyIdentifier)
+ () ->
X509KeyIdentifierUtil.getAuthorityKeyIdentifierBytes(trailingAuthorityKeyIdentifier)
);
assertThrows(
IllegalArgumentException.class,
- () ->
BouncyCastleUtils.getSubjectKeyIdentifierBytes(indefiniteLengthSubjectKeyIdentifier)
+ () ->
X509KeyIdentifierUtil.getSubjectKeyIdentifierBytes(indefiniteLengthSubjectKeyIdentifier)
);
}
diff --git
a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/DERDecoderTest.java
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/DERDecoderTest.java
new file mode 100644
index 000000000..2462b3cb5
--- /dev/null
+++
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/DERDecoderTest.java
@@ -0,0 +1,166 @@
+/**
+ * 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.crypto;
+
+import org.apache.wss4j.common.ext.WSSecurityException;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * Tests for the bounds handling of DERDecoder. A DER length is
attacker-controlled wherever the
+ * decoder is pointed at a certificate extension, so an over-long or
non-minimal length must be
+ * rejected rather than acted upon.
+ */
+public class DERDecoderTest {
+
+ /**
+ * A declared length of Integer.MAX_VALUE from an eleven byte input. The
bounds check must not
+ * be computed as "pos + length", which overflows to a negative number and
lets the decoder
+ * through to a two gigabyte allocation.
+ */
+ @Test
+ public void testGetBytesRejectsLengthThatWouldOverflowThePosition() throws
Exception {
+ // 04 09 | 04 84 7FFFFFFF | 01 02 03 -- the shape
CryptoBase.getSKIBytesFromCert decodes
+ byte[] extension = {4, 9, 4, (byte)0x84, 0x7F, (byte)0xFF, (byte)0xFF,
(byte)0xFF, 1, 2, 3};
+
+ DERDecoder decoder = new DERDecoder(extension);
+ decoder.expect(DERDecoder.TYPE_OCTET_STRING);
+ decoder.getLength();
+ decoder.expect(DERDecoder.TYPE_OCTET_STRING);
+ int keyIdentifierLength = decoder.getLength();
+ assertEquals(Integer.MAX_VALUE, keyIdentifierLength);
+
+ assertThrows(WSSecurityException.class, () ->
decoder.getBytes(keyIdentifierLength));
+ }
+
+ @Test
+ public void testGetBytesRejectsLengthBeyondTheRemainingInput() throws
Exception {
+ DERDecoder decoder = new DERDecoder(new byte[] {1, 2, 3, 4});
+ decoder.skip(2);
+
+ assertArrayEquals(new byte[] {3, 4}, new DERDecoder(new byte[] {3,
4}).getBytes(2));
+ assertThrows(WSSecurityException.class, () -> decoder.getBytes(3));
+ }
+
+ @Test
+ public void testGetBytesRejectsNegativeLength() throws Exception {
+ DERDecoder decoder = new DERDecoder(new byte[] {1, 2, 3, 4});
+
+ assertThrows(WSSecurityException.class, () -> decoder.getBytes(-1));
+ }
+
+ /**
+ * Skipping past the end must fail rather than leave the position out of
bounds, where a large
+ * enough length would wrap it negative and turn a later read into an
ArrayIndexOutOfBounds.
+ */
+ @Test
+ public void testSkipRejectsLengthBeyondTheRemainingInput() throws
Exception {
+ DERDecoder decoder = new DERDecoder(new byte[] {1, 2, 3, 4});
+ decoder.skip(4);
+ assertThrows(WSSecurityException.class, () -> decoder.skip(1));
+
+ DERDecoder overflowing = new DERDecoder(new byte[] {1, 2, 3, 4});
+ overflowing.skip(2);
+ assertThrows(WSSecurityException.class, () ->
overflowing.skip(Integer.MAX_VALUE));
+ }
+
+ @Test
+ public void testGetLengthReadsShortAndLongForm() throws Exception {
+ assertEquals(0, new DERDecoder(new byte[] {0}).getLength());
+ assertEquals(127, new DERDecoder(new byte[] {0x7F}).getLength());
+ assertEquals(128, new DERDecoder(new byte[] {(byte)0x81,
(byte)0x80}).getLength());
+ assertEquals(256, new DERDecoder(new byte[] {(byte)0x82, 1,
0}).getLength());
+ assertEquals(Integer.MAX_VALUE,
+ new DERDecoder(new byte[] {(byte)0x84, 0x7F, (byte)0xFF,
(byte)0xFF, (byte)0xFF}).getLength());
+ }
+
+ /**
+ * Indefinite length is reported as -1, as the method contract has always
stated. Every caller
+ * feeds the result to getBytes or skip, both of which reject a negative
length.
+ */
+ @Test
+ public void testGetLengthReportsIndefiniteLengthAsMinusOne() throws
Exception {
+ assertEquals(-1, new DERDecoder(new byte[] {(byte)0x80, 0,
0}).getLength());
+
+ DERDecoder decoder = new DERDecoder(new byte[] {(byte)0x80, 0, 0});
+ int length = decoder.getLength();
+ assertThrows(WSSecurityException.class, () ->
decoder.getBytes(length));
+ }
+
+ /**
+ * DER requires the shortest possible length encoding, so a long form that
could have been
+ * short, or one carrying a leading zero, is invalid.
+ */
+ @Test
+ public void testGetLengthRejectsNonMinimalEncodings() {
+ assertThrows(WSSecurityException.class,
+ () -> new DERDecoder(new byte[] {(byte)0x81, 0x7F}).getLength());
+ assertThrows(WSSecurityException.class,
+ () -> new DERDecoder(new byte[] {(byte)0x82, 0,
(byte)0x80}).getLength());
+ assertThrows(WSSecurityException.class,
+ () -> new DERDecoder(new byte[] {(byte)0x83, 0, 1,
0}).getLength());
+ }
+
+ @Test
+ public void testGetLengthRejectsLengthsThatDoNotFitInAnInt() {
+ // 0x80000000 is one past Integer.MAX_VALUE
+ assertThrows(WSSecurityException.class,
+ () -> new DERDecoder(new byte[] {(byte)0x84, (byte)0x80, 0, 0,
0}).getLength());
+ assertThrows(WSSecurityException.class,
+ () -> new DERDecoder(new byte[] {(byte)0x84, (byte)0xFF,
(byte)0xFF, (byte)0xFF, (byte)0xFF}).getLength());
+ // more length bytes than an int can hold
+ assertThrows(WSSecurityException.class,
+ () -> new DERDecoder(new byte[] {(byte)0x85, 1, 0, 0, 0,
0}).getLength());
+ }
+
+ @Test
+ public void testGetLengthRejectsTruncatedLengthSpecification() {
+ assertThrows(WSSecurityException.class, () -> new DERDecoder(new
byte[0]).getLength());
+ assertThrows(WSSecurityException.class,
+ () -> new DERDecoder(new byte[] {(byte)0x82, 1}).getLength());
+ }
+
+ @Test
+ public void testExpectEnd() throws Exception {
+ DERDecoder decoder = new DERDecoder(new byte[] {4, 1, 9});
+ decoder.expect(DERDecoder.TYPE_OCTET_STRING);
+ decoder.getBytes(decoder.getLength());
+ decoder.expectEnd();
+
+ DERDecoder trailing = new DERDecoder(new byte[] {4, 1, 9, 0});
+ trailing.expect(DERDecoder.TYPE_OCTET_STRING);
+ trailing.getBytes(trailing.getLength());
+ assertThrows(WSSecurityException.class, trailing::expectEnd);
+ }
+
+ @Test
+ public void testHasRemaining() throws Exception {
+ DERDecoder decoder = new DERDecoder(new byte[] {1, 2});
+ decoder.skip(1);
+ assertEquals(true, decoder.hasRemaining());
+ decoder.skip(1);
+ assertEquals(false, decoder.hasRemaining());
+
+ assertEquals(false, new DERDecoder(new byte[0]).hasRemaining());
+ }
+}
diff --git
a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/MalformedKeyIdentifierTest.java
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/MalformedKeyIdentifierTest.java
new file mode 100644
index 000000000..b3616f37f
--- /dev/null
+++
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/MalformedKeyIdentifierTest.java
@@ -0,0 +1,154 @@
+/**
+ * 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.crypto;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.security.KeyStore;
+import java.security.cert.CertificateFactory;
+import java.security.cert.X509Certificate;
+import java.util.Base64;
+
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import org.apache.wss4j.common.ext.WSSecurityException;
+import org.apache.wss4j.common.token.DOMX509SKI;
+import org.apache.wss4j.common.util.Loader;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.w3c.dom.Document;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * A certificate carrying a malformed key identifier extension reaches these
paths from the wire.
+ * The decoder signals a malformed extension with an IllegalArgumentException,
which must not
+ * escape a trust path declared to throw WSSecurityException.
+ */
+public class MalformedKeyIdentifierTest {
+
+ /**
+ * Self-signed, CN=malformed-aki. Its AuthorityKeyIdentifier extnValue is
+ * 04 08 30 05 80 03 01 02 03 00 -- a well formed SEQUENCE followed by a
trailing byte.
+ */
+ private static final String MALFORMED_AKI_CERT =
+
"MIIBuTCCASKgAwIBAgIBATANBgkqhkiG9w0BAQsFADAYMRYwFAYDVQQDDA1tYWxmb3JtZWQtYWtp"
+ +
"MB4XDTI1MDEwMTAwMDAwMFoXDTM1MDEwMTAwMDAwMFowGDEWMBQGA1UEAwwNbWFsZm9ybWVkLWFr"
+ +
"aTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAq6urq6urq6urq6urq6urq6urq6urq6urq6ur"
+ +
"q6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6ur"
+ +
"q6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6sCAwEAAaMTMBEwDwYD"
+ +
"VR0jBAgwBYADAQIDADANBgkqhkiG9w0BAQsFAAOBgQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ +
"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==";
+
+ /**
+ * Self-signed, CN=malformed-ski. Its SubjectKeyIdentifier extnValue is
+ * 04 09 04 84 7F FF FF FF 01 02 03 -- an inner OCTET STRING declaring
Integer.MAX_VALUE bytes.
+ */
+ private static final String MALFORMED_SKI_CERT =
+
"MIIBujCCASOgAwIBAgIBATANBgkqhkiG9w0BAQsFADAYMRYwFAYDVQQDDA1tYWxmb3JtZWQtc2tp"
+ +
"MB4XDTI1MDEwMTAwMDAwMFoXDTM1MDEwMTAwMDAwMFowGDEWMBQGA1UEAwwNbWFsZm9ybWVkLXNr"
+ +
"aTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAq6urq6urq6urq6urq6urq6urq6urq6urq6ur"
+ +
"q6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6ur"
+ +
"q6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6sCAwEAAaMUMBIwEAYD"
+ +
"VR0OBAkEhH////8BAgMwDQYJKoZIhvcNAQELBQADgYEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ +
"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
+
+ /**
+ * A received certificate whose AuthorityKeyIdentifier will not decode
must fail trust
+ * verification with a WSSecurityException, not an unchecked exception.
+ */
+ @Test
+ public void testMerlinAKIRejectsMalformedAuthorityKeyIdentifier() throws
Exception {
+ X509Certificate cert = decode(MALFORMED_AKI_CERT, "2.5.29.35");
+
+ MerlinAKI crypto = new MerlinAKI();
+ crypto.setTrustStore(loadKeyStore("keys/wss40CA.jks", "security"));
+
+ WSSecurityException ex = assertThrows(
+ WSSecurityException.class,
+ () -> crypto.verifyTrust(new X509Certificate[]{cert}, false, null)
+ );
+ assertNotNull(ex);
+ }
+
+ /**
+ * A truststore entry whose SubjectKeyIdentifier will not decode is
skipped rather than
+ * failing the lookup. The certificate under test is not trusted, so every
alias is visited
+ * and the malformed entry is guaranteed to be reached; trust must then
fail with a
+ * WSSecurityException rather than an unchecked exception from the skipped
entry.
+ */
+ @Test
+ public void
testMerlinAKISkipsTruststoreEntryWithMalformedSubjectKeyIdentifier() throws
Exception {
+ X509Certificate untrusted =
(X509Certificate)loadKeyStore("keys/wss86.keystore", "security")
+ .getCertificate("wss86");
+ assertNotNull(untrusted);
+
+ KeyStore trustStore = loadKeyStore("keys/wss40CA.jks", "security");
+ trustStore.setCertificateEntry("malformed-ski",
decode(MALFORMED_SKI_CERT, "2.5.29.14"));
+
+ MerlinAKI crypto = new MerlinAKI();
+ crypto.setTrustStore(trustStore);
+
+ WSSecurityException ex = assertThrows(
+ WSSecurityException.class,
+ () -> crypto.verifyTrust(new X509Certificate[]{untrusted}, false,
null)
+ );
+ assertEquals(WSSecurityException.ErrorCode.FAILURE, ex.getErrorCode());
+ }
+
+ /**
+ * Writing an SKI key identifier for a certificate we cannot decode must
fail with a
+ * WSSecurityException rather than an unchecked exception.
+ */
+ @Test
+ public void testDOMX509SKIRejectsMalformedSubjectKeyIdentifier() throws
Exception {
+ X509Certificate cert = decode(MALFORMED_SKI_CERT, "2.5.29.14");
+
+ DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+ factory.setNamespaceAware(true);
+ Document doc = factory.newDocumentBuilder().newDocument();
+
+ assertThrows(WSSecurityException.class, () -> new DOMX509SKI(doc,
cert));
+ }
+
+ private X509Certificate decode(String base64Certificate, String
expectedExtensionOid) throws Exception {
+ CertificateFactory certificateFactory =
CertificateFactory.getInstance("X.509");
+ X509Certificate cert;
+ try (InputStream input = new
ByteArrayInputStream(Base64.getDecoder().decode(base64Certificate))) {
+ cert =
(X509Certificate)certificateFactory.generateCertificate(input);
+ }
+ // a JDK that rejects the malformed extension outright leaves nothing
for us to decode
+ Assumptions.assumeTrue(cert.getExtensionValue(expectedExtensionOid) !=
null);
+ return cert;
+ }
+
+ private KeyStore loadKeyStore(String path, String password) throws
Exception {
+ KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
+ ClassLoader loader =
Loader.getClassLoader(MalformedKeyIdentifierTest.class);
+ try (InputStream input = Merlin.loadInputStream(loader, path)) {
+ keyStore.load(input, password.toCharArray());
+ }
+ return keyStore;
+ }
+}
diff --git
a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/NameConstraintsTest.java
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/NameConstraintsTest.java
index f9b4a3b23..ed7132145 100644
---
a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/NameConstraintsTest.java
+++
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/NameConstraintsTest.java
@@ -33,8 +33,10 @@ import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
+import java.util.HashSet;
import java.util.List;
import java.util.Properties;
+import java.util.Set;
import java.util.regex.Pattern;
import org.apache.wss4j.common.ext.WSSecurityException;
@@ -262,6 +264,50 @@ public class NameConstraintsTest {
getMerlinAkiBc());
}
+ /**
+ * getNameConstraints reports "no constraints" as an empty array, but
TrustAnchor reads an
+ * empty array as an undecodable extension and only accepts null. Most CA
certificates carry
+ * no NameConstraints, so building anchors for them must still work with
the
+ * cert.provider.nameconstraints property enabled.
+ */
+ @Test
+ public void testTrustAnchorsForCertificatesWithoutNameConstraints() throws
Exception {
+ assumeFalse(isIBMJdK);
+
+ Merlin merlin = merlinHandlingNameConstraints();
+ Set<TrustAnchor> anchors = new HashSet<>();
+
+ merlin.addTrustAnchors(anchors, getSelfKeyStore());
+
+ assertThat(anchors.size(), equalTo(1));
+ assertNull(anchors.iterator().next().getNameConstraints());
+ }
+
+ /**
+ * The constraints of a certificate that does carry the extension must
still reach the anchor.
+ */
+ @Test
+ public void testTrustAnchorsRetainNameConstraints() throws Exception {
+ assumeFalse(isIBMJdK);
+
+ Merlin merlin = merlinHandlingNameConstraints();
+ Set<TrustAnchor> anchors = new HashSet<>();
+
+ merlin.addTrustAnchors(anchors, getRootKeyStore());
+
+ assertThat(anchors.size(), equalTo(1));
+ TrustAnchor anchor = anchors.iterator().next();
+ assertNotNull(anchor.getNameConstraints());
+ assertThat("Tag byte is wrong", anchor.getNameConstraints()[0],
is(DERDecoder.TYPE_SEQUENCE));
+ }
+
+ private Merlin merlinHandlingNameConstraints() throws Exception {
+ Properties properties = new Properties();
+
properties.setProperty("org.apache.wss4j.crypto.merlin.cert.provider.nameconstraints",
+ "true");
+ return new Merlin(properties, this.getClass().getClassLoader(), null);
+ }
+
@Test
public void testNameConstraintsWithKeyStoreUsingMerlinBreaking() throws
Exception {
assumeFalse(isIBMJdK);
diff --git
a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/SKITest.java
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/SKITest.java
index 14345287c..208a24611 100644
---
a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/SKITest.java
+++
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/SKITest.java
@@ -19,16 +19,23 @@
package org.apache.wss4j.common.crypto;
+import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.security.KeyStore;
import java.security.Security;
+import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
+import java.util.Base64;
+import org.apache.wss4j.common.ext.WSSecurityException;
import org.apache.wss4j.common.util.Loader;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.junit.jupiter.api.Assumptions;
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.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
@@ -82,6 +89,47 @@ public class SKITest {
assertTrue(knownBase64Encoding.equals(org.apache.xml.security.utils.XMLUtils.encodeToString(skiBytes)));
}
+ /**
+ * A self-signed certificate whose SubjectKeyIdentifier extension declares
a key identifier of
+ * Integer.MAX_VALUE bytes. The certificate itself is 442 bytes. Its
extnValue is
+ * 04 09 04 84 7F FF FF FF 01 02 03: an OCTET STRING wrapping an OCTET
STRING whose long-form
+ * length is 0x7FFFFFFF. Certificates reach this code path from an inbound
+ * wsse:BinarySecurityToken, so the declared length is attacker-controlled.
+ */
+ private static final String HOSTILE_SKI_CERT =
+
"MIIBtjCCAR+gAwIBAgIBATANBgkqhkiG9w0BAQsFADAWMRQwEgYDVQQDDAtob3N0aWxlLXNraTAe"
+ +
"Fw0yNTAxMDEwMDAwMDBaFw0zNTAxMDEwMDAwMDBaMBYxFDASBgNVBAMMC2hvc3RpbGUtc2tpMIGf"
+ +
"MA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCrq6urq6urq6urq6urq6urq6urq6urq6urq6urq6ur"
+ +
"q6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6ur"
+ +
"q6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urqwIDAQABoxQwEjAQBgNVHQ4E"
+ +
"CQSEf////wECAzANBgkqhkiG9w0BAQsFAAOBgQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ +
"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==";
+
+ /**
+ * An over-long declared length in the SubjectKeyIdentifier must produce a
WSSecurityException,
+ * not a multi-gigabyte allocation and not an unchecked exception.
+ */
+ @Test
+ public void testSKIWithOverlongDeclaredLength() throws Exception {
+ CertificateFactory certificateFactory =
CertificateFactory.getInstance("X.509");
+ X509Certificate cert;
+ try (InputStream input = new
ByteArrayInputStream(Base64.getDecoder().decode(HOSTILE_SKI_CERT))) {
+ cert =
(X509Certificate)certificateFactory.generateCertificate(input);
+ }
+
+ byte[] extensionValue = cert.getExtensionValue(CryptoBase.SKI_OID);
+ // A JDK that rejects the malformed extension outright leaves nothing
for us to decode
+ Assumptions.assumeTrue(extensionValue != null);
+ assertArrayEquals(
+ new byte[] {4, 9, 4, (byte)0x84, 0x7F, (byte)0xFF, (byte)0xFF,
(byte)0xFF, 1, 2, 3},
+ extensionValue
+ );
+
+ Crypto crypto = new Merlin();
+ assertThrows(WSSecurityException.class, () ->
crypto.getSKIBytesFromCert(cert));
+ }
+
@Test
public void testBouncyCastlePKCS12() throws Exception {
try {
