tls: show human-readable error messages

doc/api/tls.markdown

@@ -294,7 +294,7 @@ Creates a new client connection to the given `port` and `host` (old API) or
 
   - `rejectUnauthorized`: If `true`, the server certificate is verified against
     the list of supplied CAs. An `'error'` event is emitted if verification
-    fails. Default: `true`.
+    fails; `err.code` contains the OpenSSL error code. Default: `true`.
 
   - `NPNProtocols`: An array of strings or `Buffer`s containing supported NPN
     protocols. `Buffer`s should have following format: `0x05hello0x05world`,

lib/_tls_wrap.js

@@ -526,7 +526,7 @@ function Server(/* [options], listener */) {
       if (socket._requestCert) {
         var verifyError = socket.ssl.verifyError();
         if (verifyError) {
-          socket.authorizationError = verifyError.message;
+          socket.authorizationError = verifyError.code;
 
           if (socket._rejectUnauthorized)
             socket.destroy();
@@ -788,7 +788,7 @@ exports.connect = function(/* [port, host], options, cb */) {
 
       if (verifyError) {
         result.authorized = false;
-        result.authorizationError = verifyError.message;
+        result.authorizationError = verifyError.code || verifyError.message;
 
         if (options.rejectUnauthorized) {
           result.emit('error', verifyError);

src/node_crypto.cc

@@ -83,6 +83,7 @@ using v8::FunctionTemplate;
 using v8::Handle;
 using v8::HandleScope;
 using v8::Integer;
+using v8::Isolate;
 using v8::Local;
 using v8::Null;
 using v8::Object;
@@ -1248,32 +1249,32 @@ void SSLWrap<Base>::IsInitFinished(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-#define CASE_X509_ERR(CODE) case X509_V_ERR_##CODE: reason = #CODE; break;
 template <class Base>
 void SSLWrap<Base>::VerifyError(const FunctionCallbackInfo<Value>& args) {
-  HandleScope scope(node_isolate);
+  Isolate* isolate = args.GetIsolate();
+  HandleScope scope(isolate);
 
   Base* w = Unwrap<Base>(args.This());
 
-  // XXX(indutny) Do this check in JS land?
-  X509* peer_cert = SSL_get_peer_certificate(w->ssl_);
-  if (peer_cert == NULL) {
-    // We requested a certificate and they did not send us one.
-    // Definitely an error.
-    // XXX(indutny) is this the right error message?
-    Local<String> s =
-        FIXED_ONE_BYTE_STRING(node_isolate, "UNABLE_TO_GET_ISSUER_CERT");
-    return args.GetReturnValue().Set(Exception::Error(s));
+  // XXX(bnoordhuis) The UNABLE_TO_GET_ISSUER_CERT error when there is no
+  // peer certificate is questionable but it's compatible with what was
+  // here before.
+  long x509_verify_error = X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT;
+  if (X509* peer_cert = SSL_get_peer_certificate(w->ssl_)) {
+    X509_free(peer_cert);
+    x509_verify_error = SSL_get_verify_result(w->ssl_);
   }
-  X509_free(peer_cert);
 
-  long x509_verify_error = SSL_get_verify_result(w->ssl_);
+  if (x509_verify_error == X509_V_OK)
+    return args.GetReturnValue().SetNull();
 
-  const char* reason = NULL;
-  Local<String> s;
+  // XXX(bnoordhuis) X509_verify_cert_error_string() is not actually thread-safe
+  // in the presence of invalid error codes.  Probably academical but something
+  // to keep in mind if/when node ever grows multi-isolate capabilities.
+  const char* reason = X509_verify_cert_error_string(x509_verify_error);
+  const char* code = reason;
+#define CASE_X509_ERR(CODE) case X509_V_ERR_##CODE: code = #CODE; break;
   switch (x509_verify_error) {
-    case X509_V_OK:
-      return args.GetReturnValue().SetNull();
     CASE_X509_ERR(UNABLE_TO_GET_ISSUER_CERT)
     CASE_X509_ERR(UNABLE_TO_GET_CRL)
     CASE_X509_ERR(UNABLE_TO_DECRYPT_CERT_SIGNATURE)
@@ -1301,18 +1302,16 @@ void SSLWrap<Base>::VerifyError(const FunctionCallbackInfo<Value>& args) {
     CASE_X509_ERR(INVALID_PURPOSE)
     CASE_X509_ERR(CERT_UNTRUSTED)
     CASE_X509_ERR(CERT_REJECTED)
-    default:
-      s = OneByteString(node_isolate,
-                        X509_verify_cert_error_string(x509_verify_error));
-      break;
   }
+#undef CASE_X509_ERR
 
-  if (s.IsEmpty())
-    s = OneByteString(node_isolate, reason);
-
-  args.GetReturnValue().Set(Exception::Error(s));
+  Local<String> reason_string = OneByteString(isolate, reason);
+  Local<Value> exception_value = Exception::Error(reason_string);
+  Local<Object> exception_object = exception_value->ToObject();
+  exception_object->Set(FIXED_ONE_BYTE_STRING(isolate, "code"),
+                        OneByteString(isolate, code));
+  args.GetReturnValue().Set(exception_object);
 }
-#undef CASE_X509_ERR
 
 
 template <class Base>

test/simple/test-tls-friendly-error-message.js

@@ -0,0 +1,45 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+if (!process.versions.openssl) {
+  console.error('Skipping because node compiled without OpenSSL.');
+  process.exit(0);
+}
+
+var common = require('../common');
+var assert = require('assert');
+var fs = require('fs');
+var tls = require('tls');
+
+var key = fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem');
+var cert = fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem');
+
+tls.createServer({ key: key, cert: cert }, function(conn) {
+  conn.end();
+  this.close();
+}).listen(0, function() {
+  var options = { port: this.address().port, rejectUnauthorized: true };
+  tls.connect(options).on('error', common.mustCall(function(err) {
+    assert.equal(err.code, 'UNABLE_TO_VERIFY_LEAF_SIGNATURE');
+    assert.equal(err.message, 'unable to verify the first certificate');
+    this.destroy();
+  }));
+});