RFR: 8277474: jarsigner does not check if algorithm parameters are disabled [v2]

Hai-May Chao hchao at openjdk.java.net
Wed Mar 2 17:50:07 UTC 2022


On Wed, 2 Mar 2022 16:20:53 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Removed unneeded import and updated -verbose output
>
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line 1414:
> 
>> 1412:             } catch (CertPathValidatorException e) {
>> 1413:                 disabledAlgFound = true;
>> 1414:                 return String.format(rb.getString("with.disabled"), algParams);
> 
> The return value of this method will be shown as the "Signature algorithm" in the output. It's OK to include an optional "weak" (or "disabled") tag, but the core part still must be an algorithm name. Here, the updated code returns the string format of `algParams`, which is not an algorithm name.
> 
> I'm not sure how to fix this nicely. Certainly you want to show the user why it is weak so the weak part should be displayed. A verbose solution could be "RSSSSA-PSS using PSSParameterSpec(...SHA-1...) (weak)", but the `toString()` output of `PSSParameterSpec` is quite long.
> 
> Same comment to the code change below.

I add "RSSSSA-PSS using “ to the `-verbose` output as suggested, and keep the remaining output as the parameters for the RSASSA-PSS contain hashAlgorithm and maskGenAlgorithm that could be disabled or weak. At the same time, strip off the saltLength and trailerField display.

> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line 1438:
> 
>> 1436:             try {
>> 1437:                 LEGACY_CHECK.permits(algParams, jcp);
>> 1438:                 return alg;
> 
> No need to return here since it will be returned on line 1445 anyway.

Removed.

> test/jdk/sun/security/tools/jarsigner/CheckAlgParams.java line 30:
> 
>> 28:  *          its signature algorithm use disabled or legacy algorithms
>> 29:  * @library /test/lib
>> 30:  * @modules java.base/sun.security.x509
> 
> Is this `@modules` line useful?

No, removed it. Thanks for the review!

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

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



More information about the security-dev mailing list