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.

Sign up for GitHub

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

Integrate Vertx HTTP with CredentialsProvider #26380

Merged
merged 1 commit into from
Jul 6, 2022
Merged

Integrate Vertx HTTP with CredentialsProvider #26380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions extensions/vertx-http/runtime/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
<groupId>io.quarkus</groupId>
<artifactId>quarkus-security-runtime-spi</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-credentials</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-mutiny</artifactId>
Expand Down
72 changes: 68 additions & 4 deletions ...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/CertificateConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import io.quarkus.runtime.annotations.ConfigGroup;
import io.quarkus.runtime.annotations.ConfigItem;
import io.quarkus.runtime.annotations.ConvertWith;
import io.quarkus.runtime.configuration.TrimmedStringConverter;

/**
* A certificate configuration. Either the certificate and key files must be given, or a key store must be given.
Expand All @@ -14,6 +16,30 @@
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
public class CertificateConfig {

/**
* The {@linkplain CredentialsProvider}}.
* If this property is configured then a matching 'CredentialsProvider' will be used
* to get the keystore, keystore key and truststore passwords unless these passwords have already been configured.
*
* Please note that using MicroProfile {@linkplain ConfigSource} which is directly supported by Quarkus Configuration
* should be preferred unless using `CredentialsProvider` provides for some additional security and dynamism.
*/
@ConfigItem
@ConvertWith(TrimmedStringConverter.class)
public Optional<String> credentialsProvider = Optional.empty();

/**
* The credentials provider bean name.
* <p>
* It is the {@code &#64;Named} value of the credentials provider bean. It is used to discriminate if multiple
* CredentialsProvider beans are available.
* It is recommended to set this property even if there is only one credentials provider currently available
* to ensure the same provider is always found in deployments where more than one provider may be available.
*/
@ConfigItem
@ConvertWith(TrimmedStringConverter.class)
public Optional<String> credentialsProviderName = Optional.empty();

/**
* The file path to a server certificate or certificate chain in PEM format.
*
Expand Down Expand Up @@ -69,10 +95,23 @@ public class CertificateConfig {
public Optional<String> keyStoreProvider;

/**
* A parameter to specify the password of the key store file. If not given, the default ("password") is used.
* A parameter to specify the password of the key store file. If not given, and if it can not be retrieved from
* {@linkplain CredentialsProvider}, then the default ("password") is used.
*
* @see {@link #credentialsProvider}
*/
@ConfigItem(defaultValue = "password")
public String keyStorePassword;
@ConfigItem(defaultValueDocumentation = "password")
public Optional<String> keyStorePassword;

/**
* A parameter to specify a {@linkplain CredentialsProvider} property key which can be used to get the password of the key
* store file
* from {@linkplain CredentialsProvider}.
*
* @see {@link #credentialsProvider}
*/
@ConfigItem
public Optional<String> keyStorePasswordKey;

/**
* An optional parameter to select a specific key in the key store. When SNI is disabled, if the key store contains multiple
Expand All @@ -82,11 +121,23 @@ public class CertificateConfig {
public Optional<String> keyStoreKeyAlias;

/**
* An optional parameter to define the password for the key, in case it's different from {@link #keyStorePassword}.
* An optional parameter to define the password for the key, in case it's different from {@link #keyStorePassword}
* If not given then it may be retrieved from {@linkplain CredentialsProvider}.
*
* @see {@link #credentialsProvider}.
*/
@ConfigItem
public Optional<String> keyStoreKeyPassword;

/**
* A parameter to specify a {@linkplain CredentialsProvider} property key which can be used to get the password for the key
* from {@linkplain CredentialsProvider}.
*
* @see {@link #credentialsProvider}
*/
@ConfigItem
public Optional<String> keyStoreKeyPasswordKey;

/**
* An optional trust store which holds the certificate information of the certificates to trust.
*/
Expand All @@ -109,10 +160,23 @@ public class CertificateConfig {

/**
* A parameter to specify the password of the trust store file.
* If not given then it may be retrieved from {@linkplain CredentialsProvider}.
*
* @see {@link #credentialsProvider}.
*/
@ConfigItem
public Optional<String> trustStorePassword;

/**
* A parameter to specify a {@linkplain CredentialsProvider} property key which can be used to get the password of the trust
* store file
* from {@linkplain CredentialsProvider}.
*
* @see {@link #credentialsProvider}
*/
@ConfigItem
public Optional<String> trustStorePasswordKey;

/**
* An optional parameter to trust only one specific certificate in the trust store (instead of trusting all certificates in
* the store).
Expand Down
35 changes: 31 additions & 4 deletions ...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
import io.quarkus.arc.Arc;
import io.quarkus.arc.runtime.BeanContainer;
import io.quarkus.bootstrap.runner.Timing;
import io.quarkus.credentials.CredentialsProvider;
import io.quarkus.credentials.runtime.CredentialsProviderFinder;
import io.quarkus.dev.spi.DevModeType;
import io.quarkus.dev.spi.HotReplacementContext;
import io.quarkus.netty.runtime.virtual.VirtualAddress;
Expand Down Expand Up @@ -781,10 +783,22 @@ private static HttpServerOptions createSslOptions(HttpBuildTimeConfig buildTimeC
certificates.add(certFile.get());
}

// credentials provider
Map<String, String> credentials = Map.of();
if (sslConfig.certificate.credentialsProvider.isPresent()) {
String beanName = sslConfig.certificate.credentialsProviderName.orElse(null);
CredentialsProvider credentialsProvider = CredentialsProviderFinder.find(beanName);
Copy link
Member

Choose a reason for hiding this comment

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

How does this lookup work? Is it using @Identifier?

Copy link
Member Author

@sberyozkin sberyozkin Jun 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It looks for the named beans such as this one, this pattern of checking it is used across various Quarkus extensions. This name does not have to be specified if there is only one bean, that look up code will return the single bean it will find, only needed if several different providers are used.
Once the provider is found, the string (such as custom used in this PR's test) is passed to the found provider to return the credentials, it is not used here, but in Quarkus File Vault it will identify the keystore where to get the secrets from: https://quarkiverse.github.io/quarkiverse-docs/quarkus-file-vault/dev/index.html#_how_to_use_the_extension, for example:

quarkus.datasource.credentials-provider=quarkus.file.vault.provider.db1

quarkus.file.vault.provider.db1.path=dbpasswords.p12
quarkus.file.vault.provider.db1.secret=storepassword
quarkus.file.vault.provider.db1.alias=quarkus_test

So here Agroal will pass quarkus.file.vault.provider.db1 to the provider which will figure out that db1 is a keystore tag, and use its db1 configuration to get the data, etc

Copy link
Member

Choose a reason for hiding this comment

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

Yeah... it uses @nAmed. I don't believe we want this. @mkouba WDYT?
My guess is that a proper qualifier would be better.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cescoffier Sure; @Named would only come into effect if there are more than one CredentialsProvider loaded, with a single one it make no difference. Not sure we can change that though as a few such providers use @Named (the one in quarkus-vault for example)

Copy link
Member Author

@sberyozkin sberyozkin Jul 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@cescoffier Sure; @Named would only come into effect if there are more than one CredentialsProvider loaded, with a single one it makes no difference. Not sure we can change that though as a few such providers use @Named (the one in quarkus-vault (the one based aroiund HashiCorp Vault) and also quarkus-file-vault, but there could be custom ones too)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah... it uses @nAmed. I don't believe we want this. @mkouba WDYT? My guess is that a proper qualifier would be better.

+1

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess it is not something that we can do in this PR, right ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@cescoffier @mkouba As far as this PR is concerned, I can make a credentials-provider-name property not optional but required if you are concerned about a random provider being picked up, it will ensure that only a provider with a matching Named value will be picked up.

I think the actual CredentialsProvider resolution logic update is out of scope for this PR as many extensions depend on it (making the credentials-provider-name required for all those extensions can be the 1st step forward), changing it willbe a breaking change and will make this PR non-backportable if it will have to be backported.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cescoffier I've noticed you've approved, thanks, but your concern about @Named is noted for sure. As I said, I can enforce at the vertx-http level that credentials-provider-name is required, however, I'm having a 2nd thought about it now, if CredentialsProvider resolution logic will indeed get changed in the future then having credentials-provider-name explicitly required may make it harder to migrate.
Perhaps a reasonable compromise is to recommend in JavaDocs of this property that it should be used, will do it now

Copy link
Member Author

Choose a reason for hiding this comment

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

Just pushed a JavaDoc update recommending to use the name property all the time

String name = sslConfig.certificate.credentialsProvider.get();
credentials = credentialsProvider.getCredentials(name);
}
final Optional<Path> keyStoreFile = sslConfig.certificate.keyStoreFile;
final String keystorePassword = sslConfig.certificate.keyStorePassword;
final Optional<String> keyStorePassword = getCredential(sslConfig.certificate.keyStorePassword, credentials,
sslConfig.certificate.keyStorePasswordKey);
final Optional<String> keyStoreKeyPassword = getCredential(sslConfig.certificate.keyStoreKeyPassword, credentials,
sslConfig.certificate.keyStoreKeyPasswordKey);
final Optional<Path> trustStoreFile = sslConfig.certificate.trustStoreFile;
final Optional<String> trustStorePassword = sslConfig.certificate.trustStorePassword;
final Optional<String> trustStorePassword = getCredential(sslConfig.certificate.trustStorePassword, credentials,
sslConfig.certificate.trustStorePasswordKey);
final HttpServerOptions serverOptions = new HttpServerOptions();

//ssl
Expand All @@ -801,11 +815,11 @@ private static HttpServerOptions createSslOptions(HttpBuildTimeConfig buildTimeC
} else if (keyStoreFile.isPresent()) {
KeyStoreOptions options = createKeyStoreOptions(
keyStoreFile.get(),
keystorePassword,
keyStorePassword.orElse("password"),
sslConfig.certificate.keyStoreFileType,
sslConfig.certificate.keyStoreProvider,
sslConfig.certificate.keyStoreKeyAlias,
sslConfig.certificate.keyStoreKeyPassword);
keyStoreKeyPassword);
serverOptions.setKeyCertOptions(options);
} else {
return null;
Expand Down Expand Up @@ -846,6 +860,19 @@ private static HttpServerOptions createSslOptions(HttpBuildTimeConfig buildTimeC
return serverOptions;
}

private static Optional<String> getCredential(Optional<String> password, Map<String, String> credentials,
Optional<String> passwordKey) {
if (password.isPresent()) {
return password;
}

if (passwordKey.isPresent()) {
return Optional.ofNullable(credentials.get(passwordKey.get()));
} else {
return Optional.empty();
}
}

private static void applyCommonOptions(HttpServerOptions httpServerOptions,
HttpBuildTimeConfig buildTimeConfig,
HttpConfiguration httpConfiguration,
Expand Down
23 changes: 23 additions & 0 deletions ...tion-tests/bouncycastle-jsse/src/main/java/io/quarkus/it/bouncycastle/SecretProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.quarkus.it.bouncycastle;

import java.util.HashMap;
import java.util.Map;

import javax.enterprise.context.ApplicationScoped;

import io.quarkus.arc.Unremovable;
import io.quarkus.credentials.CredentialsProvider;

@ApplicationScoped
@Unremovable
public class SecretProvider implements CredentialsProvider {

@Override
public Map<String, String> getCredentials(String credentialsProviderName) {
Map<String, String> creds = new HashMap<>();
creds.put("keystore-password", "password");
creds.put("truststore-password", "password");
return creds;
}

}
6 changes: 4 additions & 2 deletions integration-tests/bouncycastle-jsse/src/main/resources/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
quarkus.security.security-providers=BCJSSE

quarkus.http.ssl.certificate.key-store-file=server-keystore.jks
quarkus.http.ssl.certificate.key-store-password=password
quarkus.http.ssl.certificate.key-store-password-key=key-store-password
quarkus.http.ssl.certificate.trust-store-file=server-truststore.jks
quarkus.http.ssl.certificate.trust-store-password=password
quarkus.http.ssl.certificate.trust-store-password-key=truststore-password
quarkus.http.ssl.certificate.credentials-provider=custom

quarkus.http.ssl.client-auth=REQUIRED
quarkus.native.additional-build-args=-H:IncludeResources=.*\\.jks

Expand Down