From c7d859e756794fdc1460444eb238affd432b7fd6 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 17 Jun 2020 18:06:29 -0700 Subject: [PATCH] quic: refactor and improve ipv6Only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ignore `ipv6Only: true` when binding to `'udp4'` (this differs from dgram which will still attempt to apply the flag and will fail during bind). Improve the test so that it should work consistently. Signed-off-by: James M Snell PR-URL: https://github.com/nodejs/node/pull/33935 Reviewed-By: Daniel Bevenius Reviewed-By: Tobias Nießen Reviewed-By: Anna Henningsen --- doc/api/quic.md | 21 ++++++++++---- lib/internal/quic/core.js | 6 ++-- test/parallel/test-quic-ipv6only.js | 44 +++++++++++++++++------------ 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/doc/api/quic.md b/doc/api/quic.md index fb00f441530f2f..f27ad221548772 100644 --- a/doc/api/quic.md +++ b/doc/api/quic.md @@ -261,8 +261,11 @@ added: REPLACEME to an IP address. * `port` {number} The local port to bind to. * `type` {string} Either `'udp4'` or `'upd6'` to use either IPv4 or IPv6, - respectively. - * `ipv6Only` {boolean} + respectively. **Default**: `'udp4'`. + * `ipv6Only` {boolean} If `type` is `'udp6'`, then setting `ipv6Only` to + `true` will disable dual-stack support on the UDP binding -- that is, + binding to address `'::'` will not make `'0.0.0.0'` be bound. The option + is ignored if `type` is `'udp4'`. **Default**: `false`. * `lookup` {Function} A custom DNS lookup function. Default `dns.lookup()`. * `maxConnections` {number} The maximum number of total active inbound connections. @@ -1387,8 +1390,11 @@ added: REPLACEME to an IP address. * `port` {number} The local port to bind to. * `type` {string} Either `'udp4'` or `'upd6'` to use either IPv4 or IPv6, - respectively. - * `ipv6Only` {boolean} + respectively. **Default**: `'udp4'`. + * `ipv6Only` {boolean} If `type` is `'udp6'`, then setting `ipv6Only` to + `true` will disable dual-stack support on the UDP binding -- that is, + binding to address `'::'` will not make `'0.0.0.0'` be bound. The option + is ignored if `type` is `'udp4'`. **Default**: `false`. * Returns: {QuicEndpoint} Creates and adds a new `QuicEndpoint` to the `QuicSocket` instance. @@ -1519,7 +1525,10 @@ added: REPLACEME `SSL_OP_CIPHER_SERVER_PREFERENCE` to be set in `secureOptions`, see [OpenSSL Options][] for more information. * `idleTimeout` {number} - * `ipv6Only` {boolean} + * `ipv6Only` {boolean} If `type` is `'udp6'`, then setting `ipv6Only` to + `true` will disable dual-stack support on the UDP binding -- that is, + binding to address `'::'` will not make `'0.0.0.0'` be bound. The option + is ignored if `type` is `'udp4'`. **Default**: `false`. * `key` {string|string[]|Buffer|Buffer[]|Object[]} Private keys in PEM format. PEM allows the option of private keys being encrypted. Encrypted keys will be decrypted with `options.passphrase`. Multiple keys using different @@ -1578,7 +1587,7 @@ added: REPLACEME `QuicClientSession` object. * `type`: {string} Identifies the type of UDP socket. The value must either be `'udp4'`, indicating UDP over IPv4, or `'udp6'`, indicating UDP over - IPv6. Defaults to `'udp4'`. + IPv6. **Default**: `'udp4'`. Create a new `QuicClientSession`. This function can be called multiple times to create sessions associated with different endpoints on the same diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index e4a378c4820da2..6e3eab03b7f6f9 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -667,6 +667,7 @@ class QuicEndpoint { this.#lookup = lookup || (type === AF_INET6 ? lookup6 : lookup4); this.#port = port; this.#reuseAddr = !!reuseAddr; + this.#type = type; this.#udpSocket = dgram.createSocket(type === AF_INET6 ? 'udp6' : 'udp4'); // kUDPHandleForTesting is only used in the Node.js test suite to @@ -688,7 +689,7 @@ class QuicEndpoint { const obj = { address: this.address, fd: this.#fd, - type: this.#type + type: this.#type === AF_INET6 ? 'udp6' : 'udp4' }; return `QuicEndpoint ${util.format(obj)}`; } @@ -726,9 +727,10 @@ class QuicEndpoint { // what conditions does this happen? return; } + const flags = (this.#reuseAddr ? UV_UDP_REUSEADDR : 0) | - (this.#ipv6Only ? UV_UDP_IPV6ONLY : 0); + (this.#type === AF_INET6 && this.#ipv6Only ? UV_UDP_IPV6ONLY : 0); const ret = udpHandle.bind(ip, this.#port, flags); if (ret) { diff --git a/test/parallel/test-quic-ipv6only.js b/test/parallel/test-quic-ipv6only.js index e9f4d44963178a..e4846158208bbc 100644 --- a/test/parallel/test-quic-ipv6only.js +++ b/test/parallel/test-quic-ipv6only.js @@ -16,26 +16,24 @@ const { once } = require('events'); const kALPN = 'zzz'; -// Setting `type` to `udp4` while setting `ipv6Only` to `true` is possible -// and it will throw an error. -{ +// Setting `type` to `udp4` while setting `ipv6Only` to `true` is possible. +// The ipv6Only setting will be ignored. +async function ipv4() { const server = createQuicSocket({ endpoint: { type: 'udp4', ipv6Only: true - } }); - - server.on('error', common.mustCall((err) => { - assert.strictEqual(err.code, 'EINVAL'); - assert.strictEqual(err.message, 'bind EINVAL 0.0.0.0'); - })); - + } + }); + server.on('error', common.mustNotCall()); server.listen({ key, cert, ca, alpn: kALPN }); + await once(server, 'ready'); + server.close(); } -// Connecting ipv6 server by "127.0.0.1" should work when `ipv6Only` -// is set to `false`. -(async () => { +// Connecting to ipv6 server using "127.0.0.1" should work when +// `ipv6Only` is set to `false`. +async function ipv6() { const server = createQuicSocket({ endpoint: { type: 'udp6', @@ -54,6 +52,7 @@ const kALPN = 'zzz'; const session = client.connect({ address: common.localhostIPv4, port: server.endpoints[0].address.port, + ipv6Only: true, }); await once(session, 'secure'); @@ -70,11 +69,11 @@ const kALPN = 'zzz'; once(client, 'close'), once(server, 'close') ]); -})().then(common.mustCall()); +} // When the `ipv6Only` set to `true`, a client cann't connect to it // through "127.0.0.1". -(async () => { +async function ipv6Only() { const server = createQuicSocket({ endpoint: { type: 'udp6', @@ -87,10 +86,13 @@ const kALPN = 'zzz'; await once(server, 'ready'); + // This will attempt to connect to the ipv4 localhost address + // but should fail as the connection idle timeout expires. const session = client.connect({ address: common.localhostIPv4, port: server.endpoints[0].address.port, idleTimeout: common.platformTimeout(1), + ipv6Only: true, }); session.on('secure', common.mustNotCall()); @@ -104,11 +106,11 @@ const kALPN = 'zzz'; once(client, 'close'), once(server, 'close') ]); -})(); +} // Creating the QuicSession fails when connect type does not match the // the connect IP address... -(async () => { +async function mismatch() { const server = createQuicSocket({ endpoint: { type: 'udp6' } }); const client = createQuicSocket({ client: { key, cert, ca, alpn: kALPN } }); @@ -137,4 +139,10 @@ const kALPN = 'zzz'; once(client, 'close'), once(server, 'close') ]); -})().then(common.mustCall()); +} + +ipv4() + .then(ipv6) + .then(ipv6Only) + .then(mismatch) + .then(common.mustCall());