This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/main by this push:
new 1e9d0937bf3 Don't skip hostname verification with a custom trust
manager (#3411)
1e9d0937bf3 is described below
commit 1e9d0937bf3975aea4a732dd19ff284c5d3b3f35
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Mon Aug 31 10:11:53 2026 +0100
Don't skip hostname verification with a custom trust manager (#3411)
---
.../org/apache/cxf/transport/https/SSLUtils.java | 54 ++++++++++++------
.../apache/cxf/transport/https/SSLUtilsTest.java | 64 ++++++++++++++++++++++
2 files changed, 100 insertions(+), 18 deletions(-)
diff --git
a/rt/transports/http/src/main/java/org/apache/cxf/transport/https/SSLUtils.java
b/rt/transports/http/src/main/java/org/apache/cxf/transport/https/SSLUtils.java
index 970f9382710..f1085557b8a 100644
---
a/rt/transports/http/src/main/java/org/apache/cxf/transport/https/SSLUtils.java
+++
b/rt/transports/http/src/main/java/org/apache/cxf/transport/https/SSLUtils.java
@@ -45,6 +45,7 @@ import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLPeerUnverifiedException;
import javax.net.ssl.SSLSession;
import javax.net.ssl.SSLSessionContext;
+import javax.net.ssl.SSLSocket;
import javax.net.ssl.StandardConstants;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509ExtendedTrustManager;
@@ -245,6 +246,11 @@ public final class SSLUtils {
} else {
delegate.checkServerTrusted(chain, s);
}
+ // certificates are valid, now check the hostname regardless of the
+ // delegate's type - see the SSLEngine overload below
+ if (socket instanceof SSLSocket) {
+ verifyPeerHostname(chain,
((SSLSocket)socket).getHandshakeSession());
+ }
}
private String getHostName(List<SNIServerName> names) {
@@ -268,27 +274,39 @@ public final class SSLUtils {
throws CertificateException {
if (extendedDelegate != null) {
extendedDelegate.checkServerTrusted(chain, s, new
SSLEngineWrapper(engine));
- //certificates are valid, now check hostnames
- SSLSession session = engine.getHandshakeSession();
- List<SNIServerName> names = null;
- if (session instanceof ExtendedSSLSession) {
- ExtendedSSLSession extSession =
(ExtendedSSLSession)session;
- names = extSession.getRequestedServerNames();
- }
-
- boolean identifiable = false;
- String peerHost = session.getPeerHost();
- String hostname = getHostName(names);
- session = new SSLSessionWrapper(session, chain);
- if (hostname != null && verifier.verify(hostname, session)) {
- identifiable = true;
- }
- if (!identifiable && !verifier.verify(peerHost, session)) {
- throw new CertificateException("No name matching " +
peerHost + " found");
- }
} else {
delegate.checkServerTrusted(chain, s);
}
+ // certificates are valid, now check the hostname. This must run
regardless
+ // of the delegate's type: JSSE endpoint identification is
deliberately
+ // suppressed (SSLEngineWrapper.getSSLParameters), so if the
verifier were
+ // skipped for a plain X509TrustManager delegate no hostname check
would
+ // happen anywhere and any certificate the delegate trusts would
enable MITM.
+ verifyPeerHostname(chain, engine.getHandshakeSession());
+ }
+
+ private void verifyPeerHostname(X509Certificate[] chain, SSLSession
session)
+ throws CertificateException {
+ if (session == null) {
+ throw new CertificateException(
+ "No handshake session available to verify the peer
hostname");
+ }
+ List<SNIServerName> names = null;
+ if (session instanceof ExtendedSSLSession) {
+ ExtendedSSLSession extSession = (ExtendedSSLSession)session;
+ names = extSession.getRequestedServerNames();
+ }
+
+ boolean identifiable = false;
+ String peerHost = session.getPeerHost();
+ String hostname = getHostName(names);
+ SSLSession wrappedSession = new SSLSessionWrapper(session, chain);
+ if (hostname != null && verifier.verify(hostname, wrappedSession))
{
+ identifiable = true;
+ }
+ if (!identifiable && !verifier.verify(peerHost, wrappedSession)) {
+ throw new CertificateException("No name matching " + peerHost
+ " found");
+ }
}
@Override
diff --git
a/rt/transports/http/src/test/java/org/apache/cxf/transport/https/SSLUtilsTest.java
b/rt/transports/http/src/test/java/org/apache/cxf/transport/https/SSLUtilsTest.java
index 66a1cc27867..4c56afd0f50 100644
---
a/rt/transports/http/src/test/java/org/apache/cxf/transport/https/SSLUtilsTest.java
+++
b/rt/transports/http/src/test/java/org/apache/cxf/transport/https/SSLUtilsTest.java
@@ -20,11 +20,18 @@
package org.apache.cxf.transport.https;
import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.util.Collections;
+import javax.net.ssl.ExtendedSSLSession;
+import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
+import javax.net.ssl.X509TrustManager;
import org.apache.cxf.transport.https.SSLUtils.SSLEngineWrapper;
+import org.apache.cxf.transport.https.SSLUtils.X509TrustManagerWrapper;
import org.junit.After;
import org.junit.Before;
@@ -34,6 +41,13 @@ import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
public class SSLUtilsTest {
@@ -61,4 +75,54 @@ public class SSLUtilsTest {
assertThat(wrapper.getSSLParameters(), is(not(nullValue())));
}
+
+ /**
+ * Regression test: a plain (non-extended) X509TrustManager delegate must
still get
+ * hostname verification. JSSE endpoint identification is suppressed by
+ * SSLEngineWrapper, so skipping the CXF HostnameVerifier for plain
delegates left
+ * no hostname check at all - any certificate the trust manager accepted
(e.g. any
+ * cert from a pinned corporate CA, issued for any host) enabled silent
MITM.
+ */
+ @Test
+ public void testPlainTrustManagerStillGetsHostnameVerification() throws
Exception {
+ X509TrustManager plainTrustManager = mock(X509TrustManager.class);
+ HostnameVerifier failingVerifier = mock(HostnameVerifier.class);
+ when(failingVerifier.verify(anyString(), any())).thenReturn(false);
+
+ ExtendedSSLSession session = mock(ExtendedSSLSession.class);
+ when(session.getPeerHost()).thenReturn("evil.example.net");
+
when(session.getRequestedServerNames()).thenReturn(Collections.emptyList());
+ SSLEngine mockEngine = mock(SSLEngine.class);
+ when(mockEngine.getHandshakeSession()).thenReturn(session);
+
+ X509TrustManagerWrapper wrapper =
+ new X509TrustManagerWrapper(plainTrustManager, failingVerifier);
+ X509Certificate[] chain = new X509Certificate[0];
+ try {
+ wrapper.checkServerTrusted(chain, "RSA", mockEngine);
+ fail("hostname verification must run for plain X509TrustManager
delegates");
+ } catch (CertificateException expected) {
+ // expected: no name matching evil.example.net
+ }
+ // the delegate's chain validation was still consulted
+ verify(plainTrustManager).checkServerTrusted(chain, "RSA");
+ }
+
+ @Test
+ public void testPlainTrustManagerAcceptedWhenHostnameMatches() throws
Exception {
+ X509TrustManager plainTrustManager = mock(X509TrustManager.class);
+ HostnameVerifier passingVerifier = mock(HostnameVerifier.class);
+ when(passingVerifier.verify(anyString(), any())).thenReturn(true);
+
+ ExtendedSSLSession session = mock(ExtendedSSLSession.class);
+ when(session.getPeerHost()).thenReturn("service.example.com");
+
when(session.getRequestedServerNames()).thenReturn(Collections.emptyList());
+ SSLEngine mockEngine = mock(SSLEngine.class);
+ when(mockEngine.getHandshakeSession()).thenReturn(session);
+
+ X509TrustManagerWrapper wrapper =
+ new X509TrustManagerWrapper(plainTrustManager, passingVerifier);
+ wrapper.checkServerTrusted(new X509Certificate[0], "RSA", mockEngine);
+ verify(passingVerifier).verify(eq("service.example.com"), any());
+ }
}
