RFR: 8302017: Allocate BadPaddingException only if it will be thrown

Xue-Lei Andrew Fan xuelei at openjdk.org
Wed Jul 12 17:02:04 UTC 2023


On Tue, 11 Jul 2023 23:19:40 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

Changes requested by xuelei (Reviewer).

src/java.base/share/classes/sun/security/rsa/RSAPadding.java line 372:

> 370:             return Output.FAIL;
> 371:         } else {
> 372:             return Output.pass(data);

The Output.pass(byte[]) will create a new instance and thus make the behavior detectable.  Maybe, the Output class is not necessary, 'null' value return could be used instead.


- if (bp) {
-     return Output.FAIL;
- } else {
-     return Output.pass(data);
+ return bp ? null : data;

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

> 215:             byte[] digest = getDigestValue();
> 216:             byte[] decrypted = RSACore.rsa(sigBytes, publicKey);
> 217:             RSAPadding.Output po = padding.unpad(decrypted);

In case you are already here, what if comparing the padded/encoded result, without use unpad() any longer? I meant to follow the spec as described in RFC8017#section-8.2.2: encode the `decryped` bytes and then compare the result with the `digest` bytes.

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

PR Review: https://git.openjdk.org/jdk/pull/14839#pullrequestreview-1526794400
PR Review Comment: https://git.openjdk.org/jdk/pull/14839#discussion_r1261451456
PR Review Comment: https://git.openjdk.org/jdk/pull/14839#discussion_r1261463911



More information about the security-dev mailing list