Skip to content

Commit bcbd35a

Browse files
sam-githubsam-github
committed
crypto: add openssl specific error properties
Don't force the user to parse the long-style OpenSSL error message, decorate the error with the library, reason, code, function. PR-URL: #26868 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
1 parent 805e614 commit bcbd35a

File tree

11 files changed

+206
-24
lines changed

11 files changed

+206
-24
lines changed

‎doc/api/errors.md‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,6 @@ circumstance of why the error occurred. `Error` objects capture a "stack trace"
186186
detailing the point in the code at which the `Error` was instantiated, and may
187187
provide a text description of the error.
188188

189-
For crypto only, `Error` objects will include the OpenSSL error stack in a
190-
separate property called `opensslErrorStack` if it is available when the error
191-
is thrown.
192-
193189
All errors generated by Node.js, including all System and JavaScript errors,
194190
will either be instances of, or inherit from, the `Error` class.
195191

@@ -589,6 +585,30 @@ program. For a comprehensive list, see the [`errno`(3) man page][].
589585
encountered by [`http`][] or [`net`][] — often a sign that a `socket.end()`
590586
was not properly called.
591587

588+
## OpenSSL Errors
589+
590+
Errors originating in `crypto` or `tls` are of class `Error`, and in addition to
591+
the standard `.code` and `.message` properties, may have some additional
592+
OpenSSL-specific properties.
593+
594+
### error.opensslErrorStack
595+
596+
An array of errors that can give context to where in the OpenSSL library an
597+
error originates from.
598+
599+
### error.function
600+
601+
The OpenSSL function the error originates in.
602+
603+
### error.library
604+
605+
The OpenSSL library the error originates in.
606+
607+
### error.reason
608+
609+
A human-readable string describing the reason for the error.
610+
611+
592612
<a id="nodejs-error-codes"></a>
593613
## Node.js Error Codes
594614

‎src/node_crypto.cc‎

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,113 @@ static int NoPasswordCallback(char* buf, int size, int rwflag, void* u) {
212212
}
213213

214214

215+
// namespace node::crypto::error
216+
namespace error {
217+
void Decorate(Environment* env, Local<Object> obj,
218+
unsigned long err) { // NOLINT(runtime/int)
219+
if (err == 0) return; // No decoration possible.
220+
221+
const char* ls = ERR_lib_error_string(err);
222+
const char* fs = ERR_func_error_string(err);
223+
const char* rs = ERR_reason_error_string(err);
224+
225+
Isolate* isolate = env->isolate();
226+
Local<Context> context = isolate->GetCurrentContext();
227+
228+
if (ls != nullptr) {
229+
if (obj->Set(context, env->library_string(),
230+
OneByteString(isolate, ls)).IsNothing()) {
231+
return;
232+
}
233+
}
234+
if (fs != nullptr) {
235+
if (obj->Set(context, env->function_string(),
236+
OneByteString(isolate, fs)).IsNothing()) {
237+
return;
238+
}
239+
}
240+
if (rs != nullptr) {
241+
if (obj->Set(context, env->reason_string(),
242+
OneByteString(isolate, rs)).IsNothing()) {
243+
return;
244+
}
245+
246+
// SSL has no API to recover the error name from the number, so we
247+
// transform reason strings like "this error" to "ERR_SSL_THIS_ERROR",
248+
// which ends up being close to the original error macro name.
249+
std::string reason(rs);
250+
251+
for (auto& c : reason) {
252+
if (c == ' ')
253+
c = '_';
254+
else
255+
c = ToUpper(c);
256+
}
257+
258+
#define OSSL_ERROR_CODES_MAP(V) \
259+
V(SYS) \
260+
V(BN) \
261+
V(RSA) \
262+
V(DH) \
263+
V(EVP) \
264+
V(BUF) \
265+
V(OBJ) \
266+
V(PEM) \
267+
V(DSA) \
268+
V(X509) \
269+
V(ASN1) \
270+
V(CONF) \
271+
V(CRYPTO) \
272+
V(EC) \
273+
V(SSL) \
274+
V(BIO) \
275+
V(PKCS7) \
276+
V(X509V3) \
277+
V(PKCS12) \
278+
V(RAND) \
279+
V(DSO) \
280+
V(ENGINE) \
281+
V(OCSP) \
282+
V(UI) \
283+
V(COMP) \
284+
V(ECDSA) \
285+
V(ECDH) \
286+
V(OSSL_STORE) \
287+
V(FIPS) \
288+
V(CMS) \
289+
V(TS) \
290+
V(HMAC) \
291+
V(CT) \
292+
V(ASYNC) \
293+
V(KDF) \
294+
V(SM2) \
295+
V(USER) \
296+
297+
#define V(name) case ERR_LIB_##name: lib = #name "_"; break;
298+
const char* lib = "";
299+
const char* prefix = "OSSL_";
300+
switch (ERR_GET_LIB(err)) { OSSL_ERROR_CODES_MAP(V) }
301+
#undef V
302+
#undef OSSL_ERROR_CODES_MAP
303+
// Don't generate codes like "ERR_OSSL_SSL_".
304+
if (lib && strcmp(lib, "SSL_") == 0)
305+
prefix = "";
306+
307+
// All OpenSSL reason strings fit in a single 80-column macro definition,
308+
// all prefix lengths are <= 10, and ERR_OSSL_ is 9, so 128 is more than
309+
// sufficient.
310+
char code[128];
311+
snprintf(code, sizeof(code), "ERR_%s%s%s", prefix, lib, reason.c_str());
312+
313+
if (obj->Set(env->isolate()->GetCurrentContext(),
314+
env->code_string(),
315+
OneByteString(env->isolate(), code)).IsNothing())
316+
return;
317+
}
318+
}
319+
} // namespace error
320+
321+
215322
struct CryptoErrorVector : public std::vector<std::string> {
216323
inline void Capture() {
217324
clear();
@@ -258,6 +365,8 @@ struct CryptoErrorVector : public std::vector<std::string> {
258365

259366
void ThrowCryptoError(Environment* env,
260367
unsigned long err, // NOLINT(runtime/int)
368+
// Default, only used if there is no SSL `err` which can
369+
// be used to create a long-style message string.
261370
const char* message = nullptr) {
262371
char message_buffer[128] = {0};
263372
if (err != 0 || message == nullptr) {
@@ -270,7 +379,11 @@ void ThrowCryptoError(Environment* env,
270379
.ToLocalChecked();
271380
CryptoErrorVector errors;
272381
errors.Capture();
273-
auto exception = errors.ToException(env, exception_string);
382+
Local<Value> exception = errors.ToException(env, exception_string);
383+
Local<Object> obj;
384+
if (!exception->ToObject(env->context()).ToLocal(&obj))
385+
return;
386+
error::Decorate(env, obj, err);
274387
env->isolate()->ThrowException(exception);
275388
}
276389

‎src/tls_wrap.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ Local<Value> TLSWrap::GetSSLError(int status, int* err, std::string* msg) {
453453
if (c == ' ')
454454
c = '_';
455455
else
456-
c = ::toupper(c);
456+
c = ToUpper(c);
457457
}
458458
obj->Set(context, env()->code_string(),
459459
OneByteString(isolate, ("ERR_SSL_" + code).c_str()))

‎src/util-inl.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,17 @@ std::string ToLower(const std::string& in) {
274274
return out;
275275
}
276276

277+
char ToUpper(char c) {
278+
return c >= 'a' && c <= 'z' ? (c - 'a') + 'A' : c;
279+
}
280+
281+
std::string ToUpper(const std::string& in) {
282+
std::string out(in.size(), 0);
283+
for (size_t i = 0; i < in.size(); ++i)
284+
out[i] = ToUpper(in[i]);
285+
return out;
286+
}
287+
277288
bool StringEqualNoCase(const char* a, const char* b) {
278289
do {
279290
if (*a == '\0')

‎src/util.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,10 @@ inline void SwapBytes64(char* data, size_t nbytes);
284284
inline char ToLower(char c);
285285
inline std::string ToLower(const std::string& in);
286286

287+
// toupper() is locale-sensitive. Use ToUpper() instead.
288+
inline char ToUpper(char c);
289+
inline std::string ToUpper(const std::string& in);
290+
287291
// strcasecmp() is locale-sensitive. Use StringEqualNoCase() instead.
288292
inline bool StringEqualNoCase(const char* a, const char* b);
289293

‎test/parallel/test-crypto-dh.js‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,13 @@ const secret4 = dh4.computeSecret(key2, 'hex', 'base64');
9191

9292
assert.strictEqual(secret1, secret4);
9393

94-
const wrongBlockLength =
95-
/^Error: error:0606506D:digital envelope routines:EVP_DecryptFinal_ex:wrong final block length$/;
94+
const wrongBlockLength = {
95+
message: 'error:0606506D:digital envelope' +
96+
' routines:EVP_DecryptFinal_ex:wrong final block length',
97+
code: 'ERR_OSSL_EVP_WRONG_FINAL_BLOCK_LENGTH',
98+
library: 'digital envelope routines',
99+
reason: 'wrong final block length'
100+
};
96101

97102
// Run this one twice to make sure that the dh3 clears its error properly
98103
{
@@ -111,7 +116,7 @@ const wrongBlockLength =
111116

112117
assert.throws(() => {
113118
dh3.computeSecret('');
114-
}, /^Error: Supplied key is too small$/);
119+
}, { message: 'Supplied key is too small' });
115120

116121
// Create a shared using a DH group.
117122
const alice = crypto.createDiffieHellmanGroup('modp5');
@@ -260,7 +265,7 @@ if (availableCurves.has('prime256v1') && availableCurves.has('secp256k1')) {
260265

261266
assert.throws(() => {
262267
ecdh4.setPublicKey(ecdh3.getPublicKey());
263-
}, /^Error: Failed to convert Buffer to EC_POINT$/);
268+
}, { message: 'Failed to convert Buffer to EC_POINT' });
264269

265270
// Verify that we can use ECDH without having to use newly generated keys.
266271
const ecdh5 = crypto.createECDH('secp256k1');

‎test/parallel/test-crypto-key-objects.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,13 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii');
165165
// This should not cause a crash: https://github.com/nodejs/node/issues/25247
166166
assert.throws(() => {
167167
createPrivateKey({ key: '' });
168-
}, /null/);
168+
}, {
169+
message: 'error:2007E073:BIO routines:BIO_new_mem_buf:null parameter',
170+
code: 'ERR_OSSL_BIO_NULL_PARAMETER',
171+
reason: 'null parameter',
172+
library: 'BIO routines',
173+
function: 'BIO_new_mem_buf',
174+
});
169175
}
170176

171177
[

‎test/parallel/test-crypto-padding.js‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,12 @@ assert.strictEqual(enc(EVEN_LENGTH_PLAIN, true), EVEN_LENGTH_ENCRYPTED);
8282
assert.throws(function() {
8383
// Input must have block length %.
8484
enc(ODD_LENGTH_PLAIN, false);
85-
}, /data not multiple of block length/);
85+
}, {
86+
message: 'error:0607F08A:digital envelope routines:EVP_EncryptFinal_ex:' +
87+
'data not multiple of block length',
88+
code: 'ERR_OSSL_EVP_DATA_NOT_MULTIPLE_OF_BLOCK_LENGTH',
89+
reason: 'data not multiple of block length',
90+
});
8691

8792
assert.strictEqual(
8893
enc(EVEN_LENGTH_PLAIN, false), EVEN_LENGTH_ENCRYPTED_NOPAD
@@ -99,7 +104,12 @@ assert.strictEqual(dec(EVEN_LENGTH_ENCRYPTED, false).length, 48);
99104
assert.throws(function() {
100105
// Must have at least 1 byte of padding (PKCS):
101106
assert.strictEqual(dec(EVEN_LENGTH_ENCRYPTED_NOPAD, true), EVEN_LENGTH_PLAIN);
102-
}, /bad decrypt/);
107+
}, {
108+
message: 'error:06065064:digital envelope routines:EVP_DecryptFinal_ex:' +
109+
'bad decrypt',
110+
reason: 'bad decrypt',
111+
code: 'ERR_OSSL_EVP_BAD_DECRYPT',
112+
});
103113

104114
// No-pad encrypted string should return the same:
105115
assert.strictEqual(

‎test/parallel/test-crypto-rsa-dsa.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,14 @@ const dsaKeyPemEncrypted = fixtures.readSync('test_dsa_privkey_encrypted.pem',
2424
const rsaPkcs8KeyPem = fixtures.readSync('test_rsa_pkcs8_privkey.pem');
2525
const dsaPkcs8KeyPem = fixtures.readSync('test_dsa_pkcs8_privkey.pem');
2626

27-
const decryptError =
28-
/^Error: error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt$/;
27+
const decryptError = {
28+
message: 'error:06065064:digital envelope routines:EVP_DecryptFinal_ex:' +
29+
'bad decrypt',
30+
code: 'ERR_OSSL_EVP_BAD_DECRYPT',
31+
reason: 'bad decrypt',
32+
function: 'EVP_DecryptFinal_ex',
33+
library: 'digital envelope routines',
34+
};
2935

3036
// Test RSA encryption/decryption
3137
{

‎test/parallel/test-crypto-stream.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,11 @@ const cipher = crypto.createCipheriv('aes-128-cbc', key, iv);
7171
const decipher = crypto.createDecipheriv('aes-128-cbc', badkey, iv);
7272

7373
cipher.pipe(decipher)
74-
.on('error', common.mustCall(function end(err) {
75-
assert(/bad decrypt/.test(err));
74+
.on('error', common.expectsError({
75+
message: /bad decrypt/,
76+
function: 'EVP_DecryptFinal_ex',
77+
library: 'digital envelope routines',
78+
reason: 'bad decrypt',
7679
}));
7780

7881
cipher.end('Papaya!'); // Should not cause an unhandled exception.

0 commit comments

Comments
 (0)