> Can I please get a review of this change which proposes to fix the issue > reported in https://bugs.openjdk.org/browse/JDK-8301686? > > The internal implementation of SSLContext caches SSLSession(s). These > sessions are for a particular combination or peer host and port. When a TLS > handshake completes successfully, the session is then stored in this cache. > If/when subsequent handshake attempts against the same host/port combination > happens, using this same SSLContext instance, then the internal > implementation triggers a session resumption, which is allowed by the TLS > RFC. During session resumption, the client then uses the pre-shared key from > the previous successful handshake and sends it as part of the `ClientHello` > message. > > One other part of the TLS handshake is the `server_name` extension. The > client sends a `SNI` in the handshake which the server side can either reject > or accept. To facilitate this matching on the server side, the > `javax.net.ssl.SNIMatcher` can be configured on the (server side) > `SSLParameters`. Setting of `SNIMatcher` is optional. > > If a successful handshake session (that was cached by the client) used a SNI > name which the server accepted, then this SNI name is sent as part of the > session resumption `ClientHello` along with the pre-shared key. The current > issue is that, during session resumption, on the server side, if the > `SNIMatcher` is no longer present, then the server rightly aborts the session > resumption, but still ends up sending the pre-shared key extension as part of > the `ServerHello` message. The client, upon seeing this pre-shared key > extension in the `ServerHello` considers that the session resumption > succeeded and ends up using that pre-shared key to derive the early secret. > On the server side though, the server has already rejected this resumption > request and thus when the client next sends data, the server will no longer > be able to decode the data and will fail with > `javax.crypto.AEADBadTagException: Tag mismatch` as noted in the JBS issue. > > The change in this PR, removes the pre-shared key extension data from the > `ServerHello` message, when the server side notices that the session > resumption is being aborted. > > A new jtreg test has been added which reproduces the issue and verifies the > fix. Existing tests in tier1, tier2 and tier3 continue to pass with this > change.
Jaikiran Pai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision: - limit line length in test case to 80 chars - Brad's suggestion - replace "the" with "this" - merge latest from master branch - review comment - use SSLContextTemplate for SSLContext creation in test - 8301686: TLS 1.3 handshake fails if server_name doesn't match resuming session ------------- Changes: - all: https://git.openjdk.org/jdk/pull/13669/files - new: https://git.openjdk.org/jdk/pull/13669/files/57df739d..c6ec3eb9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=13669&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13669&range=01-02 Stats: 578613 lines in 9655 files changed: 336694 ins; 149888 del; 92031 mod Patch: https://git.openjdk.org/jdk/pull/13669.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13669/head:pull/13669 PR: https://git.openjdk.org/jdk/pull/13669
