portmap: Don't accept a port number of 0

This also adds more testing around configuration parsing.

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
+	if netConf.ContIPv4 != nil {
-	// per family
+		if err := forwardPorts(netConf, netConf.ContIPv4); err != nil {
-	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 {
 			return err
 		}
+	}
 
-		if ip.Version == "6" {
+	if netConf.ContIPv6 != nil {
-			hasV6 = true
+		if err := forwardPorts(netConf, netConf.ContIPv6); err != nil {
-		} else {
+			return err
-			hasV4 = true
 		}
 	}
 
@@ -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
 }

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)

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() {