From a1a51eba2c1432e5704e425150fa519a395858a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 07:40:23 +0000 Subject: [PATCH] fix: make SSL socket hostname verification visible to CodeQL CodeQL's java/unsafe-cert-trust (alert #5) flagged the socket written to in AbstractSocketAppender. Client sockets already verify the server's hostname by default (#479), but the endpoint identification algorithm was set inside SSLConfigurableSocket, on a field, so the query could not see it on the path from ConfigurableSSLSocketFactory.createSocket() to getOutputStream(). ConfigurableSSLSocketFactory now applies hostname verification to the SSLSocket it returns, through SSLConfigurableSocket.applyHostnameVerification(), which returns the socket. SSLParametersConfiguration still makes the decision (and logs it) in configureExceptHostnameVerification(); its public configure() behaves as before. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019BddTdtdSnfEZadKuaxXYz --- .../net/ssl/ConfigurableSSLSocketFactory.java | 18 +++++----- .../core/net/ssl/SSLConfigurableSocket.java | 25 ++++++++++--- .../net/ssl/SSLParametersConfiguration.java | 14 +++++++- .../net/ssl/SSLHostnameVerificationTest.java | 35 +++++++++++++++++++ 4 files changed, 78 insertions(+), 14 deletions(-) diff --git a/logback-android/src/main/java/ch/qos/logback/core/net/ssl/ConfigurableSSLSocketFactory.java b/logback-android/src/main/java/ch/qos/logback/core/net/ssl/ConfigurableSSLSocketFactory.java index 0341fb8d5f..4170ff5160 100644 --- a/logback-android/src/main/java/ch/qos/logback/core/net/ssl/ConfigurableSSLSocketFactory.java +++ b/logback-android/src/main/java/ch/qos/logback/core/net/ssl/ConfigurableSSLSocketFactory.java @@ -62,8 +62,7 @@ public Socket createSocket(InetAddress address, int port, InetAddress localAddress, int localPort) throws IOException { SSLSocket socket = (SSLSocket) delegate.createSocket(address, port, localAddress, localPort); - parameters.configure(new SSLConfigurableSocket(socket)); - return socket; + return configure(socket); } /** @@ -72,8 +71,7 @@ public Socket createSocket(InetAddress address, int port, @Override public Socket createSocket(InetAddress host, int port) throws IOException { SSLSocket socket = (SSLSocket) delegate.createSocket(host, port); - parameters.configure(new SSLConfigurableSocket(socket)); - return socket; + return configure(socket); } /** @@ -84,8 +82,7 @@ public Socket createSocket(String host, int port, InetAddress localHost, int localPort) throws IOException, UnknownHostException { SSLSocket socket = (SSLSocket) delegate.createSocket(host, port, localHost, localPort); - parameters.configure(new SSLConfigurableSocket(socket)); - return socket; + return configure(socket); } /** @@ -95,8 +92,13 @@ public Socket createSocket(String host, int port, InetAddress localHost, public Socket createSocket(String host, int port) throws IOException, UnknownHostException { SSLSocket socket = (SSLSocket) delegate.createSocket(host, port); - parameters.configure(new SSLConfigurableSocket(socket)); - return socket; + return configure(socket); + } + + private SSLSocket configure(SSLSocket socket) { + boolean verifyHostname = parameters.configureExceptHostnameVerification( + new SSLConfigurableSocket(socket)); + return SSLConfigurableSocket.applyHostnameVerification(socket, verifyHostname); } } diff --git a/logback-android/src/main/java/ch/qos/logback/core/net/ssl/SSLConfigurableSocket.java b/logback-android/src/main/java/ch/qos/logback/core/net/ssl/SSLConfigurableSocket.java index 596a9d9c2f..0baeb90f8d 100644 --- a/logback-android/src/main/java/ch/qos/logback/core/net/ssl/SSLConfigurableSocket.java +++ b/logback-android/src/main/java/ch/qos/logback/core/net/ssl/SSLConfigurableSocket.java @@ -76,13 +76,28 @@ public void setWantClientAuth(boolean state) { @Override public void setHostnameVerification(boolean hostnameVerification) { + applyHostnameVerification(delegate, hostnameVerification); + } + + /** + * Makes the socket verify that the server's certificate matches its hostname. + *

+ * This returns the socket so that a caller can return the result: CodeQL's + * java/unsafe-cert-trust query only sees hostname verification when + * {@code setSSLParameters()} is called in the method the socket flows through. + * + * @param socket the socket to configure + * @param hostnameVerification whether to verify the server's hostname + * @return {@code socket} + */ + static SSLSocket applyHostnameVerification(SSLSocket socket, boolean hostnameVerification) { // SSLParameters.setEndpointIdentificationAlgorithm() is API 24+; below // that, SSLParametersConfiguration warns that the hostname is not verified - if (!hostnameVerification || Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { - return; + if (hostnameVerification && Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + SSLParameters sslParameters = socket.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + socket.setSSLParameters(sslParameters); } - SSLParameters sslParameters = delegate.getSSLParameters(); - sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); - delegate.setSSLParameters(sslParameters); + return socket; } } diff --git a/logback-android/src/main/java/ch/qos/logback/core/net/ssl/SSLParametersConfiguration.java b/logback-android/src/main/java/ch/qos/logback/core/net/ssl/SSLParametersConfiguration.java index e542eea402..a9df9ca427 100644 --- a/logback-android/src/main/java/ch/qos/logback/core/net/ssl/SSLParametersConfiguration.java +++ b/logback-android/src/main/java/ch/qos/logback/core/net/ssl/SSLParametersConfiguration.java @@ -50,6 +50,18 @@ public class SSLParametersConfiguration extends ContextAwareBase { * @param socket the subject configurable */ public void configure(SSLConfigurable socket) { + socket.setHostnameVerification(configureExceptHostnameVerification(socket)); + } + + /** + * Configures every SSL parameter but hostname verification on an + * {@link SSLConfigurable}, and decides whether the peer's hostname should be + * verified. {@link ConfigurableSSLSocketFactory} applies that decision to the + * {@link javax.net.ssl.SSLSocket} it returns itself. + * @param socket the subject configurable + * @return whether the peer's hostname should be verified + */ + boolean configureExceptHostnameVerification(SSLConfigurable socket) { socket.setEnabledProtocols(enabledProtocols( socket.getSupportedProtocols(), socket.getDefaultProtocols())); socket.setEnabledCipherSuites(enabledCipherSuites( @@ -66,7 +78,7 @@ public void configure(SSLConfigurable socket) { addWarn("hostnameVerification requires Android 7.0 (API 24) or newer;" + " the peer's hostname will not be verified"); } - socket.setHostnameVerification(verifyHostname); + return verifyHostname; } /** diff --git a/logback-android/src/test/java/ch/qos/logback/core/net/ssl/SSLHostnameVerificationTest.java b/logback-android/src/test/java/ch/qos/logback/core/net/ssl/SSLHostnameVerificationTest.java index e0f5812e96..089f51e444 100644 --- a/logback-android/src/test/java/ch/qos/logback/core/net/ssl/SSLHostnameVerificationTest.java +++ b/logback-android/src/test/java/ch/qos/logback/core/net/ssl/SSLHostnameVerificationTest.java @@ -16,15 +16,22 @@ package ch.qos.logback.core.net.ssl; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.io.IOException; +import java.net.InetAddress; +import java.net.Socket; + import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLServerSocket; import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; import android.os.Build; @@ -87,6 +94,27 @@ public void clientSocketHostnameVerificationCanBeDisabled() { verify(socket, never()).setSSLParameters(any(SSLParameters.class)); } + @Test + @Config(sdk = 29) + public void socketFactoryVerifiesHostnameByDefault() throws IOException { + Socket created = newSocketFactory().createSocket(InetAddress.getLoopbackAddress(), 4560); + + assertSame(socket, created); + ArgumentCaptor captor = ArgumentCaptor.forClass(SSLParameters.class); + verify(socket).setSSLParameters(captor.capture()); + assertEquals("HTTPS", captor.getValue().getEndpointIdentificationAlgorithm()); + } + + @Test + @Config(sdk = 29) + public void socketFactoryHostnameVerificationCanBeDisabled() throws IOException { + configuration.setHostnameVerification(false); + Socket created = newSocketFactory().createSocket("localhost", 4560); + + assertSame(socket, created); + verify(socket, never()).setSSLParameters(any(SSLParameters.class)); + } + @Test @Config(sdk = 29) public void serverSocketDoesNotVerifyHostnameByDefault() { @@ -122,4 +150,11 @@ public void belowApi24HostnameIsNotVerifiedAndAWarningIsLogged() { verify(socket, never()).setSSLParameters(any(SSLParameters.class)); assertEquals(Status.WARN, new StatusUtil(context).getHighestLevel(0)); } + + private ConfigurableSSLSocketFactory newSocketFactory() throws IOException { + SSLSocketFactory delegate = mock(SSLSocketFactory.class); + when(delegate.createSocket(any(InetAddress.class), anyInt())).thenReturn(socket); + when(delegate.createSocket(any(String.class), anyInt())).thenReturn(socket); + return new ConfigurableSSLSocketFactory(configuration, delegate); + } }