RFR: 8302017: Allocate BadPaddingException only if it will be thrown [v2]

[Xue-Lei Andrew Fan]

On Wed, 12 Jul 2023 23:12:18 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> This change refactors the RSAPadding class to return an output record containing the status instead of relying on exception object to indicate a failure.
>> 
>> Thanks in advance for review~
>> Valerie
>
> Valerie Peng has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review feedbacks, e.g. Removed RSAPadding.Output and use byte[] as before.

Thank you for the update.

src/java.base/share/classes/sun/security/rsa/RSASignature.java line 196:

> 194:                 return RSACore.rsa(padded, privateKey, true);
> 195:             }
> 196:             throw new SignatureException("Could not sign data");

It may be clearer if the throw line is moved to the end of the method.  Otherwise, I have to check if SignatureException is a sub-class of GeneralSecurityException.

src/java.base/share/classes/sun/security/rsa/RSASignature.java line 231:

> 229:                             RSAUtil.decodeSignature(digestOID, unpadded));
> 230:                 }
> 231:             }

I understand where the fallback code came from.  As the padding code is exactly the same as engineSign(), the risk may be minimal.  With the fallback code, the security concern (time-constant) we cared about will come back.  Did you run into testing failure without the fallback doe?

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

PR Review: https://git.openjdk.org/jdk/pull/14839#pullrequestreview-1527547950
PR Review Comment: https://git.openjdk.org/jdk/pull/14839#discussion_r1261964861
PR Review Comment: https://git.openjdk.org/jdk/pull/14839#discussion_r1261970478