Skip to content

Commit

Permalink
Fix NP not working on Hairpin
Browse files Browse the repository at this point in the history
Fix #5681

Network policy didn't work when using a server Pod to establish a
connection to the service provided by itself. This hairpin service
connection initiated through a local Pod will be SNATed to the
gateway IP, which will prevent it from being correctly categorized by
the network policy during the Ingress rule enforcement.

This commit added a bypass flow to always allow the hairpin service
connection to address this issue. Given we don't consider self-access
blocking to be a valid case.

Signed-off-by: graysonwu <wgrayson@vmware.com>
  • Loading branch information
GraysonWu committed Nov 9, 2023
1 parent 799548c commit 59c17d0
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/agent/openflow/network_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1394,6 +1394,7 @@ func networkPolicyInitFlows(ovsMeterSupported, externalNodeEnabled, l7NetworkPol
"cookie=0x1020000000000, table=IngressSecurityClassifier, priority=200,reg0=0x20/0xf0 actions=goto_table:IngressMetric",
"cookie=0x1020000000000, table=IngressSecurityClassifier, priority=200,reg0=0x10/0xf0 actions=goto_table:IngressMetric",
"cookie=0x1020000000000, table=IngressSecurityClassifier, priority=200,reg0=0x40/0xf0 actions=goto_table:IngressMetric",
"cookie=0x1020000000000, table=IngressSecurityClassifier, priority=200,ct_mark=0x40/0x40 actions=goto_table:IngressMetric",
"cookie=0x1020000000000, table=AntreaPolicyEgressRule, priority=64990,ct_state=-new+est,ip actions=goto_table:EgressMetric",
"cookie=0x1020000000000, table=AntreaPolicyEgressRule, priority=64990,ct_state=-new+rel,ip actions=goto_table:EgressMetric",
"cookie=0x1020000000000, table=AntreaPolicyIngressRule, priority=64990,ct_state=-new+est,ip actions=goto_table:IngressMetric",
Expand Down
6 changes: 6 additions & 0 deletions pkg/agent/openflow/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -2272,6 +2272,12 @@ func (f *featureNetworkPolicy) ingressClassifierFlows() []binding.Flow {
MatchRegMark(ToUplinkRegMark).
Action().GotoTable(IngressMetricTable.GetID()).
Done(),
// This generates the flow to match the hairpin packets and forward them to IngressMetricTable.
IngressSecurityClassifierTable.ofTable.BuildFlow(priorityNormal).
Cookie(cookieID).
MatchCTMark(HairpinCTMark).
Action().GotoTable(IngressMetricTable.GetID()).
Done(),
}
if f.enableAntreaPolicy && f.proxyAll {
// This generates the flow to match the NodePort Service packets and forward them to AntreaPolicyIngressRuleTable.
Expand Down
75 changes: 75 additions & 0 deletions test/e2e/networkpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ func TestNetworkPolicy(t *testing.T) {
t.Cleanup(exportLogsForSubtest(t, data))
testIngressPolicyWithEndPort(t, data)
})
t.Run("testNamespaceIsolation", func(t *testing.T) {
t.Cleanup(exportLogsForSubtest(t, data))
testNamespaceIsolation(t, data)
})
}

func testNetworkPolicyStats(t *testing.T, data *TestData) {
Expand Down Expand Up @@ -929,6 +933,77 @@ func testIngressPolicyWithEndPort(t *testing.T, data *TestData) {
}
}

func testNamespaceIsolation(t *testing.T, data *TestData) {
serverNode := workerNodeName(1)
serverPort := int32(80)
serverName, _, cleanupFunc := createAndWaitForPod(t, data, data.createNginxPodOnNode, "test-server-", serverNode, data.testNamespace, false)
defer cleanupFunc()

service, err := data.CreateService("nginx", data.testNamespace, serverPort, serverPort, map[string]string{"app": "nginx"}, false, false, corev1.ServiceTypeClusterIP, nil)
if err != nil {
t.Fatalf("Error when creating nginx service: %v", err)
}
defer data.deleteService(service.Namespace, service.Name)

// client1 is a Pod in the same Namespace as the server Pod.
client1Name, _, cleanupFunc := createAndWaitForPod(t, data, data.createBusyboxPodOnNode, "test-client-can-connect-", serverNode, data.testNamespace, false)
defer cleanupFunc()

// client2 is a Pod in a different Namespace from the server Pod.
client2Name, _, cleanupFunc := createAndWaitForPod(t, data, data.createBusyboxPodOnNode, "test-client-cannot-connect-", serverNode, "default", false)
defer cleanupFunc()

spec := &networkingv1.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{},
PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress},
Ingress: []networkingv1.NetworkPolicyIngressRule{
{
From: []networkingv1.NetworkPolicyPeer{
{
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"kubernetes.io/metadata.name": data.testNamespace},
},
},
},
},
},
Egress: []networkingv1.NetworkPolicyEgressRule{
{
To: []networkingv1.NetworkPolicyPeer{
{
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"kubernetes.io/metadata.name": data.testNamespace},
},
},
},
},
},
}
np, err := data.createNetworkPolicy("test-networkpolicy-ns-iso", spec)
if err != nil {
t.Fatalf("Error when creating network policy: %v", err)
}
defer func() {
if err = data.deleteNetworkpolicy(np); err != nil {
t.Fatalf("Error when deleting network policy: %v", err)
}
}()

npCheck := func(clientName, serverIP, containerName string, serverPort int32, wantErr bool) {
if err = data.runNetcatCommandFromTestPodWithProtocol(clientName, data.testNamespace, containerName, serverIP, serverPort, "tcp"); wantErr && err == nil {
t.Fatalf("Pod %s should not be able to connect %s, but was able to connect", clientName, net.JoinHostPort(serverIP, fmt.Sprint(serverPort)))
} else if !wantErr && err != nil {
t.Fatalf("Pod %s should be able to connect %s, but was not able to connect", clientName, net.JoinHostPort(serverIP, fmt.Sprint(serverPort)))
}
}

for _, clusterIP := range service.Spec.ClusterIPs {
npCheck(serverName, clusterIP, nginxContainerName, serverPort, false)
npCheck(client1Name, clusterIP, busyboxContainerName, serverPort, false)
npCheck(client2Name, clusterIP, busyboxContainerName, serverPort, true)
}
}

func createAndWaitForPod(t *testing.T, data *TestData, createFunc func(name string, ns string, nodeName string, hostNetwork bool) error, namePrefix string, nodeName string, ns string, hostNetwork bool) (string, *PodIPs, func()) {
name := randName(namePrefix)
return createAndWaitForPodWithExactName(t, data, createFunc, name, nodeName, ns, hostNetwork)
Expand Down

0 comments on commit 59c17d0

Please sign in to comment.