From f77388e0726e789adcb0d6aceb7bda1265b2f364 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 20 Apr 2022 16:53:44 +0200 Subject: [PATCH 1/5] lnwire: add TestDecodeUnknownAddressType Add a test to demonstrate that if a NodeAnnouncement includes an address with an unknown type, then we incorrectly return an error. This will be fixed in the following commit. --- lnwire/lnwire_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/lnwire/lnwire_test.go b/lnwire/lnwire_test.go index 0e3671d85..9640d3a27 100644 --- a/lnwire/lnwire_test.go +++ b/lnwire/lnwire_test.go @@ -20,6 +20,7 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/tor" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( @@ -226,6 +227,52 @@ func TestChanUpdateChanFlags(t *testing.T) { } } +// TestDecodeUnknownAddressType shows that an unknown address type is currently +// incorrectly dealt with. +func TestDecodeUnknownAddressType(t *testing.T) { + // First, we'll encode all the addresses into an intermediate + // buffer. We need to do this in order to compute the total + // length of the addresses. + addrBuf := bytes.NewBuffer(make([]byte, 0, MaxMsgBody)) + + // Add a normal, clearnet address. + tcpAddr := &net.TCPAddr{ + IP: net.IP{127, 0, 0, 1}, + Port: 8080, + } + + err := WriteTCPAddr(addrBuf, tcpAddr) + require.NoError(t, err) + + // Add an onion address. + onionAddr := &tor.OnionAddr{ + OnionService: "abcdefghijklmnop.onion", + Port: 9065, + } + + err = WriteOnionAddr(addrBuf, onionAddr) + require.NoError(t, err) + + // Now add an address with an unknown type. + var newAddrType addressType = math.MaxUint8 + data := make([]byte, 0, 16) + data = append(data, uint8(newAddrType)) + _, err = addrBuf.Write(data) + require.NoError(t, err) + + // Now that we have all the addresses, we can append the length. + buffer := bytes.NewBuffer(make([]byte, 0, MaxMsgBody)) + err = writeDataWithLength(buffer, addrBuf.Bytes()) + require.NoError(t, err) + + // Now we attempt to parse the bytes and we assert that we get an + // error. + var addrs []net.Addr + err = ReadElement(buffer, &addrs) + require.Error(t, err) + require.IsType(t, err, &ErrUnknownAddrType{}) +} + func TestMaxOutPointIndex(t *testing.T) { t.Parallel() From fa0512f62fdebe53482aa1860ebadc36fb783281 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 21 Apr 2022 10:42:27 +0200 Subject: [PATCH 2/5] lnwire: add new type OpaqueAddrs In this commit, a new net.Addr implementation called OpaqueAddrs is added along with a WriteOpaqueAddrs func that is called in WriteNetAddrs. It will be used to store any address bytes that we cannot parse due to us not being aware of the address type. --- lnwire/lnwire_test.go | 24 +++++++----------------- lnwire/opaque_addrs.go | 34 ++++++++++++++++++++++++++++++++++ lnwire/writer.go | 17 +++++++++++++++++ 3 files changed, 58 insertions(+), 17 deletions(-) create mode 100644 lnwire/opaque_addrs.go diff --git a/lnwire/lnwire_test.go b/lnwire/lnwire_test.go index 9640d3a27..fb0b746ef 100644 --- a/lnwire/lnwire_test.go +++ b/lnwire/lnwire_test.go @@ -230,43 +230,33 @@ func TestChanUpdateChanFlags(t *testing.T) { // TestDecodeUnknownAddressType shows that an unknown address type is currently // incorrectly dealt with. func TestDecodeUnknownAddressType(t *testing.T) { - // First, we'll encode all the addresses into an intermediate - // buffer. We need to do this in order to compute the total - // length of the addresses. - addrBuf := bytes.NewBuffer(make([]byte, 0, MaxMsgBody)) - // Add a normal, clearnet address. tcpAddr := &net.TCPAddr{ IP: net.IP{127, 0, 0, 1}, Port: 8080, } - err := WriteTCPAddr(addrBuf, tcpAddr) - require.NoError(t, err) - // Add an onion address. onionAddr := &tor.OnionAddr{ OnionService: "abcdefghijklmnop.onion", Port: 9065, } - err = WriteOnionAddr(addrBuf, onionAddr) - require.NoError(t, err) - // Now add an address with an unknown type. var newAddrType addressType = math.MaxUint8 data := make([]byte, 0, 16) data = append(data, uint8(newAddrType)) - _, err = addrBuf.Write(data) - require.NoError(t, err) + opaqueAddrs := &OpaqueAddrs{ + Payload: data, + } - // Now that we have all the addresses, we can append the length. buffer := bytes.NewBuffer(make([]byte, 0, MaxMsgBody)) - err = writeDataWithLength(buffer, addrBuf.Bytes()) + err := WriteNetAddrs( + buffer, []net.Addr{tcpAddr, onionAddr, opaqueAddrs}, + ) require.NoError(t, err) - // Now we attempt to parse the bytes and we assert that we get an - // error. + // Now we attempt to parse the bytes and assert that we get an error. var addrs []net.Addr err = ReadElement(buffer, &addrs) require.Error(t, err) diff --git a/lnwire/opaque_addrs.go b/lnwire/opaque_addrs.go new file mode 100644 index 000000000..92f59ea21 --- /dev/null +++ b/lnwire/opaque_addrs.go @@ -0,0 +1,34 @@ +package lnwire + +import ( + "encoding/hex" + "net" +) + +// OpaqueAddrs is used to store the address bytes for address types that are +// unknown to us. +type OpaqueAddrs struct { + Payload []byte +} + +// A compile-time assertion to ensure that OpaqueAddrs meets the net.Addr +// interface. +var _ net.Addr = (*OpaqueAddrs)(nil) + +// String returns a human-readable string describing the target OpaqueAddrs. +// Since this is an unknown address (and could even be multiple addresses), we +// just return the hex string of the payload. +// +// This part of the net.Addr interface. +func (o *OpaqueAddrs) String() string { + return hex.EncodeToString(o.Payload) +} + +// Network returns the name of the network this address is bound to. Since this +// is an unknown address, we don't know the network and so just return a string +// indicating this. +// +// This part of the net.Addr interface. +func (o *OpaqueAddrs) Network() string { + return "unknown network for unrecognized address type" +} diff --git a/lnwire/writer.go b/lnwire/writer.go index 084e7d892..5b99ab368 100644 --- a/lnwire/writer.go +++ b/lnwire/writer.go @@ -32,6 +32,9 @@ var ( // ErrNilNetAddress is returned when a nil value is used in []net.Addr. ErrNilNetAddress = errors.New("cannot write nil address") + // ErrNilOpaqueAddrs is returned when the supplied address is nil. + ErrNilOpaqueAddrs = errors.New("cannot write nil OpaqueAddrs") + // ErrNilPublicKey is returned when a nil pubkey is used. ErrNilPublicKey = errors.New("cannot write nil pubkey") @@ -357,6 +360,16 @@ func WriteOnionAddr(buf *bytes.Buffer, addr *tor.OnionAddr) error { return WriteUint16(buf, uint16(addr.Port)) } +// WriteOpaqueAddrs appends the payload of the given OpaqueAddrs to buffer. +func WriteOpaqueAddrs(buf *bytes.Buffer, addr *OpaqueAddrs) error { + if addr == nil { + return ErrNilOpaqueAddrs + } + + _, err := buf.Write(addr.Payload) + return err +} + // WriteNetAddrs appends a slice of addresses to the provided buffer with the // length info. func WriteNetAddrs(buf *bytes.Buffer, addresses []net.Addr) error { @@ -376,6 +389,10 @@ func WriteNetAddrs(buf *bytes.Buffer, addresses []net.Addr) error { if err := WriteOnionAddr(addrBuf, a); err != nil { return err } + case *OpaqueAddrs: + if err := WriteOpaqueAddrs(addrBuf, a); err != nil { + return err + } default: return ErrNilNetAddress } From 1477c754f5ba4d964fa7c6bc4e1a08c5fc4ecc64 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 21 Apr 2022 10:53:06 +0200 Subject: [PATCH 3/5] lnwire: store unknown address type as OpaqueAddrs Instead of erroring out with encountering an address with an unknown type, we just store the remainder of the addrBytes as OpaqueAddrs so that we are able to rewrite them to the wire when we re-propagate the message. --- lnwire/lnwire.go | 24 +++++++++++++++++++++++- lnwire/lnwire_test.go | 7 +++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lnwire/lnwire.go b/lnwire/lnwire.go index 290fc4ed0..0361d7648 100644 --- a/lnwire/lnwire.go +++ b/lnwire/lnwire.go @@ -804,7 +804,29 @@ func ReadElement(r io.Reader, element interface{}) error { addrBytesRead += aType.AddrLen() default: - return &ErrUnknownAddrType{aType} + // If we don't understand this address type, + // we just store it along with the remaining + // address bytes as type OpaqueAddrs. We need + // to hold onto the bytes so that we can still + // write them back to the wire when we + // propagate this message. + payloadLen := 1 + addrsLen - addrBytesRead + payload := make([]byte, payloadLen) + + // First write a byte for the address type that + // we already read. + payload[0] = byte(aType) + + // Now append the rest of the address bytes. + _, err := io.ReadFull(addrBuf, payload[1:]) + if err != nil { + return err + } + + address = &OpaqueAddrs{ + Payload: payload, + } + addrBytesRead = addrsLen } addresses = append(addresses, address) diff --git a/lnwire/lnwire_test.go b/lnwire/lnwire_test.go index fb0b746ef..3cbc34803 100644 --- a/lnwire/lnwire_test.go +++ b/lnwire/lnwire_test.go @@ -259,8 +259,11 @@ func TestDecodeUnknownAddressType(t *testing.T) { // Now we attempt to parse the bytes and assert that we get an error. var addrs []net.Addr err = ReadElement(buffer, &addrs) - require.Error(t, err) - require.IsType(t, err, &ErrUnknownAddrType{}) + require.NoError(t, err) + require.Len(t, addrs, 3) + require.Equal(t, tcpAddr.String(), addrs[0].String()) + require.Equal(t, onionAddr.String(), addrs[1].String()) + require.Equal(t, hex.EncodeToString(data), addrs[2].String()) } func TestMaxOutPointIndex(t *testing.T) { From f40e2687acd44d06947612b3da51d0eee21ca345 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 21 Apr 2022 10:56:38 +0200 Subject: [PATCH 4/5] lnwire: add randOpaqueAddrs for fuzz test Let TestLightningWireProtocol also include random OpaqueAddrs. --- lnwire/lnwire_test.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/lnwire/lnwire_test.go b/lnwire/lnwire_test.go index 3cbc34803..ec528a38a 100644 --- a/lnwire/lnwire_test.go +++ b/lnwire/lnwire_test.go @@ -159,6 +159,22 @@ func randV3OnionAddr(r *rand.Rand) (*tor.OnionAddr, error) { return &tor.OnionAddr{OnionService: onionService, Port: addrPort}, nil } +func randOpaqueAddr(r *rand.Rand) (*OpaqueAddrs, error) { + payloadLen := r.Int63n(64) + 1 + payload := make([]byte, payloadLen) + + // The first byte is the address type. So set it to one that we + // definitely don't know about. + payload[0] = math.MaxUint8 + + // Generate random bytes for the rest of the payload. + if _, err := r.Read(payload[1:]); err != nil { + return nil, err + } + + return &OpaqueAddrs{Payload: payload}, nil +} + func randAddrs(r *rand.Rand) ([]net.Addr, error) { tcp4Addr, err := randTCP4Addr(r) if err != nil { @@ -180,7 +196,14 @@ func randAddrs(r *rand.Rand) ([]net.Addr, error) { return nil, err } - return []net.Addr{tcp4Addr, tcp6Addr, v2OnionAddr, v3OnionAddr}, nil + opaqueAddrs, err := randOpaqueAddr(r) + if err != nil { + return nil, err + } + + return []net.Addr{ + tcp4Addr, tcp6Addr, v2OnionAddr, v3OnionAddr, opaqueAddrs, + }, nil } // TestChanUpdateChanFlags ensures that converting the ChanUpdateChanFlags and From fa5d4195e0350171a1d08988209678e9e551935b Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 20 Apr 2022 17:00:34 +0200 Subject: [PATCH 5/5] docs/release-notes: update release notes --- docs/release-notes/release-notes-0.15.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.15.0.md b/docs/release-notes/release-notes-0.15.0.md index 36521fd2f..ddcf73f42 100644 --- a/docs/release-notes/release-notes-0.15.0.md +++ b/docs/release-notes/release-notes-0.15.0.md @@ -136,6 +136,9 @@ close to continue after a peer disconnect](https://github.com/lightningnetwork/l * [A subsystem hand-off between the contractcourt and htlcswitch has been fixed by adding a persistence layer. This avoids a rare edge case from occurring that would result in an erroneous force close.](https://github.com/lightningnetwork/lnd/pull/6250) +* [Ignore addresses with unknown types in NodeAnnouncements]( + https://github.com/lightningnetwork/lnd/pull/6435) + ## Routing * [Add a new `time_pref` parameter to the QueryRoutes and SendPayment APIs](https://github.com/lightningnetwork/lnd/pull/6024) that