From 0dd9ef028b67461ea2f15154c93c5349ead62484 Mon Sep 17 00:00:00 2001 From: Kleissner Date: Tue, 20 Apr 2021 17:20:55 +0200 Subject: [PATCH] Fix race condition in sequence validation and invalidation. Once they are now checked, they can be immediately invalidated. --- Message Sequence.go | 9 ++++++--- Network.go | 24 ++++++++++++++---------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/Message Sequence.go b/Message Sequence.go index 41da2e4..91a8a22 100644 --- a/Message Sequence.go +++ b/Message Sequence.go @@ -91,7 +91,7 @@ func msgArbitrarySequence(publicKey *btcec.PublicKey) (sequence uint32) { } // msgValidateSequence validates the sequence number of an incoming message -func msgValidateSequence(raw *MessageRaw) (valid bool, rtt time.Duration) { +func msgValidateSequence(raw *MessageRaw, invalidate bool) (valid bool, rtt time.Duration) { // Only Response and Pong if raw.Command != CommandResponse && raw.Command != CommandPong { return true, rtt @@ -116,8 +116,11 @@ func msgValidateSequence(raw *MessageRaw) (valid bool, rtt time.Duration) { sequence.counter++ - // Special case CommandResponse: Extend validity in case there are follow-up responses by half of the round-trip time since they will be sent one-way. - if raw.Command == CommandResponse { + // invalidate the sequence immediately? + if invalidate { + delete(sequences, key) + } else if raw.Command == CommandResponse { + // Special case CommandResponse: Extend validity in case there are follow-up responses by half of the round-trip time since they will be sent one-way. sequence.expires = time.Now().Add(time.Duration(ReplyTimeout) * time.Second / 2) } diff --git a/Network.go b/Network.go index 0f507c6..6494b9d 100644 --- a/Network.go +++ b/Network.go @@ -158,14 +158,6 @@ func packetWorker(packets <-chan networkWire) { // process the packet raw := &MessageRaw{SenderPublicKey: senderPublicKey, PacketRaw: *decoded, connection: connection} - // Response, Pong: Validate sequence number which prevents unsolicited responses. - if valid, rtt := msgValidateSequence(raw); !valid { - log.Printf("packetWorker message with invalid sequence %d command %d from %s\n", raw.Sequence, raw.Command, raw.connection.Address.String()) // Only log for debug purposes. - continue - } else if rtt > 0 { - connection.RoundTripTime = rtt - } - switch decoded.Command { case CommandAnnouncement: // Announce if announce, _ := msgDecodeAnnouncement(raw); announce != nil { @@ -174,8 +166,12 @@ func packetWorker(packets <-chan networkWire) { case CommandResponse: // Response if response, _ := msgDecodeResponse(raw); response != nil { - if (response.Actions & (1 << ActionSequenceLast)) > 0 { - msgInvalidateSequence(raw) + // Validate sequence number which prevents unsolicited responses. + if valid, rtt := msgValidateSequence(raw, response.Actions&(1< 0); !valid { + log.Printf("packetWorker message with invalid sequence %d command %d from %s\n", raw.Sequence, raw.Command, raw.connection.Address.String()) // Only log for debug purposes. + continue + } else if rtt > 0 { + connection.RoundTripTime = rtt } peer.cmdResponse(response) @@ -190,6 +186,14 @@ func packetWorker(packets <-chan networkWire) { peer.cmdPing(raw) case CommandPong: // Ping + // Validate sequence number which prevents unsolicited responses. + if valid, rtt := msgValidateSequence(raw, true); !valid { + log.Printf("packetWorker message with invalid sequence %d command %d from %s\n", raw.Sequence, raw.Command, raw.connection.Address.String()) // Only log for debug purposes. + continue + } else if rtt > 0 { + connection.RoundTripTime = rtt + } + peer.cmdPong(raw) case CommandChat: // Chat [debug]