Code Review Needed: 7031830: bad_record_mac failure on TLSv1.2 enabled connection with SSLEngine (original) (raw)
Xuelei Fan xuelei.fan at oracle.com
Tue Oct 18 01:02:22 UTC 2011
- Previous message (by thread): Code Review Needed: 7031830: bad_record_mac failure on TLSv1.2 enabled connection with SSLEngine
- Next message (by thread): hg: jdk8/tl/jdk: 7099399: cannot deal with CRL file larger than 16MB
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Both (JDK 7u2 and JDK 8) look fine to me. Thanks for the update.
Xuelei
On 10/18/2011 8:41 AM, Brad Wetmore wrote:
(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 closenotify 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: badrecordmac 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
- Previous message (by thread): Code Review Needed: 7031830: bad_record_mac failure on TLSv1.2 enabled connection with SSLEngine
- Next message (by thread): hg: jdk8/tl/jdk: 7099399: cannot deal with CRL file larger than 16MB
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]