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 695d6379e WSS-721 - Search the truststore before the keystore when
verifying trust (#750)
695d6379e is described below
commit 695d6379e7d292fdc6a5daf80757a7b57b82dffe
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Tue Oct 13 07:55:19 2026 +0100
WSS-721 - Search the truststore before the keystore when verifying trust
(#750)
---
.../org/apache/wss4j/common/crypto/Merlin.java | 80 +++---
.../org/apache/wss4j/common/crypto/MerlinAKI.java | 46 ++--
.../wss4j/common/crypto/MerlinTrustTest.java | 280 +++++++++++++++++++++
3 files changed, 347 insertions(+), 59 deletions(-)
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 6483ff4da..0ae67eda8 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
@@ -760,33 +760,20 @@ public class Merlin extends CryptoBase {
Collection<Pattern> subjectCertConstraints
) throws WSSecurityException {
//
- // FIRST step - Search the keystore for the transmitted certificate
+ // FIRST step - Search the truststore and keystore for the transmitted
certificate
//
- if (certs.length == 1 && !enableRevocation) {
- String issuerString = certs[0].getIssuerX500Principal().getName();
- BigInteger issuerSerial = certs[0].getSerialNumber();
-
- CryptoType cryptoType = new
CryptoType(CryptoType.TYPE.ISSUER_SERIAL);
- cryptoType.setIssuerSerial(issuerString, issuerSerial);
- X509Certificate[] foundCerts = getX509Certificates(cryptoType);
-
- //
- // If a certificate has been found, the certificates must be
compared
- // to ensure against phony DNs (compare encoded form including
signature)
- //
- if (foundCerts != null && foundCerts.length > 0 && foundCerts[0]
!= null && foundCerts[0].equals(certs[0])) {
- try {
- certs[0].checkValidity();
- } catch (CertificateExpiredException |
CertificateNotYetValidException e) {
- throw new WSSecurityException(
- WSSecurityException.ErrorCode.FAILED_CHECK, e,
"invalidCert"
- );
- }
- LOG.debug(
- "Direct trust for certificate with {}",
certs[0].getSubjectX500Principal().getName()
+ if (certs.length == 1 && !enableRevocation &&
isDirectlyTrusted(certs[0])) {
+ try {
+ certs[0].checkValidity();
+ } catch (CertificateExpiredException |
CertificateNotYetValidException e) {
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.FAILED_CHECK, e,
"invalidCert"
);
- return;
}
+ LOG.debug(
+ "Direct trust for certificate with {}",
certs[0].getSubjectX500Principal().getName()
+ );
+ return;
}
//
@@ -799,13 +786,13 @@ public class Merlin extends CryptoBase {
Object subject = convertSubjectToPrincipal(issuerString);
- if (keystore != null) {
- foundIssuingCertChains = getCertificates(subject, keystore,
false);
+ if (truststore != null) {
+ foundIssuingCertChains = getCertificates(subject, truststore,
true);
}
- //If we can't find the issuer in the keystore then look at the
truststore
- if ((foundIssuingCertChains == null ||
foundIssuingCertChains.isEmpty()) && truststore != null) {
- foundIssuingCertChains = getCertificates(subject, truststore,
true);
+ //If we can't find the issuer in the truststore then look at the
keystore
+ if ((foundIssuingCertChains == null ||
foundIssuingCertChains.isEmpty()) && keystore != null) {
+ foundIssuingCertChains = getCertificates(subject, keystore,
false);
}
if (foundIssuingCertChains == null ||
foundIssuingCertChains.isEmpty()
@@ -901,6 +888,41 @@ public class Merlin extends CryptoBase {
}
}
+ /**
+ * Check whether the given certificate is directly trusted, i.e. whether
the exact same
+ * certificate is stored in the truststore or the keystore. The truststore
is searched
+ * first, as that is where trusted certificates are normally stored, so
that the keystore
+ * does not need to be searched in the common case (WSS-721).
+ *
+ * @param cert The certificate to check
+ * @return true if the certificate was found in the truststore or keystore
+ * @throws WSSecurityException
+ */
+ protected boolean isDirectlyTrusted(X509Certificate cert) throws
WSSecurityException {
+ Object issuerName =
convertSubjectToPrincipal(cert.getIssuerX500Principal().getName());
+ BigInteger serialNumber = cert.getSerialNumber();
+
+ //
+ // If a certificate has been found, the certificates must be compared
+ // to ensure against phony DNs (compare encoded form including
signature)
+ //
+ if (truststore != null) {
+ Certificate[] foundCerts = getCertificates(issuerName,
serialNumber, truststore, true);
+ if (foundCerts.length > 0 && cert.equals(foundCerts[0])) {
+ return true;
+ }
+ }
+
+ if (keystore != null) {
+ Certificate[] foundCerts = getCertificates(issuerName,
serialNumber, keystore, false);
+ if (foundCerts.length > 0 && cert.equals(foundCerts[0])) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
@Override
public void verifyTrust(X509Certificate[] certs, boolean enableRevocation,
Collection<Pattern> subjectCertConstraints,
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 549f64189..90a1782cb 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
@@ -20,7 +20,6 @@
package org.apache.wss4j.common.crypto;
import java.io.IOException;
-import java.math.BigInteger;
import java.security.InvalidAlgorithmParameterException;
import java.security.KeyStore;
import java.security.KeyStoreException;
@@ -87,33 +86,20 @@ public class MerlinAKI extends Merlin {
Collection<Pattern> subjectCertConstraints
) throws WSSecurityException {
//
- // FIRST step - Search the keystore for the transmitted certificate
+ // FIRST step - Search the truststore and keystore for the transmitted
certificate
//
- if (certs.length == 1 && !enableRevocation) {
- String issuerString = certs[0].getIssuerX500Principal().getName();
- BigInteger issuerSerial = certs[0].getSerialNumber();
-
- CryptoType cryptoType = new
CryptoType(CryptoType.TYPE.ISSUER_SERIAL);
- cryptoType.setIssuerSerial(issuerString, issuerSerial);
- X509Certificate[] foundCerts = getX509Certificates(cryptoType);
-
- //
- // If a certificate has been found, the certificates must be
compared
- // to ensure against phony DNs (compare encoded form including
signature)
- //
- if (foundCerts != null && foundCerts.length > 0 && foundCerts[0]
!= null && foundCerts[0].equals(certs[0])) {
- try {
- certs[0].checkValidity();
- } catch (CertificateExpiredException |
CertificateNotYetValidException e) {
- throw new WSSecurityException(
- WSSecurityException.ErrorCode.FAILED_CHECK, e,
"invalidCert"
- );
- }
- LOG.debug(
- "Direct trust for certificate with {}",
certs[0].getSubjectX500Principal().getName()
+ if (certs.length == 1 && !enableRevocation &&
isDirectlyTrusted(certs[0])) {
+ try {
+ certs[0].checkValidity();
+ } catch (CertificateExpiredException |
CertificateNotYetValidException e) {
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.FAILED_CHECK, e,
"invalidCert"
);
- return;
}
+ LOG.debug(
+ "Direct trust for certificate with {}",
certs[0].getSubjectX500Principal().getName()
+ );
+ return;
}
//
@@ -220,13 +206,13 @@ public class MerlinAKI extends Merlin {
}
Certificate[] certs = null;
- if (keystore != null) {
- certs = getCertificates(keyIdentifierBytes, keystore);
+ if (truststore != null) {
+ certs = getCertificates(keyIdentifierBytes, truststore);
}
- //If we can't find the issuer in the keystore then look at the
truststore
- if ((certs == null || certs.length == 0) && truststore != null) {
- certs = getCertificates(keyIdentifierBytes, truststore);
+ //If we can't find the issuer in the truststore then look at the
keystore
+ if ((certs == null || certs.length == 0) && keystore != null) {
+ certs = getCertificates(keyIdentifierBytes, keystore);
}
if (certs == null || certs.length == 0) {
diff --git
a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/MerlinTrustTest.java
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/MerlinTrustTest.java
new file mode 100644
index 000000000..b4b2dc93a
--- /dev/null
+++
b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/MerlinTrustTest.java
@@ -0,0 +1,280 @@
+/**
+ * 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.InputStream;
+import java.io.OutputStream;
+import java.security.Key;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.KeyStoreSpi;
+import java.security.cert.Certificate;
+import java.security.cert.X509Certificate;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Enumeration;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.wss4j.common.ext.WSSecurityException;
+import org.apache.wss4j.common.util.Loader;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * Tests for the order in which Merlin searches the truststore and keystore
when verifying
+ * trust in a certificate (WSS-721).
+ */
+public class MerlinTrustTest {
+
+ private static X509Certificate wss40Cert;
+
+ @BeforeAll
+ public static void setup() throws Exception {
+ WSProviderConfig.init();
+ wss40Cert =
(X509Certificate)loadKeyStore("keys/wss40.jks").getCertificate("wss40");
+ }
+
+ /**
+ * A certificate that is in the truststore is directly trusted without
searching the keystore.
+ */
+ @Test
+ public void testDirectTrustSearchesTruststoreFirst() throws Exception {
+ verifyDirectTrustSearchesTruststoreFirst(new Merlin());
+ }
+
+ @Test
+ public void testDirectTrustSearchesTruststoreFirstAKI() throws Exception {
+ verifyDirectTrustSearchesTruststoreFirst(new MerlinAKI());
+ }
+
+ private void verifyDirectTrustSearchesTruststoreFirst(Merlin crypto)
throws Exception {
+ KeyStore truststore = createEmptyKeyStore();
+ truststore.setCertificateEntry("wss40", wss40Cert);
+
+ AtomicInteger keystoreSearches = new AtomicInteger();
+
crypto.setKeyStore(countingKeyStore(loadKeyStore("keys/wss40_server.jks"),
keystoreSearches));
+ crypto.setTrustStore(truststore);
+
+ crypto.verifyTrust(new X509Certificate[] {wss40Cert}, false, null,
null);
+ assertEquals(0, keystoreSearches.get());
+ }
+
+ /**
+ * The issuer of a certificate is looked up in the truststore before the
keystore. The keystore
+ * is only searched once, for the certificate itself, and not again for
its issuer.
+ */
+ @Test
+ public void testIssuerSearchesTruststoreFirst() throws Exception {
+ AtomicInteger keystoreSearches = new AtomicInteger();
+ Merlin crypto = new Merlin();
+
crypto.setKeyStore(countingKeyStore(loadKeyStore("keys/wss40_server.jks"),
keystoreSearches));
+ crypto.setTrustStore(loadKeyStore("keys/wss40CA.jks"));
+
+ crypto.verifyTrust(new X509Certificate[] {wss40Cert}, false, null,
null);
+ assertEquals(1, keystoreSearches.get());
+ }
+
+ /**
+ * A certificate that is only in the keystore is still directly trusted.
+ */
+ @Test
+ public void testDirectTrustInKeystore() throws Exception {
+ Merlin crypto = new Merlin();
+ crypto.setKeyStore(loadKeyStore("keys/wss40.jks"));
+ crypto.setTrustStore(createEmptyKeyStore());
+
+ crypto.verifyTrust(new X509Certificate[] {wss40Cert}, false, null,
null);
+ }
+
+ /**
+ * A certificate in neither the truststore nor the keystore, whose issuer
is only in the keystore,
+ * is not trusted when a truststore is configured.
+ */
+ @Test
+ public void testNotTrusted() throws Exception {
+ Merlin crypto = new Merlin();
+ crypto.setKeyStore(loadKeyStore("keys/wss40_server.jks"));
+ crypto.setTrustStore(createEmptyKeyStore());
+
+ assertThrows(WSSecurityException.class,
+ () -> crypto.verifyTrust(new X509Certificate[] {wss40Cert}, false,
null, null));
+ }
+
+ private static KeyStore loadKeyStore(String path) throws Exception {
+ KeyStore keyStore = KeyStore.getInstance("JKS");
+ ClassLoader loader = Loader.getClassLoader(MerlinTrustTest.class);
+ try (InputStream input = Merlin.loadInputStream(loader, path)) {
+ keyStore.load(input, "security".toCharArray());
+ }
+ return keyStore;
+ }
+
+ private static KeyStore createEmptyKeyStore() throws Exception {
+ KeyStore keyStore = KeyStore.getInstance("JKS");
+ keyStore.load(null, null);
+ return keyStore;
+ }
+
+ /**
+ * Wrap a KeyStore so that each enumeration of its aliases is counted.
+ */
+ private static KeyStore countingKeyStore(KeyStore delegate, AtomicInteger
aliasesCount) throws Exception {
+ KeyStore keyStore = new KeyStore(new CountingKeyStoreSpi(delegate,
aliasesCount),
+ delegate.getProvider(),
delegate.getType()) { };
+ keyStore.load(null, null);
+ return keyStore;
+ }
+
+ private static final class CountingKeyStoreSpi extends KeyStoreSpi {
+
+ private final KeyStore delegate;
+ private final AtomicInteger aliasesCount;
+
+ CountingKeyStoreSpi(KeyStore delegate, AtomicInteger aliasesCount) {
+ this.delegate = delegate;
+ this.aliasesCount = aliasesCount;
+ }
+
+ @Override
+ public Enumeration<String> engineAliases() {
+ aliasesCount.incrementAndGet();
+ try {
+ return delegate.aliases();
+ } catch (KeyStoreException e) {
+ return Collections.emptyEnumeration();
+ }
+ }
+
+ @Override
+ public Key engineGetKey(String alias, char[] password) {
+ try {
+ return delegate.getKey(alias, password);
+ } catch (Exception e) {
+ return null;
+ }
+ }
+
+ @Override
+ public Certificate[] engineGetCertificateChain(String alias) {
+ try {
+ return delegate.getCertificateChain(alias);
+ } catch (KeyStoreException e) {
+ return null;
+ }
+ }
+
+ @Override
+ public Certificate engineGetCertificate(String alias) {
+ try {
+ return delegate.getCertificate(alias);
+ } catch (KeyStoreException e) {
+ return null;
+ }
+ }
+
+ @Override
+ public Date engineGetCreationDate(String alias) {
+ try {
+ return delegate.getCreationDate(alias);
+ } catch (KeyStoreException e) {
+ return null;
+ }
+ }
+
+ @Override
+ public void engineSetKeyEntry(String alias, Key key, char[] password,
Certificate[] chain)
+ throws KeyStoreException {
+ delegate.setKeyEntry(alias, key, password, chain);
+ }
+
+ @Override
+ public void engineSetKeyEntry(String alias, byte[] key, Certificate[]
chain) throws KeyStoreException {
+ delegate.setKeyEntry(alias, key, chain);
+ }
+
+ @Override
+ public void engineSetCertificateEntry(String alias, Certificate cert)
throws KeyStoreException {
+ delegate.setCertificateEntry(alias, cert);
+ }
+
+ @Override
+ public void engineDeleteEntry(String alias) throws KeyStoreException {
+ delegate.deleteEntry(alias);
+ }
+
+ @Override
+ public boolean engineContainsAlias(String alias) {
+ try {
+ return delegate.containsAlias(alias);
+ } catch (KeyStoreException e) {
+ return false;
+ }
+ }
+
+ @Override
+ public int engineSize() {
+ try {
+ return delegate.size();
+ } catch (KeyStoreException e) {
+ return 0;
+ }
+ }
+
+ @Override
+ public boolean engineIsKeyEntry(String alias) {
+ try {
+ return delegate.isKeyEntry(alias);
+ } catch (KeyStoreException e) {
+ return false;
+ }
+ }
+
+ @Override
+ public boolean engineIsCertificateEntry(String alias) {
+ try {
+ return delegate.isCertificateEntry(alias);
+ } catch (KeyStoreException e) {
+ return false;
+ }
+ }
+
+ @Override
+ public String engineGetCertificateAlias(Certificate cert) {
+ try {
+ return delegate.getCertificateAlias(cert);
+ } catch (KeyStoreException e) {
+ return null;
+ }
+ }
+
+ @Override
+ public void engineStore(OutputStream stream, char[] password) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void engineLoad(InputStream stream, char[] password) {
+ // The delegate is already loaded
+ }
+ }
+}
