firewall: support ingressPolicy=(open|same-bridge) for isolating bridges as in Docker

[AkihiroSuda]

This commit adds a new parameter ingressPolicy (string) to the firewall plugin.
The supported values are open and same-bridge.

• open is the default and does NOP.

• same-bridge creates "CNI-ISOLATION-STAGE-1" and "CNI-ISOLATION-STAGE-2" that are similar to Docker libnetwork's "DOCKER-ISOLATION-STAGE-1" and "DOCKER-ISOLATION-STAGE-2" rules.

e.g., when ns1 and ns2 are connected to bridge cni1, and ns3 is connected to bridge cni2, the same-bridge ingress policy disallows communications between ns1 and ns3, while allowing communications between ns1 and ns2.

Please refer to the comment lines in ingresspolicy.go for the actual iptables rules.

The same-bridge ingress policy is expected to be used in conjunction with bridge plugin. May not work as expected with other "main" plugins.

It should be also noted that the same-bridge ingress policy executes raw iptables commands directly, even when the backend is set to firewalld.
We could potentially use the "direct" API of firewalld to execute iptables via firewalld, but it doesn't seem to have a clear benefit over just directly executing raw iptables commands.
(Anyway, we have been already executing raw iptables commands in the portmap plugin)

This commit replaces the isolation plugin proposal (issue #573, PR #574).

The design of ingressPolicy was discussed in the comments of the withdrawn PR #574 , but same-network was renamed to same-bridge then.

[AkihiroSuda]

@mars1024 @jellonek @squeed @bboreham Could you take a look?

[bboreham]

Thank you for this PR. Reading through the code it looks very competently done, although I have not taken the time to test it in different scenarios.

As we are in the process of releasing version 1.0 of CNI, we felt it best to hold off merging until that is out of the way. Should be a few weeks.

[erig0]

It should be also noted that the same-network ingress policy executes raw iptables commands directly, even when the backend is set to firewalld.
We could potentially use the "direct" API of firewalld to execute iptables via firewalld, but it doesn't seem to have a clear benefit over just directly executing raw iptables commands.
(Anyway, we have been already executing raw iptables commands in the portmap plugin)

firewalld v0.9.0+ supports policy objects which allows forward/output filtering. There are two ways you can achieve what you want.

1. single policy and block inter-bridge forwarding with rich rules - less flexible
   Create a single policy to affect any traffic received from a podman zone (e.g. trusted). Then use rich rules to block based on IP addresses. You'll need a rich rule for every permutation of address blocks. I think this would be the simplest approach for CNI based on its current implementation.
   In firewalld CLI this would look something like:

# firewall-cmd --permanent --new-policy noCNIInterForward
# firewall-cmd --permanent --policy noCNIInterForward --add-ingress-zone trusted
# firewall-cmd --permanent --policy noCNIInterForward --add-egress-zone trusted
# firewall-cmd --permanent --policy noCNIInterForward --add-rich-rule='rule family=ipv4 source address=10.0.88.0/24 destination address=10.0.89.0/24 drop`
# firewall-cmd --permanent --policy noCNIInterForward --add-rich-rule='rule family=ipv4 source address=10.0.89.0/24 destination address=10.0.88.0/24 drop`

2. 1:1 policy mapping - most flexible
   You can disallow the forwarding between zones. e.g. block cni-1 (zone podman-cni-1) <---> cni-2 (zone podman-cni-2). This requires the CNI bridges to be in different zones. This has the advantage that specifying IP addresses is not necessary. As such it works for both IPv4 and IPv6.
   In firewalld CLI this would look something like:

# firewall-cmd --permanent --new-policy noCNI1toCNI2
# firewall-cmd --permanent --policy noCNI1toCNI2 --set-target DROP
# firewall-cmd --permanent --policy noCNI1toCNI2 --add-ingress-zone podman-cni-1
# firewall-cmd --permanent --policy noCNI1toCNI2 --add-egress-zone podman-cni-2

# firewall-cmd --permanent --new-policy noCNI2toCNI1
# firewall-cmd --permanent --policy noCNI2toCNI1 --set-target DROP
# firewall-cmd --permanent --policy noCNI2toCNI1 --add-ingress-zone podman-cni-2
# firewall-cmd --permanent --policy noCNI2toCNI1 --add-egress-zone podman-cni-1

Ref:

• https://firewalld.org/2020/09/policy-objects-introduction
• https://firewalld.org/2020/09/policy-objects-filtering-container-and-vm-traffic

[AkihiroSuda]

@bboreham @erig0 Thanks a lot for reviewing this.

As we are in the process of releasing version 1.0 of CNI, we felt it best to hold off merging until that is out of the way. Should be a few weeks.

SGTM

firewalld v0.9.0+ supports policy objects which allows forward/output filtering.

firewalld >= 0.9.0 (Sep 2, 2020) seems available only in very new distros.
So, we also need to support invoking raw iptables for firewalld < 0.9.0, right?

There are two ways you can achieve what you want.

Thanks for the information. The first way sounds good.
Do I need to implement it in this PR, or can it be a separate PR?

[erig0]

@bboreham @erig0 Thanks a lot for reviewing this.

As we are in the process of releasing version 1.0 of CNI, we felt it best to hold off merging until that is out of the way. Should be a few weeks.

SGTM

firewalld v0.9.0+ supports policy objects which allows forward/output filtering.

firewalld >= 0.9.0 (Sep 2, 2020) seems available only in very new distros.
So, we also need to support invoking raw iptables for firewalld < 0.9.0, right?

Yes. You would need raw iptables support anyways for the distros that don't use firewalld.

There are two ways you can achieve what you want.

Thanks for the information. The first way sounds good.
Do I need to implement it in this PR, or can it be a separate PR?

I don't know. I'm not involved with CNI. I'm the firewalld maintainer. :)

[AkihiroSuda]

@bboreham

As we are in the process of releasing version 1.0 of CNI, we felt it best to hold off merging until that is out of the way. Should be a few weeks.

Is there an ETA?

[bboreham]

Really trying to get it out by KubeCon.

[06kellyjac]

Are there any updates post-KubeCon EU? 🙂

[AkihiroSuda]

Rebased with the current master for CNI v1.0.0

[AkihiroSuda]

@bboreham Could you take a look?

[bboreham]

Sorry I am no longer a CNI maintainer.
Your ping should get noticed at the regular weekly meeting (on Wednesday I think).

[AkihiroSuda]

Rebased

@squeed @dcbw Could you take a look?

[rhatdan]

@squeed @dcbw any update on this?

[AkihiroSuda]

Maybe we should consider reopening #574 (separate plugins/meta/isolation plugin), if it is easier to review?

[squeed]

Someday in the future, it might be nice to have L3 isolation, rather than relying on the bridge interface (e.g. for p2p "networks").

What if we call this "same-bridge" for now? That will make it explicit that this is dependent on bridges.

[AkihiroSuda]

Thanks for reviewing, renamed same-network to same-bridge. PTAL.

[squeed]

@AkihiroSuda Really sorry for the delayed review here. No excuse, just really busy.

I have one naming comment, because someday it might be nice for this to also work with p2p interfaces. Otherwise we can merge this as-is.

[AkihiroSuda]

Thank you @squeed !
Renamed same-network to same-bridge.

[dcbw]

/lgtm, though now that it's a year later we really should get a firewalld native version of this patch done too.

[AkihiroSuda]

Thank you for merging, opened doc PR:

• firewall: document ingressPolicy=(open|same-bridge) cni.dev#98