Code review request, CR 7180038 regression test failure, SSLEngineBadBufferArrayAccess.java

[Xuelei Fan]

On 7/2/2012 4:35 PM, Weijun Wang wrote:
> I take a look at the test output. When the last handshake starts:
> 
> ================
> server unwrap: OK/NEED_TASK, 230/0 bytes
>     running delegated task...
>     new HandshakeStatus: NEED_WRAP
> ----
> server wrap: OK/NEED_WRAP, 0/86 bytes
> ================
> 
> Here the first wrap only generates 86 bytes, I guess that's the
> ServerHello message? It keeps the state at NEED_WRAP but then never
> really generates the Certificate message. What might be the problem?
> 
Good catch!

It was the abbreviated handshaking. I guess that the previous client has
not closed the socket completely, so for *this* handshaking, the
abbreviated handshaking rather than the full handshaking is used.

For full handshaking, it is the client sending the "Finished" message at
first. However, for abbreviated handshaking, the server send the
"Finished" message at first.

In the current scenarios, it is expected that the client sends its
application data (26 bytes), and then the server sends its application
data (29 bytes). However, the abbreviated handshaking disorder the
sequence in that it is the sever sends it application data (29 bytes)
before client. In such cases, the following logic does not stand any more:
    if (!closed && (serverOut.remaining() == 0)) {
       closed = true;
       ...
       if (serverIn.remaining() != clientMsg.length) {

           throw new Exception("Client:  Data length error");
       }
       ...
    }

Because the server has not receive the client message when the server
sends its application data.

I think it is a test issue, the current fix should has already addressed
the issue.

Xuelei

> Thanks
> Max
> 
> On 07/02/2012 10:39 AM, Xuelei Fan wrote:
>> Hi Weijun,
>>
>> Would you please review the test update for CR 7180038?
>>      http://cr.openjdk.java.net./~xuelei/7180038/webrev.00/
>>
>> We cannot reproduce the issue. However, from the test log, there is two
>> possible issues exposed by this CR.
>> 1. the improper test case senarios of un/wrap()
>>     In the test case, the scenarios is
>> unwrap()->wrap()->serverOut.remaining()->"serverIn.remaining() !=
>> clientMsg.length". After the wrap(), the next operation may need to be
>> unwrap() to get more incoming data before comparing serverIn buffer with
>> the expected client message.
>>
>>      This fix is trying to do the comparing after the engine has closed.
>>
>> 2. From the log, the engine status and handshaking status move from
>> CLOSED/NOT_HANDSHAKING to OK/FINISHED. FINISHED means the TLS
>> handshaking just finished. As the handshaking should have completed for
>> a while, it does not sound like a correct status change.
>>
>>      However, I did not find why this happens. Need more info. So I added
>> a line of log (suggested by Brad Wetmore) to collect the next failure:
>>
>>      IF THIS FAILS, PLEASE REPORT THIS TO THE SECURITY TEAM.  WE HAVE
>>      BEEN UNABLE TO RELIABLY DUPLICATE.
>>
>>
>> Thanks,
>> Xuelei
>>
>