diff options
author | Alba Mendez <me@alba.sh> | 2019-05-24 20:02:04 +0200 |
---|---|---|
committer | Ujjwal Sharma <usharma1998@gmail.com> | 2019-05-29 20:44:24 +0530 |
commit | 28ec14fded61eed6d76707d3e003df1952a2d5ee (patch) | |
tree | b651407451ea971987ce64a93106f3ffc0cc115c /src/tls_wrap.cc | |
parent | b84b4d8c7dc957dd630a66ee372c6f29e173e98d (diff) | |
download | android-node-v8-28ec14fded61eed6d76707d3e003df1952a2d5ee.tar.gz android-node-v8-28ec14fded61eed6d76707d3e003df1952a2d5ee.tar.bz2 android-node-v8-28ec14fded61eed6d76707d3e003df1952a2d5ee.zip |
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 <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Diffstat (limited to 'src/tls_wrap.cc')
-rw-r--r-- | src/tls_wrap.cc | 90 |
1 files changed, 45 insertions, 45 deletions
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<uv_buf_t> 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<int>(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<int>(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<int>(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<int>(length)); + Debug(this, "Writing %zu bytes, written = %d", length, written); + + if (written == -1) { int err; Local<Value> 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<Value>& 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) |