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

The implicit blacklist is insufficient. #223

Closed
ydahhrk opened this issue Sep 6, 2016 · 7 comments
Closed

The implicit blacklist is insufficient. #223

ydahhrk opened this issue Sep 6, 2016 · 7 comments
Labels
Milestone

Comments

@ydahhrk
Copy link
Member

ydahhrk commented Sep 6, 2016

This applies to both Jool 3.4 and the upcoming 3.5, though the code was already fixed in 3.5's test branch.

As stated in --blacklist, SIIT Jool has an additional implicit blacklist that prevents translation of certain IPv4 addresses regardless of the EAMT and pool6.

Rather bafflingly, that list does not currently include recognizable directed broadcast. If pool6 has 64:ff9b::/96 and Jool is connected to 192.0.2.0/24, 192.0.2.255 is happily translated into 64:ff9b::192.0.2.255 and vice versa.

Also, much like the explicit blacklist, the implicit blacklist does not currently apply to the EAMT. Among other (predictable) consequences of this, it's causing the translator itself to lose contact with IPv4 if its IPv4 address is included in the EAMT. For example, if the EAMT contains EAM [0.0.0.0/0, 2001:db8::/96] and the SIIT machine has address 192.0.2.24, then an incoming packet towards 192.0.2.24 will be translated into 2001:db8::192.0.2.24 and likely end up dropped.

Thanks to Eduardo Montoya for reporting this.

ydahhrk added a commit that referenced this issue Sep 6, 2016
- Stop translating broacast.
- Apply implicit blacklist to both the EAMT and pool6.

Fixes issue #223.

Also includes a cgcc review.
@toreanderson
Copy link
Contributor

Just an observation here: I've seen folks that are working with IPTables and DNAT add secondary IP addresses to local interfaces on the DNAT boxes in order to attract traffic to those addreses without requiring routes to be added on the upstream router, i.e., something like:

ip address add 192.0.2.2/24 dev eth0 # primary
ip address add 192.0.2.3/32 dev eth0 label eth0:1 # for DNAT44 purposes
iptables -t nat -I PREROUTING -d 192.0.2.3 -j DNAT --to 10.1.2.3

This would be roughly equivalent to:

ip address add 192.0.2.2/24 dev eth0 # primary
ip address add 192.0.2.3/32 dev eth0 label eth0:1 # for SIIT-DC purposes
jool_siit --eamt --add 192.0.2.3 fd01:2345::6789

If you implicitly blacklist all local IPv4 addresses, then it might be that this use case no longer works. To be perfectly honest, though, I don't know if it works right now either, since I'm not using such a config myself.

That said, it might be possible to accomplish the same thing without assigning 192.0.2.3/32 to a local interface, e.g., using ip neigh add proxy functionality. If so I guess it's fine to blacklist local IPv4 addresses and instead document how it could be done instead.

@edmont
Copy link

edmont commented Sep 7, 2016

Just tested both the broadcast issue and the EAMT blacklist and everything works well now. Great job!

@ydahhrk
Copy link
Member Author

ydahhrk commented Sep 7, 2016

I've seen folks that are working with IPTables and DNAT add secondary IP addresses to local interfaces on the DNAT boxes in order to attract traffic to those addreses without requiring routes to be added on the upstream router

Good catch!

It's interesting to note that you used the word "secondary". It also seems reasonable to expect all such addresses to be /32. Perhaps it would be cleaner if only primary addresses were blacklisted. Or we could make exceptions out of /32s.

I'm interested to see if NAT44 also handles these addresses differently; I'll take a look at the iptables code.

(Also reminder to myself: If /32s end up being supposed to be translated anyway, be careful not to blacklist the "broadcast".)

@ydahhrk
Copy link
Member Author

ydahhrk commented Sep 7, 2016

Just tested both the broadcast issue and the EAMT blacklist and everything works well now.

Thanks!

@ydahhrk
Copy link
Member Author

ydahhrk commented Sep 7, 2016

Never mind. Your own example clearly shows that DNAT couldn't care less whether the --to address belongs to itself or not. I suppose it can afford that because it lets the user specify --dport, which makes it less intrusive. But we can't do that, because having to look at ports (in layer 4 headers) would force SIIT Jool to defragment.

So the solution of iptables is unsuitable here.

By the way:

Creating an iproute2 NAT mapping has the side effect of causing the kernel to answer ARP requests for the NAT IP.

(http://linux-ip.net/html/nat-stateless.html. I guess they got tired of adding /32 addresses?)

My current tendency is to let the SIIT translate host addresses that belong to /32s. These are (AFAIK) generally more specific and useless than secondary address and so they would more likely be intended for translation.

Though it still feels like an ugly hack. I suppose enclosing Jool is a namespace (or the device driver model) would be cleaner; the node would consume its own packets first, Jool would translate second. But that's also a lot of documentation excess.

I'll roll the code tomorrow.

ydahhrk added a commit that referenced this issue Sep 9, 2016
The implicit blacklist prevents SIIT Jool from translating its own
node's addresses. This commits makes addresses that belong to /32
"networks" an exception.

This is because some people prefer to add these addresses to
attract to-be-translated traffic, as opposed to add explicit routes
in all neighbors.
@ydahhrk ydahhrk added the Bug label Sep 14, 2016
@ydahhrk ydahhrk added this to the 3.4.5 milestone Sep 14, 2016
@ydahhrk ydahhrk added the Status: Tested Needs release label Sep 19, 2016
@ydahhrk ydahhrk removed the Status: Tested Needs release label Sep 19, 2016
@ydahhrk
Copy link
Member Author

ydahhrk commented Jul 1, 2020

Update: During the testing phase of the 4.1.1 release, I ran into a slight problem with relevant code that made me rethink the solution to this issue.

It's interesting to note that you used the word "secondary". It also seems reasonable to expect all such addresses to be /32.

I can't recall why I went with the /32 check. There's an easy means to tell whether an address is secondary or not.

Before Jool 4.1.1, this used to be implemented like this:

If address belongs to interface, and its prefix length is 32:
	address is not blacklisted

else if address belongs to interface:
	address is blacklisted

else if address is broadcast:
	address is blacklisted

else
	address is not blacklisted

I now think that's rather dumb. Starting from 4.1.1, it will be implemented like this:

If address is broadcast:
	address is blacklisted

else if address belongs to interface and is secondary:
	address is not blacklisted

else if address belongs to interface:
	address is blacklisted

else:
	address is not blacklisted

@ydahhrk
Copy link
Member Author

ydahhrk commented Oct 12, 2020

Update: Because of #342, I changed the algorithm to

If address is broadcast:
	deny address translation

else if address belongs to interface and is /32:
	allow address translation

else if address belongs to interface:
	deny address translation

else:
	allow address translation

Why? We have 1 person requesting the /32 exception, and none requesting the secondary address exception. (Tore's was hypothetical.)

The secondary address exception can be added back in the future if somebody wants it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants