diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index 7455928dbeba27..bcaf9b48e13db8 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -929,8 +929,8 @@ Server.prototype._setServerData = function(data) {
};
-Server.prototype.getTicketKeys = function getTicketKeys(keys) {
- return this._sharedCreds.context.getTicketKeys(keys);
+Server.prototype.getTicketKeys = function getTicketKeys() {
+ return this._sharedCreds.context.getTicketKeys();
};
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index c5db8377c442e7..1d9214f18dee58 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -1475,7 +1475,7 @@ int SSLWrap::NewSessionCallback(SSL* s, SSL_SESSION* sess) {
reinterpret_cast(session_id),
session_id_length).ToLocalChecked();
Local argv[] = { session, buff };
- w->new_session_wait_ = true;
+ w->awaiting_new_session_ = true;
w->MakeCallback(env->onnewsession_string(), arraysize(argv), argv);
return 0;
@@ -2003,6 +2003,7 @@ void SSLWrap::Renegotiate(const FunctionCallbackInfo& args) {
ClearErrorOnReturn clear_error_on_return;
+ // XXX(sam) Return/throw an error, don't discard the SSL error reason.
bool yes = SSL_renegotiate(w->ssl_.get()) == 1;
args.GetReturnValue().Set(yes);
}
@@ -2036,7 +2037,7 @@ template
void SSLWrap::NewSessionDone(const FunctionCallbackInfo& args) {
Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());
- w->new_session_wait_ = false;
+ w->awaiting_new_session_ = false;
w->NewSessionDoneCb();
}
diff --git a/src/node_crypto.h b/src/node_crypto.h
index ec784fff88f199..b64a8c249a1303 100644
--- a/src/node_crypto.h
+++ b/src/node_crypto.h
@@ -218,7 +218,7 @@ class SSLWrap {
kind_(kind),
next_sess_(nullptr),
session_callbacks_(false),
- new_session_wait_(false),
+ awaiting_new_session_(false),
cert_cb_(nullptr),
cert_cb_arg_(nullptr),
cert_cb_running_(false) {
@@ -234,7 +234,7 @@ class SSLWrap {
inline void enable_session_callbacks() { session_callbacks_ = true; }
inline bool is_server() const { return kind_ == kServer; }
inline bool is_client() const { return kind_ == kClient; }
- inline bool is_waiting_new_session() const { return new_session_wait_; }
+ inline bool is_awaiting_new_session() const { return awaiting_new_session_; }
inline bool is_waiting_cert_cb() const { return cert_cb_ != nullptr; }
protected:
@@ -324,7 +324,7 @@ class SSLWrap {
SSLSessionPointer next_sess_;
SSLPointer ssl_;
bool session_callbacks_;
- bool new_session_wait_;
+ bool awaiting_new_session_;
// SSL_set_cert_cb
CertCb cert_cb_;
diff --git a/src/node_crypto_bio.h b/src/node_crypto_bio.h
index 1c62fbbd359405..b7f1d4f169edfe 100644
--- a/src/node_crypto_bio.h
+++ b/src/node_crypto_bio.h
@@ -34,8 +34,8 @@ namespace node {
namespace crypto {
// This class represents buffers for OpenSSL I/O, implemented as a singly-linked
-// list of chunks. It can be used both for writing data from Node to OpenSSL
-// and back, but only one direction per instance.
+// list of chunks. It can be used either for writing data from Node to OpenSSL,
+// or for reading data back, but not both.
// The structure is only accessed, and owned by, the OpenSSL BIOPointer
// (a.k.a. std::unique_ptr).
class NodeBIO : public MemoryRetainer {
@@ -80,11 +80,12 @@ class NodeBIO : public MemoryRetainer {
// Put `len` bytes from `data` into buffer
void Write(const char* data, size_t size);
- // Return pointer to internal data and amount of
- // contiguous data available for future writes
+ // Return pointer to contiguous block of reserved data and the size available
+ // for future writes. Call Commit() once the write is complete.
char* PeekWritable(size_t* size);
- // Commit reserved data
+ // Specify how much data was written into the block returned by
+ // PeekWritable().
void Commit(size_t size);
diff --git a/src/node_crypto_clienthello.h b/src/node_crypto_clienthello.h
index 687e9589b6d932..3a834c85c20b65 100644
--- a/src/node_crypto_clienthello.h
+++ b/src/node_crypto_clienthello.h
@@ -30,6 +30,9 @@
namespace node {
namespace crypto {
+// Parse the client hello so we can do async session resumption. OpenSSL's
+// session resumption uses synchronous callbacks, see SSL_CTX_sess_set_get_cb
+// and get_session_cb.
class ClientHelloParser {
public:
inline ClientHelloParser();
diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
index ea3c98591b7409..10444fea4ab5bf 100644
--- a/src/stream_wrap.cc
+++ b/src/stream_wrap.cc
@@ -222,7 +222,7 @@ void LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) {
type = uv_pipe_pending_type(reinterpret_cast(stream()));
}
- // We should not be getting this callback if someone as already called
+ // We should not be getting this callback if someone has already called
// uv_close() on the handle.
CHECK_EQ(persistent().IsEmpty(), false);
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index f510b20488b1bd..e3daf9b650e318 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -21,6 +21,7 @@
#include "tls_wrap.h"
#include "async_wrap-inl.h"
+#include "debug_utils.h"
#include "node_buffer.h" // Buffer
#include "node_crypto.h" // SecureContext
#include "node_crypto_bio.h" // NodeBIO
@@ -74,15 +75,18 @@ TLSWrap::TLSWrap(Environment* env,
stream->PushStreamListener(this);
InitSSL();
+ Debug(this, "Created new TLSWrap");
}
TLSWrap::~TLSWrap() {
+ Debug(this, "~TLSWrap()");
sc_ = nullptr;
}
bool TLSWrap::InvokeQueued(int status, const char* error_str) {
+ Debug(this, "InvokeQueued(%d, %s)", status, error_str);
if (!write_callback_scheduled_)
return false;
@@ -97,6 +101,7 @@ bool TLSWrap::InvokeQueued(int status, const char* error_str) {
void TLSWrap::NewSessionDoneCb() {
+ Debug(this, "NewSessionDoneCb()");
Cycle();
}
@@ -116,6 +121,11 @@ void TLSWrap::InitSSL() {
#endif // SSL_MODE_RELEASE_BUFFERS
SSL_set_app_data(ssl_.get(), this);
+ // Using InfoCallback isn't how we are supposed to check handshake progress:
+ // https://github.com/openssl/openssl/issues/7199#issuecomment-420915993
+ //
+ // Note on when this gets called on various openssl versions:
+ // https://github.com/openssl/openssl/issues/7199#issuecomment-420670544
SSL_set_info_callback(ssl_.get(), SSLInfoCallback);
if (is_server()) {
@@ -169,6 +179,7 @@ void TLSWrap::Receive(const FunctionCallbackInfo& args) {
CHECK(Buffer::HasInstance(args[0]));
char* data = Buffer::Data(args[0]);
size_t len = Buffer::Length(args[0]);
+ Debug(wrap, "Receiving %zu bytes injected from JS", len);
// Copy given buffer entirely or partiall if handle becomes closed
while (len > 0 && wrap->IsAlive() && !wrap->IsClosing()) {
@@ -194,6 +205,9 @@ void TLSWrap::Start(const FunctionCallbackInfo& args) {
// Send ClientHello handshake
CHECK(wrap->is_client());
+ // Seems odd to read when when we want to send, but SSL_read() triggers a
+ // handshake if a session isn't established, and handshake will cause
+ // encrypted data to become available for output.
wrap->ClearOut();
wrap->EncOut();
}
@@ -213,8 +227,13 @@ void TLSWrap::SSLInfoCallback(const SSL* ssl_, int where, int ret) {
Local