add interface name validation

SPEC.md

@@ -68,7 +68,7 @@ The operations that CNI plugins must support are:
     - **Network namespace path**. This represents the path to the network namespace to be added, i.e. /proc/[pid]/ns/net or a bind-mount/link to it.
     - **Network configuration**. This is a JSON document describing a network to which a container can be joined. The schema is described below.
     - **Extra arguments**. This provides an alternative mechanism to allow simple configuration of CNI plugins on a per-container basis.
-    - **Name of the interface inside the container**. This is the name that should be assigned to the interface created inside the container (network namespace); consequently it must comply with the standard Linux restrictions on interface names.
+    - **Name of the interface inside the container**. This is the name that should be assigned to the interface created inside the container (network namespace); consequently it must comply with the standard Linux restrictions on interface names, must not be empty, must not be "." or "..", must be less than 16 characters and must not contain / or : or any whitespace characters.
   - Result:
     - **Interfaces list**. Depending on the plugin, this can include the sandbox (eg, container or hypervisor) interface name and/or the host interface name, the hardware addresses of each interface, and details about the sandbox (if any) the interface is in.
     - **IP configuration assigned to each interface**. The IPv4 and/or IPv6 addresses, gateways, and routes assigned to sandbox and/or host interfaces.

libcni/api.go

@@ -409,6 +409,9 @@ func (c *CNIConfig) addNetwork(ctx context.Context, name, cniVersion string, net
 	if err := utils.ValidateNetworkName(name); err != nil {
 		return nil, err
 	}
+	if err := utils.ValidateInterfaceName(rt.IfName); err != nil {
+		return nil, err
+	}
 
 	newConf, err := buildOneConfig(name, cniVersion, net, prevResult, rt)
 	if err != nil {

libcni/api_test.go

@@ -1012,7 +1012,7 @@ var _ = Describe("Invoking plugins", func() {
 				It("returns the error", func() {
 					_, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig)
 					Expect(err).To(Equal(&types.Error{
-						Code:    4,
+						Code:    types.ErrInvalidEnvironmentVariables,
 						Msg:     "invalid characters in containerID",
 						Details: "some-%%container-id",
 					}))
@@ -1027,13 +1027,92 @@ var _ = Describe("Invoking plugins", func() {
 				It("returns the error", func() {
 					_, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig)
 					Expect(err).To(Equal(&types.Error{
-						Code:    7,
+						Code:    types.ErrInvalidNetworkConfig,
 						Msg:     "invalid characters found in network name",
 						Details: "invalid-%%-name",
 					}))
 				})
 			})
 
+			Context("return errors when interface name is invalid", func() {
+				It("interface name is empty", func() {
+					runtimeConfig.IfName = ""
+
+					_, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig)
+					Expect(err).To(Equal(&types.Error{
+						Code:    types.ErrInvalidEnvironmentVariables,
+						Msg:     "interface name is empty",
+						Details: "",
+					}))
+				})
+
+				It("interface name is too long", func() {
+					runtimeConfig.IfName = "1234567890123456"
+
+					_, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig)
+					Expect(err).To(Equal(&types.Error{
+						Code:    types.ErrInvalidEnvironmentVariables,
+						Msg:     "interface name is too long",
+						Details: "interface name should be less than 16 characters",
+					}))
+				})
+
+				It("interface name is .", func() {
+					runtimeConfig.IfName = "."
+
+					_, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig)
+					Expect(err).To(Equal(&types.Error{
+						Code:    types.ErrInvalidEnvironmentVariables,
+						Msg:     "interface name is . or ..",
+						Details: "",
+					}))
+				})
+
+				It("interface name is ..", func() {
+					runtimeConfig.IfName = ".."
+
+					_, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig)
+					Expect(err).To(Equal(&types.Error{
+						Code:    types.ErrInvalidEnvironmentVariables,
+						Msg:     "interface name is . or ..",
+						Details: "",
+					}))
+				})
+
+				It("interface name contains invalid characters /", func() {
+					runtimeConfig.IfName = "test/test"
+
+					_, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig)
+					Expect(err).To(Equal(&types.Error{
+						Code:    types.ErrInvalidEnvironmentVariables,
+						Msg:     "interface name contains / or : or whitespace characters",
+						Details: "",
+					}))
+				})
+
+				It("interface name contains invalid characters :", func() {
+					runtimeConfig.IfName = "test:test"
+
+					_, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig)
+					Expect(err).To(Equal(&types.Error{
+						Code:    types.ErrInvalidEnvironmentVariables,
+						Msg:     "interface name contains / or : or whitespace characters",
+						Details: "",
+					}))
+				})
+
+				It("interface name contains invalid characters whitespace", func() {
+					runtimeConfig.IfName = "test test"
+
+					_, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig)
+					Expect(err).To(Equal(&types.Error{
+						Code:    types.ErrInvalidEnvironmentVariables,
+						Msg:     "interface name contains / or : or whitespace characters",
+						Details: "",
+					}))
+				})
+			})
+
 			Context("when the second plugin errors", func() {
 				BeforeEach(func() {
 					plugins[1].debug.ReportError = "plugin error: banana"

pkg/skel/skel.go

@@ -202,12 +202,13 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdCheck, cmdDel func(_ *CmdArgs) error,
 	}
 
 	if cmd != "VERSION" {
-		err = validateConfig(cmdArgs.StdinData)
-		if err != nil {
+		if err = validateConfig(cmdArgs.StdinData); err != nil {
 			return err
 		}
-		err = utils.ValidateContainerID(cmdArgs.ContainerID)
-		if err != nil {
+		if err = utils.ValidateContainerID(cmdArgs.ContainerID); err != nil {
+			return err
+		}
+		if err = utils.ValidateInterfaceName(cmdArgs.IfName); err != nil {
 			return err
 		}
 	}

pkg/skel/skel_test.go

@@ -119,12 +119,86 @@ var _ = Describe("dispatching to the correct callback", func() {
 			err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "")
 			Expect(err).To(HaveOccurred())
 			Expect(err).To(Equal(&types.Error{
-				Code:    4,
+				Code:    types.ErrInvalidEnvironmentVariables,
 				Msg:     "invalid characters in containerID",
 				Details: "some-%%container-id",
 			}))
 		})
 
+		Context("return errors when interface name is invalid", func() {
+			It("interface name is too long", func() {
+				environment["CNI_IFNAME"] = "1234567890123456"
+
+				err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "")
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(Equal(&types.Error{
+					Code:    types.ErrInvalidEnvironmentVariables,
+					Msg:     "interface name is too long",
+					Details: "interface name should be less than 16 characters",
+				}))
+			})
+
+			It("interface name is .", func() {
+				environment["CNI_IFNAME"] = "."
+
+				err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "")
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(Equal(&types.Error{
+					Code:    types.ErrInvalidEnvironmentVariables,
+					Msg:     "interface name is . or ..",
+					Details: "",
+				}))
+			})
+
+			It("interface name is ..", func() {
+				environment["CNI_IFNAME"] = ".."
+
+				err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "")
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(Equal(&types.Error{
+					Code:    types.ErrInvalidEnvironmentVariables,
+					Msg:     "interface name is . or ..",
+					Details: "",
+				}))
+			})
+
+			It("interface name contains invalid characters /", func() {
+				environment["CNI_IFNAME"] = "test/test"
+
+				err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "")
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(Equal(&types.Error{
+					Code:    types.ErrInvalidEnvironmentVariables,
+					Msg:     "interface name contains / or : or whitespace characters",
+					Details: "",
+				}))
+			})
+
+			It("interface name contains invalid characters :", func() {
+				environment["CNI_IFNAME"] = "test:test"
+
+				err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "")
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(Equal(&types.Error{
+					Code:    types.ErrInvalidEnvironmentVariables,
+					Msg:     "interface name contains / or : or whitespace characters",
+					Details: "",
+				}))
+			})
+
+			It("interface name contains invalid characters whitespace", func() {
+				environment["CNI_IFNAME"] = "test test"
+
+				err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "")
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(Equal(&types.Error{
+					Code:    types.ErrInvalidEnvironmentVariables,
+					Msg:     "interface name contains / or : or whitespace characters",
+					Details: "",
+				}))
+			})
+		})
+
 		It("does not call cmdCheck or cmdDel", func() {
 			err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "")
 

pkg/utils/utils.go

@@ -15,14 +15,22 @@
 package utils
 
 import (
+	"bytes"
+	"fmt"
 	"regexp"
+	"unicode"
 
 	"github.com/containernetworking/cni/pkg/types"
 )
 
-// cniValidNameChars is the regexp used to validate valid characters in
-// containerID and networkName
-const cniValidNameChars = `[a-zA-Z0-9][a-zA-Z0-9_.\-]`
+const (
+	// cniValidNameChars is the regexp used to validate valid characters in
+	// containerID and networkName
+	cniValidNameChars = `[a-zA-Z0-9][a-zA-Z0-9_.\-]`
+
+	// maxInterfaceNameLength is the length max of a valid interface name
+	maxInterfaceNameLength = 15
+)
 
 var cniReg = regexp.MustCompile(`^` + cniValidNameChars + `*$`)
 
@@ -49,3 +57,28 @@ func ValidateNetworkName(networkName string) *types.Error {
 	}
 	return nil
 }
+
+// ValidateInterfaceName will validate the interface name based on the three rules below
+// 1. The name must not be empty
+// 2. The name must be less than 16 characters
+// 3. The name must not be "." or ".."
+// 3. The name must not contain / or : or any whitespace characters
+// ref to https://github.com/torvalds/linux/blob/master/net/core/dev.c#L1024
+func ValidateInterfaceName(ifName string) *types.Error {
+	if len(ifName) == 0 {
+		return types.NewError(types.ErrInvalidEnvironmentVariables, "interface name is empty", "")
+	}
+	if len(ifName) > maxInterfaceNameLength {
+		return types.NewError(types.ErrInvalidEnvironmentVariables, "interface name is too long", fmt.Sprintf("interface name should be less than %d characters", maxInterfaceNameLength+1))
+	}
+	if ifName == "." || ifName == ".." {
+		return types.NewError(types.ErrInvalidEnvironmentVariables, "interface name is . or ..", "")
+	}
+	for _, r := range bytes.Runes([]byte(ifName)) {
+		if r == '/' || r == ':' || unicode.IsSpace(r) {
+			return types.NewError(types.ErrInvalidEnvironmentVariables, "interface name contains / or : or whitespace characters", "")
+		}
+	}
+
+	return nil
+}