Decorate crypto openssl errors with .code, .reason, ...

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.
+
+
 <a id="nodejs-error-codes"></a>
 ## Node.js Error Codes
 
@@ -1751,11 +1771,6 @@ Valid TLS protocol versions are `'TLSv1'`, `'TLSv1.1'`, or `'TLSv1.2'`.
 Attempting to set a TLS protocol `minVersion` or `maxVersion` conflicts with an
 attempt to set the `secureProtocol` explicitly. Use one mechanism or the other.
 
-<a id="ERR_TLS_RENEGOTIATE"></a>
-### ERR_TLS_RENEGOTIATE
-
-An attempt to renegotiate the TLS session failed.
-
 <a id="ERR_TLS_RENEGOTIATION_DISABLED"></a>
 ### ERR_TLS_RENEGOTIATION_DISABLED
 

doc/api/tls.md

@@ -1046,9 +1046,10 @@ added: v0.11.8
     `true`.
   * `requestCert`
 * `callback` {Function} If `renegotiate()` returned `true`, callback is
-   attached once to the `'secure'` event. If it returned `false`, it will be
-   called in the next tick with `ERR_TLS_RENEGOTIATE`, unless the `tlsSocket`
-   has been destroyed, in which case it will not be called at all.
+   attached once to the `'secure'` event. If `renegotiate()` returned `false`,
+   `callback` will be called in the next tick with an error, unless the
+   `tlsSocket` has been destroyed, in which case `callback` will not be called
+   at all.
 
 * Returns: {boolean} `true` if renegotiation was initiated, `false` otherwise.
 

lib/_tls_wrap.js

@@ -48,7 +48,6 @@ const {
   ERR_SOCKET_CLOSED,
   ERR_TLS_DH_PARAM_SIZE,
   ERR_TLS_HANDSHAKE_TIMEOUT,
-  ERR_TLS_RENEGOTIATE,
   ERR_TLS_RENEGOTIATION_DISABLED,
   ERR_TLS_REQUIRED_SERVER_NAME,
   ERR_TLS_SESSION_ATTACK,
@@ -661,9 +660,11 @@ TLSSocket.prototype.renegotiate = function(options, callback) {
   // Ensure that we'll cycle through internal openssl's state
   this.write('');
 
-  if (!this._handle.renegotiate()) {
+  try {
+    this._handle.renegotiate();
+  } catch (err) {
     if (callback) {
-      process.nextTick(callback, new ERR_TLS_RENEGOTIATE());
+      process.nextTick(callback, err);
     }
     return false;
   }

lib/internal/errors.js

@@ -1046,7 +1046,6 @@ E('ERR_TLS_INVALID_PROTOCOL_VERSION',
   '%j is not a valid %s TLS protocol version', TypeError);
 E('ERR_TLS_PROTOCOL_VERSION_CONFLICT',
   'TLS protocol version %j conflicts with secureProtocol %j', TypeError);
-E('ERR_TLS_RENEGOTIATE', 'Attempt to renegotiate TLS session failed', Error);
 E('ERR_TLS_RENEGOTIATION_DISABLED',
   'TLS session renegotiation disabled for this socket', Error);
 

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<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> {
   inline void Capture() {
     clear();
@@ -258,6 +365,8 @@ struct CryptoErrorVector : public std::vector<std::string> {
 
 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<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);
 }
 
@@ -2226,9 +2339,9 @@ void SSLWrap<Base>::Renegotiate(const FunctionCallbackInfo<Value>& args) {
 
   ClearErrorOnReturn clear_error_on_return;
 
-  // TODO(@sam-github) Return/throw an error, don't discard the SSL error info.
-  bool yes = SSL_renegotiate(w->ssl_.get()) == 1;
-  args.GetReturnValue().Set(yes);
+  if (SSL_renegotiate(w->ssl_.get()) != 1) {
+    return ThrowCryptoError(w->ssl_env(), ERR_get_error());
+  }
 }
 
 

src/tls_wrap.cc

@@ -453,7 +453,7 @@ Local<Value> 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()))

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')

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);
 

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');

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',
+  });
 }
 
 [

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(

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

@@ -22,8 +22,14 @@ const dsaKeyPem = fixtures.readSync('test_dsa_privkey.pem', 'ascii');
 const dsaKeyPemEncrypted = fixtures.readSync('test_dsa_privkey_encrypted.pem',
                                              'ascii');
 
-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
 {

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.