Skip to content

Commit

Permalink
validate containerID and networkName
Browse files Browse the repository at this point in the history
ensure they contain only letters and numbers
  • Loading branch information
mccv1r0 committed Sep 3, 2019
1 parent 13b6ad6 commit d8dfb56
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 3 deletions.
6 changes: 3 additions & 3 deletions SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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).

Expand Down
7 changes: 7 additions & 0 deletions libcni/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand Down
30 changes: 30 additions & 0 deletions libcni/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 8 additions & 0 deletions pkg/skel/skel.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"strings"

"github.com/containernetworking/cni/pkg/types"
"github.com/containernetworking/cni/pkg/utils"
"github.com/containernetworking/cni/pkg/version"
)

Expand Down Expand Up @@ -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
}

Expand All @@ -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 {
Expand Down
23 changes: 23 additions & 0 deletions pkg/skel/skel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")

Expand Down Expand Up @@ -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" }`)
Expand Down
51 changes: 51 additions & 0 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit d8dfb56

Please sign in to comment.