@@ -365,12 +365,7 @@ func (keys MapState) addKeyWithChanges(key Key, entry MapStateEntry, changes Cha
365365 // Keep all owners that need this entry so that it is deleted only if all the owners delete their contribution
366366 oldEntry , exists := keys [key ]
367367 var datapathEqual bool
368- if exists {
369- // Deny entry can only be overridden by another deny entry
370- if oldEntry .IsDeny && ! entry .IsDeny {
371- return
372- }
373-
368+ if exists && oldEntry .IsDeny == entry .IsDeny {
374369 if entry .DeepEqual (& oldEntry ) {
375370 return // nothing to do
376371 }
@@ -383,7 +378,7 @@ func (keys MapState) addKeyWithChanges(key Key, entry MapStateEntry, changes Cha
383378 datapathEqual = oldEntry .DatapathEqual (& entry )
384379 oldEntry .Merge (& entry )
385380 keys [key ] = oldEntry
386- } else {
381+ } else if ! exists || entry . IsDeny {
387382 // Newly inserted entries must have their own containers, so that they
388383 // remain separate when new owners/dependents are added to existing entries
389384 entry .DerivedFromRules = slices .Clone (entry .DerivedFromRules )
@@ -564,6 +559,7 @@ func (keys MapState) denyPreferredInsertWithChanges(newKey Key, newEntry MapStat
564559 return
565560 }
566561 if newEntry .IsDeny {
562+ bailed := false
567563 for k , v := range keys {
568564 // Protocols and traffic directions that don't match ensure that the policies
569565 // do not interact in anyway.
@@ -587,44 +583,96 @@ func (keys MapState) denyPreferredInsertWithChanges(newKey Key, newEntry MapStat
587583 // identity is removed.
588584 newEntry .AddDependent (newKeyCpy )
589585 }
590- } else if (newKey .Identity == k .Identity ||
591- identityIsSupersetOf (newKey .Identity , k .Identity , identities )) &&
592- (newKey .PortProtoIsBroader (k ) || newKey .PortProtoIsEqual (k )) {
593- // If the new-entry is a superset (or equal) of the iterated-allow-entry and
594- // the new-entry has a broader (or equal) port-protocol then we
595- // should delete the iterated-allow-entry
596- keys .deleteKeyWithChanges (k , nil , changes )
586+ } else if newKey .PortProtoIsBroader (k ) || newKey .PortProtoIsEqual (k ) {
587+ // If newKey has a broader (or equal) port-protocol then we should
588+ // either delete the iterated-allow-entry (if the identity is the
589+ // same or the newKey is L3 wildcard), or change it to a deny entry
590+ // if the newKey's identity is a superset of the iterated identity
591+ // (e.g., newKey has a wider CIDR (say 10/8 covering the iterated
592+ // identity of more specific CIDR (say 10.1.1.1). Note that the
593+ // security identities assigned to these CIDRs have no numerical
594+ // relation to each other (e.g, they could be any numbers X and Y)
595+ // and the datapath does an exact match on them.
596+ if newKey .Identity == 0 || newKey .Identity == k .Identity {
597+ keys .deleteKeyWithChanges (k , nil , changes )
598+ } else if identityIsSupersetOf (newKey .Identity , k .Identity , identities ) {
599+ // When newKey.Identity is not ANY and is different from the
600+ // subset key, but still a superset (e.g., in CIDR sense) we
601+ // must keep the subset key and make it a deny instead.
602+ l3l4DenyEntry := NewMapStateEntry (newKey , newEntry .DerivedFromRules , false , true , DefaultAuthType , AuthTypeDisabled )
603+ keys .addKeyWithChanges (k , l3l4DenyEntry , changes )
604+ newEntry .AddDependent (k )
605+ }
606+ } else if newKey .Identity != 0 {
607+ if identityIsSupersetOf (newKey .Identity , k .Identity , identities ) {
608+ // k.PortProtoIsBroader(newKey) // due to if statements above
609+
610+ // Deny takes precedence for the port/proto of the newKey
611+ // for each allow with broader port/proto and narrower ID.
612+
613+ // If newKey is a superset of the iterated allow key and newKey has
614+ // more specific port-protocol than the iterated allow key then an
615+ // additional deny entry with port/proto of newKey and with the
616+ // identity of the iterated allow key must be added.
617+ denyKeyCpy := newKey
618+ denyKeyCpy .Identity = k .Identity
619+ l3l4DenyEntry := NewMapStateEntry (newKey , newEntry .DerivedFromRules , false , true , DefaultAuthType , AuthTypeDisabled )
620+ keys .addKeyWithChanges (denyKeyCpy , l3l4DenyEntry , changes )
621+ newEntry .AddDependent (denyKeyCpy )
622+ }
597623 }
598- } else if (newKey .Identity == k .Identity ||
599- identityIsSupersetOf (k .Identity , newKey .Identity , identities )) &&
600- k .DestPort == 0 && k .Nexthdr == 0 &&
601- ! v .HasDependent (newKey ) {
602- // If this iterated-deny-entry is a supserset (or equal) of the new-entry and
603- // the iterated-deny-entry is an L3-only policy then we
604- // should not insert the new entry (as long as it is not one
605- // of the special L4-only denies we created to cover the special
606- // case of a superset-allow with a more specific port-protocol).
607624 //
608- // NOTE: This condition could be broader to reject more deny entries,
609- // but there *may* be performance tradeoffs.
610- return
611- } else if (newKey .Identity == k .Identity ||
612- identityIsSupersetOf (newKey .Identity , k .Identity , identities )) &&
613- newKey .DestPort == 0 && newKey .Nexthdr == 0 &&
614- ! newEntry .HasDependent (k ) {
615- // If this iterated-deny-entry is a subset (or equal) of the new-entry and
616- // the new-entry is an L3-only policy then we
617- // should delete the iterated-deny-entry (as long as it is not one
618- // of the special L4-only denies we created to cover the special
619- // case of a superset-allow with a more specific port-protocol).
625+ // END OF ITERATED-ALLOW PROCESSING, ALL REMAINING CONDITIONS HIT
626+ // ITERATED-DENY ENTRIES
620627 //
621- // NOTE: This condition could be broader to reject more deny entries,
622- // but there *may* be performance tradeoffs.
628+ } else if bailed {
629+ // Skip processing further deny entries if we are bailing out.
630+ // We still need to loop through all the allow entries, so we can not
631+ // break out when bailing!
632+ continue
633+ } else if (k .Identity == 0 || k .Identity == newKey .Identity ) &&
634+ (k .PortProtoIsEqual (newKey ) || k .PortProtoIsBroader (newKey )) {
635+ // A narrower of two deny keys is redundant in the datapath only if
636+ // the broader ID is 0, or the IDs are the same. This is because the
637+ // ID will be assigned from the ipcache and datapath has no notion
638+ // of one ID being related to another (e.g., in a CIDR sense).
639+
640+ // If this iterated-deny-entry is an deny-all-L3 or has the same ID
641+ // as the new-entry and the iterated-deny-entry has a broader (or
642+ // equal) port-protocol it will match all the packets the newKey
643+ // would, given that we do not allow more specific allow rules to be
644+ // inserted.
645+
646+ // Identical key needs to be added if entries are different (to merge
647+ // them). This has no effect on the datapath policy map but is
648+ // needed for internal bookkeeping.
649+ if k != newKey || v .DeepEqual (& newEntry ) {
650+ bailed = true
651+ continue
652+ }
653+ } else if (newKey .Identity == 0 || newKey .Identity == k .Identity ) &&
654+ (newKey .PortProtoIsEqual (k ) || newKey .PortProtoIsBroader (k )) &&
655+ ! newEntry .HasDependent (k ) {
656+ // If this iterated-deny-entry is a subset (or equal) of the
657+ // new-entry and the new-entry has a broader (or equal)
658+ // port-protocol the newKey will match all the packets the iterated
659+ // key would, given that there are no more specific or L4-only allow
660+ // entries. We remove(d) the more specific allow rules in the blocks
661+ // above, and added more specific deny rules if there was an L4-only
662+ // allow rule. We use 'HasDependant' to figure out that 'k' must
663+ // remain to take precedence over the L4-only allow key.
664+
665+ // Identical key would have been captured in the block above, so we
666+ // do not need to check for it here.
623667 keys .deleteKeyWithChanges (k , nil , changes )
624668 }
625669 }
626- keys .addKeyWithChanges (newKey , newEntry , changes )
670+ if ! bailed {
671+ keys .addKeyWithChanges (newKey , newEntry , changes )
672+ }
627673 } else {
674+ insertAsDeny := false
675+ var denyEntry MapStateEntry
628676 for k , v := range keys {
629677 // Protocols and traffic directions that don't match ensure that the policies
630678 // do not interact in anyway.
@@ -650,17 +698,55 @@ func (keys MapState) denyPreferredInsertWithChanges(newKey Key, newEntry MapStat
650698 // identity is removed.
651699 keys .addDependentOnEntry (k , v , denyKeyCpy , changes )
652700 }
653- } else if (k .Identity == newKey .Identity ||
654- identityIsSupersetOf (k .Identity , newKey .Identity , identities )) &&
655- (k .PortProtoIsBroader (newKey ) || k .PortProtoIsEqual (newKey )) &&
656- ! v .HasDependent (newKey ) {
657- // If the iterated-deny-entry is a superset (or equal) of the new-entry and has a
658- // broader (or equal) port-protocol than the new-entry then the new
659- // entry should not be inserted.
660- return
701+ } else if k .PortProtoIsBroader (newKey ) || k .PortProtoIsEqual (newKey ) {
702+ if k .Identity == 0 || k .Identity == newKey .Identity {
703+ // If the iterated-deny-entry is a datapath superset (or
704+ // equal) of the new-entry and has a broader (or equal)
705+ // port-protocol than the new-entry then the new entry
706+ // should not be inserted.
707+ return
708+ } else if identityIsSupersetOf (k .Identity , newKey .Identity , identities ) {
709+ // if newKey is not bailed due to being covered in the
710+ // datapath by a deny entry, but is covered by a deny entry
711+ // in the CIDR sense, we must change this allow entry to a
712+ // deny entry so that the covering deny policy is honored
713+ // also for this ID in the datapath.
714+ if ! insertAsDeny {
715+ insertAsDeny = true
716+ denyEntry = NewMapStateEntry (k , v .DerivedFromRules , false , true , DefaultAuthType , AuthTypeDisabled )
717+ } else {
718+ // Collect the owners and labels of all the contributing deny rules
719+ denyEntry .Merge (& v )
720+ }
721+ }
722+ } else if k .Identity != 0 {
723+ // newKey.PortProtoIsBroader(k) as per previous if statements
724+ //
725+ // New L3/4 deny entry is not needed if the iterated key has wildcard L3,
726+ // as in that case it has precedence due to it having more specific L4.
727+ // So here we are only concerned about 'k' having a superset identity in
728+ // the CIDR sense, in which case a new L3/4 deny entry is needed.
729+ if identityIsSupersetOf (k .Identity , newKey .Identity , identities ) {
730+ // If the new-entry is a subset of the iterated-deny-entry
731+ // and the new-entry has a less specific port-protocol than
732+ // the iterated-deny-entry then an additional copy of the
733+ // iterated-deny-entry with the identity of the new-entry
734+ // must be added.
735+ denyKeyCpy := k
736+ denyKeyCpy .Identity = newKey .Identity
737+ l3l4DenyEntry := NewMapStateEntry (k , v .DerivedFromRules , false , true , DefaultAuthType , AuthTypeDisabled )
738+ keys .addKeyWithChanges (denyKeyCpy , l3l4DenyEntry , changes )
739+ // L3-only entries can be deleted incrementally so we need
740+ // to track their effects on other entries so that those
741+ // effects can be reverted when the identity is removed.
742+ keys .addDependentOnEntry (k , v , denyKeyCpy , changes )
743+ }
661744 }
662745 }
663746 }
747+ if insertAsDeny {
748+ newEntry = denyEntry
749+ }
664750 keys .authPreferredInsert (newKey , newEntry , features , changes )
665751 }
666752}
0 commit comments