sun.security.ssl.ProtocolVersion.valueOf(...) in Java8 and TLSv1.3

[Norman Maurer]

So I did some more testing and all in all it seems to work. I will do some more investigating still and then log the request if I can not see any negative side-effects.

Bye
Norman


> On 18. Sep 2018, at 18:08, Norman Maurer <norman.maurer at googlemail.com> wrote:
> 
> I did some testing and it seems to “just work”. I will do a bit more and then come back you + filling the enhancement request.
> 
> Bye
> Norman
> 
> 
>> On 17. Sep 2018, at 18:04, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>> 
>> Hi Norman,
>> 
>> In general, it is risk to support unknown protocol version in the key/trust manager implementation.  The trust manager implemented for TLS 1.2 and prior versions might not work with TLS 1.3 probably.  Did you make the evaluation?  Would you mind contribute a patch?
>> 
>> Please feel free to file an enhancement request, for further evaluation when there is a chance.
>> 
>> Thanks,
>> Xuelei
>> 
>> On 9/17/2018 5:28 AM, Norman Maurer wrote:
>>> Hi all,
>>> In netty we support using OpenSSL for native SSL which recently added TLSv1.3 support as part of OpenSSL 1.1.1. In an ideal world we would be able to use this even with Java8 which is almost true except the fact that sun.security.ssl.ProtocolVersion.valueOf(…) will throw an IllegalArgumentException when the string “TLSv1.3” is provided.  This is problematic as this happens during validation in the default TrustManager used by the OpenJDK.
>>> The stack looks something like this:
>>> java.lang.IllegalArgumentException: TLSv1.3
>>> 	at sun.security.ssl.ProtocolVersion.valueOf(ProtocolVersion.java:187)
>>> 	at sun.security.ssl.X509TrustManagerImpl.checkTrusted(X509TrustManagerImpl.java:258)
>>> 	at sun.security.ssl.X509TrustManagerImpl.checkServerTrusted(X509TrustManagerImpl.java:136)
>>> I could work around this by just wrap the SSLSession and return TLSv1.2 during validation as the same validation should be done as in the TLSv1.2 implementation but this is really just a hack.So I wonder if you would consider to either add support for parsing TLSv1.3 to the internal enum or just handle it more gracefully. Another thing that would work of course is to always provide a custom X509ExtendedTrustManager but I hoped to be able to re-use the default implementation as it already implements a lot of verification logic.
>>> WDYT ?
>>> Norman
>