From d8dfb56fd8290f367fcf0c2f9de11ee8a442b041 Mon Sep 17 00:00:00 2001 From: Michael Cambria Date: Wed, 7 Aug 2019 12:57:33 -0400 Subject: [PATCH] validate containerID and networkName ensure they contain only letters and numbers --- SPEC.md | 6 ++--- libcni/api.go | 7 ++++++ libcni/api_test.go | 30 +++++++++++++++++++++++++ pkg/skel/skel.go | 8 +++++++ pkg/skel/skel_test.go | 23 +++++++++++++++++++ pkg/utils/utils.go | 51 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 pkg/utils/utils.go diff --git a/SPEC.md b/SPEC.md index e6b031b0..16fbe24e 100644 --- a/SPEC.md +++ b/SPEC.md @@ -64,7 +64,7 @@ The operations that CNI plugins must support are: - `ADD`: Add container to network - Parameters: - - **Container ID**. A unique plaintext identifier for a container, allocated by the runtime. Must not be empty. + - **Container ID**. A unique plaintext identifier for a container, allocated by the runtime. Must not be empty. Must start with a alphanumeric character, optionally followed by any combination of one or more alphanumeric characters, underscore (_), dot (.) or hyphen (-). - **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. @@ -226,7 +226,7 @@ In addition, stderr can be used for unstructured output such as logs. The network configuration is described in JSON form. The configuration may be stored on disk or generated from other sources by the container runtime. The following fields are well-known and have the following meaning: - `cniVersion` (string): [Semantic Version 2.0](https://semver.org) of CNI specification to which this configuration conforms. -- `name` (string): Network name. This should be unique across all containers on the host (or other administrative domain). +- `name` (string): Network name. This should be unique across all containers on the host (or other administrative domain). Must start with a alphanumeric character, optionally followed by any combination of one or more alphanumeric characters, underscore (_), dot (.) or hyphen (-). - `type` (string): Refers to the filename of the CNI plugin executable. - `args` (dictionary, optional): Additional arguments provided by the container runtime. For example a dictionary of labels could be passed to CNI plugins by adding them to a labels field under `args`. - `ipMasq` (boolean, optional): If supported by the plugin, sets up an IP masquerade on the host for this network. This is necessary if the host will act as a gateway to subnets that are not able to route to the IP assigned to the container. @@ -305,7 +305,7 @@ The list is composed of well-known fields and list of one or more standard CNI n The list is described in JSON form, and can be stored on disk or generated from other sources by the container runtime. The following fields are well-known and have the following meaning: - `cniVersion` (string): [Semantic Version 2.0](https://semver.org) of CNI specification to which this configuration list and all the individual configurations conform. -- `name` (string): Network name. This should be unique across all containers on the host (or other administrative domain). +- `name` (string): Network name. This should be unique across all containers on the host (or other administrative domain). Must start with a alphanumeric character, optionally followed by any combination of one or more alphanumeric characters, underscore (_), dot (.) or hyphen (-). - `disableCheck` (string): Either `true` or `false`. If `disableCheck` is `true`, runtimes must not call `CHECK` for this network configuration list. This allows an administrator to prevent `CHECK`ing where a combination of plugins is known to return spurious errors. - `plugins` (list): A list of standard CNI network configuration dictionaries (see above). diff --git a/libcni/api.go b/libcni/api.go index e0642b94..fdaca17e 100644 --- a/libcni/api.go +++ b/libcni/api.go @@ -25,6 +25,7 @@ import ( "github.com/containernetworking/cni/pkg/invoke" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/utils" "github.com/containernetworking/cni/pkg/version" ) @@ -379,6 +380,12 @@ func (c *CNIConfig) addNetwork(ctx context.Context, name, cniVersion string, net if err != nil { return nil, err } + if err := utils.ValidateContainerID(rt.ContainerID); err != nil { + return nil, err + } + if err := utils.ValidateNetworkName(name); 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 48f96c81..eee496a2 100644 --- a/libcni/api_test.go +++ b/libcni/api_test.go @@ -1004,6 +1004,36 @@ var _ = Describe("Invoking plugins", func() { }) }) + Context("when there is an invalid containerID", func() { + BeforeEach(func() { + runtimeConfig.ContainerID = "some-%%container-id" + }) + + It("returns the error", func() { + _, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig) + Expect(err).To(Equal(&types.Error{ + Code: 4, + Msg: "invalid characters in containerID", + Details: "some-%%container-id", + })) + }) + }) + + Context("when there is an invalid networkName", func() { + BeforeEach(func() { + netConfigList.Name = "invalid-%%-name" + }) + + It("returns the error", func() { + _, err := cniConfig.AddNetworkList(ctx, netConfigList, runtimeConfig) + Expect(err).To(Equal(&types.Error{ + Code: 7, + Msg: "invalid characters found in network name", + Details: "invalid-%%-name", + })) + }) + }) + 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 dc1bdd85..af506b48 100644 --- a/pkg/skel/skel.go +++ b/pkg/skel/skel.go @@ -27,6 +27,7 @@ import ( "strings" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/utils" "github.com/containernetworking/cni/pkg/version" ) @@ -183,6 +184,9 @@ func validateConfig(jsonBytes []byte) *types.Error { if conf.Name == "" { return types.NewError(types.ErrInvalidNetworkConfig, "missing network name", "") } + if err := utils.ValidateNetworkName(conf.Name); err != nil { + return err + } return nil } @@ -202,6 +206,10 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdCheck, cmdDel func(_ *CmdArgs) error, if err != nil { return err } + err = utils.ValidateContainerID(cmdArgs.ContainerID) + if err != nil { + return err + } } switch cmd { diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index d1d95c25..26691d16 100644 --- a/pkg/skel/skel_test.go +++ b/pkg/skel/skel_test.go @@ -114,6 +114,17 @@ var _ = Describe("dispatching to the correct callback", func() { Expect(cmdAdd.Received.CmdArgs).To(Equal(expectedCmdArgs)) }) + It("returns an error when containerID has invalid characters", func() { + environment["CNI_CONTAINERID"] = "some-%%container-id" + err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") + Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(&types.Error{ + Code: 4, + Msg: "invalid characters in containerID", + Details: "some-%%container-id", + })) + }) + It("does not call cmdCheck or cmdDel", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") @@ -279,6 +290,18 @@ var _ = Describe("dispatching to the correct callback", func() { }) }) + Context("when the config has a bad name", func() { + It("immediately returns invalid network config", func() { + dispatch.Stdin = strings.NewReader(`{ "cniVersion": "0.4.0", "some": "config", "name": "te%%st" }`) + versionInfo = version.PluginSupports("0.1.0", "0.2.0", "0.3.0", "0.4.0") + err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") + Expect(err.Code).To(Equal(uint(types.ErrInvalidNetworkConfig))) + Expect(cmdAdd.CallCount).To(Equal(0)) + Expect(cmdCheck.CallCount).To(Equal(0)) + Expect(cmdDel.CallCount).To(Equal(0)) + }) + }) + Context("when the plugin has a bad version", func() { It("immediately returns a useful error", func() { dispatch.Stdin = strings.NewReader(`{ "cniVersion": "0.4.0", "some": "config", "name": "test" }`) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go new file mode 100644 index 00000000..324c40de --- /dev/null +++ b/pkg/utils/utils.go @@ -0,0 +1,51 @@ +// Copyright 2019 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package utils + +import ( + "regexp" + + "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_.\-]` + +var cniReg = regexp.MustCompile(`^` + cniValidNameChars + `*$`) + +// ValidateContainerID will validate that the supplied containerID is not empty does not contain invalid characters +func ValidateContainerID(containerID string) *types.Error { + + if containerID == "" { + return types.NewError(types.ErrUnknownContainer, "missing containerID", "") + } + if !cniReg.MatchString(containerID) { + return types.NewError(types.ErrInvalidEnvironmentVariables, "invalid characters in containerID", containerID) + } + return nil +} + +// ValidateNetworkName will validate that the supplied networkName does not contain invalid characters +func ValidateNetworkName(networkName string) *types.Error { + + if networkName == "" { + return types.NewError(types.ErrInvalidNetworkConfig, "missing network name:", "") + } + if !cniReg.MatchString(networkName) { + return types.NewError(types.ErrInvalidNetworkConfig, "invalid characters found in network name", networkName) + } + return nil +}