Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

htlcswitch: attributable errors #7139

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions channeldb/payments.go
Original file line number Diff line number Diff line change
Expand Up @@ -1176,6 +1176,12 @@ func serializeHop(w io.Writer, h *route.Hop) error {
records = append(records, record.NewMetadataRecord(&h.Metadata))
}

// Signal attributable errors.
if h.AttrError {
attrError := record.NewAttributableError()
records = append(records, attrError.Record())
}

// Final sanity check to absolutely rule out custom records that are not
// custom and write into the standard range.
if err := h.CustomRecords.Validate(); err != nil {
Expand Down Expand Up @@ -1297,6 +1303,13 @@ func deserializeHop(r io.Reader) (*route.Hop, error) {
h.Metadata = metadata
}

attributableErrorType := uint64(record.AttributableErrorOnionType)
if _, ok := tlvMap[attributableErrorType]; ok {
delete(tlvMap, attributableErrorType)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why is this done, to check that all parsed fields have been processed? But it's missing that check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because right below here, the remaining records are stored as custom records. If we wouldn't delete here, the attr error record would also show up as a custom record.


h.AttrError = true
}

h.CustomRecords = tlvMap

return h, nil
Expand Down
5 changes: 5 additions & 0 deletions feature/default_sets.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,9 @@ var defaultSetDesc = setDesc{
SetInit: {}, // I
SetNodeAnn: {}, // N
},
lnwire.AttributableErrorsOptional: {
SetInit: {}, // I
SetNodeAnn: {}, // N
SetInvoice: {}, // 9
},
}
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ replace github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2
// allows us to specify that as an option.
replace google.golang.org/protobuf => github.com/lightninglabs/protobuf-go-hex-display v1.30.0-hex-display

replace github.com/lightningnetwork/lightning-onion => github.com/joostjager/lightning-onion v0.0.0-20230808121011-787ad3d102b0

// If you change this please also update .github/pull_request_template.md and
// docs/INSTALL.md.
go 1.19
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,8 @@ github.com/jessevdk/go-flags v1.4.0 h1:4IU2WS7AumrZ/40jfhf4QVDMsQwqA7VEHozFRrGAR
github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/jonboulle/clockwork v0.2.2 h1:UOGuzwb1PwsrDAObMuhUnj0p5ULPj8V/xJ7Kx9qUBdQ=
github.com/jonboulle/clockwork v0.2.2/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8=
github.com/joostjager/lightning-onion v0.0.0-20230808121011-787ad3d102b0 h1:7WYIggNpUFlFZkcTg3K8EONITymhUnl+B7a6EMsBelI=
github.com/joostjager/lightning-onion v0.0.0-20230808121011-787ad3d102b0/go.mod h1:jXOT3eGidi7oYbmB9LeGZOQLlItWglDVGZwvTcf62Wk=
github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4=
github.com/jrick/logrotate v1.0.0 h1:lQ1bL/n9mBNeIXoTUoYRlK4dHuNJVofX9oWqBtPnSzI=
github.com/jrick/logrotate v1.0.0/go.mod h1:LNinyqDIJnpAur+b8yyulnQw/wDuN1+BYKlTRt3OuAQ=
Expand Down Expand Up @@ -440,8 +442,6 @@ github.com/lightninglabs/neutrino/cache v1.1.1 h1:TllWOSlkABhpgbWJfzsrdUaDH2fBy/
github.com/lightninglabs/neutrino/cache v1.1.1/go.mod h1:XJNcgdOw1LQnanGjw8Vj44CvguYA25IMKjWFZczwZuo=
github.com/lightninglabs/protobuf-go-hex-display v1.30.0-hex-display h1:pRdza2wleRN1L2fJXd6ZoQ9ZegVFTAb2bOQfruJPKcY=
github.com/lightninglabs/protobuf-go-hex-display v1.30.0-hex-display/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
github.com/lightningnetwork/lightning-onion v1.2.1-0.20221202012345-ca23184850a1 h1:Wm0g70gkcAu2pGpNZwfWPSVOY21j8IyYsNewwK4OkT4=
github.com/lightningnetwork/lightning-onion v1.2.1-0.20221202012345-ca23184850a1/go.mod h1:7dDx73ApjEZA0kcknI799m2O5kkpfg4/gr7N092ojNo=
github.com/lightningnetwork/lnd/cert v1.2.1 h1:CTrTcU0L66J73oqdRLVfNylZyp1Fh97ZezX6IuzkrqE=
github.com/lightningnetwork/lnd/cert v1.2.1/go.mod h1:04JhIEodoR6usBN5+XBRtLEEmEHsclLi0tEyxZQNP+w=
github.com/lightningnetwork/lnd/clock v1.0.1/go.mod h1:KnQudQ6w0IAMZi1SgvecLZQZ43ra2vpDNj7H/aasemg=
Expand Down
2 changes: 1 addition & 1 deletion htlcswitch/circuit.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (c *PaymentCircuit) Decode(r io.Reader) error {

case hop.EncrypterTypeSphinx:
// Sphinx encrypter was used as this is a forwarded HTLC.
c.ErrorEncrypter = hop.NewSphinxErrorEncrypter()
c.ErrorEncrypter = hop.NewSphinxErrorEncrypterUninitialized()

case hop.EncrypterTypeMock:
// Test encrypter.
Expand Down
6 changes: 3 additions & 3 deletions htlcswitch/circuit_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,9 @@ type CircuitMapConfig struct {
FetchClosedChannels func(
pendingOnly bool) ([]*channeldb.ChannelCloseSummary, error)

// ExtractErrorEncrypter derives the shared secret used to encrypt
// ExtractSharedSecret derives the shared secret used to encrypt
// errors from the obfuscator's ephemeral public key.
ExtractErrorEncrypter hop.ErrorEncrypterExtracter
ExtractSharedSecret hop.SharedSecretGenerator

// CheckResolutionMsg checks whether a given resolution message exists
// for the passed CircuitKey.
Expand Down Expand Up @@ -633,7 +633,7 @@ func (cm *circuitMap) decodeCircuit(v []byte) (*PaymentCircuit, error) {
// Otherwise, we need to reextract the encrypter, so that the shared
// secret is rederived from what was decoded.
err := circuit.ErrorEncrypter.Reextract(
cm.cfg.ExtractErrorEncrypter,
cm.cfg.ExtractSharedSecret,
)
if err != nil {
return nil, err
Expand Down
49 changes: 33 additions & 16 deletions htlcswitch/circuit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ var (
// testExtracter is a precomputed extraction of testEphemeralKey, using
// the sphinxPrivKey.
testExtracter *hop.SphinxErrorEncrypter

// testAttributableExtracter is a precomputed extraction of
// testEphemeralKey, using the sphinxPrivKey.
testAttributableExtracter *hop.SphinxErrorEncrypter
)

func init() {
Expand Down Expand Up @@ -66,20 +70,25 @@ func initTestExtracter() {
onionProcessor := newOnionProcessor(nil)
defer onionProcessor.Stop()

obfuscator, _ := onionProcessor.ExtractErrorEncrypter(
sharedSecret, failCode := onionProcessor.ExtractSharedSecret(
testEphemeralKey,
)

sphinxExtracter, ok := obfuscator.(*hop.SphinxErrorEncrypter)
if !ok {
panic("did not extract sphinx error encrypter")
if failCode != lnwire.CodeNone {
panic("did not extract shared secret")
}

testExtracter = sphinxExtracter
testExtracter = hop.NewSphinxErrorEncrypter(
testEphemeralKey, sharedSecret, false,
)

testAttributableExtracter = hop.NewSphinxErrorEncrypter(
testEphemeralKey, sharedSecret, true,
)

// We also set this error extracter on startup, otherwise it will be nil
// at compile-time.
halfCircuitTests[2].encrypter = testExtracter
halfCircuitTests[3].encrypter = testAttributableExtracter
}

// newOnionProcessor creates starts a new htlcswitch.OnionProcessor using a temp
Expand Down Expand Up @@ -107,10 +116,10 @@ func newCircuitMap(t *testing.T, resMsg bool) (*htlcswitch.CircuitMapConfig,

db := makeCircuitDB(t, "")
circuitMapCfg := &htlcswitch.CircuitMapConfig{
DB: db,
FetchAllOpenChannels: db.ChannelStateDB().FetchAllOpenChannels,
FetchClosedChannels: db.ChannelStateDB().FetchClosedChannels,
ExtractErrorEncrypter: onionProcessor.ExtractErrorEncrypter,
DB: db,
FetchAllOpenChannels: db.ChannelStateDB().FetchAllOpenChannels,
FetchClosedChannels: db.ChannelStateDB().FetchClosedChannels,
ExtractSharedSecret: onionProcessor.ExtractSharedSecret,
}

if resMsg {
Expand Down Expand Up @@ -175,6 +184,14 @@ var halfCircuitTests = []struct {
// repopulate this encrypter.
encrypter: testExtracter,
},
{
hash: hash3,
inValue: 10000,
outValue: 9000,
chanID: lnwire.NewShortChanIDFromInt(3),
htlcID: 3,
encrypter: testAttributableExtracter,
},
}

// TestHalfCircuitSerialization checks that the half circuits can be properly
Expand Down Expand Up @@ -217,7 +234,7 @@ func TestHalfCircuitSerialization(t *testing.T) {
// encrypters, this will be a NOP.
if circuit2.ErrorEncrypter != nil {
err := circuit2.ErrorEncrypter.Reextract(
onionProcessor.ExtractErrorEncrypter,
onionProcessor.ExtractSharedSecret,
)
if err != nil {
t.Fatalf("unable to reextract sphinx error "+
Expand Down Expand Up @@ -646,11 +663,11 @@ func restartCircuitMap(t *testing.T, cfg *htlcswitch.CircuitMapConfig) (
// Reinitialize circuit map with same db path.
db := makeCircuitDB(t, dbPath)
cfg2 := &htlcswitch.CircuitMapConfig{
DB: db,
FetchAllOpenChannels: db.ChannelStateDB().FetchAllOpenChannels,
FetchClosedChannels: db.ChannelStateDB().FetchClosedChannels,
ExtractErrorEncrypter: cfg.ExtractErrorEncrypter,
CheckResolutionMsg: cfg.CheckResolutionMsg,
DB: db,
FetchAllOpenChannels: db.ChannelStateDB().FetchAllOpenChannels,
FetchClosedChannels: db.ChannelStateDB().FetchClosedChannels,
ExtractSharedSecret: cfg.ExtractSharedSecret,
CheckResolutionMsg: cfg.CheckResolutionMsg,
}
cm2, err := htlcswitch.NewCircuitMap(cfg2)
require.NoError(t, err, "unable to recreate persistent circuit map")
Expand Down
63 changes: 59 additions & 4 deletions htlcswitch/failure.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ package htlcswitch

import (
"bytes"
"encoding/binary"
"fmt"
"strings"

sphinx "github.com/lightningnetwork/lightning-onion"
"github.com/lightningnetwork/lnd/htlcswitch/hop"
"github.com/lightningnetwork/lnd/lnwire"
)

var byteOrder = binary.BigEndian

// ClearTextError is an interface which is implemented by errors that occur
// when we know the underlying wire failure message. These errors are the
// opposite to opaque errors which are onion-encrypted blobs only understandable
Expand Down Expand Up @@ -166,7 +170,25 @@ type OnionErrorDecrypter interface {
// SphinxErrorDecrypter wraps the sphinx data SphinxErrorDecrypter and maps the
// returned errors to concrete lnwire.FailureMessage instances.
type SphinxErrorDecrypter struct {
OnionErrorDecrypter
decrypter interface{}
}

// NewSphinxErrorDecrypter instantiates a new error decryptor.
func NewSphinxErrorDecrypter(circuit *sphinx.Circuit,
attrError bool) *SphinxErrorDecrypter {

var decrypter interface{}
if !attrError {
decrypter = sphinx.NewOnionErrorDecrypter(circuit)
} else {
decrypter = sphinx.NewOnionAttrErrorDecrypter(
circuit, hop.AttrErrorStruct,
)
}

return &SphinxErrorDecrypter{
decrypter: decrypter,
}
}

// DecryptError peels off each layer of onion encryption from the first hop, to
Expand All @@ -177,9 +199,42 @@ type SphinxErrorDecrypter struct {
func (s *SphinxErrorDecrypter) DecryptError(reason lnwire.OpaqueReason) (
*ForwardingError, error) {

failure, err := s.OnionErrorDecrypter.DecryptError(reason)
if err != nil {
return nil, err
var failure *sphinx.DecryptedError

switch decrypter := s.decrypter.(type) {
case OnionErrorDecrypter:
legacyError, err := decrypter.DecryptError(reason)
if err != nil {
return nil, err
}

failure = legacyError

case *sphinx.OnionAttrErrorDecrypter:
attributableError, err := decrypter.DecryptError(reason)
if err != nil {
return nil, err
}

// Log hold times.
//
// TODO: Use to penalize nodes.
var holdTimes []string
for _, payload := range attributableError.Payloads {
// Read hold time.
holdTimeMs := byteOrder.Uint32(payload)

holdTimes = append(
holdTimes,
fmt.Sprintf("%v", holdTimeMs),
)
}
log.Debugf("Hold times: %v", strings.Join(holdTimes, "/"))

failure = &attributableError.DecryptedError

default:
panic("unexpected decrypter type")
}

// Decode the failure. If an error occurs, we leave the failure message
Expand Down
2 changes: 1 addition & 1 deletion htlcswitch/failure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestLongFailureMessage(t *testing.T) {
}

errorDecryptor := &SphinxErrorDecrypter{
OnionErrorDecrypter: sphinx.NewOnionErrorDecrypter(circuit),
decrypter: sphinx.NewOnionErrorDecrypter(circuit),
}

// Assert that the failure message can still be extracted.
Expand Down
Loading