From 31612729ef07e7bb7dbbc0043f79b04dfd71321d Mon Sep 17 00:00:00 2001 From: vakr Date: Thu, 31 Mar 2022 13:26:28 -0700 Subject: [PATCH 1/4] fix: [NPM] updating NPM marks with a mask to not clash with kube-proxy --- npm/iptm/helper.go | 11 ----------- npm/pkg/dataplane/policies/chain-management_linux.go | 4 ---- npm/util/const.go | 9 +++++---- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/npm/iptm/helper.go b/npm/iptm/helper.go index 82735a2b7b..81f1e9fe68 100755 --- a/npm/iptm/helper.go +++ b/npm/iptm/helper.go @@ -100,17 +100,6 @@ func getAzureNPMChainRules() [][]string { // getAzureNPMAcceptChainRules clears all marks and accepts packets func getAzureNPMAcceptChainRules() [][]string { return [][]string{ - { - util.IptablesAzureAcceptChain, - util.IptablesJumpFlag, - util.IptablesMark, - util.IptablesSetMarkFlag, - util.IptablesAzureClearMarkHex, - util.IptablesModuleFlag, - util.IptablesCommentModuleFlag, - util.IptablesCommentFlag, - "Clear-AZURE-NPM-MARKS", - }, { util.IptablesAzureAcceptChain, util.IptablesJumpFlag, diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index f913578fcc..8983ef5387 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -321,10 +321,6 @@ func (pMgr *PolicyManager) creatorForBootup(currentChains map[string]struct{}) * creator.AddLine("", nil, jumpOnIngressMatchSpecs...) // add AZURE-NPM-ACCEPT chain rules - clearSpecs := []string{util.IptablesAppendFlag, util.IptablesAzureAcceptChain} - clearSpecs = append(clearSpecs, setMarkSpecs(util.IptablesAzureClearMarkHex)...) - clearSpecs = append(clearSpecs, commentSpecs("CLEAR-AZURE-NPM-MARKS")...) - creator.AddLine("", nil, clearSpecs...) creator.AddLine("", nil, util.IptablesAppendFlag, util.IptablesAzureAcceptChain, util.IptablesJumpFlag, util.IptablesAccept) creator.AddLine("", nil, util.IptablesRestoreCommit) return creator diff --git a/npm/util/const.go b/npm/util/const.go index 60c9554bee..4fe62dceda 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -116,12 +116,13 @@ const ( IptablesAzureEgressToPodChain string = "AZURE-NPM-EGRESS-TO-POD" // Below are the skb->mark NPM will use for different criteria - IptablesAzureClearMarkHex string = "0x0" + // Deprecated + IptablesAzureClearMarkHex string = "0x0/0x00000F00" // marks in NPM v2 - IptablesAzureIngressAllowMarkHex string = "0x2000" // same as old IptablesAzureIngressMarkHex - IptablesAzureIngressDropMarkHex string = "0x4000" - IptablesAzureEgressDropMarkHex string = "0x5000" + IptablesAzureIngressAllowMarkHex string = "0x200/0x200" // same as old IptablesAzureIngressMarkHex + IptablesAzureIngressDropMarkHex string = "0x400/0x400" + IptablesAzureEgressDropMarkHex string = "0x500/0x500" // marks in NPM v1 IptablesAzureIngressMarkHex string = "0x2000" From 7c383f88de9b3b28cf1d726738bddbff9d9f2090 Mon Sep 17 00:00:00 2001 From: vakr Date: Tue, 5 Apr 2022 12:47:49 -0700 Subject: [PATCH 2/4] updating mark to single bit instead of 2 bits for 500 --- npm/util/const.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/npm/util/const.go b/npm/util/const.go index 4fe62dceda..9bc0622d57 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -117,12 +117,15 @@ const ( // Below are the skb->mark NPM will use for different criteria // Deprecated - IptablesAzureClearMarkHex string = "0x0/0x00000F00" + IptablesAzureClearMarkHex string = "0x0/0xE00" // marks in NPM v2 - IptablesAzureIngressAllowMarkHex string = "0x200/0x200" // same as old IptablesAzureIngressMarkHex + // NPM uses the 3rd word of the 32-bit mark for the purpose of + // identifying the traffic direction and decision making. + // NPM uses 9th, 10th and 11th bit for marking + IptablesAzureIngressAllowMarkHex string = "0x200/0x200" IptablesAzureIngressDropMarkHex string = "0x400/0x400" - IptablesAzureEgressDropMarkHex string = "0x500/0x500" + IptablesAzureEgressDropMarkHex string = "0x800/0x800" // marks in NPM v1 IptablesAzureIngressMarkHex string = "0x2000" From 35655583fdf99cdf7c708a7fc550f31b7cf14017 Mon Sep 17 00:00:00 2001 From: vakr Date: Wed, 6 Apr 2022 10:09:30 -0700 Subject: [PATCH 3/4] not touching v1 and adjusting UTs for V2 --- npm/iptm/helper.go | 11 ++++++ .../policies/chain-management_linux_test.go | 36 +++++++++---------- npm/util/const.go | 6 ++-- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/npm/iptm/helper.go b/npm/iptm/helper.go index 81f1e9fe68..82735a2b7b 100755 --- a/npm/iptm/helper.go +++ b/npm/iptm/helper.go @@ -100,6 +100,17 @@ func getAzureNPMChainRules() [][]string { // getAzureNPMAcceptChainRules clears all marks and accepts packets func getAzureNPMAcceptChainRules() [][]string { return [][]string{ + { + util.IptablesAzureAcceptChain, + util.IptablesJumpFlag, + util.IptablesMark, + util.IptablesSetMarkFlag, + util.IptablesAzureClearMarkHex, + util.IptablesModuleFlag, + util.IptablesCommentModuleFlag, + util.IptablesCommentFlag, + "Clear-AZURE-NPM-MARKS", + }, { util.IptablesAzureAcceptChain, util.IptablesJumpFlag, diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index 1713a6734a..a33ca1d809 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -213,12 +213,11 @@ func TestCreatorForBootup(t *testing.T) { ":AZURE-NPM-INGRESS-ALLOW-MARK - -", ":AZURE-NPM-EGRESS - -", ":AZURE-NPM-ACCEPT - -", - "-A AZURE-NPM-INGRESS -j DROP -m mark --mark 0x4000 -m comment --comment DROP-ON-INGRESS-DROP-MARK-0x4000", - "-A AZURE-NPM-INGRESS-ALLOW-MARK -j MARK --set-mark 0x2000 -m comment --comment SET-INGRESS-ALLOW-MARK-0x2000", + "-A AZURE-NPM-INGRESS -j DROP -m mark --mark 0x400/0x400 -m comment --comment DROP-ON-INGRESS-DROP-MARK-0x400/0x400", + "-A AZURE-NPM-INGRESS-ALLOW-MARK -j MARK --set-mark 0x200/0x200 -m comment --comment SET-INGRESS-ALLOW-MARK-0x200/0x200", "-A AZURE-NPM-INGRESS-ALLOW-MARK -j AZURE-NPM-EGRESS", - "-A AZURE-NPM-EGRESS -j DROP -m mark --mark 0x5000 -m comment --comment DROP-ON-EGRESS-DROP-MARK-0x5000", - "-A AZURE-NPM-EGRESS -j AZURE-NPM-ACCEPT -m mark --mark 0x2000 -m comment --comment ACCEPT-ON-INGRESS-ALLOW-MARK-0x2000", - "-A AZURE-NPM-ACCEPT -j MARK --set-mark 0x0 -m comment --comment CLEAR-AZURE-NPM-MARKS", + "-A AZURE-NPM-EGRESS -j DROP -m mark --mark 0x800/0x800 -m comment --comment DROP-ON-EGRESS-DROP-MARK-0x800/0x800", + "-A AZURE-NPM-EGRESS -j AZURE-NPM-ACCEPT -m mark --mark 0x200/0x200 -m comment --comment ACCEPT-ON-INGRESS-ALLOW-MARK-0x200/0x200", "-A AZURE-NPM-ACCEPT -j ACCEPT", "COMMIT", "", @@ -246,12 +245,11 @@ func TestCreatorForBootup(t *testing.T) { "-F AZURE-NPM-ACCEPT", "-F AZURE-NPM-INGRESS-123456", "-F AZURE-NPM-EGRESS-123456", - "-A AZURE-NPM-INGRESS -j DROP -m mark --mark 0x4000 -m comment --comment DROP-ON-INGRESS-DROP-MARK-0x4000", - "-A AZURE-NPM-INGRESS-ALLOW-MARK -j MARK --set-mark 0x2000 -m comment --comment SET-INGRESS-ALLOW-MARK-0x2000", + "-A AZURE-NPM-INGRESS -j DROP -m mark --mark 0x400/0x400 -m comment --comment DROP-ON-INGRESS-DROP-MARK-0x400/0x400", + "-A AZURE-NPM-INGRESS-ALLOW-MARK -j MARK --set-mark 0x200/0x200 -m comment --comment SET-INGRESS-ALLOW-MARK-0x200/0x200", "-A AZURE-NPM-INGRESS-ALLOW-MARK -j AZURE-NPM-EGRESS", - "-A AZURE-NPM-EGRESS -j DROP -m mark --mark 0x5000 -m comment --comment DROP-ON-EGRESS-DROP-MARK-0x5000", - "-A AZURE-NPM-EGRESS -j AZURE-NPM-ACCEPT -m mark --mark 0x2000 -m comment --comment ACCEPT-ON-INGRESS-ALLOW-MARK-0x2000", - "-A AZURE-NPM-ACCEPT -j MARK --set-mark 0x0 -m comment --comment CLEAR-AZURE-NPM-MARKS", + "-A AZURE-NPM-EGRESS -j DROP -m mark --mark 0x800/0x800 -m comment --comment DROP-ON-EGRESS-DROP-MARK-0x800/0x800", + "-A AZURE-NPM-EGRESS -j AZURE-NPM-ACCEPT -m mark --mark 0x200/0x200 -m comment --comment ACCEPT-ON-INGRESS-ALLOW-MARK-0x200/0x200", "-A AZURE-NPM-ACCEPT -j ACCEPT", "COMMIT", "", @@ -276,12 +274,11 @@ func TestCreatorForBootup(t *testing.T) { "-F AZURE-NPM-ACCEPT", "-F AZURE-NPM-INGRESS", "-F AZURE-NPM-INGRESS-ALLOW-MARK", - "-A AZURE-NPM-INGRESS -j DROP -m mark --mark 0x4000 -m comment --comment DROP-ON-INGRESS-DROP-MARK-0x4000", - "-A AZURE-NPM-INGRESS-ALLOW-MARK -j MARK --set-mark 0x2000 -m comment --comment SET-INGRESS-ALLOW-MARK-0x2000", + "-A AZURE-NPM-INGRESS -j DROP -m mark --mark 0x400/0x400 -m comment --comment DROP-ON-INGRESS-DROP-MARK-0x400/0x400", + "-A AZURE-NPM-INGRESS-ALLOW-MARK -j MARK --set-mark 0x200/0x200 -m comment --comment SET-INGRESS-ALLOW-MARK-0x200/0x200", "-A AZURE-NPM-INGRESS-ALLOW-MARK -j AZURE-NPM-EGRESS", - "-A AZURE-NPM-EGRESS -j DROP -m mark --mark 0x5000 -m comment --comment DROP-ON-EGRESS-DROP-MARK-0x5000", - "-A AZURE-NPM-EGRESS -j AZURE-NPM-ACCEPT -m mark --mark 0x2000 -m comment --comment ACCEPT-ON-INGRESS-ALLOW-MARK-0x2000", - "-A AZURE-NPM-ACCEPT -j MARK --set-mark 0x0 -m comment --comment CLEAR-AZURE-NPM-MARKS", + "-A AZURE-NPM-EGRESS -j DROP -m mark --mark 0x800/0x800 -m comment --comment DROP-ON-EGRESS-DROP-MARK-0x800/0x800", + "-A AZURE-NPM-EGRESS -j AZURE-NPM-ACCEPT -m mark --mark 0x200/0x200 -m comment --comment ACCEPT-ON-INGRESS-ALLOW-MARK-0x200/0x200", "-A AZURE-NPM-ACCEPT -j ACCEPT", "COMMIT", "", @@ -305,12 +302,11 @@ func TestCreatorForBootup(t *testing.T) { "-F AZURE-NPM-EGRESS-DROPS", "-F AZURE-NPM-EGRESS-FROM", "-F AZURE-NPM-EGRESS-PORTS", - "-A AZURE-NPM-INGRESS -j DROP -m mark --mark 0x4000 -m comment --comment DROP-ON-INGRESS-DROP-MARK-0x4000", - "-A AZURE-NPM-INGRESS-ALLOW-MARK -j MARK --set-mark 0x2000 -m comment --comment SET-INGRESS-ALLOW-MARK-0x2000", + "-A AZURE-NPM-INGRESS -j DROP -m mark --mark 0x400/0x400 -m comment --comment DROP-ON-INGRESS-DROP-MARK-0x400/0x400", + "-A AZURE-NPM-INGRESS-ALLOW-MARK -j MARK --set-mark 0x200/0x200 -m comment --comment SET-INGRESS-ALLOW-MARK-0x200/0x200", "-A AZURE-NPM-INGRESS-ALLOW-MARK -j AZURE-NPM-EGRESS", - "-A AZURE-NPM-EGRESS -j DROP -m mark --mark 0x5000 -m comment --comment DROP-ON-EGRESS-DROP-MARK-0x5000", - "-A AZURE-NPM-EGRESS -j AZURE-NPM-ACCEPT -m mark --mark 0x2000 -m comment --comment ACCEPT-ON-INGRESS-ALLOW-MARK-0x2000", - "-A AZURE-NPM-ACCEPT -j MARK --set-mark 0x0 -m comment --comment CLEAR-AZURE-NPM-MARKS", + "-A AZURE-NPM-EGRESS -j DROP -m mark --mark 0x800/0x800 -m comment --comment DROP-ON-EGRESS-DROP-MARK-0x800/0x800", + "-A AZURE-NPM-EGRESS -j AZURE-NPM-ACCEPT -m mark --mark 0x200/0x200 -m comment --comment ACCEPT-ON-INGRESS-ALLOW-MARK-0x200/0x200", "-A AZURE-NPM-ACCEPT -j ACCEPT", "COMMIT", "", diff --git a/npm/util/const.go b/npm/util/const.go index 9bc0622d57..2f5c58de72 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -116,8 +116,10 @@ const ( IptablesAzureEgressToPodChain string = "AZURE-NPM-EGRESS-TO-POD" // Below are the skb->mark NPM will use for different criteria - // Deprecated - IptablesAzureClearMarkHex string = "0x0/0xE00" + // for V1 + IptablesAzureClearMarkHex string = "0x0" + // for v2, deprecated + IptablesAzureClearMarkHexV2 string = "0x0/0xE00" // marks in NPM v2 // NPM uses the 3rd word of the 32-bit mark for the purpose of From b8f0852360d6ea712d38bc61922b50ed1473a55c Mon Sep 17 00:00:00 2001 From: vakr Date: Fri, 8 Apr 2022 09:57:01 -0700 Subject: [PATCH 4/4] correcting an UT --- .../dataplane/policies/policymanager_linux_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/npm/pkg/dataplane/policies/policymanager_linux_test.go b/npm/pkg/dataplane/policies/policymanager_linux_test.go index 12aa2eb791..9e26fb5589 100644 --- a/npm/pkg/dataplane/policies/policymanager_linux_test.go +++ b/npm/pkg/dataplane/policies/policymanager_linux_test.go @@ -89,14 +89,19 @@ const ( // iptables rule variables for ACLs var ( ingressDropRule = fmt.Sprintf( - "-j MARK --set-mark 0x4000 -p TCP --dport 222:333 -m set --match-set %s src -m set ! --match-set %s dst -m comment --comment %s", + "-j MARK --set-mark %s -p TCP --dport 222:333 -m set --match-set %s src -m set ! --match-set %s dst -m comment --comment %s", + util.IptablesAzureIngressDropMarkHex, ipsets.TestCIDRSet.HashedName, ipsets.TestKeyPodSet.HashedName, ingressDropComment, ) ingressAllowRule = fmt.Sprintf("-j AZURE-NPM-INGRESS-ALLOW-MARK -m set --match-set %s src -m comment --comment %s", ipsets.TestCIDRSet.HashedName, ingressAllowComment) - egressDropRule = fmt.Sprintf("-j MARK --set-mark 0x5000 -p UDP --dport 144 -m set --match-set %s dst -m comment --comment %s", ipsets.TestCIDRSet.HashedName, egressDropComment) - egressAllowRule = fmt.Sprintf("-j AZURE-NPM-ACCEPT -m set --match-set %s dst -m comment --comment %s", ipsets.TestNamedportSet.HashedName, egressAllowComment) + egressDropRule = fmt.Sprintf("-j MARK --set-mark %s -p UDP --dport 144 -m set --match-set %s dst -m comment --comment %s", + util.IptablesAzureEgressDropMarkHex, + ipsets.TestCIDRSet.HashedName, + egressDropComment, + ) + egressAllowRule = fmt.Sprintf("-j AZURE-NPM-ACCEPT -m set --match-set %s dst -m comment --comment %s", ipsets.TestNamedportSet.HashedName, egressAllowComment) ) // NetworkPolicies