github-advanced-security[bot] commented on code in PR #610:
URL: https://github.com/apache/airavata/pull/610#discussion_r3012055642
##########
airavata-api/src/main/java/org/apache/airavata/security/util/SecurityUtil.java:
##########
@@ -35,52 +39,59 @@
public static final String PASSWORD_HASH_METHOD_PLAINTEXT = "PLAINTEXT";
public static final String CHARSET_ENCODING = "UTF-8";
- public static final String PADDING_MECHANISM = "AES/CBC/PKCS5Padding";
+ public static final String CIPHER_NAME = "AES/GCM/NoPadding";
+ public static final int GCM_IV_BYTES = 12; // 96 bits
+ public static final int GCM_TAG_BITS = 128;
private static final Logger logger =
LoggerFactory.getLogger(SecurityUtil.class);
- public static byte[] encryptString(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, String value)
- throws GeneralSecurityException, IOException {
- return encrypt(keyStorePath, keyAlias, passwordCallback,
value.getBytes(CHARSET_ENCODING));
- }
-
- public static byte[] encrypt(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, byte[] value)
- throws GeneralSecurityException, IOException {
-
- Key secretKey = getSymmetricKey(keyStorePath, keyAlias,
passwordCallback);
-
- Cipher cipher = Cipher.getInstance(PADDING_MECHANISM);
- cipher.init(Cipher.ENCRYPT_MODE, secretKey, new IvParameterSpec(new
byte[16]));
- return cipher.doFinal(value);
- }
-
- private static Key getSymmetricKey(String keyStorePath, String keyAlias,
KeyStorePasswordCallback passwordCallback)
+ public static Key getSymmetricKey(String keyStorePath, String keyAlias,
KeyStorePasswordCallback passwordCallback)
throws CertificateException, NoSuchAlgorithmException,
KeyStoreException, IOException,
UnrecoverableKeyException {
KeyStore ks = SecurityUtil.loadKeyStore(keyStorePath,
passwordCallback);
return ks.getKey(keyAlias,
passwordCallback.getSecretKeyPassPhrase(keyAlias));
}
- public static byte[] decrypt(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, byte[] encrypted)
- throws GeneralSecurityException, IOException {
-
- Key secretKey = getSymmetricKey(keyStorePath, keyAlias,
passwordCallback);
+ public static byte[] encrypt(byte[] data, Key key) throws
GeneralSecurityException {
+ var cipher = Cipher.getInstance(CIPHER_NAME);
+ cipher.init(Cipher.ENCRYPT_MODE, key);
+ var iv = cipher.getIV();
+ var encryptedData = cipher.doFinal(data);
+ return ByteBuffer.allocate(iv.length + encryptedData.length)
+ .put(iv)
+ .put(encryptedData)
+ .array();
+ }
- Cipher cipher = Cipher.getInstance(PADDING_MECHANISM);
- cipher.init(Cipher.DECRYPT_MODE, secretKey, new IvParameterSpec(new
byte[16]));
+ public static byte[] decrypt(byte[] tag, Key key) throws
GeneralSecurityException {
+ var iv = Arrays.copyOfRange(tag, 0, GCM_IV_BYTES);
+ var encryptedData = Arrays.copyOfRange(tag, GCM_IV_BYTES, tag.length);
+ var cipher = Cipher.getInstance(CIPHER_NAME);
+ var spec = new GCMParameterSpec(GCM_TAG_BITS, iv);
+ cipher.init(Cipher.DECRYPT_MODE, key, spec);
+ return cipher.doFinal(encryptedData);
+ }
- return cipher.doFinal(encrypted);
+ /**
+ * Try GCM decryption first; if the auth tag fails, fall back to the legacy
+ * AES/CBC/PKCS5Padding scheme (static zero IV) used before the GCM
migration.
+ */
+ public static byte[] decryptWithLegacyFallback(byte[] data, Key key)
throws GeneralSecurityException {
+ try {
+ return decrypt(data, key);
+ } catch (AEADBadTagException e) {
+ logger.info("GCM decryption failed, falling back to legacy
AES/CBC");
+ return decryptLegacy(data, key);
+ }
}
- public static String decryptString(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, byte[] encrypted)
- throws GeneralSecurityException, IOException {
+ private static final String LEGACY_CIPHER_NAME = "AES/CBC/PKCS5Padding";
+ private static final int LEGACY_IV_BYTES = 16;
- byte[] decrypted = decrypt(keyStorePath, keyAlias, passwordCallback,
encrypted);
- return new String(decrypted, CHARSET_ENCODING);
+ private static byte[] decryptLegacy(byte[] encrypted, Key key) throws
GeneralSecurityException {
+ var cipher = Cipher.getInstance(LEGACY_CIPHER_NAME);
+ cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(new
byte[LEGACY_IV_BYTES]));
Review Comment:
## Using a static initialization vector for encryption
A [static initialization vector](1) should not be used for encryption.
[Show more
details](https://github.com/apache/airavata/security/code-scanning/128)
##########
airavata-api/src/main/java/org/apache/airavata/security/util/SecurityUtil.java:
##########
@@ -35,52 +39,59 @@
public static final String PASSWORD_HASH_METHOD_PLAINTEXT = "PLAINTEXT";
public static final String CHARSET_ENCODING = "UTF-8";
- public static final String PADDING_MECHANISM = "AES/CBC/PKCS5Padding";
+ public static final String CIPHER_NAME = "AES/GCM/NoPadding";
+ public static final int GCM_IV_BYTES = 12; // 96 bits
+ public static final int GCM_TAG_BITS = 128;
private static final Logger logger =
LoggerFactory.getLogger(SecurityUtil.class);
- public static byte[] encryptString(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, String value)
- throws GeneralSecurityException, IOException {
- return encrypt(keyStorePath, keyAlias, passwordCallback,
value.getBytes(CHARSET_ENCODING));
- }
-
- public static byte[] encrypt(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, byte[] value)
- throws GeneralSecurityException, IOException {
-
- Key secretKey = getSymmetricKey(keyStorePath, keyAlias,
passwordCallback);
-
- Cipher cipher = Cipher.getInstance(PADDING_MECHANISM);
- cipher.init(Cipher.ENCRYPT_MODE, secretKey, new IvParameterSpec(new
byte[16]));
- return cipher.doFinal(value);
- }
-
- private static Key getSymmetricKey(String keyStorePath, String keyAlias,
KeyStorePasswordCallback passwordCallback)
+ public static Key getSymmetricKey(String keyStorePath, String keyAlias,
KeyStorePasswordCallback passwordCallback)
throws CertificateException, NoSuchAlgorithmException,
KeyStoreException, IOException,
UnrecoverableKeyException {
KeyStore ks = SecurityUtil.loadKeyStore(keyStorePath,
passwordCallback);
return ks.getKey(keyAlias,
passwordCallback.getSecretKeyPassPhrase(keyAlias));
}
- public static byte[] decrypt(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, byte[] encrypted)
- throws GeneralSecurityException, IOException {
-
- Key secretKey = getSymmetricKey(keyStorePath, keyAlias,
passwordCallback);
+ public static byte[] encrypt(byte[] data, Key key) throws
GeneralSecurityException {
+ var cipher = Cipher.getInstance(CIPHER_NAME);
+ cipher.init(Cipher.ENCRYPT_MODE, key);
+ var iv = cipher.getIV();
+ var encryptedData = cipher.doFinal(data);
+ return ByteBuffer.allocate(iv.length + encryptedData.length)
+ .put(iv)
+ .put(encryptedData)
+ .array();
+ }
- Cipher cipher = Cipher.getInstance(PADDING_MECHANISM);
- cipher.init(Cipher.DECRYPT_MODE, secretKey, new IvParameterSpec(new
byte[16]));
+ public static byte[] decrypt(byte[] tag, Key key) throws
GeneralSecurityException {
+ var iv = Arrays.copyOfRange(tag, 0, GCM_IV_BYTES);
+ var encryptedData = Arrays.copyOfRange(tag, GCM_IV_BYTES, tag.length);
+ var cipher = Cipher.getInstance(CIPHER_NAME);
+ var spec = new GCMParameterSpec(GCM_TAG_BITS, iv);
+ cipher.init(Cipher.DECRYPT_MODE, key, spec);
+ return cipher.doFinal(encryptedData);
+ }
- return cipher.doFinal(encrypted);
+ /**
+ * Try GCM decryption first; if the auth tag fails, fall back to the legacy
+ * AES/CBC/PKCS5Padding scheme (static zero IV) used before the GCM
migration.
+ */
+ public static byte[] decryptWithLegacyFallback(byte[] data, Key key)
throws GeneralSecurityException {
+ try {
+ return decrypt(data, key);
+ } catch (AEADBadTagException e) {
+ logger.info("GCM decryption failed, falling back to legacy
AES/CBC");
+ return decryptLegacy(data, key);
+ }
}
- public static String decryptString(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, byte[] encrypted)
- throws GeneralSecurityException, IOException {
+ private static final String LEGACY_CIPHER_NAME = "AES/CBC/PKCS5Padding";
+ private static final int LEGACY_IV_BYTES = 16;
- byte[] decrypted = decrypt(keyStorePath, keyAlias, passwordCallback,
encrypted);
- return new String(decrypted, CHARSET_ENCODING);
+ private static byte[] decryptLegacy(byte[] encrypted, Key key) throws
GeneralSecurityException {
+ var cipher = Cipher.getInstance(LEGACY_CIPHER_NAME);
Review Comment:
## Use of a broken or risky cryptographic algorithm
Cryptographic algorithm [AES/CBC/PKCS5Padding](1) is insecure. CBC mode with
PKCS#5 or PKCS#7 padding is vulnerable to padding oracle attacks. Consider
using GCM instead.
[Show more
details](https://github.com/apache/airavata/security/code-scanning/127)
--
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]
