runtime: Validate CID

Validate the container ID as we cannot rely on the container manager
doing this.

Fixes: #1520.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

src/runtime/cli/kata-exec.go

@@ -87,9 +87,11 @@ var kataExecCLICommand = cli.Command{
 		span.SetAttributes(label.Key("port").Uint64(port))
 
 		sandboxID := context.Args().Get(0)
-		if sandboxID == "" {
+
-			return fmt.Errorf("SandboxID not found")
+		if err := katautils.VerifyContainerID(sandboxID); err != nil {
+			return err
 		}
+
 		span.SetAttributes(label.Key("sandbox").String(sandboxID))
 
 		conn, err := getConn(namespace, sandboxID, port)

src/runtime/containerd-shim-v2/service.go

@@ -367,6 +367,10 @@ func (s *service) Create(ctx context.Context, r *taskAPI.CreateTaskRequest) (_ *
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
+	if err := katautils.VerifyContainerID(r.ID); err != nil {
+		return nil, err
+	}
+
 	type Result struct {
 		container *container
 		err       error

src/runtime/containerd-shim-v2/service_test.go

@@ -0,0 +1,80 @@
+// Copyright (c) 2021 Intel Corporation
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package containerdshim
+
+import (
+	"context"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	taskAPI "github.com/containerd/containerd/runtime/v2/task"
+	ktu "github.com/kata-containers/kata-containers/src/runtime/pkg/katatestutils"
+	"github.com/stretchr/testify/assert"
+)
+
+func NewService(id string) (service, error) {
+	ctx := context.Background()
+
+	ctx, cancel := context.WithCancel(ctx)
+
+	s := service{
+		id:         id,
+		pid:        uint32(os.Getpid()),
+		ctx:        ctx,
+		containers: make(map[string]*container),
+		events:     make(chan interface{}, chSize),
+		ec:         make(chan exit, bufferSize),
+		cancel:     cancel,
+	}
+
+	return s, nil
+}
+
+func TestServiceCreate(t *testing.T) {
+	const badCIDErrorPrefix = "invalid container/sandbox ID"
+	const blankCIDError = "ID cannot be blank"
+
+	assert := assert.New(t)
+
+	tmpdir, err := ioutil.TempDir("", "")
+	defer os.RemoveAll(tmpdir)
+
+	bundleDir := filepath.Join(tmpdir, "bundle")
+	err = makeOCIBundle(bundleDir)
+	assert.NoError(err)
+
+	ctx := context.Background()
+
+	s, err := NewService("foo")
+	assert.NoError(err)
+
+	for i, d := range ktu.ContainerIDTestData {
+		msg := fmt.Sprintf("test[%d]: %+v", i, d)
+
+		// Only consider error scenarios as we are only testing invalid CIDs here.
+		if d.Valid {
+			continue
+		}
+
+		task := taskAPI.CreateTaskRequest{
+			ID:     d.ID,
+			Bundle: bundleDir,
+		}
+
+		_, err = s.Create(ctx, &task)
+		assert.Error(err, msg)
+
+		if d.ID == "" {
+			assert.Equal(err.Error(), blankCIDError, msg)
+		} else {
+			assert.True(strings.HasPrefix(err.Error(), badCIDErrorPrefix), msg)
+		}
+	}
+}

src/runtime/pkg/katatestutils/utils.go

@@ -47,6 +47,62 @@ type RuntimeConfigOptions struct {
 	JaegerPassword       string
 }
 
+// ContainerIDTestDataType is a type used to test Container and Sandbox ID's.
+type ContainerIDTestDataType struct {
+	ID    string
+	Valid bool
+}
+
+// Set of test data that lists valid and invalid Container IDs
+var ContainerIDTestData = []ContainerIDTestDataType{
+	{"", false},   // Cannot be blank
+	{" ", false},  // Cannot be a space
+	{".", false},  // Must start with an alphanumeric
+	{"-", false},  // Must start with an alphanumeric
+	{"_", false},  // Must start with an alphanumeric
+	{" a", false}, // Must start with an alphanumeric
+	{".a", false}, // Must start with an alphanumeric
+	{"-a", false}, // Must start with an alphanumeric
+	{"_a", false}, // Must start with an alphanumeric
+	{"..", false}, // Must start with an alphanumeric
+	{"a", false},  // Too short
+	{"z", false},  // Too short
+	{"A", false},  // Too short
+	{"Z", false},  // Too short
+	{"0", false},  // Too short
+	{"9", false},  // Too short
+	{"-1", false}, // Must start with an alphanumeric
+	{"/", false},
+	{"a/", false},
+	{"a/../", false},
+	{"../a", false},
+	{"../../a", false},
+	{"../../../a", false},
+	{"foo/../bar", false},
+	{"foo bar", false},
+	{"a.", true},
+	{"a..", true},
+	{"aa", true},
+	{"aa.", true},
+	{"hello..world", true},
+	{"hello/../world", false},
+	{"aa1245124sadfasdfgasdga.", true},
+	{"aAzZ0123456789_.-", true},
+	{"abcdefghijklmnopqrstuvwxyz0123456789.-_", true},
+	{"0123456789abcdefghijklmnopqrstuvwxyz.-_", true},
+	{" abcdefghijklmnopqrstuvwxyz0123456789.-_", false},
+	{".abcdefghijklmnopqrstuvwxyz0123456789.-_", false},
+	{"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_", true},
+	{"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ.-_", true},
+	{" ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_", false},
+	{".ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_", false},
+	{"/a/b/c", false},
+	{"a/b/c", false},
+	{"foo/../../../etc/passwd", false},
+	{"../../../../../../etc/motd", false},
+	{"/etc/passwd", false},
+}
+
 func MakeRuntimeConfigFileData(config RuntimeConfigOptions) string {
 	return `
 	# Runtime configuration file

src/runtime/pkg/katautils/utils.go

@@ -7,15 +7,21 @@
 package katautils
 
 import (
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
 	"syscall"
 
 	"golang.org/x/sys/unix"
 )
 
+// validCIDRegex is a regular expression used to determine
+// if a container ID (or sandbox ID) is valid.
+const validCIDRegex = `^[a-zA-Z0-9][a-zA-Z0-9_.-]+$`
+
 // FileExists test is a file exiting or not
 func FileExists(path string) bool {
 	if _, err := os.Stat(path); os.IsNotExist(err) {
@@ -109,3 +115,22 @@ func GetFileContents(file string) (string, error) {
 
 	return string(bytes), nil
 }
+
+// VerifyContainerID checks if the specified container ID
+// (or sandbox ID) is valid.
+func VerifyContainerID(id string) error {
+	if id == "" {
+		return errors.New("ID cannot be blank")
+	}
+
+	// Note: no length check.
+	validPattern := regexp.MustCompile(validCIDRegex)
+
+	matches := validPattern.MatchString(id)
+
+	if !matches {
+		return fmt.Errorf("invalid container/sandbox ID (should match %q)", validCIDRegex)
+	}
+
+	return nil
+}

src/runtime/pkg/katautils/utils_test.go

@@ -18,6 +18,7 @@ import (
 	"syscall"
 	"testing"
 
+	ktu "github.com/kata-containers/kata-containers/src/runtime/pkg/katatestutils"
 	"github.com/kata-containers/kata-containers/src/runtime/pkg/utils"
 	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/compatoci"
 	"github.com/stretchr/testify/assert"
@@ -366,3 +367,18 @@ func TestGetFileContents(t *testing.T) {
 		assert.Equal(t, contents, d.contents)
 	}
 }
+
+func TestVerifyContainerID(t *testing.T) {
+	assert := assert.New(t)
+
+	for i, d := range ktu.ContainerIDTestData {
+		msg := fmt.Sprintf("test[%d]: %+v", i, d)
+
+		err := VerifyContainerID(d.ID)
+		if d.Valid {
+			assert.NoError(err, msg)
+		} else {
+			assert.Error(err, msg)
+		}
+	}
+}