github-advanced-security[bot] commented on code in PR #19828:
URL: https://github.com/apache/druid/pull/19828#discussion_r3684992904
##########
processing/src/main/java/org/apache/druid/crypto/CryptoService.java:
##########
@@ -132,29 +158,91 @@
public byte[] decrypt(byte[] data)
{
try {
- EncryptedData encryptedData = EncryptedData.fromByteArray(data);
-
- SecretKey tmp = getKeyFromPassword(passPhrase, encryptedData.getSalt());
- 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);
- dcipher.init(Cipher.DECRYPT_MODE, secret, new
IvParameterSpec(encryptedData.getIv()));
- return dcipher.doFinal(encryptedData.getCipher());
+ if (hasAuthenticatedFormatMagic(data)) {
+ return decryptAuthenticated(data);
+ } else {
+ return decryptLegacy(EncryptedData.fromByteArray(data));
+ }
}
catch (Exception ex) {
log.noStackTrace().warn(ex, "Decryption failed");
throw InternalServerError.exception("Decryption failed. Check service
logs.");
}
}
- 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());
+ }
+
+ /**
+ * Decrypts the unversioned CBC format written before authenticated
encryption was introduced. This path is retained
+ * so that short-lived pac4j session cookies remain readable during rolling
upgrades. New ciphertext is never written
+ * with this configurable transformation.
+ */
+ @SuppressWarnings({"InsecureCryptoUsage",
"java/potentially-weak-cryptographic-algorithm"})
+ private byte[] decryptLegacy(final EncryptedData encryptedData) throws
Exception
+ {
+ final SecretKey tmp = getKeyFromPassword(passPhrase,
encryptedData.getSalt());
+ final SecretKey secret = new SecretKeySpec(tmp.getEncoded(),
legacyCipherAlgName);
+ // This configurable transformation is used exclusively to read ciphertext
written by earlier versions.
+ // codeql[java/potentially-weak-cryptographic-algorithm]
+ final Cipher dcipher = Cipher.getInstance(legacyTransformation);
Review Comment:
## CodeQL / Use of a potentially broken or risky cryptographic algorithm
Cryptographic algorithm [CBC](1) may not be secure. Consider using a
different algorithm.
Cryptographic algorithm [PKCS5Padding](2) may not be secure. Consider using
a different algorithm.
Cryptographic algorithm [CBC](3) may not be secure. Consider using a
different algorithm.
Cryptographic algorithm [PKCS5Padding](4) may not be secure. Consider using
a different algorithm.
Cryptographic algorithm [%s/%s/%s](5) may not be secure. Consider using a
different algorithm.
[Show more
details](https://github.com/apache/druid/security/code-scanning/11434)
##########
processing/src/main/java/org/apache/druid/crypto/CryptoService.java:
##########
@@ -132,29 +158,91 @@
public byte[] decrypt(byte[] data)
{
try {
- EncryptedData encryptedData = EncryptedData.fromByteArray(data);
-
- SecretKey tmp = getKeyFromPassword(passPhrase, encryptedData.getSalt());
- 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);
- dcipher.init(Cipher.DECRYPT_MODE, secret, new
IvParameterSpec(encryptedData.getIv()));
- return dcipher.doFinal(encryptedData.getCipher());
+ if (hasAuthenticatedFormatMagic(data)) {
+ return decryptAuthenticated(data);
+ } else {
+ return decryptLegacy(EncryptedData.fromByteArray(data));
+ }
}
catch (Exception ex) {
log.noStackTrace().warn(ex, "Decryption failed");
throw InternalServerError.exception("Decryption failed. Check service
logs.");
}
}
- 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());
+ }
+
+ /**
+ * Decrypts the unversioned CBC format written before authenticated
encryption was introduced. This path is retained
+ * so that short-lived pac4j session cookies remain readable during rolling
upgrades. New ciphertext is never written
+ * with this configurable transformation.
+ */
+ @SuppressWarnings({"InsecureCryptoUsage",
"java/potentially-weak-cryptographic-algorithm"})
+ private byte[] decryptLegacy(final EncryptedData encryptedData) throws
Exception
+ {
+ final SecretKey tmp = getKeyFromPassword(passPhrase,
encryptedData.getSalt());
+ final SecretKey secret = new SecretKeySpec(tmp.getEncoded(),
legacyCipherAlgName);
+ // This configurable transformation is used exclusively to read ciphertext
written by earlier versions.
+ // codeql[java/potentially-weak-cryptographic-algorithm]
+ final Cipher dcipher = Cipher.getInstance(legacyTransformation);
+ dcipher.init(Cipher.DECRYPT_MODE, secret, new
IvParameterSpec(encryptedData.getIv()));
+ return dcipher.doFinal(encryptedData.getCipher());
+ }
+
+ @SuppressWarnings("InsecureCryptoUsage")
+ private void validateLegacyCipherConfiguration()
+ {
+ try {
+ // Preserve eager validation of the backward-compatible decryption
configuration.
+ // codeql[java/potentially-weak-cryptographic-algorithm]
+ Cipher.getInstance(legacyTransformation);
Review Comment:
## CodeQL / Use of a potentially broken or risky cryptographic algorithm
Cryptographic algorithm [CBC](1) may not be secure. Consider using a
different algorithm.
Cryptographic algorithm [PKCS5Padding](2) may not be secure. Consider using
a different algorithm.
Cryptographic algorithm [CBC](3) may not be secure. Consider using a
different algorithm.
Cryptographic algorithm [PKCS5Padding](4) may not be secure. Consider using
a different algorithm.
Cryptographic algorithm [%s/%s/%s](5) may not be secure. Consider using a
different algorithm.
[Show more
details](https://github.com/apache/druid/security/code-scanning/11435)
--
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]
