> On May 5, 2020, at 3:48 AM, Hai-May Chao <hai-may.c...@oracle.com> wrote:
>
> Hi Max,
>
>> On May 2, 2020, at 5:25 PM, Weijun Wang <weijun.w...@oracle.com> wrote:
>>
>> In jarsigner/Main, you can just call
>>
>> pkixParameters.setRevocationEnabled(revocationCheck);
>
> Ok
>
>>
>> Last time, you mentioned that "Event.clearReportListener()" cannot be called
>> immediately after validation check because of some race conditions. Is that
>> easily reproducible? I still find it strange.
>
> It is easily reproducible. I should have better explained why the suggested
> changes did not work but not due to race condition. The code
> pkixParameters.setRevocationEnabled(revocationCheck) only sets the revocation
> checker enabled. It is the validateCertChain() in Main.java, which calls into
> RevocationChecker.check() and ultimately calls Event.report() prior to making
> OCSP and CRL connections. If we call clearReportListener in loadKeyStore()
> method (i.e finally{ }as suggested), for OCSP, by the time when
> OCSP.getOCSPBytes() comes in to report the OCSP event, the reporter has been
> cleared. And this would be same problem for CRL. So it cannot be called
> immediately.
Then I assume the following is OK.
try {
setReportListener();
validator.validate();
} finally {
clearReportListener();
}
Thanks,
Max
>
> Thanks,
> Hai-May
>
>>
>> Thanks,
>> Max
>>
>>> On May 3, 2020, at 2:19 AM, Hai-May Chao <hai-may.c...@oracle.com> wrote:
>>>
>>> Hi Sean,
>>>
>>> Thanks for your review. Reply inline.
>>>
>>>> On May 1, 2020, at 11:50 AM, Sean Mullan <sean.mul...@oracle.com> wrote:
>>>>
>>>> * Main.java:
>>>>
>>>> 2067 Event.setReportListener(new Event.Reporter() {
>>>> 2068 @Override
>>>> 2069 public void handle(String t, Object... o)
>>>> {
>>>> 2070 System.out.println(String.format(rb.getString(t), o));
>>>> 2071 }
>>>> 2072 });
>>>>
>>>> I think you could use a lambda expression above.
>>>
>>> Done.
>>>
>>>>
>>>> * Event.java:
>>>>
>>>> 35 static Reporter reporter = null;
>>>>
>>>> Make this private? Also, no need to explicitly initialize to null as that
>>>> is the default value.
>>>
>>> Done (made it private).
>>>
>>>>
>>>> Can you add some comments at the top of the class describing the purpose
>>>> of this class?
>>>
>>> Done.
>>>
>>>>
>>>> * EnableRevocation.java
>>>>
>>>> - How long does this test take - does it hang for a little while trying to
>>>> make a connection or timeout right away? If it takes a while, you could
>>>> experiment with overriding the default timeouts for CRLs and OCSP checks
>>>> to make this test finish faster. Use the system properties
>>>> com.sun.security.ocsp.timeout and com.sun.security.crl.readtimeout.
>>>
>>> It does not hang for OCSP as the certificate is set with localhost as OCSP
>>> responder. It hangs just a little for CRL, thus I’ve changed the
>>> certificate to local host instead of remote host. The whole test is
>>> finishing very fast now.
>>>
>>>>
>>>> Looks good otherwise. Please add a release-note and open a follow-on issue
>>>> to update the man page with the new option.
>>>
>>> Done (Release note: JDK-8244285, and man page: JDK-8244274).
>>>
>>> Updated webrev:
>>> https://cr.openjdk.java.net/~hchao/8242060/webrev.02/
>>>
>>> Thanks,
>>> Hai-May
>>>
>>>
>>>>
>>>> --Sean
>>>>
>>>> On 5/1/20 12:02 PM, Hai-May Chao wrote:
>>>>> Hi,
>>>>> With small change added to ‘Usages.java' test, here is the updated webrev:
>>>>> https://cr.openjdk.java.net/~hchao/8242060/webrev.01/
>>>>> Thanks,
>>>>> Hai-May
>>>>>> On Apr 30, 2020, at 4:29 PM, Hai-May Chao <hai-may.c...@oracle.com>
>>>>>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I’d like to request a review for:
>>>>>>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8242060
>>>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8244046
>>>>>> Webrev: https://cr.openjdk.java.net/~hchao/8242060/webrev.00/
>>>>>>
>>>>>> The jarsigner command currently does certificate chain validation, but
>>>>>> does not check revocation. Users won’t be able to know if the
>>>>>> certificates are revoked. This change is to provide an option in
>>>>>> jarsigner to enable the revocation check, and to emit progress messages
>>>>>> when jarsigner starts network connections to get OCSP responses and CRL.
>>>>>>
>>>>>> Thanks,
>>>>>> Hai-May
>>>>>>
>>>>>>
>>>>>>
>>>
>>
>
