Skip to content

Commit

Permalink
p2p: Move check for getheaders locators earlier in the call
Browse files Browse the repository at this point in the history
This moves the check that ensures that a reply to a getheaders connects
to the existing locators ealier in the call stack for processing a
headers response.

This ensures the wallet disconnects from misbehaving peers before
processing block hashes.
  • Loading branch information
matheusd committed Aug 29, 2024
1 parent 68ed4d3 commit 08c05e9
Showing 1 changed file with 32 additions and 27 deletions.
59 changes: 32 additions & 27 deletions p2p/peering.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,10 @@ type RemotePeer struct {

// headers message management. Headers can either be fetched synchronously
// or used to push block notifications with sendheaders.
requestedHeaders chan<- *wire.MsgHeaders // non-nil result chan when synchronous getheaders in process
sendheaders bool // whether a sendheaders message was sent
requestedHeadersMu sync.Mutex
requestedHeaders chan<- *wire.MsgHeaders // non-nil result chan when synchronous getheaders in process
requestedHeadersLoc []*chainhash.Hash // non-nil when requested headers with getheaders
sendheaders bool // whether a sendheaders message was sent
requestedHeadersMu sync.Mutex

// init state message management.
requestedInitState chan<- *wire.MsgInitState // non-nil result chan when synchronous getinitstate in process
Expand Down Expand Up @@ -1097,7 +1098,7 @@ func (rp *RemotePeer) receivedCFilterV2(ctx context.Context, msg *wire.MsgCFilte
}
}

func (rp *RemotePeer) addRequestedHeaders(c chan<- *wire.MsgHeaders) (sendheaders, newRequest bool) {
func (rp *RemotePeer) addRequestedHeaders(c chan<- *wire.MsgHeaders, loc []*chainhash.Hash) (sendheaders, newRequest bool) {
rp.requestedHeadersMu.Lock()
if rp.sendheaders {
rp.requestedHeadersMu.Unlock()
Expand All @@ -1108,13 +1109,15 @@ func (rp *RemotePeer) addRequestedHeaders(c chan<- *wire.MsgHeaders) (sendheader
return false, false
}
rp.requestedHeaders = c
rp.requestedHeadersLoc = loc
rp.requestedHeadersMu.Unlock()
return false, true
}

func (rp *RemotePeer) deleteRequestedHeaders() {
rp.requestedHeadersMu.Lock()
rp.requestedHeaders = nil
rp.requestedHeadersLoc = nil
rp.requestedHeadersMu.Unlock()
}

Expand Down Expand Up @@ -1146,6 +1149,29 @@ func (rp *RemotePeer) receivedHeaders(ctx context.Context, msg *wire.MsgHeaders)
return
}

// The parent of the first header (if there is one) MUST be one of the
// block locators we used to request headers from the peer when this
// is a response to a getheaders request.
if len(msg.Headers) > 0 && rp.requestedHeadersLoc != nil {
wantParent := msg.Headers[0].PrevBlock
contains := false
for _, loc := range rp.requestedHeadersLoc {
if *loc == wantParent {
contains = true
break
}
}
if !contains {
op := errors.Opf(opf, rp.raddr)
err := errors.E(op, errors.Protocol,
"peer sent headers that do not connect "+
"to block locators")
rp.Disconnect(err)
rp.requestedHeadersMu.Unlock()
return
}
}

// Sanity check the headers connect to each other in sequence.
var prevHash chainhash.Hash
var prevHeight uint32
Expand Down Expand Up @@ -1186,6 +1212,7 @@ func (rp *RemotePeer) receivedHeaders(ctx context.Context, msg *wire.MsgHeaders)
// Headers as a response to getheaders.
c := rp.requestedHeaders
rp.requestedHeaders = nil
rp.requestedHeadersLoc = nil
rp.requestedHeadersMu.Unlock()
select {
case <-ctx.Done():
Expand Down Expand Up @@ -1988,7 +2015,7 @@ func (rp *RemotePeer) Headers(ctx context.Context, blockLocators []*chainhash.Ha
HashStop: *hashStop,
}
c := make(chan *wire.MsgHeaders, 1)
sendheaders, newRequest := rp.addRequestedHeaders(c)
sendheaders, newRequest := rp.addRequestedHeaders(c, blockLocators)
if sendheaders {
op := errors.Opf(opf, rp.raddr)
return nil, errors.E(op, errors.Invalid, "synchronous getheaders after sendheaders is unsupported")
Expand Down Expand Up @@ -2020,28 +2047,6 @@ func (rp *RemotePeer) Headers(ctx context.Context, blockLocators []*chainhash.Ha
case m := <-c:
stalled.Stop()

// The parent of the first header (if there is one) MUST
// be one of the block locators we used to request
// headers from the peer.
if len(m.Headers) > 0 {
wantParent := m.Headers[0].PrevBlock
contains := false
for _, loc := range blockLocators {
if *loc == wantParent {
contains = true
break
}
}
if !contains {
op := errors.Opf(opf, rp.raddr)
err := errors.E(op, errors.Protocol,
"peer sent headers that do not connect "+
"to block locators")
rp.Disconnect(err)
return nil, err
}
}

return m.Headers, nil
}
}
Expand Down

0 comments on commit 08c05e9

Please sign in to comment.