diff --git a/identifiers/validate.go b/identifiers/validate.go new file mode 100644 index 000000000000..7fea820b1a91 --- /dev/null +++ b/identifiers/validate.go @@ -0,0 +1,58 @@ +// Package identifiers provides common validation for identifiers, keys and ids +// 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. +// +// While the character set may expand in the future, we guarantee that the +// identifiers will be safe for use as filesystem path components. +package identifiers + +import ( + "regexp" + + "github.com/pkg/errors" +) + +const ( + label = `[A-Za-z][A-Za-z0-9]+(?:[-]+[A-Za-z0-9]+)*` +) + +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)) + "*")) + + errIdentifierInvalid = errors.Errorf("invalid, must match %v", identifierRe) +) + +// IsInvalid return true if the error was due to an invalid identifer. +func IsInvalid(err error) bool { + return errors.Cause(err) == errIdentifierInvalid +} + +// 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 +// 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. +func Validate(s string) error { + if !identifierRe.MatchString(s) { + return errors.Wrapf(errIdentifierInvalid, "identifier %q", s) + } + return nil +} + +func reGroup(s string) string { + return `(?:` + s + `)` +} + +func reAnchor(s string) string { + return `^` + s + `$` +} diff --git a/identifiers/validate_test.go b/identifiers/validate_test.go new file mode 100644 index 000000000000..9b28dd4aca07 --- /dev/null +++ b/identifiers/validate_test.go @@ -0,0 +1,47 @@ +package identifiers + +import ( + "testing" +) + +func TestValidIdentifiers(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! + } { + t.Run(input, func(t *testing.T) { + if err := Validate(input); err != nil { + t.Fatalf("unexpected error: %v != nil", err) + } + }) + } +} + +func TestInvalidIdentifiers(t *testing.T) { + for _, input := range []string{ + ".foo..foo", + "foo/foo", + "foo/..", + "foo..foo", + "foo.-boo", + "-foo.boo", + "foo.boo-", + "foo_foo.boo_underscores", // boo-urns? + } { + + t.Run(input, func(t *testing.T) { + if err := Validate(input); err == nil { + t.Fatal("expected invalid error") + } else if !IsInvalid(err) { + t.Fatal("error should be an invalid identifier error") + } + }) + } +} diff --git a/metadata/containers.go b/metadata/containers.go index 6fcf82e8aeb0..836309865e94 100644 --- a/metadata/containers.go +++ b/metadata/containers.go @@ -6,6 +6,7 @@ import ( "github.com/boltdb/bolt" "github.com/containerd/containerd/containers" + "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/namespaces" "github.com/pkg/errors" ) @@ -77,6 +78,10 @@ func (s *containerStore) Create(ctx context.Context, container containers.Contai return containers.Container{}, err } + if err := identifiers.Validate(container.ID); err != nil { + return containers.Container{}, err + } + bkt, err := createContainersBucket(s.tx, namespace) if err != nil { return containers.Container{}, err diff --git a/metadata/namespaces.go b/metadata/namespaces.go index b2b429500ce6..89a5af1bc5bd 100644 --- a/metadata/namespaces.go +++ b/metadata/namespaces.go @@ -4,6 +4,7 @@ import ( "context" "github.com/boltdb/bolt" + "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/namespaces" ) @@ -21,7 +22,7 @@ func (s *namespaceStore) Create(ctx context.Context, namespace string, labels ma return err } - if err := namespaces.Validate(namespace); err != nil { + if err := identifiers.Validate(namespace); err != nil { return err } diff --git a/namespaces/context.go b/namespaces/context.go index 4fae0e991f8d..d7515b54d82c 100644 --- a/namespaces/context.go +++ b/namespaces/context.go @@ -3,6 +3,7 @@ package namespaces import ( "os" + "github.com/containerd/containerd/identifiers" "github.com/pkg/errors" "golang.org/x/net/context" ) @@ -61,8 +62,8 @@ func NamespaceRequired(ctx context.Context) (string, error) { return "", errNamespaceRequired } - if err := Validate(namespace); err != nil { - return "", err + if err := identifiers.Validate(namespace); err != nil { + return "", errors.Wrap(err, "namespace validation") } return namespace, nil diff --git a/namespaces/validate.go b/namespaces/validate.go deleted file mode 100644 index 033ed4b85ac2..000000000000 --- a/namespaces/validate.go +++ /dev/null @@ -1,52 +0,0 @@ -package namespaces - -import ( - "regexp" - - "github.com/pkg/errors" -) - -const ( - label = `[a-z][a-z0-9]+(?:[-]+[a-z0-9]+)*` -) - -func reGroup(s string) string { - return `(?:` + s + `)` -} - -func reAnchor(s string) string { - return `^` + s + `$` -} - -var ( - // namespaceRe validates that a namespace matches valid namespaces. - // - // Rules for domains, defined in RFC 1035, section 2.3.1, are used for - // namespaces. - namespaceRe = regexp.MustCompile(reAnchor(label + reGroup("[.]"+reGroup(label)) + "*")) - - errNamespaceInvalid = errors.Errorf("invalid namespace, must match %v", namespaceRe) -) - -// IsNamespacesValid return true if the error was due to an invalid namespace -// name. -func IsNamespaceInvalid(err error) bool { - return errors.Cause(err) == errNamespaceInvalid -} - -// Validate return nil if the string s is a valid namespace name. -// -// Namespaces 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, namespaces that pass this validation, should be safe for use as -// a domain name or filesystem path component. -// -// Typically, this function is used through NamespacesRequired, rather than -// directly. -func Validate(s string) error { - if !namespaceRe.MatchString(s) { - return errors.Wrapf(errNamespaceInvalid, "namespace %q", s) - } - return nil -} diff --git a/namespaces/validate_test.go b/namespaces/validate_test.go deleted file mode 100644 index 2202ca007638..000000000000 --- a/namespaces/validate_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package namespaces - -import ( - "testing" - - "github.com/pkg/errors" -) - -func TestValidNamespaces(t *testing.T) { - for _, testcase := range []struct { - input string - err error - }{ - { - input: "default", - }, - { - input: "default-default", - }, - { - input: "default--default", - }, - { - input: "containerd.io", - }, - { - input: "foo.boo", - }, - { - input: "swarmkit.docker.io", - }, - { - input: "zn--e9.org", // or something like it! - }, - { - input: ".foo..foo", - err: errNamespaceInvalid, - }, - { - input: "foo/foo", - err: errNamespaceInvalid, - }, - { - input: "foo/..", - err: errNamespaceInvalid, - }, - { - input: "foo..foo", - err: errNamespaceInvalid, - }, - { - input: "foo.-boo", - err: errNamespaceInvalid, - }, - { - input: "-foo.boo", - err: errNamespaceInvalid, - }, - { - input: "foo.boo-", - err: errNamespaceInvalid, - }, - { - input: "foo_foo.boo_underscores", // boo-urns? - err: errNamespaceInvalid, - }, - } { - t.Run(testcase.input, func(t *testing.T) { - if err := Validate(testcase.input); errors.Cause(err) != testcase.err { - t.Log(errors.Cause(err), testcase.err) - if testcase.err == nil { - t.Fatalf("unexpected error: %v != nil", err) - } else { - t.Fatalf("expected error %v to be %v", err, testcase.err) - } - } - }) - } -} diff --git a/services/containers/helpers.go b/services/containers/helpers.go index 0add18caa7e6..99960c567500 100644 --- a/services/containers/helpers.go +++ b/services/containers/helpers.go @@ -3,6 +3,7 @@ package containers import ( api "github.com/containerd/containerd/api/services/containers/v1" "github.com/containerd/containerd/containers" + "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/metadata" "github.com/containerd/containerd/namespaces" "github.com/gogo/protobuf/types" @@ -60,7 +61,7 @@ func mapGRPCError(err error, id string) error { return grpc.Errorf(codes.AlreadyExists, "container %v already exists", id) case namespaces.IsNamespaceRequired(err): return grpc.Errorf(codes.InvalidArgument, "namespace required, please set %q header", namespaces.GRPCHeader) - case namespaces.IsNamespaceInvalid(err): + case identifiers.IsInvalid(err): return grpc.Errorf(codes.InvalidArgument, err.Error()) } diff --git a/services/images/helpers.go b/services/images/helpers.go index 8e29ffebde30..575ef0468af7 100644 --- a/services/images/helpers.go +++ b/services/images/helpers.go @@ -3,6 +3,7 @@ package images import ( imagesapi "github.com/containerd/containerd/api/services/images/v1" "github.com/containerd/containerd/api/types" + "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/images" "github.com/containerd/containerd/metadata" "github.com/containerd/containerd/namespaces" @@ -85,7 +86,7 @@ func mapGRPCError(err error, id string) error { return grpc.Errorf(codes.AlreadyExists, "image %v already exists", id) case namespaces.IsNamespaceRequired(err): return grpc.Errorf(codes.InvalidArgument, "namespace required, please set %q header", namespaces.GRPCHeader) - case namespaces.IsNamespaceInvalid(err): + case identifiers.IsInvalid(err): return grpc.Errorf(codes.InvalidArgument, err.Error()) }