From 63d8e92231bc7c9de5ae1cab5957684f46e5f007 Mon Sep 17 00:00:00 2001 From: Javid Khan Date: Mon, 19 Oct 2026 04:10:07 +0530 Subject: [PATCH] Apply endpoint checking to FTPS data connection setEndpointCheckingEnabled(true) was only applied to the control socket in sslNegotiation; the data socket handshake in _openDataConnection_ ran without endpoint identification. The data socket has no peer host name (it is connected to an address or accepted), so the control host name is also supplied as the SNI server name for the trust manager to check. --- .../apache/commons/net/ftp/FTPSClient.java | 33 +++++++++++++++++-- .../commons/net/ftp/AbstractFtpsTest.java | 5 ++- .../commons/net/ftp/FTPSClientTest.java | 28 ++++++++++++++++ 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/commons/net/ftp/FTPSClient.java b/src/main/java/org/apache/commons/net/ftp/FTPSClient.java index b6e931b1f..26a2f8136 100644 --- a/src/main/java/org/apache/commons/net/ftp/FTPSClient.java +++ b/src/main/java/org/apache/commons/net/ftp/FTPSClient.java @@ -28,12 +28,16 @@ import java.net.ServerSocket; import java.net.Socket; import java.util.Base64; +import java.util.Collections; +import java.util.regex.Pattern; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.KeyManager; +import javax.net.ssl.SNIHostName; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLException; import javax.net.ssl.SSLHandshakeException; +import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.TrustManager; @@ -72,6 +76,9 @@ public class FTPSClient extends FTPClient { /** Default PROT Command */ private static final String DEFAULT_PROT = "C"; + /** Matches an IPv4 address literal, which is not a valid SNI host name. */ + private static final Pattern IPV4_LITERAL = Pattern.compile("\\d{1,3}(\\.\\d{1,3}){3}"); + /** Default secure socket protocol name, i.e. TLS */ private static final String DEFAULT_PROTOCOL = "TLS"; @@ -285,8 +292,12 @@ protected Socket _openDataConnection_(final String command, final String arg) th final SSLSocket sslSocket = (SSLSocket) socket; sslSocket.setUseClientMode(isClientMode); sslSocket.setEnableSessionCreation(isCreation); - // server mode - if (!isClientMode) { + // client mode + if (isClientMode) { + if (tlsEndpointChecking) { + enableEndpointNameVerification(sslSocket); + } + } else { // server mode sslSocket.setNeedClientAuth(isNeedClientAuth); sslSocket.setWantClientAuth(isWantClientAuth); } @@ -1112,6 +1123,24 @@ protected void sslNegotiation() throws IOException { verifyHostName(socket); } + /** + * Enables endpoint identification on a data connection socket. Unlike the control socket, the data socket is not created with the server host name (it + * is connected to an address or accepted), so endpoint identification alone would match the certificate against the peer IP address only. Supplying + * the host name as the SNI server name makes the trust manager check it instead, as it does for the control connection. + */ + private void enableEndpointNameVerification(final SSLSocket sslSocket) { + final SSLParameters sslParameters = sslSocket.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + if (_hostname_ != null && !IPV4_LITERAL.matcher(_hostname_).matches()) { + try { + sslParameters.setServerNames(Collections.singletonList(new SNIHostName(_hostname_))); + } catch (final IllegalArgumentException e) { + // not a valid SNI host name (for example an IPv6 literal): the certificate is matched against the peer address + } + } + sslSocket.setSSLParameters(sslParameters); + } + private void verifyHostName(final SSLSocket socket) throws SSLHandshakeException { if (isClientMode && hostnameVerifier != null && !hostnameVerifier.verify(_hostname_, socket.getSession())) { throw new SSLHandshakeException("Hostname doesn't match certificate"); diff --git a/src/test/java/org/apache/commons/net/ftp/AbstractFtpsTest.java b/src/test/java/org/apache/commons/net/ftp/AbstractFtpsTest.java index 4c1771479..a78990ca0 100644 --- a/src/test/java/org/apache/commons/net/ftp/AbstractFtpsTest.java +++ b/src/test/java/org/apache/commons/net/ftp/AbstractFtpsTest.java @@ -151,8 +151,11 @@ protected void assertClientCode(final FTPSClient client) { } protected FTPSClient loginClient() throws SocketException, IOException { + return loginClient(new FTPSClient(IMPLICIT)); + } + + protected FTPSClient loginClient(final FTPSClient client) throws SocketException, IOException { trace(">>loginClient"); - final FTPSClient client = new FTPSClient(IMPLICIT); if (ADD_LISTENER) { client.addProtocolCommandListener(new PrintCommandListener(System.err)); } diff --git a/src/test/java/org/apache/commons/net/ftp/FTPSClientTest.java b/src/test/java/org/apache/commons/net/ftp/FTPSClientTest.java index 32e8eaaf0..bad71f1a4 100644 --- a/src/test/java/org/apache/commons/net/ftp/FTPSClientTest.java +++ b/src/test/java/org/apache/commons/net/ftp/FTPSClientTest.java @@ -22,12 +22,16 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.net.Socket; import java.net.SocketException; import java.time.Instant; import java.util.Calendar; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; +import javax.net.ssl.SSLSocket; + import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; @@ -97,6 +101,30 @@ void testHostnameVerifierUsedForDataConnection() throws SocketException, IOExcep } } + @Test + @Timeout(TEST_TIMEOUT) + void testEndpointCheckingUsedForDataConnection() throws SocketException, IOException { + setEndpointCheckingEnabled(true); + final AtomicReference algorithm = new AtomicReference<>(); + final FTPSClient client = loginClient(new FTPSClient(IMPLICIT) { + @Override + protected Socket _openDataConnection_(final String command, final String arg) throws IOException { + final Socket socket = super._openDataConnection_(command, arg); + if (socket instanceof SSLSocket) { + algorithm.set(((SSLSocket) socket).getSSLParameters().getEndpointIdentificationAlgorithm()); + } + return socket; + } + }); + try { + // listFiles opens a data connection, which must be verified like the control connection + assertNotNull(client.listFiles("")); + assertEquals("HTTPS", algorithm.get(), "endpoint identification should be enabled on the data connection"); + } finally { + client.disconnect(); + } + } + private void testListFiles(final String pathname) throws SocketException, IOException { final FTPSClient client = loginClient(); try {