From 628469c7e1327fe6b3369c7824d7bcbb1b70f9c1 Mon Sep 17 00:00:00 2001 From: Giovanni Bucci Date: Fri, 23 Aug 2024 11:11:43 +0200 Subject: [PATCH] net: exclude ipv6 loopback addresses from server.listen Fixes: https://github.com/nodejs/node/issues/51732 PR-URL: https://github.com/nodejs/node/pull/54264 Reviewed-By: Tim Perry Reviewed-By: Paolo Insogna Reviewed-By: Matteo Collina --- lib/net.js | 39 +++++- src/cares_wrap.cc | 21 +++ ...er-close-before-calling-lookup-callback.js | 2 +- .../test-net-server-listen-ipv6-link-local.js | 131 ++++++++++++++++++ 4 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 test/sequential/test-net-server-listen-ipv6-link-local.js diff --git a/lib/net.js b/lib/net.js index 4de6f9c5f6f23a..be21c566610286 100644 --- a/lib/net.js +++ b/lib/net.js @@ -62,6 +62,7 @@ const { UV_ECANCELED, UV_ETIMEDOUT, } = internalBinding('uv'); +const { convertIpv6StringToBuffer } = internalBinding('cares_wrap'); const { Buffer } = require('buffer'); const { ShutdownWrap } = internalBinding('stream_wrap'); @@ -2118,19 +2119,51 @@ Server.prototype.listen = function(...args) { throw new ERR_INVALID_ARG_VALUE('options', options); }; +function isIpv6LinkLocal(ip) { + if (!isIPv6(ip)) { return false; } + + const ipv6Buffer = convertIpv6StringToBuffer(ip); + const firstByte = ipv6Buffer[0]; // The first 8 bits + const secondByte = ipv6Buffer[1]; // The next 8 bits + + // The link-local prefix is `1111111010`, which in hexadecimal is `fe80` + // First 8 bits (firstByte) should be `11111110` (0xfe) + // The next 2 bits of the second byte should be `10` (0x80) + + const isFirstByteCorrect = (firstByte === 0xfe); // 0b11111110 == 0xfe + const isSecondByteCorrect = (secondByte & 0xc0) === 0x80; // 0b10xxxxxx == 0x80 + + return isFirstByteCorrect && isSecondByteCorrect; +} + +function filterOnlyValidAddress(addresses) { + // Return the first non IPV6 link-local address if present + for (const address of addresses) { + if (!isIpv6LinkLocal(address.address)) { + return address; + } + } + + // Otherwise return the first address + return addresses[0]; +} + function lookupAndListen(self, port, address, backlog, exclusive, flags) { if (dns === undefined) dns = require('dns'); const listeningId = self._listeningId; - dns.lookup(address, function doListen(err, ip, addressType) { + + dns.lookup(address, { all: true }, (err, addresses) => { if (listeningId !== self._listeningId) { return; } if (err) { self.emit('error', err); } else { - addressType = ip ? addressType : 4; - listenInCluster(self, ip, port, addressType, + const validAddress = filterOnlyValidAddress(addresses); + const family = validAddress?.family || 4; + + listenInCluster(self, validAddress.address, port, family, backlog, undefined, exclusive, flags); } }); diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 26877f3ddd8f69..436b2e3e002d81 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1566,6 +1566,24 @@ void CanonicalizeIP(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(val); } +void ConvertIpv6StringToBuffer(const FunctionCallbackInfo& args) { + Isolate* isolate = args.GetIsolate(); + node::Utf8Value ip(isolate, args[0]); + unsigned char dst[16]; // IPv6 addresses are 128 bits (16 bytes) + + if (uv_inet_pton(AF_INET6, *ip, dst) != 0) { + isolate->ThrowException(v8::Exception::Error( + String::NewFromUtf8(isolate, "Invalid IPv6 address").ToLocalChecked())); + return; + } + + Local buffer = + node::Buffer::Copy( + isolate, reinterpret_cast(dst), sizeof(dst)) + .ToLocalChecked(); + args.GetReturnValue().Set(buffer); +} + void GetAddrInfo(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -1902,6 +1920,8 @@ void Initialize(Local target, SetMethod(context, target, "getaddrinfo", GetAddrInfo); SetMethod(context, target, "getnameinfo", GetNameInfo); SetMethodNoSideEffect(context, target, "canonicalizeIP", CanonicalizeIP); + SetMethodNoSideEffect( + context, target, "convertIpv6StringToBuffer", ConvertIpv6StringToBuffer); SetMethod(context, target, "strerror", StrError); @@ -1995,6 +2015,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(GetAddrInfo); registry->Register(GetNameInfo); registry->Register(CanonicalizeIP); + registry->Register(ConvertIpv6StringToBuffer); registry->Register(StrError); registry->Register(ChannelWrap::New); diff --git a/test/parallel/test-net-server-close-before-calling-lookup-callback.js b/test/parallel/test-net-server-close-before-calling-lookup-callback.js index 58cfc5504c04fa..0c42639523288f 100644 --- a/test/parallel/test-net-server-close-before-calling-lookup-callback.js +++ b/test/parallel/test-net-server-close-before-calling-lookup-callback.js @@ -3,5 +3,5 @@ const common = require('../common'); const net = require('net'); // Process should exit because it does not create a real TCP server. -// Paas localhost to ensure create TCP handle asynchronously because It causes DNS resolution. +// Pass localhost to ensure create TCP handle asynchronously because it causes DNS resolution. net.createServer().listen(0, 'localhost', common.mustNotCall()).close(); diff --git a/test/sequential/test-net-server-listen-ipv6-link-local.js b/test/sequential/test-net-server-listen-ipv6-link-local.js new file mode 100644 index 00000000000000..56664c227cb5b7 --- /dev/null +++ b/test/sequential/test-net-server-listen-ipv6-link-local.js @@ -0,0 +1,131 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); +const dns = require('dns'); +const { mock } = require('node:test'); + +if (!common.hasIPv6) { + common.printSkipMessage('IPv6 support is required for this test'); + return; +} + +// Test on IPv6 Server, dns.lookup throws an error +{ + mock.method(dns, 'lookup', (hostname, options, callback) => { + callback(new Error('Mocked error')); + }); + const host = 'ipv6_link_local'; + + const server = net.createServer(); + + server.on('error', common.mustCall((e) => { + assert.strictEqual(e.message, 'Mocked error'); + })); + + server.listen(common.PORT + 2, host); +} + + +// Test on IPv6 Server, server.listen throws an error +{ + mock.method(dns, 'lookup', (hostname, options, callback) => { + if (hostname === 'ipv6_link_local') { + callback(null, [{ address: 'fe80::1', family: 6 }]); + } else { + dns.lookup.wrappedMethod(hostname, options, callback); + } + }); + const host = 'ipv6_link_local'; + + const server = net.createServer(); + + server.on('error', common.mustCall((e) => { + assert.strictEqual(e.address, 'fe80::1'); + assert.strictEqual(e.syscall, 'listen'); + })); + + server.listen(common.PORT + 2, host); +} + +// Test on IPv6 Server, picks 127.0.0.1 between that and a bunch of link-local addresses +{ + + mock.method(dns, 'lookup', (hostname, options, callback) => { + if (hostname === 'ipv6_link_local_with_many_entries') { + callback(null, [ + { address: 'fe80::1', family: 6 }, + { address: 'fe80::abcd:1234', family: 6 }, + { address: 'fe80::1ff:fe23:4567:890a', family: 6 }, + { address: 'fe80::200:5aee:feaa:20a2', family: 6 }, + { address: 'fe80::f2de:f1ff:fe2b:3c4b', family: 6 }, + { address: 'fe81::1', family: 6 }, + { address: 'fe82::abcd:1234', family: 6 }, + { address: 'fe83::1ff:fe23:4567:890a', family: 6 }, + { address: 'fe84::200:5aee:feaa:20a2', family: 6 }, + { address: 'fe85::f2de:f1ff:fe2b:3c4b', family: 6 }, + { address: 'fe86::1', family: 6 }, + { address: 'fe87::abcd:1234', family: 6 }, + { address: 'fe88::1ff:fe23:4567:890a', family: 6 }, + { address: 'fe89::200:5aee:feaa:20a2', family: 6 }, + { address: 'fe8a::f2de:f1ff:fe2b:3c4b', family: 6 }, + { address: 'fe8b::1', family: 6 }, + { address: 'fe8c::abcd:1234', family: 6 }, + { address: 'fe8d::1ff:fe23:4567:890a', family: 6 }, + { address: 'fe8e::200:5aee:feaa:20a2', family: 6 }, + { address: 'fe8f::f2de:f1ff:fe2b:3c4b', family: 6 }, + { address: 'fea0::1', family: 6 }, + { address: 'febf::abcd:1234', family: 6 }, + { address: 'febf:ffff:ffff:ffff:ffff:ffff:ffff:ffff', family: 6 }, + { address: '127.0.0.1', family: 4 }, + ]); + } else { + dns.lookup.wrappedMethod(hostname, options, callback); + } + }); + + const host = 'ipv6_link_local_with_many_entries'; + + const server = net.createServer(); + + server.on('error', common.mustNotCall()); + + server.listen(common.PORT + 3, host, common.mustCall(() => { + const address = server.address(); + assert.strictEqual(address.address, '127.0.0.1'); + assert.strictEqual(address.port, common.PORT + 3); + assert.strictEqual(address.family, 'IPv4'); + server.close(); + })); +} + + +// Test on IPv6 Server, picks ::1 because the other address is a link-local address +{ + + const host = 'ipv6_link_local_with_double_entry'; + const validIpv6Address = '::1'; + + mock.method(dns, 'lookup', (hostname, options, callback) => { + if (hostname === 'ipv6_link_local_with_double_entry') { + callback(null, [ + { address: 'fe80::1', family: 6 }, + { address: validIpv6Address, family: 6 }, + ]); + } else { + dns.lookup.wrappedMethod(hostname, options, callback); + } + }); + + const server = net.createServer(); + + server.on('error', common.mustNotCall()); + + server.listen(common.PORT + 4, host, common.mustCall(() => { + const address = server.address(); + assert.strictEqual(address.address, validIpv6Address); + assert.strictEqual(address.port, common.PORT + 4); + assert.strictEqual(address.family, 'IPv6'); + server.close(); + })); +}