RFR: 8261502: ECDHKeyAgreement: Allows alternate ECPrivateKey impl and revised exception handling [v2]

[Jamil Nimeh]

On Tue, 23 Feb 2021 20:40:05 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

>> Hi,
>> 
>> I need a code review of this change to ECDH.  It is a combination of fixing the implementation to not only accept ECPrivateKeyImpl along with a fix to the exception handling.  They started as two fixes, but with the exception handling the underlying code changed significantly that made the ECPrivateKey change in a different place.  The new exception handling is a result of no longer having the native library.  Many of the checks waited until generateSecret() to send the keys to the native library. Now that native is gone, checks can happen when keys are provided to the methods and proper exceptions can be thrown instead of wrapping everything as a ProviderException
>> 
>> thanks,
>> 
>> Tony
>
> Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Simpler fix for ECPrivateKey

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

> 209:         try {
> 210:             result = deriveKeyImpl(privateKey, privateKeyOps, publicKey);
> 211:         } catch (Exception e) {

Why such a broad exception catch here?  It looks like deriveKeyImpl is only explicitly throwing IKE.  Are there other unchecked exceptions that you're trying to snag here that I'm missing in the deriveKeyImpl below?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2659