openshift / openshift-sdn

Apache License 2.0
69 stars 63 forks source link

Egress firewall support #319

Closed danwinship closed 8 years ago

danwinship commented 8 years ago

This is missing the origin side, but that side doesn't quite work yet anyway. (Some problem with how I've defined the new resource type... it worked before when I had the firewall rules on the ClusterNetwork object instead of on a new type of object.) The interesting bit from origin is:

+// EgressFirewallPolicy gives the type of an EgressFirewallRule
+type EgressFirewallPolicy string
+
+const (
+   EgressFirewallPolicyAllow EgressFirewallPolicy = "allow"
+   EgressFirewallPolicyDeny  EgressFirewallPolicy = "deny"
+)
+
+// EgressFirewallRule contains a single outgoing traffic firewall rule
+type EgressFirewallRule struct {
+   Policy EgressFirewallPolicy
+   From   string // Namespace name or empty
+   To     string // CIDR range
+}
+
+// EgressFirewall provides a list of firewall rules on outgoing traffic
+type EgressFirewall struct {
+   unversioned.TypeMeta
+   kapi.ObjectMeta
+
+   Rules []EgressFirewallRule
+}

To avoid race conditions when updating the firewall rules, there are actually 3 tables in OVS for the firewall: table 9, which says "output:2" when there are no firewall rules, or "goto_table:10" or "goto_table:11" when there are, and then the actual rules are in either table 10 or 11, where only one of them is in use at any time, and when we need to update the rules, we update them into the other table, and then change table 9 to point to it only after everything is done.

After implementing that though, I realized that there are still other issues. Eg, if a namespace is firewalled, and you change its VNID, then there's a race condition, where some traffic coming out of a pod in that namespace could get assigned the new VNID while the firewall rules are still looking for the old VNID. Meh.

@openshift/networking PTAL. See https://trello.com/c/iH5IjWI8 for the use cases. As per earlier email, this is basically completely ignoring anything that might be going on upstream with respect to egress policy, due to timing/scope, but we do at least hopefully want this to be a subset of what eventually gets implemented upstream.

dcbw commented 8 years ago

Looks OK to me, just a small nit.

danwinship commented 8 years ago

So this has the same service Endpoint problem as multitenant used to have; since the firewalling happens in OVS, you can bypass it by manually creating a service endpoint pointing to the firewalled address; that translation won't happen until after the traffic leaves OVS, at which point it's too late.

eparis commented 8 years ago

well that sounds like a non-starter for a lot of people, probably...

danwinship commented 8 years ago

merged into https://github.com/openshift/origin/pull/9227