preflight check for password_secret #21491
Conversation
data-node/src/main/java/org/graylog/datanode/bindings/PreflightChecksBindings.java
Outdated
Show resolved
Hide resolved
data-node/src/main/java/org/graylog/datanode/bindings/PreflightChecksBindings.java
Outdated
Show resolved
Hide resolved
| final EncryptedValue encryptedSecret = preflightEncryptedSecret.encryptedSecret(); | ||
|
|
||
| try { | ||
| final String decrypted = encryptionService.decrypt(encryptedSecret); |
There was a problem hiding this comment.
unfortunately, our implementation in AESTools doesn't guarantee that the secret is identical as it modifies the key. You can reproduce that by removing a single letter from the end of a long password secret. This will still lead to problems with the keystore password.
There was a problem hiding this comment.
I don't think this is a problem related to this PR. For the mongodb entries encryption, this will detect any problem. Keystore reading will be a local problem that we can (and should) capture by proper error handling.
There was a problem hiding this comment.
I am not sure. I would expect the check to make sure that password_secrets are identical on all nodes, no matter if they are used for encryption or anything else. This is also what the class' javadoc implies.
With the given check, there is still a margin for error which is difficult to track down. Something that we wanted to avoid with this check.
There was a problem hiding this comment.
How could we fix that?
There was a problem hiding this comment.
Haven't thought it through completely yet, but what if we don't encrypt a constant value but the secret itself? Then, even if you can decrypt it with a shorter secret, you could still make sure that they are identical
...log2-server/src/main/java/org/graylog2/bootstrap/preflight/PasswordSecretPreflightCheck.java
Outdated
Show resolved
Hide resolved
…/PasswordSecretPreflightCheck.java Co-authored-by: Matthias Oesterheld <[email protected]>
Description
This PR adds a preflight check for both server and data node. Each side verifies if the cluster_config collection contains a predefined and encrypted value. If not, then it will be created. If yes, then the check will try to read and decrypt it. The value is known and easy to verify. So if the decrypting fails, we know that this node uses a different password than the one that originally encrypted the value. That leads to a controlled stop of the startup and proper error message. Rather fail early and explicitly than wait till a problem occurs later, during normal operation. Failing early means fixing the problem early, before it can cause big troubles.
Motivation and Context
Fixes #21504
How Has This Been Tested?
Manually and with a unit test
Types of changes
Checklist: