[Update]: JEP 249 (OCSP Stapling for TLS)

[Jamil Nimeh]

Hi Xuelei, I'm working on your comments now.  Thanks for all the useful 
feedback.  I'm working on the comments in your other emails, too.

On 06/30/2015 02:46 AM, Xuelei Fan wrote:
> src/java.base/share/classes/sun/security/ssl/HandshakeMessage.java
> ==================================================================
>
>   676     private List<byte[]> encodedResponses = new ArrayList<>(4);
>
> 4 may be not the best estimate.  Maybe better to make the initialization
> in the constructor.
Fair enough.  I've moved it into the constructors.  It's based on the 
chain length in the version where the chain is provided, and it uses the 
default constructor in the case where you're bringing it in from a 
HandshakeInStream (since you don't know the number of objects up-front).
>
> ---------------
> line 713/714, 730/731 throws SSLHandshakeException for extension
> constructor in server side.  That's unlikely to happen, I think.  I was
> wondering, if CertificateStatus cannot be constructed, the server may
> not want to send the message, rather than terminate the connection
> immediately.
I think you're right.  While the exception is unlikely, I'd like to have 
the HandshakeMessage throw the exception if something bad happens.  I do 
however, agree that we shouldn't make it a fatal error.  I'll catch the 
exception in ServerHandshaker, log it, and just not send the message as 
you suggested since that is legal.  OK?
>
> -------------
>   674     private int encodedResponseLen;
> s/encodedResponseLen/encodedResponsesLen
>
> Suggest to add
>
> --------------
>   820                 if (respBytes != null) {
>   821                     s.putBytes24(respBytes);
>   822                 } else {
>   823                     s.putBytes24(new byte[0]);
>   824                 }
>
> HandshakeOutStream.putBytes24() accepts null parameter.
Yeah, that is a better way to do it and not have the unnecessary object 
creation.
>
> -------------
> line 827/828, unlikely to happen.  I would suggest you add a comment or
> remove the lines.
I'll comment it.
>
> Xuelei
>
> On 6/27/2015 11:06 PM, Jamil Nimeh wrote:
>> Hello all, I've posted an updated webrev based on comments I've received
>> so far:
>>
>> http://cr.openjdk.java.net/~jnimeh/reviews/8046321/webrev.1
>>
>> Thanks,
>> --Jamil
>>
>> On 06/18/2015 05:27 PM, Jamil Nimeh wrote:
>>> Hello all,
>>>
>>> I have a first cut at the OCSP stapling webrev posted for your review:
>>>
>>> JEP: https://bugs.openjdk.java.net/browse/JDK-8046321
>>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8046321/webrev.0/
>>>
>>> A couple items to note:
>>>
>>>    * I'm in the process of updating the JEP with some more details.  I
>>>      should be done with these changes by tonight (PDT).
>>>    * Missing are some of the TLS end-to-end tests.  These tests have
>>>      been coded and run outside the jtreg framework, but for some
>>>      reason things hang in jtreg.  I've included some of the supporting
>>>      classes that these tests will use (CertificateBuilder.java and
>>>      SimpleOCSPResponder.java) so folks could review those if they're
>>>      interested.  I will update the webrev and notify the list as soon
>>>      as I've got the tests working in jtreg.
>>>
>>> Thanks to everyone who has helped along the way.
>>>
>>> --Jamil
>>>
>>>