Skip to content

Commit

Permalink
src: add .code and SSL specific error properties
Browse files Browse the repository at this point in the history 
SSL errors have a long structured message, but lacked the standard .code
property which can be used for stable comparisons. Add a `code`
property, as well as the 3 string components of an SSL error: `reason`,
`library`, and `function`.

PR-URL: #25093
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
  • Loading branch information
@sam-github
sam-github committed Dec 27, 2018
1 parent 455bcca commit ca9c0c9
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 30 deletions.
4 changes: 3 additions & 1 deletion src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
V(fingerprint_string, "fingerprint") \
V(flags_string, "flags") \
V(fragment_string, "fragment") \
V(function_string, "function") \
V(get_data_clone_error_string, "_getDataCloneError") \
V(get_shared_array_buffer_id_string, "_getSharedArrayBufferId") \
V(gid_string, "gid") \
Expand All @@ -204,6 +205,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
V(issuercert_string, "issuerCertificate") \
V(kill_signal_string, "killSignal") \
V(kind_string, "kind") \
V(library_string, "library") \
V(mac_string, "mac") \
V(main_string, "main") \
V(max_buffer_string, "maxBuffer") \
Expand Down Expand Up @@ -313,7 +315,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
V(write_host_object_string, "_writeHostObject") \
V(write_queue_size_string, "writeQueueSize") \
V(x_forwarded_string, "x-forwarded-for") \
V(zero_return_string, "ZERO_RETURN")
V(zero_return_string, "ZERO_RETURN") \

#define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) \
V(as_external, v8::External) \
Expand Down
38 changes: 37 additions & 1 deletion src/tls_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ using v8::Exception;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::Isolate;
using v8::Local;
using v8::Object;
using v8::ReadOnly;
Expand Down Expand Up @@ -347,15 +348,50 @@ Local<Value> TLSWrap::GetSSLError(int status, int* err, std::string* msg) {
{
CHECK(*err == SSL_ERROR_SSL || *err == SSL_ERROR_SYSCALL);

unsigned long ssl_err = ERR_peek_error(); // NOLINT(runtime/int)
BIO* bio = BIO_new(BIO_s_mem());
ERR_print_errors(bio);

BUF_MEM* mem;
BIO_get_mem_ptr(bio, &mem);

Isolate* isolate = env()->isolate();
Local<Context> context = isolate->GetCurrentContext();

Local<String> message =
OneByteString(env()->isolate(), mem->data, mem->length);
OneByteString(isolate, mem->data, mem->length);
Local<Value> exception = Exception::Error(message);
Local<Object> obj = exception->ToObject(context).ToLocalChecked();

const char* ls = ERR_lib_error_string(ssl_err);
const char* fs = ERR_func_error_string(ssl_err);
const char* rs = ERR_reason_error_string(ssl_err);

if (ls != nullptr)
obj->Set(context, env()->library_string(),
OneByteString(isolate, ls)).FromJust();
if (fs != nullptr)
obj->Set(context, env()->function_string(),
OneByteString(isolate, fs)).FromJust();
if (rs != nullptr) {
obj->Set(context, env()->reason_string(),
OneByteString(isolate, rs)).FromJust();

// 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);

for (auto& c : code) {
if (c == ' ')
c = '_';
else
c = ::toupper(c);
}
obj->Set(context, env()->code_string(),
OneByteString(isolate, ("ERR_SSL_" + code).c_str()))
.FromJust();
}

if (msg != nullptr)
msg->assign(mem->data, mem->data + mem->length);
Expand Down
14 changes: 12 additions & 2 deletions test/parallel/test-tls-alert-handling.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ if (!common.hasCrypto)
if (!common.opensslCli)
common.skip('node compiled without OpenSSL CLI');

const assert = require('assert');
const net = require('net');
const tls = require('tls');
const fixtures = require('../common/fixtures');
Expand All @@ -29,7 +30,11 @@ const opts = {
const max_iter = 20;
let iter = 0;

const errorHandler = common.mustCall(() => {
const errorHandler = common.mustCall((err) => {
assert.strictEqual(err.code, 'ERR_SSL_WRONG_VERSION_NUMBER');
assert.strictEqual(err.library, 'SSL routines');
assert.strictEqual(err.function, 'ssl3_get_record');
assert.strictEqual(err.reason, 'wrong version number');
errorReceived = true;
if (canCloseServer())
server.close();
Expand Down Expand Up @@ -76,5 +81,10 @@ function sendBADTLSRecord() {
socket.write(BAD_RECORD);
socket.end();
}));
client.on('error', common.mustCall());
client.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ERR_SSL_TLSV1_ALERT_PROTOCOL_VERSION');
assert.strictEqual(err.library, 'SSL routines');
assert.strictEqual(err.function, 'ssl3_read_bytes');
assert.strictEqual(err.reason, 'tlsv1 alert protocol version');
}));
}
49 changes: 23 additions & 26 deletions test/parallel/test-tls-min-max-version.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const {
const DEFAULT_MIN_VERSION = tls.DEFAULT_MIN_VERSION;

function test(cmin, cmax, cprot, smin, smax, sprot, expect) {
assert(expect);
connect({
client: {
checkServerIdentity: (servername, cert) => { },
Expand All @@ -26,23 +27,18 @@ function test(cmin, cmax, cprot, smin, smax, sprot, expect) {
secureProtocol: sprot,
},
}, common.mustCall((err, pair, cleanup) => {
if (expect && expect.match(/^ERR/)) {
assert.strictEqual(err.code, expect);
if (err) {
assert.strictEqual(err.code, expect, err + '.code !== ' + expect);
return cleanup();
}

if (expect) {
assert.ifError(pair.server.err);
assert.ifError(pair.client.err);
assert(pair.server.conn);
assert(pair.client.conn);
assert.strictEqual(pair.client.conn.getProtocol(), expect);
assert.strictEqual(pair.server.conn.getProtocol(), expect);
return cleanup();
}

assert(pair.server.err);
assert(pair.client.err);
assert.ifError(err);
assert.ifError(pair.server.err);
assert.ifError(pair.client.err);
assert(pair.server.conn);
assert(pair.client.conn);
assert.strictEqual(pair.client.conn.getProtocol(), expect);
assert.strictEqual(pair.server.conn.getProtocol(), expect);
return cleanup();
}));
}
Expand Down Expand Up @@ -83,17 +79,18 @@ test(U, U, 'TLS_method', U, U, 'TLSv1_method', 'TLSv1');
test(U, U, 'TLSv1_2_method', U, U, 'SSLv23_method', 'TLSv1.2');

if (DEFAULT_MIN_VERSION === 'TLSv1.2') {
test(U, U, 'TLSv1_1_method', U, U, 'SSLv23_method', null);
test(U, U, 'TLSv1_method', U, U, 'SSLv23_method', null);
test(U, U, 'SSLv23_method', U, U, 'TLSv1_1_method', null);
test(U, U, 'SSLv23_method', U, U, 'TLSv1_method', null);
test(U, U, 'TLSv1_1_method', U, U, 'SSLv23_method', 'ECONNRESET');
test(U, U, 'TLSv1_method', U, U, 'SSLv23_method', 'ECONNRESET');
test(U, U, 'SSLv23_method', U, U, 'TLSv1_1_method',
'ERR_SSL_VERSION_TOO_LOW');
test(U, U, 'SSLv23_method', U, U, 'TLSv1_method', 'ERR_SSL_VERSION_TOO_LOW');
}

if (DEFAULT_MIN_VERSION === 'TLSv1.1') {
test(U, U, 'TLSv1_1_method', U, U, 'SSLv23_method', 'TLSv1.1');
test(U, U, 'TLSv1_method', U, U, 'SSLv23_method', null);
test(U, U, 'TLSv1_method', U, U, 'SSLv23_method', 'ECONNRESET');
test(U, U, 'SSLv23_method', U, U, 'TLSv1_1_method', 'TLSv1.1');
test(U, U, 'SSLv23_method', U, U, 'TLSv1_method', null);
test(U, U, 'SSLv23_method', U, U, 'TLSv1_method', 'ERR_SSL_VERSION_TOO_LOW');
}

if (DEFAULT_MIN_VERSION === 'TLSv1') {
Expand All @@ -111,18 +108,18 @@ test(U, U, 'TLSv1_method', U, U, 'TLSv1_method', 'TLSv1');

// The default default.
if (DEFAULT_MIN_VERSION === 'TLSv1.2') {
test(U, U, 'TLSv1_1_method', U, U, U, null);
test(U, U, 'TLSv1_method', U, U, U, null);
test(U, U, U, U, U, 'TLSv1_1_method', null);
test(U, U, U, U, U, 'TLSv1_method', null);
test(U, U, 'TLSv1_1_method', U, U, U, 'ECONNRESET');
test(U, U, 'TLSv1_method', U, U, U, 'ECONNRESET');
test(U, U, U, U, U, 'TLSv1_1_method', 'ERR_SSL_VERSION_TOO_LOW');
test(U, U, U, U, U, 'TLSv1_method', 'ERR_SSL_VERSION_TOO_LOW');
}

// The default with --tls-v1.1.
if (DEFAULT_MIN_VERSION === 'TLSv1.1') {
test(U, U, 'TLSv1_1_method', U, U, U, 'TLSv1.1');
test(U, U, 'TLSv1_method', U, U, U, null);
test(U, U, 'TLSv1_method', U, U, U, 'ECONNRESET');
test(U, U, U, U, U, 'TLSv1_1_method', 'TLSv1.1');
test(U, U, U, U, U, 'TLSv1_method', null);
test(U, U, U, U, U, 'TLSv1_method', 'ERR_SSL_VERSION_TOO_LOW');
}

// The default with --tls-v1.0.
Expand Down

0 comments on commit ca9c0c9

Please sign in to comment.