Add Cipher validation to AES GCM SIV.

This resolved the issue:
#18.

#tinkApiChange
Encryption and decryption with AES GCM SIV on Android API versions 29 and older will now fail, instead of using the wrong algorithm.

For each thread this primitive is used with, a decryption will be run to validate the cipher. We don't expect this to be a big performance penalty, as creating and starting a thread will be more costly than this additional check.

This check could also be done on initialization of the class, or on creation of the object. But I'm worried that this might cause problems in rare cases when initialization is done before AES GCM SIV is registered. So I prefer not to do this.

This could be solved in a completely different way, by checking the state of the system (for example, which android version is used, or which provides are available). But I think such an approach would be more brittle than this solution here.

PiperOrigin-RevId: 585546163
Change-Id: I457ea40b21cf702688c9b29e4a9e286428bf0a39

Author: juergw

src/main/java/com/google/crypto/tink/aead/subtle/AesGcmSiv.java

@@ -25,6 +25,7 @@
 import com.google.crypto.tink.annotations.Alpha;
 import com.google.crypto.tink.subtle.Bytes;
 import com.google.crypto.tink.subtle.EngineFactory;
+import com.google.crypto.tink.subtle.Hex;
 import com.google.crypto.tink.subtle.Random;
 import com.google.crypto.tink.subtle.SubtleUtil;
 import com.google.crypto.tink.subtle.Validators;
@@ -49,12 +50,41 @@
  */
 @Alpha
 public final class AesGcmSiv implements Aead {
-  private static final ThreadLocal<Cipher> localCipher =
+
+  // Test vector from https://www.rfc-editor.org/rfc/rfc8452.html#appendix-C.1
+  private static final byte[] TEST_PLAINTEXT = Hex.decode("7a806c");
+  private static final byte[] TEST_AAD = Hex.decode("46bb91c3c5");
+  private static final byte[] TEST_KEY = Hex.decode("36864200e0eaf5284d884a0e77d31646");
+  private static final byte[] TEST_NOUNCE = Hex.decode("bae8e37fc83441b16034566b");
+  private static final byte[] TEST_RESULT = Hex.decode("af60eb711bd85bc1e4d3e0a462e074eea428a8");
+
+  // On Android API version 29 and older, the security provider returns an AES GCM cipher instead
+  // an AES GCM SIV cipher. This function tests if we have a correct cipher.
+  private static boolean isAesGcmSivCipher(Cipher cipher) {
+    try {
+      // Use test vector to validate that cipher implements AES GCM SIV.
+      AlgorithmParameterSpec params = getParams(TEST_NOUNCE);
+      cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(TEST_KEY, "AES"), params);
+      cipher.updateAAD(TEST_AAD);
+      byte[] output = cipher.doFinal(TEST_RESULT, 0, TEST_RESULT.length);
+      return Bytes.equal(output, TEST_PLAINTEXT);
+    } catch (GeneralSecurityException ex) {
+      return false;
+    }
+  }
+
+  // localAesGcmSivCipher.get() may be null if the cipher returned by EngineFactory is not a valid
+  // AES GCM SIV cipher.
+  private static final ThreadLocal<Cipher> localAesGcmSivCipher =
       new ThreadLocal<Cipher>() {
         @Override
         protected Cipher initialValue() {
           try {
-            return EngineFactory.CIPHER.getInstance("AES/GCM-SIV/NoPadding");
+            Cipher cipher = EngineFactory.CIPHER.getInstance("AES/GCM-SIV/NoPadding");
+            if (!isAesGcmSivCipher(cipher)) {
+              return null;
+            }
+            return cipher;
           } catch (GeneralSecurityException ex) {
             throw new IllegalStateException(ex);
           }
@@ -86,8 +116,17 @@ public AesGcmSiv(final byte[] key) throws GeneralSecurityException {
     this(key, new byte[0]);
   }
 
+  private Cipher getAesGcmSivCipher() throws GeneralSecurityException {
+    Cipher cipher = localAesGcmSivCipher.get();
+    if (cipher == null) {
+      throw new GeneralSecurityException("AES GCM SIV cipher is not available or is invalid.");
+    }
+    return cipher;
+  }
+
   private byte[] rawEncrypt(final byte[] plaintext, final byte[] associatedData)
       throws GeneralSecurityException {
+    Cipher cipher = getAesGcmSivCipher();
     // Check that ciphertext is not longer than the max. size of a Java array.
     if (plaintext.length > Integer.MAX_VALUE - IV_SIZE_IN_BYTES - TAG_SIZE_IN_BYTES) {
       throw new GeneralSecurityException("plaintext too long");
@@ -97,12 +136,11 @@ private byte[] rawEncrypt(final byte[] plaintext, final byte[] associatedData)
     System.arraycopy(iv, 0, ciphertext, 0, IV_SIZE_IN_BYTES);
 
     AlgorithmParameterSpec params = getParams(iv);
-    localCipher.get().init(Cipher.ENCRYPT_MODE, keySpec, params);
+    cipher.init(Cipher.ENCRYPT_MODE, keySpec, params);
     if (associatedData != null && associatedData.length != 0) {
-      localCipher.get().updateAAD(associatedData);
+      cipher.updateAAD(associatedData);
     }
-    int written =
-        localCipher.get().doFinal(plaintext, 0, plaintext.length, ciphertext, IV_SIZE_IN_BYTES);
+    int written = cipher.doFinal(plaintext, 0, plaintext.length, ciphertext, IV_SIZE_IN_BYTES);
     // For security reasons, AES-GCM encryption must always use tag of TAG_SIZE_IN_BYTES bytes. If
     // so, written must be equal to plaintext.length + TAG_SIZE_IN_BYTES.
 
@@ -134,18 +172,17 @@ public byte[] encrypt(final byte[] plaintext, final byte[] associatedData)
 
   private byte[] rawDecrypt(final byte[] ciphertext, final byte[] associatedData)
       throws GeneralSecurityException {
+    Cipher cipher = getAesGcmSivCipher();
     if (ciphertext.length < IV_SIZE_IN_BYTES + TAG_SIZE_IN_BYTES) {
       throw new GeneralSecurityException("ciphertext too short");
     }
 
     AlgorithmParameterSpec params = getParams(ciphertext, 0, IV_SIZE_IN_BYTES);
-    localCipher.get().init(Cipher.DECRYPT_MODE, keySpec, params);
+    cipher.init(Cipher.DECRYPT_MODE, keySpec, params);
     if (associatedData != null && associatedData.length != 0) {
-      localCipher.get().updateAAD(associatedData);
+      cipher.updateAAD(associatedData);
     }
-    return localCipher
-        .get()
-        .doFinal(ciphertext, IV_SIZE_IN_BYTES, ciphertext.length - IV_SIZE_IN_BYTES);
+    return cipher.doFinal(ciphertext, IV_SIZE_IN_BYTES, ciphertext.length - IV_SIZE_IN_BYTES);
   }
 
   /**

src/main/java/com/google/crypto/tink/aead/subtle/BUILD.bazel

@@ -33,6 +33,7 @@ java_library(
         "//src/main/java/com/google/crypto/tink/annotations:alpha",
         "//src/main/java/com/google/crypto/tink/internal:util",
         "//src/main/java/com/google/crypto/tink/subtle:bytes",
+        "//src/main/java/com/google/crypto/tink/subtle:hex",
         "//src/main/java/com/google/crypto/tink/subtle:random",
         "//src/main/java/com/google/crypto/tink/subtle:subtle_util_cluster",
         "//src/main/java/com/google/crypto/tink/subtle:validators",
@@ -70,6 +71,7 @@ android_library(
         "//src/main/java/com/google/crypto/tink/annotations:alpha-android",
         "//src/main/java/com/google/crypto/tink/internal:util-android",
         "//src/main/java/com/google/crypto/tink/subtle:bytes-android",
+        "//src/main/java/com/google/crypto/tink/subtle:hex-android",
         "//src/main/java/com/google/crypto/tink/subtle:random-android",
         "//src/main/java/com/google/crypto/tink/subtle:subtle_util_cluster-android",
         "//src/main/java/com/google/crypto/tink/subtle:validators-android",

src/test/java/com/google/crypto/tink/aead/AesGcmSivKeyManagerTest.java

@@ -142,6 +142,9 @@ public void createKey_multipleTimes() throws Exception {
 
   @Test
   public void getPrimitive() throws Exception {
+    @Nullable Integer apiLevel = Util.getAndroidApiLevel();
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
+
     AesGcmSivKey key = factory.createKey(AesGcmSivKeyFormat.newBuilder().setKeySize(16).build());
     Aead managerAead = manager.getPrimitive(key, Aead.class);
     Aead directAead = new AesGcmSiv(key.getKeyValue().toByteArray());
@@ -154,6 +157,9 @@ public void getPrimitive() throws Exception {
 
   @Test
   public void testCiphertextSize() throws Exception {
+    @Nullable Integer apiLevel = Util.getAndroidApiLevel();
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
+
     AesGcmSivKey key = factory.createKey(AesGcmSivKeyFormat.newBuilder().setKeySize(32).build());
     Aead aead = new AesGcmSivKeyManager().getPrimitive(key, Aead.class);
     byte[] plaintext = "plaintext".getBytes(UTF_8);
@@ -290,6 +296,9 @@ public void testCreateKeyFromRandomness_slowInputStream_works() throws Exception
 
   @Test
   public void testEncryptDecrypt_works() throws Exception {
+    @Nullable Integer apiLevel = Util.getAndroidApiLevel();
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
+
     com.google.crypto.tink.aead.AesGcmSivKey aesGcmSivKey =
         com.google.crypto.tink.aead.AesGcmSivKey.builder()
             .setParameters(
@@ -315,13 +324,11 @@ public void testEncryptDecrypt_works() throws Exception {
   }
 
   @Test
-  public void testGetPrimitiveReturnsAesGcmBeforeAndroid30() throws Exception {
+  public void testEncryptAndDecryptFailBeforeAndroid30() throws Exception {
     @Nullable Integer apiLevel = Util.getAndroidApiLevel();
     Assume.assumeNotNull(apiLevel);
     Assume.assumeTrue(apiLevel < 30);
 
-    // TODO(b/303637541) This is a bug, it should instead throw an error.
-
     // Use an AES GCM test vector from AesGcmJceTest.testWithAesGcmKey_noPrefix_works
     byte[] keyBytes = Hex.decode("5b9604fe14eadba931b0ccf34843dab9");
     AesGcmSivParameters parameters =
@@ -341,7 +348,61 @@ public void testGetPrimitiveReturnsAesGcmBeforeAndroid30() throws Exception {
             .build();
     Aead aead = keysetHandle.getPrimitive(Aead.class);
 
+    assertThrows(GeneralSecurityException.class, () -> aead.encrypt(new byte[] {}, new byte[] {}));
     byte[] fixedCiphertext = Hex.decode("c3561ce7f48b8a6b9b8d5ef957d2e512368f7da837bcf2aeebe176e3");
-    assertThat(aead.decrypt(fixedCiphertext, new byte[] {})).isEmpty();
+    assertThrows(
+        GeneralSecurityException.class, () -> aead.decrypt(fixedCiphertext, new byte[] {}));
+  }
+
+  // This test shows how ciphertexts created with older versions of Tink on older versions of
+  // Android can still be decrypted with the current version of Tink.
+  @Test
+  public void testDecryptCiphertextCreatedOnOlderVersionOfAndroid() throws Exception {
+    @Nullable Integer apiLevel = Util.getAndroidApiLevel();
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
+
+    // A valid AES GCM SIV key.
+    com.google.crypto.tink.aead.AesGcmSivKey aesGcmSivKey =
+        com.google.crypto.tink.aead.AesGcmSivKey.builder()
+            .setParameters(
+                AesGcmSivParameters.builder()
+                    .setKeySizeBytes(16)
+                    .setVariant(AesGcmSivParameters.Variant.NO_PREFIX)
+                    .build())
+            .setKeyBytes(
+                SecretBytes.copyFrom(
+                    Hex.decode("5b9604fe14eadba931b0ccf34843dab9"), InsecureSecretKeyAccess.get()))
+            .build();
+
+    // Valid ciphertext of an empty plaintext created with aesGcmSivKey.
+    byte[] validCiphertext = Hex.decode("17871550708697c27881d04753337526f2bed57b7e2eac30ecde0202");
+
+    // Ciphertext created with aesGcmSivKey on Android version 29 before
+    // https://github.com/tink-crypto/tink-java/issues/18 was fixed.
+    byte[] legacyCiphertext =
+        Hex.decode("c3561ce7f48b8a6b9b8d5ef957d2e512368f7da837bcf2aeebe176e3");
+
+    // Create an Aead instance that can decrypt in both AES GCM and AES GCM SIV.
+    com.google.crypto.tink.aead.AesGcmKey legacyKey =
+        com.google.crypto.tink.aead.AesGcmKey.builder()
+            .setParameters(
+                AesGcmParameters.builder()
+                    .setIvSizeBytes(12)
+                    .setKeySizeBytes(16)
+                    .setTagSizeBytes(16)
+                    .setVariant(AesGcmParameters.Variant.NO_PREFIX)
+                    .build())
+            .setKeyBytes(aesGcmSivKey.getKeyBytes())
+            .build();
+    KeysetHandle backwardsCompatibleKeysetHandle =
+        KeysetHandle.newBuilder()
+            .addEntry(KeysetHandle.importKey(aesGcmSivKey).withRandomId().makePrimary())
+            .addEntry(KeysetHandle.importKey(legacyKey).withRandomId())
+            .build();
+    Aead backwardsCompatibleAead = backwardsCompatibleKeysetHandle.getPrimitive(Aead.class);
+
+    // Check that backwardsCompatibleAead can decrypt both valid and legacy ciphertexts.
+    assertThat(backwardsCompatibleAead.decrypt(validCiphertext, new byte[] {})).isEmpty();
+    assertThat(backwardsCompatibleAead.decrypt(legacyCiphertext, new byte[] {})).isEmpty();
   }
 }

src/test/java/com/google/crypto/tink/aead/subtle/AesGcmSivTest.java

@@ -74,6 +74,9 @@ public void setUpConscrypt() throws Exception {
 
   @Test
   public void testEncryptDecrypt() throws Exception {
+    @Nullable Integer apiLevel = Util.getAndroidApiLevel();
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
+
     byte[] aad = new byte[] {1, 2, 3};
     for (int keySize : keySizeInBytes) {
       byte[] key = Random.randBytes(keySize);
@@ -111,6 +114,9 @@ public void testLongMessages() throws Exception {
 
   @Test
   public void testModifyCiphertext() throws Exception {
+    @Nullable Integer apiLevel = Util.getAndroidApiLevel();
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
+
     byte[] aad = Random.randBytes(33);
     byte[] key = Random.randBytes(16);
     byte[] message = Random.randBytes(32);
@@ -215,6 +221,9 @@ public void testWycheproofVectors() throws Exception {
 
   @Test
   public void testNullPlaintextOrCiphertext() throws Exception {
+    @Nullable Integer apiLevel = Util.getAndroidApiLevel();
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
+
     for (int keySize : keySizeInBytes) {
       AesGcmSiv gcm = new AesGcmSiv(Random.randBytes(keySize));
       byte[] aad = new byte[] {1, 2, 3};
@@ -243,6 +252,9 @@ public void testNullPlaintextOrCiphertext() throws Exception {
 
   @Test
   public void testEmptyAssociatedData() throws Exception {
+    @Nullable Integer apiLevel = Util.getAndroidApiLevel();
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
+
     byte[] aad = new byte[0];
     for (int keySize : keySizeInBytes) {
       byte[] key = Random.randBytes(keySize);
@@ -285,6 +297,9 @@ public void testEmptyAssociatedData() throws Exception {
    * multiple ciphertexts of the same message are distinct.
    */
   public void testRandomNonce() throws Exception {
+    @Nullable Integer apiLevel = Util.getAndroidApiLevel();
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
+
     final int samples = 1 << 17;
     byte[] key = Random.randBytes(16);
     byte[] message = new byte[0];
@@ -300,13 +315,11 @@ public void testRandomNonce() throws Exception {
   }
 
   @Test
-  public void testCreateImplementsAesGcmBeforeAndroid30() throws Exception {
+  public void testCreate_encryptAndDecryptFailBeforeAndroid30() throws Exception {
     @Nullable Integer apiLevel = Util.getAndroidApiLevel();
     Assume.assumeNotNull(apiLevel);
     Assume.assumeTrue(apiLevel < 30);
 
-    // TODO(b/303637541) This is a bug, it should instead throw an error.
-
     // Use an AES GCM test vector from AesGcmJceTest.testWithAesGcmKey_noPrefix_works
     byte[] keyBytes = Hex.decode("5b9604fe14eadba931b0ccf34843dab9");
     AesGcmSivParameters parameters =
@@ -321,8 +334,10 @@ public void testCreateImplementsAesGcmBeforeAndroid30() throws Exception {
             .build();
     Aead aead = AesGcmSiv.create(key);
 
+    assertThrows(GeneralSecurityException.class, () -> aead.encrypt(new byte[] {}, new byte[] {}));
     byte[] fixedCiphertext = Hex.decode("c3561ce7f48b8a6b9b8d5ef957d2e512368f7da837bcf2aeebe176e3");
-    assertThat(aead.decrypt(fixedCiphertext, new byte[] {})).isEmpty();
+    assertThrows(
+        GeneralSecurityException.class, () -> aead.decrypt(fixedCiphertext, new byte[] {}));
   }
 
   @Test
@@ -395,8 +410,7 @@ public void testWithAesGcmSivKey_tinkPrefix_works() throws Exception {
   @Test
   public void testWithAesGcmSivKey_crunchyPrefix_works() throws Exception {
     @Nullable Integer apiLevel = Util.getAndroidApiLevel();
-    Assume.assumeNotNull(apiLevel);
-    Assume.assumeTrue(apiLevel >= 30);
+    Assume.assumeTrue(apiLevel == null || apiLevel >= 30); // Run the test on java and android >= 30
 
     // Test vector draft-irtf-cfrg-gcmsiv-09 in Wycheproof
     byte[] plaintext = Hex.decode("7a806c");