Change in closeOutbound() behavior for unused SSLEngine

[Xuelei Fan]

Hi Simone,

The compatibility impact makes sense to me. Would you mind file a new bug?

Thanks,
Xuelei

On 10/16/2019 9:13 AM, Simone Bordet wrote:
> Hi,
> 
> On Wed, Oct 16, 2019 at 5:42 PM Xuelei Fan <xuelei.fan at oracle.com> wrote:
>>
>> The TLS protocol was changed to use half-close policy since TLS 1.3.  As
>> means that sslEngine.closeOutbound() will close the outbound and keep
>> the inbound open.  "NEED_UNWRAP" is used to indicate that the engine can
>> still be used to decode input message.
> 
> Maybe it's a matter of interpretation, but the SSLEngine state was not
> used to indicate what may _possibly_ be done in the future.
> It was an exact indication of what an application should do _now_.
> In the normal case (after the handshake is completed), closeOutbound()
> turns the state into NEED_WRAP because wrapping will generate the
> close_notify.
> In this case, NEED_UNWRAP is strange because I just closed the
> outbound, and it's not clear what I need to read right now.
> In fact, most of the times the read will yield 0 bytes because the
> other peer is not aware that we have closed the outbound (as nothing
> was produced to be sent to the other peer).
> 
>> For the specific case bellow, it is reasonable to expect
>> "NOT_HANDSHAKING" as the handshaking has not been started.  On the other
>> side, as only the inbound open, it is also reasonable to me to use
>> "NEED_UNWRAP" although there is nothing to unwrap.  I think, using
>> ""NOT_HANDSHAKING" may lead to confusing about what the next operation,
>> wrap() or unwrap(), could be in practice.  CLOSED is not an option to me
>> as the inbound is still open.
> 
> CLOSED is the _result_ (not the state) after a wrap().
> Seems to me that you are saying that NOT_HANDSHAKING is wrong because
> it does not tell what to do now, and NEED_UNWRAP is also wrong
> because, well, there is nothing to unwrap.
> What remains is NEED_WRAP, which will correctly tell what an
> application needs to do.
> This is exactly what happens after the TLS handshake is completed: the
> application is in NOT_HANDSHAKING and calling closeOutbound() changes
> the state into NEED_WRAP.
> So I think there is an asymmetry between the behavior of
> closeOutbound() before and after the handshake that does not seem
> justified?
> 
>> I understand there might be some compatibility issues for the use of
>> half-close policy.  I may close both inbound and outbound of an engine
>> in the application code if the connection is not used.
> 
> I think that if an application calls closeOutbound(), the
> implementation should only do that.
> 
>> Is there a known compatibility impact on you applications?
> 
> Yes.
> It's not possible to use the same code JDK 8 and JDK 11, unless there
> is some "if (isJDK11Behavior())" throughout the code.
> 
> Looking just at JDK 11 behavior, what happens is:
> * closeOutbound()
> * state is now NEED_UNWRAP
> * read from network, 0 bytes
> * call unwrap(), produces a BUFFER_UNDERFLOW result, basically says
> "wait for bytes to read" that will never come
> * 0 bytes have been produced so nothing gets sent to the other peer
> * the application does not know _when_ to send a FIN to the other peer
> (it's waiting for the state to move away from NEED_UNWRAP and/or for a
> CLOSED result)
> 
> Contrast that with:
> * closeOutbound()
> * state is now NEED_WRAP
> * call wrap(), produces CLOSED result, basically says "we are done
> with the output"
> * 0 bytes have been produced, so no close_notify is sent to the other
> peer, but we know from the CLOSED result that we can now send the FIN
> to the other peer.
> 
> Thanks!
>