8305091: Change ChaCha20 cipher init behavior to match AES-GCM

Reviewed-by: djelinski, ascarpino

Author: Jamil Nimeh

src/java.base/share/classes/com/sun/crypto/provider/ChaCha20Cipher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -87,6 +87,7 @@ abstract class ChaCha20Cipher extends CipherSpi {
     // The counter
     private static final long MAX_UINT32 = 0x00000000FFFFFFFFL;
     private long finalCounterValue;
+    private long initCounterValue;
     private long counter;
 
     // The base state is created at initialization time as a 16-int array
@@ -336,7 +337,9 @@ protected void engineInit(int opmode, Key key,
                 }
                 ChaCha20ParameterSpec chaParams = (ChaCha20ParameterSpec)params;
                 newNonce = chaParams.getNonce();
-                counter = ((long)chaParams.getCounter()) & 0x00000000FFFFFFFFL;
+                initCounterValue = ((long)chaParams.getCounter()) &
+                        0x00000000FFFFFFFFL;
+                counter = initCounterValue;
                 break;
             case MODE_AEAD:
                 if (!(params instanceof IvParameterSpec)) {
@@ -545,9 +548,12 @@ private void init(int opmode, Key key, byte[] newNonce)
         }
 
         // Make sure that the provided key and nonce are unique before
-        // assigning them to the object.
+        // assigning them to the object.  Key and nonce uniqueness
+        // protection is for encryption operations only.
         byte[] newKeyBytes = getEncodedKey(key);
-        checkKeyAndNonce(newKeyBytes, newNonce);
+        if (opmode == Cipher.ENCRYPT_MODE) {
+            checkKeyAndNonce(newKeyBytes, newNonce);
+        }
         if (this.keyBytes != null) {
             Arrays.fill(this.keyBytes, (byte)0);
         }
@@ -704,9 +710,8 @@ protected byte[] engineDoFinal(byte[] in, int inOfs, int inLen)
         } catch (ShortBufferException | KeyException exc) {
             throw new RuntimeException(exc);
         } finally {
-            // Regardless of what happens, the cipher cannot be used for
-            // further processing until it has been freshly initialized.
-            initialized = false;
+            // Reset the cipher's state to post-init values.
+            resetStartState();
         }
         return output;
     }
@@ -742,9 +747,8 @@ protected int engineDoFinal(byte[] in, int inOfs, int inLen, byte[] out,
         } catch (KeyException ke) {
             throw new RuntimeException(ke);
         } finally {
-            // Regardless of what happens, the cipher cannot be used for
-            // further processing until it has been freshly initialized.
-            initialized = false;
+            // Reset the cipher's state to post-init values.
+            resetStartState();
         }
         return bytesUpdated;
     }
@@ -1170,6 +1174,23 @@ private void authWriteLengths(long aLen, long dLen, byte[] buf) {
         asLongLittleEndian.set(buf, Long.BYTES, dLen);
     }
 
+    /**
+     * reset the Cipher's state to the values it had after
+     * the initial init() call.
+     *
+     * Note: The cipher's internal "initialized" field is set differently
+     * for ENCRYPT_MODE and DECRYPT_MODE in order to allow DECRYPT_MODE
+     * ciphers to reuse the key/nonce/counter values.  This kind of reuse
+     * is disallowed in ENCRYPT_MODE.
+     */
+    private void resetStartState() {
+        keyStrLimit = 0;
+        keyStrOffset = 0;
+        counter = initCounterValue;
+        aadDone = false;
+        initialized = (direction == Cipher.DECRYPT_MODE);
+    }
+
     /**
      * Interface for the underlying processing engines for ChaCha20
      */
@@ -1275,7 +1296,8 @@ public int getOutputSize(int inLength, boolean isFinal) {
 
         private EngineAEADEnc() throws InvalidKeyException {
             initAuthenticator();
-            counter = 1;
+            initCounterValue = 1;
+            counter = initCounterValue;
         }
 
         @Override
@@ -1347,7 +1369,8 @@ public int getOutputSize(int inLen, boolean isFinal) {
 
         private EngineAEADDec() throws InvalidKeyException {
             initAuthenticator();
-            counter = 1;
+            initCounterValue = 1;
+            counter = initCounterValue;
             cipherBuf = new ByteArrayOutputStream(CIPHERBUF_BASE);
             tag = new byte[TAG_LENGTH];
         }

test/jdk/com/sun/crypto/provider/Cipher/ChaCha20/ChaCha20NoReuse.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug 8153029
+ * @bug 8153029 8305091
  * @library /test/lib
  * @run main ChaCha20NoReuse
  * @summary ChaCha20 Cipher Implementation (key/nonce reuse protection)
@@ -376,26 +376,20 @@ public boolean run(String algorithm) {
                 }
                 SecretKey key = new SecretKeySpec(testData.key, ALG_CC20);
 
-                // Initialize and encrypt
+                // Initialize and decrypt
                 cipher.init(testData.direction, key, spec);
                 if (algorithm.equals(ALG_CC20_P1305)) {
                     cipher.updateAAD(testData.aad);
                 }
                 cipher.doFinal(testData.input);
                 System.out.println("First decryption complete");
 
-                // Now attempt to encrypt again without changing the key/IV
-                // This should fail.
-                try {
-                    if (algorithm.equals(ALG_CC20_P1305)) {
-                        cipher.updateAAD(testData.aad);
-                    }
-                    cipher.doFinal(testData.input);
-                    throw new RuntimeException(
-                            "Expected IllegalStateException not thrown");
-                } catch (IllegalStateException ise) {
-                    // Do nothing, this is what we expected to happen
+                // Now attempt to decrypt again without changing the key/IV
+                // We allow this scenario.
+                if (algorithm.equals(ALG_CC20_P1305)) {
+                    cipher.updateAAD(testData.aad);
                 }
+                cipher.doFinal(testData.input);
             } catch (Exception exc) {
                 System.out.println("Unexpected exception: " + exc);
                 exc.printStackTrace();
@@ -408,7 +402,8 @@ public boolean run(String algorithm) {
 
     /**
      * Perform an AEAD decryption with corrupted data so the tag does not
-     * match.  Then attempt to reuse the cipher without initialization.
+     * match.  Then use the uncorrupted test vector input and attempt to
+     * reuse the cipher without initialization.
      */
     public static final TestMethod decFailNoInit = new TestMethod() {
         @Override
@@ -441,16 +436,16 @@ public boolean run(String algorithm) {
                     System.out.println("Expected decryption failure occurred");
                 }
 
-                // Make sure that despite the exception, the Cipher object is
-                // not in a state that would leave it initialized and able
-                // to process future decryption operations without init.
-                try {
-                    cipher.updateAAD(testData.aad);
-                    cipher.doFinal(testData.input);
-                    throw new RuntimeException(
-                            "Expected IllegalStateException not thrown");
-                } catch (IllegalStateException ise) {
-                    // Do nothing, this is what we expected to happen
+                // Even though an exception occurred during decryption, the
+                // Cipher object should be returned to its post-init state.
+                // Since this is a decryption operation, we should allow
+                // key/nonce reuse.  It should properly decrypt the uncorrupted
+                // input.
+                cipher.updateAAD(testData.aad);
+                byte[] pText = cipher.doFinal(testData.input);
+                if (!Arrays.equals(pText, testData.expOutput)) {
+                    throw new RuntimeException("FAIL: Attempted decryption " +
+                            "did not match expected plaintext");
                 }
             } catch (Exception exc) {
                 System.out.println("Unexpected exception: " + exc);
@@ -562,18 +557,17 @@ public boolean run(String algorithm) {
                 if (algorithm.equals(ALG_CC20_P1305)) {
                     cipher.updateAAD(testData.aad);
                 }
-                cipher.doFinal(testData.input);
+                byte[] pText = cipher.doFinal(testData.input);
+                if (!Arrays.equals(pText, testData.expOutput)) {
+                    throw new RuntimeException("FAIL: Attempted decryption " +
+                            "did not match expected plaintext");
+                }
                 System.out.println("First decryption complete");
 
                 // Initializing after the completed decryption with
-                // the same key and nonce should fail.
-                try {
-                    cipher.init(testData.direction, key, spec);
-                    throw new RuntimeException(
-                            "Expected InvalidKeyException not thrown");
-                } catch (InvalidKeyException ike) {
-                    // Do nothing, this is what we expected to happen
-                }
+                // the same key and nonce is allowed.
+                cipher.init(testData.direction, key, spec);
+                System.out.println("Successful reinit in DECRYPT_MODE");
             } catch (Exception exc) {
                 System.out.println("Unexpected exception: " + exc);
                 exc.printStackTrace();