Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 345ffbe

Browse files
sam-githubsam-github
committedJan 7, 2019
src: add .code and SSL specific error properties
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`. Backport-PR-URL: https://github.com/nodejs/node/25376 PR-URL: nodejs#25093 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
1 parent f6e341a commit 345ffbe

File tree

4 files changed

+74
-29
lines changed

4 files changed

+74
-29
lines changed
 

Diff for: ‎src/env.h

+2
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
182182
V(fingerprint_string, "fingerprint") \
183183
V(flags_string, "flags") \
184184
V(fragment_string, "fragment") \
185+
V(function_string, "function") \
185186
V(get_data_clone_error_string, "_getDataCloneError") \
186187
V(get_shared_array_buffer_id_string, "_getSharedArrayBufferId") \
187188
V(gid_string, "gid") \
@@ -203,6 +204,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
203204
V(issuercert_string, "issuerCertificate") \
204205
V(kill_signal_string, "killSignal") \
205206
V(kind_string, "kind") \
207+
V(library_string, "library") \
206208
V(mac_string, "mac") \
207209
V(main_string, "main") \
208210
V(max_buffer_string, "maxBuffer") \

Diff for: ‎src/tls_wrap.cc

+37-1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ using v8::Exception;
4141
using v8::Function;
4242
using v8::FunctionCallbackInfo;
4343
using v8::FunctionTemplate;
44+
using v8::Isolate;
4445
using v8::Local;
4546
using v8::Object;
4647
using v8::ReadOnly;
@@ -347,15 +348,50 @@ Local<Value> TLSWrap::GetSSLError(int status, int* err, std::string* msg) {
347348
{
348349
CHECK(*err == SSL_ERROR_SSL || *err == SSL_ERROR_SYSCALL);
349350

351+
unsigned long ssl_err = ERR_peek_error(); // NOLINT(runtime/int)
350352
BIO* bio = BIO_new(BIO_s_mem());
351353
ERR_print_errors(bio);
352354

353355
BUF_MEM* mem;
354356
BIO_get_mem_ptr(bio, &mem);
355357

358+
Isolate* isolate = env()->isolate();
359+
Local<Context> context = isolate->GetCurrentContext();
360+
356361
Local<String> message =
357-
OneByteString(env()->isolate(), mem->data, mem->length);
362+
OneByteString(isolate, mem->data, mem->length);
358363
Local<Value> exception = Exception::Error(message);
364+
Local<Object> obj = exception->ToObject(context).ToLocalChecked();
365+
366+
const char* ls = ERR_lib_error_string(ssl_err);
367+
const char* fs = ERR_func_error_string(ssl_err);
368+
const char* rs = ERR_reason_error_string(ssl_err);
369+
370+
if (ls != nullptr)
371+
obj->Set(context, env()->library_string(),
372+
OneByteString(isolate, ls)).FromJust();
373+
if (fs != nullptr)
374+
obj->Set(context, env()->function_string(),
375+
OneByteString(isolate, fs)).FromJust();
376+
if (rs != nullptr) {
377+
obj->Set(context, env()->reason_string(),
378+
OneByteString(isolate, rs)).FromJust();
379+
380+
// SSL has no API to recover the error name from the number, so we
381+
// transform reason strings like "this error" to "ERR_SSL_THIS_ERROR",
382+
// which ends up being close to the original error macro name.
383+
std::string code(rs);
384+
385+
for (auto& c : code) {
386+
if (c == ' ')
387+
c = '_';
388+
else
389+
c = ::toupper(c);
390+
}
391+
obj->Set(context, env()->code_string(),
392+
OneByteString(isolate, ("ERR_SSL_" + code).c_str()))
393+
.FromJust();
394+
}
359395

360396
if (msg != nullptr)
361397
msg->assign(mem->data, mem->data + mem->length);

Diff for: ‎test/parallel/test-tls-alert-handling.js

+12-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ if (!common.hasCrypto)
77
if (!common.opensslCli)
88
common.skip('node compiled without OpenSSL CLI');
99

10+
const assert = require('assert');
1011
const net = require('net');
1112
const tls = require('tls');
1213
const fixtures = require('../common/fixtures');
@@ -29,7 +30,11 @@ const opts = {
2930
const max_iter = 20;
3031
let iter = 0;
3132

32-
const errorHandler = common.mustCall(() => {
33+
const errorHandler = common.mustCall((err) => {
34+
assert.strictEqual(err.code, 'ERR_SSL_WRONG_VERSION_NUMBER');
35+
assert.strictEqual(err.library, 'SSL routines');
36+
assert.strictEqual(err.function, 'ssl3_get_record');
37+
assert.strictEqual(err.reason, 'wrong version number');
3338
errorReceived = true;
3439
if (canCloseServer())
3540
server.close();
@@ -76,5 +81,10 @@ function sendBADTLSRecord() {
7681
socket.write(BAD_RECORD);
7782
socket.end();
7883
}));
79-
client.on('error', common.mustCall());
84+
client.on('error', common.mustCall((err) => {
85+
assert.strictEqual(err.code, 'ERR_SSL_TLSV1_ALERT_PROTOCOL_VERSION');
86+
assert.strictEqual(err.library, 'SSL routines');
87+
assert.strictEqual(err.function, 'ssl3_read_bytes');
88+
assert.strictEqual(err.reason, 'tlsv1 alert protocol version');
89+
}));
8090
}

Diff for: ‎test/parallel/test-tls-min-max-version.js

+23-26
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const DEFAULT_MIN_VERSION = tls.DEFAULT_MIN_VERSION;
1313
assert.strictEqual(DEFAULT_MIN_VERSION, 'TLSv1');
1414

1515
function test(cmin, cmax, cprot, smin, smax, sprot, expect) {
16+
assert(expect);
1617
connect({
1718
client: {
1819
checkServerIdentity: (servername, cert) => { },
@@ -29,23 +30,18 @@ function test(cmin, cmax, cprot, smin, smax, sprot, expect) {
2930
secureProtocol: sprot,
3031
},
3132
}, common.mustCall((err, pair, cleanup) => {
32-
if (expect && !expect.match(/^TLS/)) {
33-
assert(err.message.match(expect));
33+
if (err) {
34+
assert.strictEqual(err.code, expect, err + '.code !== ' + expect);
3435
return cleanup();
3536
}
3637

37-
if (expect) {
38-
assert.ifError(pair.server.err);
39-
assert.ifError(pair.client.err);
40-
assert(pair.server.conn);
41-
assert(pair.client.conn);
42-
assert.strictEqual(pair.client.conn.getProtocol(), expect);
43-
assert.strictEqual(pair.server.conn.getProtocol(), expect);
44-
return cleanup();
45-
}
46-
47-
assert(pair.server.err);
48-
assert(pair.client.err);
38+
assert.ifError(err);
39+
assert.ifError(pair.server.err);
40+
assert.ifError(pair.client.err);
41+
assert(pair.server.conn);
42+
assert(pair.client.conn);
43+
assert.strictEqual(pair.client.conn.getProtocol(), expect);
44+
assert.strictEqual(pair.server.conn.getProtocol(), expect);
4945
return cleanup();
5046
}));
5147
}
@@ -82,17 +78,18 @@ test(U, U, 'TLS_method', U, U, 'TLSv1_method', 'TLSv1');
8278
test(U, U, 'TLSv1_2_method', U, U, 'SSLv23_method', 'TLSv1.2');
8379

8480
if (DEFAULT_MIN_VERSION === 'TLSv1.2') {
85-
test(U, U, 'TLSv1_1_method', U, U, 'SSLv23_method', null);
86-
test(U, U, 'TLSv1_method', U, U, 'SSLv23_method', null);
87-
test(U, U, 'SSLv23_method', U, U, 'TLSv1_1_method', null);
88-
test(U, U, 'SSLv23_method', U, U, 'TLSv1_method', null);
81+
test(U, U, 'TLSv1_1_method', U, U, 'SSLv23_method', 'ECONNRESET');
82+
test(U, U, 'TLSv1_method', U, U, 'SSLv23_method', 'ECONNRESET');
83+
test(U, U, 'SSLv23_method', U, U, 'TLSv1_1_method',
84+
'ERR_SSL_VERSION_TOO_LOW');
85+
test(U, U, 'SSLv23_method', U, U, 'TLSv1_method', 'ERR_SSL_VERSION_TOO_LOW');
8986
}
9087

9188
if (DEFAULT_MIN_VERSION === 'TLSv1.1') {
9289
test(U, U, 'TLSv1_1_method', U, U, 'SSLv23_method', 'TLSv1.1');
93-
test(U, U, 'TLSv1_method', U, U, 'SSLv23_method', null);
90+
test(U, U, 'TLSv1_method', U, U, 'SSLv23_method', 'ECONNRESET');
9491
test(U, U, 'SSLv23_method', U, U, 'TLSv1_1_method', 'TLSv1.1');
95-
test(U, U, 'SSLv23_method', U, U, 'TLSv1_method', null);
92+
test(U, U, 'SSLv23_method', U, U, 'TLSv1_method', 'ERR_SSL_VERSION_TOO_LOW');
9693
}
9794

9895
if (DEFAULT_MIN_VERSION === 'TLSv1') {
@@ -110,18 +107,18 @@ test(U, U, 'TLSv1_method', U, U, 'TLSv1_method', 'TLSv1');
110107

111108
// The default default.
112109
if (DEFAULT_MIN_VERSION === 'TLSv1.2') {
113-
test(U, U, 'TLSv1_1_method', U, U, U, null);
114-
test(U, U, 'TLSv1_method', U, U, U, null);
115-
test(U, U, U, U, U, 'TLSv1_1_method', null);
116-
test(U, U, U, U, U, 'TLSv1_method', null);
110+
test(U, U, 'TLSv1_1_method', U, U, U, 'ECONNRESET');
111+
test(U, U, 'TLSv1_method', U, U, U, 'ECONNRESET');
112+
test(U, U, U, U, U, 'TLSv1_1_method', 'ERR_SSL_VERSION_TOO_LOW');
113+
test(U, U, U, U, U, 'TLSv1_method', 'ERR_SSL_VERSION_TOO_LOW');
117114
}
118115

119116
// The default with --tls-v1.1.
120117
if (DEFAULT_MIN_VERSION === 'TLSv1.1') {
121118
test(U, U, 'TLSv1_1_method', U, U, U, 'TLSv1.1');
122-
test(U, U, 'TLSv1_method', U, U, U, null);
119+
test(U, U, 'TLSv1_method', U, U, U, 'ECONNRESET');
123120
test(U, U, U, U, U, 'TLSv1_1_method', 'TLSv1.1');
124-
test(U, U, U, U, U, 'TLSv1_method', null);
121+
test(U, U, U, U, U, 'TLSv1_method', 'ERR_SSL_VERSION_TOO_LOW');
125122
}
126123

127124
// The default with --tls-v1.0.

0 commit comments

Comments
 (0)
Please sign in to comment.