From dda22a520b8f9247938acd9d791d28bc1da8b450 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sun, 21 Jul 2013 23:16:26 +0700 Subject: [PATCH] tls_wrap: parse tls session ticket extension And, if present and non-empty, don't invoke `resumeSession` callback. fix #5872 --- lib/_tls_wrap.js | 1 + src/tls_wrap.cc | 77 ++++++++++++++++++++++++--- test/simple/test-tls-session-cache.js | 34 ++++++++---- 3 files changed, 95 insertions(+), 17 deletions(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 2591e97f66..65c0fd94cf 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -63,6 +63,7 @@ function onclienthello(hello) { } if (hello.sessionId.length <= 0 || + hello.tlsTicket || this.server && !this.server.emit('resumeSession', hello.sessionId, callback)) { callback(null, null); diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index b5c775aa17..6261ec83c3 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -30,6 +30,7 @@ namespace node { using crypto::SecureContext; using v8::Array; +using v8::Boolean; using v8::Exception; using v8::Function; using v8::FunctionCallbackInfo; @@ -63,6 +64,7 @@ static Cached name_sym; static Cached version_sym; static Cached ext_key_usage_sym; static Cached sessionid_sym; +static Cached tls_ticket_sym; static Persistent tlsWrap; @@ -682,6 +684,8 @@ void TLSCallbacks::ParseClientHello() { bool is_clienthello = false; uint8_t session_size = -1; uint8_t* session_id = NULL; + uint16_t tls_ticket_size = -1; + uint8_t* tls_ticket = NULL; Local hello_obj; Handle argv[1]; @@ -739,6 +743,60 @@ void TLSCallbacks::ParseClientHello() { session_size = *body; session_id = body + 1; } + + size_t cipher_offset = session_offset + 1 + session_size; + + // Session OOB failure + if (cipher_offset + 1 >= avail) + return ParseFinish(); + + uint16_t cipher_len = + (data[cipher_offset] << 8) + data[cipher_offset + 1]; + size_t comp_offset = cipher_offset + 2 + cipher_len; + + // Cipher OOB failure + if (comp_offset >= avail) + return ParseFinish(); + + uint8_t comp_len = data[comp_offset]; + size_t extension_offset = comp_offset + 1 + comp_len; + + // Compression OOB failure + if (extension_offset > avail) + return ParseFinish(); + + // Extensions present + if (extension_offset != avail) { + size_t ext_off = extension_offset + 2; + + // Parse known extensions + while (ext_off < avail) { + // Extension OOB + if (avail - ext_off < 4) + return ParseFinish(); + + uint16_t ext_type = (data[ext_off] << 8) + data[ext_off + 1]; + uint16_t ext_len = (data[ext_off + 2] << 8) + data[ext_off + 3]; + + // Extension OOB + if (ext_off + ext_len + 4 > avail) + return ParseFinish(); + + ext_off += 4; + + // TLS Session Ticket + if (ext_type == 35) { + tls_ticket_size = ext_len; + tls_ticket = data + ext_off; + } + + ext_off += ext_len; + } + + // Extensions OOB failure + if (ext_off > avail) + return ParseFinish(); + } } else if (hello_.state == kParseSSLHeader) { // Skip header, version session_offset = hello_.body_offset + 3; @@ -773,13 +831,17 @@ void TLSCallbacks::ParseClientHello() { return ParseFinish(); hello_.state = kParsePaused; - hello_obj = Object::New(); - hello_obj->Set(sessionid_sym, - Buffer::New(reinterpret_cast(session_id), - session_size)); - - argv[0] = hello_obj; - MakeCallback(object(), onclienthello_sym, 1, argv); + { + hello_obj = Object::New(); + hello_obj->Set(sessionid_sym, + Buffer::New(reinterpret_cast(session_id), + session_size)); + bool have_tls_ticket = (tls_ticket != NULL && tls_ticket_size != 0); + hello_obj->Set(tls_ticket_sym, Boolean::New(have_tls_ticket)); + + argv[0] = hello_obj; + MakeCallback(object(), onclienthello_sym, 1, argv); + } break; case kParseEnded: default: @@ -1364,6 +1426,7 @@ void TLSCallbacks::Initialize(Handle target) { version_sym = String::New("version"); ext_key_usage_sym = String::New("ext_key_usage"); sessionid_sym = String::New("sessionId"); + tls_ticket_sym = String::New("tlsTicket"); } } // namespace node diff --git a/test/simple/test-tls-session-cache.js b/test/simple/test-tls-session-cache.js index fdc4ae17dd..5a5f160dc3 100644 --- a/test/simple/test-tls-session-cache.js +++ b/test/simple/test-tls-session-cache.js @@ -28,10 +28,14 @@ require('child_process').exec('openssl version', function(err) { console.error('Skipping because openssl command is not available.'); process.exit(0); } - doTest(); + doTest({ tickets: false } , function() { + doTest({ tickets: true } , function() { + console.error('all done'); + }); + }); }); -function doTest() { +function doTest(testOptions, callback) { var common = require('../common'); var assert = require('assert'); var tls = require('tls'); @@ -50,6 +54,7 @@ function doTest() { requestCert: true }; var requestCount = 0; + var resumeCount = 0; var session; var badOpenSSL = false; @@ -72,6 +77,7 @@ function doTest() { }; }); server.on('resumeSession', function(id, callback) { + ++resumeCount; assert.ok(session); assert.equal(session.id.toString('hex'), id.toString('hex')); @@ -83,12 +89,12 @@ function doTest() { server.listen(common.PORT, function() { var client = spawn('openssl', [ 's_client', + '-tls1', '-connect', 'localhost:' + common.PORT, '-key', join(common.fixturesDir, 'agent.key'), '-cert', join(common.fixturesDir, 'agent.crt'), - '-reconnect', - '-no_ticket' - ], { + '-reconnect' + ].concat(testOptions.tickets ? [] : '-no_ticket'), { stdio: [ 0, 1, 'pipe' ] }); var err = ''; @@ -97,22 +103,30 @@ function doTest() { err += chunk; }); client.on('exit', function(code) { + console.error('done'); if (/^unknown option/.test(err)) { // using an incompatible version of openssl assert(code); badOpenSSL = true; } else assert.equal(code, 0); - server.close(); + server.close(function() { + setTimeout(callback, 100); + }); }); }); process.on('exit', function() { if (!badOpenSSL) { - assert.ok(session); - - // initial request + reconnect requests (5 times) - assert.equal(requestCount, 6); + if (testOptions.tickets) { + assert.equal(requestCount, 6); + assert.equal(resumeCount, 0); + } else { + // initial request + reconnect requests (5 times) + assert.ok(session); + assert.equal(requestCount, 6); + assert.equal(resumeCount, 5); + } } }); }