FrankChen021 commented on code in PR #19828:
URL: https://github.com/apache/druid/pull/19828#discussion_r3914092046
##########
processing/src/main/java/org/apache/druid/crypto/CryptoService.java:
##########
@@ -106,41 +122,52 @@ public CryptoService(
public byte[] encrypt(byte[] plain)
{
try {
- byte[] salt = new byte[saltSize];
+ final byte[] salt = new byte[saltSize];
SECURE_RANDOM_INSTANCE.nextBytes(salt);
- SecretKey tmp = getKeyFromPassword(passPhrase, salt);
- SecretKey secret = new SecretKeySpec(tmp.getEncoded(), cipherAlgName);
+ final SecretKey tmp = getKeyFromPassword(passPhrase, salt);
+ final SecretKey secret = new SecretKeySpec(tmp.getEncoded(),
AUTHENTICATED_CIPHER_ALGORITHM);
- // error-prone warns if the transformation is not a compile-time constant
- // since it cannot check it for insecure combinations.
- @SuppressWarnings("InsecureCryptoUsage")
- Cipher ecipher = Cipher.getInstance(transformation);
- ecipher.init(Cipher.ENCRYPT_MODE, secret);
- return new EncryptedData(
+ final byte[] iv = new byte[GCM_IV_SIZE];
+ SECURE_RANDOM_INSTANCE.nextBytes(iv);
+
+ final Cipher ecipher =
Cipher.getInstance(AUTHENTICATED_CIPHER_TRANSFORMATION);
+ ecipher.init(Cipher.ENCRYPT_MODE, secret, new
GCMParameterSpec(GCM_TAG_LENGTH_BITS, iv));
+ ecipher.updateAAD(AUTHENTICATED_FORMAT_HEADER);
+
+ final byte[] encryptedData = new EncryptedData(
salt,
-
ecipher.getParameters().getParameterSpec(IvParameterSpec.class).getIV(),
+ iv,
ecipher.doFinal(plain)
).toByteAray();
+ return
ByteBuffer.allocate(Math.addExact(AUTHENTICATED_FORMAT_HEADER.length,
encryptedData.length))
Review Comment:
[P1] New cookies break mixed-version rolling upgrades
New nodes now emit the versioned AES-GCM envelope, but pre-upgrade nodes
only parse the legacy length-prefixed format. When a user authenticates through
a new node and is then routed to an old node, the old parser interprets the
negative magic value as a salt length and fails, causing authentication-cookie
reads to error until the user reauthenticates or reaches a new node.
Legacy-read support only covers old cookies sent to new nodes. Use a staged
format rollout, dual-cookie/format compatibility, or another deployment
coordination mechanism, and add a test proving old-node readers handle cookies
emitted during rollout.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]