fix: harden database encryption with AES/GCM and key rotation - #441
santhoshalpha wants to merge 1 commit into
Conversation
Replace AES/ECB/PKCS5Padding with AES/GCM/NoPadding (AEAD: confidentiality + integrity, random IV per value). Ciphertext is now self-describing as "<kid>:<base64(iv||ct||tag)>", so multiple keys can coexist and be rotated by adding a new key id and switching the active key, without a big-bang re-encryption. Values encrypted by the previous AES/ECB scheme keep decrypting via a configured legacy key for backward compatibility. Closes reshaprio#427 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZykTGjxi8FEzPNyw9pd3k Signed-off-by: santhoshalpha <santhoshpkraja2004@gmail.com>
bb88185 to
061e2bf
Compare
|
@lbroudoux Heads up — the design comment on #427 was accidentally posted from my work GitHub account ( |
lbroudoux
left a comment
lbroudoux
left a comment
There was a problem hiding this comment.
Thanks for pushing this! It looks great. These are the foundations of the changes. We'll need to complete it with a KeyRotationService and an admin-only API KeyRotationResource that allows launching the re-encryption (from legacy) and the rotation of keys (when moving from v1 and v2).
Please check my comments and apply changes if you can. Otherwise, I'll probably merge this as is and apply the refactoring on tomorrow.
Cheers,
| @ConfigProperty(name = "reshapr.encryption.keys") Map<String, String> encryptionKeys, | ||
| @ConfigProperty(name = "reshapr.encryption.active-key-id") String activeKeyId, | ||
| @ConfigProperty(name = "reshapr.encryption.legacy-key") Optional<String> legacyKey) { |
There was a problem hiding this comment.
I would have created a dedicated EncryptionConfig interface for this, wrapping the 3 properties, in the io.reshapr.ctrl.config package.
| encryptionKeyBytes = encryptionKey.getBytes(); | ||
| if (encryptionKeyBytes == null || !isKeySizeValid(encryptionKeyBytes.length)) { | ||
| throw new IllegalArgumentException("Encryption key must 16, 24 or 32 characters long"); | ||
| if (encryptionKeys == null || encryptionKeys.isEmpty()) { |
There was a problem hiding this comment.
Keeping the @PostConstruct initialize() method is usually more readable because it dissociates retrieving configuration properties from validating them. I think it makes things easier to troubleshoot later on.
lbroudoux
added
enhancement
2 participants
Replace AES/ECB/PKCS5Padding with AES/GCM/NoPadding (AEAD: confidentiality + integrity, random IV per value). Ciphertext is now self-describing as ":<base64(iv||ct||tag)>", so multiple keys can coexist and be rotated by adding a new key id and switching the active key, without a big-bang re-encryption. Values encrypted by the previous AES/ECB scheme keep decrypting via a configured legacy key for backward compatibility.
Closes #427