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

Invalid exclusion routes for wrong network device #389

Open
qrkourier opened this issue Jun 12, 2022 · 6 comments
Open

Invalid exclusion routes for wrong network device #389

qrkourier opened this issue Jun 12, 2022 · 6 comments

Comments

@qrkourier
Copy link
Member

qrkourier commented Jun 12, 2022

ZET adds static "exclusion routes" for each controller host's routable IP. These are intended to exclude the controller traffic from intercept, but the logic is flawed. The wrong network adapter device is selected for the static route.

 ❯ ip route get $(dig +short 17ba74fe-afff-4d4d-aa69-c915c4d3aecf.production.netfoundry.io)
150.136.202.243 via 172.20.1.1 dev br-e42ee9426c2f src 172.20.0.1 uid 1000 
    cache 

In this example the public IP of the NC host was routed to a linkdown docker bridge interface.

@qrkourier
Copy link
Member Author

Additionally, the static routes are left behind when ziti-edge-tunnel exits gracefully.

 ❯ ip route sh     
default via 10.102.0.1 dev enp4s0 proto dhcp metric 100 
default via 10.100.0.1 dev wlp3s0 proto dhcp metric 600 
10.100.0.0/21 dev wlp3s0 proto kernel scope link src 10.100.0.242 metric 600 
10.102.0.0/21 dev enp4s0 proto kernel scope link src 10.102.0.86 metric 100 
100.64.128.0/24 dev br-a99b4ce66e0c proto kernel scope link src 100.64.128.1 linkdown 
132.145.157.243 via 172.20.1.1 dev br-e42ee9426c2f linkdown 
132.145.203.43 via 172.20.1.1 dev br-e42ee9426c2f linkdown 
150.136.202.243 via 172.20.1.1 dev br-e42ee9426c2f linkdown 
169.254.0.0/16 dev br-1c23dd5aef59 scope link metric 1000 linkdown 
172.17.0.0/16 dev docker0 proto kernel scope link src 172.17.0.1 linkdown 
172.18.0.0/16 dev br-1d31f30b517a proto kernel scope link src 172.18.0.1 linkdown 
172.19.0.0/16 dev br-1c23dd5aef59 proto kernel scope link src 172.19.0.1 linkdown 
172.20.0.0/16 dev br-e42ee9426c2f proto kernel scope link src 172.20.0.1 linkdown 

@qrkourier
Copy link
Member Author

I cleaned up the spurious routes with this command.

 ❯ docker network prune
WARNING! This will remove all custom networks not used by at least one container.
Are you sure you want to continue? [y/N] y
Deleted Networks:
docker-compose-servers_default
kentest-servers_default
tmp_ziti-dnsmasq
docker_ziti-host


~ 2s 
 ❮ ip route sh         
default via 10.102.0.1 dev enp4s0 proto dhcp metric 100 
default via 10.100.0.1 dev wlp3s0 proto dhcp metric 600 
10.100.0.0/21 dev wlp3s0 proto kernel scope link src 10.100.0.242 metric 600 
10.102.0.0/21 dev enp4s0 proto kernel scope link src 10.102.0.86 metric 100 
172.17.0.0/16 dev docker0 proto kernel scope link src 172.17.0.1 linkdown 

@qrkourier qrkourier changed the title selects the wrong nic device for underlay ZET black holes controller backhole by selecting the wrong NIC Jun 30, 2022
@qrkourier qrkourier changed the title ZET black holes controller backhole by selecting the wrong NIC ZET black holes controller backhaul by selecting the wrong NIC Jun 30, 2022
@qrkourier qrkourier changed the title ZET black holes controller backhaul by selecting the wrong NIC Invalid exclusion routes for wrong network device Jul 10, 2022
@qrkourier
Copy link
Member Author

I found another case for which the network device selection algorithm must be flawed: when the default route is assigned to a bridge device the managed network device is selected for exclusion routes, but the correct choice is the bridge device.

For example, I have this network device br0 that is the primary master interface, and wired Ethernet NIC enp2s0 that is selected erroneously for the exclusion routes despite being managed by br0.

$ ip link show br0
414: br0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether aa:47:c8:ed:0a:80 brd ff:ff:ff:ff:ff:ff

$ ip link show enp2s0
2: enp2s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq master br0 state UP mode DEFAULT group default qlen 1000
    link/ether d8:5e:d3:83:97:5e brd ff:ff:ff:ff:ff:ff

@qrkourier
Copy link
Member Author

I have not experienced a recurrence of this issue.

@jonasgroenke
Copy link

Hey,
it seems like I'm facing the same issue. 

I tried to run ziti-edge-tunnel on a vm host where the ziti controller is already running in a vm. Therefore, I already have a route for the controller.
When I run the ziti-edge-tunnel it will replace this route and use the default interface instead of the vm bridge.

I checked the source code and it looks like the logic is pretty simple at the moment. It will run ip -o route show match %s table all and take the first line of this command.

"ip -o route show match %s table all | "

For my scenario, the command returns the default route first, and that's why it uses the wrong interface (2.2.2.2 is the controller ip).

ip -o route show match 2.2.2.2 table all
default via 1.1.1.1 dev eno1 proto kernel onlink 
2.2.2.2 dev vmbr0 scope link  

As a workaround, I set a higher metric to the default interface, because ziti will take this as a template.
So I still end up with two routes, but my one has a lower metric:

ip -o route show match 2.2.2.2 table all
default via 1.1.1.1 dev eno1 proto kernel metric 100 onlink 
2.2.2.2 dev vmbr0 scope link metric 50 
2.2.2.2 via 1.1.1.1 dev eno1 metric 100 (This one was added by ziti)

@scareything
Copy link
Member

Thanks @jonasgroenke for the report and workaround for your situation!

FYI the ziti_tunneler_exclude_route function which ultimately calls the code that you referenced has some logic to prevent harmful exclusion routes. Right now it only checks if the exclusion target is a local address, and if it is the code that you referenced is not invoked. We could probably enhance this higher level logic to also check for existing routes as well as local addresses.

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

No branches or pull requests

3 participants