search

freifunk-gluon / gluon

a modular framework for creating OpenWrt-based firmwares for wireless mesh nodes
https://gluon.readthedocs.io
Other
547 stars 325 forks source link

Bug: Gluon Routes its Packets to Wrong IPv6 Default Routers #2180

Open lemoer opened 3 years ago

lemoer commented 3 years ago

Bug report

What is the problem?

Router advertisement servers advertise prefixes together with a default router in layer 2 networks. There may be multiple router advertisement servers in one layer 2 network. Each of those router advertisement servers can advertise different prefixes and a different default router.

A client can accept multiple of the advertised prefixes at the same time. If it does that, it installs an address in each of the advertised prefixes. It also has to install default routes from the router advertisements. Since default routers are not expected to be able to forward ip packets from arbitrary source addresses, the client should install routes to all default routers and use the correct one depending on which of the installed addresses it is sending from.

A gluon node receives all router advertisements on the br-client. It accepts all prefixes and therefore installs one address in each received prefix. But it installs just one default route. Therefore it sends all packets to the same default router. As this default router is not the designated default router for all prefixes, it may happen that the packet is dropped because of its source ip. This means that the gluon node will not be publicly reachable on some of its addresses.

What is the expected behaviour?

A Gluon node should either:

How do you recognize, that a gluon node is affected?

What are the conditions, this bug is triggered:

Not affected:

Possible workarounds:

Gluon Version:

Site Configuration:

Custom patches:

lemoer commented 3 years ago

We probably found the broken code:

https://github.com/openwrt/openwrt/blob/e3b88490885ef228456d1be39e6e84679cfe4cfb/package/network/ipv6/odhcp6c/files/dhcpv6.script#L134

Actually the code tries to be quite clever here, if I interpret it correctly. It applies source routing only if necessary. Only if there "duplicate" routes, then it applies source based routing if not, it does not apply source based routing. But there is a mistake in L134. It should be "$duplicate" = 0 instead of "$duplicate" = 1.

Before any changes

root@wgtest-1043-lemoer:/lib/netifd# ip -6 r | grep default
default via fe80::8ae6:40ff:feba:1014 dev br-client proto static metric 512 pref medium

After changing it to "$duplicate" = 0, we get:

root@wgtest-1043-lemoer:/lib/netifd# ip -6 r | grep default
default from 2001:678:978:114::/64 via fe80::8ae6:40ff:feba:1014 dev br-client proto static metric 512 pref medium
default from 2a02:790:ff:714::/64 via fe80::8ae6:40ff:feba:1014 dev br-client proto static metric 512 pref medium
default from 2a02:790:ff:814::/64 via fe80::8ae6:40ff:feba:1014 dev br-client proto static metric 512 pref medium
default from 2a02:790:ff:914::/64 via fe80::8ae6:40ff:feba:1014 dev br-client proto static metric 512 pref medium
default from 2a02:790:ff:1014::/64 via fe80::8ae6:40ff:feba:1014 dev br-client proto static metric 512 pref medium
default from fdca:ffee:8:14::/64 via fe80::8ae6:40ff:feba:1014 dev br-client proto static metric 512 pref medium

So changing "$duplicate" = 1 to "$duplicate" = 0 solves our problem! I guess, it's a regression bug, which was long undiscovered.

While the actual fuckup is in L134, the default in L115 has to be inverted then as well.

We should submit this upstream. I'll do this.

awlx commented 3 years ago

There was a draft for IPv6 to support what you want. But according to the actual current RFC6724 this is not part of IPv6 Default Address selection. Thus it's not a bug but maybe a missing feature. Other than that all gateways in a Layer2 segment should be able to route all addresses. See also this document for multiple prefixes in an IPv6 network.

lemoer commented 3 years ago

Thanks for the clarification and the references. From the RFC point of view, this is maybe not a bug.

If I understand the odhcpv6 script correctly, it wants to implement what we need. But unfortunately, there is a bug in an if statement. When I invert "$duplicate" = 1 to "$duplicate" = 0, the problems are solved. I updated my comment above to (hopefully) make this more clear.

awlx commented 3 years ago

Btw just to mention, even this does not help you to get rid of all cases where wrong prefixes hit the wrong gateways. Imagine a roaming client with the „wrong prefix“ from node A (gateway A) which is still valid until valid lifetime expires. And it receives another RA with the other Gateway (B) when connected to node B. Then you have the same problem. Thus you have to route between the gateways nevertheless.

CodeFetch commented 3 years ago

@awlx Yes, that there is still routing between the supernodes is clear I think, but I'm also sure that this is a bug. As far as I know support for multiple IPv6 prefixes using source routing is needed since a revision of the IPv6 protocol done for the Homenet protocol and it is part of the mainline kernel since 4.1.

Adorfer commented 3 years ago

Even in case we consider this behaviour 1) not to be a bug (of the ipv6 implementation in openwrt), and 2) as defined / in compliance with RFCs

It should be changed (at least in batman-adv gluon) in order to reduce "diagonal traffic" in multi-supernode L2 segments. Since this this type of traffic reduces overall performance of the system. (or increases performance, if it can be avoided)

awlx commented 3 years ago

@awlx Yes, that there is still routing between the supernodes is clear I think, but I'm also sure that this is a bug. As far as I know support for multiple IPv6 prefixes using source routing is needed since a revision of the IPv6 protocol done for the Homenet protocol and it is part of the mainline kernel since 4.1.

I have never seen this working yet with pure Slaac on Linux also Homenet mentions the same RFCs I mentioned. https://tools.ietf.org/id/draft-ietf-homenet-arch-04.html#rfc.section.2.1

We also run multiple prefixes at FFMUC and I can confirm that neither MacOS or most Linux Distros do any kind of source routing they just send their stuff where they want.

lemoer commented 3 years ago

This is acutally not a bug in OpenWrt. We are just disabling the feature in gluon. I created a pull request in #2183 to enable source routing.

CodeFetch commented 3 years ago

From RFC 6724 - Default Address Selection for IPv6 

Rule 5.5: Prefer addresses in a prefix advertised by the next-hop.
   If SA or SA's prefix is assigned by the selected next-hop that will
   be used to send to D and SB or SB's prefix is assigned by a different
   next-hop, then prefer SA.  Similarly, if SB or SB's prefix is
   assigned by the next-hop that will be used to send to D and SA or
   SA's prefix is assigned by a different next-hop, then prefer SB.

      Discussion: An IPv6 implementation is not required to remember
      which next-hops advertised which prefixes.  The conceptual models
      of IPv6 hosts in Section 5 of [RFC4861] and Section 3 of [RFC4191]
      have no such requirement.  Hence, Rule 5.5 is only applicable to
      implementations that track this information.

This sounds to me like SHOULD, but not MUST. So maybe we could tune the metrics...

CodeFetch commented 3 years ago

I think we could introduce a "RAforeignprefixmetricpenalty" option which adds a penalty to the metric of a route with the "from" of a prefix which was not advertised by that specific gateway. I think this should be conform with the homenet specification, too.

neocturne commented 3 years ago

As mentioned in #2183, I currently don't see how we can support IPv6 routers announcing different prefixes.

Most clients don't implement Source Routing, so if radv-filterd only allows RAs from one Router to pass through, and a client roams to a different node where radv-filterd has chosen a router with a different prefix, things will break badly. As such, all routers that announce a default route at all must be equivalent, in that they can route packets from/to the same address space. And as long as this is the case, it doesn't matter which default route Gluon uses.

For networks that don't have their own IP space, and can't or don't want to have BGP peering on each router, a common solution that was also used by FFHL in the past is Network Prefix Translation (NPTv6). This is not perfect, as connections will still break when a client roams to a different default route, but at least no client retains IP addresses that aren't usable anymore for an indefinite amount of time.

CodeFetch commented 3 years ago

@NeoRaider

all IPv6 routers must ~announce~ be able to route the same set of prefixes

I'm aware of that. It means that we must add a route for every gateway nonetheless, but they should have a worse metric if that "from" prefix was not announced by that specific gateway, because it means the packet must be re-routed in the backbone and therefore it is logical that the metric should have added a penalty for which we do not account, yet.

My suggestion is to add an option to the dhcp6c script for defining a metric penalty which will be applied for source routes which use a prefix which was not announced by that specific gateway.

My thought: The only reason why we don't open up a VPN connection to every supernode is that we get the broadcast and Batman management traffic for each. If Batman or when using Babel mmfd would use a more sophisticated multicast approach, we could limit the amount of duplicate management traffic which allows us to get rid of most cross-traffic on the supernodes and allows us to do more decentralization in the future.

CodeFetch commented 3 years ago

Most clients don't implement Source Routing, so if radv-filterd only allows RAs from one Router to pass through, and a client roams to a different node where radv-filterd has chosen a router with a different prefix, things will break badly.

I'm not suggesting to stop routing between the supernodes...

neocturne commented 3 years ago

@NeoRaider My suggestion is to add an option to the dhcp6c script for defining a metric penalty which will be applied for source routes which use a prefix which was not announced by that specific gateway.

I think the dhcpv6.script or netifd will need to choose one default route as the "primary" one and install that route without a source restriction, so it will be used for source address selection. Having this in netifd might make sense for route timeout?

awlx commented 3 years ago

To be honest, I wouldn't fiddle with default Linux behaviour as none standard behaviour is hard to debug. We basically provoke this problem ourselves with stretching L2 domains over different datacenters.

CodeFetch commented 3 years ago

@awlx We are the people who define Linux default behavior. If we don't, never will anyone else, especially when it comes to routers.

awlx commented 3 years ago

@CodeFetch Then I would open a bug way upstream in the kernel and not here. Otherwise it's just a "variant of Linux" which behaves completely different.

CodeFetch commented 3 years ago

@awlx I think the kernel already has all features implemented. In this line the router is set to "any" and the information is not propagated to the script here.

This is correct according to the standard, but if possible the prefix that was actually announced by the gateway should be preferred (by setting e.g. a metric penalty for the routes with the prefixes where the gateway didn't advertise the prefix itself - as we can assume that it needs to be rerouted in the backbone this is factually correct). I just propose to introduce a configurable metric penalty for these routes. This should not break anything and is still conform to the IPv6 standard. 

      Hence, Rule 5.5 is only applicable to
      implementations that track this information.

I think we should implement it.