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

IPv4 multicast should probably be implicitly blacklisted #168

Closed
toreanderson opened this Issue Aug 19, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@toreanderson
Contributor

toreanderson commented Aug 19, 2015

I've noticed that Jool will happily by default translate traffic destined for multicast addresses. Here's an example showing a VRRP packet being translated:

09:01:38.217668 IP 87.238.62.197 > 224.0.0.18: VRRPv2, Advertisement, vrid 1, prio 254, authtype none, intvl 1s, length 24
09:01:38.217756 IP6 2a02:c0::46:42:57ee:3ec5 > 2a02:c0::46:42:e000:12: ip-proto-112 24

RFC 6052 is silent on the subject of multicast, while RFC 6145 says:

The IP/ICMP header translation specified in this document is consistent with requirements of multicast IP/ICMP headers. However, IPv4 multicast addresses [RFC5771] cannot be mapped to IPv6 multicast addresses [RFC3307] based on the unicast mapping rule [RFC6052].

The last sentence there makes me think that IPv4 multicast (224.0.0.0/4) should probably be implicitly blacklisted. At the very least, IPv4 multicast addresses which the local node has subscribed to should be - there's some application running on the Jool node that's interested in those packets; Jool shouldn't steal them.

@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Aug 19, 2015

Member

Yet another argument in favor of #140. Once traffic intended to be translated has to be manually routed towards Jool, it will naturally stop seeing link-local traffic.

As a workaround, 224.0.0.0/24 /4 can be manually blacklisted; correct?

Member

ydahhrk commented Aug 19, 2015

Yet another argument in favor of #140. Once traffic intended to be translated has to be manually routed towards Jool, it will naturally stop seeing link-local traffic.

As a workaround, 224.0.0.0/24 /4 can be manually blacklisted; correct?

@toreanderson

This comment has been minimized.

Show comment
Hide comment
@toreanderson

toreanderson Aug 19, 2015

Contributor

As a workaround, 224.0.0.0/24 can be manually blacklisted; correct?

Correct. So this is just a suggestion, it doesn't cause me any operational problems in any way.

Contributor

toreanderson commented Aug 19, 2015

As a workaround, 224.0.0.0/24 can be manually blacklisted; correct?

Correct. So this is just a suggestion, it doesn't cause me any operational problems in any way.

ydahhrk added a commit that referenced this issue Aug 19, 2015

Issue #168, simple hack version.
I'm only adding the 224.0.0.0/24 blacklist. I still gotta check whether other similar ranges should also be ignored.
@mcr

This comment has been minimized.

Show comment
Hide comment
@mcr

mcr Aug 19, 2015

Tore Anderson notifications@github.com wrote:
> The last sentence there makes me think that IPv4 multicast
> (224.0.0.0/4) should probably be implicitly blacklisted. At the very
> least, IPv4 multicast addresses which the local node has subscribed to
> should be - there's some application running on the Jool node that's
> interested in those packets; Jool shouldn't steal them.

I can't see a good reason to translate class D addresses (multicast).
Probably should blacklist 127/8, 0/8 and some other invalid things.
I don't know what to do with 240/16 (class E).

mcr commented Aug 19, 2015

Tore Anderson notifications@github.com wrote:
> The last sentence there makes me think that IPv4 multicast
> (224.0.0.0/4) should probably be implicitly blacklisted. At the very
> least, IPv4 multicast addresses which the local node has subscribed to
> should be - there's some application running on the Jool node that's
> interested in those packets; Jool shouldn't steal them.

I can't see a good reason to translate class D addresses (multicast).
Probably should blacklist 127/8, 0/8 and some other invalid things.
I don't know what to do with 240/16 (class E).

@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Aug 19, 2015

Member

Probably should blacklist 127/8, 0/8 and some other invalid things.

Don't worry; those are already blacklisted. This is the relevant code.

I don't know what to do with 240/16 (class E).

Well, since they are experimental, I don't think we should completely ban the user from doing whatever with them.

They can always be manually blacklisted if use cases or security implications arise.

Member

ydahhrk commented Aug 19, 2015

Probably should blacklist 127/8, 0/8 and some other invalid things.

Don't worry; those are already blacklisted. This is the relevant code.

I don't know what to do with 240/16 (class E).

Well, since they are experimental, I don't think we should completely ban the user from doing whatever with them.

They can always be manually blacklisted if use cases or security implications arise.

ydahhrk added a commit that referenced this issue Aug 25, 2015

Three bugfixes:
1. There are separate pool4s now for protocols TCP, UDP and ICMP.
2. Applied #168 to this branch (because the blacklist is implemented differently).
3. ICMP errors reporting incorrect MTUs during fragmentation neededs/packet too bigs.

@ydahhrk ydahhrk added this to the 3.3.4 milestone Sep 17, 2015

@ydahhrk ydahhrk closed this Sep 22, 2015

@jm493

This comment has been minimized.

Show comment
Hide comment
@jm493

jm493 Sep 22, 2015

The code blacklists 224.0.0.0/4
https://github.com/NICMx/NAT64/blob/master/mod/common/blacklist.c

But this documentation doesn't mention that range can't be used:
https://www.jool.mx/usr-flags-blacklist.html

jm493 commented Sep 22, 2015

The code blacklists 224.0.0.0/4
https://github.com/NICMx/NAT64/blob/master/mod/common/blacklist.c

But this documentation doesn't mention that range can't be used:
https://www.jool.mx/usr-flags-blacklist.html

@ydahhrk ydahhrk reopened this Sep 22, 2015

@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Sep 22, 2015

Member

Fixed; thank you!

Member

ydahhrk commented Sep 22, 2015

Fixed; thank you!

@ydahhrk ydahhrk closed this Sep 22, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment