On Wed, 12 Jul 2023 23:12:18 GMT, Valerie Peng <valer...@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
