portmap: Don't accept a port number of 0

This also adds more testing around configuration parsing.

diff --git a/plugins/meta/portmap/main.go b/plugins/meta/portmap/main.go
index c0c34ae..c58db6a 100644 (file)
--- a/plugins/meta/portmap/main.go
+++ b/plugins/meta/portmap/main.go
@@ -28,6 +28,7 @@ package main
 import (
        "encoding/json"
        "fmt"
+       "net"
 
        "github.com/containernetworking/cni/pkg/skel"
        "github.com/containernetworking/cni/pkg/types"
@@ -54,11 +55,16 @@ type PortMapConf struct {
        } `json:"runtimeConfig,omitempty"`
        RawPrevResult map[string]interface{} `json:"prevResult,omitempty"`
        PrevResult    *current.Result        `json:"-"`
-       ContainerID   string
+
+       // These are fields parsed out of the config or the environment;
+       // included here for convenience
+       ContainerID string `json:"-"`
+       ContIPv4    net.IP `json:"-"`
+       ContIPv6    net.IP `json:"-"`
 }
 
 func cmdAdd(args *skel.CmdArgs) error {
-       netConf, err := parseConfig(args.StdinData)
+       netConf, err := parseConfig(args.StdinData, args.IfName)
        if err != nil {
                return fmt.Errorf("failed to parse config: %v", err)
        }
@@ -73,31 +79,15 @@ func cmdAdd(args *skel.CmdArgs) error {
 
        netConf.ContainerID = args.ContainerID
 
-       // Loop through IPs, setting up forwarding to the first container IP
-       // per family
-       hasV4 := false
-       hasV6 := false
-       for _, ip := range netConf.PrevResult.IPs {
-               if ip.Version == "6" && hasV6 {
-                       continue
-               } else if ip.Version == "4" && hasV4 {
-                       continue
-               }
-
-               // Skip known non-sandbox interfaces
-               intIdx := ip.Interface
-               if intIdx >= 0 && intIdx < len(netConf.PrevResult.Interfaces) && netConf.PrevResult.Interfaces[intIdx].Name != args.IfName {
-                       continue
-               }
-
-               if err := forwardPorts(netConf, ip.Address.IP); err != nil {
+       if netConf.ContIPv4 != nil {
+               if err := forwardPorts(netConf, netConf.ContIPv4); err != nil {
                        return err
                }
+       }
 
-               if ip.Version == "6" {
-                       hasV6 = true
-               } else {
-                       hasV4 = true
+       if netConf.ContIPv6 != nil {
+               if err := forwardPorts(netConf, netConf.ContIPv6); err != nil {
+                       return err
                }
        }
 
@@ -106,7 +96,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 }
 
 func cmdDel(args *skel.CmdArgs) error {
-       netConf, err := parseConfig(args.StdinData)
+       netConf, err := parseConfig(args.StdinData, args.IfName)
        if err != nil {
                return fmt.Errorf("failed to parse config: %v", err)
        }
@@ -126,7 +116,7 @@ func main() {
 }
 
 // parseConfig parses the supplied configuration (and prevResult) from stdin.
-func parseConfig(stdin []byte) (*PortMapConf, error) {
+func parseConfig(stdin []byte, ifName string) (*PortMapConf, error) {
        conf := PortMapConf{}
 
        if err := json.Unmarshal(stdin, &conf); err != nil {
@@ -155,5 +145,37 @@ func parseConfig(stdin []byte) (*PortMapConf, error) {
                conf.SNAT = &tvar
        }
 
+       // Reject invalid port numbers
+       for _, pm := range conf.RuntimeConfig.PortMaps {
+               if pm.ContainerPort <= 0 {
+                       return nil, fmt.Errorf("Invalid container port number: %d", pm.ContainerPort)
+               }
+               if pm.HostPort <= 0 {
+                       return nil, fmt.Errorf("Invalid host port number: %d", pm.HostPort)
+               }
+       }
+
+       if conf.PrevResult != nil {
+               for _, ip := range conf.PrevResult.IPs {
+                       if ip.Version == "6" && conf.ContIPv6 != nil {
+                               continue
+                       } else if ip.Version == "4" && conf.ContIPv4 != nil {
+                               continue
+                       }
+
+                       // Skip known non-sandbox interfaces
+                       intIdx := ip.Interface
+                       if intIdx >= 0 && intIdx < len(conf.PrevResult.Interfaces) && conf.PrevResult.Interfaces[intIdx].Name != ifName {
+                               continue
+                       }
+                       switch ip.Version {
+                       case "6":
+                               conf.ContIPv6 = ip.Address.IP
+                       case "4":
+                               conf.ContIPv4 = ip.Address.IP
+                       }
+               }
+       }
+
        return &conf, nil
 }

diff --git a/plugins/meta/portmap/portmap_integ_test.go b/plugins/meta/portmap/portmap_integ_test.go
index ff1a3ef..bcdbcfd 100644 (file)
--- a/plugins/meta/portmap/portmap_integ_test.go
+++ b/plugins/meta/portmap/portmap_integ_test.go
@@ -30,6 +30,8 @@ import (
        "github.com/vishvananda/netlink"
 )
 
+const TIMEOUT = 20
+
 var _ = Describe("portmap integration tests", func() {
 
        var configList *libcni.NetworkConfigList
@@ -173,7 +175,7 @@ var _ = Describe("portmap integration tests", func() {
 
                close(done)
 
-       }, 5)
+       }, TIMEOUT*3)
 })
 
 // testEchoServer returns true if we found an echo server on the port
@@ -186,7 +188,7 @@ func testEchoServer(address string) bool {
        }
        defer conn.Close()
 
-       conn.SetDeadline(time.Now().Add(20 * time.Second))
+       conn.SetDeadline(time.Now().Add(TIMEOUT * time.Second))
        fmt.Fprintln(GinkgoWriter, "connected to", address)
 
        message := "Aliquid melius quam pessimum optimum non est."
@@ -196,7 +198,7 @@ func testEchoServer(address string) bool {
                return false
        }
 
-       conn.SetDeadline(time.Now().Add(20 * time.Second))
+       conn.SetDeadline(time.Now().Add(TIMEOUT * time.Second))
        fmt.Fprintln(GinkgoWriter, "reading...")
        response := make([]byte, len(message))
        _, err = conn.Read(response)

diff --git a/plugins/meta/portmap/portmap_test.go b/plugins/meta/portmap/portmap_test.go
index 6e28461..e67907f 100644 (file)
--- a/plugins/meta/portmap/portmap_test.go
+++ b/plugins/meta/portmap/portmap_test.go
@@ -32,6 +32,100 @@ var _ = Describe("portmapping configuration", func() {
        ipv4addr := net.ParseIP("192.2.0.1")
        ipv6addr := net.ParseIP("2001:db8::1")
 
+       Context("config parsing", func() {
+               It("Correctly parses an ADD config", func() {
+                       configBytes := []byte(`{
+       "name": "test",
+       "type": "portmap",
+       "cniVersion": "0.3.1",
+       "runtimeConfig": {
+               "portMappings": [
+                       { "hostPort": 8080, "containerPort": 80, "protocol": "tcp"},
+                       { "hostPort": 8081, "containerPort": 81, "protocol": "udp"}
+               ]
+       },
+       "snat": false,
+       "conditionsV4": ["a", "b"],
+       "conditionsV6": ["c", "d"],
+       "prevResult": {
+               "interfaces": [
+                       {"name": "host"},
+                       {"name": "container", "sandbox":"netns"}
+               ],
+               "ips": [
+                       {
+                               "version": "4",
+                               "address": "10.0.0.1/24",
+                               "gateway": "10.0.0.1",
+                               "interface": 0
+                       },
+                       {
+                               "version": "6",
+                               "address": "2001:db8:1::2/64",
+                               "gateway": "2001:db8:1::1",
+                               "interface": 1
+                       },
+                       {
+                               "version": "4",
+                               "address": "10.0.0.2/24",
+                               "gateway": "10.0.0.1",
+                               "interface": 1
+                       }
+               ]
+       }
+}`)
+                       c, err := parseConfig(configBytes, "container")
+                       Expect(err).NotTo(HaveOccurred())
+                       Expect(c.CNIVersion).To(Equal("0.3.1"))
+                       Expect(c.ConditionsV4).To(Equal(&[]string{"a", "b"}))
+                       Expect(c.ConditionsV6).To(Equal(&[]string{"c", "d"}))
+                       fvar := false
+                       Expect(c.SNAT).To(Equal(&fvar))
+                       Expect(c.Name).To(Equal("test"))
+
+                       Expect(c.ContIPv4).To(Equal(net.ParseIP("10.0.0.2")))
+                       Expect(c.ContIPv6).To(Equal(net.ParseIP("2001:db8:1::2")))
+               })
+
+               It("Correctly parses a DEL config", func() {
+                       // When called with DEL, neither runtimeConfig nor prevResult may be specified
+                       configBytes := []byte(`{
+       "name": "test",
+       "type": "portmap",
+       "cniVersion": "0.3.1",
+       "snat": false,
+       "conditionsV4": ["a", "b"],
+       "conditionsV6": ["c", "d"]
+}`)
+                       c, err := parseConfig(configBytes, "container")
+                       Expect(err).NotTo(HaveOccurred())
+                       Expect(c.CNIVersion).To(Equal("0.3.1"))
+                       Expect(c.ConditionsV4).To(Equal(&[]string{"a", "b"}))
+                       Expect(c.ConditionsV6).To(Equal(&[]string{"c", "d"}))
+                       fvar := false
+                       Expect(c.SNAT).To(Equal(&fvar))
+                       Expect(c.Name).To(Equal("test"))
+               })
+
+               It("fails with invalid mappings", func() {
+                       configBytes := []byte(`{
+       "name": "test",
+       "type": "portmap",
+       "cniVersion": "0.3.1",
+       "snat": false,
+       "conditionsV4": ["a", "b"],
+       "conditionsV6": ["c", "d"],
+       "runtimeConfig": {
+               "portMappings": [
+                       { "hostPort": 0, "containerPort": 80, "protocol": "tcp"}
+               ]
+       }
+}`)
+                       _, err := parseConfig(configBytes, "container")
+                       Expect(err).To(MatchError("Invalid host port number: 0"))
+               })
+       })
+
        Describe("Generating chains", func() {
                Context("for DNAT", func() {
                        It("generates a correct container chain", func() {