From 027b16d4a8362423e07f4b2dbcc3562f99a560d0 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 17 Jan 2014 18:46:49 +0000 Subject: [PATCH 1/4] lib: introduce `.setMaxSendFragment(size)` fix #6889 --- doc/api/tls.markdown | 4 ++ lib/_tls_wrap.js | 4 ++ src/node_crypto.cc | 19 +++++++ src/node_crypto.h | 5 ++ test/simple/test-tls-max-send-fragment.js | 66 +++++++++++++++++++++++ 5 files changed, 98 insertions(+) create mode 100644 test/simple/test-tls-max-send-fragment.js diff --git a/doc/api/tls.markdown b/doc/api/tls.markdown index a0fbe9a123c5..54d7c95a5fd6 100644 --- a/doc/api/tls.markdown +++ b/doc/api/tls.markdown @@ -627,6 +627,10 @@ has been established. ANOTHER NOTE: When running as the server, socket will be destroyed with an error after `handshakeTimeout` timeout. +### tlsSocket.setMaxSendFragment(size) + +Set maximum TLS fragment size (default value is: `16384`). + ### tlsSocket.address() Returns the bound address, the address family name and port of the diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 861508b9de0c..2ffda6fa882e 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -303,6 +303,10 @@ TLSSocket.prototype.renegotiate = function(options, callback) { return true; }; +TLSSocket.prototype.setMaxSendFragment = function setMaxSendFragment(size) { + return this.ssl.setMaxSendFragment(size) == 1; +}; + TLSSocket.prototype._handleTimeout = function() { this._tlsError(new Error('TLS handshake timeout')); }; diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 6b6beb40bb9d..eca153410599 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -856,6 +856,10 @@ void SSLWrap::AddMethods(Handle t) { NODE_SET_PROTOTYPE_METHOD(t, "renegotiate", Renegotiate); NODE_SET_PROTOTYPE_METHOD(t, "shutdown", Shutdown); +#ifdef SSL_set_max_send_fragment + NODE_SET_PROTOTYPE_METHOD(t, "setMaxSendFragment", SetMaxSendFragment); +#endif // SSL_set_max_send_fragment + #ifdef OPENSSL_NPN_NEGOTIATED NODE_SET_PROTOTYPE_METHOD(t, "getNegotiatedProtocol", GetNegotiatedProto); NODE_SET_PROTOTYPE_METHOD(t, "setNPNProtocols", SetNPNProtocols); @@ -1239,6 +1243,21 @@ void SSLWrap::Shutdown(const FunctionCallbackInfo& args) { } +#ifdef SSL_set_max_send_fragment +template +void SSLWrap::SetMaxSendFragment( + const v8::FunctionCallbackInfo& args) { + HandleScope scope(node_isolate); + CHECK(args.Length() >= 1 && args[0]->IsNumber()); + + Base* w = Unwrap(args.This()); + + int rv = SSL_set_max_send_fragment(w->ssl_, args[0]->Int32Value()); + args.GetReturnValue().Set(rv); +} +#endif // SSL_set_max_send_fragment + + template void SSLWrap::IsInitFinished(const FunctionCallbackInfo& args) { HandleScope scope(node_isolate); diff --git a/src/node_crypto.h b/src/node_crypto.h index 2357ca4a2d87..7f29e8959046 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -188,6 +188,11 @@ class SSLWrap { static void Renegotiate(const v8::FunctionCallbackInfo& args); static void Shutdown(const v8::FunctionCallbackInfo& args); +#ifdef SSL_set_max_send_fragment + static void SetMaxSendFragment( + const v8::FunctionCallbackInfo& args); +#endif // SSL_set_max_send_fragment + #ifdef OPENSSL_NPN_NEGOTIATED static void GetNegotiatedProto( const v8::FunctionCallbackInfo& args); diff --git a/test/simple/test-tls-max-send-fragment.js b/test/simple/test-tls-max-send-fragment.js new file mode 100644 index 000000000000..354d42c61a39 --- /dev/null +++ b/test/simple/test-tls-max-send-fragment.js @@ -0,0 +1,66 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +if (!process.versions.openssl) { + console.error('Skipping because node compiled without OpenSSL.'); + process.exit(0); +} + +var assert = require('assert'); +var fs = require('fs'); +var net = require('net'); +var tls = require('tls'); + +var common = require('../common'); + +var buf = new Buffer(10000); +var received = 0; +var ended = 0; +var maxChunk = 768; + +var server = tls.createServer({ + key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), + cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem') +}, function(c) { + assert(c.setMaxSendFragment(maxChunk)); + c.end(buf); +}).listen(common.PORT, function() { + var c = tls.connect(common.PORT, { + rejectUnauthorized: false + }, function() { + c.on('data', function(chunk) { + assert(chunk.length <= maxChunk); + received += chunk.length; + }); + + // Ensure that we receive 'end' event anyway + c.on('end', function() { + ended++; + c.destroy(); + server.close(); + }); + }); +}); + +process.on('exit', function() { + assert.equal(ended, 1); + assert.equal(received, buf.length); +}); From 9687f321a1175bd6a06eaddaf5200c7d78ec0130 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sat, 18 Jan 2014 23:08:34 +0000 Subject: [PATCH 2/4] doc: fixes --- doc/api/tls.markdown | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/doc/api/tls.markdown b/doc/api/tls.markdown index 54d7c95a5fd6..6800b84e1213 100644 --- a/doc/api/tls.markdown +++ b/doc/api/tls.markdown @@ -629,7 +629,15 @@ with an error after `handshakeTimeout` timeout. ### tlsSocket.setMaxSendFragment(size) -Set maximum TLS fragment size (default value is: `16384`). +Set maximum TLS fragment size (default and maximum value is: `16384`, minimum +is: `512`). Returns `true` on success, `false` otherwise. + +Smaller fragment size decreases buffering latency on the client: large +fragments are buffered by the TLS layer until the entire fragment is received +and its integrity is verified; large records can span multiple roundtrips, and +their processing can be delayed due to packet loss or reordering. However, +smaller record size adds extra TLS framing bytes and CPU overhead, which may +decrease overall server throughput. ### tlsSocket.address() From 028076eb99b1d449b0e1bf7665ab187f4bfbf73e Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 20 Jan 2014 00:46:36 +0000 Subject: [PATCH 3/4] fixes --- doc/api/tls.markdown | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/tls.markdown b/doc/api/tls.markdown index 6800b84e1213..b0389f4371a5 100644 --- a/doc/api/tls.markdown +++ b/doc/api/tls.markdown @@ -634,9 +634,9 @@ is: `512`). Returns `true` on success, `false` otherwise. Smaller fragment size decreases buffering latency on the client: large fragments are buffered by the TLS layer until the entire fragment is received -and its integrity is verified; large records can span multiple roundtrips, and -their processing can be delayed due to packet loss or reordering. However, -smaller record size adds extra TLS framing bytes and CPU overhead, which may +and its integrity is verified; large fragments can span multiple roundtrips, +and their processing can be delayed due to packet loss or reordering. However, +smaller fragments add extra TLS framing bytes and CPU overhead, which may decrease overall server throughput. ### tlsSocket.address() From dd5a82ca4996222ce6eac31e9360e156d523f409 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 20 Jan 2014 18:36:58 +0400 Subject: [PATCH 4/4] add test coverage --- test/simple/test-tls-max-send-fragment.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/simple/test-tls-max-send-fragment.js b/test/simple/test-tls-max-send-fragment.js index 354d42c61a39..f6fdf2512050 100644 --- a/test/simple/test-tls-max-send-fragment.js +++ b/test/simple/test-tls-max-send-fragment.js @@ -40,7 +40,13 @@ var server = tls.createServer({ key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem') }, function(c) { + // Lower and upper limits + assert(!c.setMaxSendFragment(511)); + assert(!c.setMaxSendFragment(16385)); + + // Correct fragment size assert(c.setMaxSendFragment(maxChunk)); + c.end(buf); }).listen(common.PORT, function() { var c = tls.connect(common.PORT, {