Code Review Request, JDK-8185576 : New handshake implementation

[Xuelei Fan]

Thanks for the code review.  I will wrap up the updates in the next webrev.

I plan to close the code review for the new handshaking implementation 
by the end of January (Jan. 31, 2018).  Please let me know if you need 
more time.

Thanks,
Xuelei

On 1/18/2018 7:26 AM, Adam Petcher wrote:
> Disclaimer: I am not a Reviewer. This is not a Review.
> 
> I looked through all of the code and I didn't find any significant 
> issues. In general, I think this is a nice improvement, and it makes the 
> code easier to understand. Below are a few minor comments:
> 
> SupportedGroups.java: There are (draft) OIDs for X25519 and X448, in 
> case you want to include them here: 
> https://tools.ietf.org/html/draft-ietf-curdle-pkix-01. Probably no need 
> to worry about this now, though---we will fix all this when we add 
> support for X25519/X448 later.
> 
> Utilities.java: intent -> indent
> 
> I found some "TODO" strings in several of the files. I don't know if you 
> intended to leave them in for now. If you are going to leave them in, it 
> would be more clear if you included more information saying why those 
> parts are not done. E.g. "TODO: TLS 1.3" or "TODO: JDK-8888888".
> 
> Multiple files: concumer -> consumer
> 
> PredefinedDHParameterSpecs.java: Can we remove all the primes less than 
> 1024 bits? They are all obsolete since the last update release. Also, 
> some of these values are duplicated in 
> sun.security.provider.ParameterCache.
> 
> 
> On 12/15/2017 11:20 PM, Xuelei Fan wrote:
>> Hi,
>>
>> Please review the change:
>>    http://cr.openjdk.java.net/~xuelei/8185576/webrev.00/
>>
>> This fix is trying to re-org the TLS handshaking implementation in the 
>> SunJSSE provider.
>>
>> The handshaking process in TLS 1.3 is lot different from that of TLS 
>> 1.2.  For TLS 1.3 implementation, it is not straightforward any more 
>> to use the existing state machines for TLS 1.2.   For example, in TLS 
>> specification, the handshake message must be delivered in strict 
>> order. The handshake message order in TLS 1.3 protocol is re-designed 
>> and significant different from that of TLS 1.2.  If we re-use the 
>> state machine for TLS 1.3 and 1.2 and previous versions, the state 
>> machine becomes very complicated and hard to maintain.
>>
>> We are consider to simplify the handshake implementation by:
>> 1. Use difference handshake handler for different TLS versions.
>> 2. Improved message driven handler for different handshake messages.
>>
>> The basic ideas for the simplification look like:
>> 1. message/even driving handshake processing model.
>> An actor is a stateless service that is used to consume an input 
>> message, or produce output message, or both.
>>        message A             message B
>>      --------------> Actor A ------------> Actor B
>>
>> 2. immutable actor and centralized states.
>> Handshake states are represent in context objects (ConnectionContext), 
>> and the context object is passed to each actor.  An actor can update 
>> the state in the context object accordingly.  At the same time, an 
>> actor's behavior may be different for different states.
>>       message A                  message B
>>    --------------> Actor A --------------------------> Actor B
>>       Context               Context [Actor A updated]
>>
>> 3. dynamical actor hierarchical structure
>> The actor hierarchical structure is flexible and can be dynamically 
>> updated if needed.
>>
>>      message A                   message B
>>    --------------> Actor A ----------------------------> ...
>>    (Context)       |    (Context [Actor A updated])   ^
>>                    |                                  |
>>                    | Add Actor C between A and B      |
>>                    | [Optional]                       |
>>                    +----------------------------------+
>>
>>     <continue>
>>     ... <optional>        Message C
>>     [ Actor C --------------------------> ] Actor B
>>               (Context [Actor B updated]
>>
>> 4. consumer and producer
>> The consumer/producer pattern is used to consume input messages, and 
>> produce output messages.
>>
>>     message A                                message B
>>   ------------> Consumer A    Producer B ------------------->
>>    (Context)    |                  ^  (Context [Consumer A updated])
>>                 |                  |
>>                 | Add Producer B   |
>>                 +------------------+
>>
>>
>>                   message A
>>   Producer A -----------------------> Consumer A -------------------->
>>       |         (Context)             ^  (Context [Producer A updated])
>>       |                               |
>>       | Add consumer B                |
>>       +-------------------------------+
>>
>> In this implementation, I removed:
>> 1. the extended master secret extension
>> I will add it back soon.
>>
>> 2. the KRB5 cipher suite implementation.
>> I'm not very sure KRB5 cipher suites are still used in practice. 
>> Please let me know if you still using KRB5 cipher suite.  I may not 
>> add these cipher suite back unless it is necessary.
>>
>> 3. OCSP stapling
>> This feature will be added back later.
>>
>> As this re-org is for TLS 1.3, so it contains a few draft skeleton for 
>> TLS 1.3.  I will file these parts when doing the TLS 1.3 implementation.
>>
>> Please don't be hesitate if you have any questions.
>>
>> Regards,
>> Xuelei
>