8205476: KeyAgreement#generateSecret is not reset for ECDH based algorithm
Summary: Clarify spec of generateSecret and modify ECDH in SunEC to conform to spec
Reviewed-by: mullan
--- a/src/java.base/share/classes/javax/crypto/KeyAgreement.java Tue Oct 30 10:32:54 2018 -0700
+++ b/src/java.base/share/classes/javax/crypto/KeyAgreement.java Tue Oct 30 13:48:19 2018 -0400
@@ -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 @@
/**
* Generates the shared secret and returns it in a new buffer.
*
- * <p>This method resets this {@code KeyAgreement} object, so that it
- * can be reused for further key agreements. Unless this key agreement is
- * reinitialized with one of the {@code init} methods, the same
- * private information and algorithm parameters will be used for
- * subsequent key agreements.
+ * <p>This method resets this {@code KeyAgreement} object to the state that
+ * it was in after the most recent call to one of the {@code init} methods.
+ * After a call to {@code generateSecret}, the object can be reused for
+ * further key agreement operations by calling {@code doPhase} to supply
+ * 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 @@
* 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
- * can be reused for further key agreements. Unless this key agreement is
- * reinitialized with one of the {@code init} methods, the same
- * private information and algorithm parameters will be used for
- * subsequent key agreements.
+ * <p>This method resets this {@code KeyAgreement} object to the state that
+ * it was in after the most recent call to one of the {@code init} methods.
+ * After a call to {@code generateSecret}, the object can be reused for
+ * further key agreement operations by calling {@code doPhase} to supply
+ * 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 @@
* @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 @@
* 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
- * can be reused for further key agreements. Unless this key agreement is
- * reinitialized with one of the {@code init} methods, the same
- * private information and algorithm parameters will be used for
- * subsequent key agreements.
+ * <p>This method resets this {@code KeyAgreement} object to the state that
+ * it was in after the most recent call to one of the {@code init} methods.
+ * After a call to {@code generateSecret}, the object can be reused for
+ * further key agreement operations by calling {@code doPhase} to supply
+ * 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
--- a/src/java.base/share/classes/javax/crypto/KeyAgreementSpi.java Tue Oct 30 10:32:54 2018 -0700
+++ b/src/java.base/share/classes/javax/crypto/KeyAgreementSpi.java Tue Oct 30 13:48:19 2018 -0400
@@ -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 @@
/**
* Generates the shared secret and returns it in a new buffer.
*
- * <p>This method resets this <code>KeyAgreementSpi</code> object,
- * so that it
- * can be reused for further key agreements. Unless this key agreement is
- * reinitialized with one of the <code>engineInit</code> methods, the same
- * private information and algorithm parameters will be used for
- * subsequent key agreements.
+ * <p>This method resets this {@code KeyAgreementSpi} object to the state
+ * that it was in after the most recent call to one of the {@code init}
+ * methods. After a call to {@code generateSecret}, the object can be reused
+ * for further key agreement operations by calling {@code doPhase} to supply
+ * 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
*/
protected abstract byte[] engineGenerateSecret()
throws IllegalStateException;
@@ -153,12 +158,16 @@
* 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,
- * so that it
- * can be reused for further key agreements. Unless this key agreement is
- * reinitialized with one of the <code>engineInit</code> methods, the same
- * private information and algorithm parameters will be used for
- * subsequent key agreements.
+ * <p>This method resets this {@code KeyAgreementSpi} object to the state
+ * that it was in after the most recent call to one of the {@code init}
+ * methods. After a call to {@code generateSecret}, the object can be reused
+ * for further key agreement operations by calling {@code doPhase} to supply
+ * 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</code> where the
@@ -167,7 +176,8 @@
* @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 @@
* 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,
- * so that it
- * can be reused for further key agreements. Unless this key agreement is
- * reinitialized with one of the <code>engineInit</code> methods, the same
- * private information and algorithm parameters will be used for
- * subsequent key agreements.
+ * <p>This method resets this {@code KeyAgreementSpi} object to the state
+ * that it was in after the most recent call to one of the {@code init}
+ * methods. After a call to {@code generateSecret}, the object can be reused
+ * for further key agreement operations by calling {@code doPhase} to supply
+ * 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 requested secret key
* algorithm is not available
* @exception InvalidKeyException if the shared secret key material cannot
--- a/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java Tue Oct 30 10:32:54 2018 -0700
+++ b/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java Tue Oct 30 13:48:19 2018 -0400
@@ -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 @@
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);
--- a/test/jdk/java/security/KeyAgreement/KeyAgreementTest.java Tue Oct 30 10:32:54 2018 -0700
+++ b/test/jdk/java/security/KeyAgreement/KeyAgreementTest.java Tue Oct 30 13:48:19 2018 -0400
@@ -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 @@
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();
}
}