Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TLS: improve compliance with shutdown standard, remove hacks #36111

Closed
wants to merge 2 commits into from
Closed

TLS: improve compliance with shutdown standard, remove hacks #36111

wants to merge 2 commits into from

Commits on Nov 27, 2021

  1. TLS: improve handling of shutdown 

    RFC 5246 section-7.2.1 requires that the implementation must immediately
stop reading from the stream, as it is no longer TLS-encrypted. The
underlying stream is permitted to still pump events (and errors) to
other users, but those are now unencrypted, so we should not process
them here. But therefore, we do not want to stop the underlying stream,
as there could be another user of it, but we do need to remove ourselves
as a listener.

Per TLS v1.2, we should have also destroy the TLS state entirely here
(including the writing side), but this was revised in TLS v1.3 to permit
the stream to continue to flush output.

There appears to be some inconsistencies in the way nodejs handles
ownership of the underlying stream, with `TLS.close()` on the write side
also calling shutdown on the underlying stream (thus assuming other
users of the underlying stream are not permitted), while receiving EOF
on the read side leaves the underlying channel open. These
inconsistencies are left for a later person to resolve, if the extra
functionality is needed (as described in nodejs#35904). The current goal here
is to the fix the occasional CI exceptions depending on the timing of
these kernel messages through the TCP stack.

Refs: libuv/libuv#3036
Refs: nodejs#35904
Closes: nodejs#35946
Co-authored-by: Momtchil Momtchev <[email protected]>
    @vtjnash @mmomtchev
    vtjnash and mmomtchev committed Nov 27, 2021
    Configuration menu
    Copy the full SHA
    f199b1c View commit details
    Browse the repository at this point in the history

  2. Apply suggestions from code review

    @vtjnash
    vtjnash committed Nov 27, 2021
    Configuration menu
    Copy the full SHA
    a18156c View commit details
    Browse the repository at this point in the history