diff --git a/benchmark/tls/throughput-s2c.js b/benchmark/tls/throughput-s2c.js index 7fb93c304b20..8a0b19359527 100644 --- a/benchmark/tls/throughput-s2c.js +++ b/benchmark/tls/throughput-s2c.js @@ -6,6 +6,8 @@ const bench = common.createBenchmark(main, { sendchunklen: [256, 32 * 1024, 128 * 1024, 16 * 1024 * 1024], recvbuflen: [0, 64 * 1024, 1024 * 1024], recvbufgenfn: ['true', 'false'], + fragment: [1024, 4096, 16384], + tlsVersion: ['TLSv1.2', 'TLSv1.3'], }); const fixtures = require('../../test/common/fixtures'); @@ -14,7 +16,8 @@ let recvbuf; let received = 0; const tls = require('tls'); -function main({ dur, type, sendchunklen, recvbuflen, recvbufgenfn }) { +function main({ dur, type, sendchunklen, recvbuflen, recvbufgenfn, + fragment, tlsVersion }) { if (isFinite(recvbuflen) && recvbuflen > 0) recvbuf = Buffer.alloc(recvbuflen); @@ -41,7 +44,8 @@ function main({ dur, type, sendchunklen, recvbuflen, recvbufgenfn }) { cert: fixtures.readKey('rsa_cert.crt'), ca: fixtures.readKey('rsa_ca.crt'), ciphers: 'AES256-GCM-SHA384', - maxVersion: 'TLSv1.2', + minVersion: tlsVersion, + maxVersion: tlsVersion, }; let socketOpts; @@ -74,6 +78,7 @@ function main({ dur, type, sendchunklen, recvbuflen, recvbufgenfn }) { } const server = tls.createServer(options, (socket) => { + socket.setMaxSendFragment(fragment); socket.on('data', (buf) => { socket.on('drain', write); write(); diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index fbc5fc8c39c9..dc5d58660480 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -150,6 +150,7 @@ ClientHelloResult EarlyClientHelloCallback(const ClientHelloContext& hello) { void KeylogCallback(const SSL* s, const char* line) { TLSWrap* w = static_cast(SSL_get_app_data(s)); + if (!w->FlushPendingClearOut()) return; Environment* env = w->env(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); @@ -173,6 +174,7 @@ int NewSessionCallback(SSL* s, SSL_SESSION* sess) { if (!w->has_session_callbacks()) [[unlikely]] return 0; + if (!w->FlushPendingClearOut()) return 0; // Check if session is small enough to be stored int size = i2d_SSL_SESSION(sess, nullptr); @@ -689,6 +691,7 @@ void TLSWrap::SSLInfoCallback(const SSL* ssl_, int where, int ret) { // SSL_renegotiate_pending() should take `const SSL*`, but it does not. SSL* ssl = const_cast(ssl_); TLSWrap* c = static_cast(SSL_get_app_data(ssl_)); + if (!c->FlushPendingClearOut()) return; Environment* env = c->env(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); @@ -725,6 +728,7 @@ void TLSWrap::SSLInfoCallback(const SSL* ssl_, int where, int ret) { } void TLSWrap::EncOut() { + if (destroy_pending_) return; Debug(this, "Trying to write encrypted output"); // Write in progress @@ -872,46 +876,68 @@ void TLSWrap::ClearOut() { return; } - if (ssl_ == nullptr) { - Debug(this, "Returning from ClearOut(), ssl_ == nullptr"); + if (ssl_ == nullptr || destroy_pending_) { + Debug(this, "Returning from ClearOut(), no usable SSL context"); return; } MarkPopErrorOnReturn mark_pop_error_on_return; + // SSL_read() returns at most one TLS record per call. Records that are + // already buffered are decrypted together, up to 64 KiB, + // and emitted as one chunk: one allocation and one call into JS for a burst + // of small records instead of one per record. char out[kClearOutChunkSize]; int read; for (;;) { - { - SSLLibraryCallScope ssl_library_call_scope(this); - read = SSL_read(ssl_.get(), out, sizeof(out)); - } - Debug(this, "Read %d bytes of cleartext output", read); - - if (read <= 0) - break; - - char* current = out; - while (read > 0) { - int avail = read; - - uv_buf_t buf = EmitAlloc(avail); - if (static_cast(buf.len) < avail) - avail = buf.len; - memcpy(buf.base, current, avail); - EmitRead(avail, buf); - - // Caveat emptor: OnRead() calls into JS land which can result in - // the SSL context object being destroyed. We have to carefully - // check that ssl_ != nullptr afterwards. - if (ssl_ == nullptr) { - Debug(this, "Returning from read loop, ssl_ == nullptr"); + int length = 0; + for (;;) { + pending_cleartext_output_ = {out, static_cast(length)}; + { + SSLLibraryCallScope ssl_library_call_scope(this); + read = SSL_read(ssl_.get(), out + length, kClearOutChunkSize - length); + } + if (destroy_pending_) { + pending_cleartext_output_ = {}; return; } - - read -= avail; - current += avail; + // A protocol callback may have delivered the preceding bytes. The + // current read still wrote after that prefix, so move its output back. + if (length > 0 && pending_cleartext_output_.empty()) { + if (read > 0) memmove(out, out + length, read); + length = 0; + } + pending_cleartext_output_ = {}; + Debug(this, "Read %d bytes of cleartext output", read); + + if (read <= 0) break; + length += read; + if (length == kClearOutChunkSize) break; + if (SSL_pending(ssl_.get()) == 0) { + size_t pending; + const char* next = NodeBIO::FromBIO(enc_in_)->Peek(&pending); + // Protocol records can invoke JS callbacks, such as TLS 1.2 + // renegotiation. Deliver preceding data before processing them. + if (pending == 0 || + static_cast(next[0]) != SSL3_RT_APPLICATION_DATA) { + break; + } + } } + + if (length == 0) break; + + // An SSL_read() failure that ended this batch has to be examined before + // calling into JS (see below), and is reported after the data it follows. + HandleScope handle_scope(env()->isolate()); + int err = 0; + Local error; + const bool failed = read <= 0 && GetClearOutError(read, &err, &error); + + if (!EmitClearOut({out, static_cast(length)})) return; + + if (failed) EmitClearOutError(err, error); + if (read <= 0) return; } // We need to check whether an error occurred or the connection was @@ -922,76 +948,108 @@ void TLSWrap::ClearOut() { // altogether. if (read <= 0) { HandleScope handle_scope(env()->isolate()); + int err; Local error; - int err = SSL_get_error(ssl_.get(), read); - switch (err) { - case SSL_ERROR_ZERO_RETURN: - if (!flags_.eof) { - flags_.eof = true; - EmitRead(UV_EOF); - } - return; - - case SSL_ERROR_SSL: - case SSL_ERROR_SYSCALL: - { - unsigned long ssl_err = ERR_peek_error(); // NOLINT(runtime/int) - - Local context = env()->isolate()->GetCurrentContext(); - if (context.IsEmpty()) [[unlikely]] - return; - const std::string error_str = GetBIOError(); - Local message = OneByteString(env()->isolate(), error_str); - if (message.IsEmpty()) [[unlikely]] - return; - error = Exception::Error(message); - if (error.IsEmpty()) [[unlikely]] - return; - Local obj; - if (!error->ToObject(context).ToLocal(&obj)) [[unlikely]] - return; - - const char* ls = ERR_lib_error_string(ssl_err); + if (GetClearOutError(read, &err, &error)) EmitClearOutError(err, error); + } +} + +bool TLSWrap::EmitClearOut(std::span output) { + while (!output.empty()) { + int avail = output.size(); + uv_buf_t buf = EmitAlloc(avail); + if (static_cast(buf.len) < avail) avail = buf.len; + memcpy(buf.base, output.data(), avail); + EmitRead(avail, buf); + // OnRead() calls into JS, which can destroy the SSL context. + if (ssl_ == nullptr || destroy_pending_) return false; + output = output.subspan(avail); + } + return true; +} + +bool TLSWrap::FlushPendingClearOut() { + const auto output = pending_cleartext_output_; + pending_cleartext_output_ = {}; + return !destroy_pending_ && EmitClearOut(output); +} + +bool TLSWrap::GetClearOutError(int read, int* err, Local* error) { + *err = SSL_get_error(ssl_.get(), read); + switch (*err) { + case SSL_ERROR_ZERO_RETURN: + return true; + + case SSL_ERROR_SSL: + case SSL_ERROR_SYSCALL: { + unsigned long ssl_err = ERR_peek_error(); // NOLINT(runtime/int) + + Local context = env()->isolate()->GetCurrentContext(); + if (context.IsEmpty()) [[unlikely]] + return false; + const std::string error_str = GetBIOError(); + Local message = OneByteString(env()->isolate(), error_str); + if (message.IsEmpty()) [[unlikely]] + return false; + *error = Exception::Error(message); + if (error->IsEmpty()) [[unlikely]] + return false; + Local obj; + if (!(*error)->ToObject(context).ToLocal(&obj)) [[unlikely]] + return false; + + const char* ls = ERR_lib_error_string(ssl_err); #if NCRYPTO_USE_OPENSSL3_PROVIDER - const char* fs = nullptr; + const char* fs = nullptr; #else - const char* fs = ERR_func_error_string(ssl_err); + const char* fs = ERR_func_error_string(ssl_err); #endif - const char* rs = ERR_reason_error_string(ssl_err); - if (!Set(env(), obj, env()->library_string(), ls) || - !Set(env(), obj, env()->function_string(), fs) || - !Set(env(), obj, env()->reason_string(), rs, false)) return; - // SSL has no API to recover the error name from the number, so we - // transform reason strings like "this error" to "ERR_SSL_THIS_ERROR", - // which ends up being close to the original error macro name. - std::string code(rs); - // TODO(RaisinTen): Pass an appropriate execution policy when it is - // implemented in our supported compilers. - std::transform(code.begin(), code.end(), code.begin(), - [](char c) { return c == ' ' ? '_' : ToUpper(c); }); - if (!Set(env(), obj, - env()->code_string(), ("ERR_SSL_" + code).c_str())) return; - } - break; - - default: - return; + const char* rs = ERR_reason_error_string(ssl_err); + if (!Set(env(), obj, env()->library_string(), ls) || + !Set(env(), obj, env()->function_string(), fs) || + !Set(env(), obj, env()->reason_string(), rs, false)) + return false; + // SSL has no API to recover the error name from the number, so we + // transform reason strings like "this error" to "ERR_SSL_THIS_ERROR", + // which ends up being close to the original error macro name. + std::string code(rs); + // TODO(RaisinTen): Pass an appropriate execution policy when it is + // implemented in our supported compilers. + std::transform(code.begin(), code.end(), code.begin(), [](char c) { + return c == ' ' ? '_' : ToUpper(c); + }); + if (!Set(env(), obj, env()->code_string(), ("ERR_SSL_" + code).c_str())) { + return false; + } } + return true; - Debug(this, "Got SSL error (%d), calling onerror", err); - // When TLS Alert are stored in wbio, - // it should be flushed to socket before destroyed. - if (BIO_pending(enc_out_) != 0) - EncOut(); + default: + return false; + } +} - MakeCallback(env()->onerror_string(), 1, &error); +void TLSWrap::EmitClearOutError(int err, Local error) { + if (err == SSL_ERROR_ZERO_RETURN) { + if (!flags_.eof) { + flags_.eof = true; + EmitRead(UV_EOF); + } + return; } + + Debug(this, "Got SSL error (%d), calling onerror", err); + // When TLS Alert are stored in wbio, + // it should be flushed to socket before destroyed. + if (BIO_pending(enc_out_) != 0) EncOut(); + + MakeCallback(env()->onerror_string(), 1, &error); } void TLSWrap::ClearIn() { Debug(this, "Trying to write cleartext input"); - if (ssl_ == nullptr) { + if (ssl_ == nullptr || destroy_pending_) { Debug(this, "Returning from ClearIn(), ssl_ == nullptr"); return; } @@ -1440,6 +1498,15 @@ void TLSWrap::DestroySSL(const FunctionCallbackInfo& args) { } void TLSWrap::Destroy() { + if (in_ssl_library_call()) { + if (!destroy_pending_) { + destroy_pending_ = true; + BaseObjectPtr strong_ref{this}; + env()->SetImmediate([this, strong_ref](Environment* env) { Destroy(); }); + } + return; + } + destroy_pending_ = false; if (!ssl_) return; diff --git a/src/crypto/crypto_tls.h b/src/crypto/crypto_tls.h index ab6024018873..fc143f5f94df 100644 --- a/src/crypto/crypto_tls.h +++ b/src/crypto/crypto_tls.h @@ -32,6 +32,7 @@ #include +#include #include #include @@ -82,6 +83,7 @@ class TLSWrap : public AsyncWrap, inline bool is_cert_cb_running() const { return flags_.cert_cb_running; } inline bool is_waiting_cert_cb() const { return cert_cb_ != nullptr; } inline bool has_session_callbacks() const { return flags_.session_callbacks; } + bool FlushPendingClearOut(); // We need to suspend the ClientHello only for server session id // callbacks, and only on the first pass. inline bool should_suspend_for_client_hello() const { @@ -162,7 +164,7 @@ class TLSWrap : public AsyncWrap, // NOTE: Actually it is much more than this static constexpr int64_t kExternalSize = 6224 + 1040 + 42 * 1024; - static constexpr int kClearOutChunkSize = 16384; + static constexpr int kClearOutChunkSize = 64 * 1024; // Usual ServerHello + Certificate size static constexpr int kInitialClientBufferLength = 4096; @@ -201,6 +203,12 @@ class TLSWrap : public AsyncWrap, void EncOut(); // Write encrypted data from enc_out_ to underlying stream. void ClearIn(); // SSL_write() clear data "in" to SSL. void ClearOut(); // SSL_read() clear text "out" from SSL. + bool EmitClearOut(std::span output); + // Reads the outcome of an SSL_read() that returned `read` <= 0. Must be + // called right after it, before calling into JS. Returns false if there + // is nothing to report. + bool GetClearOutError(int read, int* err, v8::Local* error); + void EmitClearOutError(int err, v8::Local error); void Destroy(); // Call Done() on outstanding WriteWrap request. @@ -320,6 +328,9 @@ class TLSWrap : public AsyncWrap, BIO* enc_out_ = nullptr; // SSL_write()/handshake fills this for EncOut(). // Waiting for ClearIn() to pass to SSL_write(). std::unique_ptr pending_cleartext_input_; + // References ClearOut's stack buffer only while SSL_read() is on the stack. + std::span pending_cleartext_output_; + bool destroy_pending_ = false; size_t write_size_ = 0; BaseObjectPtr current_write_; BaseObjectPtr current_empty_write_; diff --git a/test/parallel/test-tls-coalesce-keyupdate.js b/test/parallel/test-tls-coalesce-keyupdate.js new file mode 100644 index 000000000000..b997419b2070 --- /dev/null +++ b/test/parallel/test-tls-coalesce-keyupdate.js @@ -0,0 +1,119 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const { opensslCli } = require('../common/crypto'); +if (!opensslCli) + common.skip('missing OpenSSL CLI'); + +const assert = require('assert'); +const { spawn } = require('child_process'); +const net = require('net'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); +const destroy = process.argv[2] === 'destroy'; +if (!destroy) { + const child = spawn(process.execPath, [__filename, 'destroy'], { + stdio: 'inherit', + }); + child.on('exit', common.mustCall((code) => assert.strictEqual(code, 0))); +} + +// TLS 1.3 control records have the same outer type as application records. +// Deliver data, KeyUpdate and more data together, and check callback ordering. +const before = 'before-key-update\n'; +const after = 'after-key-update\n'; +let received = ''; +let holding = false; +let client; + +const onUpdate = destroy ? common.mustNotCall() : common.mustCall(() => { + assert.strictEqual(received, before); +}); +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + minVersion: 'TLSv1.3', +}, common.mustCall((socket) => { + holding = true; + socket.on('data', common.mustCallAtLeast((data) => { + received += data; + if (received === before) { + if (destroy) { + socket._destroySSL(); + socket.destroy(); + server.close(); + proxy.close(); + return; + } + socket.write('ack'); + } + if (received === before + after) { + socket.end(); + server.close(); + proxy.close(); + } + })); + socket.on('close', common.mustCall(() => { + assert.strictEqual(received, destroy ? before : before + after); + })); + socket.on('error', common.mustNotCall()); + client.stdin.write(before); +})); +server.on('keylog', (line) => { + if (/^CLIENT_TRAFFIC_SECRET_(?:N|1) /.test(line.toString())) onUpdate(); +}); + +const proxy = net.createServer(common.mustCall((socket) => { + const upstream = net.connect({ + port: server.address().port, + allowHalfOpen: true, + }); + upstream.pipe(socket); + socket.on('end', () => upstream.end()); + socket.on('close', () => upstream.destroy()); + const held = []; + let pending = Buffer.alloc(0); + let records = 0; + socket.on('data', common.mustCallAtLeast((data) => { + if (!holding) { + upstream.write(data); + return; + } + held.push(data); + pending = Buffer.concat([pending, data]); + while (pending.length >= 5) { + const length = pending.readUInt16BE(3); + if (pending.length < 5 + length) break; + assert.strictEqual(pending[0], 23); + pending = pending.subarray(5 + length); + records++; + if (records === 1) client.stdin.write('k\n'); + if (records === 2) client.stdin.write(after); + if (records === 3) { + holding = false; + upstream.write(Buffer.concat(held)); + return; + } + } + })); +})); + +server.listen(0, common.mustCall(() => { + proxy.listen(0, common.mustCall(() => { + client = spawn(opensslCli, [ + 's_client', '-tls1_3', '-connect', `127.0.0.1:${proxy.address().port}`, + '-CAfile', fixtures.path('keys', 'ca1-cert.pem'), '-servername', 'agent1', + ]); + client.stdout.resume(); + client.stderr.resume(); + client.on('error', common.mustNotCall()); + client.on('exit', common.mustCall((code) => { + if (destroy) { + assert(code === 0 || code === 1); + } else { + assert.strictEqual(code, 0); + } + })); + })); +})); diff --git a/test/parallel/test-tls-coalesce-records.js b/test/parallel/test-tls-coalesce-records.js new file mode 100644 index 000000000000..eb8b49b87c09 --- /dev/null +++ b/test/parallel/test-tls-coalesce-records.js @@ -0,0 +1,125 @@ +'use strict'; +// TLS records that are already buffered are emitted as one chunk rather than +// one chunk per record. A clean or failed end of the stream that arrives with +// them must still be reported after the data that precedes it. + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const net = require('net'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +const RECORD_SIZE = 512; +const RECORDS = 256; +const payload = Buffer.alloc(RECORD_SIZE * RECORDS); +for (let i = 0; i < RECORDS; i++) + payload.fill(i & 0xff, i * RECORD_SIZE, (i + 1) * RECORD_SIZE); + +const serverOptions = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), +}; +const clientOptions = { + ca: fixtures.readKey('ca1-cert.pem'), + servername: 'agent1', +}; + +// Writes the payload as small records in one go, so that they reach the client +// together, followed by close_notify. +function sendPayload(socket) { + assert(socket.setMaxSendFragment(RECORD_SIZE)); + socket.end(payload); +} + +// Buffered records are coalesced, and 'end' follows all of the data. +{ + const server = tls.createServer(serverOptions, common.mustCall(sendPayload)); + server.listen(0, common.mustCall(() => { + const chunks = []; + const client = tls.connect({ + ...clientOptions, + port: server.address().port, + }); + client.on('data', (chunk) => chunks.push(chunk)); + client.on('end', common.mustCall(() => { + assert.deepStrictEqual(Buffer.concat(chunks), payload); + assert(chunks.length < RECORDS, + `${chunks.length} chunks for ${RECORDS} records`); + assert(chunks.every((chunk) => chunk.length <= 64 * 1024)); + server.close(); + })); + })); +} + +// A listener buffer smaller than a record still receives every byte. +{ + const server = tls.createServer(serverOptions, common.mustCall(sendPayload)); + server.listen(0, common.mustCall(() => { + const received = []; + const client = tls.connect({ + ...clientOptions, + port: server.address().port, + onread: { + buffer: Buffer.alloc(100), + callback(nread, buf) { + received.push(Buffer.from(buf.subarray(0, nread))); + }, + }, + }); + client.on('end', common.mustCall(() => { + assert.deepStrictEqual(Buffer.concat(received), payload); + server.close(); + })); + })); +} + +// A TLS error that arrives together with data is reported after that data. +{ + const server = tls.createServer(serverOptions, common.mustCall((socket) => { + socket.once('data', common.mustCall(() => sendPayload(socket))); + })); + server.listen(0, common.mustCall(() => { + // Forwards the handshake, then holds the response until the server ends + // and delivers it at once with its last record (the encrypted + // close_notify) corrupted. + let holding = false; + const held = []; + const proxy = net.createServer(common.mustCall((socket) => { + const upstream = net.connect(server.address().port); + socket.pipe(upstream); + socket.on('error', () => {}); + upstream.on('data', (chunk) => { + if (holding) + held.push(chunk); + else + socket.write(chunk); + }); + upstream.on('end', common.mustCall(() => { + const data = Buffer.concat(held); + data[data.length - 1] ^= 1; + socket.end(data); + })); + })); + proxy.listen(0, common.mustCall(() => { + const chunks = []; + const client = tls.connect({ + ...clientOptions, + port: proxy.address().port, + }, common.mustCall(() => { + holding = true; + client.write('go'); + })); + client.on('data', (chunk) => chunks.push(chunk)); + client.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, + 'ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC'); + assert.deepStrictEqual(Buffer.concat(chunks), payload); + proxy.close(); + server.close(); + })); + })); + })); +} diff --git a/test/parallel/test-tls-coalesce-renegotiation.js b/test/parallel/test-tls-coalesce-renegotiation.js new file mode 100644 index 000000000000..1fd9326a88be --- /dev/null +++ b/test/parallel/test-tls-coalesce-renegotiation.js @@ -0,0 +1,82 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const { isBoringSSL } = require('../common/crypto'); +if (isBoringSSL) + common.skip('BoringSSL does not support renegotiation'); + +const assert = require('assert'); +const net = require('net'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +// Deliver application data and a TLS 1.2 ClientHello in one transport write. +// The data handler must run before renegotiation starts, so it can disable it. +const payload = Buffer.from('disable renegotiation'); +let holding = false; +let client; + +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + maxVersion: 'TLSv1.2', +}, common.mustCall((socket) => { + socket.on('data', common.mustCall((data) => { + assert.deepStrictEqual(data, payload); + socket.disableRenegotiation(); + })); + socket.on('secure', common.mustNotCall('renegotiation completed')); + socket.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_TLS_RENEGOTIATION_DISABLED'); + client.destroy(); + socket.destroy(); + server.close(); + proxy.close(); + })); +})); + +const proxy = net.createServer(common.mustCall((socket) => { + const upstream = net.connect(server.address().port); + upstream.pipe(socket); + socket.on('end', () => upstream.end()); + socket.on('close', () => upstream.destroy()); + const held = []; + let pending = Buffer.alloc(0); + socket.on('data', (data) => { + if (!holding) { + upstream.write(data); + return; + } + held.push(data); + pending = Buffer.concat([pending, data]); + while (pending.length >= 5) { + const length = pending.readUInt16BE(3); + if (pending.length < 5 + length) + break; + if (pending[0] === 22) { // handshake + holding = false; + upstream.write(Buffer.concat(held)); + return; + } + pending = pending.subarray(5 + length); + } + }); +})); + +server.listen(0, common.mustCall(() => { + proxy.listen(0, common.mustCall(() => { + client = tls.connect({ + port: proxy.address().port, + ca: fixtures.readKey('ca1-cert.pem'), + servername: 'agent1', + maxVersion: 'TLSv1.2', + }, common.mustCall(() => { + holding = true; + client.write(payload, common.mustCall(() => { + assert(client.renegotiate({}, + common.mustNotCall('renegotiation completed'))); + })); + })); + })); +})); diff --git a/test/parallel/test-tls-max-send-fragment.js b/test/parallel/test-tls-max-send-fragment.js index d389cf50d3d7..1dc34c53481e 100644 --- a/test/parallel/test-tls-max-send-fragment.js +++ b/test/parallel/test-tls-max-send-fragment.js @@ -29,20 +29,26 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('assert'); +const net = require('net'); const tls = require('tls'); const buf = Buffer.allocUnsafe(10000); let received = 0; const maxChunk = 768; +// Largest application_data record the server sent, as seen on the wire. +let maxRecord = 0; const invalidArgumentError = { name: 'TypeError', code: 'ERR_INVALID_ARG_TYPE' }; +// TLS 1.2, so that application_data records are not confused with the +// encrypted handshake records of TLS 1.3. const server = tls.createServer({ key: fixtures.readKey('agent1-key.pem'), - cert: fixtures.readKey('agent1-cert.pem') + cert: fixtures.readKey('agent1-cert.pem'), + maxVersion: 'TLSv1.2', }, common.mustCall((c) => { // No size is passed. @@ -76,20 +82,46 @@ const server = tls.createServer({ assert(c.setMaxSendFragment(maxChunk)); c.end(buf); -})).listen(0, common.mustCall(function() { - const c = tls.connect(this.address().port, { - rejectUnauthorized: false - }, common.mustCall(function() { - c.on('data', common.mustCallAtLeast((chunk) => { - assert(chunk.length <= maxChunk); - received += chunk.length; - })); +})).listen(0, common.mustCall(() => { + // Records that arrive together are emitted as one 'data' chunk, so the + // fragment size is checked on the wire: a proxy parses the TLS record + // headers of everything the server sends. + const proxy = net.createServer(common.mustCall((socket) => { + const upstream = net.connect(server.address().port); + socket.pipe(upstream); + let pending = Buffer.alloc(0); + upstream.on('data', (data) => { + socket.write(data); + pending = Buffer.concat([pending, data]); + while (pending.length >= 5) { + const length = pending.readUInt16BE(3); + if (pending.length < 5 + length) + break; + if (pending[0] === 23) // application_data + maxRecord = Math.max(maxRecord, length); + pending = pending.subarray(5 + length); + } + }); + upstream.on('end', () => socket.end()); + })).listen(0, common.mustCall(() => { + const c = tls.connect(proxy.address().port, { + rejectUnauthorized: false + }, common.mustCall(() => { + c.on('data', common.mustCallAtLeast((chunk) => { + received += chunk.length; + })); - // Ensure that we receive 'end' event anyway - c.on('end', common.mustCall(function() { - c.destroy(); - server.close(); - assert.strictEqual(received, buf.length); + // Ensure that we receive 'end' event anyway + c.on('end', common.mustCall(() => { + c.destroy(); + proxy.close(); + server.close(); + assert.strictEqual(received, buf.length); + // A record carries at most maxChunk bytes of data plus the cipher's + // overhead (IV, MAC or tag, padding). + assert(maxRecord > 0 && maxRecord <= maxChunk + 256, + `largest record ${maxRecord}`); + })); })); })); }));