TLS v1.3 Fallback for JDK bug#1149
Conversation
| Set<String> enabledProtocols = new HashSet<>(Arrays.asList(socket.getEnabledProtocols())); | ||
| if (enabledProtocols.contains(TLS_v_1_3) && e.getMessage().contains("should not be presented in")) { | ||
|
|
||
| logger.warn("Workaround for JDK Bug JDK-8236039 applied, will connect without TLS v1.3"); |
There was a problem hiding this comment.
Seems like we're re-triggering this exception for every request to APM Server, right? Could we exclude TLSv1.3 if we know it triggers the bug on the current JVM?
There was a problem hiding this comment.
Maybe also suggest the user what to do when the exception occurs, like updating the JDK, or removing 1.3 for the server's setting (https://www.elastic.co/guide/en/apm/server/current/agent-server-ssl.html#_supported_protocols_2).
There was a problem hiding this comment.
I agree with both comments, having a single warning is better, and an actionable message is definitely better than having to deal with that in an FAQ somewhere :-).
There was a problem hiding this comment.
Also, there would be the option to disable TLS 1.3 client side, but that would impact the whole JVM, or provide a configuration for agent, which implies some extra complexity for documentation/support.
apm-agent-core/src/main/java/co/elastic/apm/agent/report/ssl/TLSFallbackSSLSocket.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| private static void tlsFallback(HttpsURLConnection connection) { | ||
| SSLSocketFactory newFactory = SslUtils.getTLSFallbackSocketFactory(connection.getSSLSocketFactory()); |
There was a problem hiding this comment.
Why don't we create our own SSLSocketFactory from scratch?
There was a problem hiding this comment.
You mean, why do we wrap the default SSLSocketFactory where we could just have created a new one ?
For me that's just a way to make sure that our implementation does not introduce obvious security bugs or unexpected behaviors here. Also, the default implementation is not public thus we can't inherit from it.
There was a problem hiding this comment.
I mean similar to this: https://gist.github.com/fkrauthan/ac8624466a4dee4fd02f
…LSFallbackSSLSocket.java Co-Authored-By: Felix Barnsteiner <felixbarny@users.noreply.github.com>
2 participants
What does this PR do?
This PR implements a fallback for #1080
TLS 1.3 has been added in JDK 11, but due to a JDK bug (JDK-8236039), it can't be used in affected versions.
By default, there is no fallback on other versions of the protocol, even if those are both available and supported by the remote server.
This PR implements the following fallback:
TLS v1.3protocol is enabled.TLS v1.3and restarts handshake.When applied, the following warning is present in agent log:
Checklist
I have made corresponding changes to the documentationI have updated supported-technologies.asciidocAdded an API method or config option? Document in which version this will be introducedAdded an instrumentation plugin? How did you make sure that old, non-supported versions are not instrumented by accident?Author's Checklist
Related issues