Skip to content

fix: strengthen cryptographic operations #19828

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.

Sign up for GitHub

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

Open
FrankChen021 wants to merge 12 commits into
base: master
Choose a base branch
from
Open

fix: strengthen cryptographic operations #19828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1da7b64
fix: strengthen cryptographic operations
FrankChen021 Jul 30, 2026
c9c0c48
Suppress legacy cipher validation warning
FrankChen021 Jul 30, 2026
d130105
fix: suppress legacy cipher CodeQL alerts
FrankChen021 Jul 30, 2026
30e4094
fix: preserve legacy cipher baseline path
FrankChen021 Jul 30, 2026
9d6dd16
style: make legacy decryption locals final
FrankChen021 Jul 30, 2026
da3f536
docs: clarify legacy cipher validation timing
FrankChen021 Jul 30, 2026
beb54f1
fix: preserve legacy cipher baseline location
FrankChen021 Jul 30, 2026
8d55057
fix: suppress legacy cipher compatibility alert
FrankChen021 Jul 31, 2026
0f4e0ad
fix: attach legacy cipher suppression to declaration
FrankChen021 Jul 31, 2026
26dd631
fix: place CodeQL suppression at alert
FrankChen021 Jul 31, 2026
b57491d
fix: scope CodeQL suppression to decrypt
FrankChen021 Jul 31, 2026
a13febd
chore: remove ineffective CodeQL suppression
FrankChen021 Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 104 additions & 31 deletions processing/src/main/java/org/apache/druid/crypto/CryptoService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.GCMParameterSpec;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;
Expand All @@ -36,6 +37,7 @@
import java.security.SecureRandom;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.KeySpec;
import java.util.Arrays;

/**
* Utility class for symmetric key encryption (i.e. same secret is used for encryption and decryption) of byte[]
Expand All @@ -51,6 +53,20 @@ public class CryptoService
// Based on Javadocs on SecureRandom, It is threadsafe as well.
private static final SecureRandom SECURE_RANDOM_INSTANCE = new SecureRandom();

private static final String AUTHENTICATED_CIPHER_ALGORITHM = "AES";
private static final String AUTHENTICATED_CIPHER_TRANSFORMATION = "AES/GCM/NoPadding";
private static final int GCM_IV_SIZE = 12;
private static final int GCM_TAG_LENGTH_BITS = 128;
private static final int AUTHENTICATED_FORMAT_MAGIC = 0xD2525549;

/**
* Negative magic followed by a format version. A valid legacy payload starts with its nonnegative salt length, so
* this header unambiguously distinguishes authenticated ciphertext from the legacy format.
*/
private static final byte[] AUTHENTICATED_FORMAT_HEADER = {
(byte) 0xD2, 0x52, 0x55, 0x49, 0x01
};

// User provided secret phrase used for encrypting data
private final char[] passPhrase;

Expand All @@ -60,7 +76,7 @@ public class CryptoService
private final int iterationCount;
private final int keyLength;

// Cipher algorithm information
// Cipher algorithm information retained for decrypting ciphertext written by earlier versions.
private final String cipherAlgName;
private final String cipherAlgMode;
private final String cipherAlgPadding;
Expand Down Expand Up @@ -95,8 +111,8 @@ public CryptoService(
this.iterationCount = iterationCount == null ? 65536 : iterationCount;
this.keyLength = keyLength == null ? 128 : keyLength;

// encrypt/decrypt a test string to ensure all params are valid
String testString = "duh! !! !!!";
// Validate authenticated parameters eagerly; the legacy transformation is decrypt-only and validated on first use.
final String testString = "duh! !! !!!";
Preconditions.checkState(
testString.equals(StringUtils.fromUtf8(decrypt(encrypt(StringUtils.toUtf8(testString))))),
"decrypt(encrypt(testString)) failed"
Expand All @@ -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))

@FrankChen021 FrankChen021 Aug 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve old-reader compatibility during rolling upgrades

Every encryption now immediately emits the versioned GCM envelope. CryptoService's only production caller is Pac4jSessionStore on Router nodes, but a pre-upgrade Router interprets the first four magic bytes as the legacy salt length; because that value is negative, it rejects the cookie. With multiple Routers behind a load balancer, a cookie issued by an upgraded Router therefore fails whenever the next request reaches an old Router. Supporting legacy reads only provides old-to-new compatibility, not new-to-old compatibility. Gate GCM writes until all readers are upgraded, retain legacy writes for a transition release, or provide another staged migration mechanism.

.put(AUTHENTICATED_FORMAT_HEADER)
.put(encryptedData)
.array();
}
catch (Exception ex) {
log.noStackTrace().warn(ex, "Encryption failed");
throw InternalServerError.exception("Encryption failed. Check service logs.");
}
}

// Legacy ciphertext may use a weaker configured transformation; new ciphertext is always written using GCM.
public byte[] decrypt(byte[] data)
{
try {
EncryptedData encryptedData = EncryptedData.fromByteArray(data);
if (hasAuthenticatedFormatMagic(data)) {
return decryptAuthenticated(data);
}

final EncryptedData encryptedData = EncryptedData.fromByteArray(data);

SecretKey tmp = getKeyFromPassword(passPhrase, encryptedData.getSalt());
SecretKey secret = new SecretKeySpec(tmp.getEncoded(), cipherAlgName);
final SecretKey tmp = getKeyFromPassword(passPhrase, encryptedData.getSalt());
final SecretKey secret = new SecretKeySpec(tmp.getEncoded(), cipherAlgName);

// error-prone warns if the transformation is not a compile-time constant
// since it cannot check it for insecure combinations.
@SuppressWarnings("InsecureCryptoUsage")
Cipher dcipher = Cipher.getInstance(transformation);
final Cipher dcipher = Cipher.getInstance(transformation);
Comment thread
FrankChen021 marked this conversation as resolved.
Dismissed
dcipher.init(Cipher.DECRYPT_MODE, secret, new IvParameterSpec(encryptedData.getIv()));
return dcipher.doFinal(encryptedData.getCipher());
}
Expand All @@ -150,11 +177,49 @@ public byte[] decrypt(byte[] data)
}
}

private SecretKey getKeyFromPassword(char[] passPhrase, byte[] salt)
private byte[] decryptAuthenticated(final byte[] data) throws Exception
{
Preconditions.checkArgument(
data.length >= AUTHENTICATED_FORMAT_HEADER.length
&& Arrays.equals(
data,
0,
AUTHENTICATED_FORMAT_HEADER.length,
AUTHENTICATED_FORMAT_HEADER,
0,
AUTHENTICATED_FORMAT_HEADER.length
),
"Unsupported encrypted data version"
);

final EncryptedData encryptedData = EncryptedData.fromByteArray(
Arrays.copyOfRange(data, AUTHENTICATED_FORMAT_HEADER.length, data.length)
);
Preconditions.checkArgument(encryptedData.getIv().length == GCM_IV_SIZE, "Invalid GCM IV size");

final SecretKey tmp = getKeyFromPassword(passPhrase, encryptedData.getSalt());
final SecretKey secret = new SecretKeySpec(tmp.getEncoded(), AUTHENTICATED_CIPHER_ALGORITHM);
final Cipher dcipher = Cipher.getInstance(AUTHENTICATED_CIPHER_TRANSFORMATION);
dcipher.init(
Cipher.DECRYPT_MODE,
secret,
new GCMParameterSpec(GCM_TAG_LENGTH_BITS, encryptedData.getIv())
);
dcipher.updateAAD(AUTHENTICATED_FORMAT_HEADER);
return dcipher.doFinal(encryptedData.getCipher());
}

private static boolean hasAuthenticatedFormatMagic(final byte[] data)
{
return data.length >= Integer.BYTES
&& ByteBuffer.wrap(data).getInt() == AUTHENTICATED_FORMAT_MAGIC;
}

private SecretKey getKeyFromPassword(final char[] passPhrase, final byte[] salt)
throws NoSuchAlgorithmException, InvalidKeySpecException
{
SecretKeyFactory factory = SecretKeyFactory.getInstance(secretKeyFactoryAlg);
KeySpec spec = new PBEKeySpec(passPhrase, salt, iterationCount, keyLength);
final SecretKeyFactory factory = SecretKeyFactory.getInstance(secretKeyFactoryAlg);
final KeySpec spec = new PBEKeySpec(passPhrase, salt, iterationCount, keyLength);
return factory.generateSecret(spec);
}

Expand Down Expand Up @@ -188,8 +253,10 @@ public byte[] getCipher()

public byte[] toByteAray()
{
int headerLength = 12;
ByteBuffer bb = ByteBuffer.allocate(salt.length + iv.length + cipher.length + headerLength);
final int headerLength = 12;
final int encryptedDataLength =
Math.addExact(Math.addExact(Math.addExact(salt.length, iv.length), cipher.length), headerLength);
final ByteBuffer bb = ByteBuffer.allocate(encryptedDataLength);
bb.putInt(salt.length)
.putInt(iv.length)
.putInt(cipher.length)
Expand All @@ -203,19 +270,25 @@ public byte[] toByteAray()

public static EncryptedData fromByteArray(byte[] array)
{
ByteBuffer bb = ByteBuffer.wrap(array);

int saltSize = bb.getInt();
int ivSize = bb.getInt();
int cipherSize = bb.getInt();

byte[] salt = new byte[saltSize];
Preconditions.checkArgument(array.length >= 12, "Invalid encrypted data");
final ByteBuffer bb = ByteBuffer.wrap(array);

final int saltSize = bb.getInt();
final int ivSize = bb.getInt();
final int cipherSize = bb.getInt();
final long payloadSize = (long) saltSize + ivSize + cipherSize;
Preconditions.checkArgument(
saltSize >= 0 && ivSize >= 0 && cipherSize >= 0 && payloadSize == bb.remaining(),
"Invalid encrypted data"
);

final byte[] salt = new byte[saltSize];
bb.get(salt);

byte[] iv = new byte[ivSize];
final byte[] iv = new byte[ivSize];
bb.get(iv);

byte[] cipher = new byte[cipherSize];
final byte[] cipher = new byte[cipherSize];
bb.get(cipher);

return new EncryptedData(salt, iv, cipher);
Expand Down
12 changes: 6 additions & 6 deletions processing/src/main/java/org/apache/druid/query/aggregation/JavaScriptAggregatorFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,18 +232,18 @@ public List<String> requiredFields()
public byte[] getCacheKey()
{
try {
MessageDigest md = MessageDigest.getInstance("SHA-1");
byte[] fieldNameBytes = StringUtils.toUtf8(Joiner.on(",").join(fieldNames));
byte[] sha1 = md.digest(StringUtils.toUtf8(fnAggregate + fnReset + fnCombine));
final MessageDigest md = MessageDigest.getInstance("SHA-256");
final byte[] fieldNameBytes = StringUtils.toUtf8(Joiner.on(",").join(fieldNames));
final byte[] scriptDigest = md.digest(StringUtils.toUtf8(fnAggregate + fnReset + fnCombine));

return ByteBuffer.allocate(1 + fieldNameBytes.length + sha1.length)
return ByteBuffer.allocate(1 + fieldNameBytes.length + scriptDigest.length)
.put(AggregatorUtil.JS_CACHE_TYPE_ID)
.put(fieldNameBytes)
.put(sha1)
.put(scriptDigest)
.array();
}
catch (NoSuchAlgorithmException e) {
throw new RuntimeException("Unable to get SHA1 digest instance", e);
throw new RuntimeException("Unable to get SHA-256 digest instance", e);
}
}

Expand Down
Loading
Loading