-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
tls: stronger validation for 'servername' in server.addContext #33943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
00b7b9f
6532918
38d50aa
9149d3b
cb77037
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -545,18 +545,19 @@ called: | |
* `tlsSocket` {tls.TLSSocket} The `tls.TLSSocket` instance from which the | ||
error originated. | ||
|
||
### `server.addContext(hostname, context)` | ||
### `server.addContext(servername, context)` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think
It is correct that TLS does not allow IP addresses in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see the point, but... the extension has never changed and "server name" === "host name server name" in everyone's minds. I personally don't think it's worth it to expose this detail to users, nor have I seen any other API do it. So I'm personally +1 for naming the parameter |
||
<!-- YAML | ||
added: v0.5.3 | ||
--> | ||
|
||
* `hostname` {string} A SNI host name or wildcard (e.g. `'*'`) | ||
* `servername` {string} A SNI server name or wildcard (e.g. `'*'`). Must not be | ||
an IP address. | ||
* `context` {Object} An object containing any of the possible properties | ||
from the [`tls.createSecureContext()`][] `options` arguments (e.g. `key`, | ||
`cert`, `ca`, etc). | ||
|
||
The `server.addContext()` method adds a secure context that will be used if | ||
the client request's SNI name matches the supplied `hostname` (or wildcard). | ||
the client request's SNI name matches the supplied `servername` (or wildcard). | ||
|
||
### `server.address()` | ||
<!-- YAML | ||
|
@@ -1953,7 +1954,7 @@ where `secureSocket` has the same API as `pair.cleartext`. | |
[`net.Server.address()`]: net.html#net_server_address | ||
[`net.Server`]: net.html#net_class_net_server | ||
[`net.Socket`]: net.html#net_class_net_socket | ||
[`server.addContext()`]: #tls_server_addcontext_hostname_context | ||
[`server.addContext()`]: #tls_server_addcontext_servername_context | ||
[`server.getTicketKeys()`]: #tls_server_getticketkeys | ||
[`server.listen()`]: net.html#net_server_listen | ||
[`server.setTicketKeys()`]: #tls_server_setticketkeys_keys | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,13 +60,13 @@ const { | |
ERR_INVALID_ARG_TYPE, | ||
ERR_INVALID_ARG_VALUE, | ||
ERR_INVALID_CALLBACK, | ||
ERR_MISSING_ARGS, | ||
ERR_MULTIPLE_CALLBACK, | ||
ERR_SOCKET_CLOSED, | ||
ERR_TLS_DH_PARAM_SIZE, | ||
ERR_TLS_HANDSHAKE_TIMEOUT, | ||
ERR_TLS_INVALID_CONTEXT, | ||
ERR_TLS_RENEGOTIATION_DISABLED, | ||
ERR_TLS_REQUIRED_SERVER_NAME, | ||
ERR_TLS_SESSION_ATTACK, | ||
ERR_TLS_SNI_FROM_SERVER, | ||
ERR_TLS_INVALID_STATE | ||
|
@@ -1411,7 +1411,19 @@ Server.prototype.setOptions = deprecate(function(options) { | |
// SNI Contexts High-Level API | ||
Server.prototype.addContext = function(servername, context) { | ||
if (!servername) { | ||
throw new ERR_TLS_REQUIRED_SERVER_NAME(); | ||
throw new ERR_MISSING_ARGS('servername'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes it semver-major, I think? (Meaning that it cannot land on any of the active release lines.) |
||
} | ||
|
||
if (typeof servername !== 'string') { | ||
throw new ERR_INVALID_ARG_TYPE('servername', 'string', servername); | ||
} | ||
|
||
if (net.isIP(servername)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, what happens if |
||
throw new ERR_INVALID_ARG_VALUE( | ||
'servername', | ||
servername, | ||
'must not be an IP address' | ||
); | ||
} | ||
|
||
const re = new RegExp('^' + | ||
|
Uh oh!
There was an error while loading. Please reload this page.