(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
