diff --git a/doc/api/errors.md b/doc/api/errors.md index 8597cece9b16c6..93e41cfb146861 100755 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -192,6 +192,10 @@ 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. diff --git a/src/env.h b/src/env.h index 21d9ab032febac..e37a0eb6311a41 100644 --- a/src/env.h +++ b/src/env.h @@ -225,6 +225,7 @@ class ModuleWrap; V(onstreamclose_string, "onstreamclose") \ V(ontrailers_string, "ontrailers") \ V(onwrite_string, "onwrite") \ + V(openssl_error_stack, "opensslErrorStack") \ V(output_string, "output") \ V(order_string, "order") \ V(owner_string, "owner") \ diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 968f7a8f629808..1c953f093146c7 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -255,13 +255,65 @@ void ThrowCryptoError(Environment* env, unsigned long err, // NOLINT(runtime/int) const char* default_message = nullptr) { HandleScope scope(env->isolate()); + Local message; + if (err != 0 || default_message == nullptr) { char errmsg[128] = { 0 }; ERR_error_string_n(err, errmsg, sizeof(errmsg)); - env->ThrowError(errmsg); + message = String::NewFromUtf8(env->isolate(), errmsg, + v8::NewStringType::kNormal) + .ToLocalChecked(); } else { - env->ThrowError(default_message); + message = String::NewFromUtf8(env->isolate(), default_message, + v8::NewStringType::kNormal) + .ToLocalChecked(); + } + + Local exception_v = Exception::Error(message); + CHECK(!exception_v.IsEmpty()); + Local exception = exception_v.As(); + ERR_STATE* es = ERR_get_state(); + + if (es->bottom != es->top) { + Local error_stack = Array::New(env->isolate()); + int top = es->top; + + // Build the error_stack array to be added to opensslErrorStack property. + for (unsigned int i = 0; es->bottom != es->top;) { + unsigned long err_buf = es->err_buffer[es->top]; // NOLINT(runtime/int) + // Only add error string if there is valid err_buffer. + if (err_buf) { + char tmp_str[256]; + ERR_error_string_n(err_buf, tmp_str, sizeof(tmp_str)); + error_stack->Set(env->context(), i, + String::NewFromUtf8(env->isolate(), tmp_str, + v8::NewStringType::kNormal) + .ToLocalChecked()).FromJust(); + // Only increment if we added to error_stack. + i++; + } + + // Since the ERR_STATE is a ring buffer, we need to use modular + // arithmetic to loop back around in the case where bottom is after top. + // Using ERR_NUM_ERRORS macro defined in openssl. + es->top = (((es->top - 1) % ERR_NUM_ERRORS) + ERR_NUM_ERRORS) % + ERR_NUM_ERRORS; + } + + // Restore top. + es->top = top; + + // Add the opensslErrorStack property to the exception object. + // The new property will look like the following: + // opensslErrorStack: [ + // 'error:0906700D:PEM routines:PEM_ASN1_read_bio:ASN1 lib', + // 'error:0D07803A:asn1 encoding routines:ASN1_ITEM_EX_D2I:nested asn1 err' + // ] + exception->Set(env->context(), env->openssl_error_stack(), error_stack) + .FromJust(); } + + env->isolate()->ThrowException(exception); } @@ -4316,8 +4368,6 @@ SignBase::Error Verify::VerifyFinal(const char* key_pem, if (!initialised_) return kSignNotInitialised; - ClearErrorOnReturn clear_error_on_return; - EVP_PKEY* pkey = nullptr; BIO* bp = nullptr; X509* x509 = nullptr; @@ -4406,6 +4456,8 @@ SignBase::Error Verify::VerifyFinal(const char* key_pem, void Verify::VerifyFinal(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + ClearErrorOnReturn clear_error_on_return; + Verify* verify; ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder()); diff --git a/test/parallel/test-crypto.js b/test/parallel/test-crypto.js index 6262e083841617..403b4eb84cc6fc 100644 --- a/test/parallel/test-crypto.js +++ b/test/parallel/test-crypto.js @@ -46,7 +46,14 @@ assert.throws(function() { const context = credentials.context; const notcontext = { setOptions: context.setOptions, setKey: context.setKey }; tls.createSecureContext({ secureOptions: 1 }, notcontext); -}, /^TypeError: Illegal invocation$/); +}, (err) => { + // Throws TypeError, so there is no opensslErrorStack property. + if ((err instanceof Error) && + /^TypeError: Illegal invocation$/.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); // PFX tests assert.doesNotThrow(function() { @@ -55,15 +62,36 @@ assert.doesNotThrow(function() { assert.throws(function() { tls.createSecureContext({ pfx: certPfx }); -}, /^Error: mac verify failure$/); +}, (err) => { + // Throws general Error, so there is no opensslErrorStack property. + if ((err instanceof Error) && + /^Error: mac verify failure$/.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); assert.throws(function() { tls.createSecureContext({ pfx: certPfx, passphrase: 'test' }); -}, /^Error: mac verify failure$/); +}, (err) => { + // Throws general Error, so there is no opensslErrorStack property. + if ((err instanceof Error) && + /^Error: mac verify failure$/.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); assert.throws(function() { tls.createSecureContext({ pfx: 'sample', passphrase: 'test' }); -}, /^Error: not enough data$/); +}, (err) => { + // Throws general Error, so there is no opensslErrorStack property. + if ((err instanceof Error) && + /^Error: not enough data$/.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); // update() should only take buffers / strings @@ -135,23 +163,62 @@ testImmutability(crypto.getCurves); // throw, not assert in C++ land. assert.throws(function() { crypto.createCipher('aes192', 'test').update('0', 'hex'); -}, common.hasFipsCrypto ? /not supported in FIPS mode/ : /Bad input string/); +}, (err) => { + const errorMessage = + common.hasFipsCrypto ? /not supported in FIPS mode/ : /Bad input string/; + // Throws general Error, so there is no opensslErrorStack property. + if ((err instanceof Error) && + errorMessage.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); assert.throws(function() { crypto.createDecipher('aes192', 'test').update('0', 'hex'); -}, common.hasFipsCrypto ? /not supported in FIPS mode/ : /Bad input string/); +}, (err) => { + const errorMessage = + common.hasFipsCrypto ? /not supported in FIPS mode/ : /Bad input string/; + // Throws general Error, so there is no opensslErrorStack property. + if ((err instanceof Error) && + errorMessage.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); assert.throws(function() { crypto.createHash('sha1').update('0', 'hex'); -}, /^TypeError: Bad input string$/); +}, (err) => { + // Throws TypeError, so there is no opensslErrorStack property. + if ((err instanceof Error) && + /^TypeError: Bad input string$/.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); assert.throws(function() { crypto.createSign('SHA1').update('0', 'hex'); -}, /^TypeError: Bad input string$/); +}, (err) => { + // Throws TypeError, so there is no opensslErrorStack property. + if ((err instanceof Error) && + /^TypeError: Bad input string$/.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); assert.throws(function() { crypto.createVerify('SHA1').update('0', 'hex'); -}, /^TypeError: Bad input string$/); +}, (err) => { + // Throws TypeError, so there is no opensslErrorStack property. + if ((err instanceof Error) && + /^TypeError: Bad input string$/.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); assert.throws(function() { const priv = [ @@ -164,7 +231,13 @@ assert.throws(function() { '' ].join('\n'); crypto.createSign('SHA256').update('test').sign(priv); -}, /digest too big for rsa key$/); +}, (err) => { + if ((err instanceof Error) && + /digest too big for rsa key$/.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); assert.throws(function() { // The correct header inside `test_bad_rsa_privkey.pem` should have been @@ -180,14 +253,31 @@ assert.throws(function() { 'ascii'); // this would inject errors onto OpenSSL's error stack crypto.createSign('sha1').sign(sha1_privateKey); -}, /asn1 encoding routines:ASN1_CHECK_TLEN:wrong tag/); +}, (err) => { + // Throws crypto error, so there is an opensslErrorStack property. + // The openSSL stack should have content. + if ((err instanceof Error) && + /asn1 encoding routines:ASN1_CHECK_TLEN:wrong tag/.test(err) && + err.opensslErrorStack !== undefined && + Array.isArray(err.opensslErrorStack) && + err.opensslErrorStack.length > 0) { + return true; + } +}); // Make sure memory isn't released before being returned console.log(crypto.randomBytes(16)); assert.throws(function() { tls.createSecureContext({ crl: 'not a CRL' }); -}, /^Error: Failed to parse CRL$/); +}, (err) => { + // Throws general error, so there is no opensslErrorStack property. + if ((err instanceof Error) && + /^Error: Failed to parse CRL$/.test(err) && + err.opensslErrorStack === undefined) { + return true; + } +}); /** * Check if the stream function uses utf8 as a default encoding.