fix: make SSL socket hostname verification visible to CodeQL

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);
   }
 
 }

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.
+   * <p>
+   * 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;
   }
 }

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;
   }
 
   /**

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<SSLParameters> 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);
+  }
 }