From 4094fc37fb69267b202e9a669de97868d89d11fe Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Tue, 21 Jul 2020 14:02:27 +0300 Subject: [PATCH] Fix broken trustStore usage in RestClient Before this change, when a user setup a trustStore or keyStore for REST Client programmatically, then in native mode that setting was being discarded due to how ClientHttpEngineBuilder43 uses the properties. Fixes: #10877 --- .../ClientHttpEngineBuilder43Replacement.java | 3 +- ...sh => generate-trust-store-for-bad-ssl.sh} | 5 +++ integration-tests/rest-client/pom.xml | 15 ++++++-- .../client/wronghost/WrongHostClient.java | 14 ++++++++ .../client/wronghost/WrongHostResource.java | 33 ++++++++++++++++++ .../rest/client/RestClientTestResource.java | 25 +++++++++++++ .../client/wronghost/ExternalWrongHostIT.java | 7 ++++ .../wronghost/ExternalWrongHostTestCase.java | 26 ++++++++++++++ integration-tests/rest-client/wrong-host | Bin 0 -> 2010 bytes 9 files changed, 124 insertions(+), 4 deletions(-) rename integration-tests/rest-client/{generate-trust-store-for-self-signed.sh => generate-trust-store-for-bad-ssl.sh} (73%) create mode 100644 integration-tests/rest-client/src/main/java/io/quarkus/it/rest/client/wronghost/WrongHostClient.java create mode 100644 integration-tests/rest-client/src/main/java/io/quarkus/it/rest/client/wronghost/WrongHostResource.java create mode 100644 integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/RestClientTestResource.java create mode 100644 integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/wronghost/ExternalWrongHostIT.java create mode 100644 integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/wronghost/ExternalWrongHostTestCase.java create mode 100644 integration-tests/rest-client/wrong-host diff --git a/extensions/rest-client/runtime/src/main/java/io/quarkus/restclient/runtime/graal/ClientHttpEngineBuilder43Replacement.java b/extensions/rest-client/runtime/src/main/java/io/quarkus/restclient/runtime/graal/ClientHttpEngineBuilder43Replacement.java index 2005f96f321e1..0fe36db78502b 100644 --- a/extensions/rest-client/runtime/src/main/java/io/quarkus/restclient/runtime/graal/ClientHttpEngineBuilder43Replacement.java +++ b/extensions/rest-client/runtime/src/main/java/io/quarkus/restclient/runtime/graal/ClientHttpEngineBuilder43Replacement.java @@ -24,7 +24,8 @@ public final class ClientHttpEngineBuilder43Replacement { @Substitute public ClientHttpEngineBuilder43Replacement resteasyClientBuilder(ResteasyClientBuilder resteasyClientBuilder) { that = resteasyClientBuilder; - if (that.getSSLContext() == null) { + // make sure we only set a context if there is none or one wouldn't be created implicitly + if ((that.getSSLContext() == null) && (that.getTrustStore() == null) && (that.getKeyStore() == null)) { try { that.sslContext(SSLContext.getDefault()); } catch (NoSuchAlgorithmException e) { diff --git a/integration-tests/rest-client/generate-trust-store-for-self-signed.sh b/integration-tests/rest-client/generate-trust-store-for-bad-ssl.sh similarity index 73% rename from integration-tests/rest-client/generate-trust-store-for-self-signed.sh rename to integration-tests/rest-client/generate-trust-store-for-bad-ssl.sh index a15a1994a3902..7907c571f9c49 100755 --- a/integration-tests/rest-client/generate-trust-store-for-self-signed.sh +++ b/integration-tests/rest-client/generate-trust-store-for-bad-ssl.sh @@ -8,3 +8,8 @@ echo -n | openssl s_client -connect self-signed.badssl.com:443 | sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' > self-signed.cert keytool -importcert -file self-signed.cert -alias self-signed -keystore self-signed -storepass changeit -noprompt rm self-signed.cert + + +echo -n | openssl s_client -connect wrong.host.badssl.com:443 | sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' > wrong-host.cert +keytool -importcert -file wrong-host.cert -alias wrong-host -keystore wrong-host -storepass changeit -noprompt +rm wrong-host.cert diff --git a/integration-tests/rest-client/pom.xml b/integration-tests/rest-client/pom.xml index b271a14d68b0e..9da844ab93625 100644 --- a/integration-tests/rest-client/pom.xml +++ b/integration-tests/rest-client/pom.xml @@ -14,6 +14,10 @@ true + ${project.basedir}/self-signed + changeit + ${project.basedir}/wrong-host + changeit @@ -70,8 +74,10 @@ en - ${project.basedir}/self-signed - changeit + ${self-signed.trust-store} + ${self-signed.trust-store-password} + ${wrong-host.trust-store} + ${wrong-host.trust-store-password} @@ -102,6 +108,8 @@ en ${project.build.directory}/${project.build.finalName}-runner + ${wrong-host.trust-store} + ${wrong-host.trust-store-password} @@ -125,7 +133,8 @@ false false ${graalvmHome} - -J-Djavax.net.ssl.trustStore=${project.basedir}/self-signed, -J-Djavax.net.ssl.trustStorePassword=changeit + -J-Djavax.net.ssl.trustStore=${self-signed.trust-store}, + -J-Djavax.net.ssl.trustStorePassword=${self-signed.trust-store-password} diff --git a/integration-tests/rest-client/src/main/java/io/quarkus/it/rest/client/wronghost/WrongHostClient.java b/integration-tests/rest-client/src/main/java/io/quarkus/it/rest/client/wronghost/WrongHostClient.java new file mode 100644 index 0000000000000..fd4b085347684 --- /dev/null +++ b/integration-tests/rest-client/src/main/java/io/quarkus/it/rest/client/wronghost/WrongHostClient.java @@ -0,0 +1,14 @@ +package io.quarkus.it.rest.client.wronghost; + +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; + +@Path("/") +public interface WrongHostClient { + + @GET + @Produces(MediaType.TEXT_PLAIN) + String root(); +} diff --git a/integration-tests/rest-client/src/main/java/io/quarkus/it/rest/client/wronghost/WrongHostResource.java b/integration-tests/rest-client/src/main/java/io/quarkus/it/rest/client/wronghost/WrongHostResource.java new file mode 100644 index 0000000000000..7e643c53cdcb9 --- /dev/null +++ b/integration-tests/rest-client/src/main/java/io/quarkus/it/rest/client/wronghost/WrongHostResource.java @@ -0,0 +1,33 @@ +package io.quarkus.it.rest.client.wronghost; + +import java.io.FileInputStream; +import java.net.URL; +import java.security.KeyStore; + +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; + +import org.apache.http.conn.ssl.NoopHostnameVerifier; +import org.eclipse.microprofile.rest.client.RestClientBuilder; + +@Path("/wrong-host") +public class WrongHostResource { + + @GET + @Path("/rest-client") + @Produces(MediaType.TEXT_PLAIN) + public String restClient() throws Exception { + KeyStore ks = KeyStore.getInstance("JKS"); + + // the system props are set in pom.xml and made available for native tests via RestClientTestResource + ks.load(new FileInputStream(System.getProperty("rest-client.trustStore")), + System.getProperty("rest-client.trustStorePassword").toCharArray()); + + return RestClientBuilder.newBuilder().baseUrl(new URL("https://wrong.host.badssl.com/")).trustStore(ks) + .hostnameVerifier(NoopHostnameVerifier.INSTANCE) + .build(WrongHostClient.class) + .root(); + } +} diff --git a/integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/RestClientTestResource.java b/integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/RestClientTestResource.java new file mode 100644 index 0000000000000..247c1e301534a --- /dev/null +++ b/integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/RestClientTestResource.java @@ -0,0 +1,25 @@ +package io.quarkus.it.rest.client; + +import java.util.HashMap; +import java.util.Map; + +import io.quarkus.test.common.QuarkusTestResourceLifecycleManager; + +/** + * The only point of this class is to propagate the properties when running the native tests + */ +public class RestClientTestResource implements QuarkusTestResourceLifecycleManager { + + @Override + public Map start() { + Map result = new HashMap<>(); + result.put("rest-client.trustStore", System.getProperty("rest-client.trustStore")); + result.put("rest-client.trustStorePassword", System.getProperty("rest-client.trustStorePassword")); + return result; + } + + @Override + public void stop() { + + } +} diff --git a/integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/wronghost/ExternalWrongHostIT.java b/integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/wronghost/ExternalWrongHostIT.java new file mode 100644 index 0000000000000..8623a0f27b301 --- /dev/null +++ b/integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/wronghost/ExternalWrongHostIT.java @@ -0,0 +1,7 @@ +package io.quarkus.it.rest.client.wronghost; + +import io.quarkus.test.junit.NativeImageTest; + +@NativeImageTest +public class ExternalWrongHostIT extends ExternalWrongHostTestCase { +} diff --git a/integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/wronghost/ExternalWrongHostTestCase.java b/integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/wronghost/ExternalWrongHostTestCase.java new file mode 100644 index 0000000000000..004f73d3cf26f --- /dev/null +++ b/integration-tests/rest-client/src/test/java/io/quarkus/it/rest/client/wronghost/ExternalWrongHostTestCase.java @@ -0,0 +1,26 @@ +package io.quarkus.it.rest.client.wronghost; + +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; + +import org.junit.jupiter.api.Test; + +import io.quarkus.it.rest.client.RestClientTestResource; +import io.quarkus.test.common.QuarkusTestResource; +import io.quarkus.test.junit.QuarkusTest; +import io.restassured.RestAssured; + +@QuarkusTest +@QuarkusTestResource(RestClientTestResource.class) +public class ExternalWrongHostTestCase { + + @Test + public void restClient() { + RestAssured.when() + .get("/wrong-host/rest-client") + .then() + .statusCode(200) + .body(is(not(empty()))); + } +} diff --git a/integration-tests/rest-client/wrong-host b/integration-tests/rest-client/wrong-host new file mode 100644 index 0000000000000000000000000000000000000000..ef49fe435bed177e146682f3a33e6a1b604909cd GIT binary patch literal 2010 zcmV<02POD0f(OuB$(}jES=+^XT!BSd!aNJEN;wvl@m>8e zOaFw2?q3VO4ABZH6+XRVn{DdfOvw1JLn2T+lz z*y1?FJhIk%lpUqAZ=IKTk^9`WED?rO~v`nd|-+-76ZTtfXj8Bzlvr8`-NN7s?b^JTz}^2WW9g0!A9ZaS6x5amf*hftpt zj+K4EC8&vaUg)Z%PkTxEt*DT>T@V917m;A`j9{8|tr1>)i_niRQY)0x29eQn6r z!)gX))RmloXa)Hn}6)<}5md<996$|N5% z!qrLcTEh<5@ft?xQeNZDZ%By+1=KOW8R;#b!9iDc`=hkzbO@%kRHKV>x6bnOwa@aO zEXY#3w-7%Ks=tQX?5WV60}eDBjixiJ=aZfPOnB748wa}Dl#`=CR>MsTps7S_PSoaY zpGu(`^|EyI4J5Lr2Chk0a(9(KuXX*`1&LtBV*qW;w#S7bscN zgRJ$$%97l@HieN;zv(}&Gn3fn-|nX0OKG%kPK>I9J5B8FnQ zdMM=sb)aaHFKR)@;0REC7`qVJ8Udv7Ttu^DWz4Bl-Jz)AxVL9i6u7C+S{Kq?yx@qW z3toJ_l|Aqy6-E;RFrY)w6O!3JaiguvNeI*Qjz`$d+wayh&fz8(EX<}oJ$NH#8Ae~c zoexQSPP?=skcXvYmbz1|-JHHsJE+~!r)fqrtDWGlO4~lZ1Ih^&( zWMl6AKC#%WJVbCEweA=)d(f-bc zfn&b^i;30lV zn0X}QjWrp~fE>6GF{;S|B^a@+y3wgBrEH%BhID%CWLP*KpD*hwhh2ZQSkN|ud9foo zz>1jBTYOnK1$k;<)w=`kpK3Uio8l0oi!5I7$^%yQrwWbk!cxvOHPapKotZUe?V$;E&S8rh72@eZ;fp-Av)v+y-($fB!##xO5t^y$$~AUaS)nd6o+g&exAeE-o(_o@XeSR~vHq zUdYEkyEt-SC$gR6%-5+U0(2AYps|vwO-wYPoX`3uv=lr#@REM=Kvzg%Y^9_v6^WY> zTcx*u_8ybBmBZ>wg)xzX&M~)_7TnBoMaLxExiCI3AutIB1uG5%0vZJX1Qak@-*WxS s-HVxYp?B;r