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

Add test plan for QUARKUS-3456 #180

Merged
merged 1 commit into from
Jul 19, 2024
Merged

Add test plan for QUARKUS-3456 #180

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
69 changes: 69 additions & 0 deletions QUARKUS-3456.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# QUARKUS-3456 - Enhance the CLI to be able to encrypt configuration values

JIRA: https://issues.redhat.com/browse/QUARKUS-3456

Upstream issue: https://github.com/quarkusio/quarkus/issues/29172 (partial motivation together with the QUARKUS-3456)

Upstream PR: https://github.com/quarkusio/quarkus/pull/34493

Red Hat Build of Quarkus has ability to handle secrets since the Ghost release.
This feature worked alright, but encrypting secrets so that built-in secret handlers can decode them was somehow awkward.
It was far from trivial to find a correct steps and encrypt the secret in a way Quarkus accepted.
Now Quarkus leverages Quarkus CLI and allows to create, update and remove configuration properties with it.
One of options is to pass the CLI plain secret and Quarkus will encode this secret for you.
You can even ask Quarkus CLI to set encoded secret directly to the application properties.
Or, if you use YAML or other formats (like if you keep secrets in the environment variables), you can just copy it from the CLI output.
We should test this new feature, including other sub-commands under `quarkus config` command as users are likely to use them when they encrypt secrets.

## Scope of the testing
Test coverage must include:

- `quarkus config set/remove/encrypt` command tested with options combined in a fashion customers are likely to use,
michalvavrik marked this conversation as resolved.
Show resolved Hide resolved
- documented flow to create a secret, add it to the Keystore, set Keystore config source and check Quarks application decodes secrets,
- default secret handler `AES/GCM/NoPadding` is used to create the secret,
- secret is decoded in FIPS-enabled environment,
- read encrypted configuration in Quarkus application user code,
- store secrets generated with the CLI in a Keystore and access them using Quarkus application,
- decode the encryption key in Base64, if the plain text key was used to encrypt the secret,
- test help command for all three `config` subcommands to assure [#41131](https://github.com/quarkusio/quarkus/issues/41131) issue is fixed,
- application startup fails when unknown secret handler is used,
- CLI command automatically creates `application.properties` file when it doesn't exist.

michalvavrik marked this conversation as resolved.
Show resolved Hide resolved
## Getting familiar with the feature
Following actions were taken to ensure familiarity:
- Focus on exploratory testing of the feature
- Ensure good user experience and simplicity of use

## Existing test coverage
Upstream test coverage in Quarkus project includes test of encryption with a plain encoded secret, plain secret.
Also, encryption tests with explicitly declared key and with a generated key.
Most importantly, the PR closed [Quarkus issue #29172](https://github.com/quarkusio/quarkus/issues/29172), however it does test it partially and all the capabilities are tested in isolation.

We test secret key handlers in the `config` module of the Quarkus QE Test Suite.

## Impact on test suites and testing automation
Three new integration test classes will be added to Quarkus CLI module of the Quarkus QE Test Suite.
One for each `quarkus config` subcommand (`set`, `remove`, `encrypt`).
Tests will run in a JVM mode only as intention is to test CLI and assure that `application.properties` are updated correctly.

## Impact on resources
We will start JVM application once per each test class and call an endpoint or two, therefore additional test execution time should be below 30 seconds.

## Future considerations

- use more Ciphers (use different algorithms, modes and padding) to encrypt a secret
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to move it out of the future scope and put it into current TD.
What would be the estimate to implement this?

Copy link
Member Author

Choose a reason for hiding this comment

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

You can chose alg/mode/padding via CLI encrypt cmd options https://github.com/quarkusio/quarkus/blob/main/devtools/cli/src/main/java/io/quarkus/cli/config/Encrypt.java#L59 but I am worried about this https://github.com/quarkusio/quarkus/blob/main/devtools/cli/src/main/java/io/quarkus/cli/config/Encrypt.java#L64. I simply didn't get to test this and I don't know if you can use different cipher while you still have there GCM spec 128 and sha256.

I cannot tell if it is additional time or not, but quarkusio/quarkus#34493 said Can encrypt arbitrary values in AES/GCM/NoPadding (will support more algorithms) so I didn't expect it is actually supported. Right now, it is not documented which other ciphers are supported.

Copy link
Member Author

Choose a reason for hiding this comment

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

@rsvoboda

    @Option(hidden = true, names = { "-a", "--algorithm" }, description = "Algorithm", defaultValue = "AES")
    String algorithm;

    @Option(hidden = true, names = { "-m", "--mode" }, description = "Mode", defaultValue = "GCM")
    String mode;

    @Option(hidden = true, names = { "-p", "--padding" }, description = "Padding", defaultValue = "NoPadding")
    String padding;

    @Option(hidden = true, names = { "-q", "--quiet" }, defaultValue = "false")
    boolean quiet;

they are hidden, that and the fact it is not documented says it all, it is not supported ATM IMHO

Copy link
Member Author

Choose a reason for hiding this comment

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

https://issues.redhat.com/browse/QUARKUS-3456?focusedId=25161121&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-25161121

- other formats (YAML, env properties)
- support for adding generated secrets into a KeyStore using Quarkus CLI
- [respective Quarkus guide](https://quarkus.io/version/main/guides/config-secrets#protect-the-keystore-password) says you need to store the Keystore password in `application.properties` and suggests to restrict access to the Keystore file itself
While the principle of least privilege is a good suggestion, we need to verify and document that other config sources can be used to store this secret key so that it is better protected.

## Contacts
* Tester: Michal Vavřík <[email protected]>

## References
- [Quarkus guide - Secrets in configuration](https://quarkus.io/guides/config-secrets)
- [JIRA ticket - QUARKUS-2727 - Encryption of configuration values](https://issues.redhat.com/browse/QUARKUS-2727)
- [Original PR - SmallRye Config ##833 - Secret Keys Handlers](https://github.com/smallrye/smallrye-config/pull/833)
- [SmallRye Config docs - Secret keys](https://smallrye.io/smallrye-config/Main/config/secret-keys/#crypto)
- [Quarkus QE internal tracked](https://issues.redhat.com/browse/QQE-10)
- [Quarkus issue #41131 - missing help text](https://github.com/quarkusio/quarkus/issues/41131)