RestClient Reactive - set HostnameVerifier or SSLContext #27901

Chexpir · 2022-09-13T14:59:59Z

Description

There is a known limitation on RestClient Reactive, we cannot set a HostnameVerifier or SSLContext.

We have switched back to rest-client-mutiny for now, even if it's deprecated, but it would be useful to get this done, so we can move back towards a fully reactive service.

Do we know the priority of this feature? At least for HostnameVerifier.

Context: It is very common to give user, when configuring webhooks, the option to use insecure SSL at their own expense, usually for testing purposes, and we want to give that to our users.

Implementation ideas

No response

quarkus-bot · 2022-09-13T15:14:36Z

/cc @Sgitario, @cescoffier, @geoand

famod · 2022-09-13T15:17:44Z

In my current project, for reasons that are not under my or my team's control, we do have to call an external service via IP in production.
So we'd also need HostnameVerifier to switch to rest-client-reactive. I have therefore added the blocking label.

geoand · 2022-09-13T15:21:42Z

@Sgitario would you be willing to look into lifting this restriction?

I can also check if your backlog is too full

geoand · 2022-09-13T15:42:03Z

I should note that it's entirely possible that we will need changes in Vert.x to support this

cescoffier · 2022-09-13T15:43:51Z

Vertx does not provide such a feature. So, I don't believe we can provide that feature. However, you can enable the trustall feature which will by-pass the verification.

cescoffier · 2022-09-13T15:46:05Z

The SSL context approach may work with vertx 4.3.4 (some rework around ssl has been achieved, it should provide such an option)

Chexpir · 2022-09-14T09:12:51Z

Thanks everybody for your prompt responses. If HostnameVerifier support is not achievable, SSLContext support is still good for our use cases.

trust-all operates in the app level, and we have the need to trust only some of the Rest Clients.

When I said "at least HostnameVerifier" I thought it would be easier, but I am totally happy without HostnameVerifier, as SSLContext will perfectly be valid to get different Rest Clients in the same service with and without SSL verification.

Sgitario · 2022-09-14T09:35:46Z

Unfortunately, it's not expected that HostnameVerifier is supported by Vert.x: eclipse-vertx/vert.x#4065.
The alternative they give is to provide a TrustManager (see comment eclipse-vertx/vert.x#4065 (comment)). However, conceptually TrustManager and HostnameVerifier do not work the same.

And the same happens with supporting a custom SSLContext (see related issue vert-x3/vertx-mongo-client#208).

So, our options are:

Allow providing a custom TrustManager. Would this work for you?
And/or map the property verifyHost (it would be something like: quarkus.rest-client.client.verify-host=false). At the moment, we do this when we set quarkus.rest-client.client.trust-all=true). The ugly thing is that this would be a Vert.x specific property...

Wdyt? @geoand @Chexpir

Also, I've found that when using a truststore:

quarkus.rest-client.client.trust-store=META-INF/resources/server.truststore
quarkus.rest-client.client.trust-store-password=password

Makes the rest client to fail with the following exception:

 [client] 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

I have a fix for this: #27925

famod · 2022-09-14T11:11:49Z

My 2 cents:

Allow providing a custom TrustManager.

How would an implemetation look like that tries to mimic io.quarkus.restclient.NoopHostnameVerifier?
As already mentioned, the manager approach would probably not verify much more than just the host?
Could Quarkus provide such a "workaround" manager OOTB?

Also, I assume that with this workaround, Quarkus would still properly set up the KeyManager?
Reason I'm asking is that it's too easy to shoot yourself in the foot when setting up SSLContext, e.g. passing null for the managers by mistake because you only want to set the TLS version (just had that in my project).

To me, the second option looks better for now.

famod · 2022-09-14T11:13:28Z

trust-all operates in the app level, and we have the need to trust only some of the Rest Clients.

+💯

geoand · 2022-09-14T11:18:41Z

I personally think that if we properly document how to address the common concerns, we don't really need to have HostnameVerifier "emulation".

famod · 2022-09-14T11:35:55Z

My point is that it's not that rare to skip hostname verification, so it would be much more developer friendly if Quarkus would close that gap by shipping something that can be used OOTB.
Otherwise people woud need to implement their own Manager and carry it around from project to project to cover a concern that most would expect to be just a flag to enable. Just saying. 😉

geoand · 2022-09-14T11:39:09Z

Sure yeah. My point is that we don't need to necessarily have a HostnameVerifier emulation layer - I think it's acceptable to have something different but equally easily usable.

famod · 2022-09-14T11:41:32Z

My point is that we don't need to necessarily have a HostnameVerifier emulation layer - I think it's acceptable to have something different but equally easily usable.

Agreed!
In the end I just need the client to not verify the hostname (without skipping every other verification), so I don't need such a layer.

Sgitario · 2022-09-14T11:41:54Z

If what we want is to skip the hostname verification, we can achieve this by setting 'verifyHost(false)'. All we would need is to add a new property for setting this flag.

Before these changes, we only can disable the hostname verification in Rest Client classic by providing the following property: ``` quarkus.rest-client.extensions-api.hostname-verifier=io.quarkus.restclient.NoopHostnameVerifier ``` However, this is not working in Rest Client reactive because setting a hostname verifier strategy is not supported by the Vert-x HTTP Client. With these changes, we have added a new property in both Rest Client classic and reactive `quarkus.rest-client.extensions-api.verify-host=true or false`. In Rest Client classic, when disabling the verify host, internally it will add the `NoopHostnameVerifier` strategy. In Rest Client reactive, it will properly configure the Vert.x HTTP client to disable the hostname verification. Therefore, in both Rest Client implementations (classic and reactive), the behaviour is the same. Fix quarkusio#27901

Chexpir · 2022-09-16T07:03:35Z

Thanks everybody for your inputs!

Hi @Sgitario ! I am sure most of the developer use cases will be covered with that, and developer experience using it is very straightforward.

I am unsure if our specific use case is covered, as due to the dynamic base URLs we have, we are forced to use RestClientBuilders programatically.

Before these changes, we only can disable the hostname verification in Rest Client classic by providing the following property: ``` quarkus.rest-client.extensions-api.hostname-verifier=io.quarkus.restclient.NoopHostnameVerifier ``` However, this is not working in Rest Client reactive because setting a hostname verifier strategy is not supported by the Vert-x HTTP Client. With these changes, we have added a new property in both Rest Client classic and reactive `quarkus.rest-client.extensions-api.verify-host=true or false`. In Rest Client classic, when disabling the verify host, internally it will add the `NoopHostnameVerifier` strategy. In Rest Client reactive, it will properly configure the Vert.x HTTP client to disable the hostname verification. Therefore, in both Rest Client implementations (classic and reactive), the behaviour is the same. Fix quarkusio#27901

Before these changes, we only can disable the hostname verification in Rest Client classic by providing the following property: ``` quarkus.rest-client.extensions-api.hostname-verifier=io.quarkus.restclient.NoopHostnameVerifier ``` However, this is not working in Rest Client reactive because setting a hostname verifier strategy is not supported by the Vert-x HTTP Client. With these changes, we have added a new property in both Rest Client classic and reactive `quarkus.rest-client.extensions-api.verify-host=true or false`. In Rest Client classic, when disabling the verify host, internally it will add the `NoopHostnameVerifier` strategy. In Rest Client reactive, it will properly configure the Vert.x HTTP client to disable the hostname verification. Therefore, in both Rest Client implementations (classic and reactive), the behaviour is the same. Fix quarkusio#27901 (cherry picked from commit 941d3a6)

Chexpir added the kind/enhancement New feature or request label Sep 13, 2022

quarkus-bot bot added the triage/needs-triage label Sep 13, 2022

famod added area/rest-client triage/blocking-resteasy-reactive-move and removed triage/needs-triage labels Sep 13, 2022

Sgitario mentioned this issue Sep 15, 2022

Rest Client: Add property to skip hostname verification #27956

Merged

Chexpir mentioned this issue Nov 28, 2022

Rest-client-mutiny issue, exhausting thread pool #29301

Closed

geoand closed this as completed in #27956 Dec 5, 2022

quarkus-bot bot added this to the 2.16 - main milestone Dec 5, 2022

gsmet modified the milestones: 2.16 - main, 2.15.0.Final Dec 6, 2022

polarctos mentioned this issue Apr 26, 2023

Fix resteasy-reactive-client hostname verification default #32932

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RestClient Reactive - set HostnameVerifier or SSLContext #27901

RestClient Reactive - set HostnameVerifier or SSLContext #27901

Chexpir commented Sep 13, 2022 •

edited

Loading

quarkus-bot bot commented Sep 13, 2022

famod commented Sep 13, 2022

geoand commented Sep 13, 2022 •

edited

Loading

geoand commented Sep 13, 2022

cescoffier commented Sep 13, 2022

cescoffier commented Sep 13, 2022

Chexpir commented Sep 14, 2022 •

edited

Loading

Sgitario commented Sep 14, 2022 •

edited

Loading

famod commented Sep 14, 2022 •

edited

Loading

famod commented Sep 14, 2022

geoand commented Sep 14, 2022

famod commented Sep 14, 2022

geoand commented Sep 14, 2022

famod commented Sep 14, 2022

Sgitario commented Sep 14, 2022

Chexpir commented Sep 16, 2022 •

edited

Loading

RestClient Reactive - set HostnameVerifier or SSLContext #27901

RestClient Reactive - set HostnameVerifier or SSLContext #27901

Comments

Chexpir commented Sep 13, 2022 • edited Loading

Description

Implementation ideas

quarkus-bot bot commented Sep 13, 2022

famod commented Sep 13, 2022

geoand commented Sep 13, 2022 • edited Loading

geoand commented Sep 13, 2022

cescoffier commented Sep 13, 2022

cescoffier commented Sep 13, 2022

Chexpir commented Sep 14, 2022 • edited Loading

Sgitario commented Sep 14, 2022 • edited Loading

famod commented Sep 14, 2022 • edited Loading

famod commented Sep 14, 2022

geoand commented Sep 14, 2022

famod commented Sep 14, 2022

geoand commented Sep 14, 2022

famod commented Sep 14, 2022

Sgitario commented Sep 14, 2022

Chexpir commented Sep 16, 2022 • edited Loading

Chexpir commented Sep 13, 2022 •

edited

Loading

geoand commented Sep 13, 2022 •

edited

Loading

Chexpir commented Sep 14, 2022 •

edited

Loading

Sgitario commented Sep 14, 2022 •

edited

Loading

famod commented Sep 14, 2022 •

edited

Loading

Chexpir commented Sep 16, 2022 •

edited

Loading