Skip to content

Commit 4ef156b

Browse files
committed
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 #27573 (comment) Fixes: #27573
1 parent 42d8011 commit 4ef156b

File tree

2 files changed

+46
-46
lines changed

2 files changed

+46
-46
lines changed

src/tls_wrap.cc

+45-45
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ void TLSWrap::EncOut() {
302302
// No encrypted output ready to write to the underlying stream.
303303
if (BIO_pending(enc_out_) == 0) {
304304
Debug(this, "No pending encrypted output");
305-
if (pending_cleartext_input_.empty()) {
305+
if (pending_cleartext_input_.size() == 0) {
306306
if (!in_dowrite_) {
307307
Debug(this, "No pending cleartext input, not inside DoWrite()");
308308
InvokeQueued(0);
@@ -573,28 +573,21 @@ void TLSWrap::ClearIn() {
573573
return;
574574
}
575575

576-
std::vector<uv_buf_t> buffers;
577-
buffers.swap(pending_cleartext_input_);
576+
if (pending_cleartext_input_.size() == 0) {
577+
Debug(this, "Returning from ClearIn(), no pending data");
578+
return;
579+
}
578580

581+
AllocatedBuffer data = std::move(pending_cleartext_input_);
579582
crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
580583

581-
size_t i;
582-
int written = 0;
583-
for (i = 0; i < buffers.size(); ++i) {
584-
size_t avail = buffers[i].len;
585-
char* data = buffers[i].base;
586-
written = SSL_write(ssl_.get(), data, avail);
587-
Debug(this, "Writing %zu bytes, written = %d", avail, written);
588-
CHECK(written == -1 || written == static_cast<int>(avail));
589-
if (written == -1)
590-
break;
591-
}
584+
int written = SSL_write(ssl_.get(), data.data(), data.size());
585+
Debug(this, "Writing %zu bytes, written = %d", data.size(), written);
586+
CHECK(written == -1 || written == static_cast<int>(data.size()));
592587

593588
// All written
594-
if (i == buffers.size()) {
589+
if (written != -1) {
595590
Debug(this, "Successfully wrote all data to SSL");
596-
// We wrote all the buffers, so no writes failed (written < 0 on failure).
597-
CHECK_GE(written, 0);
598591
return;
599592
}
600593

@@ -612,13 +605,10 @@ void TLSWrap::ClearIn() {
612605
// .code/.function/.etc, if possible.
613606
InvokeQueued(UV_EPROTO, error_str.c_str());
614607
} else {
615-
Debug(this, "Pushing back %zu buffers", buffers.size() - i);
616-
// Push back the not-yet-written pending buffers into their queue.
617-
// This can be skipped in the error case because no further writes
618-
// would succeed anyway.
619-
pending_cleartext_input_.insert(pending_cleartext_input_.end(),
620-
buffers.begin() + i,
621-
buffers.end());
608+
Debug(this, "Pushing data back");
609+
// Push back the not-yet-written data. This can be skipped in the error
610+
// case because no further writes would succeed anyway.
611+
pending_cleartext_input_ = std::move(data);
622612
}
623613

624614
return;
@@ -705,14 +695,10 @@ int TLSWrap::DoWrite(WriteWrap* w,
705695
return UV_EPROTO;
706696
}
707697

708-
bool empty = true;
698+
size_t length = 0;
709699
size_t i;
710-
for (i = 0; i < count; i++) {
711-
if (bufs[i].len > 0) {
712-
empty = false;
713-
break;
714-
}
715-
}
700+
for (i = 0; i < count; i++)
701+
length += bufs[i].len;
716702

717703
// We want to trigger a Write() on the underlying stream to drive the stream
718704
// system, but don't want to encrypt empty buffers into a TLS frame, so see
@@ -724,7 +710,7 @@ int TLSWrap::DoWrite(WriteWrap* w,
724710
// stream. Since the bufs are empty, it won't actually write non-TLS data
725711
// onto the socket, we just want the side-effects. After, make sure the
726712
// WriteWrap was accepted by the stream, or that we call Done() on it.
727-
if (empty) {
713+
if (length == 0) {
728714
Debug(this, "Empty write");
729715
ClearOut();
730716
if (BIO_pending(enc_out_) == 0) {
@@ -748,23 +734,36 @@ int TLSWrap::DoWrite(WriteWrap* w,
748734
current_write_ = w;
749735

750736
// Write encrypted data to underlying stream and call Done().
751-
if (empty) {
737+
if (length == 0) {
752738
EncOut();
753739
return 0;
754740
}
755741

742+
AllocatedBuffer data;
756743
crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
757744

758745
int written = 0;
759-
for (i = 0; i < count; i++) {
760-
written = SSL_write(ssl_.get(), bufs[i].base, bufs[i].len);
761-
CHECK(written == -1 || written == static_cast<int>(bufs[i].len));
762-
Debug(this, "Writing %zu bytes, written = %d", bufs[i].len, written);
763-
if (written == -1)
764-
break;
746+
if (count != 1) {
747+
data = env()->AllocateManaged(length);
748+
size_t offset = 0;
749+
for (i = 0; i < count; i++) {
750+
memcpy(data.data() + offset, bufs[i].base, bufs[i].len);
751+
offset += bufs[i].len;
752+
}
753+
written = SSL_write(ssl_.get(), data.data(), length);
754+
} else {
755+
// Only one buffer: try to write directly, only store if it fails
756+
written = SSL_write(ssl_.get(), bufs[0].base, bufs[0].len);
757+
if (written == -1) {
758+
data = env()->AllocateManaged(length);
759+
memcpy(data.data(), bufs[0].base, bufs[0].len);
760+
}
765761
}
766762

767-
if (i != count) {
763+
CHECK(written == -1 || written == static_cast<int>(length));
764+
Debug(this, "Writing %zu bytes, written = %d", length, written);
765+
766+
if (written == -1) {
768767
int err;
769768
Local<Value> arg = GetSSLError(written, &err, &error_);
770769

@@ -775,11 +774,10 @@ int TLSWrap::DoWrite(WriteWrap* w,
775774
return UV_EPROTO;
776775
}
777776

778-
Debug(this, "Saving %zu buffers for later write", count - i);
777+
Debug(this, "Saving data for later write");
779778
// Otherwise, save unwritten data so it can be written later by ClearIn().
780-
pending_cleartext_input_.insert(pending_cleartext_input_.end(),
781-
&bufs[i],
782-
&bufs[count]);
779+
CHECK_EQ(pending_cleartext_input_.size(), 0);
780+
pending_cleartext_input_ = std::move(data);
783781
}
784782

785783
// Write any encrypted/handshake output that may be ready.
@@ -1082,7 +1080,9 @@ void TLSWrap::GetWriteQueueSize(const FunctionCallbackInfo<Value>& info) {
10821080

10831081
void TLSWrap::MemoryInfo(MemoryTracker* tracker) const {
10841082
tracker->TrackField("error", error_);
1085-
tracker->TrackField("pending_cleartext_input", pending_cleartext_input_);
1083+
tracker->TrackFieldWithSize("pending_cleartext_input",
1084+
pending_cleartext_input_.size(),
1085+
"AllocatedBuffer");
10861086
if (enc_in_ != nullptr)
10871087
tracker->TrackField("enc_in", crypto::NodeBIO::FromBIO(enc_in_));
10881088
if (enc_out_ != nullptr)

src/tls_wrap.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ class TLSWrap : public AsyncWrap,
174174
BIO* enc_in_ = nullptr; // StreamListener fills this for SSL_read().
175175
BIO* enc_out_ = nullptr; // SSL_write()/handshake fills this for EncOut().
176176
// Waiting for ClearIn() to pass to SSL_write().
177-
std::vector<uv_buf_t> pending_cleartext_input_;
177+
AllocatedBuffer pending_cleartext_input_;
178178
size_t write_size_ = 0;
179179
WriteWrap* current_write_ = nullptr;
180180
bool in_dowrite_ = false;

0 commit comments

Comments
 (0)