Skip to content

Commit

Permalink
tls: group chunks into TLS segments
Browse files Browse the repository at this point in the history
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
  #27573 (comment)

Fixes: #27573

Backport-PR-URL: #28904
PR-URL: #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>
  • Loading branch information
mildsunrise authored and BethGriggs committed Oct 7, 2019
1 parent f78ecc3 commit fe58bca
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 45 deletions.
86 changes: 42 additions & 44 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,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)
InvokeQueued(0);
return;
}
Expand Down Expand Up @@ -509,28 +509,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;
}

std::vector<char> 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;
}

Expand All @@ -548,13 +541,10 @@ void TLSWrap::ClearIn() {
// 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;
Expand Down Expand Up @@ -640,14 +630,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
Expand All @@ -659,7 +645,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) {
Expand All @@ -683,23 +669,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;
}

std::vector<char> 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 = std::vector<char>(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 = std::vector<char>(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_);

Expand All @@ -710,11 +709,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.
Expand Down
2 changes: 1 addition & 1 deletion src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class TLSWrap : public AsyncWrap,
BIO* enc_in_ = nullptr; // StreamListener fills this for SSL_read().
BIO* enc_out_ = nullptr; // SSL_write()/handshake fills this for EncOut().
// Waiting for ClearIn() to pass to SSL_write().
std::vector<uv_buf_t> pending_cleartext_input_;
std::vector<char> pending_cleartext_input_;
size_t write_size_ = 0;
WriteWrap* current_write_ = nullptr;
WriteWrap* current_empty_write_ = nullptr;
Expand Down

0 comments on commit fe58bca

Please sign in to comment.