greenpau / cni-plugins

CNI Plugins compatible with nftables
Apache License 2.0
48 stars 13 forks source link

Port forwarding rules are too aggressive #12

Closed dafydd2277 closed 3 years ago

dafydd2277 commented 3 years ago

This is a great plug in, and I'm glad you made it.

What I'm getting is probably issue #10, but I'm not sure. On my NAT system, the nftables rules are being set up to intercept all traffic to (eg.) port 80, including traffic intended to be forwarded from the internal to the external interfaces.

Here is the relevant part of my base ruleset, with a little bit of obfuscation.

define EXT_IF = <external interface>
define INT_IF = <internal interface>

table ip filter {
  chain input {
    type filter hook input priority 0; policy drop;
    # filter rules
  }

  chain forward {
    type filter hook forward priority 0; policy drop;
    iifname $INT_IF oifname $EXT_IF counter accept
    iifname $EXT_IF oifname $INT_IF ct state related,established counter accept
  }

  chain output {
    type filter hook output priority 0; policy accept;
  }
}

table ip nat {
  chain prerouting {
    type nat hook prerouting priority -100; policy accept;
  }
  chain postrouting {
    type nat hook postrouting priority 100; policy accept;
    oifname $EXT_IF counter masquerade
  }
}

Now, run a container.

# podman run -dp 80:80 docker/getting-started
746443ba846dddfadc016ab6ffda1ec44e3b083b3b25f344039db858e90d0d2f

# podman container ls
CONTAINER ID  IMAGE                                    COMMAND               CREATED         STATUS             PORTS               NAMES
746443ba846d  docker.io/docker/getting-started:latest  nginx -g daemon o...  11 seconds ago  Up 10 seconds ago  0.0.0.0:80->80/tcp  flamboyant_williamson

# podman network ls
NETWORK ID    NAME    VERSION  PLUGINS
2f259bab93aa  podman  0.4.0    bridge,cni-nftables-portmap,cni-nftables-firewall,tuning

And the diff in the ruleset.

table ip filter {
  chain forward {
    type filter hook forward priority filter; policy drop;
    jump cni-ffw-b25f344039db858e90d0d2f
    oifname "cni-podman0" ip daddr 10.88.0.11 tcp dport 80 counter accept
    iifname $INT_IF oifname $EXT_IF counter accept
    iifname $EXT_IF oifname $INT_IF ct state established,related counter accept
  }

  chain cni-ffw-b25f344039db858e90d0d2f {
    oifname "cni-podman0" ip daddr 10.88.0.11 ct state established,related counter accept
    iifname "cni-podman0" ip saddr 10.88.0.11 counter accept
    iifname "cni-podman0" oifname "cni-podman0" counter accept
  }
}
table ip nat {
  chain prerouting {
    type nat hook prerouting priority dstnat; policy accept;
    jump cni-npr-b25f344039db858e90d0d2f
  }

  chain input {
    type nat hook input priority 100; policy accept;
  }

  chain output {
    type nat hook output priority -100; policy accept;
    jump cni-npr-b25f344039db858e90d0d2f
  }

  chain postrouting {
    type nat hook postrouting priority srcnat; policy accept;
    jump cni-npo-b25f344039db858e90d0d2f
    oifname $EXT_IF counter masquerade
  }

  chain cni-npr-b25f344039db858e90d0d2f {
    iifname != "cni-podman0" tcp dport 80 dnat to 10.88.0.11:80
  }

  chain cni-npo-b25f344039db858e90d0d2f {
    iifname "cni-podman0" ip saddr 10.88.0.11 ip daddr 224.0.0.0/24 counter return
    iifname "cni-podman0" ip saddr 10.88.0.11 ip daddr 255.255.255.255 counter return
    iifname "cni-podman0" ip saddr 10.88.0.11 counter masquerade
  }
}

I think the problem is the rule

iifname != "cni-podman0" tcp dport 80 dnat to 10.88.0.11:80

This intercepts anything going to port 80, whether it's coming to this host or just passing through. If I'm on an internal system and I try http://www.google.com/ I get redirected to the container before the request ever leaves my NAT firewall.

My suggested fix is to change the conditional in the rule to something like this:

ip daddr $INT_IPADDR tcp dport 80 dnat to 10.88.0.11:80
ip daddr $EXT_IPADDR tcp dport 80 dnat to 10.88.0.11:80

The IP addresses could be acquired automatically. Or, for more user control, you could read a list added to the plugin options.

    {
      "type": "cni-nftables-firewall",
      "forward_chain_name": "forward"
      "daddr": [
        $EXT_IPADDR,
        $INT_IPADDR
      ]
    },

(I realize I probably have that syntax wrong.) This would give users a more finely grained control over how the firewall intercepts container traffic. I'm going to fork this and see how fast I can learn Go and podman container plugins.

greenpau commented 3 years ago

@dafydd2277 , thank you for the issue! Feel free to submit a PR for this one. I might get to it later this summer. Currently, my focus is https://github.com/greenpau/caddy-auth-portal

dafydd2277 commented 3 years ago

Heh. I got as far as "Oh, crap! Go is strongly typed!" and gave up for the night. I may need a week or two. :grin:

dafydd2277 commented 3 years ago

So I don't lose the thought, the modification would also need this rule for completeness:

ip daddr 127.0.0.1 tcp dport 80 dnat to 10.88.0.11:80
dafydd2277 commented 3 years ago

After a week of reinventing the wheel, I finally look at @ististan 's PR and realize that modifying

    chain output {
                type nat hook output priority -100; policy accept;
                ip daddr 127.0.0.1 jump cni-npr-78f486a1c7999cfe8e0526d
            }

to include all IP addresses attached to the container host is exactly what I need. Except, I don't get his changes. I still get the old-style rule.

# cat /proc/sys/net/ipv4/conf/all/route_localnet 
1

# cat /proc/sys/net/ipv4/conf/default/route_localnet 
1

# podman network ls
NETWORK ID    NAME    VERSION  PLUGINS
2f259bab93aa  podman  0.4.0    bridge,cni-nftables-portmap,cni-nftables-firewall

# podman container ls
CONTAINER ID  IMAGE                                    COMMAND               CREATED         STATUS             PORTS                 NAMES
769eba2bc569  docker.io/docker/getting-started:latest  nginx -g daemon o...  30 minutes ago  Up 30 minutes ago  0.0.0.0:8080->80/tcp  boring_pike

# nft -a list chain nat output
table ip nat {
    chain output { # handle 3
        type nat hook output priority -100; policy accept;
        jump cni-npr-57bf961e3f4fa756c612076 # handle 8
    }
}

# nft -a list chain nat cni-npr-57bf961e3f4fa756c612076
table ip nat {
    chain cni-npr-57bf961e3f4fa756c612076 { # handle 6
        iifname != "cni-podman0" tcp dport 8080 dnat to 10.88.0.3:80 # handle 9
    }
}

# nft -a list chain nat cni-npo-57bf961e3f4fa756c612076
table ip nat {
    chain cni-npo-57bf961e3f4fa756c612076 { # handle 10
        iifname "cni-podman0" ip saddr 10.88.0.3 ip daddr 224.0.0.0/24 counter packets 0 bytes 0 return # handle 12
        iifname "cni-podman0" ip saddr 10.88.0.3 ip daddr 255.255.255.255 counter packets 0 bytes 0 return # handle 13
        iifname "cni-podman0" ip saddr 10.88.0.3 counter packets 0 bytes 0 masquerade # handle 14
    }
}

Neither of the 127.0.0.1 rules he added are showing up. So, I'm troubleshooting that, instead. Then, I'll look at adding something like a net.IP loop to call CreateJumpRuleWithIPSourceMatch (I think) once for each IP address a host is listening to.

dafydd2277 commented 3 years ago

Okay, troubleshooting done. @greenpau, the go get -u http://... command is still grabbing tag 1.0.6, which predates the merge of #11. So, I'm still seeing the old jump rule.

Also, if you can spare me five minutes, what's a good link for examples on how to build binaries from my (modified) git clone of your project? (Again, not even remotely a Go programmer.) My google-fu isn't giving me good answers.

dafydd2277 commented 3 years ago

After some not actually helpful advice, I'm building locally, and it's building to HEAD, not to git tag 1.0.6. So, now I can start working on possible rule changes.

greenpau commented 3 years ago

@dafydd2277 , I just released the v1.0.7 and marked it latest.

dafydd2277 commented 3 years ago

I'm still working on a loop to jump all local IP addresses, not just localhost. Right now, the firewall/forward rules don't clearly define which packets get forwarded to containers versus packets that get forwarded to an external interface.