On Tue, 12 Jan 2021 19:18:18 GMT, Hai-May Chao <hc...@openjdk.org> wrote:
>> This enhancement adds support for the nonce extension in OCSP request >> extensions by system property jdk.security.certpath.ocspNonce. >> >> Please review the CSR at: >> https://bugs.openjdk.java.net/browse/JDK-8257766 > > 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 Changes requested by mullan (Reviewer). 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`. 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. 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/ 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. ------------- PR: https://git.openjdk.java.net/jdk/pull/2039
