RFR: 8271199: Mutual TLS handshake fails signing client certificate with custom sensitive PKCS11 key [v2]

Xue-Lei Andrew Fan xuelei at openjdk.java.net
Thu Oct 21 16:52:07 UTC 2021


On Thu, 21 Oct 2021 12:59:35 GMT, Alexey Bakhtin <abakhtin at openjdk.org> wrote:

>> Hello,
>> 
>> Could you please review the small patch for the issue described in JDK-8271199: Mutual TLS handshake fails signing client certificate with custom sensitive PKCS11 key
>> 
>> I suggest updating the RSAPSSSignature.isValid() method to verify if provided key components can be applied to SunRSASign implementation. 
>> If not applied, implementation can try to select signer from other providers
>> 
>> Regards
>> Alexey
>
> Alexey Bakhtin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Refactoring isValid() for private and public keys

Changes requested by xuelei (Reviewer).

src/java.base/share/classes/sun/security/rsa/RSAPSSSignature.java line 127:

> 125:             throws InvalidKeyException {
> 126:         if (publicKey instanceof RSAPublicKey rsaPubKey) {
> 127:             this.pubKey = (RSAPublicKey) isPublicKeyValid(rsaPubKey);

Is it possible to have the isPublicKeyValid() returning RSAPublicKey, or void, so as to avoid the casting?  Same comment for the isValid() method and the isPrivateKeyValid() method.

src/java.base/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java line 96:

> 94:             try {
> 95:                 return isValid(key);
> 96:             } catch (InvalidKeyException ikEx) {

It might be an uncommon use of the exception.  Exception creation and catching are expensive.  It is not good for performance to recover from exception.  I know there are a lot of code recovering from exception.  But if it is possible, we may want to try avoid it in new code.  Maybe, the isValid() could return a boolean value instead.

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

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



More information about the security-dev mailing list