From 28ec14fded61eed6d76707d3e003df1952a2d5ee Mon Sep 17 00:00:00 2001 From: Alba Mendez Date: Fri, 24 May 2019 20:02:04 +0200 Subject: tls: group chunks into TLS segments TLSWrap::DoWrite() now concatenates data chunks and makes a single call to SSL_write(). Grouping data into a single segment: - reduces network overhead: by factors of even 2 or 3 in usages like `http2` or `form-data` - improves security: segment lengths can reveal lots of info, i.e. with `form-data`, how many fields are sent and the approximate length of every individual field and its headers - reduces encryption overhead: a quick benchmark showed a ~30% CPU time decrease for an extreme case, see https://github.com/nodejs/node/issues/27573#issuecomment-493787867 Fixes: https://github.com/nodejs/node/issues/27573 PR-URL: https://github.com/nodejs/node/pull/27861 Reviewed-By: Fedor Indutny Reviewed-By: Anna Henningsen Reviewed-By: Ujjwal Sharma Reviewed-By: Rich Trott --- src/tls_wrap.cc | 90 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 45 insertions(+), 45 deletions(-) (limited to 'src/tls_wrap.cc') diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 2b0da3e3bc..cde5419b9c 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -302,7 +302,7 @@ void TLSWrap::EncOut() { // No encrypted output ready to write to the underlying stream. if (BIO_pending(enc_out_) == 0) { Debug(this, "No pending encrypted output"); - if (pending_cleartext_input_.empty()) { + if (pending_cleartext_input_.size() == 0) { if (!in_dowrite_) { Debug(this, "No pending cleartext input, not inside DoWrite()"); InvokeQueued(0); @@ -573,28 +573,21 @@ void TLSWrap::ClearIn() { return; } - std::vector buffers; - buffers.swap(pending_cleartext_input_); + if (pending_cleartext_input_.size() == 0) { + Debug(this, "Returning from ClearIn(), no pending data"); + return; + } + AllocatedBuffer data = std::move(pending_cleartext_input_); crypto::MarkPopErrorOnReturn mark_pop_error_on_return; - size_t i; - int written = 0; - for (i = 0; i < buffers.size(); ++i) { - size_t avail = buffers[i].len; - char* data = buffers[i].base; - written = SSL_write(ssl_.get(), data, avail); - Debug(this, "Writing %zu bytes, written = %d", avail, written); - CHECK(written == -1 || written == static_cast(avail)); - if (written == -1) - break; - } + int written = SSL_write(ssl_.get(), data.data(), data.size()); + Debug(this, "Writing %zu bytes, written = %d", data.size(), written); + CHECK(written == -1 || written == static_cast(data.size())); // All written - if (i == buffers.size()) { + if (written != -1) { Debug(this, "Successfully wrote all data to SSL"); - // We wrote all the buffers, so no writes failed (written < 0 on failure). - CHECK_GE(written, 0); return; } @@ -612,13 +605,10 @@ void TLSWrap::ClearIn() { // .code/.function/.etc, if possible. InvokeQueued(UV_EPROTO, error_str.c_str()); } else { - Debug(this, "Pushing back %zu buffers", buffers.size() - i); - // Push back the not-yet-written pending buffers into their queue. - // This can be skipped in the error case because no further writes - // would succeed anyway. - pending_cleartext_input_.insert(pending_cleartext_input_.end(), - buffers.begin() + i, - buffers.end()); + Debug(this, "Pushing data back"); + // Push back the not-yet-written data. This can be skipped in the error + // case because no further writes would succeed anyway. + pending_cleartext_input_ = std::move(data); } return; @@ -705,14 +695,10 @@ int TLSWrap::DoWrite(WriteWrap* w, return UV_EPROTO; } - bool empty = true; + size_t length = 0; size_t i; - for (i = 0; i < count; i++) { - if (bufs[i].len > 0) { - empty = false; - break; - } - } + for (i = 0; i < count; i++) + length += bufs[i].len; // We want to trigger a Write() on the underlying stream to drive the stream // system, but don't want to encrypt empty buffers into a TLS frame, so see @@ -724,7 +710,7 @@ int TLSWrap::DoWrite(WriteWrap* w, // stream. Since the bufs are empty, it won't actually write non-TLS data // onto the socket, we just want the side-effects. After, make sure the // WriteWrap was accepted by the stream, or that we call Done() on it. - if (empty) { + if (length == 0) { Debug(this, "Empty write"); ClearOut(); if (BIO_pending(enc_out_) == 0) { @@ -748,23 +734,36 @@ int TLSWrap::DoWrite(WriteWrap* w, current_write_ = w; // Write encrypted data to underlying stream and call Done(). - if (empty) { + if (length == 0) { EncOut(); return 0; } + AllocatedBuffer data; crypto::MarkPopErrorOnReturn mark_pop_error_on_return; int written = 0; - for (i = 0; i < count; i++) { - written = SSL_write(ssl_.get(), bufs[i].base, bufs[i].len); - CHECK(written == -1 || written == static_cast(bufs[i].len)); - Debug(this, "Writing %zu bytes, written = %d", bufs[i].len, written); - if (written == -1) - break; + if (count != 1) { + data = env()->AllocateManaged(length); + size_t offset = 0; + for (i = 0; i < count; i++) { + memcpy(data.data() + offset, bufs[i].base, bufs[i].len); + offset += bufs[i].len; + } + written = SSL_write(ssl_.get(), data.data(), length); + } else { + // Only one buffer: try to write directly, only store if it fails + written = SSL_write(ssl_.get(), bufs[0].base, bufs[0].len); + if (written == -1) { + data = env()->AllocateManaged(length); + memcpy(data.data(), bufs[0].base, bufs[0].len); + } } - if (i != count) { + CHECK(written == -1 || written == static_cast(length)); + Debug(this, "Writing %zu bytes, written = %d", length, written); + + if (written == -1) { int err; Local arg = GetSSLError(written, &err, &error_); @@ -775,11 +774,10 @@ int TLSWrap::DoWrite(WriteWrap* w, return UV_EPROTO; } - Debug(this, "Saving %zu buffers for later write", count - i); + Debug(this, "Saving data for later write"); // Otherwise, save unwritten data so it can be written later by ClearIn(). - pending_cleartext_input_.insert(pending_cleartext_input_.end(), - &bufs[i], - &bufs[count]); + CHECK_EQ(pending_cleartext_input_.size(), 0); + pending_cleartext_input_ = std::move(data); } // Write any encrypted/handshake output that may be ready. @@ -1082,7 +1080,9 @@ void TLSWrap::GetWriteQueueSize(const FunctionCallbackInfo& info) { void TLSWrap::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("error", error_); - tracker->TrackField("pending_cleartext_input", pending_cleartext_input_); + tracker->TrackFieldWithSize("pending_cleartext_input", + pending_cleartext_input_.size(), + "AllocatedBuffer"); if (enc_in_ != nullptr) tracker->TrackField("enc_in", crypto::NodeBIO::FromBIO(enc_in_)); if (enc_out_ != nullptr) -- cgit v1.2.3