Ensure key / values are shared between resumed sessions #13819
Conversation
normanmaurer
force-pushed
the
ssl_session
branch
from
e8c92d0
to
a222c4f
Compare
|
I don't think this change updates the last accessed time on session reuse. The The |
|
@chrisvest got it... Will do some more changes |
normanmaurer
force-pushed
the
ssl_session
branch
from
177ec58
to
8bb6564
Compare
handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
Show resolved
Hide resolved
normanmaurer
changed the title
normanmaurer
force-pushed
the
ssl_session
branch
from
12cd670
to
ddd5ac7
Compare
normanmaurer
changed the title
Motivation: When a session is resumed we need to also ensure we preserve the values that were put into the internal storage of the session. Modifications: Ensure we preserve the values / keys of the internal storage on resumption Result: Correctly implement session caching and reuse Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
normanmaurer
force-pushed
the
ssl_session
branch
from
ddd5ac7
to
39d9592
Compare
normanmaurer
force-pushed
the
ssl_session
branch
from
74752ba
to
612a5bf
Compare
|
This will need to be ported to quic as well |
normanmaurer
force-pushed
the
ssl_session
branch
from
02bd4f6
to
e5492b3
Compare
Motivation: When a session is resumed we need to also ensure we preserve the values that were put into the internal storage of the session. Modifications: Ensure we preserve the values / keys of the internal storage on resumption Result: Correctly implement session caching and reuse --------- Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Motivation: When a session is resumed we need to also ensure we preserve the values that were put into the internal storage of the session. Modifications: Ensure we preserve the values / keys of the internal storage on resumption Result: Correctly implement session caching and reuse --------- Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
| final String handshakeKey = "handshake"; | ||
| TrustManagerFactory tmf = new ConstantTrustManagerFactory(new EmptyExtendedX509TrustManager() { | ||
| @Override | ||
| public void checkClientTrusted(java.security.cert.X509Certificate[] chain, | ||
| String authType, SSLEngine engine) { | ||
| // This is broken in conscrypt. | ||
| // TODO: Open an issue in the conscrypt project. | ||
| if (!Conscrypt.isEngineSupported(engine)) { | ||
| assertEquals(0, engine.getHandshakeSession().getValueNames().length); | ||
| } | ||
| engine.getHandshakeSession().putValue(handshakeKey, Boolean.TRUE); | ||
| } | ||
|
|
||
| @Override | ||
| public void checkServerTrusted(java.security.cert.X509Certificate[] chain, | ||
| String authType, SSLEngine engine) { | ||
| // This is broken in conscrypt. | ||
| // TODO: Open an issue in the conscrypt project. | ||
| if (!Conscrypt.isEngineSupported(engine)) { | ||
| assertEquals(0, engine.getHandshakeSession().getValueNames().length); | ||
| } | ||
| engine.getHandshakeSession().putValue(handshakeKey, Boolean.TRUE); | ||
| } | ||
| }); |
There was a problem hiding this comment.
@XueleiFan I want to bring your attention to the use case captured here in this test, in the context of https://bugs.openjdk.org/browse/JDK-8245551.
What is being replicated here, is that a TrustManager implementation may - as part of validating the peer certificate - also compute a principal or other authorization data, capturing the identity, privileges and/or capabilities of the peer, and put that in the handshake session. Thus allowing the surrounding framework or application to later extract this data, make authorization decisions based on it.
Using the TrustManager in this way has two main advantages.
-
Such authorization decisions needs to be based on the validated peer certificate chain, rather than the chain-as-presented (which may have the certs out-of-order), and the
TrustManageris in the correct place and time to compute and inspect validated certificate chains. -
Extracting and validating such certificate data is potentially expensive and may involve encryption operations (for instance, the JDK
TrustManagerimplementation does not share the validated chain it computes, so implementations based on it will have to compute it again), so it is desirable for performance to do this work per session, rather than per request.
From reading https://bugs.openjdk.org/browse/JDK-8245551 it sounds like we are being set up for these sessions to statelessly resume. It's not clear from the issue how distributed sessions would be enabled, or if it is somehow enabled by default and happens automatically (how would that work?). But stateless resumption would cause the authorization data to be lost, which the application might not expected, and may cause security issues.
I'm wondering if the session being stateless is an a priori assumption, or if it will be determined based on whether the handshake session gets any information added to it, or something else. If it's going to be the case that a session may or may not be in a stateless mode, and there's no changing this even as early as the TrustManager calls, then it's certainly desirable to be able to determine this fact from the session (and handshake session) so that informed decisions can be made about it; to reject sessions, to deploy work-arounds, etc.
I'll also note that in this scenario, if the session data is local-only then it won't be advantageous for the sessions to move across hosts, since the applications will be forced to have a request-level filter that re-computes validated certificate chains and extracts authorization on every move.
That's my feedback on the JDK issue.
/cc @sbordet FYI.
Motivation:
When a session is resumed we need to also ensure we preserve the values that were put into the internal storage of the session.
Modifications:
Ensure we preserve the values / keys of the internal storage on resumption
Result:
Correctly implement session caching and reuse