Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
tls: fix re-entrancy issue with TLS close_notify
Like errno, OpenSSL's API requires SSL_get_error and error queue be checked immediately after the failing operation, otherwise the error queue or SSL object may have changed state and no longer report information about the operation the caller wanted. TLSWrap almost heeds this rule, except in TLSWrap::ClearOut. If SSL_read picks up a closing alert (detected by checking SSL_get_shutdown), Node calls out to JS with EmitRead(UV_EOF) and only afterwards proceeds to dispatch on the error. But, by this point, Node has already re-entered JS, which may change the error. In particular, I've observed that, on close_notify, JS seems to sometimes call back into TLSWrap::DoShutdown, calling SSL_shutdown. (I think this comes from onStreamRead in stream_base_commons.js?) Instead, SSL_get_error and the error queue should be sampled earlier. Back in #1661, Node needed to account for GetSSLError being called after ssl_ was destroyed. This was the real cause. With this fixed, there's no need to account for this. (Any case where ssl_ may be destroyed before SSL_get_error is a case where ssl_ or the error queue could change state, so it's a bug either way.) This is the first of two fixes in error-handling here. The EmitRead(UV_EOF) seems to additionally swallow fatal alerts from the peer. Some of the ECONNRESET expectations in the tests aren't actually correct. The next commit will fix this as well. PR-URL: #44563 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
- Loading branch information