kintesh / containerise

Firefox extension to automatically open websites in a container
MIT License
411 stars 55 forks source link

Generate better {domain} container names for IP address URLs #124

Closed LoveIsGrief closed 4 years ago

LoveIsGrief commented 4 years ago

When using {domain} as the name of default containers, before 192.168.178.1 would generate a container called "178". With this PR it now generates "192.168.178.1".

Resolves #119

ghost commented 4 years ago

Perhaps I've misunderstood, I've not had any use for IP address-based rules or anything like that..... but.... Isn't "178" most correct name, here? (emphasis on "most" since obviously there is no domain name if DNS is not used, so no domain name can ever be correct)

That is the subnet applicable to the 16bit non-routable (192.168.. aka 192.168.0.0/16) network. I'd expect 192.168.x.a to fall into the same group as 192.168.x.b 192.168.x.c 192.168.x.d etc, the distinguishing variable here being x. Likewise, I'd expect a "domain" of 'x' whenever using 10.x.. (aka 10.0.0.0/8) or 172.16.x.-172.31.x. (aka 172.16.0.0/12)

Using the IP you gave as an example, let's say I have two local servers, 192.168.178.1 and 192.168.178.2, I wouldn't expect them to be considered a different domain.

I notice the change also effects "TLD" and in that case it should simply be using the first octet since that's as "top level" as we go unless we want to count bits ;)

Obviously, the term "domain" doesn't apply if we're talking unresolved IP addresses, so wouldn't "subnet" be the most appropriate analog for the term? That being the case, is the current behaviour not most appropriate?

I feel like it was already working appropriately. Am I missing something here?

LoveIsGrief commented 4 years ago

I don't think it's working well now. 192.168.8.1, 201.193.8.20 and 10.0.8.1 would all be put into the same container 8, when they are most likely (unless the DNS or /etc/hosts) not the same host.

Guessing the prefix of an IP sounds like unnecessarily complicated code that will deliver confusing results, especially for the normal user. It would additionally make assumptions for the user as to which IPs should be put into the same container.
In that case, it would be better for the user to define the container and rules themselves.

ghost commented 4 years ago

192.168.8.1, 201.193.8.20 and 10.0.8.1 would all be put into the same container 8,

Of course yes you're right, that sucks. But still, it would make more sense if these "domains" were more like 192.168.8., and the last one 10.0.8., so that they're representative of the subnet.

For example I know of a home network who uses 10.0.0. for the internal IP's, and also has 10.0.1. for the 'guest' network, an open wifi network for friends which gets full internet access but can't communicate with other subnets, and 10.0.2.* for external devices (like power grid monitoring etc which need internet access but never communicate with internal devices)

Having a totally different "domain" for each of the devices would be very unwieldly.

I skipped the second example you gave, because unlike any of the other examples we've used, that's not a defined, private, non-routable network, so in that case, just using the entire IP would make sense as the domain name, because 201.193.8.20 and 201.193.8.21 might well be entirely unrelated.

LoveIsGrief commented 4 years ago

Then what do you suggest is done for people who don't want their internal subnets to be contained together? Or for people who want different internal subnets to get different containers? Or for some internal subnets to be grouped with external subnets?

If we start making assumptions about how people want their IP addresses to be grouped, it can get very complicated. Additionally, it might be very specific code for a small group of users. Letting them sort things out on their own - which they can easily so with container rules - is much easier.

If you really want to, create a PR and introduce a templating language for container names. We can then let @kintesh decide what he wants.

ghost commented 4 years ago

Then what do you suggest is done for people who don't want their internal subnets to be contained together? Or for people who want different internal subnets to get different containers? Or for some internal subnets to be grouped with external subnets?

That would be

a small group of users.

I'm not talking about doing something odd here. Separating addresses within subnet would be the edge case, just as it is to separate hosts within a domain.

If we start making assumptions about how people want their IP addresses to be grouped, it can get very complicated

Making assumptions is definitely a bad idea. I mean this whole change exists because there was an assumption that there would be a domain name to use.

But I'm not making any assumptions, I'm just saying that if we do not have a 'domain', since we are using no DNS, but we need to give it a name other than null or something that is often going to be a problem (such as the example you gave before), that name should be the subnet, since the subnet is the closest conceptual analog we have to the domain - and yes, only where the subnet is already defined, in the RFCs.

On the other hand, assuming that all IP addresses are to be grouped separately, is exactly what this change does, and since I could see it making things very complicated for the end-user, that's why I commented.

If you really want to, create a PR and introduce a templating language for container names. We can then let @kintesh decide what he wants.

The objective here is not competition by subjective decision, but discussion of objective best solution, for this PR. It definitely lives up to it's name of "better" names, as you've demonstrated there was a problem before where different subnets would get the same name.... and now I've demonstrated a problem where groups (subnets) are not going to be grouped (as 'domains').... but wouldn't "best" be best? Why do two PRs when only one is required :) We both agree on the problems here why not put our heads together in discussion to find the best solution to that problem?

bburky commented 4 years ago

Thanks, this is what I personally desired when I filed #119.

@xcasxcursex I personally don't want communication across IP addresses by default. I don't want a poorly coded IoT webpage on 192.168.0.123 to somehow CSRF attack my router at 192.168.0.1. Putting these in separate domains/containers should improve security. I would recommend the default behavior be to consider each IP address as a separate domain.

If someone does want a subnet to all be in one domain they should be able to use a custom rule like @192\.168\.0\.\d+$ , LAN to map that whole subnet to one container.

ghost commented 4 years ago

I don't want a poorly coded IoT webpage on 192.168.0.123 to somehow CSRF attack my router at 192.168.0.1. Putting these in separate domains/containers should improve security.

Putting them in separate subnets should do it too. That way you can sort out your routing so that you can isolate this untrusted device on your network (why would you allow an untrusted device on your network anyway?)

If someone does want a subnet to all be in one domain they should be able to use a custom rule like @192\.168\.0\.\d+$ , LAN to map that whole subnet to one container.

Why set it up to require custom rules for normal behaviour? (That's a rhetorical question)