From c553ad5158a04c16df863bea7f0ddde741aada19 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 5 Jan 2021 17:14:40 +0000 Subject: [PATCH] serve sftp: fix authentication on one connection blocking others - fixes #4882 Before this change, if one connection was authenticating this would block any others from authenticating. This was due to ssh.NewServerConn not being called in a go routine after the Accept call. This is fixed by running the ssh authentication in a go routine. Thanks to @FiloSottile for advice on how to fix this. See: https://github.com/golang/go/issues/43521 --- cmd/serve/sftp/server.go | 61 ++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/cmd/serve/sftp/server.go b/cmd/serve/sftp/server.go index a617cf2ad..278630851 100644 --- a/cmd/serve/sftp/server.go +++ b/cmd/serve/sftp/server.go @@ -78,6 +78,39 @@ func (s *server) getVFS(what string, sshConn *ssh.ServerConn) (VFS *vfs.VFS) { return VFS } +// Accept a single connection - run in a go routine as the ssh +// authentication can block +func (s *server) acceptConnection(nConn net.Conn) { + what := describeConn(nConn) + + // Before use, a handshake must be performed on the incoming net.Conn. + sshConn, chans, reqs, err := ssh.NewServerConn(nConn, s.config) + if err != nil { + fs.Errorf(what, "SSH login failed: %v", err) + return + } + + fs.Infof(what, "SSH login from %s using %s", sshConn.User(), sshConn.ClientVersion()) + + // Discard all global out-of-band Requests + go ssh.DiscardRequests(reqs) + + c := &conn{ + what: what, + vfs: s.getVFS(what, sshConn), + } + if c.vfs == nil { + fs.Infof(what, "Closing unauthenticated connection (couldn't find VFS)") + _ = nConn.Close() + return + } + c.handlers = newVFSHandler(c.vfs) + + // Accept all channels + go c.handleChannels(chans) +} + +// Accept connections and call them in a go routine func (s *server) acceptConnections() { for { nConn, err := s.listener.Accept() @@ -88,33 +121,7 @@ func (s *server) acceptConnections() { fs.Errorf(nil, "Failed to accept incoming connection: %v", err) continue } - what := describeConn(nConn) - - // Before use, a handshake must be performed on the incoming net.Conn. - sshConn, chans, reqs, err := ssh.NewServerConn(nConn, s.config) - if err != nil { - fs.Errorf(what, "SSH login failed: %v", err) - continue - } - - fs.Infof(what, "SSH login from %s using %s", sshConn.User(), sshConn.ClientVersion()) - - // Discard all global out-of-band Requests - go ssh.DiscardRequests(reqs) - - c := &conn{ - what: what, - vfs: s.getVFS(what, sshConn), - } - if c.vfs == nil { - fs.Infof(what, "Closing unauthenticated connection (couldn't find VFS)") - _ = nConn.Close() - continue - } - c.handlers = newVFSHandler(c.vfs) - - // Accept all channels - go c.handleChannels(chans) + go s.acceptConnection(nConn) } }