From 24e663519aa07fcb63e14b410a495f1fa660d23f Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 16 Dec 2019 13:06:15 -0800 Subject: [PATCH 1/4] lnwire/features: add payment-addr and mpp feature bits --- lnwire/features.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lnwire/features.go b/lnwire/features.go index f26403ae..d2ffa136 100644 --- a/lnwire/features.go +++ b/lnwire/features.go @@ -81,6 +81,26 @@ const ( // party's non-delay output should not be tweaked. StaticRemoteKeyOptional FeatureBit = 13 + // PaymentAddrRequired is a required feature bit that signals that a + // node requires payment addresses, which are used to mitigate probing + // attacks on the receiver of a payment. + PaymentAddrRequired FeatureBit = 14 + + // PaymentAddrOptional is an optional feature bit that signals that a + // node supports payment addresses, which are used to mitigate probing + // attacks on the receiver of a payment. + PaymentAddrOptional FeatureBit = 15 + + // MPPOptional is a required feature bit that signals that the receiver + // of a payment requires settlement of an invoice with more than one + // HTLC. + MPPRequired FeatureBit = 16 + + // MPPOptional is an optional feature bit that signals that the receiver + // of a payment supports settlement of an invoice with more than one + // HTLC. + MPPOptional FeatureBit = 17 + // maxAllowedSize is a maximum allowed size of feature vector. // // NOTE: Within the protocol, the maximum allowed message size is 65535 From 868a5425c1b9b80598c020360230498e2042e317 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 16 Dec 2019 13:06:30 -0800 Subject: [PATCH 2/4] feature/deps: validate feature dependencies --- feature/deps.go | 124 ++++++++++++++++++++++++++++++++ feature/deps_test.go | 167 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 291 insertions(+) create mode 100644 feature/deps.go create mode 100644 feature/deps_test.go diff --git a/feature/deps.go b/feature/deps.go new file mode 100644 index 00000000..173318d5 --- /dev/null +++ b/feature/deps.go @@ -0,0 +1,124 @@ +package feature + +import ( + "fmt" + + "github.com/lightningnetwork/lnd/lnwire" +) + +type ( + // featureSet contains a set of feature bits. + featureSet map[lnwire.FeatureBit]struct{} + + // supportedFeatures maps the feature bit from a feature vector to a + // boolean indicating if this features dependencies have already been + // verified. This allows us to short circuit verification if multiple + // features have common dependencies, or map traversal starts verifying + // from the bottom up. + supportedFeatures map[lnwire.FeatureBit]bool + + // depDesc maps a features to its set of dependent features, which must + // also be present for the vector to be valid. This can be used to + // recursively check the dependency chain for features in a feature + // vector. + depDesc map[lnwire.FeatureBit]featureSet +) + +// ErrMissingFeatureDep is an error signaling that a transitive dependency in a +// feature vector is not set properly. +type ErrMissingFeatureDep struct { + dep lnwire.FeatureBit +} + +// Error returns a human-readable description of the missing dep error. +func (e ErrMissingFeatureDep) Error() string { + return fmt.Sprintf("missing feature dependency: %v", e.dep) +} + +// deps is the default set of dependencies for assigned feature bits. If a +// feature is not present in the depDesc it is assumed to have no dependencies. +// +// NOTE: For proper functioning, only the optional variant of feature bits +// should be used in the following descriptor. In the future it may be necessary +// to distinguish the dependencies for optional and required bits, but for now +// the validation code maps required bits to optional ones since it simplifies +// the number of constraints. +var deps = depDesc{ + lnwire.PaymentAddrOptional: { + lnwire.TLVOnionPayloadOptional: {}, + }, + lnwire.MPPOptional: { + lnwire.PaymentAddrOptional: {}, + }, +} + +// ValidateDeps asserts that a feature vector sets all features and their +// transitive dependencies properly. It assumes that the dependencies between +// optional and required features are identical, e.g. if a feature is required +// but its dependency is optional, that is sufficient. +func ValidateDeps(fv *lnwire.FeatureVector) error { + features := fv.Features() + supported := initSupported(features) + + return validateDeps(features, supported) +} + +// validateDeps is a subroutine that recursively checks that the passed features +// have all of their associated dependencies in the supported map. +func validateDeps(features featureSet, supported supportedFeatures) error { + for bit := range features { + // Convert any required bits to optional. + bit = mapToOptional(bit) + + // If the supported features doesn't contain the dependency, this + // vector is invalid. + checked, ok := supported[bit] + if !ok { + return ErrMissingFeatureDep{bit} + } + + // Alternatively, if we know that this depdendency is valid, we + // can short circuit and continue verifying other bits. + if checked { + continue + } + + // Recursively validate dependencies, since this method ranges + // over the subDeps. This method will return true even if + // subDeps is nil. + subDeps := deps[bit] + if err := validateDeps(subDeps, supported); err != nil { + return err + } + + // Once we've confirmed that this feature's dependencies, if + // any, are sound, we record this so other paths taken through + // `bit` return early when inspecting the supported map. + supported[bit] = true + } + + return nil +} + +// initSupported sets all bits from the feature vector as supported but not +// checked. This signals that the validity of their dependencies has not been +// verified. All required bits are mapped to optional to simplify the DAG. +func initSupported(features featureSet) supportedFeatures { + supported := make(supportedFeatures) + for bit := range features { + bit = mapToOptional(bit) + supported[bit] = false + } + + return supported +} + +// mapToOptional returns the optional variant of a given feature bit pair. Our +// dependendency graph is described using only optional feature bits, which +// reduces the number of constraints we need to express in the descriptor. +func mapToOptional(bit lnwire.FeatureBit) lnwire.FeatureBit { + if bit.IsRequired() { + bit ^= 0x01 + } + return bit +} diff --git a/feature/deps_test.go b/feature/deps_test.go new file mode 100644 index 00000000..f1d950df --- /dev/null +++ b/feature/deps_test.go @@ -0,0 +1,167 @@ +package feature + +import ( + "reflect" + "testing" + + "github.com/lightningnetwork/lnd/lnwire" +) + +type depTest struct { + name string + raw *lnwire.RawFeatureVector + expErr error +} + +var depTests = []depTest{ + { + name: "empty", + raw: lnwire.NewRawFeatureVector(), + }, + { + name: "no deps optional", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesOptional, + ), + }, + { + name: "no deps required", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadRequired, + ), + }, + { + name: "one dep optional", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadOptional, + lnwire.PaymentAddrOptional, + ), + }, + { + name: "one dep required", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadRequired, + lnwire.PaymentAddrRequired, + ), + }, + { + name: "one missing optional", + raw: lnwire.NewRawFeatureVector( + lnwire.PaymentAddrOptional, + ), + expErr: ErrMissingFeatureDep{lnwire.TLVOnionPayloadOptional}, + }, + { + name: "one missing required", + raw: lnwire.NewRawFeatureVector( + lnwire.PaymentAddrRequired, + ), + expErr: ErrMissingFeatureDep{lnwire.TLVOnionPayloadOptional}, + }, + { + name: "two dep optional", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadOptional, + lnwire.PaymentAddrOptional, + lnwire.MPPOptional, + ), + }, + { + name: "two dep required", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadRequired, + lnwire.PaymentAddrRequired, + lnwire.MPPRequired, + ), + }, + { + name: "two dep last missing optional", + raw: lnwire.NewRawFeatureVector( + lnwire.PaymentAddrOptional, + lnwire.MPPOptional, + ), + expErr: ErrMissingFeatureDep{lnwire.TLVOnionPayloadOptional}, + }, + { + name: "two dep last missing required", + raw: lnwire.NewRawFeatureVector( + lnwire.PaymentAddrRequired, + lnwire.MPPRequired, + ), + expErr: ErrMissingFeatureDep{lnwire.TLVOnionPayloadOptional}, + }, + { + name: "two dep first missing optional", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadOptional, + lnwire.MPPOptional, + ), + expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional}, + }, + { + name: "two dep first missing required", + raw: lnwire.NewRawFeatureVector( + lnwire.TLVOnionPayloadRequired, + lnwire.MPPRequired, + ), + expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional}, + }, + { + name: "forest optional", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesOptional, + lnwire.TLVOnionPayloadOptional, + lnwire.PaymentAddrOptional, + lnwire.MPPOptional, + ), + }, + { + name: "forest required", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesRequired, + lnwire.TLVOnionPayloadRequired, + lnwire.PaymentAddrRequired, + lnwire.MPPRequired, + ), + }, + { + name: "broken forest optional", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesOptional, + lnwire.TLVOnionPayloadOptional, + lnwire.MPPOptional, + ), + expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional}, + }, + { + name: "broken forest required", + raw: lnwire.NewRawFeatureVector( + lnwire.GossipQueriesRequired, + lnwire.TLVOnionPayloadRequired, + lnwire.MPPRequired, + ), + expErr: ErrMissingFeatureDep{lnwire.PaymentAddrOptional}, + }, +} + +// TestValidateDeps tests that ValidateDeps correctly asserts whether or not the +// set features constitute a valid feature chain when accounting for transititve +// dependencies. +func TestValidateDeps(t *testing.T) { + for _, test := range depTests { + test := test + t.Run(test.name, func(t *testing.T) { + testValidateDeps(t, test) + }) + } +} + +func testValidateDeps(t *testing.T, test depTest) { + fv := lnwire.NewFeatureVector(test.raw, lnwire.Features) + err := ValidateDeps(fv) + if !reflect.DeepEqual(err, test.expErr) { + t.Fatalf("validation mismatch, want: %v, got: %v", + test.expErr, err) + + } +} From 3208e287c3bd6155df944cf8d022d42c078795f0 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 16 Dec 2019 13:06:45 -0800 Subject: [PATCH 3/4] feature/manager: ensure all feature sets properly set deps --- feature/manager.go | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/feature/manager.go b/feature/manager.go index f201cad1..6d9cfe07 100644 --- a/feature/manager.go +++ b/feature/manager.go @@ -63,14 +63,27 @@ func newManager(cfg Config, desc setDesc) (*Manager, error) { } // Now, remove any features as directed by the config. - for _, fv := range fsets { + for set, raw := range fsets { if cfg.NoTLVOnion { - fv.Unset(lnwire.TLVOnionPayloadOptional) - fv.Unset(lnwire.TLVOnionPayloadRequired) + raw.Unset(lnwire.TLVOnionPayloadOptional) + raw.Unset(lnwire.TLVOnionPayloadRequired) + raw.Unset(lnwire.PaymentAddrOptional) + raw.Unset(lnwire.PaymentAddrRequired) + raw.Unset(lnwire.MPPOptional) + raw.Unset(lnwire.MPPRequired) } if cfg.NoStaticRemoteKey { - fv.Unset(lnwire.StaticRemoteKeyOptional) - fv.Unset(lnwire.StaticRemoteKeyRequired) + raw.Unset(lnwire.StaticRemoteKeyOptional) + raw.Unset(lnwire.StaticRemoteKeyRequired) + } + + // Ensure that all of our feature sets properly set any + // dependent features. + fv := lnwire.NewFeatureVector(raw, lnwire.Features) + err := ValidateDeps(fv) + if err != nil { + return nil, fmt.Errorf("invalid feature set %v: %v", + set, err) } } From 34fd27280ad71b830dd031e01fac32645a28e409 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 16 Dec 2019 13:06:59 -0800 Subject: [PATCH 4/4] peer: validate remote peer's feature deps --- peer.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/peer.go b/peer.go index be8a1d26..53092edb 100644 --- a/peer.go +++ b/peer.go @@ -22,6 +22,7 @@ import ( "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/contractcourt" + "github.com/lightningnetwork/lnd/feature" "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/lnpeer" "github.com/lightningnetwork/lnd/lnwallet" @@ -2476,6 +2477,14 @@ func (p *peer) handleInitMsg(msg *lnwire.Init) error { return err } + // Ensure the remote party's feature vector contains all transistive + // dependencies. We know ours are are correct since they are validated + // during the feature manager's instantiation. + err = feature.ValidateDeps(p.remoteFeatures) + if err != nil { + return fmt.Errorf("peer set invalid feature vector: %v", err) + } + // Now that we know we understand their requirements, we'll check to // see if they don't support anything that we deem to be mandatory. switch {