-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add test plan for QUARKUS-3456 #180
Add test plan for QUARKUS-3456 #180
Conversation
60f47d4
to
fa6751d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few things to handle + we need confirmation of scope from https://issues.redhat.com/browse/QUARKUS-3456?focusedId=25160816&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-25160816
|
||
## Future considerations | ||
|
||
- use more Ciphers (use different algorithms, modes and padding) to encrypt a secret |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fa6751d
to
57c5c9f
Compare
57c5c9f
to
a7c78fe
Compare
in a case scope wouldn't be confirmed, this would be blocked till TD is merged, I prefer to update TP later |
confirmation of scope / update of TP/TD can come later in separate PRs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for quick update. Open questions can be reflected once we get feedback on them.
Links
JIRA: https://issues.redhat.com/browse/QUARKUS-3456
Quarkus documentation: https://quarkus.io/version/main/guides/config-secrets#protect-the-keystore-password
Reminder for considerable topics
Make sure you have considered the following areas when preparing the test plan: