oci: casext: extend scope of valid reference names

Previously we only supported a very limited subset of reference names
(as per the specification), but the restrictions have been expanded to
allow for any reference of the form

    refname   ::= component ("/" component)*
    component ::= alphanum (separator alphanum)*
    alphanum  ::= [A-Za-z0-9]+
    separator ::= [-._:@+] | "--"

Add support for this newly expanded set of reference names. In addition,
refactor the checks for whether a reference name is valid.

Signed-off-by: Aleksa Sarai <[email protected]>

CHANGELOG.md

@@ -5,6 +5,19 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
+### Added
+- The number of possible tags that are now valid with `umoci` subcommands has
+  increased significantly due to an expansion in the specification of the
+  format of the `ref.name` annotation. To quote the specification, this is the
+  EBNF of valid `refname` values:
+  ```
+  refname   ::= component ("/" component)*
+  component ::= alphanum (separator alphanum)*
+  alphanum  ::= [A-Za-z0-9]+
+  separator ::= [-._:@+] | "--"
+  ```
+
+### Fixed
 - `umoci unpack` now handles out-of-order regular whiteouts correctly (though
   this ordering is not recommended by the spec -- nor is it required). This is
   an extension of openSUSE/umoci#229 that was missed during review.

cmd/umoci/tag.go

@@ -48,7 +48,7 @@ the tag and "<new-tag>" is the new name of the tag.`,
 		if ctx.Args().First() == "" {
 			return errors.Errorf("new tag cannot be empty")
 		}
-		if !refRegexp.MatchString(ctx.Args().First()) {
+		if !casext.IsValidReferenceName(ctx.Args().First()) {
 			return errors.Errorf("new tag is an invalid reference")
 		}
 		ctx.App.Metadata["new-tag"] = ctx.Args().First()

cmd/umoci/utils_ux.go

@@ -19,16 +19,13 @@ package main
 
 import (
 	"fmt"
-	"regexp"
 	"strings"
 
+	"github.com/openSUSE/umoci/oci/casext"
 	"github.com/pkg/errors"
 	"github.com/urfave/cli"
 )
 
-// refRegexp defines the regexp that a given OCI tag must obey.
-var refRegexp = regexp.MustCompile(`^([A-Za-z0-9._-]+)+$`)
-
 func flattenCommands(cmds []cli.Command) []*cli.Command {
 	var flatten []*cli.Command
 	for idx, cmd := range cmds {
@@ -106,7 +103,7 @@ func uxTag(cmd cli.Command) cli.Command {
 		// Verify tag value.
 		if ctx.IsSet("tag") {
 			tag := ctx.String("tag")
-			if !refRegexp.MatchString(tag) {
+			if !casext.IsValidReferenceName(tag) {
 				return errors.Wrap(fmt.Errorf("tag contains invalid characters: '%s'", tag), "invalid --tag")
 			}
 			if tag == "" {
@@ -161,7 +158,7 @@ func uxImage(cmd cli.Command) cli.Command {
 			}
 
 			// Verify tag value.
-			if !refRegexp.MatchString(tag) {
+			if !casext.IsValidReferenceName(tag) {
 				return errors.Wrap(fmt.Errorf("tag contains invalid characters: '%s'", tag), "invalid --image")
 			}
 			if tag == "" {

oci/casext/refname.go

@@ -18,6 +18,8 @@
 package casext
 
 import (
+	"regexp"
+
 	"github.com/apex/log"
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -37,6 +39,23 @@ func isKnownMediaType(mediaType string) bool {
 		mediaType == ispec.MediaTypeImageConfig
 }
 
+// refnameRegex is a regex that only matches reference names that are valid
+// according to the OCI specification. See IsValidReferenceName for the EBNF.
+var refnameRegex = regexp.MustCompile(`^([A-Za-z0-9]+(([-._:@+]|--)[A-Za-z0-9]+)*)(/([A-Za-z0-9]+(([-._:@+]|--)[A-Za-z0-9]+)*))*$`)
+
+// IsValidReferenceName returns whether the provided annotation value for
+// "org.opencontainers.image.ref.name" is actually valid according to the
+// OCI specification. This only matches against the MUST requirement, not the
+// SHOULD requirement. The EBNF defined in the specification is:
+//
+//   refname   ::= component ("/" component)*
+//   component ::= alphanum (separator alphanum)*
+//   alphanum  ::= [A-Za-z0-9]+
+//   separator ::= [-._:@+] | "--"
+func IsValidReferenceName(refname string) bool {
+	return refnameRegex.MatchString(refname)
+}
+
 // ResolveReference will attempt to resolve all possible descriptor paths to
 // Manifests (or any unknown blobs) that match a particular reference name (if
 // descriptors are stored in non-standard blobs, Resolve will be unable to find
@@ -49,6 +68,13 @@ func isKnownMediaType(mediaType string) bool {
 // TODO: How are we meant to implement other restrictions such as the
 //       architecture and feature flags? The API will need to change.
 func (e Engine) ResolveReference(ctx context.Context, refname string) ([]DescriptorPath, error) {
+	// XXX: It should be possible to override this somehow, in case we are
+	//      dealing with an image that abuses the image specification in some
+	//      way.
+	if !IsValidReferenceName(refname) {
+		return nil, errors.Errorf("refusing to resolve invalid reference %q", refname)
+	}
+
 	index, err := e.GetIndex(ctx)
 	if err != nil {
 		return nil, errors.Wrap(err, "get top-level index")
@@ -106,6 +132,13 @@ func (e Engine) ResolveReference(ctx context.Context, refname string) ([]Descrip
 // descriptor. If there are multiple descriptors that match the refname they
 // are all replaced with the given descriptor.
 func (e Engine) UpdateReference(ctx context.Context, refname string, descriptor ispec.Descriptor) error {
+	// XXX: It should be possible to override this somehow, in case we are
+	//      dealing with an image that abuses the image specification in some
+	//      way.
+	if !IsValidReferenceName(refname) {
+		return errors.Errorf("refusing to update invalid reference %q", refname)
+	}
+
 	// Get index to modify.
 	index, err := e.GetIndex(ctx)
 	if err != nil {
@@ -142,6 +175,13 @@ func (e Engine) UpdateReference(ctx context.Context, refname string, descriptor
 // DeleteReference removes all entries in the index that match the given
 // refname.
 func (e Engine) DeleteReference(ctx context.Context, refname string) error {
+	// XXX: It should be possible to override this somehow, in case we are
+	//      dealing with an image that abuses the image specification in some
+	//      way.
+	if !IsValidReferenceName(refname) {
+		return errors.Errorf("refusing to delete invalid reference %q", refname)
+	}
+
 	// Get index to modify.
 	index, err := e.GetIndex(ctx)
 	if err != nil {

oci/casext/refname_test.go

@@ -0,0 +1,65 @@
+/*
+ * umoci: Umoci Modifies Open Containers' Images
+ * Copyright (C) 2016, 2017, 2018 SUSE LLC.
+ *
+ * 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 casext
+
+import (
+	"testing"
+)
+
+func TestValidateRefname(t *testing.T) {
+	for _, test := range []struct {
+		refname string
+		valid   bool
+	}{
+		// No characters.
+		{"", false},
+		// Component "/" without next component.
+		{"somepath392/", false},
+		// Duplicate "/".
+		{"some//test/hello", false},
+		{"some/oth3r//teST", false},
+		// Separator without additional alphanum.
+		{"deadb33fc4f3+123888+", false},
+		// More than one separator.
+		{"anot++her", false},
+		{"dead-.meme/cafe", false},
+		// Leading separator or "/".
+		{"/a/b/c123", false},
+		{"--21js8CAS", false},
+		{".AZ94n18s", false},
+		{"@2318s88", false},
+		{":29a.158/2131--91ab", false},
+		// Plain components.
+		{"a", true},
+		{"latest", true},
+		{"42.03.19", true},
+		{"v1.3.1+dev", true},
+		{"aBC1958NaK284IT9Q0kX82jnMnis8201j", true},
+		{"Aa0Bb1Cc2-Dd3Ee4Ff5.Gg6Hh7Ii8:Jj9KkLl@MmNnOo+QqRrSs--TtUuVv.WwXxYy_Zz", true},
+		{"A--2.C+9@e_3", true},
+		// Multiple components.
+		{"Aa0-Bb1/Cc2-Dd3.Ee4Ff5/Gg6Hh7Ii8:Jj/9KkLl@Mm/NnOo+QqRrS/s--TtUu/Vv.WwXxYy_Z/z", true},
+		{"A/1--2.C+9@e_4/3", true},
+		{"etc/passwd/123", true},
+	} {
+		valid := IsValidReferenceName(test.refname)
+		if valid != test.valid {
+			t.Errorf("incorrectly determined validity of refname '%s': expected %v got %v", test.refname, test.valid, valid)
+		}
+	}
+}