From 4edd79c1b09c4d2aac22d5a7f08811e6a8fbf213 Mon Sep 17 00:00:00 2001 From: Qiyue Yao Date: Wed, 11 Dec 2024 14:48:40 -0800 Subject: [PATCH] Fix K8s NetworkPolicy Deny All Audit Logging EnableLogging was not set for default-deny-all K8s NetworkPolicy even with Namespace properly annotated. This PR fixes the issue by configuring enableLogging field. Fixes #6844 Signed-off-by: Qiyue Yao --- .../networkpolicy/networkpolicy_controller.go | 13 ++++- .../networkpolicy_controller_test.go | 35 ++++++++++++++ test/e2e/antreapolicy_test.go | 47 ++++++++++++++----- 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/pkg/controller/networkpolicy/networkpolicy_controller.go b/pkg/controller/networkpolicy/networkpolicy_controller.go index 1026d03473c..c01cf9142ab 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller.go @@ -769,15 +769,24 @@ func (n *NetworkPolicyController) processNetworkPolicy(np *networkingv1.NetworkP } } + // Set EnableLogging field for the given rule. + configureEnableLogging := func(originalRule controlplane.NetworkPolicyRule) controlplane.NetworkPolicyRule { + logConfiguredRule := originalRule + if enableLogging { + logConfiguredRule.EnableLogging = enableLogging + } + return logConfiguredRule + } + // If ingress isolation is specified explicitly and there's no ingress rule, append a deny-all ingress rule. // See https://kubernetes.io/docs/concepts/services-networking/network-policies/#default-deny-all-ingress-traffic if ingressIsolated && !ingressRuleExists { - rules = append(rules, denyAllIngressRule) + rules = append(rules, configureEnableLogging(denyAllIngressRule)) } // If egress isolation is specified explicitly and there's no egress rule, append a deny-all egress rule. // See https://kubernetes.io/docs/concepts/services-networking/network-policies/#default-deny-all-egress-traffic if egressIsolated && !egressRuleExists { - rules = append(rules, denyAllEgressRule) + rules = append(rules, configureEnableLogging(denyAllEgressRule)) } internalNetworkPolicy := &antreatypes.NetworkPolicy{ diff --git a/pkg/controller/networkpolicy/networkpolicy_controller_test.go b/pkg/controller/networkpolicy/networkpolicy_controller_test.go index 3fe5dd05efd..7ad6279c75a 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller_test.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller_test.go @@ -2007,6 +2007,41 @@ func TestProcessNetworkPolicy(t *testing.T) { expectedAppliedToGroups: 1, expectedAddressGroups: 0, }, + { + name: "default-deny-ingress-enable-logging", + existingObjects: []runtime.Object{ + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nsA", + Annotations: map[string]string{"networkpolicy.antrea.io/enable-logging": "true"}, + }, + }, + }, + inputPolicy: &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "npC", UID: "uidC"}, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + }, + }, + expectedPolicy: &antreatypes.NetworkPolicy{ + UID: "uidC", + Name: "uidC", + SourceRef: &controlplane.NetworkPolicyReference{ + Type: controlplane.K8sNetworkPolicy, + Namespace: "nsA", + Name: "npC", + UID: "uidC", + }, + Rules: []controlplane.NetworkPolicyRule{{ + Direction: controlplane.DirectionIn, + EnableLogging: true, + }}, + AppliedToGroups: []string{getNormalizedUID(antreatypes.NewGroupSelector("nsA", &metav1.LabelSelector{}, nil, nil, nil).NormalizedName)}, + }, + expectedAppliedToGroups: 1, + expectedAddressGroups: 0, + }, { name: "default-deny-egress", inputPolicy: &networkingv1.NetworkPolicy{ diff --git a/test/e2e/antreapolicy_test.go b/test/e2e/antreapolicy_test.go index 7f7c44829bb..26f0eb79092 100644 --- a/test/e2e/antreapolicy_test.go +++ b/test/e2e/antreapolicy_test.go @@ -2752,6 +2752,7 @@ func testAuditLoggingBasic(t *testing.T, data *TestData) { // tests both Allow traffic by K8s NP and Drop traffic by implicit K8s policy drop func testAuditLoggingEnableK8s(t *testing.T, data *TestData) { failOnError(data.updateNamespaceWithAnnotations(getNS("x"), map[string]string{networkpolicy.EnableNPLoggingAnnotationKey: "true"}), t) + failOnError(data.updateNamespaceWithAnnotations(getNS("y"), map[string]string{networkpolicy.EnableNPLoggingAnnotationKey: "true"}), t) // Add a K8s namespaced NetworkPolicy in ns x that allow ingress traffic from // Pod x/b to x/a which default denies other ingress including from Pod x/c to x/a npName := "allow-x-b-to-x-a" @@ -2767,21 +2768,38 @@ func testAuditLoggingEnableK8s(t *testing.T, data *TestData) { failOnError(err, t) failOnError(waitForResourceReady(t, timeout, knp), t) + // Add a K8s namespaced NetworkPolicy with no ingress rule that triggers the + // default deny all ingress traffic. + npName2 := "default-deny-all-y" + k8sNPBuilder2 := &NetworkPolicySpecBuilder{} + k8sNPBuilder2 = k8sNPBuilder2.SetName(getNS("y"), npName2).SetTypeIngress() + + knp2, err := k8sUtils.CreateOrUpdateNetworkPolicy(k8sNPBuilder2.Get()) + failOnError(err, t) + failOnError(waitForResourceReady(t, timeout, knp2), t) + podXA, err := k8sUtils.GetPodByLabel(getNS("x"), "a") if err != nil { t.Errorf("Failed to get Pod in Namespace x with label 'pod=a': %v", err) } + podYA, err := k8sUtils.GetPodByLabel(getNS("y"), "a") + if err != nil { + t.Errorf("Failed to get Pod in Namespace y with label 'pod=a': %v", err) + } - // matcher1 is for connections allowed by the K8s NP - matcher1 := NewAuditLogMatcher(npRef, "", "Ingress", "Allow") - // matcher2 is for connections dropped by the isolated behavior of the K8s NP - matcher2 := NewAuditLogMatcher("K8sNetworkPolicy", "", "Ingress", "Drop") + // matcherX1 is for connections allowed by the K8s NP1 + matcherX1 := NewAuditLogMatcher(npRef, "", "Ingress", "Allow") + // matcherX2 is for connections dropped by the isolated behavior of the K8s NP1 + matcherX2 := NewAuditLogMatcher("K8sNetworkPolicy", "", "Ingress", "Drop") + // matcherY is for connections dropped by the default deny all behavior of the K8s NP2 + matcherY := NewAuditLogMatcher("K8sNetworkPolicy", "", "Ingress", "Drop") - appliedToRef := fmt.Sprintf("%s/%s", podXA.Namespace, podXA.Name) + appliedToRefX := fmt.Sprintf("%s/%s", podXA.Namespace, podXA.Name) + appliedToRefY := fmt.Sprintf("%s/%s", podYA.Namespace, podYA.Name) // generate some traffic that will be dropped by implicit K8s policy drop var wg sync.WaitGroup - oneProbe := func(ns1, pod1, ns2, pod2 string, matcher *auditLogMatcher) { + oneProbe := func(appliedToRef, ns1, pod1, ns2, pod2 string, matcher *auditLogMatcher) { matcher.AddProbe(appliedToRef, ns1, pod1, ns2, pod2, p80) wg.Add(1) go func() { @@ -2789,18 +2807,23 @@ func testAuditLoggingEnableK8s(t *testing.T, data *TestData) { k8sUtils.Probe(ns1, pod1, ns2, pod2, p80, ProtocolTCP, nil, nil) }() } - oneProbe(getNS("x"), "b", getNS("x"), "a", matcher1) - oneProbe(getNS("x"), "c", getNS("x"), "a", matcher2) + oneProbe(appliedToRefX, getNS("x"), "b", getNS("x"), "a", matcherX1) + oneProbe(appliedToRefX, getNS("x"), "c", getNS("x"), "a", matcherX2) + oneProbe(appliedToRefY, getNS("y"), "b", getNS("y"), "a", matcherY) wg.Wait() - // nodeName is guaranteed to be set at this stage, since the framework waits for all Pods to be in Running phase - nodeName := podXA.Spec.NodeName - checkAuditLoggingResult(t, data, nodeName, "K8sNetworkPolicy", append(matcher1.Matchers(), matcher2.Matchers()...)) + // pod NodeName is guaranteed to be set at this stage, since the framework waits for all Pods to be in Running phase + checkAuditLoggingResult(t, data, podXA.Spec.NodeName, "K8sNetworkPolicy", append(matcherX1.Matchers(), matcherX2.Matchers()...)) + checkAuditLoggingResult(t, data, podYA.Spec.NodeName, "K8sNetworkPolicy", matcherY.Matchers()) - failOnError(k8sUtils.DeleteNetworkPolicy(getNS("x"), "allow-x-b-to-x-a"), t) + failOnError(k8sUtils.DeleteNetworkPolicy(getNS("x"), npName), t) + failOnError(k8sUtils.DeleteNetworkPolicy(getNS("y"), npName2), t) failOnError(data.UpdateNamespace(getNS("x"), func(namespace *v1.Namespace) { delete(namespace.Annotations, networkpolicy.EnableNPLoggingAnnotationKey) }), t) + failOnError(data.UpdateNamespace(getNS("y"), func(namespace *v1.Namespace) { + delete(namespace.Annotations, networkpolicy.EnableNPLoggingAnnotationKey) + }), t) } // testAuditLoggingK8sService tests that audit logs are generated for K8s Service access