diff --git a/identifiers/validate.go b/identifiers/validate.go index b99881ea8..37f9b72ff 100644 --- a/identifiers/validate.go +++ b/identifiers/validate.go @@ -1,17 +1,11 @@ -// Package identifiers provides common validation for identifiers, keys and ids +// Package identifiers provides common validation for identifiers and keys // across containerd. // -// To allow such identifiers to be used across various contexts safely, the character -// set has been restricted to that defined for domains in RFC 1035, section -// 2.3.1. This will make identifiers safe for use across networks, filesystems -// and other media. +// Identifiers in containerd must be a alphanumeric, allowing limited +// underscores, dashes and dots. // -// The identifier specification departs from RFC 1035 in that it allows -// "labels" to start with number and only enforces a total length restriction -// of 76 characters. -// -// While the character set may be expanded in the future, identifiers are -// guaranteed to be safely used as filesystem path components. +// While the character set may be expanded in the future, identifiers +// are guaranteed to be safely used as filesystem path components. package identifiers import ( @@ -22,27 +16,28 @@ import ( ) const ( - maxLength = 76 - charclass = `[A-Za-z0-9]+` - label = charclass + `(:?[-]+` + charclass + `)*` + maxLength = 76 + alphanum = `[A-Za-z0-9]+` + separators = `[._-]` ) var ( - // identifierRe validates that a identifier matches valid identifiers. - // - // Rules for domains, defined in RFC 1035, section 2.3.1, are used for - // identifiers. - identifierRe = regexp.MustCompile(reAnchor(label + reGroup("[.]"+reGroup(label)) + "*")) + // identifierRe defines the pattern for valid identifiers. + identifierRe = regexp.MustCompile(reAnchor(alphanum + reGroup(separators+reGroup(alphanum)) + "*")) ) // Validate return nil if the string s is a valid identifier. // -// identifiers must be valid domain identifiers according to RFC 1035, section 2.3.1. To +// identifiers must be valid domain names according to RFC 1035, section 2.3.1. To // enforce case insensitvity, all characters must be lower case. // // In general, identifiers that pass this validation, should be safe for use as -// a domain identifier or filesystem path component. +// a domain names or filesystem path component. func Validate(s string) error { + if len(s) == 0 { + return errors.Wrapf(errdefs.ErrInvalidArgument, "identifier must not be empty") + } + if len(s) > maxLength { return errors.Wrapf(errdefs.ErrInvalidArgument, "identifier %q greater than maximum length (%d characters)", s, maxLength) } diff --git a/identifiers/validate_test.go b/identifiers/validate_test.go index 4f8f9267f..4455ff4dc 100644 --- a/identifiers/validate_test.go +++ b/identifiers/validate_test.go @@ -13,13 +13,12 @@ func TestValidIdentifiers(t *testing.T) { "Default", t.Name(), "default-default", - "default--default", "containerd.io", "foo.boo", "swarmkit.docker.io", - "zn--e9.org", // or something like it! "0912341234", "task.0.0123456789", + "underscores_are_allowed", strings.Repeat("a", maxLength), } { t.Run(input, func(t *testing.T) { @@ -32,6 +31,7 @@ func TestValidIdentifiers(t *testing.T) { func TestInvalidIdentifiers(t *testing.T) { for _, input := range []string{ + "", ".foo..foo", "foo/foo", "foo/..", @@ -39,7 +39,9 @@ func TestInvalidIdentifiers(t *testing.T) { "foo.-boo", "-foo.boo", "foo.boo-", - "foo_foo.boo_underscores", // boo-urns? + "but__only_tasteful_underscores", + "zn--e9.org", // or something like it! + "default--default", strings.Repeat("a", maxLength+1), } { diff --git a/linux/runtime.go b/linux/runtime.go index 79d8fc4ee..4a7147e6e 100644 --- a/linux/runtime.go +++ b/linux/runtime.go @@ -13,6 +13,7 @@ import ( "github.com/boltdb/bolt" "github.com/containerd/containerd/api/types" + "github.com/containerd/containerd/identifiers" client "github.com/containerd/containerd/linux/shim" shim "github.com/containerd/containerd/linux/shim/v1" "github.com/containerd/containerd/log" @@ -127,6 +128,11 @@ func (r *Runtime) Create(ctx context.Context, id string, opts runtime.CreateOpts if err != nil { return nil, err } + + if err := identifiers.Validate(id); err != nil { + return nil, errors.Wrapf(err, "invalid task id") + } + bundle, err := newBundle(filepath.Join(r.root, namespace), namespace, id, opts.Spec.Value) if err != nil { return nil, err diff --git a/linux/shim/exec.go b/linux/shim/exec.go index 65135afa3..1d23c6f4b 100644 --- a/linux/shim/exec.go +++ b/linux/shim/exec.go @@ -16,6 +16,7 @@ import ( "golang.org/x/sys/unix" "github.com/containerd/console" + "github.com/containerd/containerd/identifiers" shimapi "github.com/containerd/containerd/linux/shim/v1" "github.com/containerd/fifo" runc "github.com/containerd/go-runc" @@ -40,6 +41,10 @@ type execProcess struct { } func newExecProcess(context context.Context, path string, r *shimapi.ExecProcessRequest, parent *initProcess, id string) (process, error) { + if err := identifiers.Validate(id); err != nil { + return nil, errors.Wrapf(err, "invalid exec id") + } + e := &execProcess{ id: id, parent: parent, diff --git a/linux/shim/init.go b/linux/shim/init.go index 87aa11715..a8cc0e9e6 100644 --- a/linux/shim/init.go +++ b/linux/shim/init.go @@ -17,6 +17,7 @@ import ( "golang.org/x/sys/unix" "github.com/containerd/console" + "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/linux/runcopts" shimapi "github.com/containerd/containerd/linux/shim/v1" "github.com/containerd/containerd/log" @@ -52,6 +53,9 @@ type initProcess struct { } func newInitProcess(context context.Context, path, namespace string, r *shimapi.CreateTaskRequest) (*initProcess, error) { + if err := identifiers.Validate(r.ID); err != nil { + return nil, errors.Wrapf(err, "invalid task id") + } var options runcopts.CreateOptions if r.Options != nil { v, err := typeurl.UnmarshalAny(r.Options) diff --git a/linux/shim/service.go b/linux/shim/service.go index 45f364616..b91bfff7e 100644 --- a/linux/shim/service.go +++ b/linux/shim/service.go @@ -14,6 +14,7 @@ import ( "github.com/containerd/console" events "github.com/containerd/containerd/api/services/events/v1" "github.com/containerd/containerd/api/types/task" + "github.com/containerd/containerd/errdefs" evt "github.com/containerd/containerd/events" shimapi "github.com/containerd/containerd/linux/shim/v1" "github.com/containerd/containerd/log" @@ -77,12 +78,9 @@ type Service struct { } func (s *Service) Create(ctx context.Context, r *shimapi.CreateTaskRequest) (*shimapi.CreateTaskResponse, error) { - if r.ID == "" { - return nil, grpc.Errorf(codes.InvalidArgument, "task id cannot be empty") - } process, err := newInitProcess(ctx, s.path, s.namespace, r) if err != nil { - return nil, err + return nil, errdefs.ToGRPC(err) } s.mu.Lock() // save the main task id and bundle to the shim for additional requests diff --git a/metadata/namespaces.go b/metadata/namespaces.go index 567f2f181..4053733c5 100644 --- a/metadata/namespaces.go +++ b/metadata/namespaces.go @@ -5,7 +5,6 @@ import ( "github.com/boltdb/bolt" "github.com/containerd/containerd/errdefs" - "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/namespaces" "github.com/pkg/errors" ) @@ -24,7 +23,7 @@ func (s *namespaceStore) Create(ctx context.Context, namespace string, labels ma return err } - if err := identifiers.Validate(namespace); err != nil { + if err := namespaces.Validate(namespace); err != nil { return err } diff --git a/namespaces/context.go b/namespaces/context.go index e38bda721..6f9faf8a3 100644 --- a/namespaces/context.go +++ b/namespaces/context.go @@ -4,7 +4,6 @@ import ( "os" "github.com/containerd/containerd/errdefs" - "github.com/containerd/containerd/identifiers" "github.com/pkg/errors" "golang.org/x/net/context" ) @@ -54,7 +53,7 @@ func NamespaceRequired(ctx context.Context) (string, error) { return "", errors.Wrapf(errdefs.ErrFailedPrecondition, "namespace is required") } - if err := identifiers.Validate(namespace); err != nil { + if err := Validate(namespace); err != nil { return "", errors.Wrap(err, "namespace validation") } diff --git a/namespaces/validate.go b/namespaces/validate.go new file mode 100644 index 000000000..ff97a8cc8 --- /dev/null +++ b/namespaces/validate.go @@ -0,0 +1,67 @@ +// Package namespaces provides tools for working with namespaces across +// containerd. +// +// Namespaces collect resources such as containers and images, into a unique +// identifier space. This means that two applications can use the same +// identifiers and not conflict while using containerd. +// +// This package can be used to ensure that client and server functions +// correctly store the namespace on the context. +package namespaces + +import ( + "regexp" + + "github.com/containerd/containerd/errdefs" + "github.com/pkg/errors" +) + +const ( + maxLength = 76 + alpha = `[A-Za-z]` + alphanum = `[A-Za-z0-9]+` + label = alpha + alphanum + `(:?[-]+` + alpha + alphanum + `)*` +) + +var ( + // namespaceRe validates that a namespace matches valid identifiers. + // + // Rules for domains, defined in RFC 1035, section 2.3.1, are used for + // namespaces. + namespaceRe = regexp.MustCompile(reAnchor(label + reGroup("[.]"+reGroup(label)) + "*")) +) + +// Validate returns nil if the string s is a valid namespace. +// +// To allow such namespace identifiers to be used across various contexts +// safely, the character set has been restricted to that defined for domains in +// RFC 1035, section 2.3.1. This will make namespace identifiers safe for use +// across networks, filesystems and other media. +// +// The identifier specification departs from RFC 1035 in that it allows +// "labels" to start with number and only enforces a total length restriction +// of 76 characters. +// +// While the character set may be expanded in the future, namespace identifiers +// are guaranteed to be safely used as filesystem path components. +// +// For the most part, this doesn't need to be called directly when using the +// context-oriented functions. +func Validate(s string) error { + if len(s) > maxLength { + return errors.Wrapf(errdefs.ErrInvalidArgument, "namespace %q greater than maximum length (%d characters)", s, maxLength) + } + + if !namespaceRe.MatchString(s) { + return errors.Wrapf(errdefs.ErrInvalidArgument, "namespace %q must match %v", s, namespaceRe) + } + return nil +} + +func reGroup(s string) string { + return `(?:` + s + `)` +} + +func reAnchor(s string) string { + return `^` + s + `$` +} diff --git a/namespaces/validate_test.go b/namespaces/validate_test.go new file mode 100644 index 000000000..ad48a25a8 --- /dev/null +++ b/namespaces/validate_test.go @@ -0,0 +1,55 @@ +package namespaces + +import ( + "strings" + "testing" + + "github.com/containerd/containerd/errdefs" +) + +func TestValidNamespaces(t *testing.T) { + for _, input := range []string{ + "default", + "Default", + t.Name(), + "default-default", + "default--default", + "containerd.io", + "foo.boo", + "swarmkit.docker.io", + "zn--e9.org", // or something like it! + strings.Repeat("a", maxLength), + } { + t.Run(input, func(t *testing.T) { + if err := Validate(input); err != nil { + t.Fatalf("unexpected error: %v != nil", err) + } + }) + } +} + +func TestInvalidNamespaces(t *testing.T) { + for _, input := range []string{ + ".foo..foo", + "foo/foo", + "foo/..", + "foo..foo", + "foo.-boo", + "foo.-boo.bar", + "-foo.boo", + "foo.boo-", + "foo_foo.boo_underscores", // boo-urns? + "0912341234", + "task.0.0123456789", + strings.Repeat("a", maxLength+1), + } { + + t.Run(input, func(t *testing.T) { + if err := Validate(input); err == nil { + t.Fatal("expected invalid error") + } else if !errdefs.IsInvalidArgument(err) { + t.Fatal("error should be an invalid identifier error") + } + }) + } +}