quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

Add support for the TLS registry to the OIDC Common HTTP client

Open cescoffier opened this issue 1 year ago • 2 comments

Description

With the integrated TLS registry, it should be possible to configure TLS using the TLS registry instead of the specific OIDC common configuration.

Implementation ideas

This is the code used for the mailer:

 private void configureTLS(String name, MailerRuntimeConfig config, TlsConfigurationRegistry tlsRegistry, MailConfig cfg,
            boolean globalTrustAll) {
        TlsConfiguration configuration = null;

        // Check if we have a named TLS configuration or a default configuration:
        if (config.tlsConfigurationName.isPresent()) {
            Optional<TlsConfiguration> maybeConfiguration = tlsRegistry.get(config.tlsConfigurationName.get());
            if (!maybeConfiguration.isPresent()) {
                throw new IllegalStateException("Unable to find the TLS configuration "
                        + config.tlsConfigurationName.get() + " for the mailer " + name + ".");
            }
            configuration = maybeConfiguration.get();
        } else if (tlsRegistry.getDefault().isPresent() && tlsRegistry.getDefault().get().isTlsEnabled()) {
            configuration = tlsRegistry.getDefault().get();
        }

       // Apply the configuration
        if (configuration != null) {
            // This part is often the same (or close) for every Vert.x client:
            cfg.setSsl(true);

            if (configuration.getTrustStoreOptions() != null) {
                cfg.setTrustOptions(configuration.getTrustStoreOptions());
            }

           // For mTLS:
            if (configuration.getKeyStoreOptions() != null) {
                cfg.setKeyCertOptions(configuration.getKeyStoreOptions());
            }

            if (configuration.isTrustAll()) {
                cfg.setTrustAll(true);
            }
            if (configuration.getHostnameVerificationAlgorithm().isPresent()) {
               // ACHTUNG HERE - this is protocol specific. The HTTP-based protocols should use HTTPS by default. 
                cfg.setHostnameVerificationAlgorithm(configuration.getHostnameVerificationAlgorithm().get());
            }

            SSLOptions sslOptions = configuration.getSSLOptions();
            if (sslOptions != null) {
                cfg.setSslHandshakeTimeout(sslOptions.getSslHandshakeTimeout());
                cfg.setSslHandshakeTimeoutUnit(sslOptions.getSslHandshakeTimeoutUnit());
                for (String suite : sslOptions.getEnabledCipherSuites()) {
                    cfg.addEnabledCipherSuite(suite);
                }
                for (Buffer buffer : sslOptions.getCrlValues()) {
                    cfg.addCrlValue(buffer);
                }
                cfg.setEnabledSecureTransportProtocols(sslOptions.getEnabledSecureTransportProtocols());

            }

        } else {
           // Mailer specific configuration (very incomplete as you can see:
            boolean trustAll = config.trustAll.isPresent() ? config.trustAll.get() : globalTrustAll;
            cfg.setSsl(config.ssl);
            cfg.setTrustAll(trustAll);
            applyTruststore(config, cfg);
        }
    }

cescoffier avatar Jun 06 '24 06:06 cescoffier

/cc @pedroigor (oidc), @sberyozkin (oidc)

quarkus-bot[bot] avatar Jun 06 '24 06:06 quarkus-bot[bot]

@cescoffier Thank you for driving it, it is an awesome feature. I noticed this request, I'll try to get to it soon. FYI, the OIDC common code related to setting up TLS support is covering 4 cases:

  • OIDC server MTLS authentication against OIDC providers
  • OIDC client MTLS authentication against OIDC providers
  • OIDC server Private Key (extracted from the keystore) JWT authentication against OIDC providers
  • OIDC client Private Key (extracted from the keystore) JWT authentication against OIDC providers

If more than one OIDC tenant is used, then each tenant can have its own specific combination, i.e, keystore properties from for ex keycloak TLS registry bucket won't be applicable to other OIDC tenants.

I'll investigate a bit later how to bind it with TLS registry, thanks

sberyozkin avatar Jun 27 '24 11:06 sberyozkin

@sberyozkin I just spend some time doing QE verification of TLS registry. Would you like me to implement this? If so, I will look later next week and may come with few questions. Thanks

michalvavrik avatar Sep 01 '24 20:09 michalvavrik

I take it as yes @sberyozkin , I'll draft something this week and we can iterate together to get it where you think. Thanks

michalvavrik avatar Sep 02 '24 16:09 michalvavrik

Thanks @michalvavrik

sberyozkin avatar Sep 03 '24 10:09 sberyozkin

Ad tlsRegistry.getDefault().isPresent() && tlsRegistry.getDefault().get().isTlsEnabled() from the issue description. If we were using default TLS registry everywhere if it is enabled, for 4 cases mentioned by Sergey above., it would be confusing users. Probably unintentionally configuring TLS for cases they don't want to. I think we need to use only named TLS config and advise users to explicitly declare io.quarkus.tls.runtime.config.TlsConfig#DEFAULT_NAME if they want to use the default registry. Otherwise we will need a flag everywhere.

Anyway, we don't need to solve this here, I'll write it so that we can make change in one place so we can change this logic easily.

michalvavrik avatar Sep 08 '24 13:09 michalvavrik

@michalvavrik yes, that's what we do for clients - we do not look at the default TLS configuration which is only for "servers". See https://github.com/quarkusio/quarkus/blob/main/adr/0004-using-the-tls-registry-for-clients.adoc

cescoffier avatar Sep 08 '24 14:09 cescoffier

Missed that, thank you.

michalvavrik avatar Sep 08 '24 15:09 michalvavrik