Merge pull request #45 from joostjager/payload-length-check

Add payload length check

Author: Roasbeef

path.go

@@ -249,7 +249,7 @@ func (hp *HopPayload) Decode(r io.Reader) error {
 	case 0x00:
 		// Our size is just the payload, without the HMAC. This means
 		// that this is the legacy payload type.
-		payloadSize = HopDataSize - HMACSize
+		payloadSize = LegacyHopDataSize - HMACSize
 		hp.Type = PayloadLegacy
 
 	default:
@@ -304,11 +304,15 @@ func (hp *HopPayload) HopData() (*HopData, error) {
 	return &hd, nil
 }
 
-// NumMaxHops is the maximum path length. This should be set to an estimate of
-// the upper limit of the diameter of the node graph.
-//
-// TODO(roasbeef): adjust due to var-payloads?
-const NumMaxHops = 20
+// NumMaxHops is the maximum path length. There is a maximum of 1300 bytes in
+// the routing info block. Legacy hop payloads are always 65 bytes, while tlv
+// payloads are at least 47 bytes (tlvlen 1, amt 2, timelock 2, nextchan 10,
+// hmac 32) for the intermediate hops and 37 bytes (tlvlen 1, amt 2, timelock 2,
+// hmac 32) for the exit hop. The maximum path length can therefore only be
+// reached by using tlv payloads only. With that, the maximum number of
+// intermediate hops is: Floor((1300 - 37) / 47) = 26. Including the exit hop,
+// the maximum path length is 27 hops.
+const NumMaxHops = 27
 
 // PaymentPath represents a series of hops within the Lightning Network
 // starting at a sender and terminating at a receiver. Each hop contains a set

sphinx.go

@@ -5,6 +5,7 @@ import (
 	"crypto/ecdsa"
 	"crypto/hmac"
 	"crypto/sha256"
+	"fmt"
 	"io"
 	"math/big"
 
@@ -36,29 +37,26 @@ const (
 	// utilize this space to pack in the unrolled bytes.
 	NumPaddingBytes = 12
 
-	// HopDataSize is the fixed size of hop_data. BOLT 04 currently
+	// LegacyHopDataSize is the fixed size of hop_data. BOLT 04 currently
 	// specifies this to be 1 byte realm, 8 byte channel_id, 8 byte amount
-	// to forward, 4 byte outgoing CLTV value, 12 bytes padding and 32
-	// bytes HMAC for a total of 65 bytes per hop.
-	HopDataSize = (RealmByteSize + AddressSize + AmtForwardSize +
+	// to forward, 4 byte outgoing CLTV value, 12 bytes padding and 32 bytes
+	// HMAC for a total of 65 bytes per hop.
+	LegacyHopDataSize = (RealmByteSize + AddressSize + AmtForwardSize +
 		OutgoingCLTVSize + NumPaddingBytes + HMACSize)
 
-	// MaxPayloadSize is the maximum size a payload for a single hop can
-	// be. This is the worst case scenario of a single hop, consuming all
-	// 20 frames. We need to know this in order to generate a sufficiently
-	// long stream of pseudo-random bytes when encrypting/decrypting the
-	// payload.
-	MaxPayloadSize = NumMaxHops * HopDataSize
-
-	// sharedSecretSize is the size in bytes of the shared secrets.
-	sharedSecretSize = 32
+	// MaxPayloadSize is the maximum size a payload for a single hop can be.
+	// This is the worst case scenario of a single hop, consuming all
+	// available space. We need to know this in order to generate a
+	// sufficiently long stream of pseudo-random bytes when
+	// encrypting/decrypting the payload.
+	MaxPayloadSize = routingInfoSize
 
 	// routingInfoSize is the fixed size of the the routing info. This
 	// consists of a addressSize byte address and a HMACSize byte HMAC for
 	// each hop of the route, the first pair in cleartext and the following
-	// pairs increasingly obfuscated. In case fewer than numMaxHops are
-	// used, then the remainder is padded with null-bytes, also obfuscated.
-	routingInfoSize = NumMaxHops * HopDataSize
+	// pairs increasingly obfuscated. If not all space is used up, the
+	// remainder is padded with null-bytes, also obfuscated.
+	routingInfoSize = 1300
 
 	// numStreamBytes is the number of bytes produced by our CSPRG for the
 	// key stream implementing our stream cipher to encrypt/decrypt the mix
@@ -76,6 +74,11 @@ const (
 	baseVersion = 0
 )
 
+var (
+	ErrMaxRoutingInfoSizeExceeded = fmt.Errorf(
+		"max routing info size of %v bytes exceeded", routingInfoSize)
+)
+
 // OnionPacket is the onion wrapped hop-to-hop routing information necessary to
 // propagate a message through the mix-net without intermediate nodes having
 // knowledge of their position within the route, the source, the destination,
@@ -189,6 +192,11 @@ func generateSharedSecrets(paymentPath []*btcec.PublicKey,
 func NewOnionPacket(paymentPath *PaymentPath, sessionKey *btcec.PrivateKey,
 	assocData []byte) (*OnionPacket, error) {
 
+	// Check whether total payload size doesn't exceed the hard maximum.
+	if paymentPath.TotalPayloadSize() > routingInfoSize {
+		return nil, ErrMaxRoutingInfoSizeExceeded
+	}
+
 	numHops := paymentPath.TrueRouteLength()
 
 	hopSharedSecrets := generateSharedSecrets(

sphinx_test.go

@@ -86,6 +86,8 @@ var (
 		"2cf4f0731da13b8546d1d6d4f8d75b9fce6c2341a71b0ea6f780" +
 		"df54bfdb0dd5cd9855179f602f917265f21f9190c70217774a6f" +
 		"baaa7d63ad64199f4664813b955cff954949076dcf"
+
+	testLegacyRouteNumHops = 20
 )
 
 func newTestRoute(numHops int) ([]*Router, *PaymentPath, *[]HopData, *OnionPacket, error) {
@@ -214,7 +216,7 @@ func TestBolt4Packet(t *testing.T) {
 }
 
 func TestSphinxCorrectness(t *testing.T) {
-	nodes, _, hopDatas, fwdMsg, err := newTestRoute(NumMaxHops)
+	nodes, _, hopDatas, fwdMsg, err := newTestRoute(testLegacyRouteNumHops)
 	if err != nil {
 		t.Fatalf("unable to create random onion packet: %v", err)
 	}
@@ -307,7 +309,7 @@ func TestSphinxSingleHop(t *testing.T) {
 func TestSphinxNodeRelpay(t *testing.T) {
 	// We'd like to ensure that the sphinx node itself rejects all replayed
 	// packets which share the same shared secret.
-	nodes, _, _, fwdMsg, err := newTestRoute(NumMaxHops)
+	nodes, _, _, fwdMsg, err := newTestRoute(testLegacyRouteNumHops)
 	if err != nil {
 		t.Fatalf("unable to create test route: %v", err)
 	}
@@ -332,7 +334,7 @@ func TestSphinxNodeRelpay(t *testing.T) {
 func TestSphinxNodeRelpaySameBatch(t *testing.T) {
 	// We'd like to ensure that the sphinx node itself rejects all replayed
 	// packets which share the same shared secret.
-	nodes, _, _, fwdMsg, err := newTestRoute(NumMaxHops)
+	nodes, _, _, fwdMsg, err := newTestRoute(testLegacyRouteNumHops)
 	if err != nil {
 		t.Fatalf("unable to create test route: %v", err)
 	}
@@ -378,7 +380,7 @@ func TestSphinxNodeRelpaySameBatch(t *testing.T) {
 func TestSphinxNodeRelpayLaterBatch(t *testing.T) {
 	// We'd like to ensure that the sphinx node itself rejects all replayed
 	// packets which share the same shared secret.
-	nodes, _, _, fwdMsg, err := newTestRoute(NumMaxHops)
+	nodes, _, _, fwdMsg, err := newTestRoute(testLegacyRouteNumHops)
 	if err != nil {
 		t.Fatalf("unable to create test route: %v", err)
 	}
@@ -423,7 +425,7 @@ func TestSphinxNodeRelpayLaterBatch(t *testing.T) {
 func TestSphinxNodeReplayBatchIdempotency(t *testing.T) {
 	// We'd like to ensure that the sphinx node itself rejects all replayed
 	// packets which share the same shared secret.
-	nodes, _, _, fwdMsg, err := newTestRoute(NumMaxHops)
+	nodes, _, _, fwdMsg, err := newTestRoute(testLegacyRouteNumHops)
 	if err != nil {
 		t.Fatalf("unable to create test route: %v", err)
 	}
@@ -563,8 +565,7 @@ func newEOBRoute(numHops uint32,
 	)
 	fwdMsg, err := NewOnionPacket(&route, sessionKey, nil)
 	if err != nil {
-		return nil, nil, fmt.Errorf("unable to create forwarding "+
-			"message: %#v", err)
+		return nil, nil, err
 	}
 
 	return fwdMsg, nodes, nil
@@ -587,8 +588,9 @@ func TestSphinxHopVariableSizedPayloads(t *testing.T) {
 	t.Parallel()
 
 	var testCases = []struct {
-		numNodes   uint32
-		eobMapping map[int]HopPayload
+		numNodes      uint32
+		eobMapping    map[int]HopPayload
+		expectedError error
 	}{
 		// A single hop route with a payload going to the last hop in
 		// the route. The payload is enough to fit into what would be
@@ -598,7 +600,7 @@ func TestSphinxHopVariableSizedPayloads(t *testing.T) {
 			eobMapping: map[int]HopPayload{
 				0: HopPayload{
 					Type:    PayloadTLV,
-					Payload: bytes.Repeat([]byte("a"), HopDataSize-HMACSize),
+					Payload: bytes.Repeat([]byte("a"), LegacyHopDataSize-HMACSize),
 				},
 			},
 		},
@@ -610,7 +612,7 @@ func TestSphinxHopVariableSizedPayloads(t *testing.T) {
 			eobMapping: map[int]HopPayload{
 				0: HopPayload{
 					Type:    PayloadTLV,
-					Payload: bytes.Repeat([]byte("a"), HopDataSize*3),
+					Payload: bytes.Repeat([]byte("a"), LegacyHopDataSize*3),
 				},
 			},
 		},
@@ -631,7 +633,7 @@ func TestSphinxHopVariableSizedPayloads(t *testing.T) {
 				}, nil),
 				1: HopPayload{
 					Type:    PayloadTLV,
-					Payload: bytes.Repeat([]byte("a"), HopDataSize*2),
+					Payload: bytes.Repeat([]byte("a"), LegacyHopDataSize*2),
 				},
 			},
 		},
@@ -678,16 +680,40 @@ func TestSphinxHopVariableSizedPayloads(t *testing.T) {
 				},
 			},
 		},
+
+		// A 3 hop route (4 nodes) that carries more data then what fits
+		// in the routing info.
+		{
+			numNodes: 3,
+			eobMapping: map[int]HopPayload{
+				0: HopPayload{
+					Type:    PayloadTLV,
+					Payload: bytes.Repeat([]byte("a"), 500),
+				},
+				1: HopPayload{
+					Type:    PayloadTLV,
+					Payload: bytes.Repeat([]byte("a"), 500),
+				},
+				2: HopPayload{
+					Type:    PayloadTLV,
+					Payload: bytes.Repeat([]byte("a"), 500),
+				},
+			},
+			expectedError: ErrMaxRoutingInfoSizeExceeded,
+		},
 	}
 
 	for testCaseNum, testCase := range testCases {
 		nextPkt, routers, err := newEOBRoute(
 			testCase.numNodes, testCase.eobMapping,
 		)
-		if err != nil {
+		if testCase.expectedError != err {
 			t.Fatalf("#%v: unable to create eob "+
 				"route: %v", testCase, err)
 		}
+		if err != nil {
+			continue
+		}
 
 		// We'll now walk thru manually each actual hop within the
 		// route. We use the size of the routers rather than the number