From a63314f1024bf06b9aba5b9805dec1905fdd0f68 Mon Sep 17 00:00:00 2001 From: GraysonWu Date: Tue, 7 Feb 2023 05:37:34 -0800 Subject: [PATCH] Change MCNP secFlows implementation (#4572) This PR changes how we implement MultiClusterNetworkPolicy security flows. Before, we created a security rule for each MultiClusterNetworkPolicy rule to install security flows. Now, we use the default drop flow to implement the security flows. For a MultiClusterNetworkPolicy rule, the drop flow will use the UnknownLabelIdentity and the original destination matching to drop packets. Signed-off-by: graysonwu --- pkg/agent/controller/networkpolicy/cache.go | 71 ++++--------------- .../controller/networkpolicy/cache_test.go | 5 +- .../controller/networkpolicy/reconciler.go | 12 ++-- .../networkpolicy/reconciler_test.go | 31 +++++++- pkg/agent/openflow/client.go | 2 +- pkg/agent/openflow/network_policy.go | 60 +++++++++++----- pkg/agent/openflow/network_policy_test.go | 23 ++++-- pkg/agent/openflow/pipeline.go | 11 +++ pkg/agent/openflow/testing/mock_openflow.go | 8 +-- test/integration/agent/openflow_test.go | 8 +-- 10 files changed, 131 insertions(+), 100 deletions(-) diff --git a/pkg/agent/controller/networkpolicy/cache.go b/pkg/agent/controller/networkpolicy/cache.go index 270ce0970a2..f88110cc38f 100644 --- a/pkg/agent/controller/networkpolicy/cache.go +++ b/pkg/agent/controller/networkpolicy/cache.go @@ -30,7 +30,6 @@ import ( "antrea.io/antrea/pkg/agent/config" "antrea.io/antrea/pkg/agent/metrics" - "antrea.io/antrea/pkg/agent/openflow" agenttypes "antrea.io/antrea/pkg/agent/types" v1beta "antrea.io/antrea/pkg/apis/controlplane/v1beta2" crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" @@ -695,38 +694,6 @@ func toRule(r *v1beta.NetworkPolicyRule, policy *v1beta.NetworkPolicy, maxPriori return rule } -// toStretchedNetworkPolicySecurityRule converts v1beta.NetworkPolicyRule to *rule -// which is used to drop all traffic initiated from Pods with UnknownLabelIdentity. -// Pods may have UnknownLabelIdentity when their Labels are completely new Labels -// among the whole ClusterSet. Before the new allocated LabelIdentity is imported -// in the Cluster, those Pods will have UnknownLabelIdentity. -func toStretchedNetworkPolicySecurityRule(r *v1beta.NetworkPolicyRule, policy *v1beta.NetworkPolicy, maxPriority int32) *rule { - appliedToGroups := policy.AppliedToGroups - if len(r.AppliedToGroups) != 0 { - appliedToGroups = r.AppliedToGroups - } - snpSecDropAction := crdv1alpha1.RuleActionDrop - rule := &rule{ - Direction: r.Direction, - From: v1beta.NetworkPolicyPeer{LabelIdentities: []uint32{openflow.UnknownLabelIdentity}}, - To: r.To, - Services: r.Services, - Action: &snpSecDropAction, - Priority: r.Priority, - PolicyPriority: policy.Priority, - TierPriority: policy.TierPriority, - AppliedToGroups: appliedToGroups, - Name: r.Name + "-security", - PolicyUID: policy.UID, - SourceRef: policy.SourceRef, - EnableLogging: r.EnableLogging, - } - rule.ID = hashRule(rule) - rule.PolicyName = policy.Name - rule.MaxPriority = maxPriority - return rule -} - // getMaxPriority returns the highest rule priority for v1beta.NetworkPolicy that is created // by Antrea-native policies. For K8s NetworkPolicies, it always returns -1. func getMaxPriority(policy *v1beta.NetworkPolicy) int32 { @@ -808,31 +775,23 @@ func (c *ruleCache) updateNetworkPolicyLocked(policy *v1beta.NetworkPolicy) bool anyRuleUpdate := false maxPriority := getMaxPriority(policy) for i := range policy.Rules { - rules := []*rule{toRule(&policy.Rules[i], policy, maxPriority)} - if len(policy.Rules[i].From.LabelIdentities) > 0 { - // If the rule is a StretchedNetworkPolicy rule, we also need to add a security rule. - // The security rule is used to drop all traffic initiated from Pods with - // UnknownLabelIdentity to make sure those traffic won't sneak around the Policy. - rules = append(rules, toStretchedNetworkPolicySecurityRule(&policy.Rules[i], policy, maxPriority)) - } - for _, r := range rules { - if _, exists := ruleByID[r.ID]; exists { - // If rule already exists, remove it from the map so the ones left are orphaned, - // which means those rules need to be handled by dirtyRuleHandler. - klog.V(2).InfoS("Rule was not changed", "id", r.ID) - delete(ruleByID, r.ID) + r := toRule(&policy.Rules[i], policy, maxPriority) + if _, exists := ruleByID[r.ID]; exists { + // If rule already exists, remove it from the map so the ones left are orphaned, + // which means those rules need to be handled by dirtyRuleHandler. + klog.V(2).InfoS("Rule was not changed", "id", r.ID) + delete(ruleByID, r.ID) + } else { + // If rule doesn't exist, add it to cache and mark it as dirty. + c.rules.Add(r) + // Count up antrea_agent_ingress_networkpolicy_rule_count or antrea_agent_egress_networkpolicy_rule_count + if r.Direction == v1beta.DirectionIn { + metrics.IngressNetworkPolicyRuleCount.Inc() } else { - // If rule doesn't exist, add it to cache and mark it as dirty. - c.rules.Add(r) - // Count up antrea_agent_ingress_networkpolicy_rule_count or antrea_agent_egress_networkpolicy_rule_count - if r.Direction == v1beta.DirectionIn { - metrics.IngressNetworkPolicyRuleCount.Inc() - } else { - metrics.EgressNetworkPolicyRuleCount.Inc() - } - c.dirtyRuleHandler(r.ID) - anyRuleUpdate = true + metrics.EgressNetworkPolicyRuleCount.Inc() } + c.dirtyRuleHandler(r.ID) + anyRuleUpdate = true } } diff --git a/pkg/agent/controller/networkpolicy/cache_test.go b/pkg/agent/controller/networkpolicy/cache_test.go index 5ffa0070e7d..e35316862ec 100644 --- a/pkg/agent/controller/networkpolicy/cache_test.go +++ b/pkg/agent/controller/networkpolicy/cache_test.go @@ -681,7 +681,6 @@ func TestRuleCacheAddNetworkPolicy(t *testing.T) { rule2 := toRule(networkPolicyRule2, networkPolicy2, k8sNPMaxPriority) rule3 := toRule(networkPolicyRule3, networkPolicy3, 0) rule4 := toRule(networkPolicyRule4, networkPolicy4, 0) - rule4Sec := toStretchedNetworkPolicySecurityRule(networkPolicyRule4, networkPolicy4, 0) tests := []struct { name string args *v1beta2.NetworkPolicy @@ -709,8 +708,8 @@ func TestRuleCacheAddNetworkPolicy(t *testing.T) { { "rule-with-label-identity", networkPolicy4, - []*rule{rule4, rule4Sec}, - sets.NewString(rule4.ID, rule4Sec.ID), + []*rule{rule4}, + sets.NewString(rule4.ID), }, } for _, tt := range tests { diff --git a/pkg/agent/controller/networkpolicy/reconciler.go b/pkg/agent/controller/networkpolicy/reconciler.go index 41c949bf841..d4180e4bdee 100644 --- a/pkg/agent/controller/networkpolicy/reconciler.go +++ b/pkg/agent/controller/networkpolicy/reconciler.go @@ -745,7 +745,7 @@ func (r *reconciler) update(lastRealized *lastRealized, newRule *CompletedRule, } else { addedTo := ofPortsToOFAddresses(newOFPorts.Difference(lastRealized.podOFPorts[igmpServicesKey])) deletedTo := ofPortsToOFAddresses(lastRealized.podOFPorts[igmpServicesKey].Difference(newOFPorts)) - if err := r.updateOFRule(ofID, nil, addedTo, nil, deletedTo, ofPriority, newRule.EnableLogging); err != nil { + if err := r.updateOFRule(ofID, nil, addedTo, nil, deletedTo, ofPriority, newRule.EnableLogging, false); err != nil { return err } } @@ -816,7 +816,7 @@ func (r *reconciler) update(lastRealized *lastRealized, newRule *CompletedRule, addedTo = ofPortsToOFAddresses(newOFPorts.Difference(originalOfPortsSet)) deletedTo = ofPortsToOFAddresses(originalOfPortsSet.Difference(newOFPorts)) } - if err := r.updateOFRule(ofID, addedFrom, addedTo, deletedFrom, deletedTo, ofPriority, newRule.EnableLogging); err != nil { + if err := r.updateOFRule(ofID, addedFrom, addedTo, deletedFrom, deletedTo, ofPriority, newRule.EnableLogging, len(newRule.From.LabelIdentities) > 0); err != nil { return err } // Delete valid servicesKey from staleOFIDs. @@ -908,7 +908,7 @@ func (r *reconciler) update(lastRealized *lastRealized, newRule *CompletedRule, addedTo = svcGroupIDsToOFAddresses(newGroupIDSet.Difference(originalGroupIDSet)) deletedTo = svcGroupIDsToOFAddresses(originalGroupIDSet.Difference(newGroupIDSet)) } - if err := r.updateOFRule(ofID, addedFrom, addedTo, deletedFrom, deletedTo, ofPriority, newRule.EnableLogging); err != nil { + if err := r.updateOFRule(ofID, addedFrom, addedTo, deletedFrom, deletedTo, ofPriority, newRule.EnableLogging, false); err != nil { return err } if r.fqdnController != nil { @@ -945,17 +945,17 @@ func (r *reconciler) installOFRule(ofRule *types.PolicyRule) error { return nil } -func (r *reconciler) updateOFRule(ofID uint32, addedFrom []types.Address, addedTo []types.Address, deletedFrom []types.Address, deletedTo []types.Address, priority *uint16, enableLogging bool) error { +func (r *reconciler) updateOFRule(ofID uint32, addedFrom []types.Address, addedTo []types.Address, deletedFrom []types.Address, deletedTo []types.Address, priority *uint16, enableLogging, isMCNPRule bool) error { klog.V(2).Infof("Updating ofRule %d (addedFrom: %d, addedTo: %d, deleteFrom: %d, deletedTo: %d)", ofID, len(addedFrom), len(addedTo), len(deletedFrom), len(deletedTo)) // TODO: This might be unnecessarily complex and hard for error handling, consider revising the Openflow interfaces. if len(addedFrom) > 0 { - if err := r.ofClient.AddPolicyRuleAddress(ofID, types.SrcAddress, addedFrom, priority, enableLogging); err != nil { + if err := r.ofClient.AddPolicyRuleAddress(ofID, types.SrcAddress, addedFrom, priority, enableLogging, isMCNPRule); err != nil { return fmt.Errorf("error adding policy rule source addresses for ofRule %v: %v", ofID, err) } } if len(addedTo) > 0 { - if err := r.ofClient.AddPolicyRuleAddress(ofID, types.DstAddress, addedTo, priority, enableLogging); err != nil { + if err := r.ofClient.AddPolicyRuleAddress(ofID, types.DstAddress, addedTo, priority, enableLogging, isMCNPRule); err != nil { return fmt.Errorf("error adding policy rule destination addresses for ofRule %v: %v", ofID, err) } } diff --git a/pkg/agent/controller/networkpolicy/reconciler_test.go b/pkg/agent/controller/networkpolicy/reconciler_test.go index d5fc492f2ba..5148bd40c3e 100644 --- a/pkg/agent/controller/networkpolicy/reconciler_test.go +++ b/pkg/agent/controller/networkpolicy/reconciler_test.go @@ -1099,6 +1099,7 @@ func TestReconcilerUpdate(t *testing.T) { expectedDeletedFrom []types.Address expectedDeletedTo []types.Address expectUninstall bool + isMCNPRule bool wantErr bool }{ { @@ -1119,6 +1120,7 @@ func TestReconcilerUpdate(t *testing.T) { ofPortsToOFAddresses(sets.NewInt32(1)), false, false, + false, }, { "updating-egress-rule", @@ -1138,6 +1140,7 @@ func TestReconcilerUpdate(t *testing.T) { ipsToOFAddresses(sets.NewString("1.1.1.1")), false, false, + false, }, { "updating-ingress-rule-with-missing-ofport", @@ -1157,6 +1160,7 @@ func TestReconcilerUpdate(t *testing.T) { ofPortsToOFAddresses(sets.NewInt32(1)), false, false, + false, }, { "updating-egress-rule-with-missing-ip", @@ -1176,6 +1180,7 @@ func TestReconcilerUpdate(t *testing.T) { ipsToOFAddresses(sets.NewString("1.1.1.1")), false, false, + false, }, { "updating-egress-rule-with-duplicate-ip", @@ -1207,6 +1212,7 @@ func TestReconcilerUpdate(t *testing.T) { ipsToOFAddresses(sets.NewString("1.1.1.2", "1.1.1.3")), false, false, + false, }, { "updating-egress-rule-deny-all", @@ -1226,6 +1232,7 @@ func TestReconcilerUpdate(t *testing.T) { []types.Address{}, false, false, + false, }, { "updating-cnp-ingress-rule", @@ -1245,6 +1252,7 @@ func TestReconcilerUpdate(t *testing.T) { ofPortsToOFAddresses(sets.NewInt32(1)), false, false, + false, }, { "updating-cnp-ingress-rule-uninstall", @@ -1264,6 +1272,25 @@ func TestReconcilerUpdate(t *testing.T) { []types.Address{}, true, false, + false, + }, + { + "updating-mcnp-ingress", + &CompletedRule{ + rule: &rule{ID: "ingress-rule", Direction: v1beta2.DirectionIn, PolicyPriority: &policyPriority, TierPriority: &tierPriority, From: v1beta2.NetworkPolicyPeer{LabelIdentities: []uint32{1}}, SourceRef: &cnp1, EnableLogging: false}, + TargetMembers: appliedToGroup1, + }, + &CompletedRule{ + rule: &rule{ID: "ingress-rule", Direction: v1beta2.DirectionIn, PolicyPriority: &policyPriority, TierPriority: &tierPriority, From: v1beta2.NetworkPolicyPeer{LabelIdentities: []uint32{1}}, SourceRef: &cnp1, EnableLogging: false}, + TargetMembers: appliedToGroup2, + }, + []types.Address{}, + ofPortsToOFAddresses(sets.NewInt32(2)), + []types.Address{}, + ofPortsToOFAddresses(sets.NewInt32(1)), + false, + true, + false, }, } for _, tt := range tests { @@ -1280,10 +1307,10 @@ func TestReconcilerUpdate(t *testing.T) { mockOFClient.EXPECT().UninstallPolicyRuleFlows(gomock.Any()) } if len(tt.expectedAddedFrom) > 0 { - mockOFClient.EXPECT().AddPolicyRuleAddress(gomock.Any(), types.SrcAddress, gomock.InAnyOrder(tt.expectedAddedFrom), priority, false) + mockOFClient.EXPECT().AddPolicyRuleAddress(gomock.Any(), types.SrcAddress, gomock.InAnyOrder(tt.expectedAddedFrom), priority, false, tt.isMCNPRule) } if len(tt.expectedAddedTo) > 0 { - mockOFClient.EXPECT().AddPolicyRuleAddress(gomock.Any(), types.DstAddress, gomock.InAnyOrder(tt.expectedAddedTo), priority, false) + mockOFClient.EXPECT().AddPolicyRuleAddress(gomock.Any(), types.DstAddress, gomock.InAnyOrder(tt.expectedAddedTo), priority, false, tt.isMCNPRule) } if len(tt.expectedDeletedFrom) > 0 { mockOFClient.EXPECT().DeletePolicyRuleAddress(gomock.Any(), types.SrcAddress, gomock.InAnyOrder(tt.expectedDeletedFrom), priority) diff --git a/pkg/agent/openflow/client.go b/pkg/agent/openflow/client.go index 532a0120e63..a425dee9757 100644 --- a/pkg/agent/openflow/client.go +++ b/pkg/agent/openflow/client.go @@ -126,7 +126,7 @@ type Client interface { // AddPolicyRuleAddress adds one or multiple addresses to the specified NetworkPolicy rule. If addrType is true, the // addresses are added to PolicyRule.From, else to PolicyRule.To. - AddPolicyRuleAddress(ruleID uint32, addrType types.AddressType, addresses []types.Address, priority *uint16, enableLogging bool) error + AddPolicyRuleAddress(ruleID uint32, addrType types.AddressType, addresses []types.Address, priority *uint16, enableLogging, isMCNPRule bool) error // DeletePolicyRuleAddress removes addresses from the specified NetworkPolicy rule. If addrType is srcAddress, the addresses // are removed from PolicyRule.From, else from PolicyRule.To. diff --git a/pkg/agent/openflow/network_policy.go b/pkg/agent/openflow/network_policy.go index 5b232d57e8e..4a72cc9fb00 100644 --- a/pkg/agent/openflow/network_policy.go +++ b/pkg/agent/openflow/network_policy.go @@ -720,7 +720,7 @@ func (c *client) NewDNSpacketInConjunction(id uint32) error { func (c *client) AddAddressToDNSConjunction(id uint32, addrs []types.Address) error { dnsPriority := priorityDNSIntercept - return c.AddPolicyRuleAddress(id, types.DstAddress, addrs, &dnsPriority, false) + return c.AddPolicyRuleAddress(id, types.DstAddress, addrs, &dnsPriority, false, false) } func (c *client) DeleteAddressFromDNSConjunction(id uint32, addrs []types.Address) error { @@ -728,7 +728,7 @@ func (c *client) DeleteAddressFromDNSConjunction(id uint32, addrs []types.Addres return c.DeletePolicyRuleAddress(id, types.DstAddress, addrs, &dnsPriority) } -func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPolicy, match *conjunctiveMatch, enableLogging bool) *conjMatchFlowContextChange { +func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPolicy, match *conjunctiveMatch, enableLogging, isMCNPRule bool) *conjMatchFlowContextChange { matcherKey := match.generateGlobalMapKey() _, found := c.matches[matcherKey] if found { @@ -752,9 +752,16 @@ func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPol // Generate the default drop flow if dropTable is not nil and the default drop flow is not set yet. if c.dropTable != nil && context.dropFlow == nil { - dropFlow = &flowChange{ - flow: context.featureNetworkPolicy.defaultDropFlow(c.dropTable, match.matchPairs, enableLogging), - changeType: insertion, + if isMCNPRule { + dropFlow = &flowChange{ + flow: context.featureNetworkPolicy.multiClusterNetworkPolicySecurityDropFlow(c.dropTable, match.matchPairs), + changeType: insertion, + } + } else { + dropFlow = &flowChange{ + flow: context.featureNetworkPolicy.defaultDropFlow(c.dropTable, match.matchPairs, enableLogging), + changeType: insertion, + } } } } else if context.dropFlowEnableLogging != enableLogging { @@ -958,12 +965,12 @@ func serviceToBitRanges(service v1beta2.Service) []types.BitRange { // addAddrFlows translates the specified addresses to conjunctiveMatchFlows, and returns the corresponding changes on the // conjunctiveMatchFlows. -func (c *clause) addAddrFlows(featureNetworkPolicy *featureNetworkPolicy, addrType types.AddressType, addresses []types.Address, priority *uint16, enableLogging bool) []*conjMatchFlowContextChange { +func (c *clause) addAddrFlows(featureNetworkPolicy *featureNetworkPolicy, addrType types.AddressType, addresses []types.Address, priority *uint16, enableLogging, isMCNPRule bool) []*conjMatchFlowContextChange { var conjMatchFlowContextChanges []*conjMatchFlowContextChange // Calculate Openflow changes for the added addresses. for _, addr := range addresses { match := generateAddressConjMatch(c.ruleTable.GetID(), addr, addrType, priority) - ctxChange := c.addConjunctiveMatchFlow(featureNetworkPolicy, match, enableLogging) + ctxChange := c.addConjunctiveMatchFlow(featureNetworkPolicy, match, enableLogging, isMCNPRule) if ctxChange != nil { conjMatchFlowContextChanges = append(conjMatchFlowContextChanges, ctxChange) } @@ -978,7 +985,7 @@ func (c *clause) addServiceFlows(featureNetworkPolicy *featureNetworkPolicy, ser for _, service := range services { matches := generateServiceConjMatches(c.ruleTable.GetID(), service, priority, featureNetworkPolicy.ipProtocols, matchSrc) for _, match := range matches { - ctxChange := c.addConjunctiveMatchFlow(featureNetworkPolicy, match, enableLogging) + ctxChange := c.addConjunctiveMatchFlow(featureNetworkPolicy, match, enableLogging, false) conjMatchFlowContextChanges = append(conjMatchFlowContextChanges, ctxChange) } } @@ -1176,23 +1183,24 @@ func (f *featureNetworkPolicy) calculateMatchFlowChangesForRule(conj *policyRule // Unlike calculateMatchFlowChangesForRule, it updates the context status directly and doesn't calculate flow changes. // It's used in initial batch install where we first add all rules then calculates flows change based on final state. func (f *featureNetworkPolicy) addRuleToConjunctiveMatch(conj *policyRuleConjunction, rule *types.PolicyRule) { + isMCNPRule := containsLabelIdentityAddress(rule.From) if conj.fromClause != nil { for _, addr := range rule.From { match := generateAddressConjMatch(conj.fromClause.ruleTable.GetID(), addr, types.SrcAddress, rule.Priority) - f.addActionToConjunctiveMatch(conj.fromClause, match, rule.EnableLogging) + f.addActionToConjunctiveMatch(conj.fromClause, match, rule.EnableLogging, isMCNPRule) } } if conj.toClause != nil { for _, addr := range rule.To { match := generateAddressConjMatch(conj.toClause.ruleTable.GetID(), addr, types.DstAddress, rule.Priority) - f.addActionToConjunctiveMatch(conj.toClause, match, rule.EnableLogging) + f.addActionToConjunctiveMatch(conj.toClause, match, rule.EnableLogging, isMCNPRule) } } if conj.serviceClause != nil { for _, eachService := range rule.Service { matches := generateServiceConjMatches(conj.serviceClause.ruleTable.GetID(), eachService, rule.Priority, f.ipProtocols, false) for _, match := range matches { - f.addActionToConjunctiveMatch(conj.serviceClause, match, rule.EnableLogging) + f.addActionToConjunctiveMatch(conj.serviceClause, match, rule.EnableLogging, isMCNPRule) } } } @@ -1201,7 +1209,7 @@ func (f *featureNetworkPolicy) addRuleToConjunctiveMatch(conj *policyRuleConjunc // addActionToConjunctiveMatch adds a clause to corresponding conjunctive match context. // It updates the context status directly and doesn't calculate the match flow, which is supposed to be calculated after // all actions are added. It's used in initial batch install only. -func (f *featureNetworkPolicy) addActionToConjunctiveMatch(clause *clause, match *conjunctiveMatch, enableLogging bool) { +func (f *featureNetworkPolicy) addActionToConjunctiveMatch(clause *clause, match *conjunctiveMatch, enableLogging, isMCNPRule bool) { matcherKey := match.generateGlobalMapKey() _, found := clause.matches[matcherKey] if found { @@ -1221,7 +1229,11 @@ func (f *featureNetworkPolicy) addActionToConjunctiveMatch(clause *clause, match } // Generate the default drop flow if dropTable is not nil. if clause.dropTable != nil { - context.dropFlow = context.featureNetworkPolicy.defaultDropFlow(clause.dropTable, match.matchPairs, enableLogging) + if isMCNPRule { + context.dropFlow = context.featureNetworkPolicy.multiClusterNetworkPolicySecurityDropFlow(clause.dropTable, match.matchPairs) + } else { + context.dropFlow = context.featureNetworkPolicy.defaultDropFlow(clause.dropTable, match.matchPairs, enableLogging) + } } f.globalConjMatchFlowCache[matcherKey] = context } @@ -1390,7 +1402,7 @@ func (c *policyRuleConjunction) calculateClauses(rule *types.PolicyRule) (uint8, c.fromClause = c.newClause(fromID, nClause, ruleTable, defaultTable) } if rule.To != nil { - if isEgressRule || rule.IsAntreaNetworkPolicyRule() { + if isEgressRule || (rule.IsAntreaNetworkPolicyRule() && !containsLabelIdentityAddress(rule.From)) { defaultTable = nil } else { defaultTable = dropTable @@ -1406,12 +1418,13 @@ func (c *policyRuleConjunction) calculateClauses(rule *types.PolicyRule) (uint8, // calculateChangesForRuleCreation returns the conjMatchFlowContextChanges of the new policyRuleConjunction. It // will calculate the expected conjMatchFlowContext status, and the changed Openflow entries. func (c *policyRuleConjunction) calculateChangesForRuleCreation(featureNetworkPolicy *featureNetworkPolicy, rule *types.PolicyRule) []*conjMatchFlowContextChange { + isMCNPRule := containsLabelIdentityAddress(rule.From) var ctxChanges []*conjMatchFlowContextChange if c.fromClause != nil { - ctxChanges = append(ctxChanges, c.fromClause.addAddrFlows(featureNetworkPolicy, types.SrcAddress, rule.From, rule.Priority, rule.EnableLogging)...) + ctxChanges = append(ctxChanges, c.fromClause.addAddrFlows(featureNetworkPolicy, types.SrcAddress, rule.From, rule.Priority, rule.EnableLogging, isMCNPRule)...) } if c.toClause != nil { - ctxChanges = append(ctxChanges, c.toClause.addAddrFlows(featureNetworkPolicy, types.DstAddress, rule.To, rule.Priority, rule.EnableLogging)...) + ctxChanges = append(ctxChanges, c.toClause.addAddrFlows(featureNetworkPolicy, types.DstAddress, rule.To, rule.Priority, rule.EnableLogging, isMCNPRule)...) } if c.serviceClause != nil { ctxChanges = append(ctxChanges, c.serviceClause.addServiceFlows(featureNetworkPolicy, rule.Service, rule.Priority, false, rule.EnableLogging)...) @@ -1419,6 +1432,17 @@ func (c *policyRuleConjunction) calculateChangesForRuleCreation(featureNetworkPo return ctxChanges } +func containsLabelIdentityAddress(addresses []types.Address) bool { + contains := false + for _, addr := range addresses { + if _, ok := addr.(*LabelIDAddress); ok { + contains = true + break + } + } + return contains +} + // calculateChangesForRuleDeletion returns the conjMatchFlowContextChanges of the deleted policyRuleConjunction. It // will calculate the expected conjMatchFlowContext status, and the changed Openflow entries. func (c *policyRuleConjunction) calculateChangesForRuleDeletion() []*conjMatchFlowContextChange { @@ -1587,7 +1611,7 @@ func (f *featureNetworkPolicy) replayFlows() []binding.Flow { // AddPolicyRuleAddress adds one or multiple addresses to the specified NetworkPolicy rule. If addrType is srcAddress, the // addresses are added to PolicyRule.From, else to PolicyRule.To. -func (c *client) AddPolicyRuleAddress(ruleID uint32, addrType types.AddressType, addresses []types.Address, priority *uint16, enableLogging bool) error { +func (c *client) AddPolicyRuleAddress(ruleID uint32, addrType types.AddressType, addresses []types.Address, priority *uint16, enableLogging, isMCNPRule bool) error { c.replayMutex.RLock() defer c.replayMutex.RUnlock() @@ -1606,7 +1630,7 @@ func (c *client) AddPolicyRuleAddress(ruleID uint32, addrType types.AddressType, c.featureNetworkPolicy.conjMatchFlowLock.Lock() defer c.featureNetworkPolicy.conjMatchFlowLock.Unlock() - flowChanges := clause.addAddrFlows(c.featureNetworkPolicy, addrType, addresses, priority, enableLogging) + flowChanges := clause.addAddrFlows(c.featureNetworkPolicy, addrType, addresses, priority, enableLogging, isMCNPRule) return c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChanges) } diff --git a/pkg/agent/openflow/network_policy_test.go b/pkg/agent/openflow/network_policy_test.go index 6b27ba98754..284e7e416ed 100644 --- a/pkg/agent/openflow/network_policy_test.go +++ b/pkg/agent/openflow/network_policy_test.go @@ -107,7 +107,7 @@ func TestPolicyRuleConjunction(t *testing.T) { var addedAddrs = parseAddresses([]string{"192.168.1.3", "192.168.1.30", "192.168.2.0/24", "103", "104"}) expectConjunctionsCount([]*expectConjunctionTimes{{5, ruleID1, clauseID, nClause}}) - flowChanges1 := clause1.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs, nil, false) + flowChanges1 := clause1.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs, nil, false, false) err := c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChanges1) require.Nil(t, err, "Failed to invoke addAddrFlows") checkFlowCount(t, len(addedAddrs)) @@ -132,7 +132,7 @@ func TestPolicyRuleConjunction(t *testing.T) { var addedAddrs2 = parseAddresses([]string{"192.168.1.30", "192.168.1.50"}) expectConjunctionsCount([]*expectConjunctionTimes{{2, ruleID2, clauseID2, nClause}}) expectConjunctionsCount([]*expectConjunctionTimes{{1, ruleID1, clauseID, nClause}}) - flowChanges3 := clause2.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs2, nil, false) + flowChanges3 := clause2.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs2, nil, false, false) err = c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChanges3) require.Nil(t, err, "Failed to invoke addAddrFlows") testAddr := NewIPAddress(net.ParseIP("192.168.1.30")) @@ -148,7 +148,7 @@ func TestPolicyRuleConjunction(t *testing.T) { nClause3 := uint8(1) clause3 := conj3.newClause(clauseID3, nClause3, mockEgressRuleTable, mockEgressDefaultTable) var addedAddrs3 = parseAddresses([]string{"192.168.1.30"}) - flowChanges4 := clause3.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs3, nil, false) + flowChanges4 := clause3.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs3, nil, false, false) err = c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChanges4) require.Nil(t, err, "Failed to invoke addAddrFlows") checkConjMatchFlowActions(t, c, clause3, testAddr, types.SrcAddress, 2, 1) @@ -459,7 +459,7 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) { From: parseLabelIdentityAddresses([]uint32{1, 2}), Action: &actionDrop, Priority: &priority201, - To: []types.Address{NewOFPortAddress(1)}, + To: []types.Address{NewOFPortAddress(1), NewOFPortAddress(2)}, Service: []v1beta2.Service{}, FlowID: uint32(13), TableID: AntreaPolicyIngressRuleTable.GetID(), @@ -528,6 +528,9 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) { AntreaPolicyIngressRuleTable.ofTable.BuildFlow(priority201).Cookie(cookiePolicy). MatchRegFieldWithValue(TargetOFPortField, uint32(1)). Action().Conjunction(13, 2, 3).Done(), + AntreaPolicyIngressRuleTable.ofTable.BuildFlow(priority201).Cookie(cookiePolicy). + MatchRegFieldWithValue(TargetOFPortField, uint32(2)). + Action().Conjunction(13, 2, 3).Done(), AntreaPolicyIngressRuleTable.ofTable.BuildFlow(priority100).Cookie(cookiePolicy). MatchProtocol(binding.ProtocolTCP).MatchDstPort(8080, nil). Action().Conjunction(11, 3, 3).Done(), @@ -552,6 +555,14 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) { IngressMetricTable.ofTable.BuildFlow(priorityNormal).Cookie(cookiePolicy). MatchRegMark(CnpDenyRegMark).MatchRegFieldWithValue(CNPConjIDField, 13). Action().Drop().Done(), + IngressDefaultTable.ofTable.BuildFlow(priority200).Cookie(cookiePolicy). + MatchTunnelID(uint64(UnknownLabelIdentity)). + MatchRegFieldWithValue(TargetOFPortField, uint32(1)). + Action().Drop().Done(), + IngressDefaultTable.ofTable.BuildFlow(priority200).Cookie(cookiePolicy). + MatchTunnelID(uint64(UnknownLabelIdentity)). + MatchRegFieldWithValue(TargetOFPortField, uint32(2)). + Action().Drop().Done(), } }, }, @@ -682,7 +693,7 @@ func TestConjMatchFlowContextKeyConflict(t *testing.T) { id: ruleID1, } clause1 := conj1.newClause(1, 3, mockEgressRuleTable, mockEgressDefaultTable) - flowChange1 := clause1.addAddrFlows(c.featureNetworkPolicy, types.DstAddress, parseAddresses([]string{ip.String()}), nil, false) + flowChange1 := clause1.addAddrFlows(c.featureNetworkPolicy, types.DstAddress, parseAddresses([]string{ip.String()}), nil, false, false) err := c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChange1) require.Nil(t, err, "no error expect in applyConjunctiveMatchFlows") @@ -691,7 +702,7 @@ func TestConjMatchFlowContextKeyConflict(t *testing.T) { id: ruleID2, } clause2 := conj2.newClause(1, 3, mockEgressRuleTable, mockEgressDefaultTable) - flowChange2 := clause2.addAddrFlows(c.featureNetworkPolicy, types.DstAddress, parseAddresses([]string{ipNet.String()}), nil, false) + flowChange2 := clause2.addAddrFlows(c.featureNetworkPolicy, types.DstAddress, parseAddresses([]string{ipNet.String()}), nil, false, false) err = c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChange2) require.Nil(t, err, "no error expect in applyConjunctiveMatchFlows") expectedMatchKey := fmt.Sprintf("table:%d,priority:%s,matchPair:%s", EgressRuleTable.GetID(), strconv.Itoa(int(priorityNormal)), singleMatchPair.KeyString()) diff --git a/pkg/agent/openflow/pipeline.go b/pkg/agent/openflow/pipeline.go index fb5739a5d18..50620b28bfb 100644 --- a/pkg/agent/openflow/pipeline.go +++ b/pkg/agent/openflow/pipeline.go @@ -2079,6 +2079,17 @@ func (f *featureNetworkPolicy) defaultDropFlow(table binding.Table, matchPairs [ Done() } +// multiClusterNetworkPolicySecurityDropFlow generates the security drop flows for MultiClusterNetworkPolicy. +func (f *featureNetworkPolicy) multiClusterNetworkPolicySecurityDropFlow(table binding.Table, matchPairs []matchPair) binding.Flow { + cookieID := f.cookieAllocator.Request(f.category).Raw() + fb := table.BuildFlow(priorityNormal) + fb = f.addFlowMatch(fb, MatchLabelID, UnknownLabelIdentity) + for _, eachMatchPair := range matchPairs { + fb = f.addFlowMatch(fb, eachMatchPair.matchKey, eachMatchPair.matchValue) + } + return fb.Cookie(cookieID).Action().Drop().Done() +} + // dnsPacketInFlow generates the flow to send dns response packets of fqdn policy selected Pods to the fqdnController for // processing. func (f *featureNetworkPolicy) dnsPacketInFlow(conjunctionID uint32) binding.Flow { diff --git a/pkg/agent/openflow/testing/mock_openflow.go b/pkg/agent/openflow/testing/mock_openflow.go index e2c25c15948..06356eb9391 100644 --- a/pkg/agent/openflow/testing/mock_openflow.go +++ b/pkg/agent/openflow/testing/mock_openflow.go @@ -71,17 +71,17 @@ func (mr *MockClientMockRecorder) AddAddressToDNSConjunction(arg0, arg1 interfac } // AddPolicyRuleAddress mocks base method -func (m *MockClient) AddPolicyRuleAddress(arg0 uint32, arg1 types.AddressType, arg2 []types.Address, arg3 *uint16, arg4 bool) error { +func (m *MockClient) AddPolicyRuleAddress(arg0 uint32, arg1 types.AddressType, arg2 []types.Address, arg3 *uint16, arg4, arg5 bool) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AddPolicyRuleAddress", arg0, arg1, arg2, arg3, arg4) + ret := m.ctrl.Call(m, "AddPolicyRuleAddress", arg0, arg1, arg2, arg3, arg4, arg5) ret0, _ := ret[0].(error) return ret0 } // AddPolicyRuleAddress indicates an expected call of AddPolicyRuleAddress -func (mr *MockClientMockRecorder) AddPolicyRuleAddress(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) AddPolicyRuleAddress(arg0, arg1, arg2, arg3, arg4, arg5 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddPolicyRuleAddress", reflect.TypeOf((*MockClient)(nil).AddPolicyRuleAddress), arg0, arg1, arg2, arg3, arg4) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddPolicyRuleAddress", reflect.TypeOf((*MockClient)(nil).AddPolicyRuleAddress), arg0, arg1, arg2, arg3, arg4, arg5) } // BatchInstallPolicyRuleFlows mocks base method diff --git a/test/integration/agent/openflow_test.go b/test/integration/agent/openflow_test.go index ab1e036ef13..3fd972eef77 100644 --- a/test/integration/agent/openflow_test.go +++ b/test/integration/agent/openflow_test.go @@ -324,10 +324,10 @@ func TestReplayFlowsNetworkPolicyFlows(t *testing.T) { err = c.InstallPolicyRuleFlows(rule) require.Nil(t, err, "Failed to InstallPolicyRuleFlows") - err = c.AddPolicyRuleAddress(ruleID, types.SrcAddress, prepareIPNetAddresses([]string{"192.168.5.0/24", "192.169.1.0/24"}), nil, false) + err = c.AddPolicyRuleAddress(ruleID, types.SrcAddress, prepareIPNetAddresses([]string{"192.168.5.0/24", "192.169.1.0/24"}), nil, false, false) require.Nil(t, err, "Failed to AddPolicyRuleAddress") ofport := int32(100) - err = c.AddPolicyRuleAddress(ruleID, types.DstAddress, []types.Address{ofClient.NewOFPortAddress(ofport)}, nil, false) + err = c.AddPolicyRuleAddress(ruleID, types.DstAddress, []types.Address{ofClient.NewOFPortAddress(ofport)}, nil, false, false) require.Nil(t, err, "Failed to AddPolicyRuleAddress") testReplayFlows(t) @@ -513,7 +513,7 @@ func TestNetworkPolicyFlows(t *testing.T) { checkDeleteAddress(t, ingressRuleTable, priorityNormal, ruleID, addedFrom, types.SrcAddress) ofport := int32(100) - err = c.AddPolicyRuleAddress(ruleID, types.DstAddress, []types.Address{ofClient.NewOFPortAddress(ofport)}, nil, false) + err = c.AddPolicyRuleAddress(ruleID, types.DstAddress, []types.Address{ofClient.NewOFPortAddress(ofport)}, nil, false, false) require.Nil(t, err, "Failed to AddPolicyRuleAddress") // Dump flows. @@ -889,7 +889,7 @@ func checkDefaultDropFlows(t *testing.T, table string, priority int, addrType ty } func checkAddAddress(t *testing.T, ruleTable string, priority int, ruleID uint32, addedAddress []types.Address, addrType types.AddressType) { - err := c.AddPolicyRuleAddress(ruleID, addrType, addedAddress, nil, false) + err := c.AddPolicyRuleAddress(ruleID, addrType, addedAddress, nil, false, false) require.Nil(t, err, "Failed to AddPolicyRuleAddress") // dump flows