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 0341fb8d5..4170ff516 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 596a9d9c2..0baeb90f8 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 e542eea40..a9df9ca42 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 e0f5812e9..089f51e44 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