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 {