Use AES/GCM instead of AES/CBC for json-crypto session encryption

commons/json-crypto/core/src/main/java/org/forgerock/json/crypto/simple/SimpleDecryptor.java

@@ -12,16 +12,19 @@
  * information: "Portions Copyrighted [year] [name of copyright owner]".
  *
  * Copyright 2011-2015 ForgeRock AS.
+ * Portions Copyrighted 2026 3A Systems, LLC
  */
 
 package org.forgerock.json.crypto.simple;
 
 import javax.crypto.Cipher;
+import javax.crypto.spec.GCMParameterSpec;
 import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 import java.io.IOException;
 import java.security.GeneralSecurityException;
 import java.security.Key;
+import java.security.spec.AlgorithmParameterSpec;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.forgerock.json.crypto.JsonCryptoException;
@@ -39,6 +42,9 @@ public class SimpleDecryptor implements JsonDecryptor {
     /** The type of cryptographic representation that this decryptor supports. */
     public static final String TYPE = "x-simple-encryption";
 
+    /** GCM authentication tag length, in bits, matching {@link SimpleEncryptor}. */
+    private static final int GCM_TAG_LENGTH_BITS = 128;
+
     /** Converts between JSON constructs and Java objects. */
     private final ObjectMapper mapper = new ObjectMapper();
 
@@ -84,13 +90,39 @@ public JsonValue decrypt(JsonValue value) throws JsonCryptoException {
             }
             Cipher symmetric = Cipher.getInstance(cipher);
             String iv = value.get("iv").asString();
-            IvParameterSpec ivps = (iv == null ? null : new IvParameterSpec(Base64.decode(iv)));
-            symmetric.init(Cipher.DECRYPT_MODE, symmetricKey, ivps);
+            symmetric.init(Cipher.DECRYPT_MODE, symmetricKey, parameterSpec(cipher, iv));
             byte[] plaintext = symmetric.doFinal(Base64.decode(value.get("data").required().asString()));
             return new JsonValue(mapper.readValue(plaintext, Object.class));
         } catch (GeneralSecurityException | IOException | JsonValueException e) {
             throw new JsonCryptoException(e);
         }
     }
 
+    /**
+     * Builds the algorithm parameters for the given cipher transformation and
+     * Base64-encoded IV/nonce. GCM requires a {@link GCMParameterSpec}; other
+     * modes (such as CBC) use a plain {@link IvParameterSpec}. Values written
+     * without an IV (for example legacy ECB) yield {@code null}, preserving the
+     * previous behaviour. Because the {@code cipher} field is self-describing,
+     * older CBC/ECB values continue to decrypt unchanged.
+     *
+     * @param cipher the cipher transformation stored with the encrypted value.
+     * @param iv the Base64-encoded IV/nonce, or {@code null} if none was stored.
+     * @return the algorithm parameter spec, or {@code null} when no IV is present.
+     */
+    private static AlgorithmParameterSpec parameterSpec(String cipher, String iv) {
+        if (iv == null) {
+            return null;
+        }
+        byte[] ivBytes = Base64.decode(iv);
+        return isGcm(cipher)
+                ? new GCMParameterSpec(GCM_TAG_LENGTH_BITS, ivBytes)
+                : new IvParameterSpec(ivBytes);
+    }
+
+    private static boolean isGcm(String cipher) {
+        String[] parts = cipher.split("/");
+        return parts.length > 1 && parts[1].equalsIgnoreCase("GCM");
+    }
+
 }

commons/json-crypto/core/src/main/java/org/forgerock/json/crypto/simple/SimpleEncryptor.java

@@ -20,9 +20,11 @@
 import javax.crypto.Cipher;
 import javax.crypto.KeyGenerator;
 import javax.crypto.SecretKey;
+import javax.crypto.spec.GCMParameterSpec;
 import java.io.IOException;
 import java.security.GeneralSecurityException;
 import java.security.Key;
+import java.security.SecureRandom;
 import java.util.HashMap;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -39,6 +41,15 @@ public class SimpleEncryptor implements JsonEncryptor {
     /** The type of cryptographic representation that this encryptor supports. */
     public static final String TYPE = "x-simple-encryption";
 
+    /** GCM authentication tag length, in bits. {@link SimpleDecryptor} assumes the same value. */
+    private static final int GCM_TAG_LENGTH_BITS = 128;
+
+    /** GCM nonce length, in bytes, per NIST SP 800-38D recommendation. */
+    private static final int GCM_NONCE_LENGTH_BYTES = 12;
+
+    /** Generates random GCM nonces. */
+    private static final SecureRandom RANDOM = new SecureRandom();
+
     /** Converts between Java objects and JSON constructs. */
     private final ObjectMapper mapper = new ObjectMapper();
 
@@ -79,7 +90,17 @@ public String getType() {
      */
     private Object symmetric(Object object) throws GeneralSecurityException, IOException {
         Cipher symmetric = Cipher.getInstance(cipher);
-        symmetric.init(Cipher.ENCRYPT_MODE, key);
+        if (isGcm(cipher)) {
+            // Pin the 96-bit nonce and 128-bit tag instead of relying on the
+            // provider's GCM defaults, which the JCE leaves provider-specific;
+            // SimpleDecryptor assumes a 128-bit tag. The key supplied to this
+            // encryptor is caller-managed and typically long-lived, so with
+            // random nonces NIST SP 800-38D section 8.3 bounds GCM to at most
+            // 2^32 encryptions under the same key.
+            symmetric.init(Cipher.ENCRYPT_MODE, key, new GCMParameterSpec(GCM_TAG_LENGTH_BITS, randomNonce()));
+        } else {
+            symmetric.init(Cipher.ENCRYPT_MODE, key);
+        }
         String data = Base64.encode(symmetric.doFinal(mapper.writeValueAsBytes(object)));
         byte[] iv = symmetric.getIV();
         HashMap<String, Object> result = new HashMap<>();
@@ -101,16 +122,22 @@ private Object symmetric(Object object) throws GeneralSecurityException, IOExcep
      * @throws IOException if an I/O exception occurred.
      */
     private Object asymmetric(Object object) throws GeneralSecurityException, IOException {
-        // Use CBC with a random IV rather than ECB; ECB leaks plaintext block
-        // patterns regardless of session-key freshness (CWE-327). The IV is stored
-        // alongside the data and the self-describing "cipher" field keeps this
-        // backward compatible with values previously written using ECB.
-        String symmetricCipher = "AES/CBC/PKCS5Padding";
+        // Use GCM (authenticated encryption) with a random 96-bit nonce rather
+        // than CBC or ECB. GCM provides integrity and is not vulnerable to the
+        // padding-oracle attacks that affect CBC/PKCS#5, nor does it leak block
+        // patterns like ECB (CWE-327). A fresh session key is generated for every
+        // message, so the randomly generated nonce is never reused under the same
+        // key. The nonce and tag length are passed explicitly because the JCE
+        // leaves them provider-specific and SimpleDecryptor assumes a 128-bit
+        // tag. The nonce is stored alongside the data and the self-describing
+        // "cipher" field keeps this backward compatible with values previously
+        // written using CBC or ECB.
+        String symmetricCipher = "AES/GCM/NoPadding";
         KeyGenerator generator = KeyGenerator.getInstance("AES");
         generator.init(128);
         SecretKey sessionKey = generator.generateKey();
         Cipher symmetric = Cipher.getInstance(symmetricCipher);
-        symmetric.init(Cipher.ENCRYPT_MODE, sessionKey);
+        symmetric.init(Cipher.ENCRYPT_MODE, sessionKey, new GCMParameterSpec(GCM_TAG_LENGTH_BITS, randomNonce()));
         String data = Base64.encode(symmetric.doFinal(mapper.writeValueAsBytes(object)));
         byte[] iv = symmetric.getIV();
         Cipher asymmetric = Cipher.getInstance(cipher);
@@ -129,6 +156,17 @@ private Object asymmetric(Object object) throws GeneralSecurityException, IOExce
         return result;
     }
 
+    private static byte[] randomNonce() {
+        byte[] nonce = new byte[GCM_NONCE_LENGTH_BYTES];
+        RANDOM.nextBytes(nonce);
+        return nonce;
+    }
+
+    private static boolean isGcm(String cipher) {
+        String[] parts = cipher.split("/");
+        return parts.length > 1 && parts[1].equalsIgnoreCase("GCM");
+    }
+
     @Override
     public JsonValue encrypt(JsonValue value) throws JsonCryptoException {
         Object object = value.getObject();

commons/json-crypto/core/src/test/java/org/forgerock/json/crypto/JsonCryptoTest.java

@@ -21,6 +21,8 @@
 
 import javax.crypto.KeyGenerator;
 import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
 import java.security.Key;
 import java.security.KeyPair;
@@ -44,12 +46,32 @@ public class JsonCryptoTest {
 
     private static final String SYMMETRIC_CIPHER = "AES/CBC/PKCS5Padding";
 
+    private static final String SYMMETRIC_GCM_CIPHER = "AES/GCM/NoPadding";
+
     private static final String ASYMMETRIC_CIPHER = "RSA/ECB/OAEPWithSHA1AndMGF1Padding";
 
     private static final String PASSWORD = "P@55W0RD";
 
     private static final String PLAINTEXT = "Lorem ipsum dolor sit amet, consectetur adipiscing elit.";
 
+    /**
+     * Fixed AES key for the checked-in legacy values below; unlike the random
+     * per-run keys, it lets ciphertexts produced by pre-GCM builds be kept in
+     * the test as backward-compatibility fixtures.
+     */
+    private static final SecretKey COMPAT_KEY =
+            new SecretKeySpec("0123456789abcdef".getBytes(StandardCharsets.US_ASCII), "AES");
+
+    /** {@link #PLAINTEXT} encrypted with {@link #COMPAT_KEY} using legacy AES/ECB/PKCS5Padding (no IV). */
+    private static final String LEGACY_ECB_DATA =
+            "8sDxsiLqBe63ul0Pup+Bv3CREJeNI+bpMydH8hnajinHixZAmDwzoNGBDpyFApurpvwOZa6bF153jD2Rk0Jwfg==";
+
+    /** {@link #PLAINTEXT} encrypted with {@link #COMPAT_KEY} and {@link #LEGACY_CBC_IV} using AES/CBC/PKCS5Padding. */
+    private static final String LEGACY_CBC_DATA =
+            "kgxfhkMPxKtmhfmxQt/p8bCj4bIZPH1s/AwAmVANAAuYLGRosaLsNrfclIosTJYKWAAGygDF/dkzs+2ye2+1tw==";
+
+    private static final String LEGACY_CBC_IV = "ZmVkY2JhOTg3NjU0MzIxMA==";
+
     private SecretKey secretKey;
 
     private PublicKey publicKey;
@@ -62,6 +84,8 @@ public class JsonCryptoTest {
                 return secretKey;
             } else if (key.equals("privateKey")) {
                 return privateKey;
+            } else if (key.equals("compatKey")) {
+                return COMPAT_KEY;
             } else {
                 return null;
             }
@@ -102,10 +126,48 @@ public void testAsymmetricEncryption() throws JsonCryptoException {
         JsonValue value = new JsonValue(PLAINTEXT);
         value = new SimpleEncryptor(ASYMMETRIC_CIPHER, publicKey, "privateKey").encrypt(value);
         assertThat(value.getObject()).isNotEqualTo(PLAINTEXT);
+        assertThat(((Map<?, ?>) value.getObject()).get("cipher")).isEqualTo(SYMMETRIC_GCM_CIPHER);
+        value = new SimpleDecryptor(selector).decrypt(value);
+        assertThat(value.getObject()).isEqualTo(PLAINTEXT);
+    }
+
+    @Test
+    public void testSymmetricGcmEncryption() throws JsonCryptoException {
+        JsonValue value = new JsonValue(PLAINTEXT);
+        value = new SimpleEncryptor(SYMMETRIC_GCM_CIPHER, secretKey, "secretKey").encrypt(value);
+        assertThat(value.getObject()).isNotEqualTo(PLAINTEXT);
+        assertThat(((Map<?, ?>) value.getObject()).get("cipher")).isEqualTo(SYMMETRIC_GCM_CIPHER);
+        value = new SimpleDecryptor(selector).decrypt(value);
+        assertThat(value.getObject()).isEqualTo(PLAINTEXT);
+    }
+
+    // ----- backward compatibility ----------
+
+    @Test
+    public void testDecryptLegacyEcbValue() throws JsonCryptoException {
+        JsonValue value = new JsonValue(legacyValue("AES/ECB/PKCS5Padding", LEGACY_ECB_DATA, null));
         value = new SimpleDecryptor(selector).decrypt(value);
         assertThat(value.getObject()).isEqualTo(PLAINTEXT);
     }
 
+    @Test
+    public void testDecryptLegacyCbcValue() throws JsonCryptoException {
+        JsonValue value = new JsonValue(legacyValue(SYMMETRIC_CIPHER, LEGACY_CBC_DATA, LEGACY_CBC_IV));
+        value = new SimpleDecryptor(selector).decrypt(value);
+        assertThat(value.getObject()).isEqualTo(PLAINTEXT);
+    }
+
+    private static Map<String, Object> legacyValue(String cipher, String data, String iv) {
+        HashMap<String, Object> result = new HashMap<>();
+        result.put("cipher", cipher);
+        result.put("key", "compatKey");
+        result.put("data", data);
+        if (iv != null) {
+            result.put("iv", iv);
+        }
+        return result;
+    }
+
 //    @Test
 //    public void testJsonCryptoTransformer() throws JsonCryptoException {
 //        JsonValue value = new JsonValue(PLAINTEXT);