From a3decf807bc699edcdd06454fb2c5277976678ad Mon Sep 17 00:00:00 2001 From: apostasie Date: Mon, 2 Dec 2024 20:47:28 -0800 Subject: [PATCH] Cleanup namespace validation Signed-off-by: apostasie --- cmd/nerdctl/main.go | 4 ++ pkg/containerutil/containerutil.go | 4 -- pkg/nsutil/nsutil.go | 47 ----------------------- pkg/nsutil/nsutil_test.go | 60 ------------------------------ pkg/store/filestore.go | 8 ++-- 5 files changed, 8 insertions(+), 115 deletions(-) delete mode 100644 pkg/nsutil/nsutil.go delete mode 100644 pkg/nsutil/nsutil_test.go diff --git a/cmd/nerdctl/main.go b/cmd/nerdctl/main.go index 50797e5b804..cae7dadca89 100644 --- a/cmd/nerdctl/main.go +++ b/cmd/nerdctl/main.go @@ -49,6 +49,7 @@ import ( "github.com/containerd/nerdctl/v2/pkg/errutil" "github.com/containerd/nerdctl/v2/pkg/logging" "github.com/containerd/nerdctl/v2/pkg/rootlessutil" + "github.com/containerd/nerdctl/v2/pkg/store" "github.com/containerd/nerdctl/v2/pkg/version" ) @@ -239,6 +240,9 @@ Config file ($NERDCTL_TOML): %s return fmt.Errorf("invalid cgroup-manager %q (supported values: \"systemd\", \"cgroupfs\", \"none\")", cgroupManager) } } + if err = store.ValidatePathComponent(globalOptions.Namespace); err != nil { + return err + } if appNeedsRootlessParentMain(cmd, args) { // reexec /proc/self/exe with `nsenter` into RootlessKit namespaces return rootlessutil.ParentMain(globalOptions.HostGatewayIP) diff --git a/pkg/containerutil/containerutil.go b/pkg/containerutil/containerutil.go index 8f215ecf679..4f21fa70546 100644 --- a/pkg/containerutil/containerutil.go +++ b/pkg/containerutil/containerutil.go @@ -46,7 +46,6 @@ import ( "github.com/containerd/nerdctl/v2/pkg/ipcutil" "github.com/containerd/nerdctl/v2/pkg/labels" "github.com/containerd/nerdctl/v2/pkg/labels/k8slabels" - "github.com/containerd/nerdctl/v2/pkg/nsutil" "github.com/containerd/nerdctl/v2/pkg/portutil" "github.com/containerd/nerdctl/v2/pkg/rootlessutil" "github.com/containerd/nerdctl/v2/pkg/signalutil" @@ -529,9 +528,6 @@ func Unpause(ctx context.Context, client *containerd.Client, id string) error { // ContainerStateDirPath returns the path to the Nerdctl-managed state directory for the container with the given ID. func ContainerStateDirPath(ns, dataStore, id string) (string, error) { - if err := nsutil.ValidateNamespaceName(ns); err != nil { - return "", fmt.Errorf("invalid namespace name %q for determining state dir of container %q: %s", ns, id, err) - } return filepath.Join(dataStore, "containers", ns, id), nil } diff --git a/pkg/nsutil/nsutil.go b/pkg/nsutil/nsutil.go deleted file mode 100644 index 9cde4583c87..00000000000 --- a/pkg/nsutil/nsutil.go +++ /dev/null @@ -1,47 +0,0 @@ -/* - Copyright The containerd 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 nsutil provides utilities for namespaces. -package nsutil - -import ( - "fmt" - "strings" -) - -// Ensures the provided namespace name is valid. -// Namespace names cannot be path-like strings or pre-defined aliases such as "..". -// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#path-segment-names -func ValidateNamespaceName(nsName string) error { - if nsName == "" { - return fmt.Errorf("namespace name cannot be empty") - } - - // Slash and '$' for POSIX and backslash and '%' for Windows. - pathSeparators := "/\\%$" - if strings.ContainsAny(nsName, pathSeparators) { - return fmt.Errorf("namespace name cannot contain any special characters (%q): %s", pathSeparators, nsName) - } - - specialAliases := []string{".", "..", "~"} - for _, alias := range specialAliases { - if nsName == alias { - return fmt.Errorf("namespace name cannot be special path alias %q", alias) - } - } - - return nil -} diff --git a/pkg/nsutil/nsutil_test.go b/pkg/nsutil/nsutil_test.go deleted file mode 100644 index 31d2fdffdc1..00000000000 --- a/pkg/nsutil/nsutil_test.go +++ /dev/null @@ -1,60 +0,0 @@ -/* - Copyright The containerd 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 nsutil_test - -import ( - "testing" - - "gotest.tools/v3/assert" - - "github.com/containerd/nerdctl/v2/pkg/nsutil" -) - -func TestValidateNamespaceName(t *testing.T) { - testCases := []struct { - inputs []string - errSubstr string - }{ - { - []string{"test", "test-hyphen", ".start.dot", "mid.dot", "end.dot."}, - "", - }, - { - []string{".", "..", "~"}, - "namespace name cannot be special path alias", - }, - { - []string{"$$", "a$VARiable", "a%VAR%iable", "\\.", "\\%", "\\$"}, - "namespace name cannot contain any special characters", - }, - { - []string{"/start", "mid/dle", "end/", "\\start", "mid\\dle", "end\\"}, - "namespace name cannot contain any special characters", - }, - } - - for _, tc := range testCases { - for _, input := range tc.inputs { - err := nsutil.ValidateNamespaceName(input) - if tc.errSubstr == "" { - assert.NilError(t, err) - } else { - assert.ErrorContains(t, err, tc.errSubstr) - } - } - } -} diff --git a/pkg/store/filestore.go b/pkg/store/filestore.go index ec0d98b3585..312155230fa 100644 --- a/pkg/store/filestore.go +++ b/pkg/store/filestore.go @@ -204,7 +204,7 @@ func (vs *fileStore) List(key ...string) ([]string, error) { // Unlike Get, Set and Delete, List can have zero length key for _, k := range key { - if err := validatePathComponent(k); err != nil { + if err := ValidatePathComponent(k); err != nil { return nil, err } } @@ -333,8 +333,8 @@ func (vs *fileStore) GroupSize(key ...string) (int64, error) { return size, nil } -// validatePathComponent will enforce os specific filename restrictions on a single path component -func validatePathComponent(pathComponent string) error { +// ValidatePathComponent will enforce os specific filename restrictions on a single path component +func ValidatePathComponent(pathComponent string) error { // https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits if len(pathComponent) > 255 { return errors.Join(ErrInvalidArgument, errors.New("identifiers must be stricly shorter than 256 characters")) @@ -358,7 +358,7 @@ func validateAllPathComponents(pathComponent ...string) error { } for _, key := range pathComponent { - if err := validatePathComponent(key); err != nil { + if err := ValidatePathComponent(key); err != nil { return err } }