RFR 8247960: jarsigner says "signer errors" for some normal warnings when -strict is set

Weijun Wang weijun.wang at oracle.com
Fri Jul 24 13:54:36 UTC 2020


I'd suggest changing

+        result = isSigning ? rb.getString("jar.signed.") : rb.getString("jar.verified.");
+        if ((strict) && (!errors.isEmpty())) {
+            result = isSigning ? rb.getString("jar.signed.with.signer.errors.")
+                     : rb.getString("jar.verified.with.signer.errors.");
+        }

to

+        if ((strict) && (!errors.isEmpty())) {
+            result = isSigning ? rb.getString("jar.signed.with.signer.errors.")
+                     : rb.getString("jar.verified.with.signer.errors.");
+        } else {
+            result = isSigning ? rb.getString("jar.signed.") : rb.getString("jar.verified.");
+        }

Everything else looks fine.

Also, I remember you meant to fix 2 bugs with a single changeset. What should the full commit message be?

Thanks,
Max

> On Jul 24, 2020, at 4:25 PM, Hai-May Chao <hai-may.chao at oracle.com> wrote:
> 
> 
> 
>> On Jul 15, 2020, at 12:16 AM, Weijun Wang <weijun.wang at oracle.com> wrote:
>> 
>> The following lines are useless now:
>> 
> 
> Ok, this is a separate problem from the original bug to be addressed. Cleanup/refactoring of the checking on flags is also included in this changeset.
> 
>> 1053         if (badKeyUsage || badExtendedKeyUsage || badNetscapeCertType ||
>> 1054                 notYetValidCert || chainNotValidated || hasExpiredCert ||
>> 1055                 hasUnsignedEntry || signerSelfSigned || (legacyAlg != 0) ||
>> 1056                 (disabledAlg != 0) || aliasNotInStore || notSignedByAlias ||
>> 1057                 tsaChainNotValidated ||
>> 1058                 (hasExpiredTsaCert && !signerNotExpired)) {
>> 
>> 1198         }
>> 
>> 1205         if (hasExpiringCert ||
>> 1206                 (hasExpiringTsaCert  && expireDate != null) ||
>> 1207                 (noTimestamp && expireDate != null) ||
>> 1208                 (hasExpiredTsaCert && signerNotExpired)) {
>> 
>> 1245         }
>> 
>> I would even suggest you remove the "result" variable and move the "System.out.println(result)" line into branches of the if-else block on lines 1254-1272.
> 
> Current change has the checking for sign and verify. Keep it as-is that you agreed.
> 
> https://cr.openjdk.java.net/~hchao/8247960/webrev.01/
> 
> Thanks,
> Hai-May
> 
> 
>> 
>> No other comments.
>> 
>> Thanks,
>> Max
>> 
>> 
>> 
>>> On Jul 15, 2020, at 4:09 AM, Hai-May Chao <hai-may.chao at oracle.com> wrote:
>>> 
>>> Hi,
>>> 
>>> I’d like to request a review for:
>>> 
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8247960
>>> Webrev: https://cr.openjdk.java.net/~hchao/8247960/webrev.00/
>>> 
>>> Jarsigner is changed to emit “with signer errors” only when there are errors detected during sign and verify with -strict specified.
>>> 
>>> Thanks,
>>> Hai-May




More information about the security-dev mailing list