Reverse earlier "allow tag without 'tag:' prefix" changes.

These accidentally make the tag syntax more flexible than was intended,
which will create forward compatibility problems later. Let's go back
to the old stricter parser.

Revert "cmd/tailscale/cli: fix double tag: prefix in tailscale up"
Revert "cmd/tailscale/cli, tailcfg: allow tag without "tag:" prefix in 'tailscale up'"

This reverts commit a702921620.
This reverts commit cd07437ade.

Affects #861.

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>

cmd/tailscale/cli/up.go

@@ -182,19 +182,11 @@ func runUp(ctx context.Context, args []string) error {
 	var tags []string
 	if upArgs.advertiseTags != "" {
 		tags = strings.Split(upArgs.advertiseTags, ",")
-		for i, tag := range tags {
-			if strings.HasPrefix(tag, "tag:") {
-				// Accept fully-qualified tags (starting with
-				// "tag:"), as we do in the ACL file.
-				if err := tailcfg.CheckTag(tag); err != nil {
-					fatalf("tag: %q: %v", tag, err)
-				}
-				continue
-			}
-			if err := tailcfg.CheckTagSuffix(tag); err != nil {
-				fatalf("tag: %q: %v", tag, err)
+		for _, tag := range tags {
+			err := tailcfg.CheckTag(tag)
+			if err != nil {
+				fatalf("tag: %q: %s", tag, err)
 			}
-			tags[i] = "tag:" + tag
 		}
 	}
 

tailcfg/tailcfg.go

@@ -13,7 +13,6 @@ import (
 	"reflect"
 	"strings"
 	"time"
-	"unicode/utf8"
 
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"go4.org/mem"
@@ -211,8 +210,13 @@ func (m MachineStatus) String() string {
 	}
 }
 
-func isNum(r rune) bool   { return r >= '0' && r <= '9' }
-func isAlpha(r rune) bool { return (r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') }
+func isNum(b byte) bool {
+	return b >= '0' && b <= '9'
+}
+
+func isAlpha(b byte) bool {
+	return (b >= 'A' && b <= 'Z') || (b >= 'a' && b <= 'z')
+}
 
 // CheckTag valids whether a given string can be used as an ACL tag.
 // For now we allow only ascii alphanumeric tags, and they need to start
@@ -227,34 +231,20 @@ func CheckTag(tag string) error {
 	if !strings.HasPrefix(tag, "tag:") {
 		return errors.New("tags must start with 'tag:'")
 	}
-	suffix := tag[len("tag:"):]
-	if err := CheckTagSuffix(suffix); err != nil {
-		return fmt.Errorf("invalid tag %q: %w", tag, err)
-	}
-	return nil
-}
-
-// CheckTagSuffix checks whether tag is a valid tag suffix (the part
-// appearing after "tag:"). The error message does not reference
-// "tag:", so it's suitable for use by the "tailscale up" CLI tool
-// where the "tag:" isn't required. The returned error also does not
-// reference the tag itself, so the caller can wrap it as needed with
-// either the full or short form.
-func CheckTagSuffix(tag string) error {
+	tag = tag[4:]
 	if tag == "" {
 		return errors.New("tag names must not be empty")
 	}
-	if i := strings.IndexFunc(tag, func(r rune) bool { return r >= utf8.RuneSelf }); i != -1 {
-		return errors.New("tag names must only contain ASCII")
-	}
-	if !isAlpha(rune(tag[0])) {
-		return errors.New("tag name must start with a letter")
+	if !isAlpha(tag[0]) {
+		return errors.New("tag names must start with a letter, after 'tag:'")
 	}
-	for _, r := range tag {
-		if !isNum(r) && !isAlpha(r) && r != '-' {
+
+	for _, b := range []byte(tag) {
+		if !isNum(b) && !isAlpha(b) && b != '-' {
 			return errors.New("tag names can only contain numbers, letters, or dashes")
 		}
 	}
+
 	return nil
 }