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: https://github.com/nodejs/node/pull/26868
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
This commit is contained in:
Sam Roberts 2019-03-27 12:10:21 -07:00
parent 805e614ba7
commit bcbd35a48d
11 changed files with 206 additions and 24 deletions

View File

@ -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 detailing the point in the code at which the `Error` was instantiated, and may
provide a text description of the error. 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, All errors generated by Node.js, including all System and JavaScript errors,
will either be instances of, or inherit from, the `Error` class. 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()` encountered by [`http`][] or [`net`][] — often a sign that a `socket.end()`
was not properly called. 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.
<a id="nodejs-error-codes"></a> <a id="nodejs-error-codes"></a>
## Node.js Error Codes ## Node.js Error Codes

View File

@ -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<Object> 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> 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<std::string> { struct CryptoErrorVector : public std::vector<std::string> {
inline void Capture() { inline void Capture() {
clear(); clear();
@ -258,6 +365,8 @@ struct CryptoErrorVector : public std::vector<std::string> {
void ThrowCryptoError(Environment* env, void ThrowCryptoError(Environment* env,
unsigned long err, // NOLINT(runtime/int) 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) { const char* message = nullptr) {
char message_buffer[128] = {0}; char message_buffer[128] = {0};
if (err != 0 || message == nullptr) { if (err != 0 || message == nullptr) {
@ -270,7 +379,11 @@ void ThrowCryptoError(Environment* env,
.ToLocalChecked(); .ToLocalChecked();
CryptoErrorVector errors; CryptoErrorVector errors;
errors.Capture(); errors.Capture();
auto exception = errors.ToException(env, exception_string); Local<Value> exception = errors.ToException(env, exception_string);
Local<Object> obj;
if (!exception->ToObject(env->context()).ToLocal(&obj))
return;
error::Decorate(env, obj, err);
env->isolate()->ThrowException(exception); env->isolate()->ThrowException(exception);
} }

View File

@ -453,7 +453,7 @@ Local<Value> TLSWrap::GetSSLError(int status, int* err, std::string* msg) {
if (c == ' ') if (c == ' ')
c = '_'; c = '_';
else else
c = ::toupper(c); c = ToUpper(c);
} }
obj->Set(context, env()->code_string(), obj->Set(context, env()->code_string(),
OneByteString(isolate, ("ERR_SSL_" + code).c_str())) OneByteString(isolate, ("ERR_SSL_" + code).c_str()))

View File

@ -274,6 +274,17 @@ std::string ToLower(const std::string& in) {
return out; 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) { bool StringEqualNoCase(const char* a, const char* b) {
do { do {
if (*a == '\0') if (*a == '\0')

View File

@ -284,6 +284,10 @@ inline void SwapBytes64(char* data, size_t nbytes);
inline char ToLower(char c); inline char ToLower(char c);
inline std::string ToLower(const std::string& in); 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. // strcasecmp() is locale-sensitive. Use StringEqualNoCase() instead.
inline bool StringEqualNoCase(const char* a, const char* b); inline bool StringEqualNoCase(const char* a, const char* b);

View File

@ -91,8 +91,13 @@ const secret4 = dh4.computeSecret(key2, 'hex', 'base64');
assert.strictEqual(secret1, secret4); assert.strictEqual(secret1, secret4);
const wrongBlockLength = const wrongBlockLength = {
/^Error: error:0606506D:digital envelope routines:EVP_DecryptFinal_ex:wrong final block length$/; 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 // Run this one twice to make sure that the dh3 clears its error properly
{ {
@ -111,7 +116,7 @@ const wrongBlockLength =
assert.throws(() => { assert.throws(() => {
dh3.computeSecret(''); dh3.computeSecret('');
}, /^Error: Supplied key is too small$/); }, { message: 'Supplied key is too small' });
// Create a shared using a DH group. // Create a shared using a DH group.
const alice = crypto.createDiffieHellmanGroup('modp5'); const alice = crypto.createDiffieHellmanGroup('modp5');
@ -260,7 +265,7 @@ if (availableCurves.has('prime256v1') && availableCurves.has('secp256k1')) {
assert.throws(() => { assert.throws(() => {
ecdh4.setPublicKey(ecdh3.getPublicKey()); 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. // Verify that we can use ECDH without having to use newly generated keys.
const ecdh5 = crypto.createECDH('secp256k1'); const ecdh5 = crypto.createECDH('secp256k1');

View File

@ -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 // This should not cause a crash: https://github.com/nodejs/node/issues/25247
assert.throws(() => { assert.throws(() => {
createPrivateKey({ key: '' }); 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',
});
} }
[ [

View File

@ -82,7 +82,12 @@ assert.strictEqual(enc(EVEN_LENGTH_PLAIN, true), EVEN_LENGTH_ENCRYPTED);
assert.throws(function() { assert.throws(function() {
// Input must have block length %. // Input must have block length %.
enc(ODD_LENGTH_PLAIN, false); 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( assert.strictEqual(
enc(EVEN_LENGTH_PLAIN, false), EVEN_LENGTH_ENCRYPTED_NOPAD 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() { assert.throws(function() {
// Must have at least 1 byte of padding (PKCS): // Must have at least 1 byte of padding (PKCS):
assert.strictEqual(dec(EVEN_LENGTH_ENCRYPTED_NOPAD, true), EVEN_LENGTH_PLAIN); 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: // No-pad encrypted string should return the same:
assert.strictEqual( assert.strictEqual(

View File

@ -24,8 +24,14 @@ const dsaKeyPemEncrypted = fixtures.readSync('test_dsa_privkey_encrypted.pem',
const rsaPkcs8KeyPem = fixtures.readSync('test_rsa_pkcs8_privkey.pem'); const rsaPkcs8KeyPem = fixtures.readSync('test_rsa_pkcs8_privkey.pem');
const dsaPkcs8KeyPem = fixtures.readSync('test_dsa_pkcs8_privkey.pem'); const dsaPkcs8KeyPem = fixtures.readSync('test_dsa_pkcs8_privkey.pem');
const decryptError = const decryptError = {
/^Error: error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt$/; 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 // Test RSA encryption/decryption
{ {

View File

@ -71,8 +71,11 @@ const cipher = crypto.createCipheriv('aes-128-cbc', key, iv);
const decipher = crypto.createDecipheriv('aes-128-cbc', badkey, iv); const decipher = crypto.createDecipheriv('aes-128-cbc', badkey, iv);
cipher.pipe(decipher) cipher.pipe(decipher)
.on('error', common.mustCall(function end(err) { .on('error', common.expectsError({
assert(/bad decrypt/.test(err)); message: /bad decrypt/,
function: 'EVP_DecryptFinal_ex',
library: 'digital envelope routines',
reason: 'bad decrypt',
})); }));
cipher.end('Papaya!'); // Should not cause an unhandled exception. cipher.end('Papaya!'); // Should not cause an unhandled exception.

View File

@ -238,15 +238,19 @@ assert.throws(function() {
// This would inject errors onto OpenSSL's error stack // This would inject errors onto OpenSSL's error stack
crypto.createSign('sha1').sign(sha1_privateKey); crypto.createSign('sha1').sign(sha1_privateKey);
}, (err) => { }, (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. // Throws crypto error, so there is an opensslErrorStack property.
// The openSSL stack should have content. // The openSSL stack should have content.
if ((err instanceof Error) && assert(Array.isArray(err.opensslErrorStack));
/asn1 encoding routines:[^:]*:wrong tag/.test(err) && assert(err.opensslErrorStack.length > 0);
err.opensslErrorStack !== undefined && return true;
Array.isArray(err.opensslErrorStack) &&
err.opensslErrorStack.length > 0) {
return true;
}
}); });
// Make sure memory isn't released before being returned // Make sure memory isn't released before being returned