diff --git a/SPEC.md b/SPEC.md index 16fbe24e..81456673 100644 --- a/SPEC.md +++ b/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. diff --git a/libcni/api.go b/libcni/api.go index 22b11174..893c1979 100644 --- a/libcni/api.go +++ b/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 { diff --git a/libcni/api_test.go b/libcni/api_test.go index 9f277a62..05369ef8 100644 --- a/libcni/api_test.go +++ b/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" diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go index af506b48..da42db55 100644 --- a/pkg/skel/skel.go +++ b/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 } } diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index 26691d16..2284dd06 100644 --- a/pkg/skel/skel_test.go +++ b/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, "") diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 324c40de..b8ec3887 100644 --- a/pkg/utils/utils.go +++ b/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 +}