Code Review Request, JDK-8167680, DTLS implementation bugs

[Artem Smotrakov]

Hi Xuelei,

Looks good to me.

Artem


On 10/27/2016 05:50 AM, Xuelei Fan wrote:
> Updated to handle handle optional CertificateVerify message:
>
>     http://cr.openjdk.java.net/~xuelei/8167680/webrev.02/
>
> Thanks,
> Xuelei
>
> On 10/21/2016 11:31 AM, Xuelei Fan wrote:
>> Updated webrev per Jamil's comments:
>>
>>    http://cr.openjdk.java.net/~xuelei/8167680/webrev.01/
>>
>> Xuelei
>>
>> On 10/13/2016 10:36 PM, Xuelei Fan wrote:
>>> Hi,
>>>
>>> Please review the fix for JDK-8167680:
>>>    http://cr.openjdk.java.net/~xuelei/8167680/webrev.00/
>>>
>>> There are a few implementation bugs in JDK.
>>>
>>> 1. The sequence number is increased by 2 for GCM cipher suites.
>>> Both GCM crypto operation and DTLS record use the sequence number.  The
>>> current implementation may increase the sequence number for each of the
>>> two operations.  It is not the expected behavior.
>>>
>>> 2. The implementation does not response to handshake retransmissions.
>>> In the current implementation, receiving of retransmitted handshake
>>> messages does not kick off a retransmission of the previous delivered
>>> flight.  Retransmission happens on timeout.  Timeout is expensive.
>>> Supporting response to peer retransmitted handshake messages would 
>>> speed
>>> up the handshaking.
>>>
>>> 3. the final CCC and finished message cannot be retransmitted.
>>> It is an implementation bug.  Every handshake message should be able to
>>> get retransmitted.
>>>
>>> 4. the first application data may be discarded if the last flight get
>>> lost.
>>> Applications may send application data immediately after the handshake
>>> completed.  However, the peer may have not receive the handshake
>>> complete message, and therefor discard the application data. As may
>>> impact application logic.
>>>
>>> For example
>>>
>>>     Client                 Server
>>>                ....
>>>        -- ClientKeyExchange -->
>>>        -- ChangeCipherSpec  -->
>>>        -- Finished          -->
>>>
>>>
>>>        X <-- ChangeCipherSpec --
>>>        X <-- Finished         --
>>>
>>>        <-- Application Data  --
>>>
>>>        -----    ...        --->
>>>        -- ClientKeyExchange -->
>>>        -- ChangeCipherSpec  -->
>>>        -- Finished          -->
>>>
>>>        <-- ChangeCipherSpec --
>>>        <-- Finished         --
>>>
>>>
>>> 1. (omit the previous handshake messages) server sends ChangeCipherSpec
>>> and Finished messages.
>>> 2. server handshake complete
>>> 3. server send application
>>> 4. client does not receive the ChangeCipherSpec or Finished messages.
>>> 5. client receives the application data.  Client cannot handle the
>>> encrypted message, and may discard it.  Client re-transmit the previous
>>> flight.
>>> 6. server retransmit the last flight.
>>> 7. client receives the last flight.
>>>
>>> In this update, the last flight will be transmit twice.  As may 
>>> mitigate
>>> the impact of the issue.
>>>
>>> 5. resuming handshaking need no cookie exchange.
>>> It is an implementation bug.  Cookie exchange is performed for
>>> handshaking resuming now.  It is not the expected behavior.
>>>
>>> 6. need more debug log for DTLS handshake message fragmentation and
>>> reassembly.
>>>
>>>
>>> Thanks,
>>> Xuelei