multi: update payload validation to account for blinded routes

htlcswitch/hop/payload.go

@@ -256,33 +256,64 @@ func ValidateParsedPayloadTypes(parsedTypes tlv.TypeMap,
 	_, hasNextHop := parsedTypes[record.NextHopOnionType]
 	_, hasMPP := parsedTypes[record.MPPOnionType]
 	_, hasAMP := parsedTypes[record.AMPOnionType]
+	_, hasEncryptedData := parsedTypes[record.EncryptedDataOnionType]
+
+	// All cleartext hops (including final hop) and the final hop in a
+	// blinded path require the forwading amount and expiry TLVs to be set.
+	needFwdInfo := isFinalHop || !hasEncryptedData
+
+	// No blinded hops should have a next hop specified, and only the final
+	// hop in a cleartext route should exclude it.
+	needNextHop := !(hasEncryptedData || isFinalHop)
 
 	switch {
-
+	// Hops that need forwarding info must include an amount to forward.
-	// All hops must include an amount to forward.
+	case needFwdInfo && !hasAmt:
-	case !hasAmt:
 		return ErrInvalidPayload{
 			Type:      record.AmtOnionType,
 			Violation: OmittedViolation,
 			FinalHop:  isFinalHop,
 		}
 
-	// All hops must include a cltv expiry.
+	// Hops that need forwarding info must include a cltv expiry.
-	case !hasLockTime:
+	case needFwdInfo && !hasLockTime:
 		return ErrInvalidPayload{
 			Type:      record.LockTimeOnionType,
 			Violation: OmittedViolation,
 			FinalHop:  isFinalHop,
 		}
 
-	// The exit hop should omit the next hop id, otherwise the sender must
+	// Hops that don't need forwarding info shouldn't have an amount TLV.
-	// have included a record, so we don't need to test for its
+	case !needFwdInfo && hasAmt:
-	// inclusion at intermediate hops directly.
+		return ErrInvalidPayload{
-	case isFinalHop && hasNextHop:
+			Type:      record.AmtOnionType,
+			Violation: IncludedViolation,
+			FinalHop:  isFinalHop,
+		}
+
+	// Hops that don't need forwarding info shouldn't have a cltv TLV.
+	case !needFwdInfo && hasLockTime:
+		return ErrInvalidPayload{
+			Type:      record.LockTimeOnionType,
+			Violation: IncludedViolation,
+			FinalHop:  isFinalHop,
+		}
+
+	// The exit hop and all blinded hops should omit the next hop id.
+	case !needNextHop && hasNextHop:
 		return ErrInvalidPayload{
 			Type:      record.NextHopOnionType,
 			Violation: IncludedViolation,
-			FinalHop:  true,
+			FinalHop:  isFinalHop,
+		}
+
+	// Require that the next hop is set for intermediate hops in regular
+	// routes.
+	case needNextHop && !hasNextHop:
+		return ErrInvalidPayload{
+			Type:      record.NextHopOnionType,
+			Violation: OmittedViolation,
+			FinalHop:  isFinalHop,
 		}
 
 	// Intermediate nodes should never receive MPP fields.

htlcswitch/hop/payload_test.go

@@ -254,6 +254,21 @@ var decodePayloadTests = []decodePayloadTest{
 			FinalHop:  false,
 		},
 	},
+	{
+		name:       "intermediate hop no next channel",
+		isFinalHop: false,
+		payload: []byte{
+			// amount
+			0x02, 0x00,
+			// cltv
+			0x04, 0x00,
+		},
+		expErr: hop.ErrInvalidPayload{
+			Type:      record.NextHopOnionType,
+			Violation: hop.OmittedViolation,
+			FinalHop:  false,
+		},
+	},
 	{
 		name:       "intermediate hop with encrypted data",
 		isFinalHop: false,
@@ -348,6 +363,81 @@ var decodePayloadTests = []decodePayloadTest{
 		},
 		shouldHaveTotalAmt: true,
 	},
+	{
+		name:       "final blinded hop with total amount",
+		isFinalHop: true,
+		payload: []byte{
+			// amount
+			0x02, 0x00,
+			// cltv
+			0x04, 0x00,
+			// encrypted data
+			0x0a, 0x03, 0x03, 0x02, 0x01,
+		},
+		shouldHaveEncData: true,
+	},
+	{
+		name:       "final blinded missing amt",
+		isFinalHop: true,
+		payload: []byte{
+			// cltv
+			0x04, 0x00,
+			// encrypted data
+			0x0a, 0x03, 0x03, 0x02, 0x01,
+		},
+		shouldHaveEncData: true,
+		expErr: hop.ErrInvalidPayload{
+			Type:      record.AmtOnionType,
+			Violation: hop.OmittedViolation,
+			FinalHop:  true,
+		},
+	},
+	{
+		name:       "final blinded missing cltv",
+		isFinalHop: true,
+		payload: []byte{
+			// amount
+			0x02, 0x00,
+			// encrypted data
+			0x0a, 0x03, 0x03, 0x02, 0x01,
+		},
+		shouldHaveEncData: true,
+		expErr: hop.ErrInvalidPayload{
+			Type:      record.LockTimeOnionType,
+			Violation: hop.OmittedViolation,
+			FinalHop:  true,
+		},
+	},
+	{
+		name:       "intermediate blinded has amount",
+		isFinalHop: false,
+		payload: []byte{
+			// amount
+			0x02, 0x00,
+			// encrypted data
+			0x0a, 0x03, 0x03, 0x02, 0x01,
+		},
+		expErr: hop.ErrInvalidPayload{
+			Type:      record.AmtOnionType,
+			Violation: hop.IncludedViolation,
+			FinalHop:  false,
+		},
+	},
+	{
+		name:       "intermediate blinded has expiry",
+		isFinalHop: false,
+		payload: []byte{
+			// cltv
+			0x04, 0x00,
+			// encrypted data
+			0x0a, 0x03, 0x03, 0x02, 0x01,
+		},
+		expErr: hop.ErrInvalidPayload{
+			Type:      record.LockTimeOnionType,
+			Violation: hop.IncludedViolation,
+			FinalHop:  false,
+		},
+	},
 }
 
 // TestDecodeHopPayloadRecordValidation asserts that parsing the payloads in the

routing/route/route.go

@@ -42,9 +42,9 @@ var (
 	// ErrMissingField is returned if a required TLV is missing.
 	ErrMissingField = errors.New("required tlv missing")
 
-	// ErrIncorrectField is returned if a tlv field is included when it
+	// ErrUnexpectedField is returned if a tlv field is included when it
 	// should not be.
-	ErrIncorrectField = errors.New("incorrect tlv included")
+	ErrUnexpectedField = errors.New("unexpected tlv included")
 )
 
 // Vertex is a simple alias for the serialization of a compressed Bitcoin
@@ -337,7 +337,7 @@ func optionalBlindedField(isZero, blindedHop, finalHop bool) error {
 
 	// In an intermediate hop in a blinded route and the field is not zero.
 	case !finalHop && !isZero:
-		return ErrIncorrectField
+		return ErrUnexpectedField
 	}
 
 	return nil
@@ -351,7 +351,7 @@ func validateNextChanID(nextChanIDIsSet, isBlinded, finalHop bool) error {
 	switch {
 	// Hops in a blinded route should not have a next channel ID set.
 	case isBlinded && nextChanIDIsSet:
-		return ErrIncorrectField
+		return ErrUnexpectedField
 
 	// Otherwise, blinded hops are allowed to have a zero value.
 	case isBlinded:
@@ -359,7 +359,7 @@ func validateNextChanID(nextChanIDIsSet, isBlinded, finalHop bool) error {
 
 	// The final hop in a regular route is expected to have a zero value.
 	case finalHop && nextChanIDIsSet:
-		return ErrIncorrectField
+		return ErrUnexpectedField
 
 	// Intermediate hops in regular routes require non-zero value.
 	case !finalHop && !nextChanIDIsSet:

routing/route/route_test.go

@@ -160,18 +160,112 @@ func TestAMPHop(t *testing.T) {
 	}
 }
 
-// TestNoForwardingParams tests packing of a hop payload without an amount or
+// TestBlindedHops tests packing of a hop payload for various types of hops in
-// expiry height.
+// a blinded route.
-func TestNoForwardingParams(t *testing.T) {
+func TestBlindedHops(t *testing.T) {
 	t.Parallel()
 
-	hop := Hop{
+	tests := []struct {
-		EncryptedData: []byte{1, 2, 3},
+		name        string
+		hop         Hop
+		nextChannel uint64
+		isFinal     bool
+		err         error
+	}{
+		{
+			name: "introduction point with next channel",
+			hop: Hop{
+				EncryptedData: []byte{1, 2, 3},
+				BlindingPoint: testPubKey,
+			},
+			nextChannel: 1,
+			isFinal:     false,
+			err:         ErrUnexpectedField,
+		},
+		{
+			name: "final node with next channel",
+			hop: Hop{
+				EncryptedData:    []byte{1, 2, 3},
+				AmtToForward:     150,
+				OutgoingTimeLock: 26,
+			},
+			nextChannel: 1,
+			isFinal:     true,
+			err:         ErrUnexpectedField,
+		},
+		{
+			name: "valid introduction point",
+			hop: Hop{
+				EncryptedData: []byte{1, 2, 3},
+				BlindingPoint: testPubKey,
+			},
+			nextChannel: 0,
+			isFinal:     false,
+		},
+		{
+			name: "valid intermediate blinding",
+			hop: Hop{
+				EncryptedData: []byte{1, 2, 3},
+			},
+			nextChannel: 0,
+			isFinal:     false,
+		},
+		{
+			name: "final blinded missing amount",
+			hop: Hop{
+				EncryptedData: []byte{1, 2, 3},
+			},
+			nextChannel: 0,
+			isFinal:     true,
+			err:         ErrMissingField,
+		},
+		{
+			name: "final blinded expiry missing",
+			hop: Hop{
+				EncryptedData: []byte{1, 2, 3},
+				AmtToForward:  100,
+			},
+			nextChannel: 0,
+			isFinal:     true,
+			err:         ErrMissingField,
+		},
+		{
+			name: "valid final blinded",
+			hop: Hop{
+				EncryptedData:    []byte{1, 2, 3},
+				AmtToForward:     100,
+				OutgoingTimeLock: 52,
+			},
+			nextChannel: 0,
+			isFinal:     true,
+		},
+		{
+			// The introduction node can also be the final hop.
+			name: "valid final intro blinded",
+			hop: Hop{
+				EncryptedData:    []byte{1, 2, 3},
+				BlindingPoint:    testPubKey,
+				AmtToForward:     100,
+				OutgoingTimeLock: 52,
+			},
+			nextChannel: 0,
+			isFinal:     true,
+		},
 	}
 
-	var b bytes.Buffer
+	for _, testCase := range tests {
-	err := hop.PackHopPayload(&b, 0, false)
+		testCase := testCase
-	require.NoError(t, err)
+
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+
+			var b bytes.Buffer
+			err := testCase.hop.PackHopPayload(
+				&b, testCase.nextChannel, testCase.isFinal,
+			)
+			require.ErrorIs(t, err, testCase.err)
+		})
+	}
 }
 
 // TestPayloadSize tests the payload size calculation that is provided by Hop