RE: [8u] RFR: 8074462: Handshake messages can be strictly ordered

[Prasadrao Koppula]

Hi Xuelei,

Thanks for quick review. All the points are valid and implemented with your 
suggestions. 

Thanks,
Prasad.K

-Original Message-
From: Xuelei Fan 
Sent: Friday, June 29, 2018 8:17 PM
To: Prasadrao Koppula ; 
security-dev@openjdk.java.net
Subject: Re: [8u] RFR: 8074462: Handshake messages can be strictly ordered

Hi Prasad,

Thanks for take care of this issue.

SSLEngineImpl.java
--
1422 private HandshakeStatus finishHandshake() {
1423 handshaker = null;
1424 connectionState = cs_DATA;
1425 return HandshakeStatus.FINISHED;
1426}

This method are used in two places, for different purposes.  It is a little bit 
confusing, especially the code in line 1019.  I may just remove this method, 
and update the calling line accordingly.

@@ -1024,12 +1014,11 @@:
---
-  handshaker = null;
-  receivedCCS = false;
+  finishHandshake();

May be sufficient to remove the "receivedCCS = false" only:
handshaker = null;
-  receivedCCS = false;


@@ -1040,15 +1029,14 @@
---
if (!writer.hasOutboundData()) {
-   hsStatus = HandshakeStatus.FINISHED;
+   hsStatus = finishHandshake();
}
-  handshaker = null;
connectionState = cs_DATA;
-  receivedCCS = false;
+  handshaker = null;

May be sufficient to remove the "receivedCCS = false" only:
if (!writer.hasOutboundData()) {
 hsStatus = HandshakeStatus.FINISHED;
}
handshaker = null;
connectionState = cs_DATA;
-  receivedCCS = false;

Otherwise, looks fine to me.

Xuelei

On 6/29/2018 1:06 AM, Prasadrao Koppula wrote:
> Could you please review the changes
> 
> Webrev: http://cr.openjdk.java.net/~pkoppula/8074462/webrev.00/
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8074462
> 
> To fix handshake message out-of-order issues, we extracted required 
> classes/ code from JEP 219 implementation.
> 
> Contributed by: Sean Coffey and Prasadrao Koppula
> 
> Thanks,
> Prasad.K
> 

Re: [8u] RFR: 8074462: Handshake messages can be strictly ordered

[Xuelei Fan]

Hi Prasad,

Thanks for take care of this issue.

SSLEngineImpl.java
--
1422 private HandshakeStatus finishHandshake() {
1423 handshaker = null;
1424 connectionState = cs_DATA;
1425 return HandshakeStatus.FINISHED;
1426}

This method are used in two places, for different purposes.  It is a 
little bit confusing, especially the code in line 1019.  I may just 
remove this method, and update the calling line accordingly.


@@ -1024,12 +1014,11 @@:
---
-  handshaker = null;
-  receivedCCS = false;
+  finishHandshake();

May be sufficient to remove the "receivedCCS = false" only:
   handshaker = null;
-  receivedCCS = false;


@@ -1040,15 +1029,14 @@
---
   if (!writer.hasOutboundData()) {
-   hsStatus = HandshakeStatus.FINISHED;
+   hsStatus = finishHandshake();
   }
-  handshaker = null;
   connectionState = cs_DATA;
-  receivedCCS = false;
+  handshaker = null;

May be sufficient to remove the "receivedCCS = false" only:
   if (!writer.hasOutboundData()) {
hsStatus = HandshakeStatus.FINISHED;
   }
   handshaker = null;
   connectionState = cs_DATA;
-  receivedCCS = false;

Otherwise, looks fine to me.

Xuelei

On 6/29/2018 1:06 AM, Prasadrao Koppula wrote:

Could you please review the changes

Webrev: http://cr.openjdk.java.net/~pkoppula/8074462/webrev.00/

JBS: https://bugs.openjdk.java.net/browse/JDK-8074462

To fix handshake message out-of-order issues, we extracted required 
classes/ code from JEP 219 implementation.


Contributed by: Sean Coffey and Prasadrao Koppula

Thanks,
Prasad.K