Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v4]

2021-01-20 Thread Hai-May Chao 
On Wed, 20 Jan 2021 13:46:58 GMT, Sean Mullan  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

Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v4]

2021-01-20 Thread Hai-May Chao 
On Wed, 20 Jan 2021 13:41:04 GMT, Sean Mullan  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 762:
> 
>> 760: } catch (IOException e) {
>> 761: throw new 
>> CertPathValidatorException("Failed to create the default nonce " +
>> 762: "in OCSP entensions");
> 
> Typo: s/entensions/extensions/
> 
> Also, use the `CertPathValidatorException(String, Throwable)` ctor instead 
> and pass the `IOException` as the 2nd parameter.

Fixed.

-

PR: https://git.openjdk.java.net/jdk/pull/2039

Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v4]

2021-01-20 Thread Sean Mullan 
On Fri, 15 Jan 2021 23:09:26 GMT, Hai-May Chao  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:
> 
>   Nonce creation is done in checkOCSP method

Changes requested by mullan (Reviewer).

src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
 line 762:

> 760: } catch (IOException e) {
> 761: throw new CertPathValidatorException("Failed 
> to create the default nonce " +
> 762: "in OCSP entensions");

Typo: s/entensions/extensions/

Also, use the `CertPathValidatorException(String, Throwable)` ctor instead and 
pass the `IOException` as the 2nd parameter.

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.

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.

-

PR: https://git.openjdk.java.net/jdk/pull/2039

Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v4]

2021-01-15 Thread Hai-May Chao 
> 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:

  Nonce creation is done in checkOCSP method

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2039/files
  - new: https://git.openjdk.java.net/jdk/pull/2039/files/949ee2be..da6e5bab

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2039&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2039&range=02-03

  Stats: 68 lines in 1 file changed: 36 ins; 31 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2039.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2039/head:pull/2039

PR: https://git.openjdk.java.net/jdk/pull/2039