tls: defer re-entrant calls to SSL state machine from JS - #65105
Conversation
|
Review requested:
|
Codecov Reportβ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65105 +/- ##
==========================================
- Coverage 90.18% 90.15% -0.03%
==========================================
Files 771 771
Lines 264911 265513 +602
Branches 50309 50484 +175
==========================================
+ Hits 238919 239385 +466
- Misses 16966 17054 +88
- Partials 9026 9074 +48
π New features to boost your workflow:
|
30ff143 to
23a2828
Compare
23a2828 to
9275beb
Compare
Signed-off-by: Tim Perry <pimterry@gmail.com>
9275beb to
8669486
Compare
|
@nodejs/crypto some eyes on this would be helpful. This fixes various cases (see tests) where using a TLS socket from inside our own callbacks breaks Node today, due to bad behaviour that BoringSSL is planning to soon directly guard against: https://boringssl-review.googlesource.com/c/boringssl/+/97087. #65035 has the full context. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Commit Queue failedThis pull request has multiple commits, but no landing policy was selected. Add
commit-queue-squash
The pull request was removed from the Commit Queue and labeled
commit-queue-failed
Full Commit Queue output |
|
Landed in 46bbfc4 |
This is a step towards #65035: making sure that we never mess with the OpenSSL/BoringSSL state machine from inside it.
Rather than hooking each of our callbacks/events inside the SSL library scope, I've wrapped each call that goes into OpenSSL at the top level, and then I'm checking against this in
DoWrite,DoShutdownandCycle, which can be triggered from JS. In each case, if we're inside the SSL library scope (which means we're being called from a JS callback that was triggered by the SSL library) then we defer.This covers some cases that currently work by luck but will break soon in BoringSSL and some cases that already fail today, like the ALPNCallback write example (see #65035 for details).
This just covers SSL_read/SSL_write/SSL_shutdown which are impacted by the pending BoringSSL patch, we may need to review other SSL_set... calls as well later.