github-advanced-security[bot] commented on code in PR #610:
URL: https://github.com/apache/airavata/pull/610#discussion_r3012146791
##########
airavata-api/src/main/java/org/apache/airavata/security/util/SecurityUtil.java:
##########
@@ -35,52 +38,47 @@
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);
-
- Cipher cipher = Cipher.getInstance(PADDING_MECHANISM);
- cipher.init(Cipher.DECRYPT_MODE, secretKey, new IvParameterSpec(new
byte[16]));
-
- return cipher.doFinal(encrypted);
+ 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();
}
- public static String decryptString(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, byte[] encrypted)
- throws GeneralSecurityException, IOException {
+ 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);
+ }
- byte[] decrypted = decrypt(keyStorePath, keyAlias, passwordCallback,
encrypted);
- return new String(decrypted, CHARSET_ENCODING);
+ /**
+ * Decrypt using the legacy AES/CBC/PKCS5Padding scheme with a static zero
IV.
+ * Used only by the migration script to read old credentials.
+ */
+ public static byte[] decryptLegacy(byte[] encrypted, Key key) throws
GeneralSecurityException {
+ var cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
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/129)
##########
airavata-api/src/main/java/org/apache/airavata/security/util/SecurityUtil.java:
##########
@@ -35,52 +38,47 @@
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);
-
- Cipher cipher = Cipher.getInstance(PADDING_MECHANISM);
- cipher.init(Cipher.DECRYPT_MODE, secretKey, new IvParameterSpec(new
byte[16]));
-
- return cipher.doFinal(encrypted);
+ 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();
}
- public static String decryptString(
- String keyStorePath, String keyAlias, KeyStorePasswordCallback
passwordCallback, byte[] encrypted)
- throws GeneralSecurityException, IOException {
+ 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);
+ }
- byte[] decrypted = decrypt(keyStorePath, keyAlias, passwordCallback,
encrypted);
- return new String(decrypted, CHARSET_ENCODING);
+ /**
+ * Decrypt using the legacy AES/CBC/PKCS5Padding scheme with a static zero
IV.
+ * Used only by the migration script to read old credentials.
+ */
+ public static byte[] decryptLegacy(byte[] encrypted, Key key) throws
GeneralSecurityException {
+ var cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
+ cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(new
byte[16]));
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/130)
--
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]
