src: add security warning when inspector is running on public network

[slonka]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Hi, this is an attempt to fix #23444 it's not complete but I wanted to know if I'm even heading in the right direction.

[sam-github]

I'm not sure its worth distinguishing between private and public networks - most "public" networks, such as Cafes, are going to use private IPs.

The localhost vs external access distinction is, I think, the most important, and a dire warning about external machines being able to access the inspector port should get the point across.

[devsnek]

like sam said, I think the actual distinction here needs to be external vs internal interface

[mscdex]

What about other loopback addresses? What about IPv6?

[slonka]

This is just a guard against default value that is here, even when the user does not run node with inspect parameter

[mscdex]

We already have this kind of functionality in core, it would be better to just reuse that.

[slonka]

Do you mind pointing out where it is?

[Drieger]

@slonka I believe you can find this kind of validation on internal/net.js

node/lib/internal/net.js

Lines 25 to 37 in 2f1c356

	function isIPv4(s) {
	return IPv4Reg.test(s);
	}
	function isIPv6(s) {
	return IPv6Reg.test(s);
	}
	function isIP(s) {
	if (isIPv4(s)) return 4;
	if (isIPv6(s)) return 6;
	return 0;
	}

[mscdex]

I agree that this should instead be about loopback vs non-loopback. Also as mentioned inline, there is missing IPv6 support and missing support for other loopback addresses.

[ChALkeR]

@mscdex
Re: ipv6 — yes, that's mostly my mistake, I left it out completely in the initial description in the issue.

@mscdex @devsnek
Re: loopback — ok, how about this:

• If the interface is loopback (that also corrsponds to 127.0.0.0/8 subnet for the ipv4 adresses) — no warning.
• Otherwise, for known private ipv4 adresses, show warning A.
• Otherwise, for all the rest and ipv6, show warning B.

I supposed the difference between warnings A and B be subtle, just to provide more details to users, as in some situations A does not pose a treat and is intended behavior.

[ChALkeR]

This warning does not describe the actual implications and doesn't tell the user what the actual problem is.

How about

In case if port ${port} is not filtered on your machine by a firewall, anyone in the same
private network ${subnet} could access your setup and perform a remote code execution.

Subnet could be taken from os.networkInterfaces().

[gireeshpunathil]

@slonka - can you address the review comments?

[slonka]

@slonka - can you address the review comments?

I will after new years, sorry for the delay.

[jasnell]

There's been no further activity here. Recommending closing if it does not move forward soon

[jasnell]

Closing. Can reopen if someone decides to pick this back up