Merge pull request #2412 from Roasbeef/node-alias-validation

lnwire+peer: fully validate node aliases on the wire, don't d/c due to invalid aliases

lnwire/lnwire.go

@@ -76,6 +76,11 @@ func (a addressType) AddrLen() uint16 {
 // serialization.
 func WriteElement(w io.Writer, element interface{}) error {
 	switch e := element.(type) {
+	case NodeAlias:
+		if _, err := w.Write(e[:]); err != nil {
+			return err
+		}
+
 	case ShortChanIDEncoding:
 		var b [1]byte
 		b[0] = uint8(e)
@@ -429,6 +434,18 @@ func WriteElements(w io.Writer, elements ...interface{}) error {
 func ReadElement(r io.Reader, element interface{}) error {
 	var err error
 	switch e := element.(type) {
+	case *NodeAlias:
+		var a [32]byte
+		if _, err := io.ReadFull(r, a[:]); err != nil {
+			return err
+		}
+
+		alias, err := NewNodeAlias(string(a[:]))
+		if err != nil {
+			return err
+		}
+
+		*e = alias
 	case *ShortChanIDEncoding:
 		var b [1]uint8
 		if _, err := r.Read(b[:]); err != nil {

lnwire/lnwire_test.go

@@ -41,6 +41,17 @@ var (
 	_, _ = testSig.S.SetString("18801056069249825825291287104931333862866033135609736119018462340006816851118", 10)
 )
 
+const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
+
+func randAlias(r *rand.Rand) NodeAlias {
+	var a NodeAlias
+	for i := range a {
+		a[i] = letterBytes[r.Intn(len(letterBytes))]
+	}
+
+	return a
+}
+
 func randPubKey() (*btcec.PublicKey, error) {
 	priv, err := btcec.NewPrivateKey(btcec.S256())
 	if err != nil {
@@ -551,17 +562,11 @@ func TestLightningWireProtocol(t *testing.T) {
 			v[0] = reflect.ValueOf(req)
 		},
 		MsgNodeAnnouncement: func(v []reflect.Value, r *rand.Rand) {
-			var a [32]byte
-			if _, err := r.Read(a[:]); err != nil {
-				t.Fatalf("unable to generate alias: %v", err)
-				return
-			}
-
 			var err error
 			req := NodeAnnouncement{
 				Features:  randRawFeatureVector(r),
 				Timestamp: uint32(r.Int31()),
-				Alias:     a,
+				Alias:     randAlias(r),
 				RGBColor: color.RGBA{
 					R: uint8(r.Int31()),
 					G: uint8(r.Int31()),

lnwire/node_announcement.go

@@ -28,6 +28,17 @@ func (e ErrUnknownAddrType) Error() string {
 	return fmt.Sprintf("unknown address type: %v", e.addrType)
 }
 
+// ErrInvalidNodeAlias is an error returned if a node alias we parse on the
+// wire is invalid, as in it has non UTF-8 characters.
+type ErrInvalidNodeAlias struct{}
+
+// Error returns a human readable string describing the error.
+//
+// NOTE: implements the error interface.
+func (e ErrInvalidNodeAlias) Error() string {
+	return "node alias has non-utf8 characters"
+}
+
 // NodeAlias a hex encoded UTF-8 string that may be displayed as an alternative
 // to the node's ID. Notice that aliases are not unique and may be freely
 // chosen by the node operators.
@@ -39,11 +50,12 @@ func NewNodeAlias(s string) (NodeAlias, error) {
 	var n NodeAlias
 
 	if len(s) > 32 {
-		return n, fmt.Errorf("alias too large: max is %v, got %v", 32, len(s))
+		return n, fmt.Errorf("alias too large: max is %v, got %v", 32,
+			len(s))
 	}
 
 	if !utf8.ValidString(s) {
-		return n, fmt.Errorf("invalid utf8 string")
+		return n, &ErrInvalidNodeAlias{}
 	}
 
 	copy(n[:], []byte(s))
@@ -117,7 +129,7 @@ func (a *NodeAnnouncement) Decode(r io.Reader, pver uint32) error {
 		&a.Timestamp,
 		&a.NodeID,
 		&a.RGBColor,
-		a.Alias[:],
+		&a.Alias,
 		&a.Addresses,
 	)
 	if err != nil {
@@ -149,7 +161,7 @@ func (a *NodeAnnouncement) Encode(w io.Writer, pver uint32) error {
 		a.Timestamp,
 		a.NodeID,
 		a.RGBColor,
-		a.Alias[:],
+		a.Alias,
 		a.Addresses,
 		a.ExtraOpaqueData,
 	)

lnwire/node_announcement_test.go

@@ -0,0 +1,42 @@
+package lnwire
+
+import "testing"
+
+// TestNodeAliasValidation tests that the NewNodeAlias method will only accept
+// valid node announcements.
+func TestNodeAliasValidation(t *testing.T) {
+	t.Parallel()
+
+	var testCases = []struct {
+		alias string
+		valid bool
+	}{
+		// UTF-8 alias with valid length.
+		{
+			alias: "meruem",
+			valid: true,
+		},
+
+		// UTF-8 alias with invalid length.
+		{
+			alias: "p3kysxqr23swl33m6h5grmzddgw5nsgkky3g52zc6frpwz",
+			valid: false,
+		},
+
+		// String with non UTF-8 characters.
+		{
+			alias: "\xE0\x80\x80",
+			valid: false,
+		},
+	}
+	for i, testCase := range testCases {
+		_, err := NewNodeAlias(testCase.alias)
+		switch {
+		case err != nil && testCase.valid:
+			t.Fatalf("#%v: alias should have been invalid", i)
+
+		case err == nil && !testCase.valid:
+			t.Fatalf("#%v: invalid alias was missed", i)
+		}
+	}
+}

peer.go

@@ -996,6 +996,13 @@ out:
 				idleTimer.Reset(idleTimeout)
 				continue
 
+			// If the NodeAnnouncement has an invalid alias, then
+			// we'll log that error above and continue so we can
+			// continue to read messges from the peer.
+			case *lnwire.ErrInvalidNodeAlias:
+				idleTimer.Reset(idleTimeout)
+				continue
+
 			// If the error we encountered wasn't just a message we
 			// didn't recognize, then we'll stop all processing s
 			// this is a fatal error.