Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Injected Quarkus rest client does not use default TLS registry config by default #45149

Closed
querqueq opened this issue Dec 16, 2024 · 4 comments
Closed
Labels
area/config area/rest-client kind/bug Something isn't working triage/wontfix This will not be worked on

Comments

@querqueq
Copy link

querqueq commented Dec 16, 2024

Describe the bug

Injected Quarkus rest client does not trust a HTTP server with TLS with a custom ca although I configured quarkus.tls.trust-store.pem.certs with the custom CA.

Expected behavior

Rest client trust remote server with custom ca.

Actual behavior

Rest client does not trust remote server with custom ca.

How to Reproduce?

I use the following code to reproduce the behaviour. The base URI of MyRemoteService points at some server with a custom CA.

package org.acme;

import java.net.URI;

import org.eclipse.microprofile.rest.client.inject.RestClient;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.quarkus.rest.client.reactive.QuarkusRestClientBuilder;
import io.quarkus.runtime.util.ExceptionUtil;
import io.quarkus.tls.TlsConfiguration;
import io.quarkus.tls.TlsConfigurationRegistry;
import jakarta.inject.Inject;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.WebApplicationException;
import jakarta.ws.rs.core.MediaType;

@Path("/hello")
public class GreetingResource {

	private static final Logger log = LoggerFactory.getLogger(GreetingResource.class);

	private final TlsConfigurationRegistry tlsRegistry;
	private final MyRemoteService injectedClient;

	@Inject
	public GreetingResource(@RestClient MyRemoteService injectedClient,
		TlsConfigurationRegistry tlsRegistry) {
		this.injectedClient = injectedClient;
		this.tlsRegistry = tlsRegistry;
	}

	@GET
	@Produces(MediaType.TEXT_PLAIN)
	public String hello() throws Exception {
		TlsConfiguration cfg = tlsRegistry.getDefault().orElseThrow();

		//Trust works correctly with this client
		MyRemoteService builtClientWithTlsCfg = QuarkusRestClientBuilder.newBuilder()
			.baseUri(URI.create("https://smartapp47.atvieals.local:9002/"))
			.tlsConfiguration(cfg) //explicitly setting TLS config from TLS registry
			.build(MyRemoteService.class);

		//This honours "quarkus.tls.trust-all" but not "quarkus.tls.trust-store.pem.certs"
		MyRemoteService builtClientwithoutTlsCfg = QuarkusRestClientBuilder.newBuilder()
			.baseUri(URI.create("https://smartapp47.atvieals.local:9002/"))
			.build(MyRemoteService.class);

		StringBuilder sb = new StringBuilder();

		//would expect that the injected client uses the trust configured with TLS registry by default
		//when setting "quarkus.rest-client.tls-configuration-name" to "<default>" it works
		request(injectedClient, "injected client", sb);
		request(builtClientWithTlsCfg, "built client with TLS cfg explicitly set", sb);
		request(builtClientwithoutTlsCfg, "built client without TLS cfg explicitly set", sb);

		return sb.toString();
	}

	private void request(MyRemoteService client, String name, StringBuilder sb) {
		sb.append("Request with ").append(name).append(": ");
		try {
			client.someRequest();
			sb.append("OK");
		} catch(WebApplicationException e) {
			sb.append(e.getResponse().getStatus());
		} catch(Exception e) {
			Throwable rootCause = ExceptionUtil.getRootCause(e);
			//SunCertPathBuilderException is not accessible thus checking name
			if("unable to find valid certification path to requested target".equals(rootCause.getMessage())) {
				sb.append("Not trusted");
			} else {
				sb.append("Unexpected error");
			}
			log.error("Request with {} failed", name, e);
		}
		sb.append("\n");
	}
}

Here are the outputs for the respective application.properties:
(Don't mind the 403. It just important that the remote server is being trusted by Quarkus rest client)

Injected client does not trust remote server with default TLS config

quarkus.tls.trust-store.pem.certs=/home/user/cert.pem

application.properties

Request with injected client: Not trusted
Request with built client with TLS cfg explicitly set: OK
Request with built client without TLS cfg explicitly set: Not trusted

output

Injected client trusts remote server when explicitly setting TLS config to <default>

quarkus.tls.trust-store.pem.certs=/home/user/cert.pem
quarkus.rest-client.tls-configuration-name=<default>

application.properties

Request with injected client: OK
Request with built client with TLS cfg explicitly set: OK
Request with built client without TLS cfg explicitly set: Not trusted

output

Output of uname -a or ver

Linux PC 6.11.10-200.fc40.x86_64 #1 SMP PREEMPT_DYNAMIC Sat Nov 23 00:53:13 UTC 2024 x86_64 GNU/Linux

Output of java -version

openjdk version "21.0.5" 2024-10-15

Quarkus version or git rev

3.17.4

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.9

Additional information

I was able to track down the line where the problem originates:

Optional<String> maybeTlsConfigurationName = oneOf(restClientConfig.tlsConfigurationName(),
configRoot.tlsConfigurationName());

Either fix it there or default here to <default>:

static Optional<TlsConfiguration> from(TlsConfigurationRegistry registry, Optional<String> name) {
if (name.isPresent()) {
Optional<TlsConfiguration> maybeConfiguration = registry.get(name.get());
if (maybeConfiguration.isEmpty()) {
throw new IllegalStateException("Unable to find the TLS configuration for name " + name + ".");
}
return maybeConfiguration;
}
return Optional.empty();
}

Workaround

Setting quarkus.rest-client.tls-configuration-name to <default>

@querqueq querqueq added the kind/bug Something isn't working label Dec 16, 2024
Copy link

quarkus-bot bot commented Dec 16, 2024

/cc @cescoffier (rest-client), @geoand (rest-client), @radcortez (config)

@cescoffier
Copy link
Member

For clients, we force the usage of a named configuration not to use the default one, which is often used by the server.
So, this is the desired behavior. It is specified in https://github.com/quarkusio/quarkus/blob/main/adr/0004-using-the-tls-registry-for-clients.adoc#configuring-clients-with-the-tls-registry

@cescoffier cescoffier added the triage/wontfix This will not be worked on label Dec 16, 2024
@geoand
Copy link
Contributor

geoand commented Dec 16, 2024

I agree it's the specified behavior. Had we done something different, we could have broke existing code

@geoand geoand closed this as not planned Won't fix, can't repro, duplicate, stale Dec 16, 2024
@querqueq
Copy link
Author

I see. The configuration documentation states the following:

quarkus.rest-client.tls-configuration-name

The name of the TLS configuration to use.

If not set and the default TLS configuration is configured (quarkus.tls.*) then that will be used. If a name is configured, it uses the configuration from quarkus.tls.<name>.* If a name is configured, but no TLS configuration is found with that name then an error will be thrown.

If no TLS configuration is set, then the keys-tore, trust-store, etc. properties will be used.

I did not set that property but have a default TLS configuration. According to that description I expected that the trust configuration of the default TLS registry would be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config area/rest-client kind/bug Something isn't working triage/wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants