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

Blackholes not replaced #190

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

midzelis
Copy link
Contributor

add_nexthop_routes() and or add_openvpn_routes() will not remove the blackhole added by add_blackhole_routes() because the blackhole is added via default (0.0.0.0/0) but replaced using 0.0.0.0/1, and 128.0.0.0/1.

I also noted a problem in run_rule_watcher() - I think.... its trying to remove the startup blackholes. But these routes are not the ones added by add_blackhole_routes - also, its not removing them from the ${ROUTE_TABLE} - so this basically doesn't do anything. Additionally, there is a another method for removing blackhole routes delete_blackhole_routes which seems like it ought to work, for anytype of blackhole route - using 0.0.0.0/0 or 0.0.0.0/1, 128.0.0.1.

This change fixes the problem with not removing blackholes, but please review the logic in the rule watcher - maybe I'm missing something?

@peacey
Copy link
Owner

peacey commented Jul 12, 2023

Hi @midzelis,

There's nothing wrong with this logic. So there's two types of blackhole routes that we use. One is the blackholes routes added to the the VPN route table, and one is startup blackholes routes which are only added by the user manually to the main route table via the Unifi Network GUI (these startup blackholes are for cases when the user doesn't want any traffic when the UDM reboots and the VPN script hasn't started yet - they are optional and only for advanced configurations).

Regarding the VPN backhole routes, these are not supposed to be replaced. The default VPN route is added as 0.0.0.0/1, 128.0.0.1/1 so that it can override the default backhole route without removing the blackholes. This way if something happens with those routes for whatever reason, the blackhole routes will still be there to protect anything leaking out that's routed through the VPN table.

Regarding the startup blackholes, they are supposed to be removed immediately after the script sets up the route table and routing rules. Then the rule watcher watches out if they're ever added again by the OS so it removes them constantly.

Hopefully that clears up the logic...

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

Successfully merging this pull request may close these issues.

2 participants