diff --git a/doc/api/errors.md b/doc/api/errors.md index 0161853774367d..d41de694961ca5 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -186,10 +186,6 @@ circumstance of why the error occurred. `Error` objects capture a "stack trace" detailing the point in the code at which the `Error` was instantiated, and may provide a text description of the error. -For crypto only, `Error` objects will include the OpenSSL error stack in a -separate property called `opensslErrorStack` if it is available when the error -is thrown. - All errors generated by Node.js, including all System and JavaScript errors, will either be instances of, or inherit from, the `Error` class. @@ -589,6 +585,30 @@ program. For a comprehensive list, see the [`errno`(3) man page][]. encountered by [`http`][] or [`net`][] — often a sign that a `socket.end()` was not properly called. +## OpenSSL Errors + +Errors originating in `crypto` or `tls` are of class `Error`, and in addition to +the standard `.code` and `.message` properties, may have some additional +OpenSSL-specific properties. + +### error.opensslErrorStack + +An array of errors that can give context to where in the OpenSSL library an +error originates from. + +### error.function + +The OpenSSL function the error originates in. + +### error.library + +The OpenSSL library the error originates in. + +### error.reason + +A human-readable string describing the reason for the error. + + ## Node.js Error Codes diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 8b5e6dee2fc3ee..c86414fc6296b0 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -212,6 +212,113 @@ static int NoPasswordCallback(char* buf, int size, int rwflag, void* u) { } +// namespace node::crypto::error +namespace error { +void Decorate(Environment* env, Local obj, + unsigned long err) { // NOLINT(runtime/int) + if (err == 0) return; // No decoration possible. + + const char* ls = ERR_lib_error_string(err); + const char* fs = ERR_func_error_string(err); + const char* rs = ERR_reason_error_string(err); + + Isolate* isolate = env->isolate(); + Local context = isolate->GetCurrentContext(); + + if (ls != nullptr) { + if (obj->Set(context, env->library_string(), + OneByteString(isolate, ls)).IsNothing()) { + return; + } + } + if (fs != nullptr) { + if (obj->Set(context, env->function_string(), + OneByteString(isolate, fs)).IsNothing()) { + return; + } + } + if (rs != nullptr) { + if (obj->Set(context, env->reason_string(), + OneByteString(isolate, rs)).IsNothing()) { + return; + } + + // 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 reason(rs); + + for (auto& c : reason) { + if (c == ' ') + c = '_'; + else + c = ToUpper(c); + } + +#define OSSL_ERROR_CODES_MAP(V) \ + V(SYS) \ + V(BN) \ + V(RSA) \ + V(DH) \ + V(EVP) \ + V(BUF) \ + V(OBJ) \ + V(PEM) \ + V(DSA) \ + V(X509) \ + V(ASN1) \ + V(CONF) \ + V(CRYPTO) \ + V(EC) \ + V(SSL) \ + V(BIO) \ + V(PKCS7) \ + V(X509V3) \ + V(PKCS12) \ + V(RAND) \ + V(DSO) \ + V(ENGINE) \ + V(OCSP) \ + V(UI) \ + V(COMP) \ + V(ECDSA) \ + V(ECDH) \ + V(OSSL_STORE) \ + V(FIPS) \ + V(CMS) \ + V(TS) \ + V(HMAC) \ + V(CT) \ + V(ASYNC) \ + V(KDF) \ + V(SM2) \ + V(USER) \ + +#define V(name) case ERR_LIB_##name: lib = #name "_"; break; + const char* lib = ""; + const char* prefix = "OSSL_"; + switch (ERR_GET_LIB(err)) { OSSL_ERROR_CODES_MAP(V) } +#undef V +#undef OSSL_ERROR_CODES_MAP + // Don't generate codes like "ERR_OSSL_SSL_". + if (lib && strcmp(lib, "SSL_") == 0) + prefix = ""; + + // All OpenSSL reason strings fit in a single 80-column macro definition, + // all prefix lengths are <= 10, and ERR_OSSL_ is 9, so 128 is more than + // sufficient. + char code[128]; + snprintf(code, sizeof(code), "ERR_%s%s%s", prefix, lib, reason.c_str()); + + if (obj->Set(env->isolate()->GetCurrentContext(), + env->code_string(), + OneByteString(env->isolate(), code)).IsNothing()) + return; + } +} +} // namespace error + + struct CryptoErrorVector : public std::vector { inline void Capture() { clear(); @@ -258,6 +365,8 @@ struct CryptoErrorVector : public std::vector { void ThrowCryptoError(Environment* env, unsigned long err, // NOLINT(runtime/int) + // Default, only used if there is no SSL `err` which can + // be used to create a long-style message string. const char* message = nullptr) { char message_buffer[128] = {0}; if (err != 0 || message == nullptr) { @@ -270,7 +379,11 @@ void ThrowCryptoError(Environment* env, .ToLocalChecked(); CryptoErrorVector errors; errors.Capture(); - auto exception = errors.ToException(env, exception_string); + Local exception = errors.ToException(env, exception_string); + Local obj; + if (!exception->ToObject(env->context()).ToLocal(&obj)) + return; + error::Decorate(env, obj, err); env->isolate()->ThrowException(exception); } diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 59400d8f3a4a72..14802b1dfb74b8 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -453,7 +453,7 @@ Local TLSWrap::GetSSLError(int status, int* err, std::string* msg) { if (c == ' ') c = '_'; else - c = ::toupper(c); + c = ToUpper(c); } obj->Set(context, env()->code_string(), OneByteString(isolate, ("ERR_SSL_" + code).c_str())) diff --git a/src/util-inl.h b/src/util-inl.h index 5b59f25bc5ff16..99d470205e1ce7 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -274,6 +274,17 @@ std::string ToLower(const std::string& in) { return out; } +char ToUpper(char c) { + return c >= 'a' && c <= 'z' ? (c - 'a') + 'A' : c; +} + +std::string ToUpper(const std::string& in) { + std::string out(in.size(), 0); + for (size_t i = 0; i < in.size(); ++i) + out[i] = ToUpper(in[i]); + return out; +} + bool StringEqualNoCase(const char* a, const char* b) { do { if (*a == '\0') diff --git a/src/util.h b/src/util.h index 56b2bc83382698..a9de8f8636c11b 100644 --- a/src/util.h +++ b/src/util.h @@ -284,6 +284,10 @@ inline void SwapBytes64(char* data, size_t nbytes); inline char ToLower(char c); inline std::string ToLower(const std::string& in); +// toupper() is locale-sensitive. Use ToUpper() instead. +inline char ToUpper(char c); +inline std::string ToUpper(const std::string& in); + // strcasecmp() is locale-sensitive. Use StringEqualNoCase() instead. inline bool StringEqualNoCase(const char* a, const char* b); diff --git a/test/parallel/test-crypto-dh.js b/test/parallel/test-crypto-dh.js index ead77d6a30c811..69d18f458e60bd 100644 --- a/test/parallel/test-crypto-dh.js +++ b/test/parallel/test-crypto-dh.js @@ -91,8 +91,13 @@ const secret4 = dh4.computeSecret(key2, 'hex', 'base64'); assert.strictEqual(secret1, secret4); -const wrongBlockLength = - /^Error: error:0606506D:digital envelope routines:EVP_DecryptFinal_ex:wrong final block length$/; +const wrongBlockLength = { + message: 'error:0606506D:digital envelope' + + ' routines:EVP_DecryptFinal_ex:wrong final block length', + code: 'ERR_OSSL_EVP_WRONG_FINAL_BLOCK_LENGTH', + library: 'digital envelope routines', + reason: 'wrong final block length' +}; // Run this one twice to make sure that the dh3 clears its error properly { @@ -111,7 +116,7 @@ const wrongBlockLength = assert.throws(() => { dh3.computeSecret(''); -}, /^Error: Supplied key is too small$/); +}, { message: 'Supplied key is too small' }); // Create a shared using a DH group. const alice = crypto.createDiffieHellmanGroup('modp5'); @@ -260,7 +265,7 @@ if (availableCurves.has('prime256v1') && availableCurves.has('secp256k1')) { assert.throws(() => { ecdh4.setPublicKey(ecdh3.getPublicKey()); - }, /^Error: Failed to convert Buffer to EC_POINT$/); + }, { message: 'Failed to convert Buffer to EC_POINT' }); // Verify that we can use ECDH without having to use newly generated keys. const ecdh5 = crypto.createECDH('secp256k1'); diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js index fb1c3de1d7e224..c9cb90d408500f 100644 --- a/test/parallel/test-crypto-key-objects.js +++ b/test/parallel/test-crypto-key-objects.js @@ -165,7 +165,13 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii'); // This should not cause a crash: https://github.com/nodejs/node/issues/25247 assert.throws(() => { createPrivateKey({ key: '' }); - }, /null/); + }, { + message: 'error:2007E073:BIO routines:BIO_new_mem_buf:null parameter', + code: 'ERR_OSSL_BIO_NULL_PARAMETER', + reason: 'null parameter', + library: 'BIO routines', + function: 'BIO_new_mem_buf', + }); } [ diff --git a/test/parallel/test-crypto-padding.js b/test/parallel/test-crypto-padding.js index d4a5b95cec9242..909c014bd0f87a 100644 --- a/test/parallel/test-crypto-padding.js +++ b/test/parallel/test-crypto-padding.js @@ -82,7 +82,12 @@ assert.strictEqual(enc(EVEN_LENGTH_PLAIN, true), EVEN_LENGTH_ENCRYPTED); assert.throws(function() { // Input must have block length %. enc(ODD_LENGTH_PLAIN, false); -}, /data not multiple of block length/); +}, { + message: 'error:0607F08A:digital envelope routines:EVP_EncryptFinal_ex:' + + 'data not multiple of block length', + code: 'ERR_OSSL_EVP_DATA_NOT_MULTIPLE_OF_BLOCK_LENGTH', + reason: 'data not multiple of block length', +}); assert.strictEqual( enc(EVEN_LENGTH_PLAIN, false), EVEN_LENGTH_ENCRYPTED_NOPAD @@ -99,7 +104,12 @@ assert.strictEqual(dec(EVEN_LENGTH_ENCRYPTED, false).length, 48); assert.throws(function() { // Must have at least 1 byte of padding (PKCS): assert.strictEqual(dec(EVEN_LENGTH_ENCRYPTED_NOPAD, true), EVEN_LENGTH_PLAIN); -}, /bad decrypt/); +}, { + message: 'error:06065064:digital envelope routines:EVP_DecryptFinal_ex:' + + 'bad decrypt', + reason: 'bad decrypt', + code: 'ERR_OSSL_EVP_BAD_DECRYPT', +}); // No-pad encrypted string should return the same: assert.strictEqual( diff --git a/test/parallel/test-crypto-rsa-dsa.js b/test/parallel/test-crypto-rsa-dsa.js index 589fa57a1c9e77..143b7c2c41276a 100644 --- a/test/parallel/test-crypto-rsa-dsa.js +++ b/test/parallel/test-crypto-rsa-dsa.js @@ -24,8 +24,14 @@ const dsaKeyPemEncrypted = fixtures.readSync('test_dsa_privkey_encrypted.pem', const rsaPkcs8KeyPem = fixtures.readSync('test_rsa_pkcs8_privkey.pem'); const dsaPkcs8KeyPem = fixtures.readSync('test_dsa_pkcs8_privkey.pem'); -const decryptError = - /^Error: error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt$/; +const decryptError = { + message: 'error:06065064:digital envelope routines:EVP_DecryptFinal_ex:' + + 'bad decrypt', + code: 'ERR_OSSL_EVP_BAD_DECRYPT', + reason: 'bad decrypt', + function: 'EVP_DecryptFinal_ex', + library: 'digital envelope routines', +}; // Test RSA encryption/decryption { diff --git a/test/parallel/test-crypto-stream.js b/test/parallel/test-crypto-stream.js index 9430084bbee179..2d005c89db3f09 100644 --- a/test/parallel/test-crypto-stream.js +++ b/test/parallel/test-crypto-stream.js @@ -71,8 +71,11 @@ const cipher = crypto.createCipheriv('aes-128-cbc', key, iv); const decipher = crypto.createDecipheriv('aes-128-cbc', badkey, iv); cipher.pipe(decipher) - .on('error', common.mustCall(function end(err) { - assert(/bad decrypt/.test(err)); + .on('error', common.expectsError({ + message: /bad decrypt/, + function: 'EVP_DecryptFinal_ex', + library: 'digital envelope routines', + reason: 'bad decrypt', })); cipher.end('Papaya!'); // Should not cause an unhandled exception. diff --git a/test/parallel/test-crypto.js b/test/parallel/test-crypto.js index 005a733c0779d9..1952f8908acb03 100644 --- a/test/parallel/test-crypto.js +++ b/test/parallel/test-crypto.js @@ -238,15 +238,19 @@ assert.throws(function() { // This would inject errors onto OpenSSL's error stack crypto.createSign('sha1').sign(sha1_privateKey); }, (err) => { + // Do the standard checks, but then do some custom checks afterwards. + assert.throws(() => { throw err; }, { + message: 'error:0D0680A8:asn1 encoding routines:asn1_check_tlen:wrong tag', + library: 'asn1 encoding routines', + function: 'asn1_check_tlen', + reason: 'wrong tag', + code: 'ERR_OSSL_ASN1_WRONG_TAG', + }); // Throws crypto error, so there is an opensslErrorStack property. // The openSSL stack should have content. - if ((err instanceof Error) && - /asn1 encoding routines:[^:]*:wrong tag/.test(err) && - err.opensslErrorStack !== undefined && - Array.isArray(err.opensslErrorStack) && - err.opensslErrorStack.length > 0) { - return true; - } + assert(Array.isArray(err.opensslErrorStack)); + assert(err.opensslErrorStack.length > 0); + return true; }); // Make sure memory isn't released before being returned