From 7672708750b44f0d13e15889f698b31325836950 Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Wed, 25 Sep 2024 13:26:10 +0800 Subject: [PATCH] fix: allowing the usage of authenticated http proxies for https (6371) fix: allowing the usage of authenticated http proxies for https --- review: remove duplicate proxy init in Jetty Co-authored-by: Steven Hawkins --- .../client/jetty/JettyHttpClientBuilder.java | 25 +++- .../jetty/JettyHttpClientProxyHttpsTest.java | 27 ++++ .../okhttp/OkHttpClientProxyHttpsTest.java | 35 +++++ .../client/okhttp/OkHttpClientProxyTest.java | 3 +- .../client/vertx/VertxHttpClientBuilder.java | 12 +- .../vertx/VertxHttpClientProxyHttpsTest.java | 27 ++++ .../client/utils/HttpClientUtils.java | 18 +++ .../AbstractHttpClientProxyHttpsTest.java | 129 ++++++++++++++++++ .../http/AbstractHttpClientProxyTest.java | 53 ++++++- 9 files changed, 319 insertions(+), 10 deletions(-) create mode 100644 httpclient-jetty/src/test/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientProxyHttpsTest.java create mode 100644 httpclient-okhttp/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientProxyHttpsTest.java create mode 100644 httpclient-vertx/src/test/java/io/fabric8/kubernetes/client/vertx/VertxHttpClientProxyHttpsTest.java create mode 100644 kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractHttpClientProxyHttpsTest.java diff --git a/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientBuilder.java b/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientBuilder.java index 0935ab8a234..ff1effba731 100644 --- a/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientBuilder.java +++ b/httpclient-jetty/src/main/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientBuilder.java @@ -24,19 +24,26 @@ import org.eclipse.jetty.client.HttpProxy; import org.eclipse.jetty.client.Origin; import org.eclipse.jetty.client.Socks4Proxy; +import org.eclipse.jetty.client.Socks5Proxy; +import org.eclipse.jetty.client.api.Authentication; import org.eclipse.jetty.client.dynamic.HttpClientTransportDynamic; import org.eclipse.jetty.client.http.HttpClientConnectionFactory; import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; +import org.eclipse.jetty.client.util.BasicAuthentication; import org.eclipse.jetty.http2.client.HTTP2Client; import org.eclipse.jetty.http2.client.http.ClientConnectionFactoryOverHTTP2; import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.websocket.client.WebSocketClient; +import java.net.URI; +import java.net.URISyntaxException; import java.time.Duration; import java.util.Optional; import java.util.stream.Stream; +import static io.fabric8.kubernetes.client.utils.HttpClientUtils.decodeBasicCredentials; + public class JettyHttpClientBuilder extends StandardHttpClientBuilder { @@ -91,11 +98,25 @@ public JettyHttpClient build() { case SOCKS4: sharedHttpClient.getProxyConfiguration().addProxy(new Socks4Proxy(address, false)); break; + case SOCKS5: + sharedHttpClient.getProxyConfiguration().addProxy(new Socks5Proxy(address, false)); + break; default: throw new KubernetesClientException("Unsupported proxy type"); } - sharedHttpClient.getProxyConfiguration().addProxy(new HttpProxy(address, false)); - addProxyAuthInterceptor(); + final String[] userPassword = decodeBasicCredentials(this.proxyAuthorization); + if (userPassword != null) { + URI proxyUri; + try { + proxyUri = new URI("http://" + proxyAddress.getHostString() + ":" + proxyAddress.getPort()); + } catch (URISyntaxException e) { + throw KubernetesClientException.launderThrowable(e); + } + sharedHttpClient.getAuthenticationStore() + .addAuthentication(new BasicAuthentication(proxyUri, Authentication.ANY_REALM, userPassword[0], userPassword[1])); + } else { + addProxyAuthInterceptor(); + } } clientFactory.additionalConfig(sharedHttpClient, sharedWebSocketClient); return new JettyHttpClient(this, sharedHttpClient, sharedWebSocketClient); diff --git a/httpclient-jetty/src/test/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientProxyHttpsTest.java b/httpclient-jetty/src/test/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientProxyHttpsTest.java new file mode 100644 index 00000000000..230f9f3ce76 --- /dev/null +++ b/httpclient-jetty/src/test/java/io/fabric8/kubernetes/client/jetty/JettyHttpClientProxyHttpsTest.java @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.kubernetes.client.jetty; + +import io.fabric8.kubernetes.client.http.AbstractHttpClientProxyHttpsTest; +import io.fabric8.kubernetes.client.http.HttpClient; + +@SuppressWarnings("java:S2187") +public class JettyHttpClientProxyHttpsTest extends AbstractHttpClientProxyHttpsTest { + @Override + protected HttpClient.Factory getHttpClientFactory() { + return new JettyHttpClientFactory(); + } +} diff --git a/httpclient-okhttp/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientProxyHttpsTest.java b/httpclient-okhttp/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientProxyHttpsTest.java new file mode 100644 index 00000000000..92cb39663e5 --- /dev/null +++ b/httpclient-okhttp/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientProxyHttpsTest.java @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.kubernetes.client.okhttp; + +import io.fabric8.kubernetes.client.http.AbstractHttpClientProxyHttpsTest; +import io.fabric8.kubernetes.client.http.HttpClient; +import okhttp3.OkHttpClient.Builder; + +@SuppressWarnings("java:S2187") +public class OkHttpClientProxyHttpsTest extends AbstractHttpClientProxyHttpsTest { + @Override + protected HttpClient.Factory getHttpClientFactory() { + return new OkHttpClientFactory() { + @Override + protected Builder newOkHttpClientBuilder() { + Builder builder = super.newOkHttpClientBuilder(); + builder.hostnameVerifier((hostname, session) -> true); + return builder; + } + }; + } +} diff --git a/httpclient-okhttp/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientProxyTest.java b/httpclient-okhttp/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientProxyTest.java index 7b76a1e7e81..9972a2db619 100644 --- a/httpclient-okhttp/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientProxyTest.java +++ b/httpclient-okhttp/src/test/java/io/fabric8/kubernetes/client/okhttp/OkHttpClientProxyTest.java @@ -26,8 +26,7 @@ protected HttpClient.Factory getHttpClientFactory() { } @Override - protected void proxyConfigurationAddsRequiredHeaders() { - // NO-OP + protected void proxyConfigurationOtherAuthAddsRequiredHeaders() throws Exception { // OkHttp uses a response intercept to add the auth proxy headers in case the original response failed } } diff --git a/httpclient-vertx/src/main/java/io/fabric8/kubernetes/client/vertx/VertxHttpClientBuilder.java b/httpclient-vertx/src/main/java/io/fabric8/kubernetes/client/vertx/VertxHttpClientBuilder.java index 7ff55ec8d3a..5b6e63e17e2 100644 --- a/httpclient-vertx/src/main/java/io/fabric8/kubernetes/client/vertx/VertxHttpClientBuilder.java +++ b/httpclient-vertx/src/main/java/io/fabric8/kubernetes/client/vertx/VertxHttpClientBuilder.java @@ -36,6 +36,8 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Stream; +import static io.fabric8.kubernetes.client.utils.HttpClientUtils.decodeBasicCredentials; + public class VertxHttpClientBuilder extends StandardHttpClientBuilder, F, VertxHttpClientBuilder> { @@ -74,12 +76,18 @@ public VertxHttpClient build() { } if (this.proxyType != HttpClient.ProxyType.DIRECT && this.proxyAddress != null) { - ProxyOptions proxyOptions = new ProxyOptions() + final ProxyOptions proxyOptions = new ProxyOptions() .setHost(this.proxyAddress.getHostName()) .setPort(this.proxyAddress.getPort()) .setType(convertProxyType()); + final String[] userPassword = decodeBasicCredentials(this.proxyAuthorization); + if (userPassword != null) { + proxyOptions.setUsername(userPassword[0]); + proxyOptions.setPassword(userPassword[1]); + } else { + addProxyAuthInterceptor(); + } options.setProxyOptions(proxyOptions); - addProxyAuthInterceptor(); } final String[] protocols; diff --git a/httpclient-vertx/src/test/java/io/fabric8/kubernetes/client/vertx/VertxHttpClientProxyHttpsTest.java b/httpclient-vertx/src/test/java/io/fabric8/kubernetes/client/vertx/VertxHttpClientProxyHttpsTest.java new file mode 100644 index 00000000000..c93b80b9789 --- /dev/null +++ b/httpclient-vertx/src/test/java/io/fabric8/kubernetes/client/vertx/VertxHttpClientProxyHttpsTest.java @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.kubernetes.client.vertx; + +import io.fabric8.kubernetes.client.http.AbstractHttpClientProxyHttpsTest; +import io.fabric8.kubernetes.client.http.HttpClient; + +@SuppressWarnings("java:S2187") +public class VertxHttpClientProxyHttpsTest extends AbstractHttpClientProxyHttpsTest { + @Override + protected HttpClient.Factory getHttpClientFactory() { + return new VertxHttpClientFactory(); + } +} diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java index d98d71d2ef8..0d27ffdc8a0 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java @@ -132,6 +132,24 @@ public static String basicCredentials(String username, String password) { return basicCredentials(username + ":" + password); } + public static String[] decodeBasicCredentials(String basicCredentials) { + if (basicCredentials == null) { + return null; + } + try { + final String encodedCredentials = basicCredentials.replaceFirst("Basic ", ""); + final String decodedProxyAuthorization = new String(Base64.getDecoder().decode(encodedCredentials), + StandardCharsets.UTF_8); + final String[] userPassword = decodedProxyAuthorization.split(":"); + if (userPassword.length == 2) { + return userPassword; + } + } catch (Exception ignored) { + // Ignored + } + return null; + } + /** * @deprecated you should not need to call this method directly. Please create your own HttpClient.Factory * should you need to customize your clients. diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractHttpClientProxyHttpsTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractHttpClientProxyHttpsTest.java new file mode 100644 index 00000000000..fe7b8c059f7 --- /dev/null +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractHttpClientProxyHttpsTest.java @@ -0,0 +1,129 @@ +/* + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.kubernetes.client.http; + +import io.fabric8.kubernetes.client.internal.SSLUtils; +import io.fabric8.mockwebserver.Context; +import io.fabric8.mockwebserver.DefaultMockServer; +import io.fabric8.mockwebserver.ServerRequest; +import io.fabric8.mockwebserver.ServerResponse; +import io.fabric8.mockwebserver.dsl.HttpMethod; +import io.fabric8.mockwebserver.internal.MockDispatcher; +import io.fabric8.mockwebserver.internal.MockSSLContextFactory; +import io.fabric8.mockwebserver.internal.SimpleRequest; +import io.fabric8.mockwebserver.internal.SimpleResponse; +import io.fabric8.mockwebserver.utils.ResponseProvider; +import okhttp3.Headers; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import okhttp3.mockwebserver.SocketPolicy; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.net.InetSocketAddress; +import java.util.ArrayDeque; +import java.util.HashMap; +import java.util.Map; +import java.util.Queue; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import static io.fabric8.kubernetes.client.utils.HttpClientUtils.basicCredentials; +import static org.assertj.core.api.Assertions.assertThat; + +public abstract class AbstractHttpClientProxyHttpsTest { + + private static SocketPolicy defaultResponseSocketPolicy; + private static Map> responses; + private static DefaultMockServer server; + + @BeforeAll + static void beforeAll() { + defaultResponseSocketPolicy = SocketPolicy.KEEP_OPEN; + responses = new HashMap<>(); + final MockWebServer okHttpMockWebServer = new MockWebServer(); + final MockDispatcher dispatcher = new MockDispatcher(responses) { + @Override + public MockResponse peek() { + return new MockResponse().setSocketPolicy(defaultResponseSocketPolicy); + } + }; + server = new DefaultMockServer(new Context(), okHttpMockWebServer, responses, dispatcher, true); + server.start(); + okHttpMockWebServer.useHttps(MockSSLContextFactory.create().getSocketFactory(), true); + } + + @AfterAll + static void afterAll() { + server.shutdown(); + } + + protected abstract HttpClient.Factory getHttpClientFactory(); + + @Test + @DisplayName("Proxied HttpClient adds required headers to the request") + protected void proxyConfigurationAddsRequiredHeadersForHttps() throws Exception { + final AtomicReference initialConnectRequest = new AtomicReference<>(); + final ResponseProvider bodyProvider = new ResponseProvider() { + + @Override + public String getBody(RecordedRequest request) { + return ""; + } + + @Override + public void setHeaders(Headers headers) { + } + + @Override + public int getStatusCode(RecordedRequest request) { + defaultResponseSocketPolicy = SocketPolicy.UPGRADE_TO_SSL_AT_END; // for jetty to upgrade after the challenge + if (request.getHeader(StandardHttpHeaders.PROXY_AUTHORIZATION) != null) { + initialConnectRequest.compareAndSet(null, request); + return 200; + } + return 407; + } + + @Override + public Headers getHeaders() { + return new Headers.Builder().add("Proxy-Authenticate", "Basic").build(); + } + + }; + responses.computeIfAbsent(new SimpleRequest(HttpMethod.CONNECT, "/"), k -> new ArrayDeque<>()) + .add(new SimpleResponse(true, bodyProvider, null, 0, TimeUnit.SECONDS)); + // Given + final HttpClient.Builder builder = getHttpClientFactory().newBuilder() + .sslContext(null, SSLUtils.trustManagers(null, null, true, null, null)) + .proxyAddress(new InetSocketAddress("localhost", server.getPort())) + .proxyAuthorization(basicCredentials("auth", "cred")); + try (HttpClient client = builder.build()) { + // When + client.sendAsync(client.newHttpRequestBuilder() + .uri(String.format("https://0.0.0.0:%s/not-found", server.getPort() + 1)).build(), String.class) + .get(30, TimeUnit.SECONDS); + + // if it fails, then authorization was not set + assertThat(initialConnectRequest) + .doesNotHaveNullValue() + .hasValueMatching(r -> r.getHeader("Proxy-Authorization").equals("Basic YXV0aDpjcmVk")); + } + } +} diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractHttpClientProxyTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractHttpClientProxyTest.java index d977a097e4b..c04397ad8aa 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractHttpClientProxyTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractHttpClientProxyTest.java @@ -16,6 +16,8 @@ package io.fabric8.kubernetes.client.http; import io.fabric8.mockwebserver.DefaultMockServer; +import io.fabric8.mockwebserver.utils.ResponseProvider; +import okhttp3.Headers; import okhttp3.mockwebserver.RecordedRequest; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -25,6 +27,7 @@ import java.net.InetSocketAddress; import java.util.concurrent.TimeUnit; +import static io.fabric8.kubernetes.client.utils.HttpClientUtils.basicCredentials; import static org.assertj.core.api.Assertions.assertThat; public abstract class AbstractHttpClientProxyTest { @@ -45,12 +48,54 @@ static void afterAll() { protected abstract HttpClient.Factory getHttpClientFactory(); @Test - @DisplayName("Proxied HttpClient adds required headers to the request") - protected void proxyConfigurationAddsRequiredHeaders() throws Exception { + @DisplayName("Proxied HttpClient with basic authorization adds required headers to the request") + protected void proxyConfigurationBasicAuthAddsRequiredHeaders() throws Exception { + server.expect().get().withPath("/").andReply(new ResponseProvider() { + + @Override + public String getBody(RecordedRequest request) { + return "\n"; + } + + @Override + public void setHeaders(Headers headers) { + } + + @Override + public int getStatusCode(RecordedRequest request) { + return request.getHeader(StandardHttpHeaders.PROXY_AUTHORIZATION) != null ? 200 : 407; + } + + @Override + public Headers getHeaders() { + return new Headers.Builder().add("Proxy-Authenticate", "Basic").build(); + } + + }).always(); + // Given + final HttpClient.Builder builder = getHttpClientFactory().newBuilder() + .proxyAddress(new InetSocketAddress("localhost", server.getPort())) + .proxyAuthorization(basicCredentials("auth", "cred")); + try (HttpClient client = builder.build()) { + // When + client.sendAsync(client.newHttpRequestBuilder() + .uri(String.format("http://0.0.0.0:%s/not-found", server.getPort())).build(), String.class) + .get(10L, TimeUnit.SECONDS); + // Then + assertThat(server.getLastRequest()) + .extracting(RecordedRequest::getHeaders) + .returns("0.0.0.0:" + server.getPort(), h -> h.get("Host")) + .returns("Basic YXV0aDpjcmVk", h -> h.get("Proxy-Authorization")); + } + } + + @Test + @DisplayName("Proxied HttpClient with other authorization adds required headers to the request") + protected void proxyConfigurationOtherAuthAddsRequiredHeaders() throws Exception { // Given final HttpClient.Builder builder = getHttpClientFactory().newBuilder() .proxyAddress(new InetSocketAddress("localhost", server.getPort())) - .proxyAuthorization("auth:cred"); + .proxyAuthorization("Other kind of auth"); try (HttpClient client = builder.build()) { // When client.sendAsync(client.newHttpRequestBuilder() @@ -60,7 +105,7 @@ protected void proxyConfigurationAddsRequiredHeaders() throws Exception { assertThat(server.getLastRequest()) .extracting(RecordedRequest::getHeaders) .returns("0.0.0.0:" + server.getPort(), h -> h.get("Host")) - .returns("auth:cred", h -> h.get("Proxy-Authorization")); + .returns("Other kind of auth", h -> h.get("Proxy-Authorization")); } } }