Secret Keys Handlers

[radcortez]

Replaces #799.

• We use property expressions to represent a secret key value in the format ${handler_name::encoded_secret}.
• The SecretKeys API, registers handlers by name via the ServiceLoader.
• The SecretKeysHandler is executed in the interceptor that deals with secret keys (if there is a match).
• Any source can contain secrets. It only requires the right handlers to decode/decrypt the value.
• Also includes a KeyStore source, to demonstrate how it could work with the SecretKeysHandler.

There is still some work to complete:

• Being able to configure SecretKeysHandler.
• Certificate support
• Out of the box handlers

The goal of the current PR is to just agree on the design and then move forward.

[radcortez]

/cc @dmlloyd @sberyozkin

[radcortez]

@dmlloyd let me know what you think of the current proposal. Thanks!

[dmlloyd]

This makes sense to me. We can look up secret values separately from extracting them from an expression, which also allows specialized config sources that might encode them in an idiomatic way. 👍

[dmlloyd]

So this config source requires a full config in order to work? If so, this gets away from the single-config methodology twice, once by requiring a config for the config and once for the key store itself.

[radcortez]

Not exactly. This is just a trick to be able to use a mapping to configure a ConfigSource.

For instance, consider the mapping:
https://github.com/radcortez/smallrye-config/blob/b2bf955328cf8e4382c364f898173477d80d77a3/sources/keystore/src/main/java/io/smallrye/config/source/keystore/KeyStoreConfig.java#L11

How can we create this mapping? Remember, we use SmallRyeConfig to call certain API to populate the mapping instance. Ideally, we should decouple that, so we don't need to use the trick.

We create an empty SmallRyeConfig instance, just with the mapping class and a ConfigSource that delegates to the ConfigSourceContext:

https://github.com/radcortez/smallrye-config/blob/b2bf955328cf8e4382c364f898173477d80d77a3/sources/keystore/src/main/java/io/smallrye/config/source/keystore/KeyStoreConfigSourceFactory.java#L38-L41

https://github.com/radcortez/smallrye-config/blob/b2bf955328cf8e4382c364f898173477d80d77a3/sources/keystore/src/main/java/io/smallrye/config/source/keystore/KeyStoreConfigSourceFactory.java#L70-L97

So we can get an instance of the mapping class, by wrapping all of that, but still use the original config context being build.

[UXabre]

preface: the following could be very dumb as I'm not acquainted with this project in particular, but seeing that other parts seem to support dynamic loading of config changes, e.g. the following PR https://github.com/smallrye/smallrye-config/pull/152/files#diff-60faeb32cc934956c4bbc5ac22b81c35f9c9fc6e8073e83779bb1cfbe00d7cb9 ...

Also, I'm sorry I'm discussing it here but I couldn't find a related issue on the matter.

I'm curious to learn if it shouldn't also be possible to load changes of these secrets as well? For instance, a file containing a password file should probably trigger an event if it is changed so that a consuming application could simply reload. However, as mentioned, it seems some part of this project support it but I'm finding it hard to understand if this PR builds on that or not?
To be more specific: will a change of a secretvalue lead to an event being fired

To give an example: will a change of a secretvalue lead to an event being fired that can be caught by, let's say, KeyCloak (which uses quarkus) which would be able to restart a connection to a backend resource?

[radcortez]

Also, I'm sorry I'm discussing it here but I couldn't find a related issue on the matter.

You don't need to apologize :)

I'm curious to learn if it shouldn't also be possible to load changes of these secrets as well? For instance, a file containing a password file should probably trigger an event if it is changed so that a consuming application could simply reload. However, as mentioned, it seems some part of this project support it but I'm finding it hard to understand if this PR builds on that or not? To be more specific: will a change of a secretvalue lead to an event being fired

No, we don't build on that. What determines how a configuration is read (and if it is reloadable), is the ConfigSource. This PR, only adds the ability to interpret values as secrets and handle them. It is certainly possible with the events extension to provide a ConfigSource that triggers the CDI event on a config change or reload.

To give an example: will a change of a secretvalue lead to an event being fired that can be caught by, let's say, KeyCloak (which uses quarkus) which would be able to restart a connection to a backend resource?

Our stance with Quarkus is that we don't support reloadable configuration (at least the configuration on the quarkus namespace). Everyone else is free to do what they want :) The big issue is that is hard to manage user expectations about a configuration reload. Certain configurations will always require a server restart, while others we could get away without, so we prefer to be consistent and always require a restart if you change the Quarkus configuration.

[UXabre]

Thanks for your elaborate reply on the matter, really appreciate it and I understand it fully :-) Happy coding!

[Pepo48]

Hey @radcortez, as I'm continuing with the spike around this PR, I encountered another issue (hopefully this time it's not something already known 😊) .

It seems that when you work with the Keystore Config Source and addProfileConfigSource method tries to load the optional profile resource (for example in my case it was keystore-prod, which doesn't have to exist), a runtime exception thrown here:

smallrye-config//sources/keystore/src/main/java/io/smallrye/config/source/keystore/KeyStoreConfigSourceFactory.java

Lines 149 to 152 in dddcc4b

	} catch (KeyStoreException | CertificateException | IOException | NoSuchAlgorithmException
	| UnrecoverableKeyException e) {
	throw new RuntimeException(e);
	}

is not handled properly. In my scenario, where single property file is shared among multiple tests, it affected all of them.

I tried a quick fix, where I caught the exception here:

smallrye-config/implementation/src/main/java/io/smallrye/config/AbstractLocationConfigSourceLoader.java

Line 235 in dddcc4b

} catch (FileNotFoundException | NoSuchFileException e) {

So now the line looks like this:

catch (FileNotFoundException | NoSuchFileException | RuntimeException e) {

and it helped. But you may find a more elegant solution, for sure 😊.
If it will be needed I can share a reproducer as well.

Thanks!

[radcortez]

Thanks! Let me have a look!

[Pepo48]

Thanks! Let me have a look!

@radcortez I just wanted to ask whether there is a progress regarding this?

On top of that, what secret handlers are planned to be present in the final implementation? I tried the jasypt one and it seems to cover our needs for now.

Is this still aimed for the Quarkus 3 release?

Thanks!

[radcortez]

Hey. I'm sorry, I was on PTO the last few days so I couldn't look into it, but it is on my plans to look as soon as possible. I still need to do the final move to Jakarta (including other SR Projects), and then we should be ready to go with new features.

Yes, the jasypt one is planned to be included and still aimed at Quarkus 3.

The idea is to add handlers as required, so if you need a specific one, feel free to add it.

[radcortez]

@Pepo48

I have fixed some of the issues and added some of the missing pieces (namely, supporting a way to configure the secret handlers).

Can you take a final look? Thanks :)

[michalvavrik]

Hey @radcortez ,

do you think it is possible for you to release SmallRye Config and bump the version in Quarkus once this gets merged? I'd like to do QE test development for this - https://issues.redhat.com/browse/QUARKUS-2727. I hope release won't eat too much of your time.

Thank you

[radcortez]

I'm currently working on the Quarkus integration bits and I believe it should be completed in the next few days. Thanks!