On Wed, 20 Jan 2021 13:46:58 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Nonce creation is done in checkOCSP method > > src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java > line 755: > >> 753: // create the 16-byte nonce by default >> 754: Extension nonceExt = new >> OCSPNonceExtension(DEFAULT_NONCE_BYTES); >> 755: tmpExtensions.add(nonceExt); > > I think you should add the OCSPNonce extension to the list of extensions that > the application passed in, as there may be other extensions that have been > specified and should be sent in the OCSP response, ex: > > `ocspExtensions.add(new OCSPNonceExtension(DEFAULT_NONCE_BYTES));` > > This means you don't need the `tmpExtensions` variable. During testing, I got the "java.base/java.util.Collections$UnmodifiableCollection.add(Collections.java:1062) exception with this line of change. I've changed to use a tmpExtensions variable when setting the OCSP nonce to the extension sets instead of modifying the ocspExtensions. > src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java > line 779: > >> 777: response = OCSP.check(Collections.singletonList(certId), >> 778: responderURI, issuerInfo, responderCert, null, >> 779: rp.ocspNonce ? tmpExtensions : ocspExtensions, >> params.variant()); > > Here you can just pass in `ocspExtensions` since it will contain the nonce if > the property has been set. No change as tmpExtensions is needed. ------------- PR: https://git.openjdk.java.net/jdk/pull/2039
