From 8962e94e74a8c42322bacda9d6746aaa36a8105e Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 18 Jul 2024 19:19:59 -0700 Subject: [PATCH 1/4] fix: npv1 deny-all simulation Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- cmd/policy-assistant/pkg/matcher/builder.go | 8 ++++++++ cmd/policy-assistant/pkg/matcher/explain.go | 19 +++++++++++-------- .../pkg/matcher/peermatcher.go | 10 +++++++++- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/cmd/policy-assistant/pkg/matcher/builder.go b/cmd/policy-assistant/pkg/matcher/builder.go index d3b8919c..29ab22cd 100644 --- a/cmd/policy-assistant/pkg/matcher/builder.go +++ b/cmd/policy-assistant/pkg/matcher/builder.go @@ -81,6 +81,10 @@ func BuildTarget(netpol *networkingv1.NetworkPolicy) (*Target, *Target) { } func BuildIngressMatcher(policyNamespace string, ingresses []networkingv1.NetworkPolicyIngressRule) []PeerMatcher { + if len(ingresses) == 0 { + return []PeerMatcher{&NoMatcher{}} + } + var matchers []PeerMatcher for _, ingress := range ingresses { matchers = append(matchers, BuildPeerMatcher(policyNamespace, ingress.Ports, ingress.From)...) @@ -89,6 +93,10 @@ func BuildIngressMatcher(policyNamespace string, ingresses []networkingv1.Networ } func BuildEgressMatcher(policyNamespace string, egresses []networkingv1.NetworkPolicyEgressRule) []PeerMatcher { + if len(egresses) == 0 { + return []PeerMatcher{&NoMatcher{}} + } + var matchers []PeerMatcher for _, egress := range egresses { matchers = append(matchers, BuildPeerMatcher(policyNamespace, egress.Ports, egress.To)...) diff --git a/cmd/policy-assistant/pkg/matcher/explain.go b/cmd/policy-assistant/pkg/matcher/explain.go index 3654d666..bb465307 100644 --- a/cmd/policy-assistant/pkg/matcher/explain.go +++ b/cmd/policy-assistant/pkg/matcher/explain.go @@ -2,11 +2,12 @@ package matcher import ( "fmt" + "strings" + "github.com/mattfenwick/collections/pkg/json" "github.com/mattfenwick/collections/pkg/slice" "golang.org/x/exp/slices" v1 "k8s.io/api/core/v1" - "strings" "github.com/mattfenwick/cyclonus/pkg/kube" "github.com/olekukonko/tablewriter" @@ -80,23 +81,25 @@ func (s *SliceBuilder) TargetsTableLines(targets []*Target, isIngress bool) { rules := strings.Join(sourceRulesStrings, "\n") s.Prefix = []string{ruleType, target.TargetString(), rules} - if len(target.Peers) == 0 { - s.Append("no pods, no ips", "NPv1: All peers allowed", "no ports, no protocols") - continue + if len(target.Peers) == 1 { + if _, ok := target.Peers[0].(*NoMatcher); ok { + s.Append("no peers", "NPv1:\n Allow any peers", "none") + continue + } } peers := groupAnbAndBanp(target.Peers) for _, p := range slice.SortOn(func(p PeerMatcher) string { return json.MustMarshalToString(p) }, peers) { switch t := p.(type) { case *AllPeersMatcher: - s.Append("all pods, all ips", "NPv1: All peers allowed", "all ports, all protocols") + s.Append("all pods, all ips", "NPv1:\n Allow any peers", "all ports, all protocols") case *PortsForAllPeersMatcher: pps := PortMatcherTableLines(t.Port, NetworkPolicyV1) - s.Append("all pods, all ips", "NPv1: All peers allowed", strings.Join(pps, "\n")) + s.Append("all pods, all ips", "NPv1:\n Allow any peers", strings.Join(pps, "\n")) case *IPPeerMatcher: s.IPPeerMatcherTableLines(t) case *PodPeerMatcher: - s.Append(resolveSubject(t), "NPv1: All peers allowed", strings.Join(PortMatcherTableLines(t.Port, NewV1Effect(true).PolicyKind), "\n")) + s.Append(resolveSubject(t), "NPv1:\n Allow any peers", strings.Join(PortMatcherTableLines(t.Port, NewV1Effect(true).PolicyKind), "\n")) case *peerProtocolGroup: s.peerProtocolGroupTableLines(t) default: @@ -110,7 +113,7 @@ func (s *SliceBuilder) TargetsTableLines(targets []*Target, isIngress bool) { func (s *SliceBuilder) IPPeerMatcherTableLines(ip *IPPeerMatcher) { peer := ip.IPBlock.CIDR + "\n" + fmt.Sprintf("except %+v", ip.IPBlock.Except) pps := PortMatcherTableLines(ip.Port, NetworkPolicyV1) - s.Append(peer, "NPv1: All peers allowed", strings.Join(pps, "\n")) + s.Append(peer, "NPv1:\n Allow any peers", strings.Join(pps, "\n")) } func (s *SliceBuilder) peerProtocolGroupTableLines(t *peerProtocolGroup) { diff --git a/cmd/policy-assistant/pkg/matcher/peermatcher.go b/cmd/policy-assistant/pkg/matcher/peermatcher.go index 3835e0a4..346ae3d7 100644 --- a/cmd/policy-assistant/pkg/matcher/peermatcher.go +++ b/cmd/policy-assistant/pkg/matcher/peermatcher.go @@ -18,8 +18,9 @@ These are the original PeerMatcher implementations made for v1 NetPol: - PortsForAllPeersMatcher - IPPeerMatcher - PodPeerMatcher +- NoMatcher -All PeerMatcher implementations (except AllPeersMatcher) use a PortMatcher. +All PeerMatcher implementations (except AllPeersMatcher and NoMatcher) use a PortMatcher. If the traffic doesn't match the port matcher, then Matches() will be false. Now we also have PeerMatcherAdmin, a wrapper for PodPeerMatcher to model ANP and BANP. @@ -29,6 +30,13 @@ type PeerMatcher interface { Matches(subject, peer *TrafficPeer, portInt int, portName string, protocol v1.Protocol) bool } +// NoMatcher matches no traffic. +type NoMatcher struct{} + +func (p *NoMatcher) Matches(subject, peer *TrafficPeer, portInt int, portName string, protocol v1.Protocol) bool { + return false +} + // AllPeerMatcher matches all pod to pod traffic. type AllPeersMatcher struct{} From 2ab5e8295bf40076d7c77e877f6ff7e3131285b5 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 18 Jul 2024 19:37:16 -0700 Subject: [PATCH 2/4] fix: simplify logic with NoMatcher Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- cmd/policy-assistant/pkg/matcher/simplifier.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cmd/policy-assistant/pkg/matcher/simplifier.go b/cmd/policy-assistant/pkg/matcher/simplifier.go index 871defb2..10e41b6c 100644 --- a/cmd/policy-assistant/pkg/matcher/simplifier.go +++ b/cmd/policy-assistant/pkg/matcher/simplifier.go @@ -33,6 +33,22 @@ func SimplifyV1(matchers []PeerMatcher) []PeerMatcher { } } + if len(v1Matchers) == 0 { + return nil + } + + onlyNoMatchers := true + for _, matcher := range v1Matchers { + if _, ok := matcher.(*NoMatcher); !ok { + onlyNoMatchers = false + break + } + } + + if onlyNoMatchers { + return []PeerMatcher{&NoMatcher{}} + } + matchesAll := false var portsForAllPeersMatchers []*PortsForAllPeersMatcher var ips []*IPPeerMatcher @@ -47,6 +63,8 @@ func SimplifyV1(matchers []PeerMatcher) []PeerMatcher { ips = append(ips, a) case *PodPeerMatcher: pods = append(pods, a) + case *NoMatcher: + // nothing to do default: panic(errors.Errorf("invalid matcher type %T", matcher)) } From 74a60f1b7cd1e3f7812bbb331fa5f6b137016f59 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 18 Jul 2024 19:38:16 -0700 Subject: [PATCH 3/4] test: simplifying NoMatcher as well as anp/banp --- .../pkg/matcher/simplifier_tests.go | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/cmd/policy-assistant/pkg/matcher/simplifier_tests.go b/cmd/policy-assistant/pkg/matcher/simplifier_tests.go index fdf1b4cb..188f854f 100644 --- a/cmd/policy-assistant/pkg/matcher/simplifier_tests.go +++ b/cmd/policy-assistant/pkg/matcher/simplifier_tests.go @@ -109,6 +109,49 @@ func RunSimplifierTests() { } Expect(Simplify([]PeerMatcher{dns, somePod})).To(Equal([]PeerMatcher{dns, somePod})) }) + + It("should simplify no matchers to one no matcher", func() { + no1 := &NoMatcher{} + no2 := &NoMatcher{} + + Expect(Simplify([]PeerMatcher{no1, no2})).To(Equal([]PeerMatcher{&NoMatcher{}})) + }) + + It("ignore no matchers amongst others", func() { + no1 := &NoMatcher{} + + Expect(Simplify([]PeerMatcher{no1, all, allOnTCP80, ip, allPodsAllPorts, allPodsTCP103})).To(Equal([]PeerMatcher{all})) + }) + + It("don't simplify (b)anp", func() { + anpDenyAll := &PeerMatcherAdmin{ + PodPeerMatcher: &PodPeerMatcher{ + Namespace: &AllNamespaceMatcher{}, + Pod: &AllPodMatcher{}, + Port: &AllPortMatcher{}, + }, + effectFromMatch: Effect{ + PolicyKind: AdminNetworkPolicy, + Priority: 5, + Verdict: Deny, + }, + Name: "anp", + } + banpAllowAll := &PeerMatcherAdmin{ + PodPeerMatcher: &PodPeerMatcher{ + Namespace: &AllNamespaceMatcher{}, + Pod: &AllPodMatcher{}, + Port: &AllPortMatcher{}, + }, + effectFromMatch: Effect{ + PolicyKind: BaselineAdminNetworkPolicy, + Verdict: Allow, + }, + Name: "banp", + } + Expect(Simplify([]PeerMatcher{all, allOnTCP80, ip, allPodsAllPorts, banpAllowAll, allPodsTCP103, anpDenyAll})).To(Equal([]PeerMatcher{banpAllowAll, anpDenyAll, all})) + + }) }) Describe("Port Simplifier", func() { From 62767e2cee47c2e73c90577eec82871a7779a356 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 18 Jul 2024 19:49:57 -0700 Subject: [PATCH 4/4] test: "deny all" connectivity integration test Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../test/integration/integration_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/cmd/policy-assistant/test/integration/integration_test.go b/cmd/policy-assistant/test/integration/integration_test.go index 1ed098bb..a686f866 100644 --- a/cmd/policy-assistant/test/integration/integration_test.go +++ b/cmd/policy-assistant/test/integration/integration_test.go @@ -57,6 +57,26 @@ type flow struct { func TestNetPolV1Connectivity(t *testing.T) { tests := []connectivityTest{ + { + name: "deny all", + defaultIngressBehavior: probe.ConnectivityBlocked, + defaultEgressBehavior: probe.ConnectivityAllowed, + args: args{ + resources: getResources(t, []string{"x"}, []string{"a", "b"}, []int{80}, []v1.Protocol{v1.ProtocolTCP}), + netpols: []*networkingv1.NetworkPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "x", + Name: "base", + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + }, + }, + }, + }, + }, { name: "ingress port allowed", defaultIngressBehavior: probe.ConnectivityAllowed,