From 941a123ab09c646328dff9594218078c3a1b5496 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 31 Aug 2018 19:33:05 -0700 Subject: [PATCH] lnwire: add new ExtraOpaqueData field to gossip messages In this commit, we add a new field to all the existing gossip messages: ExtraOpqueData. We do this, as before this commit, if we came across a ChannelUpdate message with a set of optional fields, then we wouldn't be able to properly parse the signatures related to the message. If we never corrected this behavior, then we would violate the forwards compatible principle we use when parsing existing messages. As these messages can now be padded out to the max message size, we've increased the MaxPayloadLength value for all of these messages. Fixes #1814. --- lnwire/announcement_signatures.go | 49 ++++++++++++++-------- lnwire/channel_announcement.go | 67 ++++++++++++++----------------- lnwire/channel_update.go | 61 ++++++++++++++-------------- lnwire/node_announcement.go | 32 +++++++++++++-- lnwire/onion_error_test.go | 6 ++- 5 files changed, 125 insertions(+), 90 deletions(-) diff --git a/lnwire/announcement_signatures.go b/lnwire/announcement_signatures.go index 748f29b1..fd82211c 100644 --- a/lnwire/announcement_signatures.go +++ b/lnwire/announcement_signatures.go @@ -1,6 +1,9 @@ package lnwire -import "io" +import ( + "io" + "io/ioutil" +) // AnnounceSignatures this is a direct message between two endpoints of a // channel and serves as an opt-in mechanism to allow the announcement of @@ -30,6 +33,14 @@ type AnnounceSignatures struct { // bitcoin key and and creating the reverse reference bitcoin_key -> // node_key. BitcoinSignature Sig + + // ExtraOpaqueData is the set of data that was appended to this + // message, some of which we may not actually know how to iterate or + // parse. By holding onto this data, we ensure that we're able to + // properly validate the set of signatures that cover these new fields, + // and ensure we're able to make upgrades to the network in a forwards + // compatible manner. + ExtraOpaqueData []byte } // A compile time check to ensure AnnounceSignatures implements the @@ -41,12 +52,29 @@ var _ Message = (*AnnounceSignatures)(nil) // // This is part of the lnwire.Message interface. func (a *AnnounceSignatures) Decode(r io.Reader, pver uint32) error { - return readElements(r, + err := readElements(r, &a.ChannelID, &a.ShortChannelID, &a.NodeSignature, &a.BitcoinSignature, ) + if err != nil { + return err + } + + // Now that we've read out all the fields that we explicitly know of, + // we'll collect the remainder into the ExtraOpaqueData field. If there + // aren't any bytes, then we'll snip off the slice to avoid carrying + // around excess capacity. + a.ExtraOpaqueData, err = ioutil.ReadAll(r) + if err != nil { + return err + } + if len(a.ExtraOpaqueData) == 0 { + a.ExtraOpaqueData = nil + } + + return nil } // Encode serializes the target AnnounceSignatures into the passed io.Writer @@ -59,6 +87,7 @@ func (a *AnnounceSignatures) Encode(w io.Writer, pver uint32) error { a.ShortChannelID, a.NodeSignature, a.BitcoinSignature, + a.ExtraOpaqueData, ) } @@ -75,19 +104,5 @@ func (a *AnnounceSignatures) MsgType() MessageType { // // This is part of the lnwire.Message interface. func (a *AnnounceSignatures) MaxPayloadLength(pver uint32) uint32 { - var length uint32 - - // ChannelID - 36 bytes - length += 36 - - // ShortChannelID - 8 bytes - length += 8 - - // NodeSignatures - 64 bytes - length += 64 - - // BitcoinSignatures - 64 bytes - length += 64 - - return length + return 65533 } diff --git a/lnwire/channel_announcement.go b/lnwire/channel_announcement.go index f9b86510..70f53241 100644 --- a/lnwire/channel_announcement.go +++ b/lnwire/channel_announcement.go @@ -3,6 +3,7 @@ package lnwire import ( "bytes" "io" + "io/ioutil" "github.com/btcsuite/btcd/chaincfg/chainhash" ) @@ -48,6 +49,14 @@ type ChannelAnnouncement struct { // multisig funding transaction output. BitcoinKey1 [33]byte BitcoinKey2 [33]byte + + // ExtraOpaqueData is the set of data that was appended to this + // message, some of which we may not actually know how to iterate or + // parse. By holding onto this data, we ensure that we're able to + // properly validate the set of signatures that cover these new fields, + // and ensure we're able to make upgrades to the network in a forwards + // compatible manner. + ExtraOpaqueData []byte } // A compile time check to ensure ChannelAnnouncement implements the @@ -59,7 +68,7 @@ var _ Message = (*ChannelAnnouncement)(nil) // // This is part of the lnwire.Message interface. func (a *ChannelAnnouncement) Decode(r io.Reader, pver uint32) error { - return readElements(r, + err := readElements(r, &a.NodeSig1, &a.NodeSig2, &a.BitcoinSig1, @@ -72,6 +81,23 @@ func (a *ChannelAnnouncement) Decode(r io.Reader, pver uint32) error { &a.BitcoinKey1, &a.BitcoinKey2, ) + if err != nil { + return err + } + + // Now that we've read out all the fields that we explicitly know of, + // we'll collect the remainder into the ExtraOpaqueData field. If there + // aren't any bytes, then we'll snip off the slice to avoid carrying + // around excess capacity. + a.ExtraOpaqueData, err = ioutil.ReadAll(r) + if err != nil { + return err + } + if len(a.ExtraOpaqueData) == 0 { + a.ExtraOpaqueData = nil + } + + return nil } // Encode serializes the target ChannelAnnouncement into the passed io.Writer @@ -91,6 +117,7 @@ func (a *ChannelAnnouncement) Encode(w io.Writer, pver uint32) error { a.NodeID2, a.BitcoinKey1, a.BitcoinKey2, + a.ExtraOpaqueData, ) } @@ -107,42 +134,7 @@ func (a *ChannelAnnouncement) MsgType() MessageType { // // This is part of the lnwire.Message interface. func (a *ChannelAnnouncement) MaxPayloadLength(pver uint32) uint32 { - var length uint32 - - // NodeSig1 - 64 bytes - length += 64 - - // NodeSig2 - 64 bytes - length += 64 - - // BitcoinSig1 - 64 bytes - length += 64 - - // BitcoinSig2 - 64 bytes - length += 64 - - // Features (max possible features) - length += 65096 - - // ChainHash - 32 bytes - length += 32 - - // ShortChannelID - 8 bytes - length += 8 - - // NodeID1 - 33 bytes - length += 33 - - // NodeID2 - 33 bytes - length += 33 - - // BitcoinKey1 - 33 bytes - length += 33 - - // BitcoinKey2 - 33 bytes - length += 33 - - return length + return 65533 } // DataToSign is used to retrieve part of the announcement message which should @@ -158,6 +150,7 @@ func (a *ChannelAnnouncement) DataToSign() ([]byte, error) { a.NodeID2, a.BitcoinKey1, a.BitcoinKey2, + a.ExtraOpaqueData, ) if err != nil { return nil, err diff --git a/lnwire/channel_update.go b/lnwire/channel_update.go index 971f7bb3..5cc3b430 100644 --- a/lnwire/channel_update.go +++ b/lnwire/channel_update.go @@ -3,6 +3,7 @@ package lnwire import ( "bytes" "io" + "io/ioutil" "github.com/btcsuite/btcd/chaincfg/chainhash" ) @@ -73,6 +74,14 @@ type ChannelUpdate struct { // FeeRate is the fee rate that will be charged per millionth of a // satoshi. FeeRate uint32 + + // ExtraOpaqueData is the set of data that was appended to this + // message, some of which we may not actually know how to iterate or + // parse. By holding onto this data, we ensure that we're able to + // properly validate the set of signatures that cover these new fields, + // and ensure we're able to make upgrades to the network in a forwards + // compatible manner. + ExtraOpaqueData []byte } // A compile time check to ensure ChannelUpdate implements the lnwire.Message @@ -84,7 +93,7 @@ var _ Message = (*ChannelUpdate)(nil) // // This is part of the lnwire.Message interface. func (a *ChannelUpdate) Decode(r io.Reader, pver uint32) error { - return readElements(r, + err := readElements(r, &a.Signature, a.ChainHash[:], &a.ShortChannelID, @@ -95,6 +104,23 @@ func (a *ChannelUpdate) Decode(r io.Reader, pver uint32) error { &a.BaseFee, &a.FeeRate, ) + if err != nil { + return err + } + + // Now that we've read out all the fields that we explicitly know of, + // we'll collect the remainder into the ExtraOpaqueData field. If there + // aren't any bytes, then we'll snip off the slice to avoid carrying + // around excess capacity. + a.ExtraOpaqueData, err = ioutil.ReadAll(r) + if err != nil { + return err + } + if len(a.ExtraOpaqueData) == 0 { + a.ExtraOpaqueData = nil + } + + return nil } // Encode serializes the target ChannelUpdate into the passed io.Writer @@ -112,6 +138,7 @@ func (a *ChannelUpdate) Encode(w io.Writer, pver uint32) error { a.HtlcMinimumMsat, a.BaseFee, a.FeeRate, + a.ExtraOpaqueData, ) } @@ -128,36 +155,7 @@ func (a *ChannelUpdate) MsgType() MessageType { // // This is part of the lnwire.Message interface. func (a *ChannelUpdate) MaxPayloadLength(pver uint32) uint32 { - var length uint32 - - // Signature - 64 bytes - length += 64 - - // ChainHash - 64 bytes - length += 32 - - // ShortChannelID - 8 bytes - length += 8 - - // Timestamp - 4 bytes - length += 4 - - // Flags - 2 bytes - length += 2 - - // Expiry - 2 bytes - length += 2 - - // HtlcMinimumMstat - 8 bytes - length += 8 - - // FeeBaseMstat - 4 bytes - length += 4 - - // FeeProportionalMillionths - 4 bytes - length += 4 - - return length + return 65533 } // DataToSign is used to retrieve part of the announcement message which should @@ -175,6 +173,7 @@ func (a *ChannelUpdate) DataToSign() ([]byte, error) { a.HtlcMinimumMsat, a.BaseFee, a.FeeRate, + a.ExtraOpaqueData, ) if err != nil { return nil, err diff --git a/lnwire/node_announcement.go b/lnwire/node_announcement.go index 3a0b7ff9..f0ed70ea 100644 --- a/lnwire/node_announcement.go +++ b/lnwire/node_announcement.go @@ -5,6 +5,7 @@ import ( "fmt" "image/color" "io" + "io/ioutil" "net" "unicode/utf8" ) @@ -83,6 +84,14 @@ type NodeAnnouncement struct { // Address includes two specification fields: 'ipv6' and 'port' on // which the node is accepting incoming connections. Addresses []net.Addr + + // ExtraOpaqueData is the set of data that was appended to this + // message, some of which we may not actually know how to iterate or + // parse. By holding onto this data, we ensure that we're able to + // properly validate the set of signatures that cover these new fields, + // and ensure we're able to make upgrades to the network in a forwards + // compatible manner. + ExtraOpaqueData []byte } // UpdateNodeAnnAddrs is a functional option that allows updating the addresses @@ -102,7 +111,7 @@ var _ Message = (*NodeAnnouncement)(nil) // // This is part of the lnwire.Message interface. func (a *NodeAnnouncement) Decode(r io.Reader, pver uint32) error { - return readElements(r, + err := readElements(r, &a.Signature, &a.Features, &a.Timestamp, @@ -111,6 +120,23 @@ func (a *NodeAnnouncement) Decode(r io.Reader, pver uint32) error { a.Alias[:], &a.Addresses, ) + if err != nil { + return err + } + + // Now that we've read out all the fields that we explicitly know of, + // we'll collect the remainder into the ExtraOpaqueData field. If there + // aren't any bytes, then we'll snip off the slice to avoid carrying + // around excess capacity. + a.ExtraOpaqueData, err = ioutil.ReadAll(r) + if err != nil { + return err + } + if len(a.ExtraOpaqueData) == 0 { + a.ExtraOpaqueData = nil + } + + return nil } // Encode serializes the target NodeAnnouncement into the passed io.Writer @@ -125,6 +151,7 @@ func (a *NodeAnnouncement) Encode(w io.Writer, pver uint32) error { a.RGBColor, a.Alias[:], a.Addresses, + a.ExtraOpaqueData, ) } @@ -156,12 +183,11 @@ func (a *NodeAnnouncement) DataToSign() ([]byte, error) { a.RGBColor, a.Alias[:], a.Addresses, + a.ExtraOpaqueData, ) if err != nil { return nil, err } - // TODO(roasbeef): also capture the excess bytes in msg padded out? - return w.Bytes(), nil } diff --git a/lnwire/onion_error_test.go b/lnwire/onion_error_test.go index 9b2ab101..d59281a2 100644 --- a/lnwire/onion_error_test.go +++ b/lnwire/onion_error_test.go @@ -6,6 +6,8 @@ import ( "encoding/binary" "reflect" "testing" + + "github.com/davecgh/go-spew/spew" ) var ( @@ -66,8 +68,8 @@ func TestEncodeDecodeCode(t *testing.T) { } if !reflect.DeepEqual(failure1, failure2) { - t.Fatalf("failure message are different, failure "+ - "code(%v)", failure1.Code()) + t.Fatalf("expected %v, got %v", spew.Sdump(failure1), + spew.Sdump(failure2)) } } }