Skip to content

Commit

Permalink
tls: add support for ALPN fallback when no ALPN protocol matches
Browse files Browse the repository at this point in the history
  • Loading branch information
pimterry committed Oct 19, 2022
1 parent ef17711 commit c906151
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 4 deletions.
11 changes: 11 additions & 0 deletions doc/api/errors.md
Expand Up @@ -2698,6 +2698,17 @@ This error represents a failed test. Additional information about the failure
is available via the `cause` property. The `failureType` property specifies
what the test was doing when the failure occurred.

<a id="ERR_TLS_ALPN_FALLBACK_WITHOUT_PROTOCOLS"></a>

### `ERR_TLS_ALPN_FALLBACK_WITHOUT_PROTOCOLS`

This error is thrown when creating a `TLSServer` if the TLS options sets
`enableALPNFallback` to `true` without providing an `ALPNProtocols` argument.

When `ALPNProtocols` is not provided, ALPN is skipped entirely, so the fallback
would not be functional. To enable ALPN for all protocols, using the fallback
in all cases, set `ALPNProtocols` to an empty array instead.

<a id="ERR_TLS_CERT_ALTNAME_FORMAT"></a>

### `ERR_TLS_CERT_ALTNAME_FORMAT`
Expand Down
8 changes: 8 additions & 0 deletions doc/api/tls.md
Expand Up @@ -2012,6 +2012,9 @@ where `secureSocket` has the same API as `pair.cleartext`.
<!-- YAML
added: v0.3.2
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/45076
description: The `options` parameter can now include `enableALPNFallback`.
- version: v19.0.0
pr-url: https://github.com/nodejs/node/pull/44031
description: If `ALPNProtocols` is set, incoming connections that send an
Expand Down Expand Up @@ -2042,6 +2045,11 @@ changes:
e.g. `0x05hello0x05world`, where the first byte is the length of the next
protocol name. Passing an array is usually much simpler, e.g.
`['hello', 'world']`. (Protocols should be ordered by their priority.)
* `enableALPNFallback`: {boolean} If `true`, if the `ALPNProtocols` option was
set and the client uses ALPN, but requests only protocols that are not do
not match the server's `ALPNProtocols`, the server will fall back to sending
an ALPN response accepting the first protocol the client suggested, instead
of closing the connection immediately with a fatal alert.
* `clientCertEngine` {string} Name of an OpenSSL engine which can provide the
client certificate.
* `enableTrace` {boolean} If `true`, [`tls.TLSSocket.enableTrace()`][] will be
Expand Down
7 changes: 7 additions & 0 deletions lib/_tls_wrap.js
Expand Up @@ -71,6 +71,7 @@ const {
ERR_INVALID_ARG_VALUE,
ERR_MULTIPLE_CALLBACK,
ERR_SOCKET_CLOSED,
ERR_TLS_ALPN_FALLBACK_WITHOUT_PROTOCOLS,
ERR_TLS_DH_PARAM_SIZE,
ERR_TLS_HANDSHAKE_TIMEOUT,
ERR_TLS_INVALID_CONTEXT,
Expand Down Expand Up @@ -716,6 +717,7 @@ TLSSocket.prototype._init = function(socket, wrap) {
ssl.onnewsession = onnewsession;
ssl.lastHandshakeTime = 0;
ssl.handshakes = 0;
ssl.enableALPNFallback = !!options.enableALPNFallback;

if (this.server) {
if (this.server.listenerCount('resumeSession') > 0 ||
Expand Down Expand Up @@ -1100,6 +1102,7 @@ function tlsConnectionListener(rawSocket) {
rejectUnauthorized: this.rejectUnauthorized,
handshakeTimeout: this[kHandshakeTimeout],
ALPNProtocols: this.ALPNProtocols,
enableALPNFallback: this.enableALPNFallback,
SNICallback: this[kSNICallback] || SNICallback,
enableTrace: this[kEnableTrace],
pauseOnConnect: this.pauseOnConnect,
Expand Down Expand Up @@ -1198,6 +1201,10 @@ function Server(options, listener) {
this._contexts = [];
this.requestCert = options.requestCert === true;
this.rejectUnauthorized = options.rejectUnauthorized !== false;
this.enableALPNFallback = options.enableALPNFallback === true;
if (this.enableALPNFallback && !options.ALPNProtocols) {
throw new ERR_TLS_ALPN_FALLBACK_WITHOUT_PROTOCOLS();
}

if (options.sessionTimeout)
this.sessionTimeout = options.sessionTimeout;
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/errors.js
Expand Up @@ -1612,6 +1612,9 @@ E('ERR_TEST_FAILURE', function(error, failureType) {
this.cause = error;
return msg;
}, Error);
E('ERR_TLS_ALPN_FALLBACK_WITHOUT_PROTOCOLS',
'The enableALPNFallback TLS option cannot be set without ALPNProtocols',
TypeError);
E('ERR_TLS_CERT_ALTNAME_FORMAT', 'Invalid subject alternative name string',
SyntaxError);
E('ERR_TLS_CERT_ALTNAME_INVALID', function(reason, host, cert) {
Expand Down
20 changes: 16 additions & 4 deletions src/crypto/crypto_tls.cc
Expand Up @@ -39,6 +39,7 @@ using v8::Array;
using v8::ArrayBuffer;
using v8::ArrayBufferView;
using v8::BackingStore;
using v8::Boolean;
using v8::Context;
using v8::DontDelete;
using v8::Exception;
Expand Down Expand Up @@ -237,6 +238,11 @@ int SelectALPNCallback(
if (UNLIKELY(alpn_buffer.IsEmpty()) || !alpn_buffer->IsArrayBufferView())
return SSL_TLSEXT_ERR_NOACK;

bool alpn_fallback_enabled = w->object()->Get(
env->context(),
env->enable_alpn_fallback_string()
).ToLocalChecked().As<Boolean>()->Value();

ArrayBufferViewContents<unsigned char> alpn_protos(alpn_buffer);
int status = SSL_select_next_proto(
const_cast<unsigned char**>(out),
Expand All @@ -249,10 +255,16 @@ int SelectALPNCallback(
// Previous versions of Node.js returned SSL_TLSEXT_ERR_NOACK if no protocol
// match was found. This would neither cause a fatal alert nor would it result
// in a useful ALPN response as part of the Server Hello message.
// We now return SSL_TLSEXT_ERR_ALERT_FATAL in that case as per Section 3.2
// of RFC 7301, which causes a fatal no_application_protocol alert.
return status == OPENSSL_NPN_NEGOTIATED ? SSL_TLSEXT_ERR_OK
: SSL_TLSEXT_ERR_ALERT_FATAL;

// By default, we now return SSL_TLSEXT_ERR_ALERT_FATAL in that case as per
// Section 3.2 of RFC 7301, which causes a fatal no_application_protocol alert,
// unless the ALPNFallback option has been set, to enable OpenSSL's default
// protocol selection behavior, where the client's first preference is used.
if (status == OPENSSL_NPN_NEGOTIATED || alpn_fallback_enabled) {
return SSL_TLSEXT_ERR_OK;
} else {
return SSL_TLSEXT_ERR_ALERT_FATAL;
}
}

int TLSExtStatusCallback(SSL* s, void* arg) {
Expand Down
1 change: 1 addition & 0 deletions src/env_properties.h
Expand Up @@ -50,6 +50,7 @@
V(ack_string, "ack") \
V(address_string, "address") \
V(aliases_string, "aliases") \
V(enable_alpn_fallback_string, "enableALPNFallback") \
V(args_string, "args") \
V(asn1curve_string, "asn1Curve") \
V(async_ids_stack_string, "async_ids_stack") \
Expand Down
60 changes: 60 additions & 0 deletions test/parallel/test-tls-alpn-server-client.js
Expand Up @@ -206,6 +206,66 @@ function TestFatalAlert() {
}
}));
}));

TestALPNFallback();
}

function TestALPNFallback() {
const serverOptions = {
ALPNProtocols: ['b'],
enableALPNFallback: true
};

const clientOptions = [{
ALPNProtocols: ['a', 'b']
}, {
ALPNProtocols: ['a']
}];

runTest(clientOptions, serverOptions, function(results) {
// 'b' is selected by ALPN if available
checkResults(results[0],
{ server: { ALPN: 'b' },
client: { ALPN: 'b' } });

// 'a' is selected by ALPN if not, due to fallback
checkResults(results[1],
{ server: { ALPN: 'a' },
client: { ALPN: 'a' } });
});

TestEmptyALPNFallback();
}

function TestEmptyALPNFallback() {
const serverOptions = {
ALPNProtocols: [],
enableALPNFallback: true
};

const clientOptions = [{
ALPNProtocols: ['a', 'b']
}];

runTest(clientOptions, serverOptions, function(results) {
// 'a' is selected by ALPN, due to fallback with empty array
checkResults(results[0],
{ server: { ALPN: 'a' },
client: { ALPN: 'a' } });
});

TestFallbackWithoutProtocols();
}

function TestFallbackWithoutProtocols() {
assert.throws(() => {
tls.createServer({
enableALPNFallback: true
});
}, {
code: 'ERR_TLS_ALPN_FALLBACK_WITHOUT_PROTOCOLS',
message: 'The enableALPNFallback TLS option cannot be set without ALPNProtocols'
});
}

Test1();

0 comments on commit c906151

Please sign in to comment.