Code Review Request: TLS 1.3 full handshake (JDK-8196584)

Xuelei Fan xuelei.fan at oracle.com
Fri Jun 8 00:02:28 UTC 2018


Looks all good catches to me.  Please take care of the update as well.

Thanks,
Xuelei

On 6/7/2018 4:03 PM, Anthony Scarpino wrote:
> Xuelei,
> 
> I'll push updates if you're are ok with the changes.
> 
> Tony
> ---
> 
> ServerHandshakeContext.java
> ServerHello.java
> - spelling nits only
> 
> HandshakeContext.java
> - Could getActiveCipherSuites() compile a list of cipher suites once per 
> ProtocolVersion instead doing it for each instance of ServerHello?  The 
> lists could then be cache for performance.  I believe all the checks, 
> like constraints or availability, are decided at startup.  This is 
> something that can be address at a later date.
> 
>   122: preferableSignatureAlgorithm is never used.
>   233: Throwing IOException is not needed.
>   503: isNegotiable(byte majorVersion, byte minorVersion) not used
> 
> TransportContext.java
>    85: baseWriteSecret, baseReadSecret never used
>   476: closeInbound()  does not need to throw SSLException
> 
>   Removing below commented out code in passiveInboundClose()
>   515 // For TLS 1.3, output closure is independent from input  closure.
>   516 //      if (isNegotiated && protocolVersion.useTLS13PlusSpec()) {
>   517 //          return;
>   518 //      }
> 
>   Removing below commented out code in initiateOutboundClose()
>   582 // For TLS 1.3, output closure is independent from input closure.
>   583 //
>   584 //      if (isNegotiated && protocolVersion.useTLS13PlusSpec()) {
>   585 //          return;
>   586 //      }
> 
>   Removing below commented out code in finishHandshake()
>   644 // inputRecord and outputRecord shares the same handshakeHash
>   645 // inputRecord.handshakeHash.finish();


More information about the security-dev mailing list