RFR JDK-8214129: SSL session resumption/SNI with TLS1.2 causes StackOverflowError

[Xue-Lei Fan]

Hi Jamil,

For a defense in depth fix, as you are already there, I may suggest 
update two more places.

ServerNameExtension.java:
------------------------
   private CHServerNamesSpec(List<SNIServerName> serverNames) {
       this.serverNames =
           Collections.<SNIServerName>unmodifiableList(serverNames);
   }

SSLSessionImpl.java:
--------------------
this.localSupportedSignAlgs = ... // two places in the constructors


The coding style of the use of unmodifiableList() looks a little bit 
weird for a code reader who does not familar with the limitation of 
unmodifiableList() method.  What do you think if adding a comment about 
why we code this way?  Just in case, someone may rollback it later.

Otherwise, looks fine to me.

Thanks,
Xuelei


On 12/5/2018 3:59 PM, Jamil Nimeh wrote:
> Hello all,
> 
> This fix covers an issue where large numbers of TLS 1.2 session 
> resumptions were causing a StackOverflowError to occur.  This was 
> happening because the SSLSessionImpl constructor creates a new 
> unmodifiableList from the SNI list attached to the HandshakeContext. 
> Since that is also an unmodifiableList, you get a new level of nesting 
> of lists with each successive instantiation of SSLSessionImpl. 
> Eventually it grows to the point that an iteration of the list causes a 
> stack overflow.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8214129
> 
> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8214129/webrev.01/
> 
> Thanks,
> 
> --Jamil
>