Skip to content

Commit 11ae253

Browse files
adamtuliniusmurali-reddy
adamtulinius
authored and
murali-reddy
committed
Validate the presence of port definitions before attempting to access (#643)
This fixes #642, which causes kube-router to crash on valid network policies, and also implements support for ingress and egress rules without a port specified.
1 parent 10ddc09 commit 11ae253

File tree

1 file changed

+66
-25
lines changed

1 file changed

+66
-25
lines changed

‎pkg/controllers/netpol/network_policy_controller.go‎

Lines changed: 66 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/base32"
66
"errors"
77
"fmt"
8+
"k8s.io/apimachinery/pkg/util/intstr"
89
"net"
910
"regexp"
1011
"strconv"
@@ -121,6 +122,16 @@ type protocolAndPort struct {
121122
port string
122123
}
123124

125+
func newProtocolAndPort(protocol string, port *intstr.IntOrString) protocolAndPort {
126+
strPort := ""
127+
128+
if port != nil {
129+
strPort = port.String()
130+
}
131+
132+
return protocolAndPort{protocol: protocol, port: strPort}
133+
}
134+
124135
// Run runs forver till we receive notification on stopCh
125136
func (npc *NetworkPolicyController) Run(healthChan chan<- *healthcheck.ControllerHeartbeat, stopCh <-chan struct{}, wg *sync.WaitGroup) {
126137
t := time.NewTicker(npc.syncPeriod)
@@ -371,16 +382,21 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo
371382

372383
if len(ingressRule.ports) != 0 {
373384
// case where 'ports' details and 'from' details specified in the ingress rule
374-
// so match on specified source and destination ip's and specified port and protocol
385+
// so match on specified source and destination ip's and specified port (if any) and protocol
375386
for _, portProtocol := range ingressRule.ports {
376387
comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " +
377388
policy.name + " namespace " + policy.namespace
378389
args := []string{"-m", "comment", "--comment", comment,
379390
"-m", "set", "--set", srcPodIpSetName, "src",
380391
"-m", "set", "--set", targetDestPodIpSetName, "dst",
381-
"-p", portProtocol.protocol,
382-
"--dport", portProtocol.port,
383-
"-j", "ACCEPT"}
392+
"-p", portProtocol.protocol}
393+
394+
if portProtocol.port != "" {
395+
args = append(args, "--dport", portProtocol.port)
396+
}
397+
398+
args = append(args, "-j", "ACCEPT")
399+
384400
err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
385401
if err != nil {
386402
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
@@ -403,16 +419,21 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo
403419
}
404420

405421
// case where only 'ports' details specified but no 'from' details in the ingress rule
406-
// so match on all sources, with specified port and protocol
422+
// so match on all sources, with specified port (if any) and protocol
407423
if ingressRule.matchAllSource && !ingressRule.matchAllPorts {
408424
for _, portProtocol := range ingressRule.ports {
409425
comment := "rule to ACCEPT traffic from all sources to dest pods selected by policy name: " +
410426
policy.name + " namespace " + policy.namespace
411427
args := []string{"-m", "comment", "--comment", comment,
412428
"-m", "set", "--set", targetDestPodIpSetName, "dst",
413-
"-p", portProtocol.protocol,
414-
"--dport", portProtocol.port,
415-
"-j", "ACCEPT"}
429+
"-p", portProtocol.protocol}
430+
431+
if portProtocol.port != "" {
432+
args = append(args, "--dport", portProtocol.port)
433+
}
434+
435+
args = append(args, "-j", "ACCEPT")
436+
416437
err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
417438
if err != nil {
418439
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
@@ -452,9 +473,14 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo
452473
args := []string{"-m", "comment", "--comment", comment,
453474
"-m", "set", "--set", srcIpBlockIpSetName, "src",
454475
"-m", "set", "--set", targetDestPodIpSetName, "dst",
455-
"-p", portProtocol.protocol,
456-
"--dport", portProtocol.port,
457-
"-j", "ACCEPT"}
476+
"-p", portProtocol.protocol}
477+
478+
if portProtocol.port != "" {
479+
args = append(args, "--dport", portProtocol.port)
480+
}
481+
482+
args = append(args, "-j", "ACCEPT")
483+
458484
err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
459485
if err != nil {
460486
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
@@ -516,16 +542,21 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo,
516542

517543
if len(egressRule.ports) != 0 {
518544
// case where 'ports' details and 'from' details specified in the egress rule
519-
// so match on specified source and destination ip's and specified port and protocol
545+
// so match on specified source and destination ip's and specified port (if any) and protocol
520546
for _, portProtocol := range egressRule.ports {
521547
comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " +
522548
policy.name + " namespace " + policy.namespace
523549
args := []string{"-m", "comment", "--comment", comment,
524550
"-m", "set", "--set", targetSourcePodIpSetName, "src",
525551
"-m", "set", "--set", dstPodIpSetName, "dst",
526-
"-p", portProtocol.protocol,
527-
"--dport", portProtocol.port,
528-
"-j", "ACCEPT"}
552+
"-p", portProtocol.protocol}
553+
554+
if portProtocol.port != "" {
555+
args = append(args, "--dport", portProtocol.port)
556+
}
557+
558+
args = append(args, "-j", "ACCEPT")
559+
529560
err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
530561
if err != nil {
531562
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
@@ -548,16 +579,21 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo,
548579
}
549580

550581
// case where only 'ports' details specified but no 'to' details in the egress rule
551-
// so match on all sources, with specified port and protocol
582+
// so match on all sources, with specified port (if any) and protocol
552583
if egressRule.matchAllDestinations && !egressRule.matchAllPorts {
553584
for _, portProtocol := range egressRule.ports {
554585
comment := "rule to ACCEPT traffic from source pods to all destinations selected by policy name: " +
555586
policy.name + " namespace " + policy.namespace
556587
args := []string{"-m", "comment", "--comment", comment,
557588
"-m", "set", "--set", targetSourcePodIpSetName, "src",
558-
"-p", portProtocol.protocol,
559-
"--dport", portProtocol.port,
560-
"-j", "ACCEPT"}
589+
"-p", portProtocol.protocol}
590+
591+
if portProtocol.port != "" {
592+
args = append(args, "--dport", portProtocol.port)
593+
}
594+
595+
args = append(args, "-j", "ACCEPT")
596+
561597
err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
562598
if err != nil {
563599
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
@@ -596,9 +632,14 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo,
596632
args := []string{"-m", "comment", "--comment", comment,
597633
"-m", "set", "--set", targetSourcePodIpSetName, "src",
598634
"-m", "set", "--set", dstIpBlockIpSetName, "dst",
599-
"-p", portProtocol.protocol,
600-
"--dport", portProtocol.port,
601-
"-j", "ACCEPT"}
635+
"-p", portProtocol.protocol}
636+
637+
if portProtocol.port != "" {
638+
args = append(args, "--dport", portProtocol.port)
639+
}
640+
641+
args = append(args, "-j", "ACCEPT")
642+
602643
err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
603644
if err != nil {
604645
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
@@ -1127,7 +1168,7 @@ func (npc *NetworkPolicyController) buildNetworkPoliciesInfo() (*[]networkPolicy
11271168
} else {
11281169
ingressRule.matchAllPorts = false
11291170
for _, port := range specIngressRule.Ports {
1130-
protocolAndPort := protocolAndPort{protocol: string(*port.Protocol), port: port.Port.String()}
1171+
protocolAndPort := newProtocolAndPort(string(*port.Protocol), port.Port)
11311172
ingressRule.ports = append(ingressRule.ports, protocolAndPort)
11321173
}
11331174
}
@@ -1174,7 +1215,7 @@ func (npc *NetworkPolicyController) buildNetworkPoliciesInfo() (*[]networkPolicy
11741215
} else {
11751216
egressRule.matchAllPorts = false
11761217
for _, port := range specEgressRule.Ports {
1177-
protocolAndPort := protocolAndPort{protocol: string(*port.Protocol), port: port.Port.String()}
1218+
protocolAndPort := newProtocolAndPort(string(*port.Protocol), port.Port)
11781219
egressRule.ports = append(egressRule.ports, protocolAndPort)
11791220
}
11801221
}
@@ -1311,7 +1352,7 @@ func (npc *NetworkPolicyController) buildBetaNetworkPoliciesInfo() (*[]networkPo
13111352

13121353
ingressRule.ports = make([]protocolAndPort, 0)
13131354
for _, port := range specIngressRule.Ports {
1314-
protocolAndPort := protocolAndPort{protocol: string(*port.Protocol), port: port.Port.String()}
1355+
protocolAndPort := newProtocolAndPort(string(*port.Protocol), port.Port)
13151356
ingressRule.ports = append(ingressRule.ports, protocolAndPort)
13161357
}
13171358

0 commit comments

Comments
 (0)