Skip to content

Commit a537df1

Browse files
csfmomocsfmomo
authored
Add nil pointer checker when reading port value to avoid panic. (#606)
1 parent 508a2bb commit a537df1

File tree

3 files changed

+143
-10
lines changed

3 files changed

+143
-10
lines changed

‎npm/testpolicies/allow-backend-to-frontend-with-missing-port.yaml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
apiVersion: networking.k8s.io/v1
2+
kind: NetworkPolicy
3+
metadata:
4+
name: allow-backend-to-frontend-on-port-8000-policy
5+
namespace: testnamespace
6+
spec:
7+
policyTypes:
8+
- Ingress
9+
podSelector:
10+
matchLabels:
11+
app: frontend
12+
ingress:
13+
- from:
14+
- podSelector:
15+
matchLabels:
16+
app: backend
17+
ports:
18+
- port:

‎npm/translatePolicy.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS
231231
// Only Ports rules exist
232232
if portRuleExists && !fromRuleExists && !allowExternal {
233233
for _, portRule := range rule.Ports {
234-
if portRule.Port.IntValue() == 0 {
234+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
235235
portName := portRule.Port.String()
236236
namedPorts = append(namedPorts, portName)
237237
entry := &iptm.IptEntry{
@@ -297,7 +297,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS
297297
}
298298
if portRuleExists {
299299
for _, portRule := range rule.Ports {
300-
if portRule.Port.IntValue() == 0 {
300+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
301301
portName := portRule.Port.String()
302302
namedPorts = append(namedPorts, portName)
303303
entry := &iptm.IptEntry{
@@ -413,7 +413,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS
413413
iptPartialNsComment := craftPartialIptablesCommentFromSelector("", fromRule.NamespaceSelector, true)
414414
if portRuleExists {
415415
for _, portRule := range rule.Ports {
416-
if portRule.Port.IntValue() == 0 {
416+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
417417
portName := portRule.Port.String()
418418
namedPorts = append(namedPorts, portName)
419419
entry := &iptm.IptEntry{
@@ -507,7 +507,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS
507507
iptPartialPodComment := craftPartialIptablesCommentFromSelector(ns, fromRule.PodSelector, false)
508508
if portRuleExists {
509509
for _, portRule := range rule.Ports {
510-
if portRule.Port.IntValue() == 0 {
510+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
511511
portName := portRule.Port.String()
512512
namedPorts = append(namedPorts, portName)
513513
entry := &iptm.IptEntry{
@@ -614,7 +614,7 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS
614614
iptPartialPodComment := craftPartialIptablesCommentFromSelector("", fromRule.PodSelector, false)
615615
if portRuleExists {
616616
for _, portRule := range rule.Ports {
617-
if portRule.Port.IntValue() == 0 {
617+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
618618
portName := portRule.Port.String()
619619
namedPorts = append(namedPorts, portName)
620620
entry := &iptm.IptEntry{
@@ -869,7 +869,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe
869869
// Only Ports rules exist
870870
if portRuleExists && !toRuleExists && !allowExternal {
871871
for _, portRule := range rule.Ports {
872-
if portRule.Port.IntValue() == 0 {
872+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
873873
portName := portRule.Port.String()
874874
namedPorts = append(namedPorts, portName)
875875
entry := &iptm.IptEntry{
@@ -935,7 +935,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe
935935
}
936936
if portRuleExists {
937937
for _, portRule := range rule.Ports {
938-
if portRule.Port.IntValue() == 0 {
938+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
939939
portName := portRule.Port.String()
940940
namedPorts = append(namedPorts, portName)
941941
entry := &iptm.IptEntry{
@@ -1057,7 +1057,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe
10571057
iptPartialNsComment := craftPartialIptablesCommentFromSelector("", toRule.NamespaceSelector, true)
10581058
if portRuleExists {
10591059
for _, portRule := range rule.Ports {
1060-
if portRule.Port.IntValue() == 0 {
1060+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
10611061
portName := portRule.Port.String()
10621062
namedPorts = append(namedPorts, portName)
10631063
entry := &iptm.IptEntry{
@@ -1151,7 +1151,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe
11511151
iptPartialPodComment := craftPartialIptablesCommentFromSelector(ns, toRule.PodSelector, false)
11521152
if portRuleExists {
11531153
for _, portRule := range rule.Ports {
1154-
if portRule.Port.IntValue() == 0 {
1154+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
11551155
portName := portRule.Port.String()
11561156
namedPorts = append(namedPorts, portName)
11571157
entry := &iptm.IptEntry{
@@ -1258,7 +1258,7 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe
12581258
iptPartialPodComment := craftPartialIptablesCommentFromSelector("", toRule.PodSelector, false)
12591259
if portRuleExists {
12601260
for _, portRule := range rule.Ports {
1261-
if portRule.Port.IntValue() == 0 {
1261+
if portRule.Port != nil && portRule.Port.IntValue() == 0 {
12621262
portName := portRule.Port.String()
12631263
namedPorts = append(namedPorts, portName)
12641264
entry := &iptm.IptEntry{

‎npm/translatePolicy_test.go‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2368,6 +2368,121 @@ func TestAllowBackendToFrontendPort8000(t *testing.T) {
23682368
}
23692369
}
23702370

2371+
func TestAllowBackendToFrontendWithMissingPort(t *testing.T) {
2372+
allowBackendToFrontendMissingPortPolicy, err := readPolicyYaml("testpolicies/allow-backend-to-frontend-with-missing-port.yaml")
2373+
if err != nil {
2374+
t.Fatal(err)
2375+
}
2376+
2377+
sets, _, lists, _, _, iptEntries := translatePolicy(allowBackendToFrontendMissingPortPolicy)
2378+
2379+
expectedSets := []string{
2380+
"app:frontend",
2381+
"ns-testnamespace",
2382+
"app:backend",
2383+
}
2384+
if !reflect.DeepEqual(sets, expectedSets) {
2385+
t.Errorf("translatedPolicy failed @ ALLOW-app:backend-TO-app:frontend-port-8000-policy sets comparison")
2386+
t.Errorf("sets: %v", sets)
2387+
t.Errorf("expectedSets: %v", expectedSets)
2388+
}
2389+
2390+
expectedLists := []string{}
2391+
if !reflect.DeepEqual(lists, expectedLists) {
2392+
t.Errorf("translatedPolicy failed @ ALLOW-app:backend-TO-app:frontend-port-8000-policy lists comparison")
2393+
t.Errorf("lists: %v", lists)
2394+
t.Errorf("expectedLists: %v", expectedLists)
2395+
}
2396+
2397+
expectedIptEntries := []*iptm.IptEntry{}
2398+
nonKubeSystemEntries := []*iptm.IptEntry{
2399+
&iptm.IptEntry{
2400+
Chain: util.IptablesAzureIngressPortChain,
2401+
Specs: []string{
2402+
util.IptablesModuleFlag,
2403+
util.IptablesSetModuleFlag,
2404+
util.IptablesMatchSetFlag,
2405+
util.GetHashedName("ns-testnamespace"),
2406+
util.IptablesDstFlag,
2407+
util.IptablesModuleFlag,
2408+
util.IptablesSetModuleFlag,
2409+
util.IptablesMatchSetFlag,
2410+
util.GetHashedName("app:frontend"),
2411+
util.IptablesDstFlag,
2412+
util.IptablesModuleFlag,
2413+
util.IptablesSetModuleFlag,
2414+
util.IptablesMatchSetFlag,
2415+
util.GetHashedName("ns-testnamespace"),
2416+
util.IptablesSrcFlag,
2417+
util.IptablesModuleFlag,
2418+
util.IptablesSetModuleFlag,
2419+
util.IptablesMatchSetFlag,
2420+
util.GetHashedName("app:backend"),
2421+
util.IptablesSrcFlag,
2422+
util.IptablesJumpFlag,
2423+
util.IptablesAccept,
2424+
util.IptablesModuleFlag,
2425+
util.IptablesCommentModuleFlag,
2426+
util.IptablesCommentFlag,
2427+
"ALLOW-app:backend-IN-ns-testnamespace-AND--TO-app:frontend-IN-ns-testnamespace",
2428+
},
2429+
},
2430+
&iptm.IptEntry{
2431+
Chain: util.IptablesAzureIngressPortChain,
2432+
IsJumpEntry: true,
2433+
Specs: []string{
2434+
util.IptablesModuleFlag,
2435+
util.IptablesSetModuleFlag,
2436+
util.IptablesMatchSetFlag,
2437+
util.GetHashedName("ns-testnamespace"),
2438+
util.IptablesDstFlag,
2439+
util.IptablesModuleFlag,
2440+
util.IptablesSetModuleFlag,
2441+
util.IptablesMatchSetFlag,
2442+
util.GetHashedName("app:frontend"),
2443+
util.IptablesDstFlag,
2444+
util.IptablesJumpFlag,
2445+
util.IptablesAzureTargetSetsChain,
2446+
util.IptablesModuleFlag,
2447+
util.IptablesCommentModuleFlag,
2448+
util.IptablesCommentFlag,
2449+
"ALLOW-ALL-TO-app:frontend-IN-ns-testnamespace-TO-JUMP-TO-" + util.IptablesAzureTargetSetsChain,
2450+
},
2451+
},
2452+
&iptm.IptEntry{
2453+
Chain: util.IptablesAzureTargetSetsChain,
2454+
Specs: []string{
2455+
util.IptablesModuleFlag,
2456+
util.IptablesSetModuleFlag,
2457+
util.IptablesMatchSetFlag,
2458+
util.GetHashedName("ns-testnamespace"),
2459+
util.IptablesDstFlag,
2460+
util.IptablesModuleFlag,
2461+
util.IptablesSetModuleFlag,
2462+
util.IptablesMatchSetFlag,
2463+
util.GetHashedName("app:frontend"),
2464+
util.IptablesDstFlag,
2465+
util.IptablesJumpFlag,
2466+
util.IptablesDrop,
2467+
util.IptablesModuleFlag,
2468+
util.IptablesCommentModuleFlag,
2469+
util.IptablesCommentFlag,
2470+
"DROP-ALL-TO-app:frontend-IN-ns-testnamespace",
2471+
},
2472+
},
2473+
}
2474+
2475+
expectedIptEntries = append(expectedIptEntries, nonKubeSystemEntries...)
2476+
expectedIptEntries = append(expectedIptEntries, getDefaultDropEntries("dangerous", allowBackendToFrontendMissingPortPolicy.Spec.PodSelector, false, false)...)
2477+
if !reflect.DeepEqual(iptEntries, expectedIptEntries) {
2478+
t.Errorf("translatedPolicy failed @ ALLOW-ALL-TO-app:backdoor-policy policy comparison")
2479+
marshalledIptEntries, _ := json.Marshal(iptEntries)
2480+
marshalledExpectedIptEntries, _ := json.Marshal(expectedIptEntries)
2481+
t.Errorf("iptEntries: %s", marshalledIptEntries)
2482+
t.Errorf("expectedIptEntries: %s", marshalledExpectedIptEntries)
2483+
}
2484+
}
2485+
23712486
func TestAllowMultipleLabelsToMultipleLabels(t *testing.T) {
23722487
allowCniOrCnsToK8sPolicy, err := readPolicyYaml("testpolicies/allow-multiple-labels-to-multiple-labels.yaml")
23732488
if err != nil {

0 commit comments

Comments
 (0)