8205476: KeyAgreement#generateSecret is not reset for ECDH based algorithm

Clarify spec of generateSecret and modify ECDH in SunEC to conform to spec

Reviewed-by: mullan

src/java.base/share/classes/javax/crypto/KeyAgreement.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -582,16 +582,22 @@ public class KeyAgreement {
     /**
      * Generates the shared secret and returns it in a new buffer.
      *
-     * <p>This method resets this {@code KeyAgreement} object, so that it
+     * <p>This method resets this {@code KeyAgreement} object to the state that
-     * can be reused for further key agreements. Unless this key agreement is
+     * it was in after the most recent call to one of the {@code init} methods.
-     * reinitialized with one of the {@code init} methods, the same
+     * After a call to {@code generateSecret}, the object can be reused for
-     * private information and algorithm parameters will be used for
+     * further key agreement operations by calling {@code doPhase} to supply
-     * subsequent key agreements.
+     * new keys, and then calling {@code generateSecret} to produce a new
+     * secret. In this case, the private information and algorithm parameters
+     * supplied to {@code init} will be used for multiple key agreement
+     * operations. The {@code init} method can be called after
+     * {@code generateSecret} to change the private information used in
+     * subsequent operations.
      *
      * @return the new buffer with the shared secret
      *
      * @exception IllegalStateException if this key agreement has not been
-     * completed yet
+     * initialized or if {@code doPhase} has not been called to supply the
+     * keys for all parties in the agreement
      */
     public final byte[] generateSecret() throws IllegalStateException {
         chooseFirstProvider();
@@ -606,11 +612,16 @@ public class KeyAgreement {
      * result, a {@code ShortBufferException} is thrown.
      * In this case, this call should be repeated with a larger output buffer.
      *
-     * <p>This method resets this {@code KeyAgreement} object, so that it
+     * <p>This method resets this {@code KeyAgreement} object to the state that
-     * can be reused for further key agreements. Unless this key agreement is
+     * it was in after the most recent call to one of the {@code init} methods.
-     * reinitialized with one of the {@code init} methods, the same
+     * After a call to {@code generateSecret}, the object can be reused for
-     * private information and algorithm parameters will be used for
+     * further key agreement operations by calling {@code doPhase} to supply
-     * subsequent key agreements.
+     * new keys, and then calling {@code generateSecret} to produce a new
+     * secret. In this case, the private information and algorithm parameters
+     * supplied to {@code init} will be used for multiple key agreement
+     * operations. The {@code init} method can be called after
+     * {@code generateSecret} to change the private information used in
+     * subsequent operations.
      *
      * @param sharedSecret the buffer for the shared secret
      * @param offset the offset in {@code sharedSecret} where the
@@ -619,7 +630,8 @@ public class KeyAgreement {
      * @return the number of bytes placed into {@code sharedSecret}
      *
      * @exception IllegalStateException if this key agreement has not been
-     * completed yet
+     * initialized or if {@code doPhase} has not been called to supply the
+     * keys for all parties in the agreement
      * @exception ShortBufferException if the given output buffer is too small
      * to hold the secret
      */
@@ -634,18 +646,24 @@ public class KeyAgreement {
      * Creates the shared secret and returns it as a {@code SecretKey}
      * object of the specified algorithm.
      *
-     * <p>This method resets this {@code KeyAgreement} object, so that it
+     * <p>This method resets this {@code KeyAgreement} object to the state that
-     * can be reused for further key agreements. Unless this key agreement is
+     * it was in after the most recent call to one of the {@code init} methods.
-     * reinitialized with one of the {@code init} methods, the same
+     * After a call to {@code generateSecret}, the object can be reused for
-     * private information and algorithm parameters will be used for
+     * further key agreement operations by calling {@code doPhase} to supply
-     * subsequent key agreements.
+     * new keys, and then calling {@code generateSecret} to produce a new
+     * secret. In this case, the private information and algorithm parameters
+     * supplied to {@code init} will be used for multiple key agreement
+     * operations. The {@code init} method can be called after
+     * {@code generateSecret} to change the private information used in
+     * subsequent operations.
      *
      * @param algorithm the requested secret-key algorithm
      *
      * @return the shared secret key
      *
      * @exception IllegalStateException if this key agreement has not been
-     * completed yet
+     * initialized or if {@code doPhase} has not been called to supply the
+     * keys for all parties in the agreement
      * @exception NoSuchAlgorithmException if the specified secret-key
      * algorithm is not available
      * @exception InvalidKeyException if the shared secret-key material cannot

src/java.base/share/classes/javax/crypto/KeyAgreementSpi.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2007, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -130,17 +130,22 @@ public abstract class KeyAgreementSpi {
     /**
      * Generates the shared secret and returns it in a new buffer.
      *
-     * <p>This method resets this <code>KeyAgreementSpi</code> object,
+     * <p>This method resets this {@code KeyAgreementSpi} object to the state
-     * so that it
+     * that it was in after the most recent call to one of the {@code init}
-     * can be reused for further key agreements. Unless this key agreement is
+     * methods. After a call to {@code generateSecret}, the object can be reused
-     * reinitialized with one of the <code>engineInit</code> methods, the same
+     * for further key agreement operations by calling {@code doPhase} to supply
-     * private information and algorithm parameters will be used for
+     * new keys, and then calling {@code generateSecret} to produce a new
-     * subsequent key agreements.
+     * secret. In this case, the private information and algorithm parameters
+     * supplied to {@code init} will be used for multiple key agreement
+     * operations. The {@code init} method can be called after
+     * {@code generateSecret} to change the private information used in
+     * subsequent operations.
      *
      * @return the new buffer with the shared secret
      *
      * @exception IllegalStateException if this key agreement has not been
-     * completed yet
+     * initialized or if {@code doPhase} has not been called to supply the
+     * keys for all parties in the agreement
      */
     protected abstract byte[] engineGenerateSecret()
         throws IllegalStateException;
@@ -153,12 +158,16 @@ public abstract class KeyAgreementSpi {
      * result, a <code>ShortBufferException</code> is thrown.
      * In this case, this call should be repeated with a larger output buffer.
      *
-     * <p>This method resets this <code>KeyAgreementSpi</code> object,
+     * <p>This method resets this {@code KeyAgreementSpi} object to the state
-     * so that it
+     * that it was in after the most recent call to one of the {@code init}
-     * can be reused for further key agreements. Unless this key agreement is
+     * methods. After a call to {@code generateSecret}, the object can be reused
-     * reinitialized with one of the <code>engineInit</code> methods, the same
+     * for further key agreement operations by calling {@code doPhase} to supply
-     * private information and algorithm parameters will be used for
+     * new keys, and then calling {@code generateSecret} to produce a new
-     * subsequent key agreements.
+     * secret. In this case, the private information and algorithm parameters
+     * supplied to {@code init} will be used for multiple key agreement
+     * operations. The {@code init} method can be called after
+     * {@code generateSecret} to change the private information used in
+     * subsequent operations.
      *
      * @param sharedSecret the buffer for the shared secret
      * @param offset the offset in <code>sharedSecret</code> where the
@@ -167,7 +176,8 @@ public abstract class KeyAgreementSpi {
      * @return the number of bytes placed into <code>sharedSecret</code>
      *
      * @exception IllegalStateException if this key agreement has not been
-     * completed yet
+     * initialized or if {@code doPhase} has not been called to supply the
+     * keys for all parties in the agreement
      * @exception ShortBufferException if the given output buffer is too small
      * to hold the secret
      */
@@ -179,19 +189,24 @@ public abstract class KeyAgreementSpi {
      * Creates the shared secret and returns it as a secret key object
      * of the requested algorithm type.
      *
-     * <p>This method resets this <code>KeyAgreementSpi</code> object,
+     * <p>This method resets this {@code KeyAgreementSpi} object to the state
-     * so that it
+     * that it was in after the most recent call to one of the {@code init}
-     * can be reused for further key agreements. Unless this key agreement is
+     * methods. After a call to {@code generateSecret}, the object can be reused
-     * reinitialized with one of the <code>engineInit</code> methods, the same
+     * for further key agreement operations by calling {@code doPhase} to supply
-     * private information and algorithm parameters will be used for
+     * new keys, and then calling {@code generateSecret} to produce a new
-     * subsequent key agreements.
+     * secret. In this case, the private information and algorithm parameters
+     * supplied to {@code init} will be used for multiple key agreement
+     * operations. The {@code init} method can be called after
+     * {@code generateSecret} to change the private information used in
+     * subsequent operations.
      *
      * @param algorithm the requested secret key algorithm
      *
      * @return the shared secret key
      *
      * @exception IllegalStateException if this key agreement has not been
-     * completed yet
+     * initialized or if {@code doPhase} has not been called to supply the
+     * keys for all parties in the agreement
      * @exception NoSuchAlgorithmException if the requested secret key
      * algorithm is not available
      * @exception InvalidKeyException if the shared secret key material cannot

src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2018, 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
@@ -127,7 +127,9 @@ public final class ECDHKeyAgreement extends KeyAgreementSpi {
 
         try {
 
-            return deriveKey(s, publicValue, encodedParams);
+            byte[] result = deriveKey(s, publicValue, encodedParams);
+            publicValue = null;
+            return result;
 
         } catch (GeneralSecurityException e) {
             throw new ProviderException("Could not derive key", e);

test/jdk/java/security/KeyAgreement/KeyAgreementTest.java

@@ -23,7 +23,7 @@
 
  /*
  * @test
- * @bug 4936763 8184359
+ * @bug 4936763 8184359 8205476
  * @summary KeyAgreement Test with all supported algorithms from JCE.
  *          Arguments order <KeyExchangeAlgorithm> <KeyGenAlgorithm> <Provider>
  *          It removes com/sun/crypto/provider/KeyAgreement/DHGenSecretKey.java
@@ -150,5 +150,17 @@ public class KeyAgreementTest {
         if (!Arrays.equals(secret1, secret2)) {
             throw new Exception("KeyAgreement secret mismatch.");
         }
+
+        // ensure that a new secret cannot be produced before the next doPhase
+        try {
+            ka2.generateSecret();
+            throw new RuntimeException("state not reset");
+        } catch (IllegalStateException ex) {
+            // this is expected
+        }
+
+        // calling doPhase and then generateSecret should succeed
+        ka2.doPhase(kp1.getPublic(), true);
+        ka2.generateSecret();
     }
 }