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

interfaces: autoconf IPv6 addresses are not being deprecated on PPPoE interface on link down #4929

Closed
2 tasks done
somova opened this issue Apr 17, 2021 · 20 comments
Closed
2 tasks done
Assignees
Labels
bug Production bug
Milestone

Comments

@somova
Copy link

somova commented Apr 17, 2021

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

Describe the bug
In case your ISP provides Internet via a PPPoE connection and the connection drops due to failures (e.g. DSL resync, PPP connection issues etc.) it can happen that IPv6 ist not working anymore when the connection comes up again. Some ISP dynamically assign IPv6 prefixes only. When the PPPoE connection drops and Opnsense reconnects to the ISP the latter assigns you a new prefix and the old one becomes outdated.

The Opnsense recognizes the new offered prefix and itself assigns new IPv6 addresses based on this prefix. The problem is that IPv6 addresses based on the outdated prefix are still assigned to the PPPoE interface. And the Opnsense still tries to use these addresses as source addresses of IPv6 packets. The ISP correctly block packets with invalid source address.

Before the PPPoE connection comes up again, old addresses must be removed from the PPPoE interface.

To Reproduce
Steps to reproduce the behavior:

  1. Choose an ISP which dynamically assigns IPv6 prefixes via PPPoE
  2. Configure your PPPoE connection to use IPv6 with the following parameters:
  • IPv6 Configuration Type: SLAAC
  • Use IPv4 connectivity: yes
  1. Check whether the ISP assigns you an IPv6 prefix (via SSH and ifconfig)
  2. Force re-eastablishing a PPPoeE connection (e.g. DSL resync, interrupting the connection between DSL modem and Opnsense in such a way the ethernet link does not get down).
  3. Re-check whether the ISP assigns you a new IPv6 prefix (via SSH and ifconfig)
  • The PPPoE interface should now contain IPv6 addresses with two different prefixes and IPv6 connections to the internet get stuck.

Expected behavior
All IPv6 addresses with the outdated prefix should be removed from the PPPoE interface after connection goes down. This should be Ok because nobody can guarantee that the prefix will still be valid after the connection comes up again. Even if it's still valid the ISP will reassign the same prefix again.

Describe alternatives you considered
I have read the documentation of the mpd5 daemon which is responsible to control the PPPoE connection but I have no clue why the IPv4 address will be removed when the link goes down but the IPv6 addresses will not. I guess the mpd5 daemon only removes addresses from the interface it itself has assigned to it. But in my configuration the provider assigns the IPv6 prefix to the interface (by sending router advertisement messages) without knowledge of the mpd5 daemon. Presumably, this is the cause the daemon is not able to remove unknown addresses from that interface.

It should be easy to solve this problem. We need to add some code to mpd5's linkdown script (/usr/local/opnsense/scripts/interfaces/ppp-linkdown.sh) to remove all IPv6 addresses from the interface. I have tested it with the following code snipped added to the script, which works well. Maybe somebody can optimize the code because it looks a little bit cobbled.

By the way we should consider that the interface could have multiple valid IPv6 addresses in case IPv6 Privacy Extensions (RFC 4941) are enabled. This scenario is also covered by the code snippet.

/usr/local/opnsense/scripts/interfaces/ppp-linkdown.sh:

[…]
elif [ "${AF}" = "inet6" ]; then
[…]
	# Do not remove gateway used during filter reload.
	rm -f /tmp/${IF}_routerv6 /tmp/${IF}upv6 /tmp/${IF}_ipv6

# <--- new inserted code - start --->
while i="`ifconfig ${IF} | grep inet6 | grep -m 1 -v '%' | cut -f2 -d ' ' | tr -d '[:space:]'`"; do
 if [ -n "$i" ] 
  then
   #echo "IPv6 Address found"
   ifconfig ${IF} inet6 $i delete
  else
   #echo "NO IPv6"
   break
 fi
done
# <--- new inserted code - end --->
fi

Software version used and hardware type if relevant, e.g.:
OPNsense 21.1.4-amd64

@somova
Copy link
Author

somova commented Apr 17, 2021

Info: related to https://forum.opnsense.org/index.php?topic=8985.0

@marjohn56
Copy link
Member

It's also an issue with non pppoe too, For example just using dhcpv6, if the upstream dhcpv6 server changes the prefix, when dhcp6c cannot get the same prefix it will keep trying until eventually it sends out a REQUEST again, however what it does not do is remove the existing prefix/delegation it has assigned from the interface(s). There is a test version of dhcp6c that now appears to handle this better. If it gets a response from the dhcpv6 server saying that the address dhcp6c is asking for is no longer available, then dhcp6c immediately gives up, releases the addresses its previously assigned and restarts with a REQUEST. Could be the same issue.

@glasid
Copy link

glasid commented Apr 25, 2021

I can reproduce the issue on my end. Proposed code snippet works.

Maybe someone comes up with a more elegant way without a while loop.

@somova
Copy link
Author

somova commented May 3, 2021

PPPoE and DHCPv6 do not exclude each other. But, if I understand you (marjohn56) correctly, DHCPv6 suffers from similar problems even if used without PPPoE. In my eyes, this is a different issue, so please open a new ticket for this one.

How do we like to proceed with this ticket? The issue affects a lot of users with dial up connections based on PPPoE and dynamic IPv6 prefixes. So, a possible fix is easy to implement, as my example above shows.

@somova
Copy link
Author

somova commented Jun 1, 2021

Either pppoe is not widespread or the interest of solving this issue is not very high. Is it better to integrate the code snippet into a pull request?

@fichtner
Copy link
Member

fichtner commented Jun 2, 2021

Sure, PR is better to start the discussion. Doing shell magic here is probably not the best approach since we already have PHP doing this but which isn't triggered on PPP linkdown...

@fichtner fichtner self-assigned this Jun 2, 2021
@fichtner fichtner added the feature Adding new functionality label Jun 2, 2021
@fichtner fichtner added this to the 21.7 milestone Jun 2, 2021
@somova
Copy link
Author

somova commented Jun 6, 2021

Ok, for the future I'll begin with a pull request :-).

From technical perspective it shouldn't be a problem to switch over from bash scripting to PHP as long as the command line arguments are correctly evaluated and the PHP scripts ensure to be non-blocking. Perhaps the return code of the scripts needs to be passed back to the mpd5 daemon.

The question is, does it make sense here because the scripts only call a couple of system commands without any sophisticated logic.

@fichtner
Copy link
Member

fichtner commented Jun 7, 2021

Eventually all good fixes break existing setups or have edge cases. interfaces_primary_address6() is a great example of a simple fix requiring extensive maintenance. If we do not have the luxury of PHP the complexity of the shell script will eventually explode , i.e. fetching a VIP entry from the configuration file...

@fichtner
Copy link
Member

@somova I took a closer look today... actually... are you using the "Use IPv4 connectivity" setting?

Cheers,
Franco

@fichtner
Copy link
Member

Ok, disregard, saw from your description :)

@fichtner
Copy link
Member

Funny or not funny, I don't know https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=146377

@fichtner
Copy link
Member

fichtner commented Jun 18, 2021

@somova on 21.1.7 you can try via:

# opnsense-patch 132ac6b05 b563c28 f7d7fd6

I am not even sure it's MPDs fault here since it does not handle IPv6 so SLAAC forgets about it, static addresses linger and dhcp6c may have issues.

Worst case the solution breaks other cases (removing a static IPv6?) which means we have to add more PHP glue from inspecting the configuration and that's why we really need the PHP to be there.

@fichtner
Copy link
Member

fichtner commented Jun 18, 2021

Maybe this only really affects "autoconf" annotated addresses (SLAAC)? Essentially I would like to try to set all "autoconf" addresses to "deprecated" during linkdown and see if that helps your case. With this, we don't have to rely on deeper configuration inspection just to do the right thing.

@fichtner
Copy link
Member

Can you try this minimal change instead? eece312182

It might not be in working condition, but looking at this toggling ifdisabled flag is the best approach to avoid messing with other things...

@somova
Copy link
Author

somova commented Jun 24, 2021

Sorry for my late response. Unfortunately, the email notification does not seem to work. I can perform all the tests this weekend and report it here

@somova
Copy link
Author

somova commented Jun 25, 2021

So, today I've update my system to version 21.1.7_1 and afterwards applied the patches with

# opnsense-patch 132ac6b05 b563c28 f7d7fd6

This looks fine. The system immediately removes all IPv6 addresses from PPPoE interface when simulating a connection shutdown by issuing the appropriate signal to the mpd5 daemon. Also the manual re-connection function in the WebUI (Interfaces -> Overview -> PPPoE Interface) still works as expected.

@fichtner You mentioned another patch eece312. Is this a replacement for the three above?

@somova
Copy link
Author

somova commented Jun 25, 2021

Funny or not funny, I don't know https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=146377

It looks like the bug report belongs to the pretty old ppp daemon which is not used anymore??

I am not even sure it's MPDs fault here since it does not handle IPv6 so SLAAC forgets about it, static addresses linger and dhcp6c may have issues.

The mpd5 daemon is not aware about the IPv6 addresses because configuration is done by the kernel itself and RTADV helper, so from the daemons perspective it's better not touching any IPv6 addresses assigned to the interface (and use the related scripts instead).

Worst case the solution breaks other cases (removing a static IPv6?) which means we have to add more PHP glue from inspecting the configuration and that's why we really need the PHP to be there.

Indeed you're right, This was missing in my thoughts. So, we should only remove IPv6 addresses containing the "autoconf" flag?

@fichtner
Copy link
Member

I would try to set the autoconf addresses to deprecated as they should be. This would be minimal impact. Fiddling with ifdisabled flag might be time consuming to get right.

@fichtner
Copy link
Member

(This can be shell code again then)

@fichtner fichtner added bug Production bug and removed feature Adding new functionality labels Jul 1, 2021
@fichtner fichtner changed the title IPv6 addresses not removed from PPPoE interface if prefix becomes invalid interfaces: autoconf IPv6 addresses are not being deprecated on PPPoE interface on link down Jul 1, 2021
@fichtner
Copy link
Member

@somova committed as 11b5fe6... need to wrap up 21.7 sooner or later so not a lot of time for waiting left

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

No branches or pull requests

4 participants