Skip to content

Commit

Permalink
Merge pull request k8snetworkplumbingwg#65 from zeeke/us-OCPBUGS-36468
Browse files Browse the repository at this point in the history
Make sure that policies with no valid peers are enforced
  • Loading branch information
dougbtv authored Sep 10, 2024
2 parents 2879925 + 26d979a commit 76cd193
Show file tree
Hide file tree
Showing 5 changed files with 265 additions and 30 deletions.
13 changes: 2 additions & 11 deletions .github/workflows/kind-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,8 @@ jobs:
export TERM=dumb
# enable ip6_tables
sudo modprobe ip6_tables
bats ./tests/simple-v4-ingress.bats
bats ./tests/simple-v4-egress.bats
bats ./tests/simple-v6-ingress.bats
bats ./tests/ipblock.bats
# stacked case
bats ./tests/stacked.bats
bats ./tests/ipblock-stacked.bats
# multiple items in egress/ingress
bats ./tests/ipblock-list.bats
bats ./tests/simple-v4-ingress-list.bats
bats ./tests/simple-v4-egress-list.bats
bats ./tests/*.bats
- name: Export kind logs
if: ${{ failure() }}
Expand Down
63 changes: 63 additions & 0 deletions e2e/tests/ingress-ns-selector-no-pods.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#!/usr/bin/env bats


# Note:
# These test cases verify that an ingress rule on the server that matches no pods
# does not allow any pods to reach the server.

setup() {
cd $BATS_TEST_DIRNAME
load "common"
server_net1=$(get_net1_ip "test-ingress-ns-selector-no-pods" "pod-server")
client_a_net1=$(get_net1_ip "test-ingress-ns-selector-no-pods" "pod-client-a")
client_b_net1=$(get_net1_ip "test-ingress-ns-selector-no-pods-blue" "pod-client-b")
}

@test "setup ingress-ns-selector-no-pods test environments" {
# create test manifests
kubectl create -f ingress-ns-selector-no-pods.yml

# verify all pods are available
run kubectl -n test-ingress-ns-selector-no-pods wait --for=condition=ready -l app=test-ingress-ns-selector-no-pods pod --timeout=${kubewait_timeout}
[ "$status" -eq "0" ]

sleep 5
}

@test "test-ingress-ns-selector-no-pods check client-a -> server" {
# nc should NOT succeed from client-a to server by policy
run kubectl -n test-ingress-ns-selector-no-pods exec pod-client-a -- sh -c "echo x | nc -w 1 ${server_net1} 5555"
[ "$status" -eq "1" ]
}

@test "test-ingress-ns-selector-no-pods check client-b -> server" {
# nc should NOT succeed from client-b to server by policy
run kubectl -n test-ingress-ns-selector-no-pods-blue exec pod-client-b -- sh -c "echo x | nc -w 1 ${server_net1} 5555"
[ "$status" -eq "1" ]
}

@test "test-ingress-ns-selector-no-pods check server -> client-a" {
# nc should succeed from server to client-a by no policy definition for direction (egress for pod-server)
run kubectl -n test-ingress-ns-selector-no-pods exec pod-server -- sh -c "echo x | nc -w 1 ${client_a_net1} 5555"
[ "$status" -eq "0" ]
}

@test "test-ingress-ns-selector-no-pods check server -> client-b" {
# nc should succeed from server to client-b by no policy definition for direction (egress for pod-server)
run kubectl -n test-ingress-ns-selector-no-pods exec pod-server -- sh -c "echo x | nc -w 1 ${client_b_net1} 5555"
[ "$status" -eq "0" ]
}

@test "cleanup environments" {
# remove test manifests
kubectl delete -f ingress-ns-selector-no-pods.yml
run kubectl -n test-ingress-ns-selector-no-pods wait --for=delete -l app=test-ingress-ns-selector-no-pods pod --timeout=${kubewait_timeout}
[ "$status" -eq "0" ]

sleep 3
# check that no iptables files in pod-iptables
pod_name=$(kubectl -n kube-system get pod -o wide | grep 'kind-worker' | grep multi-net | cut -f 1 -d ' ')
run kubectl -n kube-system exec ${pod_name} -- \
sh -c "find /var/lib/multi-networkpolicy/iptables/ -name '*.iptables' | wc -l"
[ "$output" = "0" ]
}
109 changes: 109 additions & 0 deletions e2e/tests/ingress-ns-selector-no-pods.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
---
apiVersion: "k8s.cni.cncf.io/v1"
kind: NetworkAttachmentDefinition
metadata:
namespace: default
name: macvlan1-simple
spec:
config: '{
"cniVersion": "0.3.1",
"name": "macvlan1-simple",
"plugins": [
{
"type": "macvlan",
"mode": "bridge",
"ipam":{
"type":"host-local",
"subnet":"2.2.6.0/24",
"rangeStart":"2.2.6.8",
"rangeEnd":"2.2.6.67"
}
}]
}'
---
# namespace for MultiNetworkPolicy
apiVersion: v1
kind: Namespace
metadata:
name: test-ingress-ns-selector-no-pods
---
apiVersion: v1
kind: Namespace
metadata:
name: test-ingress-ns-selector-no-pods-blue
---
# Pods
apiVersion: v1
kind: Pod
metadata:
name: pod-server
namespace: test-ingress-ns-selector-no-pods
annotations:
k8s.v1.cni.cncf.io/networks: default/macvlan1-simple
labels:
app: test-ingress-ns-selector-no-pods
name: pod-server
spec:
containers:
- name: macvlan-worker1
image: ghcr.io/k8snetworkplumbingwg/multi-networkpolicy-iptables:e2e-test
command: ["nc", "-klp", "5555"]
securityContext:
privileged: true
---
apiVersion: v1
kind: Pod
metadata:
name: pod-client-a
namespace: test-ingress-ns-selector-no-pods
annotations:
k8s.v1.cni.cncf.io/networks: default/macvlan1-simple
labels:
app: test-ingress-ns-selector-no-pods
name: pod-client-a
spec:
containers:
- name: macvlan-worker1
image: ghcr.io/k8snetworkplumbingwg/multi-networkpolicy-iptables:e2e-test
command: ["nc", "-klp", "5555"]
securityContext:
privileged: true
---
apiVersion: v1
kind: Pod
metadata:
name: pod-client-b
namespace: test-ingress-ns-selector-no-pods-blue
annotations:
k8s.v1.cni.cncf.io/networks: default/macvlan1-simple
labels:
app: test-ingress-ns-selector-no-pods
name: pod-client-b
spec:
containers:
- name: macvlan-worker1
image: ghcr.io/k8snetworkplumbingwg/multi-networkpolicy-iptables:e2e-test
command: ["nc", "-klp", "5555"]
securityContext:
privileged: true
---
# MultiNetworkPolicies
# this policy accepts ingress trafic from pod-client-a to pod-server
apiVersion: k8s.cni.cncf.io/v1beta1
kind: MultiNetworkPolicy
metadata:
name: policy-test-ingress-ns-selector-no-pods
namespace: test-ingress-ns-selector-no-pods
annotations:
k8s.v1.cni.cncf.io/policy-for: default/macvlan1-simple
spec:
podSelector:
matchLabels:
name: pod-server
ingress:
- from:
- namespaceSelector:
matchLabels:
foo: bar # No namespace with this label exists
policyTypes:
- Ingress
12 changes: 2 additions & 10 deletions pkg/server/policyrules.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
writeLine(ipt.policyIndex, "-A", fmt.Sprintf("MULTI-%d-INGRESS", pIndex), "-j", chainName)

s.podMap.Update(s.podChanges)
validPeers := 0
for _, peer := range from {
if peer.PodSelector != nil || peer.NamespaceSelector != nil {
podSelectorMap, err := metav1.LabelSelectorAsMap(peer.PodSelector)
Expand Down Expand Up @@ -334,7 +333,6 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
writeLine(ipt.ingressFrom, "-A", chainName,
"-i", podIntf.InterfaceName, "-s", ip,
"-j", "MARK", "--set-xmark", "0x20000/0x20000")
validPeers++
}
}
// ingress should accept reverse path
Expand All @@ -357,7 +355,6 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
if ipt.isIPFamilyCompatible(except) {
writeLine(ipt.ingressFrom, "-A", chainName,
"-i", podIntf.InterfaceName, "-s", except, "-j", "DROP")
validPeers++
}
}
}
Expand All @@ -369,7 +366,6 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
writeLine(ipt.ingressFrom, "-A", chainName,
"-i", podIntf.InterfaceName, "-s", peer.IPBlock.CIDR,
"-j", "MARK", "--set-xmark", "0x20000/0x20000")
validPeers++
}
}
for _, podIntf := range podInfo.Interfaces {
Expand All @@ -390,7 +386,7 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
}

// Add skip rule if no froms
if len(from) == 0 || validPeers == 0 {
if len(from) == 0 {
writeLine(ipt.ingressFrom, "-A", chainName,
"-m", "comment", "--comment", "\"no ingress from, skipped\"",
"-j", "MARK", "--set-xmark", "0x20000/0x20000")
Expand Down Expand Up @@ -509,7 +505,6 @@ func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo
writeLine(ipt.policyIndex, "-A", fmt.Sprintf("MULTI-%d-EGRESS", pIndex), "-j", chainName)

s.podMap.Update(s.podChanges)
validPeers := 0
for _, peer := range to {
if peer.PodSelector != nil || peer.NamespaceSelector != nil {
podSelectorMap, err := metav1.LabelSelectorAsMap(peer.PodSelector)
Expand Down Expand Up @@ -564,7 +559,6 @@ func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo
writeLine(ipt.egressTo, "-A", chainName,
"-o", podIntf.InterfaceName, "-d", ip,
"-j", "MARK", "--set-xmark", "0x20000/0x20000")
validPeers++
}
}
// egress should accept reverse path
Expand All @@ -587,7 +581,6 @@ func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo
if ipt.isIPFamilyCompatible(except) {
writeLine(ipt.egressTo, "-A", chainName,
"-o", multi.InterfaceName, "-d", except, "-j", "DROP")
validPeers++
}
}
}
Expand All @@ -599,7 +592,6 @@ func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo
writeLine(ipt.egressTo, "-A", chainName,
"-o", podIntf.InterfaceName, "-d", peer.IPBlock.CIDR,
"-j", "MARK", "--set-xmark", "0x20000/0x20000")
validPeers++
}
}
// egress should accept reverse path
Expand All @@ -621,7 +613,7 @@ func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo
}

// Add skip rules if no to
if len(to) == 0 || validPeers == 0 {
if len(to) == 0 {
writeLine(ipt.egressTo, "-A", chainName,
"-m", "comment", "--comment", "\"no egress to, skipped\"",
"-j", "MARK", "--set-xmark", "0x20000/0x20000")
Expand Down
Loading

0 comments on commit 76cd193

Please sign in to comment.