Skip to content

Commit

Permalink
Resteasy Rest Client: Fix truststore password issue with Vert.x
Browse files Browse the repository at this point in the history
The truststore password was being sent as empty ("") in the JksOptions. This caused the following exception:

```
Caused by: io.vertx.core.VertxException: java.security.UnrecoverableKeyException: Get Key failed: Given final block not properly padded. Such issues can arise if a bad key is used during decryption.
[09:59:27.352] [INFO] [client]  at io.vertx.core.net.impl.SSLHelper.getContext(SSLHelper.java:480)
[09:59:27.353] [INFO] [client]  at io.vertx.core.net.impl.SSLHelper.getContext(SSLHelper.java:469)
[09:59:27.353] [INFO] [client]  at io.vertx.core.net.impl.SSLHelper.validate(SSLHelper.java:507)
[09:59:27.353] [INFO] [client]  at io.vertx.core.net.impl.NetClientImpl.<init>(NetClientImpl.java:95)
[09:59:27.353] [INFO] [client]  at io.vertx.core.http.impl.HttpClientImpl.<init>(HttpClientImpl.java:155)
[09:59:27.354] [INFO] [client]  at io.vertx.core.impl.VertxImpl.createHttpClient(VertxImpl.java:338)
[09:59:27.354] [INFO] [client]  at io.vertx.core.impl.VertxImpl.createHttpClient(VertxImpl.java:350)
[09:59:27.354] [INFO] [client]  at org.jboss.resteasy.reactive.client.impl.ClientImpl.<init>(ClientImpl.java:170)
[09:59:27.354] [INFO] [client]  at org.jboss.resteasy.reactive.client.impl.ClientBuilderImpl.build(ClientBuilderImpl.java:244)
[09:59:27.354] [INFO] [client]  at io.quarkus.rest.client.reactive.runtime.RestClientBuilderImpl.build(RestClientBuilderImpl.java:332)
[09:59:27.355] [INFO] [client]  at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.build(RestClientCDIDelegateBuilder.java:64)
[09:59:27.355] [INFO] [client]  at io.quarkus.rest.client.reactive.runtime.RestClientCDIDelegateBuilder.createDelegate(RestClientCDIDelegateBuilder.java:42)
[09:59:27.355] [INFO] [client]  at io.quarkus.rest.client.reactive.runtime.RestClientReactiveCDIWrapperBase.<init>(RestClientReactiveCDIWrapperBase.java:20)
[09:59:27.355] [INFO] [client]  at io.jester.examples.quarkus.greetings.Client$$CDIWrapper.<init>(Unknown Source)
[09:59:27.356] [INFO] [client]  at io.jester.examples.quarkus.greetings.Client$$CDIWrapper_ClientProxy.<init>(Unknown Source)
[09:59:27.356] [INFO] [client]  at io.jester.examples.quarkus.greetings.Client$$CDIWrapper_Bean.proxy(Unknown Source)
[09:59:27.356] [INFO] [client]  at io.jester.examples.quarkus.greetings.Client$$CDIWrapper_Bean.get(Unknown Source)
[09:59:27.356] [INFO] [client]  at io.jester.examples.quarkus.greetings.Client$$CDIWrapper_Bean.get(Unknown Source)
[09:59:27.357] [INFO] [client]  ... 26 more
[09:59:27.357] [INFO] [client] Caused by: java.security.UnrecoverableKeyException: Get Key failed: Given final block not properly padded. Such issues can arise if a bad key is used during decryption.
[09:59:27.357] [INFO] [client]  at java.base/sun.security.pkcs12.PKCS12KeyStore.engineGetKey(PKCS12KeyStore.java:446)
[09:59:27.357] [INFO] [client]  at java.base/sun.security.util.KeyStoreDelegator.engineGetKey(KeyStoreDelegator.java:90)
[09:59:27.357] [INFO] [client]  at java.base/java.security.KeyStore.getKey(KeyStore.java:1057)
[09:59:27.357] [INFO] [client]  at io.vertx.core.net.impl.KeyStoreHelper.<init>(KeyStoreHelper.java:109)
[09:59:27.358] [INFO] [client]  at io.vertx.core.net.KeyStoreOptionsBase.getHelper(KeyStoreOptionsBase.java:187)
[09:59:27.358] [INFO] [client]  at io.vertx.core.net.KeyStoreOptionsBase.getTrustManagerFactory(KeyStoreOptionsBase.java:217)
[09:59:27.358] [INFO] [client]  at io.vertx.core.net.impl.SSLHelper.getTrustMgrFactory(SSLHelper.java:327)
[09:59:27.358] [INFO] [client]  at io.vertx.core.net.impl.SSLHelper.getContext(SSLHelper.java:478)
[09:59:27.358] [INFO] [client]  ... 43 more
[09:59:27.359] [INFO] [client] Caused by: javax.crypto.BadPaddingException: Given final block not properly padded. Such issues can arise if a bad key is used during decryption.
[09:59:27.359] [INFO] [client]  at java.base/com.sun.crypto.provider.CipherCore.unpad(CipherCore.java:975)
[09:59:27.359] [INFO] [client]  at java.base/com.sun.crypto.provider.CipherCore.fillOutputBuffer(CipherCore.java:1056)
[09:59:27.359] [INFO] [client]  at java.base/com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:853)
[09:59:27.359] [INFO] [client]  at java.base/com.sun.crypto.provider.PKCS12PBECipherCore.implDoFinal(PKCS12PBECipherCore.java:408)
[09:59:27.360] [INFO] [client]  at java.base/com.sun.crypto.provider.PKCS12PBECipherCore$PBEWithSHA1AndDESede.engineDoFinal(PKCS12PBECipherCore.java:440)
[09:59:27.360] [INFO] [client]  at java.base/javax.crypto.Cipher.doFinal(Cipher.java:2202)
[09:59:27.360] [INFO] [client]  at java.base/sun.security.pkcs12.PKCS12KeyStore.lambda$engineGetKey$0(PKCS12KeyStore.java:387)
[09:59:27.360] [INFO] [client]  at java.base/sun.security.pkcs12.PKCS12KeyStore$RetryWithZero.run(PKCS12KeyStore.java:283)
[09:59:27.360] [INFO] [client]  at java.base/sun.security.pkcs12.PKCS12KeyStore.engineGetKey(PKCS12KeyStore.java:381)
[09:59:27.361] [INFO] [client]  ... 50 more
```

(cherry picked from commit b653c64)
  • Loading branch information
Sgitario authored and gsmet committed Sep 20, 2022
1 parent 0c7cd20 commit 3106008
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ public RestClientBuilderImpl trustStore(KeyStore trustStore) {
return this;
}

public RestClientBuilderImpl trustStore(KeyStore trustStore, String trustStorePassword) {
clientBuilder.trustStore(trustStore, trustStorePassword.toCharArray());
return this;
}

@Override
public RestClientBuilderImpl keyStore(KeyStore keyStore, String keystorePassword) {
clientBuilder.keyStore(keyStore, keystorePassword);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ private void configureShared(RestClientBuilder builder) {
}
}

private void configureSsl(RestClientBuilder builder) {
private void configureSsl(RestClientBuilderImpl builder) {

Optional<String> maybeTrustStore = oneOf(clientConfigByClassName().trustStore, clientConfigByConfigKey().trustStore,
configRoot.trustStore);
Expand Down Expand Up @@ -249,7 +249,7 @@ private void registerKeyStore(String keyStorePath, RestClientBuilder builder) {
}
}

private void registerTrustStore(String trustStorePath, RestClientBuilder builder) {
private void registerTrustStore(String trustStorePath, RestClientBuilderImpl builder) {
Optional<String> maybeTrustStorePassword = oneOf(clientConfigByClassName().trustStorePassword,
clientConfigByConfigKey().trustStorePassword, configRoot.trustStorePassword);
Optional<String> maybeTrustStoreType = oneOf(clientConfigByClassName().trustStoreType,
Expand All @@ -269,7 +269,7 @@ private void registerTrustStore(String trustStorePath, RestClientBuilder builder
e);
}

builder.trustStore(trustStore);
builder.trustStore(trustStore, password);
} catch (KeyStoreException e) {
throw new IllegalArgumentException("Failed to initialize trust store from " + trustStorePath, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public void testClientSpecificConfigs() {
Mockito.verify(restClientBuilderMock).register(MyResponseFilter1.class);
Mockito.verify(restClientBuilderMock).queryParamStyle(QueryParamStyle.COMMA_SEPARATED);

Mockito.verify(restClientBuilderMock).trustStore(Mockito.any());
Mockito.verify(restClientBuilderMock).trustStore(Mockito.any(), Mockito.anyString());
Mockito.verify(restClientBuilderMock).keyStore(Mockito.any(), Mockito.anyString());
}

Expand Down Expand Up @@ -151,7 +151,7 @@ public void testGlobalConfigs() {
Mockito.verify(restClientBuilderMock).register(MyResponseFilter2.class);
Mockito.verify(restClientBuilderMock).queryParamStyle(QueryParamStyle.MULTI_PAIRS);

Mockito.verify(restClientBuilderMock).trustStore(Mockito.any());
Mockito.verify(restClientBuilderMock).trustStore(Mockito.any(), Mockito.anyString());
Mockito.verify(restClientBuilderMock).keyStore(Mockito.any(), Mockito.anyString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public class ClientBuilderImpl extends ClientBuilder {
private char[] keystorePassword;
private SSLContext sslContext;
private KeyStore trustStore;
private char[] trustStorePassword;

private String proxyHost;
private int proxyPort;
Expand Down Expand Up @@ -88,7 +89,12 @@ public ClientBuilder keyStore(KeyStore keyStore, char[] password) {

@Override
public ClientBuilder trustStore(KeyStore trustStore) {
return trustStore(trustStore, null);
}

public ClientBuilder trustStore(KeyStore trustStore, char[] password) {
this.trustStore = trustStore;
this.trustStorePassword = password;
return this;
}

Expand Down Expand Up @@ -164,7 +170,7 @@ public ClientBuilder clientLogger(ClientLogger clientLogger) {
@Override
public ClientImpl build() {
Buffer keyStore = asBuffer(this.keyStore, keystorePassword);
Buffer trustStore = asBuffer(this.trustStore, EMPTY_CHAR_ARARAY);
Buffer trustStore = asBuffer(this.trustStore, this.trustStorePassword);

HttpClientOptions options = Optional.ofNullable(configuration.getFromContext(HttpClientOptions.class))
.orElseGet(HttpClientOptions::new);
Expand All @@ -185,7 +191,7 @@ public ClientImpl build() {
if (trustStore != null) {
JksOptions jks = new JksOptions();
jks.setValue(trustStore);
jks.setPassword("");
jks.setPassword(trustStorePassword == null ? "" : new String(trustStorePassword));
options.setTrustStoreOptions(jks);
}
}
Expand Down
30 changes: 30 additions & 0 deletions integration-tests/rest-client-reactive/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
<artifactId>quarkus-integration-test-rest-client-reactive</artifactId>
<name>Quarkus - Integration Tests - REST Client Reactive</name>

<properties>
<self-signed.trust-store>${project.build.directory}/self-signed.p12</self-signed.trust-store>
<self-signed.trust-store-password>changeit</self-signed.trust-store-password>
</properties>

<!--todo add ssl tests-->

<dependencies>
Expand Down Expand Up @@ -165,6 +170,31 @@
</execution>
</executions>
</plugin>

<plugin>
<groupId>uk.co.automatictester</groupId>
<artifactId>truststore-maven-plugin</artifactId>
<version>${truststore-maven-plugin.version}</version>
<executions>
<execution>
<id>self-signed-truststore</id>
<phase>generate-test-resources</phase>
<goals>
<goal>generate-truststore</goal>
</goals>
<configuration>
<truststoreFormat>PKCS12</truststoreFormat>
<truststoreFile>${self-signed.trust-store}</truststoreFile>
<truststorePassword>${self-signed.trust-store-password}</truststorePassword>
<servers>
<server>self-signed.badssl.com:443</server>
</servers>
<trustAllCertificates>true</trustAllCertificates>
<includeCertificates>LEAF</includeCertificates>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter;
import io.quarkus.it.rest.client.main.MyResponseExceptionMapper.MyException;
import io.quarkus.it.rest.client.main.selfsigned.ExternalSelfSignedClient;
import io.smallrye.mutiny.Uni;
import io.vertx.core.Future;
import io.vertx.core.json.Json;
Expand All @@ -44,6 +45,9 @@ public class ClientCallingResource {
@RestClient
FaultToleranceOnInterfaceClient faultToleranceOnInterfaceClient;

@RestClient
ExternalSelfSignedClient externalSelfSignedClient;

@Inject
InMemorySpanExporter inMemorySpanExporter;

Expand Down Expand Up @@ -165,6 +169,9 @@ void init(@Observes Router router) {
});

router.get("/with%20space").handler(rc -> rc.response().setStatusCode(200).end());

router.get("/self-signed").blockingHandler(
rc -> rc.response().setStatusCode(200).end(String.valueOf(externalSelfSignedClient.invoke().getStatus())));
}

private Future<Void> success(RoutingContext rc, String body) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package io.quarkus.it.rest.client.main.selfsigned;

import javax.ws.rs.GET;
import javax.ws.rs.core.Response;

import org.eclipse.microprofile.faulttolerance.Retry;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;

@RegisterRestClient(baseUri = "https://self-signed.badssl.com/", configKey = "self-signed")
public interface ExternalSelfSignedClient {

@GET
@Retry(delay = 1000)
Response invoke();
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
w-exception-mapper/mp-rest/url=${test.url}
w-fault-tolerance/mp-rest/url=${test.url}
io.quarkus.it.rest.client.main.ParamClient/mp-rest/url=${test.url}
io.quarkus.it.rest.client.multipart.MultipartClient/mp-rest/url=${test.url}
io.quarkus.it.rest.client.multipart.MultipartClient/mp-rest/url=${test.url}
# HTTPS
quarkus.rest-client.self-signed.trust-store=${self-signed.trust-store}
quarkus.rest-client.self-signed.trust-store-password=${self-signed.trust-store-password}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package io.quarkus.it.rest.client.selfsigned;

import static io.restassured.RestAssured.when;
import static org.hamcrest.Matchers.is;

import org.junit.jupiter.api.Test;

import io.quarkus.test.junit.QuarkusTest;

@QuarkusTest
public class ExternalSelfSignedTestCase {

@Test
public void should_accept_self_signed_certs() {
when()
.get("/self-signed")
.then()
.statusCode(200)
.body(is("200"));
}
}

0 comments on commit 3106008

Please sign in to comment.