Skip to content

Fix version error report race: write directly before disconnect #167

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.

Sign up for GitHub

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

Open
lance0 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix version error report race: write directly before disconnect #167

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions lib/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"net/http"
"net/netip"
"sync"
"time"

"golang.org/x/crypto/ssh"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -734,7 +735,18 @@ func (c *Client) checkVersion(newversion uint8) error {
if c.log != nil {
c.log.Debugf("%v: has bad version (received: v%v, current: v%v) error", c.String(), newversion, c.version)
}
// Send the Error Report through the normal channel, then give
// sendLoop time to flush it before we disconnect. The error
// report must use the server's highest supported version so the
// client knows what to downgrade to (RFC 8210 §7).
if !c.versionset {
c.SetVersion(PROTOCOL_VERSION_1)
}
c.SendWrongVersionError()
// Brief yield to let sendLoop drain the error report. Not
// guaranteed, but materially reduces the race window vs the
// previous immediate-disconnect path.
time.Sleep(50 * time.Millisecond)
c.Disconnect()
return fmt.Errorf("%v: has bad version (received: v%v, current: v%v)", c.String(), newversion, c.version)
}
Expand Down Expand Up @@ -805,7 +817,11 @@ func (c *Client) readLoop(ctx context.Context) error {
if c.log != nil {
c.log.Debugf("Bad version error")
}
// c.version already holds the server's enforced version
// (set at connection accept time), so SendWrongVersionError
// stamps the correct version for the client to downgrade to.
c.SendWrongVersionError()
time.Sleep(50 * time.Millisecond)
c.Disconnect()
return fmt.Errorf("%s: bad version error", c.String())
}
Expand Down