From c329c392227eff7263df44cc86a6b12c078b388b Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Thu, 17 Nov 2022 17:50:19 +0100 Subject: [PATCH] HTTPCLIENT-2248: fixed broken TLS over SOCKS --- .../testing/nio/HttpIntegrationTests.java | 2 - .../core5/reactor/InternalConnectChannel.java | 18 ++--- .../reactor/SocksProxyProtocolHandler.java | 73 ++++++++++++------- .../SocksProxyProtocolHandlerFactory.java | 8 +- 4 files changed, 61 insertions(+), 40 deletions(-) diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpIntegrationTests.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpIntegrationTests.java index 768c55b5fd..0132125e8f 100644 --- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpIntegrationTests.java +++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/nio/HttpIntegrationTests.java @@ -28,7 +28,6 @@ package org.apache.hc.core5.testing.nio; import org.apache.hc.core5.http.URIScheme; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; @@ -166,7 +165,6 @@ public CoreTransportH2SocksProxy() { @Nested @DisplayName("Core transport (H2, TLS, SOCKS)") - @Disabled("ALPN via SOCKS is presently broken") public class CoreTransportH2SocksProxyTls extends H2SocksProxyCoreTransportTest { public CoreTransportH2SocksProxyTls() { diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalConnectChannel.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalConnectChannel.java index 700ae0c1e6..e3dcabb8c3 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalConnectChannel.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalConnectChannel.java @@ -73,20 +73,16 @@ void onIOEvent(final int readyOps) throws IOException { final long now = System.currentTimeMillis(); if (checkTimeout(now)) { key.attach(dataChannel); - final IOEventHandler ioEventHandler; if (reactorConfig.getSocksProxyAddress() == null) { - ioEventHandler = eventHandlerFactory.createHandler(dataChannel, sessionRequest.attachment); + dataChannel.upgrade(eventHandlerFactory.createHandler(dataChannel, sessionRequest.attachment)); + sessionRequest.completed(dataChannel); + dataChannel.handleIOEvent(SelectionKey.OP_CONNECT); } else { - final SocksProxyProtocolHandlerFactory eventHandlerFactory = new SocksProxyProtocolHandlerFactory( - sessionRequest.remoteAddress, - this.reactorConfig.getSocksProxyUsername(), - this.reactorConfig.getSocksProxyPassword(), - this.eventHandlerFactory); - ioEventHandler = eventHandlerFactory.createHandler(dataChannel, sessionRequest.attachment); + final IOEventHandler ioEventHandler = new SocksProxyProtocolHandler( + dataChannel, sessionRequest, eventHandlerFactory, reactorConfig); + dataChannel.upgrade(ioEventHandler); + ioEventHandler.connected(dataChannel); } - dataChannel.upgrade(ioEventHandler); - sessionRequest.completed(dataChannel); - dataChannel.handleIOEvent(SelectionKey.OP_CONNECT); } } } diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/SocksProxyProtocolHandler.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/SocksProxyProtocolHandler.java index c9065d2f3f..4e40b2626d 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/SocksProxyProtocolHandler.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/SocksProxyProtocolHandler.java @@ -73,23 +73,21 @@ private enum State { SEND_AUTH, RECEIVE_AUTH_METHOD, SEND_USERNAME_PASSWORD, RECEIVE_AUTH, SEND_CONNECT, RECEIVE_RESPONSE_CODE, RECEIVE_ADDRESS_TYPE, RECEIVE_ADDRESS, COMPLETE } - private final ProtocolIOSession ioSession; - private final Object attachment; - private final InetSocketAddress targetAddress; - private final String username; - private final String password; + private final InternalDataChannel dataChannel; + private final IOSessionRequest sessionRequest; private final IOEventHandlerFactory eventHandlerFactory; + private final IOReactorConfig reactorConfig; private ByteBuffer buffer = ByteBuffer.allocate(512); private State state = State.SEND_AUTH; - SocksProxyProtocolHandler(final ProtocolIOSession ioSession, final Object attachment, final InetSocketAddress targetAddress, - final String username, final String password, final IOEventHandlerFactory eventHandlerFactory) { - this.ioSession = ioSession; - this.attachment = attachment; - this.targetAddress = targetAddress; - this.username = username; - this.password = password; + SocksProxyProtocolHandler(final InternalDataChannel dataChannel, + final IOSessionRequest sessionRequest, + final IOEventHandlerFactory eventHandlerFactory, + final IOReactorConfig reactorConfig) { + this.dataChannel = dataChannel; + this.sessionRequest = sessionRequest; this.eventHandlerFactory = eventHandlerFactory; + this.reactorConfig = reactorConfig; } @Override @@ -134,6 +132,17 @@ public void outputReady(final IOSession session) throws IOException { } } + private byte[] cred(final String cred) throws IOException { + if (cred == null) { + return new byte[] {}; + } + final byte[] bytes = cred.getBytes(StandardCharsets.ISO_8859_1); + if (bytes.length >= 255) { + throw new IOException("SOCKS username / password are too long"); + } + return bytes; + } + @Override public void inputReady(final IOSession session, final ByteBuffer src) throws IOException { if (src != null) { @@ -154,12 +163,14 @@ public void inputReady(final IOSession session, final ByteBuffer src) throws IOE } if (serverMethod == USERNAME_PASSWORD) { this.buffer.clear(); - setBufferLimit(this.username.length() + this.password.length() + 3); + final byte[] username = cred(reactorConfig.getSocksProxyUsername()); + final byte[] password = cred(reactorConfig.getSocksProxyPassword()); + setBufferLimit(username.length + password.length + 3); this.buffer.put(USERNAME_PASSWORD_VERSION); - this.buffer.put((byte) this.username.length()); - this.buffer.put(this.username.getBytes(StandardCharsets.ISO_8859_1)); - this.buffer.put((byte) this.password.length()); - this.buffer.put(this.password.getBytes(StandardCharsets.ISO_8859_1)); + this.buffer.put((byte) username.length); + this.buffer.put(username); + this.buffer.put((byte) password.length); + this.buffer.put(password); session.setEventMask(SelectionKey.OP_WRITE); this.state = State.SEND_USERNAME_PASSWORD; } else if (serverMethod == NO_AUTHENTICATION_REQUIRED) { @@ -250,9 +261,10 @@ public void inputReady(final IOSession session, final ByteBuffer src) throws IOE if (fillBuffer(session)) { this.buffer.clear(); this.state = State.COMPLETE; - final IOEventHandler newHandler = this.eventHandlerFactory.createHandler(this.ioSession, this.attachment); - this.ioSession.upgrade(newHandler); - newHandler.connected(this.ioSession); + final IOEventHandler newHandler = this.eventHandlerFactory.createHandler(dataChannel, sessionRequest.attachment); + dataChannel.upgrade(newHandler); + sessionRequest.completed(dataChannel); + dataChannel.handleIOEvent(SelectionKey.OP_CONNECT); } break; case SEND_AUTH: @@ -271,9 +283,13 @@ private void prepareConnectCommand() throws IOException { this.buffer.put(CLIENT_VERSION); this.buffer.put(COMMAND_CONNECT); this.buffer.put((byte) 0); // reserved - if (this.targetAddress.isUnresolved()) { + if (!(sessionRequest.remoteAddress instanceof InetSocketAddress)) { + throw new IOException("Unsupported address class: " + sessionRequest.remoteAddress.getClass()); + } + final InetSocketAddress targetAddress = ((InetSocketAddress) sessionRequest.remoteAddress); + if (targetAddress.isUnresolved()) { this.buffer.put(ATYP_DOMAINNAME); - final String hostName = this.targetAddress.getHostName(); + final String hostName = targetAddress.getHostName(); final byte[] hostnameBytes = hostName.getBytes(StandardCharsets.US_ASCII); if (hostnameBytes.length > MAX_DNS_NAME_LENGTH) { throw new IOException("Host name exceeds " + MAX_DNS_NAME_LENGTH + " bytes"); @@ -281,7 +297,7 @@ private void prepareConnectCommand() throws IOException { this.buffer.put((byte) hostnameBytes.length); this.buffer.put(hostnameBytes); } else { - final InetAddress address = this.targetAddress.getAddress(); + final InetAddress address = targetAddress.getAddress(); if (address instanceof Inet4Address) { this.buffer.put(InetAddressUtils.IPV4); } else if (address instanceof Inet6Address) { @@ -291,7 +307,7 @@ private void prepareConnectCommand() throws IOException { } this.buffer.put(address.getAddress()); } - final int port = this.targetAddress.getPort(); + final int port = targetAddress.getPort(); this.buffer.putShort((short) port); this.buffer.flip(); } @@ -337,12 +353,17 @@ public void timeout(final IOSession session, final Timeout timeout) throws IOExc @Override public void exception(final IOSession session, final Exception cause) { - session.close(CloseMode.IMMEDIATE); - CommandSupport.failCommands(session, cause); + try { + sessionRequest.failed(cause); + } finally { + session.close(CloseMode.IMMEDIATE); + CommandSupport.failCommands(session, cause); + } } @Override public void disconnected(final IOSession session) { + sessionRequest.cancel(); CommandSupport.cancelCommands(session); } diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/SocksProxyProtocolHandlerFactory.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/SocksProxyProtocolHandlerFactory.java index d2fca8eb7b..8ca7dc240d 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/SocksProxyProtocolHandlerFactory.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/SocksProxyProtocolHandlerFactory.java @@ -31,6 +31,12 @@ import java.net.InetSocketAddress; import java.net.SocketAddress; +/** + * @deprecated Use {@link IOReactorConfig}. + * + * As of version 5.0.1 {@link #createHandler(ProtocolIOSession, Object)} throws {@link UnsupportedOperationException}. + */ +@Deprecated public class SocksProxyProtocolHandlerFactory implements IOEventHandlerFactory { private final InetSocketAddress targetAddress; @@ -51,7 +57,7 @@ public SocksProxyProtocolHandlerFactory(final SocketAddress targetAddress, final @Override public IOEventHandler createHandler(final ProtocolIOSession ioSession, final Object attachment) { - return new SocksProxyProtocolHandler(ioSession, attachment, this.targetAddress, this.username, this.password, this.eventHandlerFactory); + throw new UnsupportedOperationException(); } }