From 2181fd432ce4b2893307a10a599dc09b5e8d246d Mon Sep 17 00:00:00 2001 From: Matt Morehouse Date: Tue, 2 May 2023 17:04:48 -0500 Subject: [PATCH 1/4] lnwire: fuzz onion failure messages Fuzz tests for decoding and encoding of onion failure messages, based on the fuzz harness for other lnwire messages. The onion failure messages were uncovered by existing fuzz tests. --- lnwire/fuzz_test.go | 158 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/lnwire/fuzz_test.go b/lnwire/fuzz_test.go index 8ff38db1b1..41f0f1fb5d 100644 --- a/lnwire/fuzz_test.go +++ b/lnwire/fuzz_test.go @@ -4,6 +4,7 @@ import ( "bytes" "compress/zlib" "encoding/binary" + "reflect" "testing" "github.com/stretchr/testify/require" @@ -635,3 +636,160 @@ func FuzzConvertFixedSignature(f *testing.F) { require.Equal(t, derBytes, derBytes2, "signature mismatch") }) } + +// prefixWithFailCode adds a failure code prefix to data. +func prefixWithFailCode(data []byte, code FailCode) []byte { + var codeBytes [2]byte + binary.BigEndian.PutUint16(codeBytes[:], uint16(code)) + data = append(codeBytes[:], data...) + + return data +} + +// onionFailureHarness performs the actual fuzz testing of the appropriate onion +// failure message. This function will check that the passed-in message passes +// wire length checks, is a valid message once deserialized, and passes a +// sequence of serialization and deserialization checks. +func onionFailureHarness(t *testing.T, data []byte, code FailCode) { + data = prefixWithFailCode(data, code) + + // Don't waste time fuzzing messages larger than we'll ever accept. + if len(data) > MaxSliceLength { + return + } + + // First check whether the failure message can be decoded. + r := bytes.NewReader(data) + msg, err := DecodeFailureMessage(r, 0) + if err != nil { + return + } + + // We now have a valid decoded message. Verify that encoding and + // decoding the message does not mutate it. + + var b bytes.Buffer + if err := EncodeFailureMessage(&b, msg, 0); err != nil { + t.Fatalf("failed to encode failure message: %v", err) + } + + newMsg, err := DecodeFailureMessage(&b, 0) + if err != nil { + t.Fatalf("failed to decode serialized failure message: %v", err) + } + + if !reflect.DeepEqual(msg, newMsg) { + t.Fatalf("original message and deserialized message are not "+ + "equal: %v != %v", msg, newMsg) + } + + // Now verify that encoding/decoding full packets works as expected. + + var pktBuf bytes.Buffer + if err := EncodeFailure(&pktBuf, msg, 0); err != nil { + // EncodeFailure returns an error if the encoded message would + // exceed FailureMessageLength bytes, as LND always encodes + // fixed-size packets for privacy. But it is valid to decode + // messages longer than this, so we should not report an error + // if the original message was longer. + // + // We add 2 to the length of the original message since it may + // have omitted a channel_update type prefix of 2 bytes. When + // we re-encode such a message, we will add the 2-byte prefix + // as prescribed by the spec. + if len(data)+2 > FailureMessageLength { + return + } + + t.Fatalf("failed to encode failure packet: %v", err) + } + + // We should use FailureMessageLength sized packets plus 2 bytes to + // encode the message length and 2 bytes to encode the padding length, + // as recommended by the spec. + if pktBuf.Len() != FailureMessageLength+4 { + t.Fatalf("wrong failure message length: %v", pktBuf.Len()) + } + + pktMsg, err := DecodeFailure(&pktBuf, 0) + if err != nil { + t.Fatalf("failed to decode failure packet: %v", err) + } + + if !reflect.DeepEqual(msg, pktMsg) { + t.Fatalf("original message and decoded packet message are not "+ + "equal: %v != %v", msg, pktMsg) + } +} + +func FuzzFailInvalidOnionVersion(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeInvalidOnionVersion) + }) +} + +func FuzzFailInvalidOnionHmac(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeInvalidOnionHmac) + }) +} + +func FuzzFailInvalidOnionKey(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeInvalidOnionKey) + }) +} + +func FuzzFailTemporaryChannelFailure(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeTemporaryChannelFailure) + }) +} + +func FuzzFailAmountBelowMinimum(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeAmountBelowMinimum) + }) +} + +func FuzzFailFeeInsufficient(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeFeeInsufficient) + }) +} + +func FuzzFailIncorrectCltvExpiry(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeIncorrectCltvExpiry) + }) +} + +func FuzzFailExpiryTooSoon(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeExpiryTooSoon) + }) +} + +func FuzzFailChannelDisabled(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeChannelDisabled) + }) +} + +func FuzzFailFinalIncorrectCltvExpiry(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeFinalIncorrectCltvExpiry) + }) +} + +func FuzzFailFinalIncorrectHtlcAmount(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeFinalIncorrectHtlcAmount) + }) +} + +func FuzzInvalidOnionPayload(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + onionFailureHarness(t, data, CodeInvalidOnionPayload) + }) +} From a7ee45bea09b518b39deb2865658b96417038a03 Mon Sep 17 00:00:00 2001 From: Matt Morehouse Date: Tue, 2 May 2023 18:07:49 -0500 Subject: [PATCH 2/4] lnwire: add FuzzFailIncorrectDetails test The test is identical to other onion failure fuzz tests, except that it uses a custom equality function to get around the nil != []byte{} issue with reflect.DeepEqual. --- lnwire/fuzz_test.go | 53 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/lnwire/fuzz_test.go b/lnwire/fuzz_test.go index 41f0f1fb5d..95f35884a3 100644 --- a/lnwire/fuzz_test.go +++ b/lnwire/fuzz_test.go @@ -646,11 +646,17 @@ func prefixWithFailCode(data []byte, code FailCode) []byte { return data } -// onionFailureHarness performs the actual fuzz testing of the appropriate onion -// failure message. This function will check that the passed-in message passes -// wire length checks, is a valid message once deserialized, and passes a +// equalFunc is a function used to determine whether two deserialized messages +// are equivalent. +type equalFunc func(x, y any) bool + +// onionFailureHarnessCustom performs the actual fuzz testing of the appropriate +// onion failure message. This function will check that the passed-in message +// passes wire length checks, is a valid message once deserialized, and passes a // sequence of serialization and deserialization checks. -func onionFailureHarness(t *testing.T, data []byte, code FailCode) { +func onionFailureHarnessCustom(t *testing.T, data []byte, code FailCode, + eq equalFunc) { + data = prefixWithFailCode(data, code) // Don't waste time fuzzing messages larger than we'll ever accept. @@ -678,7 +684,7 @@ func onionFailureHarness(t *testing.T, data []byte, code FailCode) { t.Fatalf("failed to decode serialized failure message: %v", err) } - if !reflect.DeepEqual(msg, newMsg) { + if !eq(msg, newMsg) { t.Fatalf("original message and deserialized message are not "+ "equal: %v != %v", msg, newMsg) } @@ -716,12 +722,47 @@ func onionFailureHarness(t *testing.T, data []byte, code FailCode) { t.Fatalf("failed to decode failure packet: %v", err) } - if !reflect.DeepEqual(msg, pktMsg) { + if !eq(msg, pktMsg) { t.Fatalf("original message and decoded packet message are not "+ "equal: %v != %v", msg, pktMsg) } } +func onionFailureHarness(t *testing.T, data []byte, code FailCode) { + t.Helper() + onionFailureHarnessCustom(t, data, code, reflect.DeepEqual) +} + +func FuzzFailIncorrectDetails(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte) { + // Since FailIncorrectDetails.Decode can leave extraOpaqueData + // as nil while FailIncorrectDetails.Encode writes an empty + // slice, we need to use a custom equality function. + eq := func(x, y any) bool { + msg1, ok := x.(*FailIncorrectDetails) + if !ok { + t.Fatal("msg1 was not FailIncorrectDetails") + } + + msg2, ok := y.(*FailIncorrectDetails) + if !ok { + t.Fatalf("msg2 was not FailIncorrectDetails") + } + + return msg1.amount == msg2.amount && + msg1.height == msg2.height && + bytes.Equal( + msg1.extraOpaqueData, + msg2.extraOpaqueData, + ) + } + + onionFailureHarnessCustom( + t, data, CodeIncorrectOrUnknownPaymentDetails, eq, + ) + }) +} + func FuzzFailInvalidOnionVersion(f *testing.F) { f.Fuzz(func(t *testing.T, data []byte) { onionFailureHarness(t, data, CodeInvalidOnionVersion) From ed1b54ac3ad52005915221b3c6f78e2d32f6e06f Mon Sep 17 00:00:00 2001 From: Matt Morehouse Date: Fri, 27 Oct 2023 14:30:56 -0500 Subject: [PATCH 3/4] lnwire: use require package for onion failure fuzz tests --- lnwire/fuzz_test.go | 52 ++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/lnwire/fuzz_test.go b/lnwire/fuzz_test.go index 95f35884a3..f789e7ce72 100644 --- a/lnwire/fuzz_test.go +++ b/lnwire/fuzz_test.go @@ -675,19 +675,18 @@ func onionFailureHarnessCustom(t *testing.T, data []byte, code FailCode, // decoding the message does not mutate it. var b bytes.Buffer - if err := EncodeFailureMessage(&b, msg, 0); err != nil { - t.Fatalf("failed to encode failure message: %v", err) - } + err = EncodeFailureMessage(&b, msg, 0) + require.NoError(t, err, "failed to encode failure message") newMsg, err := DecodeFailureMessage(&b, 0) - if err != nil { - t.Fatalf("failed to decode serialized failure message: %v", err) - } + require.NoError(t, err, "failed to decode serialized failure message") - if !eq(msg, newMsg) { - t.Fatalf("original message and deserialized message are not "+ - "equal: %v != %v", msg, newMsg) - } + require.True( + t, eq(msg, newMsg), + "original message and deserialized message are not equal: "+ + "%v != %v", + msg, newMsg, + ) // Now verify that encoding/decoding full packets works as expected. @@ -713,19 +712,20 @@ func onionFailureHarnessCustom(t *testing.T, data []byte, code FailCode, // We should use FailureMessageLength sized packets plus 2 bytes to // encode the message length and 2 bytes to encode the padding length, // as recommended by the spec. - if pktBuf.Len() != FailureMessageLength+4 { - t.Fatalf("wrong failure message length: %v", pktBuf.Len()) - } + require.Equal( + t, pktBuf.Len(), FailureMessageLength+4, + "wrong failure message length", + ) pktMsg, err := DecodeFailure(&pktBuf, 0) - if err != nil { - t.Fatalf("failed to decode failure packet: %v", err) - } + require.NoError(t, err, "failed to decode failure packet") - if !eq(msg, pktMsg) { - t.Fatalf("original message and decoded packet message are not "+ - "equal: %v != %v", msg, pktMsg) - } + require.True( + t, eq(msg, pktMsg), + "original message and decoded packet message are not equal: "+ + "%v != %v", + msg, pktMsg, + ) } func onionFailureHarness(t *testing.T, data []byte, code FailCode) { @@ -740,14 +740,14 @@ func FuzzFailIncorrectDetails(f *testing.F) { // slice, we need to use a custom equality function. eq := func(x, y any) bool { msg1, ok := x.(*FailIncorrectDetails) - if !ok { - t.Fatal("msg1 was not FailIncorrectDetails") - } + require.True( + t, ok, "msg1 was not FailIncorrectDetails", + ) msg2, ok := y.(*FailIncorrectDetails) - if !ok { - t.Fatalf("msg2 was not FailIncorrectDetails") - } + require.True( + t, ok, "msg2 was not FailIncorrectDetails", + ) return msg1.amount == msg2.amount && msg1.height == msg2.height && From 66d6a8455d4c4dafd679aeb797635efe50b4aeca Mon Sep 17 00:00:00 2001 From: Matt Morehouse Date: Fri, 27 Oct 2023 14:54:29 -0500 Subject: [PATCH 4/4] docs: update release notes --- docs/release-notes/release-notes-0.18.0.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/release-notes/release-notes-0.18.0.md b/docs/release-notes/release-notes-0.18.0.md index dd24c85098..cc346ff8b5 100644 --- a/docs/release-notes/release-notes-0.18.0.md +++ b/docs/release-notes/release-notes-0.18.0.md @@ -82,6 +82,10 @@ # Technical and Architectural Updates ## BOLT Spec Updates ## Testing + +* Added fuzz tests for [onion + errors](https://github.com/lightningnetwork/lnd/pull/7669). + ## Database * [Add context to InvoiceDB @@ -102,5 +106,6 @@ * Carla Kirk-Cohen * Elle Mouton * Keagan McClelland +* Matt Morehouse * Ononiwu Maureen Chiamaka * Yong Yu