Validate NodeNames, Address and Tags fields

cmd/serf/command/agent/agent.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
+	"regexp"
 	"strings"
 	"sync"
 
@@ -85,6 +86,18 @@ func Create(agentConf *Config, conf *serf.Config, logOutput io.Writer) (*Agent,
 		}
 	}
 
+	if agentConf.ValidateNodeNames {
+		var InvalidNameRe = regexp.MustCompile(`[^A-Za-z0-9\\-]+`)
+		if InvalidNameRe.MatchString(agentConf.NodeName) {
+			return nil, fmt.Errorf("NodeName contains invalid characters %v , Valid characters include "+
+				"all alpha-numerics and dashes.", agentConf.NodeName)
+		}
+		if len(agentConf.NodeName) > serf.MaxNodeNameLength {
+			return nil, fmt.Errorf("NodeName is %v characters. "+
+				"Valid length is between 1 and 128 characters", len(agentConf.NodeName))
+		}
+	}
+
 	return agent, nil
 }
 

cmd/serf/command/agent/agent_test.go

@@ -318,3 +318,30 @@ func TestAgentKeyringFile_NoKeys(t *testing.T) {
 		t.Fatalf("bad: %s", err)
 	}
 }
+func TestAgentCreate_ValidateNodeName(t *testing.T) {
+	type test struct {
+		nodename string
+		want     string
+	}
+
+	tests := []test{
+		{
+			nodename: "!BadChars-*",
+			want:     "invalid characters",
+		},
+		{
+			nodename: "thisisonehundredandtwentyeightcharacterslongnodenametestaswehavetotesteachcaseeventheoonesonehundredandtwentyeightcharacterslong1",
+			want:     "Valid length",
+		},
+	}
+	for _, tc := range tests {
+		agentConfig := DefaultConfig()
+		agentConfig.NodeName = tc.nodename
+		agentConfig.ValidateNodeNames = true
+		_, err := Create(agentConfig, serf.DefaultConfig(), nil)
+		if !strings.Contains(err.Error(), tc.want) {
+			t.Fatalf("expected: %v, got: %v", tc.want, err)
+		}
+	}
+
+}

cmd/serf/command/agent/config.go

@@ -8,6 +8,7 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"regexp"
 	"sort"
 	"strings"
 	"time"
@@ -227,6 +228,10 @@ type Config struct {
 	// 5 seconds.
 	BroadcastTimeoutRaw string        `mapstructure:"broadcast_timeout"`
 	BroadcastTimeout    time.Duration `mapstructure:"-"`
+
+	//ValidateNodeNames specifies whether or not nodenames should
+	// be alphanumeric and within 128 characters
+	ValidateNodeNames bool `mapstructure:"validate_node_names"`
 }
 
 // BindAddrParts returns the parts of the BindAddr that should be
@@ -344,6 +349,21 @@ func DecodeConfig(r io.Reader) (*Config, error) {
 		result.BroadcastTimeout = dur
 	}
 
+	if result.NodeName != "" && result.ValidateNodeNames {
+		var InvalidNameRe = regexp.MustCompile(`[^A-Za-z0-9\\-]+`)
+		if InvalidNameRe.MatchString(result.NodeName) {
+			err = fmt.Errorf("NodeName contains invalid characters %v , Valid characters include "+
+				"all alpha-numerics and dashes.", result.NodeName)
+			return nil, err
+		}
+
+		if len(result.NodeName) > serf.MaxNodeNameLength {
+			err = fmt.Errorf("NodeName is %v characters. "+
+				"Valid length is between 1 and 128 characters", len(result.NodeName))
+			return nil, err
+		}
+	}
+
 	return &result, nil
 }
 
@@ -363,7 +383,9 @@ func containsKey(keys []string, key string) bool {
 func MergeConfig(a, b *Config) *Config {
 	var result Config = *a
 
-	// Copy the strings if they're set
+	//TODO(schristoff): do i need to check nodename
+	//validatity here?
+
 	if b.NodeName != "" {
 		result.NodeName = b.NodeName
 	}

cmd/serf/command/agent/config_test.go

@@ -585,3 +585,16 @@ func TestReadConfigPaths_dir(t *testing.T) {
 		t.Fatalf("bad: %#v", config)
 	}
 }
+
+func TestBadNodeName(t *testing.T) {
+	input := `{"node_name": "9b07b1d05b25d78a3f55d0ccde714f02
+	96230786636dd72ded4c731746517331
+	394b395a0524b495d40fdd20ad9e536c
+	d15c8a7872ca8617226f02c175dfc0ff
+	dff8012a7c25e608e4775107fd2a3aeb"}`
+	_, err := DecodeConfig(bytes.NewReader([]byte(input)))
+	if err == nil {
+		t.Fatalf("should have err")
+	}
+
+}

serf/config.go

@@ -9,6 +9,8 @@ import (
 	"github.com/hashicorp/memberlist"
 )
 
+const MaxNodeNameLength int = 128
+
 // ProtocolVersionMap is the mapping of Serf delegate protocol versions
 // to memberlist protocol versions. We mask the memberlist protocols using
 // our own protocol version.
@@ -253,6 +255,10 @@ type Config struct {
 	//
 	// WARNING: this should ONLY be used in tests
 	messageDropper func(typ messageType) bool
+
+	//ValidateNodeNames specifies whether or not nodenames should
+	// be alphanumeric and within 128 characters
+	ValidateNodeNames bool
 }
 
 // Init allocates the subdata structures
@@ -298,6 +304,7 @@ func DefaultConfig() *Config {
 		QuerySizeLimit:               1024,
 		EnableNameConflictResolution: true,
 		DisableCoordinates:           false,
+		ValidateNodeNames:            true,
 		UserEventSizeLimit:           512,
 	}
 }

serf/merge_delegate.go

@@ -1,7 +1,9 @@
 package serf
 
 import (
+	"fmt"
 	"net"
+	"regexp"
 
 	"github.com/hashicorp/memberlist"
 )
@@ -17,22 +19,31 @@ type mergeDelegate struct {
 func (m *mergeDelegate) NotifyMerge(nodes []*memberlist.Node) error {
 	members := make([]*Member, len(nodes))
 	for idx, n := range nodes {
-		members[idx] = m.nodeToMember(n)
+		var err error
+		members[idx], err = m.nodeToMember(n)
+		if err != nil {
+			return err
+		}
 	}
 	return m.serf.config.Merge.NotifyMerge(members)
 }
 
 func (m *mergeDelegate) NotifyAlive(peer *memberlist.Node) error {
-	member := m.nodeToMember(peer)
+	member, err := m.nodeToMember(peer)
+	if err != nil {
+		return err
+	}
 	return m.serf.config.Merge.NotifyMerge([]*Member{member})
 }
 
-func (m *mergeDelegate) nodeToMember(n *memberlist.Node) *Member {
+func (m *mergeDelegate) nodeToMember(n *memberlist.Node) (*Member, error) {
 	status := StatusNone
 	if n.State == memberlist.StateLeft {
 		status = StatusLeft
 	}
-
+	if err := m.validiateMemberInfo(n); err != nil {
+		return nil, err
+	}
 	return &Member{
 		Name:        n.Name,
 		Addr:        net.IP(n.Addr),
@@ -45,5 +56,31 @@ func (m *mergeDelegate) nodeToMember(n *memberlist.Node) *Member {
 		DelegateMin: n.DMin,
 		DelegateMax: n.DMax,
 		DelegateCur: n.DCur,
+	}, nil
+}
+
+// validateMemberInfo checks that the data we are sending is valid
+func (m *mergeDelegate) validiateMemberInfo(n *memberlist.Node) error {
+	var InvalidNameRe = regexp.MustCompile(`[^A-Za-z0-9\\-]+`)
+	var InvalidIPv4Re = regexp.MustCompile(`([0-9]{1,3}\.){3}[0-9]{1,3}:[0-9]+`)
+	var InvalidIPv6Re = regexp.MustCompile(`([0-9a-f]{1,4}:){7}[0-9a-f]{1,4}`)
+	if m.serf.config.ValidateNodeNames {
+		if len(n.Name) > 128 {
+			return fmt.Errorf("NodeName length is %v characters. Valid length is between "+
+				"1 and 128 characters.", len(n.Name))
+		}
+		if InvalidNameRe.MatchString(n.Name) {
+			return fmt.Errorf("Nodename contains invalid characters %v , Valid characters include "+
+				"all alpha-numerics and dashes", n.Name)
+		}
+	}
+
+	if InvalidIPv4Re.MatchString(string(n.Addr)) || InvalidIPv6Re.MatchString(string(n.Addr)) {
+		return fmt.Errorf("Address is %v . Must be a valid representation of an IP address. ", n.Addr)
+	}
+	if len(n.Meta) > memberlist.MetaMaxSize {
+		return fmt.Errorf("Encoded length of tags exceeds limit of %d bytes",
+			memberlist.MetaMaxSize)
 	}
+	return nil
 }