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

Xue-Lei Andrew Fan xuelei at openjdk.org
Thu Jul 20 05:09:45 UTC 2023


On Thu, 20 Jul 2023 00:39:08 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:
> 
>   Update to address review feedbacks

Let make sure the fall-back is really necessary before integration.  Thanks!

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

> 225:                 byte[] padded2 = padding.pad(encoded2);
> 226:                 return MessageDigest.isEqual(padded2, decrypted);
> 227:             }

I had a check of the specification (Section A.2.4 of RFC 8017), and the [update](https://github.com/openjdk/jdk/pull/8365) and the [JBS entry](https://bugs.openjdk.org/browse/JDK-8285404) that added the comment "some vendors might omit the NULL params".

Per section A.2.4 of RFC 8017, it is said "For each OID, the parameters field associated with this OID in a value of type  AlgorithmIdentifier SHALL have a value of type NULL."

Per the key words specification, RFC 2119, "SHALL" is the same as MUST which  "mean that the definition is an absolute requirement of the specification."

In the bug description of bug JDK-8285404, there is a section "*Update*: We think it's possible that there might be signers omitting the NULL params in the digest algorithm identifier. "

For this case, if the signers omitting the NULL params, does it means the signer does not follow the specification and should be rejected?  @wangweij could you recall if there is a real case that omits the NULL params in practice?

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

Changes requested by xuelei (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14839#pullrequestreview-1538434261
PR Review Comment: https://git.openjdk.org/jdk/pull/14839#discussion_r1268937806


More information about the security-dev mailing list