diff --git a/doc/api/tls.markdown b/doc/api/tls.markdown index a0fbe9a123c5..ceb1304a7a29 100644 --- a/doc/api/tls.markdown +++ b/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`, diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 861508b9de0c..80887771b157 100644 --- a/lib/_tls_wrap.js +++ b/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); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 6b6beb40bb9d..936f3a903857 100644 --- a/src/node_crypto.cc +++ b/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,31 @@ void SSLWrap::IsInitFinished(const FunctionCallbackInfo& args) { } -#define CASE_X509_ERR(CODE) case X509_V_ERR_##CODE: reason = #CODE; break; template void SSLWrap::VerifyError(const FunctionCallbackInfo& args) { HandleScope scope(node_isolate); Base* w = Unwrap(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 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 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 +1301,17 @@ void SSLWrap::VerifyError(const FunctionCallbackInfo& 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)); + Isolate* isolate = args.GetIsolate(); + Local reason_string = OneByteString(isolate, reason); + Local exception_value = Exception::Error(reason_string); + Local 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 diff --git a/test/simple/test-tls-friendly-error-message.js b/test/simple/test-tls-friendly-error-message.js new file mode 100644 index 000000000000..39fb3023167b --- /dev/null +++ b/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(); + })); +});