Skip to content

Commit

Permalink
tls: in-line comments and other cleanups
Browse files Browse the repository at this point in the history
PR-URL: #25861
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Backport-PR-URL: #25968

Reviewed-By: Michael Dawson <[email protected]>
  • Loading branch information
sam-github authored and addaleax committed Feb 8, 2019
1 parent 7cf484c commit fb83f84
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 47 deletions.
31 changes: 24 additions & 7 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ const kSNICallback = Symbol('snicallback');

const noop = () => {};

// Server side times how long a handshake is taking to protect against slow
// handshakes being used for DoS.
function onhandshakestart(now) {
debug('onhandshakestart');

Expand Down Expand Up @@ -118,13 +120,19 @@ 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();
}

if (hello.sessionId.length <= 0 ||
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();
}
}
Expand Down Expand Up @@ -219,13 +227,17 @@ function onnewsessionclient(sessionId, session) {
}

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;
Expand Down Expand Up @@ -316,8 +328,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 it 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.
Expand All @@ -337,7 +353,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);

Expand Down Expand Up @@ -433,8 +449,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;
Expand Down Expand Up @@ -484,8 +500,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;

Expand All @@ -506,6 +522,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)
Expand Down Expand Up @@ -728,7 +745,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) {
Expand Down Expand Up @@ -779,7 +796,7 @@ function tlsConnectionListener(rawSocket) {
SNICallback: this[kSNICallback] || SNICallback
});

socket.on('secure', onSocketSecure);
socket.on('secure', onServerSocketSecure);

socket[kErrorEmitted] = false;
socket.on('close', onSocketClose);
Expand Down
48 changes: 10 additions & 38 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2643,47 +2643,19 @@ int SSLWrap<Base>::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;
}

Expand Down
6 changes: 4 additions & 2 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo<Value>& 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();
Expand All @@ -744,7 +744,9 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo<Value>& 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;
}

Expand Down
4 changes: 4 additions & 0 deletions src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ 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:
Expand All @@ -88,6 +90,8 @@ class TLSWrap : public AsyncWrap,
SET_SELF_SIZE(TLSWrap)

protected:
// Alternative to StreamListener::stream(), that returns a StreamBase instead
// of a StreamResource.
inline StreamBase* underlying_stream() {
return static_cast<StreamBase*>(stream_);
}
Expand Down

0 comments on commit fb83f84

Please sign in to comment.