8257769: Cipher.getParameters() throws NPE for ChaCha20-Poly1305

Reviewed-by: mullan, valeriep

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, 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
@@ -207,7 +207,7 @@ abstract class ChaCha20Cipher extends CipherSpi {
      */
     @Override
     protected byte[] engineGetIV() {
-        return nonce.clone();
+        return (nonce != null) ? nonce.clone() : null;
     }
 
     /**
@@ -226,11 +226,16 @@ abstract class ChaCha20Cipher extends CipherSpi {
     protected AlgorithmParameters engineGetParameters() {
         AlgorithmParameters params = null;
         if (mode == MODE_AEAD) {
+            // In a pre-initialized state or any state without a nonce value
+            // this call should cause a random nonce to be generated, but
+            // not attached to the object.
+            byte[] nonceData = (initialized || nonce != null) ? nonce :
+                    createRandomNonce(null);
             try {
                 // Place the 12-byte nonce into a DER-encoded OCTET_STRING
                 params = AlgorithmParameters.getInstance("ChaCha20-Poly1305");
                 params.init((new DerValue(
-                        DerValue.tag_OctetString, nonce).toByteArray()));
+                        DerValue.tag_OctetString, nonceData).toByteArray()));
             } catch (NoSuchAlgorithmException | IOException exc) {
                 throw new RuntimeException(exc);
             }
@@ -504,7 +509,7 @@ abstract class ChaCha20Cipher extends CipherSpi {
      *
      * @return a 12-byte array containing the random nonce.
      */
-    private byte[] createRandomNonce(SecureRandom random) {
+    private static byte[] createRandomNonce(SecureRandom random) {
         byte[] newNonce = new byte[12];
         SecureRandom rand = (random != null) ? random : new SecureRandom();
         rand.nextBytes(newNonce);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, 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 8257769
  * @library /test/lib
  * @build jdk.test.lib.Convert
  * @run main ChaCha20Poly1305ParamTest
@@ -42,6 +42,10 @@ import java.security.spec.AlgorithmParameterSpec;
 import java.security.AlgorithmParameters;
 import java.security.NoSuchAlgorithmException;
 import java.nio.ByteBuffer;
+import java.security.InvalidKeyException;
+import java.security.MessageDigest;
+import java.security.spec.InvalidParameterSpecException;
+import javax.crypto.spec.IvParameterSpec;
 import jdk.test.lib.Convert;
 
 public class ChaCha20Poly1305ParamTest {
@@ -232,6 +236,111 @@ public class ChaCha20Poly1305ParamTest {
             System.out.println("Caught expected exception: " + ioe);
         }
 
+        // The next set of tests cover cases where ChaCha20-Poly1305 cipher
+        // objects have the getParameters() call executed after instantiation
+        // but before initialization.
+        System.out.println("*** Test: getParameters before init");
+        cc20p1305 = Cipher.getInstance("ChaCha20-Poly1305");
+        AlgorithmParameters algParams = cc20p1305.getParameters();
+        byte[] preInitNonce = getNonceFromParams(algParams);
+        // A second pre-init getParameters() call should return a new set of
+        // random parameters.
+        AlgorithmParameters algParamsTwo = cc20p1305.getParameters();
+        byte[] secondNonce = getNonceFromParams(algParamsTwo);
+        if (MessageDigest.isEqual(preInitNonce, secondNonce)) {
+            throw new RuntimeException("Unexpected nonce match between " +
+                    "two pre-init getParameters() calls");
+        }
+
+        // Next we will initialize the Cipher object using a form of init
+        // that doesn't take AlgorithmParameters or AlgorithmParameterSpec.
+        // The nonce created using the pre-init getParameters() call should
+        // be overwritten by a freshly generated set of random parameters.
+        cc20p1305.init(Cipher.ENCRYPT_MODE, DEF_KEY);
+        AlgorithmParameters postInitAps = cc20p1305.getParameters();
+        byte[] postInitNonce = getNonceFromParams(postInitAps);
+        if (MessageDigest.isEqual(preInitNonce, postInitNonce)) {
+            throw new RuntimeException("Unexpected nonce match between " +
+                    "pre and post-init getParameters() calls");
+        }
+        System.out.println("Test Passed");
+
+        // After an initialization, subsequent calls to getParameters() should
+        // return the same parameter value until the next initialization takes
+        // place.
+        System.out.println("*** Test: getParameters after init");
+        AlgorithmParameters postInitApsTwo = cc20p1305.getParameters();
+        byte[] postInitNonceTwo = getNonceFromParams(postInitApsTwo);
+        if (!MessageDigest.isEqual(postInitNonce, postInitNonceTwo)) {
+            throw new RuntimeException("Unexpected nonce mismatch between " +
+                    "two post-init getParameters() calls");
+        }
+        System.out.println("Test Passed");
+
+        // Test reinitialization use cases.
+        // First test: instantiate, init(no param), encrypt.  Get params
+        // and attempt to reinit with same parameters.  Should fail.
+        System.out.println("*** Test: Init w/ random nonce, init 2nd time");
+        cc20p1305 = Cipher.getInstance("ChaCha20-Poly1305");
+        cc20p1305.init(Cipher.ENCRYPT_MODE, DEF_KEY);
+        algParams = cc20p1305.getParameters();
+        preInitNonce = getNonceFromParams(algParams);
+        // Perform a simple encryption operation
+        cc20p1305.doFinal(aeadTestList.get(0).input);
+        try {
+            // Now try to reinitialize using the same parameters
+            cc20p1305.init(Cipher.ENCRYPT_MODE, DEF_KEY, algParams);
+            throw new RuntimeException("Illegal key/nonce reuse");
+        } catch (InvalidKeyException ike) {
+            System.out.println("Caught expected exception: " + ike);
+        }
+
+        // Test the reinit guard using an AlgorithmParameterSpec with the
+        // Same nonce value.  This should also be a failure.
+        try {
+            cc20p1305.init(Cipher.ENCRYPT_MODE, DEF_KEY,
+                    new IvParameterSpec(preInitNonce));
+            throw new RuntimeException("Illegal key/nonce reuse");
+        } catch (InvalidKeyException ike) {
+            System.out.println("Caught expected exception: " + ike);
+        }
+
+        // Try one more time, this time providing a new 12-byte nonce, which
+        // should be allowed even if the key is the same.
+        cc20p1305.init(Cipher.ENCRYPT_MODE, DEF_KEY,
+                new IvParameterSpec(NONCE_OCTET_STR_12, 2, 12));
+        System.out.println("Test Passed");
+
+        // Reinit test: instantiate, init(no param), getParam, encrypt,
+        // then init(no param).  Should work and the parameters should be
+        // different after each init.
+        cc20p1305 = Cipher.getInstance("ChaCha20-Poly1305");
+        cc20p1305.init(Cipher.ENCRYPT_MODE, DEF_KEY);
+        byte[] paramInitOne = getNonceFromParams(cc20p1305.getParameters());
+        // Perform a simple encryption operation
+        cc20p1305.doFinal(aeadTestList.get(0).input);
+        // reinit (no params)
+        cc20p1305.init(Cipher.ENCRYPT_MODE, DEF_KEY);
+        byte[] paramInitTwo = getNonceFromParams(cc20p1305.getParameters());
+        if (MessageDigest.isEqual(paramInitOne, paramInitTwo)) {
+            throw new RuntimeException("Unexpected nonce match between " +
+                    "pre and post-init getParameters() calls");
+        }
+        System.out.println("Test Passed");
+
+        // Reinit test: instantiate, init(no param), doFinal, then doFinal
+        // again without intervening init.  Should fail due to no-reuse
+        // protections.
+        try {
+            cc20p1305 = Cipher.getInstance("ChaCha20-Poly1305");
+            cc20p1305.init(Cipher.ENCRYPT_MODE, DEF_KEY);
+            cc20p1305.doFinal(aeadTestList.get(0).input);
+            cc20p1305.doFinal(aeadTestList.get(0).input);
+            throw new RuntimeException("Illegal key/nonce reuse");
+        } catch (IllegalStateException ise) {
+            System.out.println("Caught expected exception: " + ise);
+        }
+
         System.out.println("----- AEAD Tests -----");
         for (TestData test : aeadTestList) {
             System.out.println("*** Test " + ++testNumber + ": " +
@@ -374,6 +483,11 @@ public class ChaCha20Poly1305ParamTest {
         return result;
     }
 
+    private static byte[] getNonceFromParams(AlgorithmParameters params)
+            throws InvalidParameterSpecException {
+        return params.getParameterSpec(IvParameterSpec.class).getIV();
+    }
+
     /**
      * Dump the hex bytes of a buffer into string form.
      *