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>
This commit is contained in:
Avery Pennarun 2020-11-10 22:26:23 -05:00
parent d192bd0f86
commit f99f6608ff
2 changed files with 18 additions and 36 deletions

View File

@ -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
for _, tag := range tags {
err := tailcfg.CheckTag(tag)
if err != nil {
fatalf("tag: %q: %s", tag, err)
}
if err := tailcfg.CheckTagSuffix(tag); err != nil {
fatalf("tag: %q: %v", tag, err)
}
tags[i] = "tag:" + tag
}
}

View File

@ -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 validates tag for use 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(tag[0]) {
return errors.New("tag names must start with a letter, after 'tag:'")
}
if !isAlpha(rune(tag[0])) {
return errors.New("tag name must start with a letter")
}
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
}