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

[Xuelei Fan]

On 7/1/2015 10:02 AM, Jamil Nimeh wrote:
> 
> 
> On 06/30/2015 06:04 PM, Xuelei Fan wrote:
>> On 7/1/2015 6:39 AM, Jamil Nimeh wrote:
>>>> src/java.base/share/classes/sun/security/ssl/HandshakeMessage.java
>>>> ==================================================================
>>>> 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?
>> I have not read the server side implementation.  I would like firstly
>> check whether the message should be delivered, and than new the
>> instance.  Exception catching is not performance friendly, and looks a
>> little bit not-straightforward.  I think you may want a static method
>> for the validity checking in CertificateStatus class,  instead.
> As it is written today, the ServerHandshaker will only create a new
> CertificateStatus instance if the following is true:
> 
>   * Stapling has already been activated in the server (meaning that the
>     client requested it and the server has the feature enabled)
>   * A "get" operation was done from the StatusResponseManager and at
>     least one OCSP response was returned.  In other words, if no
>     responses can be brought back from the SRM, then there's no point in
>     even asserting status_request[_v2] in the ServerHello.
> 
> I don't see the advantage of making a static method that does what is
> already accomplished in two lines of code in ServerHandshaker (873-4).
> 
Good you can accomplish it in ServerHandshaker.

>>
>> It's OK to throw exception if something bad happens.  For easy reading,
>> please have a comment that it is unlikely to happen if you keep the
>> throw exception blocks.
> Okay, I can definitely do that.  There are some cases where I think we
> need to throw an exception, particularly on the constructor from a
> HandshakeInStream.
It's the expected behavior to throw exception for reading issues.  What
we are talking about previously is for write side constructor.

Thanks,
Xuelei

> That's a case where we want the client to Alert if
> the server asserts some weird/unsupported/illegal type or does something
> like type = ocsp and a zero length response (also illegal according to
> the spec).  Zero length responses are OK for ocsp_multi, though.
>