github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-06-27 15:57:09 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #1521 from jodh-intel/verify-cid

Browse Source 
Verify container ID
This commit is contained in:
James O. D. Hunt 2021-03-24 13:27:58 +00:00 committed by GitHub
commit 2fc7f75724
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 435 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

246
src/agent/src/rpc.rs
View File

@ -80,6 +80,23 @@ pub struct AgentService {
sandbox: Arc<Mutex<Sandbox>>,
}
// A container ID must match this regex:
//
// ^[a-zA-Z0-9][a-zA-Z0-9_.-]+$
//
fn verify_cid(id: &str) -> Result<()> {
let valid = id.len() > 1
&& id.chars().next().unwrap().is_alphanumeric()
&& id
.chars()
.all(|c| (c.is_alphanumeric() || ['.', '-', '_'].contains(&c)));
match valid {
true => Ok(()),
false => Err(anyhow!("invalid container ID: {:?}", id)),
}
}
impl AgentService {
async fn do_create_container(
&self,
@ -87,6 +104,8 @@ impl AgentService {
) -> Result<()> {
let cid = req.container_id.clone();
let _ = verify_cid(&cid)?;
let mut oci_spec = req.OCI.clone();
let use_sandbox_pidns = req.get_sandbox_pidns();
@ -1778,4 +1797,231 @@ mod tests {
assert!(result.is_err(), "expected add arp neighbors to fail");
}
#[tokio::test]
async fn test_verify_cid() {
#[derive(Debug)]
struct TestData<'a> {
id: &'a str,
expect_error: bool,
}
let tests = &[
TestData {
// Cannot be blank
id: "",
expect_error: true,
},
TestData {
// Cannot be a space
id: " ",
expect_error: true,
},
TestData {
// Must start with an alphanumeric
id: ".",
expect_error: true,
},
TestData {
// Must start with an alphanumeric
id: "-",
expect_error: true,
},
TestData {
// Must start with an alphanumeric
id: "_",
expect_error: true,
},
TestData {
// Must start with an alphanumeric
id: " a",
expect_error: true,
},
TestData {
// Must start with an alphanumeric
id: ".a",
expect_error: true,
},
TestData {
// Must start with an alphanumeric
id: "-a",
expect_error: true,
},
TestData {
// Must start with an alphanumeric
id: "_a",
expect_error: true,
},
TestData {
// Must start with an alphanumeric
id: "..",
expect_error: true,
},
TestData {
// Too short
id: "a",
expect_error: true,
},
TestData {
// Too short
id: "z",
expect_error: true,
},
TestData {
// Too short
id: "A",
expect_error: true,
},
TestData {
// Too short
id: "Z",
expect_error: true,
},
TestData {
// Too short
id: "0",
expect_error: true,
},
TestData {
// Too short
id: "9",
expect_error: true,
},
TestData {
// Must start with an alphanumeric
id: "-1",
expect_error: true,
},
TestData {
id: "/",
expect_error: true,
},
TestData {
id: "a/",
expect_error: true,
},
TestData {
id: "a/../",
expect_error: true,
},
TestData {
id: "../a",
expect_error: true,
},
TestData {
id: "../../a",
expect_error: true,
},
TestData {
id: "../../../a",
expect_error: true,
},
TestData {
id: "foo/../bar",
expect_error: true,
},
TestData {
id: "foo bar",
expect_error: true,
},
TestData {
id: "a.",
expect_error: false,
},
TestData {
id: "a..",
expect_error: false,
},
TestData {
id: "aa",
expect_error: false,
},
TestData {
id: "aa.",
expect_error: false,
},
TestData {
id: "hello..world",
expect_error: false,
},
TestData {
id: "hello/../world",
expect_error: true,
},
TestData {
id: "aa1245124sadfasdfgasdga.",
expect_error: false,
},
TestData {
id: "aAzZ0123456789_.-",
expect_error: false,
},
TestData {
id: "abcdefghijklmnopqrstuvwxyz0123456789.-_",
expect_error: false,
},
TestData {
id: "0123456789abcdefghijklmnopqrstuvwxyz.-_",
expect_error: false,
},
TestData {
id: " abcdefghijklmnopqrstuvwxyz0123456789.-_",
expect_error: true,
},
TestData {
id: ".abcdefghijklmnopqrstuvwxyz0123456789.-_",
expect_error: true,
},
TestData {
id: "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_",
expect_error: false,
},
TestData {
id: "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ.-_",
expect_error: false,
},
TestData {
id: " ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_",
expect_error: true,
},
TestData {
id: ".ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_",
expect_error: true,
},
TestData {
id: "/a/b/c",
expect_error: true,
},
TestData {
id: "a/b/c",
expect_error: true,
},
TestData {
id: "foo/../../../etc/passwd",
expect_error: true,
},
TestData {
id: "../../../../../../etc/motd",
expect_error: true,
},
TestData {
id: "/etc/passwd",
expect_error: true,
},
];
for (i, d) in tests.iter().enumerate() {
let msg = format!("test[{}]: {:?}", i, d);
let result = verify_cid(d.id);
let msg = format!("{}, result: {:?}", msg, result);
if result.is_ok() {
assert!(!d.expect_error, msg);
} else {
assert!(d.expect_error, msg);
}
}
}
}

6
src/runtime/cli/kata-exec.go
View File

@ -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)

4
src/runtime/containerd-shim-v2/service.go
View File

@ -368,6 +368,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

80
src/runtime/containerd-shim-v2/service_test.go Normal file
View File

@ -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)
}
}
}

56
src/runtime/pkg/katatestutils/utils.go
View File

@ -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

25
src/runtime/pkg/katautils/utils.go
View File

@ -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
}

16
src/runtime/pkg/katautils/utils_test.go
View File

@ -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)
}
}
}

4
src/runtime/virtcontainers/pkg/vcmock/sandbox.go
View File

@ -255,3 +255,7 @@ func (s *Sandbox) GetAgentURL() (string, error) {
}
return "", nil
}
func (s *Sandbox) GetHypervisorPid() (int, error) {
return 0, nil
}