drop-forwarding: Add ClusterSubnets to allowed forwarding CIDRs

Commit f062cbb introduced a default FORWARD DROP for breth0 and allow-listed Service CIDRs and the masquerade IPs. However, this breaks Egress Service traffic. Add the ClusterCIDRs to the allow-list. Reported-at: ovn-org/ovn-kubernetes#4042 Signed-off-by: Andreas Karis <ak.karis@gmail.com> (cherry picked from commit e071df4)
openshift · May 9, 2024 · 21402bf · 21402bf
1 parent 21ab231
commit 21402bf
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 45 deletions.
diff --git a/go-controller/pkg/node/gateway_init_linux_test.go b/go-controller/pkg/node/gateway_init_linux_test.go
@@ -1182,6 +1182,8 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`,
 					"-s 169.254.169.1 -j ACCEPT",
 					"-d 172.16.1.0/24 -j ACCEPT",
 					"-s 172.16.1.0/24 -j ACCEPT",
+					"-d 10.1.0.0/16 -j ACCEPT",
+					"-s 10.1.0.0/16 -j ACCEPT",
 					"-i ovn-k8s-mp0 -j ACCEPT",
 					"-o ovn-k8s-mp0 -j ACCEPT",
 					"-i breth0 -j DROP",

diff --git a/go-controller/pkg/node/gateway_iptables.go b/go-controller/pkg/node/gateway_iptables.go
@@ -279,50 +279,66 @@ func getExternalIPTRules(svcPort kapi.ServicePort, externalIP, dstIP string, svc
 	}
 }
 
-func getGatewayForwardRules(svcCIDR *net.IPNet) []nodeipt.Rule {
-	protocol := getIPTablesProtocol(svcCIDR.IP.String())
-	masqueradeIP := config.Gateway.MasqueradeIPs.V4OVNMasqueradeIP
-	if protocol == iptables.ProtocolIPv6 {
-		masqueradeIP = config.Gateway.MasqueradeIPs.V6OVNMasqueradeIP
-	}
-	return []nodeipt.Rule{
-		{
-			Table: "filter",
-			Chain: "FORWARD",
-			Args: []string{
-				"-s", svcCIDR.String(),
-				"-j", "ACCEPT",
+func getGatewayForwardRules(cidrs []*net.IPNet) []nodeipt.Rule {
+	var returnRules []nodeipt.Rule
+	protocols := make(map[iptables.Protocol]struct{})
+
+	// Add rules for all CIDRs.
+	for _, cidr := range cidrs {
+		protocol := getIPTablesProtocol(cidr.IP.String())
+		protocols[protocol] = struct{}{}
+
+		returnRules = append(returnRules, []nodeipt.Rule{
+			{
+				Table: "filter",
+				Chain: "FORWARD",
+				Args: []string{
+					"-s", cidr.String(),
+					"-j", "ACCEPT",
+				},
+				Protocol: protocol,
 			},
-			Protocol: protocol,
-		},
-		{
-			Table: "filter",
-			Chain: "FORWARD",
-			Args: []string{
-				"-d", svcCIDR.String(),
-				"-j", "ACCEPT",
+			{
+				Table: "filter",
+				Chain: "FORWARD",
+				Args: []string{
+					"-d", cidr.String(),
+					"-j", "ACCEPT",
+				},
+				Protocol: protocol,
 			},
-			Protocol: protocol,
-		},
-		{
-			Table: "filter",
-			Chain: "FORWARD",
-			Args: []string{
-				"-s", masqueradeIP.String(),
-				"-j", "ACCEPT",
+		}...)
+	}
+
+	// Add rules for MasqueraIPs.
+	for protocol := range protocols {
+		masqueradeIP := config.Gateway.MasqueradeIPs.V4OVNMasqueradeIP
+		if protocol == iptables.ProtocolIPv6 {
+			masqueradeIP = config.Gateway.MasqueradeIPs.V6OVNMasqueradeIP
+		}
+		returnRules = append(returnRules, []nodeipt.Rule{
+			{
+				Table: "filter",
+				Chain: "FORWARD",
+				Args: []string{
+					"-s", masqueradeIP.String(),
+					"-j", "ACCEPT",
+				},
+				Protocol: protocol,
 			},
-			Protocol: protocol,
-		},
-		{
-			Table: "filter",
-			Chain: "FORWARD",
-			Args: []string{
-				"-d", masqueradeIP.String(),
-				"-j", "ACCEPT",
+			{
+				Table: "filter",
+				Chain: "FORWARD",
+				Args: []string{
+					"-d", masqueradeIP.String(),
+					"-j", "ACCEPT",
+				},
+				Protocol: protocol,
 			},
-			Protocol: protocol,
-		},
+		}...)
 	}
+
+	return returnRules
 }
 
 func getGatewayDropRules(ifName string) []nodeipt.Rule {
@@ -357,8 +373,8 @@ func getGatewayDropRules(ifName string) []nodeipt.Rule {
 // -A FORWARD -d 10.96.0.0/16 -j ACCEPT
 // -A FORWARD -s 169.254.169.1 -j ACCEPT
 // -A FORWARD -d 169.254.169.1 -j ACCEPT
-func initExternalBridgeServiceForwardingRules(cidr *net.IPNet) error {
-	return insertIptRules(getGatewayForwardRules(cidr))
+func initExternalBridgeServiceForwardingRules(cidrs []*net.IPNet) error {
+	return insertIptRules(getGatewayForwardRules(cidrs))
 }
 
 // initExternalBridgeDropRules sets up iptables rules to block forwarding

diff --git a/go-controller/pkg/node/gateway_shared_intf.go b/go-controller/pkg/node/gateway_shared_intf.go
@@ -1861,10 +1861,13 @@ func newNodePortWatcher(gwBridge *bridgeConfiguration, ofm *openflowManager,
 	}
 
 	if config.Gateway.DisableForwarding {
-		for _, subnet := range config.Kubernetes.ServiceCIDRs {
-			if err := initExternalBridgeServiceForwardingRules(subnet); err != nil {
-				return nil, fmt.Errorf("failed to add forwarding rules for bridge %s: err %v", gwBridge.bridgeName, err)
-			}
+		var subnets []*net.IPNet
+		for _, subnet := range config.Default.ClusterSubnets {
+			subnets = append(subnets, subnet.CIDR)
+		}
+		subnets = append(subnets, config.Kubernetes.ServiceCIDRs...)
+		if err := initExternalBridgeServiceForwardingRules(subnets); err != nil {
+			return nil, fmt.Errorf("failed to add forwarding rules for bridge %s: err %v", gwBridge.bridgeName, err)
 		}
 		if err := initExternalBridgeDropForwardingRules(gwBridge.bridgeName); err != nil {
 			return nil, fmt.Errorf("failed to add forwarding rules for bridge %s: err %v", gwBridge.bridgeName, err)