On Thu, 14 Jan 2021 14:35:25 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> update to use List.of() and typo changes
>
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 88:
>
>> 86: boolean ocspNonce;
>> 87: }
>> 88: private RevocationProperties rp;
>
> I think this field could be `final`.
No change made due to getting an error: cannot assign a value to final variable
rp.
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 107:
>
>> 105:
>> 106: private void setDefaultNonce() {
>> 107: byte[] nonce = null;
>
> This variable looks like it is not used and can be removed.
The setDefaultNonce() method is removed.
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 109:
>
>> 107: byte[] nonce = null;
>> 108:
>> 109: // Set the nonce by default in OCSP request extension when the
>> sytem property
>
> Typo: s/sytem/system/
The setDefaultNonce() method is removed as creating nonce is done in
checkOCSP() method now.
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 113:
>
>> 111: if (rp.ocspNonce) {
>> 112: try {
>> 113: setOcspExtensions(List.of(new
>> OCSPNonceExtension(DEFAULT_NONCE_BYTES)));
>
> I don't think we should use the `PKIXRevocationChecker.setOcspExtensions()`
> API to add an OCSP Nonce extension. That API is intended to be called by
> applications. I think the Nonce extension should be set by the implementation
> only and not exposed via the standard API. Also, a nonce should be unique for
> each OCSP request, but setting it here means that it could re-use the same
> nonce for different OCSP requests.
>
> I think a better place to create/add the OCSPExtension is in the `checkOCSP`
> method, and the extension should be created each time that method is called
> (if the system property is enabled), so a new nonce is created for each OCSP
> request.
Creating the nonce is moved to checkOCSP() method.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2039
