Skip to content

Commit

Permalink
gbn: complete handshake if client has completed it
Browse files Browse the repository at this point in the history
This fixes a bug where the server would not complete the handshake if
the server restarted the handshake after sending it's SYN, which the
client then received and then completed the handshake.
This could previously happen if the server timedout when waiting for the
clients SYN response, or if the client's SYNACK was lost due to packet
loss.
  • Loading branch information
ViktorTigerstrom committed Jan 17, 2024
1 parent 52608a0 commit 6055a75
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions gbn/gbn_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func (g *GoBackNConn) serverHandshake() error { // nolint:gocyclo
var n uint8
var resent bool

handshakeLoop:
for {
g.log.Debugf("Waiting for client SYN")
select {
Expand Down Expand Up @@ -111,6 +112,24 @@ func (g *GoBackNConn) serverHandshake() error { // nolint:gocyclo

switch msg.(type) {
case *PacketSYN:

case *PacketSYNACK, *PacketData:
// If we receive a SYNACK or DATA packet after we have
// restarted the handshake, we can be sure that the
// client has received our SYN and has completed the
// handshake. We can therefore complete the handshake
// ourselves.
if resent {
g.log.Tracef("Received %T after restarting "+
"handshake", msg)
g.timeoutManager.Received(msg)

break handshakeLoop
}

g.log.Tracef("Expected SYN, got %T", msg)

continue
default:
g.log.Tracef("Expected SYN, got %T", msg)
continue
Expand Down Expand Up @@ -169,6 +188,8 @@ func (g *GoBackNConn) serverHandshake() error { // nolint:gocyclo

switch msg.(type) {
case *PacketSYNACK:
g.log.Debugf("Received SYNACK")

// Notify the timeout manager we've received the SYNACK
// response from the counterparty.
g.timeoutManager.Received(msg)
Expand All @@ -185,8 +206,6 @@ func (g *GoBackNConn) serverHandshake() error { // nolint:gocyclo
break
}

g.log.Debugf("Received SYNACK")

// Set all variables that are dependent on the value of N that we get
// from the client
g.setN(n)
Expand Down

0 comments on commit 6055a75

Please sign in to comment.