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

Expand certificate support in REST Client #38813

Closed
wants to merge 1 commit into from

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Feb 16, 2024

@geoand geoand changed the title Start expanding supported keystore / truststore types in REST Client Expand certificate support in REST Client Feb 16, 2024
@geoand geoand requested a review from cescoffier February 16, 2024 09:39

This comment has been minimized.

* The classpath path or file path to the corresponding certificate private key file in PEM format.
*/
@ConfigItem
public Optional<String> key;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be a list of files

jks.setValue(keyStore);
jks.setPassword(new String(keystorePassword));
options = options.setKeyStoreOptions(jks);
if (certificate != null && key != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to that code, but I would move the creation of these closer to the vertx http client creation. The reason is that for reloading we need to execute the same logic (re-read from files)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we can certainly do that in a follow up

PemKeyCertOptions pemKeyCertOptions = new PemKeyCertOptions()
.setCertValue(Buffer.buffer(certBytes))
.setKeyValue(Buffer.buffer(keyBytes));
options.setKeyCertOptions(pemKeyCertOptions);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW the trust store can also be a pem file (ca.crt are pem files)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... I just followed what we are already doing for gRPC client

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it's also missing for gRPC :-D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case we would need the same part of keys / certificates, correct? That also means making the names of the properties explicit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, there are 2 aspects:

  • trust store
  • key/cert

On the client side, the trust store is the most essential part, but key/cert is used when using mTLS.

The trust store (basically a list of certs) can be a JKS file (with an alias and password - a single file), a p12 file (with alias and password), or a pem file (list of files, one cert per file)

The key/cert can be:

  • Pem files for the key and the certs - for both, it may be a list, even if in the last majority of the case, it will be one key (the private key) and one cert (the cert)
  • a JKS file or a P12 file with an alias and password

When using pem file, the extension can be .pem, but also .key for the private key, .crt for the certs and trust store.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the gRPC client, I will likely use the following:

package io.quarkus.grpc.runtime.config;

import io.quarkus.runtime.annotations.ConfigGroup;
import io.quarkus.runtime.annotations.ConfigItem;

import java.nio.file.Path;
import java.util.List;
import java.util.Optional;

/**
 * Shared configuration for setting up client-side TLS/SSL.
 */
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
@ConfigGroup
public class SslClientConfig {
    /**
     * The classpath path or file path to a server certificate or certificate chain in PEM format.
     * This parameter is only used when mutual TLS is enabled.
     *
     * @deprecated Use 'certificates' instead
     */
    @ConfigItem
    @Deprecated
    public Optional<Path> certificate;

    /**
     * The classpath paths or file paths to <strong>server</strong> certificates or certificates chain in PEM format.
     * This parameter is only used when mutual TLS is enabled.
     */
    @ConfigItem
    public Optional<List<Path>> certificates;

    /**
     * The classpath path or file path to the corresponding certificate private key file in PEM format.
     * This parameter is only used when mutual TLS is enabled.
     *
     * @deprecated Use 'keys' instead
     */
    @ConfigItem
    @Deprecated
    public Optional<Path> key;

    /**
     * The classpath path or file path to the corresponding certificate private key file in PEM format.
     * This parameter is only used when mutual TLS is enabled.
     * <p>
     * When multiple paths are specified the order must match the order of the certificates.
     */
    @ConfigItem
    public Optional<List<Path>> keys;

    /**
     * An optional trust store which holds the certificate information of the certificates to trust.
     * <p>
     * The classpath path or file path to a JKS or P12 key store. For backward compatibility reasons, it can also be a PEM file.
     * However, in this case, {@link #trustedCertificates} should be used instead.
     * <p>
     * For P12 and JKS, the truststore may contain multiple certificates.
     * If the alias is set, only the one certificate and key pair will be used (matching the alias).
     * Otherwise, SNI will be used (SNI will need to be enabled).
     */
    @ConfigItem
    public Optional<Path> trustStore;

    /**
     * Optional list of paths which hold the information of the certificates to trust in PEM format.
     * <p>
     * The trust store can be either on classpath or in an external file.
     */
    @ConfigItem
    public Optional<List<Path>> trustedCertificates;


    /**
     * The classpath path or file path to a JKS or P12 key store.
     * <p>
     * The key store may contain multiple certificates and keys.
     * If the alias is set, only the one certificate and key pair will be used (matching the alias).
     * Otherwise, SNI will be used (SNI will need to be enabled).
     */
    @ConfigItem
    public Optional<Path> keyStore;

    /**
     * An optional parameter to specify a provider of the keystore file.
     * If not given, the provider is automatically detected based on the keystore file type.
     */
    @ConfigItem
    public Optional<String> keyStoreProvider;

    /**
     * A parameter to specify the password of the keystore file.
     */
    @ConfigItem(defaultValueDocumentation = "password")
    public Optional<String> keyStorePassword;

    /**
     * An optional parameter to select a specific key in the keystore.
     * When SNI is disabled, and the keystore contains multiple
     * keys and no alias is specified; the behavior is undefined.
     */
    @ConfigItem
    public Optional<String> keyStoreAlias;


    /**
     * An optional parameter to specify a provider of the trust store file.
     * If not given, the provider is automatically detected based on the trust store file type.
     */
    @ConfigItem
    public Optional<String> trustStoreProvider;

    /**
     * A parameter to specify the password of the trust store file.
     * <p>
     * Passwords are only supported for PKCS12 and JKS key stores
     */
    @ConfigItem(defaultValueDocumentation = "password")
    public Optional<String> trustStorePassword;


    /**
     * An optional parameter to trust a single certificate from the trust store rather than trusting all certificates in the
     * store.
     * <p>
     * Alias is only supported for PKCS and JKS trust stores.
     */
    @ConfigItem
    public Optional<String> trustStoreAlias;

}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully next week I'll find th e courage to update this PR with that change :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hold on until further instructions :-D

I'm doing an inventory of how to configure TLS in our clients... We have enough approaches to open a museum.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have enough approaches to open a museum

Hahaha

Hold on until further instructions

Will do :)

This comment has been minimized.

This comment has been minimized.

Copy link

quarkus-bot bot commented Feb 19, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 556549b.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@geoand
Copy link
Contributor Author

geoand commented Mar 5, 2024

I'm going to close this and rework the PR (based on what the gRPC client does) after the client rename

@geoand geoand closed this Mar 5, 2024
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The reactive REST client does not support P12, PEM and CRT/KEY files
2 participants