Limit the number of closures created and functions called
throughout the lifecycle of TLSSocket, Server & connect.
PR-URL: https://github.com/nodejs/node/pull/15575
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Makes tls.connect() behave as documented, preferring options.path over
options.port. This makes it consistent with net.connect(), so the
included test demonstrates that both behave in this way.
Also, for consistency, noting the precedence of options.path in net
doc.
PR-URL: https://github.com/nodejs/node/pull/14564
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Allows TLS renegotiation to be disabled per `TLSSocket` instance.
Per HTTP/2, TLS renegotiation is forbidden after the initial
connection prefix is exchanged.
PR-URL: https://github.com/nodejs/node/pull/14239
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13476
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
In anticipation of stricter linting for indentation, remove instances of
extra indentation that will be flagged by the new rules.
PR-URL: https://github.com/nodejs/node/pull/14090
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13741
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Add more information to the "ECONNRESET" errors generated when the
socket hang ups before establishing the secure connection.
These kind of errors are really hard to troubleshoot without this info.
PR-URL: https://github.com/nodejs/node/pull/7476
Reviewed-By: Trevor Norris <trevnorris@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Yazhong Liu <yorkiefixer@gmail.com>
* Take RegExp creation out of cycles.
* Use test(), not match() in boolean context.
* Remove redundant RegExp parts.
PR-URL: https://github.com/nodejs/node/pull/13536
Reviewed-By: Anna Henningsen <anna@addaleax.net>
`net.connect()` and consequently `http.Agent` support custom DNS
`lookup` option. However, as we move to `https.Agent` - this option no
longer works because it is not proxied by `tls.connect`.
Fix this inconsistency by passing it down to `net.connect`.
PR-URL: https://github.com/nodejs/node/pull/12839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This partually reverts commit 4cdb0e89d8.
A nullptr check in TSLWrap::IsAlive() and the added test were left.
PR-URL: https://github.com/nodejs/node/pull/11947
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
rejectUnauthorized used to be false when the property was undefined or
null, quietly allowing client connections for which certificates have
been requested (requestCert is true) even when the client certificate
was not authorized (signed by a trusted CA). Change this so
rejectUnauthorized is always true unless it is explicitly set to false.
PR-URL: https://github.com/nodejs/node/pull/5923
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
TLSWrap object keeps a pointer reference to the underlying
TCPWrap object. This TCPWrap object could be closed and deleted
by the event-loop which leaves us with a dangling pointer.
So the TLSWrap object needs to track the "close" event on the
TCPWrap object.
PR-URL: https://github.com/nodejs/node/pull/11776
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
`TLSSocket` should not have a hard dependency on `tls.Server`, since it
may be running without it in cases like `STARTTLS`.
Fix: #10704
PR-URL: https://github.com/nodejs/node/pull/10706
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The first parameter to `util._extend` is the target object. Assigning
the target object to the result of `util._extend` is not necessary.
PR-URL: https://github.com/nodejs/node/pull/11364
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Add support to new tls.TLSSocket() to create a SecureContext object with
all its supported options, in the same way they are supported for all
the other APIs that need SecureContext objects.
Fix: https://github.com/nodejs/node/issues/10538
PR-URL: https://github.com/nodejs/node/pull/11005
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
The return value of loadSession is ultimately ignored, so don't fill it
in. This inches Node closer to 1.1.0 compatibility and is less code.
Also remove a comment which appears to have long since become invalid.
It dates to 048e0e77e0 when the SNI value
was actually extracted from the session.
This also fixes a segfault should d2i_SSL_SESSION fail to parse the
input and return NULL. Add a test for this case based on
test-tls-session-cache.js.
PR-URL: https://github.com/nodejs/node/pull/10882
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
The `exports` parameter is unnecessary.
PR-URL: https://github.com/nodejs/node/pull/10834
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <sam@strongloop.com>
Either the options or the listener argument to tls.createServer() was
optional, but not both. This makes no sense, so align the argument
checking and documentation with net.createServer(), which accepts the
same option sequence, and which tls.createServer() is modelled on.
PR-URL: https://github.com/nodejs/node/pull/9800
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Minor fix to favor strict equality in http_server.js and tls_wrap.js
to ensure accurate comparisons without type coercion.
PR-URL: https://github.com/nodejs/node/pull/9849
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.
Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.
To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.
`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.
PR-URL: https://github.com/nodejs/node/pull/9586
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The `.` character does not need to be escaped when it appears inside a
regular expression character class. This removes instances of
unnecessary escapes of the `.` character.
This also removes a few unnecessary escapes of the `(` and `)`
characters within character classes too.
PR-URL: https://github.com/nodejs/node/pull/9449
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: James Snell <jasnell@gmail.com>
In most cases, named functions match the variable or property to which
they are being assigned. That also seems to be the practice in a series
of PRs currently being evaluated that name currently-anonymous
functions.
This change applies that rule to instances in the code base that don't
comply with that practice.
This will be enforceable with a lint rule once we upgrade to ESLint
3.8.0.
PR-URL: https://github.com/nodejs/node/pull/9113
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Removes branch that would make TLSSocket emit '_tlsError' event if error
occured on handshake and control was not released, as it was never happening.
Addedd test for tls.Server to ensure it still emits 'tlsClientError' as expected.
Fixes: https://github.com/nodejs/node/issues/8803
PR-URL: https://github.com/nodejs/node/pull/8805
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Previously `TLSSocket#_emitTLSError` was used as an `error` event
handler. However that function can emit `error` event itself, so it is
not suitable for such use. Luckily the event can be emitted only when
the control is released, so this looping-error can't happen.
Replace the error handler for clarity and correctness.
PR-URL: https://github.com/nodejs/node/pull/8889
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This PR simplifies Server.prototype.listen, removing some redundancy and
inconsistency. Because listen and connect have a similar function signature,
normalizeConnectArgs can be reused for listen.
listenAfterLookup renamed to lookupAndListen for consistency with
lookupAndConnect, and moved out of Server.prototype.listen.
PR-URL: https://github.com/nodejs/node/pull/4039
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Glen Keane <glenkeane.94@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
When node began using the OneByte API (f150d56) it also switched to
officially supporting ISO-8859-1. Though at the time no new encoding
string was introduced.
Introduce the new encoding string 'latin1' to be more explicit. The
previous 'binary' and documented as an alias to 'latin1'. While many
tests have switched to use 'latin1', there are still plenty that do both
'binary' and 'latin1' checks side-by-side to ensure there is no
regression.
PR-URL: https://github.com/nodejs/node/pull/7111
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
There is no official way to figure out if the socket that you have on
hand is still connecting to the remote host. Introduce
`Socket#connecting`, which is essentially an unprefixed `_connecting`
property that we already had.
PR-URL: https://github.com/nodejs/node/pull/6404
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Introduce a new MACRO to check if the data is a String object and
update existing MACROs to include the actual object description to
be printed in case of an error.
PR-URL: https://github.com/nodejs/node/pull/3100
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fail early in require('crypto'), require('tls'),
require('https'), etc when crypto is not available
(rather than depending on an internal try/catch).
Add documentation for detecting when crypto is not available.
PR-URL: https://github.com/nodejs/node/pull/5611
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Several changes:
* Soft-Deprecate Buffer() constructors
* Add `Buffer.from()`, `Buffer.alloc()`, and `Buffer.allocUnsafe()`
* Add `--zero-fill-buffers` command line option
* Add byteOffset and length to `new Buffer(arrayBuffer)` constructor
* buffer.fill('') previously had no effect, now zero-fills
* Update the docs
PR-URL: https://github.com/nodejs/node/pull/4682
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Instead of leaking the arguments object by passing it as an
argument to a function, copy it's contents to a new array,
then pass the array. This allows V8 to optimize the function
that contains this code, improving performance.
PR-URL: https://github.com/nodejs/node/pull/4361
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
The role of `this.server` is now split between `this._server` and
`this.server`. Where the first one is used for counting active
connections of `net.Server`, and the latter one is just a public API for
users' consumption.
The reasoning for this is simple, `TLSSocket` instances wrap
`net.Socket` instances, thus both refer to the `net.Server` through the
`this.server` property. However, only one of them should be used for
`net.Server` connection count book-keeping, otherwise double-decrement
will happen on socket destruction.
Fix: #5083
PR-URL: https://github.com/nodejs/node/pull/5262
Reviewed-By: James M Snell <jasnell@gmail.com>
This is an intermediate fix for an issue of accessing `TLSWrap` fields
after the parent handle was destroyed. While `close` listener cleans up
this field automatically, it can be done even earlier at the
`TLSWrap.close` call.
Proper fix is going to be submitted and landed after this one.
Fix: #5108
PR-URL: https://github.com/nodejs/node/pull/5168
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
This commit adds a new method for TLS sockets that returns the
negotiated protocol version.
PR-URL: https://github.com/nodejs/node/pull/4995
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
With an indentation style of two spaces, it is not possible to indent
multiline variable declarations by four spaces. Instead, the var keyword
is used on every new line.
Use const instead of var where applicable for changed lines.
PR-URL: https://github.com/nodejs/io.js/pull/2286
Reviewed-By: Roman Reiss <me@silverwind.io>
`clientError` will have `http.Server`-specific behavior, and we don't
want to shadow it in `tls.Server`.
PR-URL: https://github.com/nodejs/node/pull/4557
Reviewed-By: Brian White <mscdex@mscdex.net>
use `arrow functions` instead of `bind(this)` in order to improve
performance through optimizations.
PR-URL: https://github.com/nodejs/node/pull/3622
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Add `secureContext` option to `tls.connect`. It is useful for caching
client certificates, key, and CA certificates.
PR-URL: https://github.com/nodejs/node/pull/4246
Reviewed-By: James M Snell <jasnell@gmail.com>
FIPS 140-2 disallows use of MD5, which is used to derive the
default sessionIdContext for tls.createServer().
PR-URL: https://github.com/nodejs/node/pull/3866
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
FIPS 140-2 disallows use of MD5, which is used to derive the
default sessionIdContext for tls.createServer().
PR-URL: https://github.com/nodejs/node/pull/3755
Reviewed-By: Fedor Indutny <fedor@indutny.com>
This commit fixes some error messages that are not consistent with
some general rules which most of the error messages follow.
PR-URL: https://github.com/nodejs/node/pull/3374
Reviewed-By: Roman Reiss <me@silverwind.io>
Check that tls.connect() fails in the expected way when passing in
invalid minDHSize options.
PR-URL: https://github.com/nodejs/node/pull/3629
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Anatoli Papirovski
Bryan English
James M Snell
Bidisha Pyne
Rich Trott
Ruben Bridgewater
José F. Romaniello
Vse Mozhet Byt
Fedor Indutny
Ruslan Bekenev
Trevor Norris
ghaiklor
jBarz
Sakthipriyan Vairamani (thefourtheye)
Sam Roberts
David Benjamin
Jackson Tian
Walter Beller-Morales
Mariusz 'koder' Chwalba
Teddy Katz
Jan Schär
Ben Noordhuis
Nathan Woltman
Brian White
Michaël Zasso
Minwoo Jung
Stefan Budeanu
micnic