Skip to content
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

DNS lookup should try to avoid IPv6 link-local addresses #51732

Open
vbraun opened this issue Feb 12, 2024 · 3 comments
Open

DNS lookup should try to avoid IPv6 link-local addresses #51732

vbraun opened this issue Feb 12, 2024 · 3 comments
Labels
feature request Issues that request new features to be added to Node.js.

Comments

@vbraun
Copy link

vbraun commented Feb 12, 2024

What is the problem this feature will solve?

Naive calls to createServer happily resolve host names to link-local addresses, which then fail to listen:

$ node -e "net.createServer(c=>console.log).listen(5555,'zen')"
node:events:492
      throw er; // Unhandled 'error' event
      ^

Error: listen EINVAL: invalid argument fe80::2d8:xxxx:xxxx:xxxx:5555
    at Server.setupListenHandle [as _listen2] (node:net:1855:21)
    at listenInCluster (node:net:1920:12)
    at GetAddrInfoReqWrap.doListen [as callback] (node:net:2069:7)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:109:8)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1899:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'EINVAL',
  errno: -22,
  syscall: 'listen',
  address: 'fe80::2d8:xxxx:xxxx:xxxx',
  port: 5555
}

Node.js v20.10.0

What is the feature you are proposing to solve the problem?

DNS lookup randomly picks the first available entry

> dns.lookup('zen', { verbatim: true }, console.info)
GetAddrInfoReqWrap {
  callback: [Function: info],
  family: 0,
  hostname: 'zen',
  oncomplete: [Function: onlookup]
}
> null fe80::2d8:xxxx:xxxx:xxxx 6

if there are multiple addresses returned by getaddrinfo

> dns.lookup('zen', { verbatim: true, all: true }, console.info)
GetAddrInfoReqWrap {
  callback: [Function: info],
  family: 0,
  hostname: 'zen',
  oncomplete: [Function: onlookupall]
}
> null [
  { address: 'fe80::2d8:xxxx:xxxx:xxxx', family: 6 },
  { address: '2a02:8109:xxxx:xxxx:2d8:xxxx:xxxx:xxxx', family: 6 },
  { address: '192.168.178.3', family: 4 },
  { address: '192.168.122.1', family: 4 }

Instead, it should pick the first non-link-local address. It is extremely unlikely that you want to listen to a link-local address (starting with fe80::). And in the unlikely case that you do, you certainly would call with options.all to get all resolutions.

What alternatives have you considered?

We could alter all client code to always add options.all and filter, but it seems like node is the correct way to fix this once and for all.

Related issues

@vbraun vbraun added the feature request Issues that request new features to be added to Node.js. label Feb 12, 2024
@vbraun vbraun changed the title DNS lookup should try to avoid link-local addresses DNS lookup should try to avoid IPv6 link-local addresses Feb 13, 2024
@ryker-uptycs
Copy link

I wish to take this up

@marco-ippolito
Copy link
Member

I guess it can be fixed with something like this:

diff --git a/lib/dns.js b/lib/dns.js
index 681f0aa3e5..de5bb73d39 100644
--- a/lib/dns.js
+++ b/lib/dns.js
@@ -24,6 +24,7 @@
 const {
   ObjectDefineProperties,
   ObjectDefineProperty,
+  StringPrototypeStartsWith,
   Symbol,
 } = primordials;
 
@@ -127,6 +128,14 @@ function onlookupall(err, addresses) {
     };
   }
 
+  if (addresses.length > 1) {
+    const firstAddr = addresses[0];
+    // Skip ipv6 link local address
+    if (firstAddr.family === 6 && StringPrototypeStartsWith(firstAddr.address, 'fe80::')) {
+      addresses.push(addresses.shift());
+    }
+  }
+
   this.callback(null, addresses);
   if (this[kPerfHooksDnsLookupContext] && hasObserver('dns')) {
     stopPerf(this, kPerfHooksDnsLookupContext, { detail: { addresses } });

@wathika-eng
Copy link

Guess this is my error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js.
Projects
Status: Pending Triage
Development

No branches or pull requests

4 participants