Code Review Needed: 7031830: bad_record_mac failure on TLSv1.2 enabled connection with SSLEngine

[Brad Wetmore]

(Max/Valerie/Sean, I am hoping to get this into 7u2, thus need a second 
codereview.  The code itself is pretty straightforward.)

Xuelei wrote:
 > The change of SSLEngineImpl looks fine to me.
 >
 > In the new regression test:

Changes made.  Thanks for the feedback, Xuelei.  I tweaked things a bit 
even more in the test case only.  Now, I alternate between the client 
and the server initiating the closing.

http://cr.openjdk.java.net/~wetmore/7031830/

See the .01 versions.

Brad




On 10/14/2011 7:23 PM, Xuelei Fan wrote:
> The change of SSLEngineImpl looks fine to me.
>
> In the new regression test:
>
> A. class name
>
> 28  * @run main/othervm SSLEngineTemplate
> 29  *
> 30  *     SunJSSE does not support dynamic system properties, no way to
> re-use
> 31  *     system properties in samevm/agentvm mode.
>
> I think the class name should be SSLEngineBadBufferArrayAccess.
> - 28  * @run main/othervm SSLEngineTemplate
> + 28  * @run main/othervm SSLEngineBadBufferArrayAccess
>
> B. a minor comment, reuse the serverEngine.wrap() logic.
> Is it possible to remove line 286 ~ 297, and have
> "while(!isEngineClosed())" handle the close notify? So that the server
> engine would be able to handle the client close_notify response.
>
>     boolean exchangeHasDone = false;
>     while (!isEngineClosed()) {
> 283   if (!exchangeHasDone&&  clientMsg.length == serverIn.position()) {
>           // sanity check
>           ...
>           serverEngine.closeOutbound();
>           exchangeHasDone = true.
>       }
>     } catch (...) {
>
>
> Xuelei
>
> On 10/15/2011 8:52 AM, Brad Wetmore wrote:
>> Hi Xuelei,
>>
>> I need code reviews for the bug I mentioned to you earlier.
>>
>> 7031830: bad_record_mac failure on TLSv1.2 enabled connection with
>> SSLEngine
>>
>> The MAC calculation was summing the wrong data range when using
>> non-direct byte buffers and TLS1.1/1.2.
>>
>> The new regression test will now interop-test SSLEngine with SSLSockets
>> using both direct and non-direct ByteBuffers, over SSLv3, TLSv1,
>> TLSv1.1, and TLSv1.2.
>>
>> http://cr.openjdk.java.net/~wetmore/7031830/
>>
>> I plan to push this to both JDK 8 and 7u2, so there are 2 webrevs there.
>>   They should be the same.
>>
>> Brad
>