security: upgrade credential encryption from AES/CBC to AES/GCM#610
Merged
security: upgrade credential encryption from AES/CBC to AES/GCM#610
Conversation
Replace AES/CBC/PKCS5Padding with AES/GCM/NoPadding for credential store encryption. The old code used a static zero IV which is insecure. GCM provides authenticated encryption with random IVs prepended to the ciphertext. Extracted from #556.
On decrypt, try GCM first. If the auth tag fails (AEADBadTagException), fall back to the old AES/CBC/PKCS5Padding with static zero IV. This allows existing credentials to be read without re-encryption. Credentials will migrate to GCM format on next update/rotation — writes always use AES/GCM.
- testFallbackDecryptsLegacyCBC: CBC-encrypted data decrypted via fallback - testFallbackDecryptsNewGCM: GCM-encrypted data decrypted via fallback - testGcmDecryptRejectsLegacyData: GCM decrypt throws on CBC data
Replace transparent fallback with a one-time migration script
(MigrateCredentialEncryption) that re-encrypts all CREDENTIALS rows
from legacy AES/CBC to AES/GCM. Run before deploying the GCM-only code.
Usage:
java MigrateCredentialEncryption <jdbcUrl> <dbUser> <dbPass> \
<keystorePath> <keyAlias> <keystorePass>
The script skips rows already in GCM format.
…ky cryptographic algorithm Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extracted from #556. Upgrades credential store encryption from
AES/CBC/PKCS5Paddingwith a static zero IV toAES/GCM/NoPaddingwith random IVs.What changed
SecurityUtil: Newencrypt(byte[], Key)/decrypt(byte[], Key)API using GCM with 12-byte random IV prepended to ciphertext. LegacydecryptLegacy()retained for migration use only.CredentialsDAO: Updated to use new API (get key once, then encrypt/decrypt)MigrateCredentialEncryption: One-time migration script to re-encrypt existing CBC credentials to GCMencryptString/decryptStringmethods that baked in keystore accessWhy
new byte[16]IV is insecure — identical plaintexts produce identical ciphertextsMigration
Run before deploying:
The script decrypts each
CREDENTIALSrow with legacy CBC, re-encrypts with GCM, and updates in-place. Already-migrated rows are skipped.All 9 modules build, all tests pass (5 SecurityUtil tests including legacy round-trip and CBC→GCM migration).