From ffc57aa74260c859a2bb091315a888e9316cb4b2 Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:43:52 -0400 Subject: [PATCH 1/2] fix: send proxy credentials with the CONNECT request of HTTPS tunnels The Netty client opened CONNECT tunnels without credentials, so an authenticating proxy refused them, and the Proxy-Authorization header of the request was sent to the origin server inside the tunnel. - NettyClientCall: move the Proxy-Authorization header of tunneled exchanges out of the request headers, and return a refusal of the proxy carrying a Proxy-Authenticate challenge as a 407 response, like for a forward proxy. - NettyClientHelper: send the header with the CONNECT request, keep tunnels apart per proxy credentials, and only add the TLS and HTTP handlers once the tunnel is established, so that the refusal of the proxy is reported as is rather than as a TLS handshake failure. - ProxyTestCase: authenticating test proxy, tunnel reuse, 407 challenge. Closes #1569. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Jerome Louvel <374450+jlouvel@users.noreply.github.com> --- changes.md | 4 + .../restlet/engine/netty/NettyClientCall.java | 78 +++++++++ .../engine/netty/NettyClientHelper.java | 78 +++++++-- .../engine/connector/ProxyTestCase.java | 150 +++++++++++++++--- 4 files changed, 279 insertions(+), 31 deletions(-) diff --git a/changes.md b/changes.md index 9de252a92d..3f7022419d 100644 --- a/changes.md +++ b/changes.md @@ -169,6 +169,10 @@ Changes log - Fixed the OpenAPI extension build path for Commons Lang types inherited from Swagger. - Slf4j bridge reports wrong line number and caller class in log4j. Issue #233. - When GET is an allowed method of a server resource, HEAD is now also listed (e.g. in the Allow header of a 405 response). Issue #266. + - The HTTP client sends the proxy credentials of HTTPS requests with the CONNECT request + of the tunnel, instead of sending them to the origin server inside the tunnel. A + refusal of the proxy to open the tunnel with a challenge is returned as a 407 response. + Issue #1569. - The `Max-Age` and `Path` attributes of a cookie setting are no longer quoted. Issue #1334. - `Restlet.handle()` no longer fails with a `NullPointerException` when the Restlet has no context and cannot be started. - `CookieAuthenticator` no longer lets the next restlet overwrite the redirection sent after diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java index 065679a078..96ae33bdf2 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java @@ -24,6 +24,7 @@ import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.HttpVersion; import io.netty.handler.codec.http.LastHttpContent; +import io.netty.handler.proxy.HttpProxyHandler.HttpProxyConnectException; import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; @@ -39,6 +40,7 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Level; import org.restlet.Uniform; +import org.restlet.data.ChallengeResponse; import org.restlet.data.Header; import org.restlet.data.Method; import org.restlet.data.Protocol; @@ -81,6 +83,12 @@ class NettyClientCall { /** Indicates whether the exchange is secured with TLS. */ private final boolean secure; + /** The "Proxy-Authorization" header value for the CONNECT request of a tunnel, if any. */ + private volatile String tunnelProxyAuthorization; + + /** The identity of the tunnel proxy credentials, if any, keeping tunnels apart per user. */ + private volatile String tunnelProxyIdentity; + /** The request headers. */ private final Series
requestHeaders = new Series<>(Header.class); @@ -204,6 +212,51 @@ boolean isConfidential() { return secure; } + /** + * Returns the "Proxy-Authorization" header value to send with the CONNECT request of a new + * tunnel, if any. + * + * @return The header value or null. + */ + String getTunnelProxyAuthorization() { + return tunnelProxyAuthorization; + } + + /** + * Returns the identity of the credentials sent to the tunnel proxy, if any. + * + * @return The identity or null. + */ + String getTunnelProxyIdentity() { + return tunnelProxyIdentity; + } + + /** Indicates whether the exchange goes through a CONNECT tunnel of the HTTP proxy. */ + private boolean isTunneled() { + return secure && (helper.getProxyHost() != null); + } + + /** + * Moves the "Proxy-Authorization" header out of the request headers of a tunneled exchange: the + * proxy only sees the CONNECT request, the request headers go to the origin server. + * + * @param request The Restlet request. + */ + private void moveTunnelProxyAuthorization(org.restlet.Request request) { + String authorization = + requestHeaders.getFirstValue(HeaderConstants.HEADER_PROXY_AUTHORIZATION, true); + if (authorization == null) { + return; + } + requestHeaders.removeAll(HeaderConstants.HEADER_PROXY_AUTHORIZATION, true); + ChallengeResponse credentials = request.getProxyChallengeResponse(); + this.tunnelProxyAuthorization = authorization; + this.tunnelProxyIdentity = + (credentials == null) + ? authorization + : credentials.getScheme().getName() + " " + credentials.getIdentifier(); + } + /** * Sends the request and updates the response. Asynchronous requests invoke their "onSent" and * "onResponse" callbacks once the response is updated. @@ -223,6 +276,9 @@ void handle(org.restlet.Request request, org.restlet.Response response) { // NOTE: This must stay at the end because the AWS challenge scheme requires access to all // HTTP headers HeaderUtils.addRequestHeaders(request, requestHeaders); + if (isTunneled()) { + moveTunnelProxyAuthorization(request); + } Status status = sendRequest(request); updateResponse(response, status); @@ -328,6 +384,8 @@ private Status sendRequest(org.restlet.Request request) { awaitHead(); result = new Status(statusCode, reasonPhrase); finishRequestBody(request, result, backgroundWriterStarted); + } catch (HttpProxyConnectException e) { + result = handleProxyConnectFailure(e); } catch (ConnectException e) { result = handleConnectFailure(e); } catch (IOException e) { @@ -416,6 +474,26 @@ private Status handleConnectFailure(ConnectException cause) { return new Status(Status.CONNECTOR_ERROR_CONNECTION, cause); } + /** + * Handles the refusal of the HTTP proxy to open a CONNECT tunnel. A challenge for proxy + * credentials is returned as a 407 response, like for a forward proxy, so the caller can + * provide or fix the proxy credentials. + */ + private Status handleProxyConnectFailure(HttpProxyConnectException cause) { + HttpHeaders headers = cause.headers(); + if ((headers == null) || !headers.contains(HttpHeaderNames.PROXY_AUTHENTICATE)) { + return handleConnectFailure(cause); + } + + completeEntityRelease(); + helper.logFailure("The HTTP proxy requires authentication to open a tunnel", cause); + abort(); + for (Map.Entry header : headers) { + responseHeaders.add(header.getKey(), header.getValue()); + } + return Status.CLIENT_ERROR_PROXY_AUTHENTIFICATION_REQUIRED; + } + private Status handleCommunicationFailure(IOException cause) { currentRequest.notifyDisconnect(cause); completeEntityRelease(); diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java index 2b45c6e3dc..d8df6c5c2b 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java @@ -23,14 +23,18 @@ import io.netty.channel.nio.NioIoHandler; import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioSocketChannel; +import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.HttpClientCodec; import io.netty.handler.codec.http.HttpContentDecompressor; +import io.netty.handler.codec.http.HttpHeaderNames; +import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http2.Http2FrameCodecBuilder; import io.netty.handler.codec.http2.Http2MultiplexHandler; import io.netty.handler.codec.http2.Http2StreamChannel; import io.netty.handler.codec.http2.Http2StreamChannelBootstrap; import io.netty.handler.codec.http2.Http2StreamFrameToHttpObjectCodec; import io.netty.handler.proxy.HttpProxyHandler; +import io.netty.handler.proxy.ProxyConnectException; import io.netty.handler.ssl.ApplicationProtocolConfig; import io.netty.handler.ssl.ApplicationProtocolNames; import io.netty.handler.ssl.ApplicationProtocolNegotiationHandler; @@ -444,12 +448,15 @@ && getProxyHost() == null) { return acquireHttp3(call); } + // Tunnels opened with different proxy credentials are kept apart. + String proxyIdentity = call.getTunnelProxyIdentity(); OriginPool pool = pools.computeIfAbsent( (call.isConfidential() ? "https://" : "http://") + call.getTargetHost() + ":" - + call.getTargetPort(), + + call.getTargetPort() + + ((proxyIdentity == null) ? "" : "#" + proxyIdentity), k -> new OriginPool( call.isConfidential(), @@ -471,7 +478,7 @@ && getProxyHost() == null) { } if (getTransportMode() == TransportMode.HTTP1_1) { - channel = connect(pool); + channel = connect(pool, call.getTunnelProxyAuthorization()); } else { pool.connectLock.lock(); try { @@ -479,7 +486,7 @@ && getProxyHost() == null) { if (http2 != null && http2.isActive()) { return openStream(http2, call); } - channel = connect(pool); + channel = connect(pool, call.getTunnelProxyAuthorization()); if (channel.pipeline().get(Http2MultiplexHandler.class) != null) { pool.http2 = channel; return openStream(channel, call); @@ -610,7 +617,8 @@ protected void initChannel(Channel stream) { return NettyFutures.await(future, getConnectTimeout(), "Unable to open an HTTP/2 stream"); } - private Channel connect(final OriginPool pool) throws IOException { + private Channel connect(final OriginPool pool, final String proxyAuthorization) + throws IOException { long timeout = getConnectTimeout(); try { if (!pool.permits.tryAcquire(timeout, TimeUnit.MILLISECONDS)) { @@ -643,7 +651,11 @@ private Channel connect(final OriginPool pool) throws IOException { @Override protected void initChannel(SocketChannel ch) { initConnection( - ch, pool, ssl, pool.secure ? proxy : null); + ch, + pool, + ssl, + pool.secure ? proxy : null, + proxyAuthorization); } }); @@ -690,7 +702,11 @@ protected void initChannel(SocketChannel ch) { } private void initConnection( - SocketChannel ch, OriginPool pool, SslContext ssl, InetSocketAddress tunnelProxy) { + SocketChannel ch, + OriginPool pool, + SslContext ssl, + InetSocketAddress tunnelProxy, + String proxyAuthorization) { final Promise ready = ch.eventLoop().newPromise(); ch.attr(READY).set(ready); ch.attr(POOL).set(pool); @@ -704,12 +720,54 @@ private void initConnection( } p.addLast("writability", Writability.HANDLER); - if (tunnelProxy != null) { - HttpProxyHandler proxyHandler = new HttpProxyHandler(tunnelProxy); - proxyHandler.setConnectTimeoutMillis(getConnectTimeout()); - p.addLast("proxy", proxyHandler); + if (tunnelProxy == null) { + initProtocols(ch, pool, ssl, ready); + return; } + HttpHeaders connectHeaders = new DefaultHttpHeaders(); + if (proxyAuthorization != null) { + connectHeaders.set(HttpHeaderNames.PROXY_AUTHORIZATION, proxyAuthorization); + } + HttpProxyHandler proxyHandler = new HttpProxyHandler(tunnelProxy, connectHeaders); + proxyHandler.setConnectTimeoutMillis(getConnectTimeout()); + p.addLast("proxy", proxyHandler); + p.addLast( + "proxy-failure", + new ChannelInboundHandlerAdapter() { + @Override + public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) { + // Already reported with the ready promise. + if (!(cause instanceof ProxyConnectException)) { + ctx.fireExceptionCaught(cause); + } + } + }); + // The protocol handlers are only added once the tunnel is established, so that a refusal of + // the proxy (e.g. a 407 response) is reported as is, not as a TLS handshake failure. + proxyHandler + .connectFuture() + .addListener( + f -> { + if (f.isSuccess()) { + initProtocols(ch, pool, ssl, ready); + } else { + ready.tryFailure(f.cause()); + } + }); + } + + /** + * Adds the TLS and HTTP handlers of a connection, directly connected or tunneled. + * + * @param ch The connection channel. + * @param pool The connection pool of the origin. + * @param ssl The SSL context, or null for a cleartext connection. + * @param ready The promise completed with the negotiated protocol. + */ + private void initProtocols( + SocketChannel ch, OriginPool pool, SslContext ssl, Promise ready) { + ChannelPipeline p = ch.pipeline(); final TransportMode mode = getTransportMode(); if (ssl != null) { SSLEngine engine = ssl.newEngine(ch.alloc(), pool.host, pool.port); diff --git a/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java b/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java index 1ef0fc8f83..54d7743fce 100644 --- a/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java @@ -22,6 +22,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Base64; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutorService; @@ -36,11 +37,15 @@ import org.restlet.Response; import org.restlet.Restlet; import org.restlet.Server; +import org.restlet.data.ChallengeResponse; +import org.restlet.data.ChallengeScheme; import org.restlet.data.MediaType; import org.restlet.data.Method; import org.restlet.data.Protocol; import org.restlet.data.Status; import org.restlet.engine.Engine; +import org.restlet.engine.header.HeaderConstants; +import org.restlet.engine.security.HttpBasicHelper; /** * Tests the HTTP proxy support of the client connector: absolute-form requests for HTTP, and @@ -60,6 +65,7 @@ class ProxyTestCase { void setUp() throws Exception { Engine.clearThreadLocalVariables(); Engine.register(false); + Engine.getInstance().getRegisteredAuthenticators().add(new HttpBasicHelper()); Engine.getInstance().getRegisteredServers().add(new HttpServerHelper(null)); Engine.getInstance().getRegisteredServers().add(new HttpsServerHelper(null)); Engine.getInstance().getRegisteredClients().add(new HttpClientHelper(null)); @@ -87,6 +93,20 @@ public void handle(Request request, Response response) { } }; + /** Returns the "Proxy-Authorization" header received by the origin server, if any. */ + private static final Restlet PROXY_AUTHORIZATION_ECHO = + new Restlet() { + @Override + public void handle(Request request, Response response) { + String authorization = + request.getHeaders() + .getFirstValue( + HeaderConstants.HEADER_PROXY_AUTHORIZATION, true); + response.setEntity( + (authorization == null) ? "none" : authorization, MediaType.TEXT_PLAIN); + } + }; + @Test void forwardsHttpRequestsWithAbsoluteUris() throws Exception { server = new Server(new Context(), Protocol.HTTP, 0, HELLO); @@ -112,27 +132,8 @@ void forwardsHttpRequestsWithAbsoluteUris() throws Exception { @Test void tunnelsHttpsRequestsWithConnect() throws Exception { - Path keystore = tempDir.resolve("dummy.p12"); - try (InputStream in = SslBaseConnectorsTestCase.class.getResourceAsStream("dummy.p12")) { - Files.copy(in, keystore, REPLACE_EXISTING); - } - - Context serverContext = new Context(); - serverContext.getParameters().add("keystorePath", keystore.toString()); - serverContext.getParameters().add("keystorePassword", "testtest"); - serverContext.getParameters().add("keyStoreType", "PKCS12"); - serverContext.getParameters().add("keyPassword", "testtest"); - server = new Server(serverContext, Protocol.HTTPS, 0, HELLO); - server.start(); - - Context context = new Context(); - context.getParameters().add("proxyHost", "localhost"); - context.getParameters().add("proxyPort", Integer.toString(proxy.port())); - context.getParameters().add("truststorePath", keystore.toString()); - context.getParameters().add("truststorePassword", "testtest"); - context.getParameters().add("trustStoreType", "PKCS12"); - client = new Client(context, Protocol.HTTPS); - client.start(); + startHttpsServer(HELLO); + client = newHttpsClient(); Response response = client.handle( @@ -148,6 +149,49 @@ void tunnelsHttpsRequestsWithConnect() throws Exception { proxy.requestLines.getFirst()); } + @Test + void sendsProxyCredentialsWithConnectOnly() throws Exception { + proxy.requiredAuthorization = + "Basic " + + Base64.getEncoder() + .encodeToString("scott:tiger".getBytes(StandardCharsets.UTF_8)); + startHttpsServer(PROXY_AUTHORIZATION_ECHO); + client = newHttpsClient(); + + for (int i = 0; i < 2; i++) { + Request request = + new Request(Method.GET, "https://localhost:" + server.getActualPort() + "/s"); + request.setProxyChallengeResponse( + new ChallengeResponse(ChallengeScheme.HTTP_BASIC, "scott", "tiger")); + Response response = client.handle(request); + assertEquals(Status.SUCCESS_OK, response.getStatus()); + // The origin server never receives the proxy credentials. + assertEquals("none", response.getEntityAsText()); + } + + // The tunnel is reused for the second request. + assertEquals(1, proxy.requestLines.size()); + assertEquals(List.of(proxy.requiredAuthorization), proxy.proxyAuthorizations); + } + + @Test + void returnsProxyChallengeWhenTunnelIsRefused() throws Exception { + proxy.requiredAuthorization = "Basic c2NvdHQ6dGlnZXI="; + startHttpsServer(HELLO); + client = newHttpsClient(); + + Response response = + client.handle( + new Request( + Method.GET, "https://localhost:" + server.getActualPort() + "/s")); + assertEquals(Status.CLIENT_ERROR_PROXY_AUTHENTIFICATION_REQUIRED, response.getStatus()); + assertEquals(1, response.getProxyChallengeRequests().size()); + assertEquals( + ChallengeScheme.HTTP_BASIC, + response.getProxyChallengeRequests().getFirst().getScheme()); + assertEquals("proxy", response.getProxyChallengeRequests().getFirst().getRealm()); + } + @Test void reportsUnreachableProxy() throws Exception { server = new Server(new Context(), Protocol.HTTP, 0, HELLO); @@ -170,6 +214,38 @@ void reportsUnreachableProxy() throws Exception { assertTrue(response.getStatus().isConnectorError(), response.getStatus().toString()); } + private Path keystore() throws IOException { + Path keystore = tempDir.resolve("dummy.p12"); + try (InputStream in = SslBaseConnectorsTestCase.class.getResourceAsStream("dummy.p12")) { + Files.copy(in, keystore, REPLACE_EXISTING); + } + return keystore; + } + + private void startHttpsServer(Restlet next) throws Exception { + Path keystore = keystore(); + Context serverContext = new Context(); + serverContext.getParameters().add("keystorePath", keystore.toString()); + serverContext.getParameters().add("keystorePassword", "testtest"); + serverContext.getParameters().add("keyStoreType", "PKCS12"); + serverContext.getParameters().add("keyPassword", "testtest"); + server = new Server(serverContext, Protocol.HTTPS, 0, next); + server.start(); + } + + private Client newHttpsClient() throws Exception { + Path keystore = keystore(); + Context context = new Context(); + context.getParameters().add("proxyHost", "localhost"); + context.getParameters().add("proxyPort", Integer.toString(proxy.port())); + context.getParameters().add("truststorePath", keystore.toString()); + context.getParameters().add("truststorePassword", "testtest"); + context.getParameters().add("trustStoreType", "PKCS12"); + Client result = new Client(context, Protocol.HTTPS); + result.start(); + return result; + } + /** * Minimal HTTP proxy: forwards absolute-form requests (one connection to the origin per client * connection, bytes relayed both ways) and opens CONNECT tunnels. @@ -178,6 +254,12 @@ private static final class TestProxy implements AutoCloseable { final List requestLines = new CopyOnWriteArrayList<>(); + /** The "Proxy-Authorization" headers received with CONNECT requests. */ + final List proxyAuthorizations = new CopyOnWriteArrayList<>(); + + /** The "Proxy-Authorization" header required to open a tunnel, if any. */ + volatile String requiredAuthorization; + private final ServerSocket serverSocket; private final ExecutorService executor = Executors.newVirtualThreadPerTaskExecutor(); @@ -211,6 +293,21 @@ private void handle(Socket client) { String[] parts = requestLine.split(" "); if ("CONNECT".equals(parts[0])) { + String authorization = headerValue(head, "Proxy-Authorization"); + if (authorization != null) { + proxyAuthorizations.add(authorization); + } + if ((requiredAuthorization != null) + && !requiredAuthorization.equals(authorization)) { + client.getOutputStream() + .write( + ("HTTP/1.1 407 Proxy Authentication Required\r\n" + + "Proxy-Authenticate: Basic realm=\"proxy\"\r\n" + + "Content-Length: 0\r\n\r\n") + .getBytes(StandardCharsets.US_ASCII)); + client.getOutputStream().flush(); + return; + } String[] hostPort = parts[1].split(":"); try (Socket origin = new Socket(hostPort[0], Integer.parseInt(hostPort[1]))) { client.getOutputStream() @@ -277,6 +374,17 @@ private void relay(Socket client, InputStream clientIn, Socket origin) throws IO } } + /** Returns the value of a header in a request head, ignoring case, or null. */ + private static String headerValue(String head, String name) { + for (String line : head.split("\r\n")) { + int colon = line.indexOf(':'); + if ((colon > 0) && line.substring(0, colon).trim().equalsIgnoreCase(name)) { + return line.substring(colon + 1).trim(); + } + } + return null; + } + /** Reads the request head, up to and including the empty line. */ private static String readHead(InputStream in) throws IOException { ByteArrayOutputStream head = new ByteArrayOutputStream(); From c42cc64d90b474edf1fb329d93c8776c30fb27b0 Mon Sep 17 00:00:00 2001 From: Jerome Louvel <374450+jlouvel@users.noreply.github.com> Date: Fri, 30 Oct 2026 16:06:41 -0400 Subject: [PATCH 2/2] fix: isolate proxy tunnels per credentials and share the origin limit Review follow-ups for the CONNECT credentials of HTTPS tunnels. - NettyClientCall: identify the tunnels of proxy credentials with a SHA-256 fingerprint of the scheme, raw value, identifier, secret and realm, so that different tokens or passwords never share a tunnel and the pool keys don't keep credentials in clear. Return only the Proxy-Authenticate challenges of a refused tunnel, not the entity headers of its discarded body. - NettyClientHelper: the pools of an origin share its connection permits, so maxConnectionsPerDestination still bounds the connections across proxy credentials, an idle tunnel of other credentials being closed when needed. The pools of proxy credentials are dropped once they have no connection left. - ProxyTestCase: tunnel per credentials, shared connection limit, no entity headers on a refused tunnel. Co-Authored-By: Claude Opus 5.5 --- .../restlet/engine/netty/NettyClientCall.java | 63 ++++++++-- .../engine/netty/NettyClientHelper.java | 113 ++++++++++++++++-- .../engine/connector/ProxyTestCase.java | 72 ++++++++++- 3 files changed, 226 insertions(+), 22 deletions(-) diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java index 96ae33bdf2..8b98638774 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java @@ -31,6 +31,12 @@ import java.io.InterruptedIOException; import java.io.PushbackInputStream; import java.net.ConnectException; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.HexFormat; import java.util.Locale; import java.util.Map; import java.util.concurrent.CompletableFuture; @@ -249,12 +255,53 @@ private void moveTunnelProxyAuthorization(org.restlet.Request request) { return; } requestHeaders.removeAll(HeaderConstants.HEADER_PROXY_AUTHORIZATION, true); - ChallengeResponse credentials = request.getProxyChallengeResponse(); this.tunnelProxyAuthorization = authorization; - this.tunnelProxyIdentity = - (credentials == null) - ? authorization - : credentials.getScheme().getName() + " " + credentials.getIdentifier(); + this.tunnelProxyIdentity = fingerprint(authorization, request.getProxyChallengeResponse()); + } + + /** + * Returns a fingerprint of the proxy credentials, identifying the tunnels they can reuse. It + * covers the secret or raw value, so that different credentials never share a tunnel, but not + * the values changing with each request (e.g. Digest nonces). It is a SHA-256 hash, so the pool + * keys don't retain the credentials in clear. + * + * @param authorization The "Proxy-Authorization" header value. + * @param credentials The proxy credentials of the request, or null. + * @return The fingerprint. + */ + static String fingerprint(String authorization, ChallengeResponse credentials) { + MessageDigest digest; + try { + digest = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-256 not available", e); + } + + if (credentials == null) { + update(digest, authorization); + } else { + update(digest, credentials.getScheme().getName()); + update(digest, credentials.getRawValue()); + update(digest, credentials.getIdentifier()); + update(digest, credentials.getSecret()); + update(digest, credentials.getRealm()); + } + return HexFormat.of().formatHex(digest.digest()); + } + + private static void update(MessageDigest digest, String value) { + update(digest, (value == null) ? null : value.toCharArray()); + } + + private static void update(MessageDigest digest, char[] value) { + if (value == null) { + digest.update((byte) 0); + } else { + digest.update((byte) 1); + ByteBuffer bytes = StandardCharsets.UTF_8.encode(CharBuffer.wrap(value)); + digest.update(ByteBuffer.allocate(Integer.BYTES).putInt(bytes.remaining()).flip()); + digest.update(bytes); + } } /** @@ -488,8 +535,10 @@ private Status handleProxyConnectFailure(HttpProxyConnectException cause) { completeEntityRelease(); helper.logFailure("The HTTP proxy requires authentication to open a tunnel", cause); abort(); - for (Map.Entry header : headers) { - responseHeaders.add(header.getKey(), header.getValue()); + // Only the challenges are kept: the body of the refusal is discarded, so its entity + // headers would not describe the response. + for (String challenge : headers.getAll(HttpHeaderNames.PROXY_AUTHENTICATE)) { + responseHeaders.add(HeaderConstants.HEADER_PROXY_AUTHENTICATE, challenge); } return Status.CLIENT_ERROR_PROXY_AUTHENTIFICATION_REQUIRED; } diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java index d8df6c5c2b..b72c58fc19 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java @@ -60,6 +60,7 @@ import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Level; import javax.net.ssl.SSLEngine; @@ -218,9 +219,18 @@ protected enum TransportMode { DYNAMIC } - /** Connection pool of an origin. */ + /** + * Connection pool of an origin. The tunnels opened with proxy credentials have a pool per + * credentials, sharing the connection limit of the origin. + */ static final class OriginPool { + /** The key of the pool. */ + final String key; + + /** Indicates whether the pool is specific to proxy credentials, and dropped once unused. */ + final boolean transientPool; + final boolean secure; final String host; @@ -229,8 +239,12 @@ static final class OriginPool { final Deque idle = new ConcurrentLinkedDeque<>(); + /** The connection permits of the origin, shared by its pools. */ final Semaphore permits; + /** The number of connections of the pool, open or being opened. */ + final AtomicInteger connections = new AtomicInteger(); + /** * Serializes connection establishment when HTTP/2 may be negotiated, so that concurrent * first calls share a single HTTP/2 connection instead of opening one each. @@ -239,11 +253,19 @@ static final class OriginPool { volatile Channel http2; - OriginPool(boolean secure, String host, int port, int maxConnections) { + OriginPool( + String key, + boolean transientPool, + boolean secure, + String host, + int port, + Semaphore permits) { + this.key = key; + this.transientPool = transientPool; this.secure = secure; this.host = host; this.port = port; - this.permits = new Semaphore(Math.max(1, maxConnections)); + this.permits = permits; } } @@ -262,9 +284,12 @@ static final class OriginPool { /** The QUIC connections, per origin, for the experimental HTTP/3 support. */ private final Map http3Connections = new ConcurrentHashMap<>(); - /** The connection pools, per origin. */ + /** The connection pools, per origin, and per proxy credentials for tunnels. */ private final Map pools = new ConcurrentHashMap<>(); + /** The connection permits, per origin. */ + private final Map permits = new ConcurrentHashMap<>(); + /** * Constructor. * @@ -449,20 +474,23 @@ && getProxyHost() == null) { } // Tunnels opened with different proxy credentials are kept apart. + String origin = + (call.isConfidential() ? "https://" : "http://") + + call.getTargetHost() + + ":" + + call.getTargetPort(); String proxyIdentity = call.getTunnelProxyIdentity(); OriginPool pool = pools.computeIfAbsent( - (call.isConfidential() ? "https://" : "http://") - + call.getTargetHost() - + ":" - + call.getTargetPort() - + ((proxyIdentity == null) ? "" : "#" + proxyIdentity), + (proxyIdentity == null) ? origin : origin + "#" + proxyIdentity, k -> new OriginPool( + k, + proxyIdentity != null, call.isConfidential(), call.getTargetHost(), call.getTargetPort(), - getMaxConnectionsPerDestination())); + getPermits(origin))); Channel http2 = pool.http2; if (http2 != null && http2.isActive()) { @@ -620,12 +648,15 @@ protected void initChannel(Channel stream) { private Channel connect(final OriginPool pool, final String proxyAuthorization) throws IOException { long timeout = getConnectTimeout(); + pool.connections.incrementAndGet(); try { - if (!pool.permits.tryAcquire(timeout, TimeUnit.MILLISECONDS)) { + if (!acquirePermit(pool, timeout)) { + releaseConnection(pool, false); throw new ConnectException( "Too many connections to " + pool.host + ":" + pool.port); } } catch (InterruptedException _) { + releaseConnection(pool, false); Thread.currentThread().interrupt(); throw new InterruptedIOException("Interrupted while waiting for a connection"); } @@ -683,7 +714,7 @@ protected void initChannel(SocketChannel ch) { timeout, "Unable to connect to " + pool.host + ":" + pool.port); connections.add(channel); - channel.closeFuture().addListener(f -> pool.permits.release()); + channel.closeFuture().addListener(f -> releaseConnection(pool, true)); acquired = false; Promise ready = channel.attr(READY).get(); @@ -696,9 +727,64 @@ protected void initChannel(SocketChannel ch) { return channel; } finally { if (acquired) { - pool.permits.release(); + releaseConnection(pool, true); + } + } + } + + /** + * Returns the connection permits of an origin, shared by its pools. + * + * @param origin The origin. + * @return The connection permits. + */ + private Semaphore getPermits(String origin) { + return permits.computeIfAbsent( + origin, o -> new Semaphore(Math.max(1, getMaxConnectionsPerDestination()))); + } + + /** + * Acquires a connection permit of the origin of a pool. When they are all taken, an idle + * connection of another pool of the origin (opened with other proxy credentials) is closed to + * free one. + * + * @param pool The pool. + * @param timeout The max time to wait, in milliseconds. + * @return True if a permit was acquired. + * @throws InterruptedException If interrupted while waiting. + */ + private boolean acquirePermit(OriginPool pool, long timeout) throws InterruptedException { + if (pool.permits.tryAcquire()) { + return true; + } + + for (OriginPool other : pools.values()) { + if ((other != pool) && (other.permits == pool.permits)) { + Channel idle = other.idle.pollLast(); + if (idle != null) { + idle.close(); + break; + } } } + return pool.permits.tryAcquire(timeout, TimeUnit.MILLISECONDS); + } + + /** + * Releases a connection of a pool, closed or not opened. A pool specific to proxy credentials + * is dropped once it has no connection left, so that such pools do not accumulate. A call + * holding it meanwhile still works, its connection is simply not reused by the next calls. + * + * @param pool The pool. + * @param permit True if the connection held a permit. + */ + private void releaseConnection(OriginPool pool, boolean permit) { + if (permit) { + pool.permits.release(); + } + if ((pool.connections.decrementAndGet() == 0) && pool.transientPool) { + pools.remove(pool.key, pool); + } } private void initConnection( @@ -897,6 +983,7 @@ public synchronized void stop() throws Exception { connections.close().awaitUninterruptibly(5, TimeUnit.SECONDS); } pools.clear(); + permits.clear(); http3Connections.clear(); http3Client = null; if (g != null) { diff --git a/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java b/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java index 54d7743fce..8ba8ddc87e 100644 --- a/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java @@ -10,6 +10,7 @@ import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.ByteArrayOutputStream; @@ -174,6 +175,57 @@ void sendsProxyCredentialsWithConnectOnly() throws Exception { assertEquals(List.of(proxy.requiredAuthorization), proxy.proxyAuthorizations); } + @Test + void keepsTunnelsApartPerProxyCredentials() throws Exception { + startHttpsServer(PROXY_AUTHORIZATION_ECHO); + client = newHttpsClient(); + + List credentials = + List.of( + new ChallengeResponse(ChallengeScheme.HTTP_BASIC, "scott", "tiger"), + new ChallengeResponse(ChallengeScheme.HTTP_BASIC, "scott", "lion"), + rawCredentials("token1"), + rawCredentials("token2"), + new ChallengeResponse(ChallengeScheme.HTTP_BASIC, "scott", "tiger")); + for (ChallengeResponse challengeResponse : credentials) { + Request request = + new Request(Method.GET, "https://localhost:" + server.getActualPort() + "/s"); + request.setProxyChallengeResponse(challengeResponse); + Response response = client.handle(request); + assertEquals(Status.SUCCESS_OK, response.getStatus()); + assertEquals("none", response.getEntityAsText()); + } + + // A tunnel per credentials, the last request reusing the tunnel of the first one. + assertEquals( + List.of( + "Basic c2NvdHQ6dGlnZXI=", + "Basic c2NvdHQ6bGlvbg==", + "Bearer token1", + "Bearer token2"), + proxy.proxyAuthorizations); + } + + @Test + void sharesConnectionLimitAcrossProxyCredentials() throws Exception { + startHttpsServer(PROXY_AUTHORIZATION_ECHO); + client = newHttpsClient("maxConnectionsPerDestination", "1"); + + for (String password : List.of("tiger", "lion", "tiger")) { + Request request = + new Request(Method.GET, "https://localhost:" + server.getActualPort() + "/s"); + request.setProxyChallengeResponse( + new ChallengeResponse(ChallengeScheme.HTTP_BASIC, "scott", password)); + Response response = client.handle(request); + assertEquals(Status.SUCCESS_OK, response.getStatus()); + assertEquals("none", response.getEntityAsText()); + } + + // A single connection at a time: the idle tunnel of the other credentials is closed to open + // a new one, so the first tunnel can't be reused by the third request. + assertEquals(3, proxy.requestLines.size()); + } + @Test void returnsProxyChallengeWhenTunnelIsRefused() throws Exception { proxy.requiredAuthorization = "Basic c2NvdHQ6dGlnZXI="; @@ -190,6 +242,17 @@ void returnsProxyChallengeWhenTunnelIsRefused() throws Exception { ChallengeScheme.HTTP_BASIC, response.getProxyChallengeRequests().getFirst().getScheme()); assertEquals("proxy", response.getProxyChallengeRequests().getFirst().getRealm()); + // The body of the refusal is not returned, nor its entity headers. + assertNull(response.getHeaders().getFirstValue(HeaderConstants.HEADER_CONTENT_TYPE, true)); + assertNull( + response.getHeaders().getFirstValue(HeaderConstants.HEADER_CONTENT_LENGTH, true)); + } + + private static ChallengeResponse rawCredentials(String token) { + ChallengeResponse result = + new ChallengeResponse(new ChallengeScheme("HTTP_BEARER", "Bearer")); + result.setRawValue(token); + return result; } @Test @@ -233,9 +296,12 @@ private void startHttpsServer(Restlet next) throws Exception { server.start(); } - private Client newHttpsClient() throws Exception { + private Client newHttpsClient(String... parameters) throws Exception { Path keystore = keystore(); Context context = new Context(); + for (int i = 0; i < parameters.length; i += 2) { + context.getParameters().add(parameters[i], parameters[i + 1]); + } context.getParameters().add("proxyHost", "localhost"); context.getParameters().add("proxyPort", Integer.toString(proxy.port())); context.getParameters().add("truststorePath", keystore.toString()); @@ -303,7 +369,9 @@ private void handle(Socket client) { .write( ("HTTP/1.1 407 Proxy Authentication Required\r\n" + "Proxy-Authenticate: Basic realm=\"proxy\"\r\n" - + "Content-Length: 0\r\n\r\n") + + "Content-Type: text/plain\r\n" + + "Content-Length: 6\r\n\r\n" + + "denied") .getBytes(StandardCharsets.US_ASCII)); client.getOutputStream().flush(); return;