From defcbb7935d3e1990b203ae6f727ebc8624cd29e Mon Sep 17 00:00:00 2001 From: Roberto Bonafiglia Date: Tue, 16 Jan 2024 18:12:13 +0100 Subject: [PATCH] Moved policy log outside of the loop to avoid logs of not dropped packets Signed-off-by: Roberto Bonafiglia --- pkg/controllers/netpol/policy.go | 62 ++++++++++++++++---------------- pkg/controllers/netpol/utils.go | 2 +- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/pkg/controllers/netpol/policy.go b/pkg/controllers/netpol/policy.go index ccd1a55a0..55d59d033 100644 --- a/pkg/controllers/netpol/policy.go +++ b/pkg/controllers/netpol/policy.go @@ -142,6 +142,20 @@ func (npc *NetworkPolicyController) syncNetworkPolicyChains(networkPoliciesInfo } activePolicyIPSets[targetSourcePodIPSetName] = true } + // Extra logs to get more information about the policy dropping the packet via ulog2 + logRuleComment := "\"rule to log dropped traffic\"" + + // The network policy annotation can include a log config + limit, limitBurst := getIptablesNFlogLimit(policy.annotations) + + // LogComment is capped at 64 characters and we are using 16, hence policyNamespace and policyName + // must fit in 48 characters otherwise we can only log the first 24 characters + policyNamespaceAndName := safeJoin(policy.namespace, policy.name) + logComment := "\"DROP by policy " + policyNamespaceAndName + "\"" + logArgs := []string{"-A", policyChainName, "-m", "comment", "--comment", logRuleComment, "-m", "limit", + "--limit", limit, "--limit-burst", limitBurst, "-m", "mark", "!", "--mark", "0x10000/0x10000", + "-j", "NFLOG", "--nflog-group", "100", "--nflog-prefix", logComment, "\n"} + npc.filterTableRules[ipFamily].WriteString(strings.Join(logArgs, " ")) } } @@ -227,7 +241,7 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, srcPodIPSetName, namedPortIPSetName, - endPoints.protocol, endPoints.port, endPoints.endport, ipFamily, policy); err != nil { + endPoints.protocol, endPoints.port, endPoints.endport, ipFamily); err != nil { return err } } @@ -240,7 +254,7 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, - comment, srcPodIPSetName, targetDestPodIPSetName, "", "", "", ipFamily, policy); err != nil { + comment, srcPodIPSetName, targetDestPodIPSetName, "", "", "", ipFamily); err != nil { return err } } @@ -254,7 +268,7 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo comment := "rule to ACCEPT traffic from all sources to dest pods selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, "", targetDestPodIPSetName, - portProtocol.protocol, portProtocol.port, portProtocol.endport, ipFamily, policy); err != nil { + portProtocol.protocol, portProtocol.port, portProtocol.endport, ipFamily); err != nil { return err } } @@ -272,7 +286,7 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo comment := "rule to ACCEPT traffic from all sources to dest pods selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, "", namedPortIPSetName, - endPoints.protocol, endPoints.port, endPoints.endport, ipFamily, policy); err != nil { + endPoints.protocol, endPoints.port, endPoints.endport, ipFamily); err != nil { return err } } @@ -284,7 +298,7 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo comment := "rule to ACCEPT traffic from all sources to dest pods selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, "", targetDestPodIPSetName, - "", "", "", ipFamily, policy); err != nil { + "", "", "", ipFamily); err != nil { return err } } @@ -301,7 +315,7 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, srcIPBlockIPSetName, targetDestPodIPSetName, portProtocol.protocol, portProtocol.port, - portProtocol.endport, ipFamily, policy); err != nil { + portProtocol.endport, ipFamily); err != nil { return err } } @@ -318,7 +332,7 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo comment := "rule to ACCEPT traffic from specified ipBlocks to dest pods selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, srcIPBlockIPSetName, namedPortIPSetName, - endPoints.protocol, endPoints.port, endPoints.endport, ipFamily, policy); err != nil { + endPoints.protocol, endPoints.port, endPoints.endport, ipFamily); err != nil { return err } } @@ -327,7 +341,7 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo comment := "rule to ACCEPT traffic from specified ipBlocks to dest pods selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, srcIPBlockIPSetName, - targetDestPodIPSetName, "", "", "", ipFamily, policy); err != nil { + targetDestPodIPSetName, "", "", "", ipFamily); err != nil { return err } } @@ -384,7 +398,7 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo, comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, targetSourcePodIPSetName, - namedPortIPSetName, endPoints.protocol, endPoints.port, endPoints.endport, ipFamily, policy); err != nil { + namedPortIPSetName, endPoints.protocol, endPoints.port, endPoints.endport, ipFamily); err != nil { return err } } @@ -397,7 +411,7 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo, comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, targetSourcePodIPSetName, - dstPodIPSetName, "", "", "", ipFamily, policy); err != nil { + dstPodIPSetName, "", "", "", ipFamily); err != nil { return err } } @@ -411,7 +425,7 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo, comment := "rule to ACCEPT traffic from source pods to all destinations selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, targetSourcePodIPSetName, - "", portProtocol.protocol, portProtocol.port, portProtocol.endport, ipFamily, policy); err != nil { + "", portProtocol.protocol, portProtocol.port, portProtocol.endport, ipFamily); err != nil { return err } } @@ -419,7 +433,7 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo, comment := "rule to ACCEPT traffic from source pods to all destinations selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, targetSourcePodIPSetName, - "", portProtocol.protocol, portProtocol.port, portProtocol.endport, ipFamily, policy); err != nil { + "", portProtocol.protocol, portProtocol.port, portProtocol.endport, ipFamily); err != nil { return err } } @@ -431,7 +445,7 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo, comment := "rule to ACCEPT traffic from source pods to all destinations selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, targetSourcePodIPSetName, - "", "", "", "", ipFamily, policy); err != nil { + "", "", "", "", ipFamily); err != nil { return err } } @@ -446,7 +460,7 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo, policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, targetSourcePodIPSetName, dstIPBlockIPSetName, portProtocol.protocol, portProtocol.port, - portProtocol.endport, ipFamily, policy); err != nil { + portProtocol.endport, ipFamily); err != nil { return err } } @@ -455,17 +469,18 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo, comment := "rule to ACCEPT traffic from source pods to specified ipBlocks selected by policy name: " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyChainName, comment, targetSourcePodIPSetName, - dstIPBlockIPSetName, "", "", "", ipFamily, policy); err != nil { + dstIPBlockIPSetName, "", "", "", ipFamily); err != nil { return err } } } } + return nil } func (npc *NetworkPolicyController) appendRuleToPolicyChain(policyChainName, comment, srcIPSetName, dstIPSetName, - protocol, dPort, endDport string, ipFamily api.IPFamily, policy networkPolicyInfo) error { + protocol, dPort, endDport string, ipFamily api.IPFamily) error { args := make([]string, 0) args = append(args, "-A", policyChainName) @@ -498,21 +513,6 @@ func (npc *NetworkPolicyController) appendRuleToPolicyChain(policyChainName, com args = append(args, "-m", "mark", "--mark", "0x10000/0x10000", "-j", "RETURN", "\n") npc.filterTableRules[ipFamily].WriteString(strings.Join(args, " ")) - // Extra logs to get more information about the policy dropping the packet via ulog2 - logRuleComment := "\"rule to log dropped traffic\"" - - // The network policy annotation can include a log config - limit, limitBurst := getIptablesNFlogLimit(policy.annotations) - - // LogComment is capped at 64 characters and we are using 16, hence policyNamespace and policyName - // must fit in 48 characters otherwise we can only log the first 24 characters - policyNamespaceAndName := safeJoin(policy.namespace, policy.name) - logComment := "\"DROP by policy " + policyNamespaceAndName + "\"" - logArgs := []string{"-A", policyChainName, "-m", "comment", "--comment", logRuleComment, "-m", "limit", - "--limit", limit, "--limit-burst", limitBurst, "-m", "mark", "!", "--mark", "0x10000/0x10000", - "-j", "NFLOG", "--nflog-group", "100", "--nflog-prefix", logComment, "\n"} - npc.filterTableRules[ipFamily].WriteString(strings.Join(logArgs, " ")) - return nil } diff --git a/pkg/controllers/netpol/utils.go b/pkg/controllers/netpol/utils.go index 3f4def8f5..66e759a99 100644 --- a/pkg/controllers/netpol/utils.go +++ b/pkg/controllers/netpol/utils.go @@ -125,7 +125,7 @@ func (npc *NetworkPolicyController) createPodWithPortPolicyRule(ports []protocol comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " + policy.name + " namespace " + policy.namespace if err := npc.appendRuleToPolicyChain(policyName, comment, srcSetName, dstSetName, portProtocol.protocol, - portProtocol.port, portProtocol.endport, ipFamily, policy); err != nil { + portProtocol.port, portProtocol.endport, ipFamily); err != nil { return err } }