Merge pull request #3701 from joostjager/isolate-odd-even

tlv+hop: contain odd/even logic in payload parsing

htlcswitch/hop/payload.go

@@ -29,6 +29,12 @@ const (
 	RequiredViolation
 )
 
+const (
+	// customTypeStart is the start of the custom tlv type range as defined
+	// in BOLT 01.
+	customTypeStart = 65536
+)
+
 // String returns a human-readable description of the violation as a verb.
 func (v PayloadViolation) String() string {
 	switch v {
@@ -124,28 +130,6 @@ func NewPayloadFromReader(r io.Reader) (*Payload, error) {
 
 	parsedTypes, err := tlvStream.DecodeWithParsedTypes(r)
 	if err != nil {
-		// Promote any required type failures into ErrInvalidPayload.
-		if e, required := err.(tlv.ErrUnknownRequiredType); required {
-			// If the parser returned an unknown required type
-			// failure, we'll first check that the payload is
-			// properly formed according to our known set of
-			// constraints. If an error is discovered, this
-			// overrides the required type failure.
-			nextHop := lnwire.NewShortChanIDFromInt(cid)
-			err = ValidateParsedPayloadTypes(parsedTypes, nextHop)
-			if err != nil {
-				return nil, err
-			}
-
-			// Otherwise the known constraints were applied
-			// successfully, report the invalid type failure
-			// returned by the parser.
-			return nil, ErrInvalidPayload{
-				Type:      tlv.Type(e),
-				Violation: RequiredViolation,
-				FinalHop:  nextHop == Exit,
-			}
-		}
 		return nil, err
 	}
 
@@ -157,6 +141,16 @@ func NewPayloadFromReader(r io.Reader) (*Payload, error) {
 		return nil, err
 	}
 
+	// Check for violation of the rules for mandatory fields.
+	violatingType := getMinRequiredViolation(parsedTypes)
+	if violatingType != nil {
+		return nil, ErrInvalidPayload{
+			Type:      *violatingType,
+			Violation: RequiredViolation,
+			FinalHop:  nextHop == Exit,
+		}
+	}
+
 	// If no MPP field was parsed, set the MPP field on the resulting
 	// payload to nil.
 	if _, ok := parsedTypes[record.MPPOnionType]; !ok {
@@ -239,3 +233,35 @@ func ValidateParsedPayloadTypes(parsedTypes tlv.TypeSet,
 func (h *Payload) MultiPath() *record.MPP {
 	return h.MPP
 }
+
+// getMinRequiredViolation checks for unrecognized required (even) fields in the
+// standard range and returns the lowest required type. Always returning the
+// lowest required type allows a failure message to be deterministic.
+func getMinRequiredViolation(set tlv.TypeSet) *tlv.Type {
+	var (
+		requiredViolation        bool
+		minRequiredViolationType tlv.Type
+	)
+	for t, known := range set {
+		// If a type is even but not known to us, we cannot process the
+		// payload. We are required to understand a field that we don't
+		// support.
+		//
+		// We always accept custom fields, because a higher level
+		// application may understand them.
+		if known || t%2 != 0 || t >= customTypeStart {
+			continue
+		}
+
+		if !requiredViolation || t < minRequiredViolationType {
+			minRequiredViolationType = t
+		}
+		requiredViolation = true
+	}
+
+	if requiredViolation {
+		return &minRequiredViolationType
+	}
+
+	return nil
+}

htlcswitch/hop/payload_test.go

@@ -130,6 +130,12 @@ var decodePayloadTests = []decodePayloadTest{
 			FinalHop:  false,
 		},
 	},
+	{
+		name: "required type in custom range",
+		payload: []byte{0x02, 0x00, 0x04, 0x00,
+			0xfe, 0x00, 0x01, 0x00, 0x00, 0x00,
+		},
+	},
 	{
 		name: "valid intermediate hop",
 		payload: []byte{0x02, 0x00, 0x04, 0x00, 0x06, 0x08, 0x01, 0x00,