diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 816fcb689607c7..d9f85cf8a847cd 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -61,6 +61,8 @@ const noop = () => {}; let ipServernameWarned = false; +// Server side times how long a handshake is taking to protect against slow +// handshakes being used for DoS. function onhandshakestart(now) { debug('onhandshakestart'); @@ -120,6 +122,7 @@ function loadSession(hello) { return owner.destroy(new ERR_SOCKET_CLOSED()); owner._handle.loadSession(session); + // Session is loaded. End the parser to allow handshaking to continue. owner._handle.endParser(); } @@ -127,6 +130,11 @@ function loadSession(hello) { hello.tlsTicket || owner.server && !owner.server.emit('resumeSession', hello.sessionId, onSession)) { + // Sessions without identifiers can't be resumed. + // Sessions with tickets can be resumed directly from the ticket, no server + // session storage is necessary. + // Without a call to a resumeSession listener, a session will never be + // loaded, so end the parser to allow handshaking to continue. owner._handle.endParser(); } } @@ -215,13 +223,17 @@ function requestOCSPDone(socket) { function onnewsession(sessionId, session) { + debug('onnewsession'); const owner = this[owner_symbol]; + // XXX(sam) no server to emit the event on, but handshake won't continue + // unless newSessionDone() is called, should it be? if (!owner.server) return; var once = false; const done = () => { + debug('onnewsession done'); if (once) return; once = true; @@ -273,19 +285,19 @@ function onerror(err) { // Used by both client and server TLSSockets to start data flowing from _handle, // read(0) causes a StreamBase::ReadStart, via Socket._read. -function initRead(tls, socket) { +function initRead(tlsSocket, socket) { // If we were destroyed already don't bother reading - if (!tls._handle) + if (!tlsSocket._handle) return; // Socket already has some buffered data - emulate receiving it if (socket && socket.readableLength) { var buf; while ((buf = socket.read()) !== null) - tls._handle.receive(buf); + tlsSocket._handle.receive(buf); } - tls.read(0); + tlsSocket.read(0); } /** @@ -312,8 +324,12 @@ function TLSSocket(socket, opts) { var wrap; if ((socket instanceof net.Socket && socket._handle) || !socket) { + // 1. connected socket + // 2. no socket, one will be created with net.Socket().connect wrap = socket; } else { + // 3. socket has no handle so is js not c++ + // 4. unconnected sockets are wrapped // TLS expects to interact from C++ with a net.Socket that has a C++ stream // handle, but a JS stream doesn't have one. Wrap it up to make it look like // a socket. @@ -333,7 +349,7 @@ function TLSSocket(socket, opts) { }); // Proxy for API compatibility - this.ssl = this._handle; + this.ssl = this._handle; // C++ TLSWrap object this.on('error', this._tlsError); @@ -429,8 +445,8 @@ TLSSocket.prototype._wrapHandle = function(wrap) { const res = tls_wrap.wrap(externalStream, context.context, !!options.isServer); - res._parent = handle; - res._parentWrap = wrap; + res._parent = handle; // C++ "wrap" object: TCPWrap, JSStream, ... + res._parentWrap = wrap; // JS object: net.Socket, JSStreamSocket, ... res._secureContext = context; res.reading = handle.reading; this[kRes] = res; @@ -480,8 +496,8 @@ TLSSocket.prototype._init = function(socket, wrap) { this.server = options.server; - // For clients, we will always have either a given ca list or be using - // default one + // Clients (!isServer) always request a cert, servers request a client cert + // only on explicit configuration. const requestCert = !!options.requestCert || !options.isServer; const rejectUnauthorized = !!options.rejectUnauthorized; @@ -502,6 +518,7 @@ TLSSocket.prototype._init = function(socket, wrap) { if (this.server) { if (this.server.listenerCount('resumeSession') > 0 || this.server.listenerCount('newSession') > 0) { + // Also starts the client hello parser as a side effect. ssl.enableSessionCallbacks(); } if (this.server.listenerCount('OCSPRequest') > 0) @@ -709,7 +726,7 @@ TLSSocket.prototype.getCipher = function(err) { // TODO: support anonymous (nocert) and PSK -function onSocketSecure() { +function onServerSocketSecure() { if (this._requestCert) { const verifyError = this._handle.verifyError(); if (verifyError) { @@ -760,7 +777,7 @@ function tlsConnectionListener(rawSocket) { SNICallback: this[kSNICallback] || SNICallback }); - socket.on('secure', onSocketSecure); + socket.on('secure', onServerSocketSecure); socket[kErrorEmitted] = false; socket.on('close', onSocketClose); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 9715e8b7764326..4f0ce4c3d71fb5 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2648,47 +2648,19 @@ int SSLWrap::SetCACerts(SecureContext* sc) { } int VerifyCallback(int preverify_ok, X509_STORE_CTX* ctx) { - // Quoting SSL_set_verify(3ssl): + // From https://www.openssl.org/docs/man1.1.1/man3/SSL_verify_cb: // - // The VerifyCallback function is used to control the behaviour when - // the SSL_VERIFY_PEER flag is set. It must be supplied by the - // application and receives two arguments: preverify_ok indicates, - // whether the verification of the certificate in question was passed - // (preverify_ok=1) or not (preverify_ok=0). x509_ctx is a pointer to - // the complete context used for the certificate chain verification. - // - // The certificate chain is checked starting with the deepest nesting - // level (the root CA certificate) and worked upward to the peer's - // certificate. At each level signatures and issuer attributes are - // checked. Whenever a verification error is found, the error number is - // stored in x509_ctx and VerifyCallback is called with preverify_ok=0. - // By applying X509_CTX_store_* functions VerifyCallback can locate the - // certificate in question and perform additional steps (see EXAMPLES). - // If no error is found for a certificate, VerifyCallback is called - // with preverify_ok=1 before advancing to the next level. - // - // The return value of VerifyCallback controls the strategy of the - // further verification process. If VerifyCallback returns 0, the - // verification process is immediately stopped with "verification - // failed" state. If SSL_VERIFY_PEER is set, a verification failure - // alert is sent to the peer and the TLS/SSL handshake is terminated. If - // VerifyCallback returns 1, the verification process is continued. If + // If VerifyCallback returns 1, the verification process is continued. If // VerifyCallback always returns 1, the TLS/SSL handshake will not be - // terminated with respect to verification failures and the connection - // will be established. The calling process can however retrieve the - // error code of the last verification error using - // SSL_get_verify_result(3) or by maintaining its own error storage - // managed by VerifyCallback. - // - // If no VerifyCallback is specified, the default callback will be - // used. Its return value is identical to preverify_ok, so that any - // verification failure will lead to a termination of the TLS/SSL - // handshake with an alert message, if SSL_VERIFY_PEER is set. + // terminated with respect to verification failures and the connection will + // be established. The calling process can however retrieve the error code + // of the last verification error using SSL_get_verify_result(3) or by + // maintaining its own error storage managed by VerifyCallback. // - // Since we cannot perform I/O quickly enough in this callback, we ignore - // all preverify_ok errors and let the handshake continue. It is - // imperative that the user use Connection::VerifyError after the - // 'secure' callback has been made. + // Since we cannot perform I/O quickly enough with X509_STORE_CTX_ APIs in + // this callback, we ignore all preverify_ok errors and let the handshake + // continue. It is imperative that the user use Connection::VerifyError after + // the 'secure' callback has been made. return 1; } diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index e467e2d167f628..1f9e8ddfb71bd4 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -210,10 +210,9 @@ void TLSWrap::SSLInfoCallback(const SSL* ssl_, int where, int ret) { if (!(where & (SSL_CB_HANDSHAKE_START | SSL_CB_HANDSHAKE_DONE))) return; - // Be compatible with older versions of OpenSSL. SSL_get_app_data() wants - // a non-const SSL* in OpenSSL <= 0.9.7e. + // SSL_renegotiate_pending() should take `const SSL*`, but it does not. SSL* ssl = const_cast(ssl_); - TLSWrap* c = static_cast(SSL_get_app_data(ssl)); + TLSWrap* c = static_cast(SSL_get_app_data(ssl_)); Environment* env = c->env(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); @@ -349,14 +348,14 @@ Local TLSWrap::GetSSLError(int status, int* err, std::string* msg) { case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: case SSL_ERROR_WANT_X509_LOOKUP: - break; + return Local(); + case SSL_ERROR_ZERO_RETURN: return scope.Escape(env()->zero_return_string()); - break; - default: - { - CHECK(*err == SSL_ERROR_SSL || *err == SSL_ERROR_SYSCALL); + case SSL_ERROR_SSL: + case SSL_ERROR_SYSCALL: + { unsigned long ssl_err = ERR_peek_error(); // NOLINT(runtime/int) BIO* bio = BIO_new(BIO_s_mem()); ERR_print_errors(bio); @@ -409,8 +408,11 @@ Local TLSWrap::GetSSLError(int status, int* err, std::string* msg) { return scope.Escape(exception); } + + default: + UNREACHABLE(); } - return Local(); + UNREACHABLE(); } @@ -768,7 +770,7 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo& args) { if (wrap->is_server()) { bool request_cert = args[0]->IsTrue(); if (!request_cert) { - // Note reject_unauthorized ignored. + // If no cert is requested, there will be none to reject as unauthorized. verify_mode = SSL_VERIFY_NONE; } else { bool reject_unauthorized = args[1]->IsTrue(); @@ -777,7 +779,9 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo& args) { verify_mode |= SSL_VERIFY_FAIL_IF_NO_PEER_CERT; } } else { - // Note request_cert and reject_unauthorized are ignored for clients. + // Servers always send a cert if the cipher is not anonymous (anon is + // disabled by default), so use VERIFY_NONE and check the cert after the + // handshake has completed. verify_mode = SSL_VERIFY_NONE; } diff --git a/src/tls_wrap.h b/src/tls_wrap.h index cd2701cd6d9fb7..d3cbb992bafb29 100644 --- a/src/tls_wrap.h +++ b/src/tls_wrap.h @@ -57,16 +57,19 @@ class TLSWrap : public AsyncWrap, v8::Local context, void* priv); - int GetFD() override; + // Implement StreamBase: bool IsAlive() override; bool IsClosing() override; - - // JavaScript functions - int ReadStart() override; - int ReadStop() override; - + bool IsIPCPipe() override; + int GetFD() override; ShutdownWrap* CreateShutdownWrap( v8::Local req_wrap_object) override; + AsyncWrap* GetAsyncWrap() override; + + + // Implement StreamResource: + int ReadStart() override; // Exposed to JS + int ReadStop() override; // Exposed to JS int DoShutdown(ShutdownWrap* req_wrap) override; int DoWrite(WriteWrap* w, uv_buf_t* bufs, @@ -77,14 +80,18 @@ class TLSWrap : public AsyncWrap, // Reset error_ string to empty. Not related to "clear text". void ClearError() override; + + // Called by the done() callback of the 'newSession' event. void NewSessionDoneCb(); + // Implement MemoryRetainer: void MemoryInfo(MemoryTracker* tracker) const override; - SET_MEMORY_INFO_NAME(TLSWrap) SET_SELF_SIZE(TLSWrap) protected: + // Alternative to StreamListener::stream(), that returns a StreamBase instead + // of a StreamResource. inline StreamBase* underlying_stream() { return static_cast(stream_); } @@ -136,13 +143,11 @@ class TLSWrap : public AsyncWrap, } } - AsyncWrap* GetAsyncWrap() override; - bool IsIPCPipe() override; - - // Resource implementation - void OnStreamAfterWrite(WriteWrap* w, int status) override; + // Implement StreamListener: + // Returns buf that points into enc_in_. uv_buf_t OnStreamAlloc(size_t size) override; void OnStreamRead(ssize_t nread, const uv_buf_t& buf) override; + void OnStreamAfterWrite(WriteWrap* w, int status) override; v8::Local GetSSLError(int status, int* err, std::string* msg); @@ -153,7 +158,6 @@ class TLSWrap : public AsyncWrap, static void SetVerifyMode(const v8::FunctionCallbackInfo& args); static void EnableSessionCallbacks( const v8::FunctionCallbackInfo& args); - static void EnableTrace(const v8::FunctionCallbackInfo& args); static void EnableCertCb(const v8::FunctionCallbackInfo& args); static void DestroySSL(const v8::FunctionCallbackInfo& args); static void GetServername(const v8::FunctionCallbackInfo& args);