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

[Xuelei Fan]

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