[Real world security] Http blacklist or similar

[JLLeitschuh]

There are certain things you may not want your server users able to poke around with. For example, most routers are running on 192.168.1.1. I really don't want people on my server able to poke around and access my router's admin console (I have a password but let's be honest here, how many people actually change the default password).

Many people running servers probably aren't using a hosting service and having the computer craft http module enabled currently gives players full access to the server's internal network.

Example:

http.get("http://192.168.1.1"):readAll()

This will dump the HTML of most server's router web interfaces.
It's a simple bit of code to go from here to tinkering with the server hosters router.

[Wojbie]

Hmm... This post makes sense. We have a http whitelist. It would make sense to have a blacklist that can be set by server owner too if they wanted to protect some addresses while letting rest of internet be free. Unless there is some magic way to blacklist one (or few) addresses using current implementation of whitelist.

[JLLeitschuh]

We also need to be wary of mDNS too. So things with *.local in the path should be disabled.
There should probably be a setting that disables local http requests by default that can be enabled if the server owner feels like they know what they are doing.

[paolobarbolini]

I think that RFC 1918 addresses should be blacklisted by default so that unexperienced users don't get hacked by computercraft computers on local servers or downloaded maps.

It should be possible to disable it so that contraptions like this one with a raspberry pi are still possible https://youtu.be/86Zj_59P52c

[SquidDev]

I rather like the way CCTweaks and "The computer mod which Must Not Be Named" handle this: you can specify a blacklist and whitelist composed of host names and IP address, which can be in in the CIDR format. (so example.com, 127.0.0.0, 127.0.0.0/8`).

127.0.0.0/8, 10.0.0.0/8, 192.168.0.0/16, 172.16.0.0/12 should be sufficient to block all local domains. When performing your HTTP request, you'd also need to resolve the host name to an IP address beforehand, so you can double check it hasn't been blacklisted.

[JLLeitschuh]

It should be possible to disable it so that contraptions like this one with a raspberry pi are still possible https://youtu.be/86Zj_59P52c

I totally agree with this. That's really awesome. But yes, in the general use case it would be good to keep server owners safe from having their players hack their network.

[dmarcuse]

CC should also resolve hostnames before checking against the black/whitelist, otherwise you may still be able to access local devices if the server is running avahi or similar.

[JLLeitschuh]

I agree, but the blacklist/whitelist needs to check the hostnames AND the resolved address, the admin could have declared the whitelist/blacklist with either.

Also, there needs to be a discussion about priority.

What takes priority? The whitelist or the blacklist?

[paolobarbolini]

I think we should proritize the most specific filter, for example if i blacklist 192.168.0.0/16 and i whitelist 192.168.1.7, i should be able to access 192.168.1.7 but not the rest of the /16 subnet.

[SquidDev]

I would personally do something like:

• If whitelist is empty then ensure the IP and Host isn't in the blacklist.
• If whitelist has elements and blacklist is empty, then ensure the IP or host is in the whitelist.
• Otherwise they are both non-empty. Here the whitelist takes priority.

You could potentially leave out the last step, but I'd consider it useful - as @paolobarbolini says, it can be used to allow a limited number of local ips, while still blocking the rest.

[JLLeitschuh]

Looks like the InetAddress library in java covers this use case nicely.
I'm working on a PR that includes a few unit tests to make sure this works correctly.

if (InetAddress.getByName(url.getHost()).isSiteLocalAddress()) {
  // Do something
}

[SquidDev]

@JLLeitschuh It is worth noting that InetAddress.getByName will block the thread, which isn't really desirable. For requests this is easy to fix - you can just move the check inside the request thread. However, for http.checkURL you'd have to make it run on a separate thread and wait for an event.

From a configuration point of view, I'd rather the user can specify a whole list of IP addresses/ranges to block, rather than just using isSiteLocalAddress(). For IPv4 it's just 10/8, 172.16/12 and 192.168/16. Ipv6 seems to be fec0:/120.

[JLLeitschuh]

I'm not sure what you mean by "run on a separate thread and wait for an event".
How do I await an event to return a method call made in lua?
Is there an example somewhere?

[Restioson]

If this is implemented, please just keep in mind that routers can also be on 192.168.0.1 (mine is)

[dmarcuse]

@Restioson that would be covered by 192.168.0.0/16 as mentioned above.

[Restioson]

@apemanzilla ah OK. I'm not really familiar with that syntax. I'll look it up. Sorry!

[EngineerCoding]

The flows should be like this:

1. Check if url/ip/whatever is on whitelist, serve if in this list.
2. Check if url/ip/whatever is on blacklist, don't serve if in list.
3. Check if only the whitelist is allowed (a boolean flag added to the settings), if so don't serve. Otherwise serve.

With this one can blacklist entire ranges and just enable just one specific IP. This is how Pihole does it anyway (without the boolean flag). It would be really clear to the user editing the config since the flow is fixed, and not based on empty/filled lists as @SquidDev suggests.

[dan200]

If someone wants to write a PR for this, I suggest the following:

• Allow the specification of IP wildcards in http_whitelist
• Add http_blacklist, supporting both IP and domain wildcards
• Set the default value of http_blacklist to "192.168.."
  If we only supported blocking IPs, i'd prefer the /16 syntax apemanzilla suggests, but for this context it's better to be consistent with what's already there as it's clearer and more obvious to config editors who aren't network engineers.

Make the filtering logic work as follows:
if( (address matches whitelist) && !(address matches blacklist) ) { serve(); } else { error(); }
Remember that the default whitelist contains "*", so I don't think we need a third flag or anything more complicated

[dmarcuse]

@dan200 other private subnets (10.0.0.0/24 and 172.16.0.0/20) should also be blacklisted by default, they're often used in enterprise networks.

I'd suggest that we allow both 192.168.*.* and 192.168.0.0/16 formats to be used for IPs, so you can use the more precise CIDR format or the simpler wildcard format.

I can start working on a PR for this.

[dan200]

Adding those ranges makes sense. Maybe add 127.0.0.1 as well?

…

On 7 May 2017 at 18:56, apemanzilla ***@***.***> wrote: @dan200 <https://github.com/dan200> other private subnets (10.0.0.0/24 and 172.16.0.0/20) should also be blacklisted by default, they're often used in enterprise networks. I'd suggest that we allow both 192.168.*.* and 192.168.0.0/16 formats to be used, so you can use the more precise CIDR format or the simpler wildcard format. I can start working on a PR for this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#170 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB3bbGSLh7eM_Nwt3dNpStNpJUYXU1Z0ks5r3gXdgaJpZM4NNjCD> .

[dmarcuse]

127.0.0.0/8, but yeah good point. Are you OK with the wildcard and CIDR formats for IP ranges?

[dan200]

Yeah, if supporting both is easy, do it.

[dmarcuse]

Alright, I'll start on this in a bit.