golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.06k stars 17.68k forks source link

x/crypto/ssh: add support for alive interval #19338

Open dsnet opened 7 years ago

dsnet commented 7 years ago

When the underlying TCP connection for a long-standing SSH connection abruptly dies, operations on an ssh.Client can hang forever. This is because the client remains stuck in an io.ReadFull call with no preceding or concurrent calls to net.Conn.SetDeadline. Without any deadline set, there is no guarantee that Read will ever return.

Even though the user has access the underlying net.Conn and can set the deadline themselves, they have no way of determining if the underlying connection is actually dead or just idle. Thus, the ssh package should support this functionality that allows sending of empty messages to intentionally invoke a response from the remote endpoint, in order to determine if it is still alive.

This is a feature request for the equivalent options for OpenSSH:

If this is reasonable, I can implement this.

dsnet commented 7 years ago

\cc @hanwen

hanwen commented 7 years ago

you can trivially do this yourself by sending a message in a loop on a timer.

nhooyr commented 7 years ago

@hanwen see https://github.com/golang/go/issues/21478 for why that's not efficient.

georgmu commented 5 years ago

I have a case where I do not have access to the connection, since the hang happened during Dial:

goroutine 29 [chan receive, 18 minutes]:
golang.org/x/crypto/ssh.(*handshakeTransport).readPacket(...)
        golang.org/x/crypto/ssh/handshake.go:187 +0x4e
golang.org/x/crypto/ssh.confirmKeyAck(...)
        golang.org/x/crypto/ssh/client_auth.go:289 +0xaf
golang.org/x/crypto/ssh.validateKey(...)
        golang.org/x/crypto/ssh/client_auth.go:281 +0x213
golang.org/x/crypto/ssh.publicKeyCallback.auth(...)
        golang.org/x/crypto/ssh/client_auth.go:202 +0x127
golang.org/x/crypto/ssh.(*connection).clientAuthenticate(...)
        golang.org/x/crypto/ssh/client_auth.go:44 +0x31f
golang.org/x/crypto/ssh.(*connection).clientHandshake(...)
        golang.org/x/crypto/ssh/client.go:113 +0x2b4
golang.org/x/crypto/ssh.NewClientConn(...)
        golang.org/x/crypto/ssh/client.go:83 +0xf8
golang.org/x/crypto/ssh.Dial(...)
        golang.org/x/crypto/ssh/client.go:177 +0xb3
georgmu commented 5 years ago

This could solve it for the Dial case:

 func Dial(network, addr string, config *ClientConfig) (*Client, error) {
        conn, err := net.DialTimeout(network, addr, config.Timeout)
        if err != nil {
                return nil, err
        }
+       if config.Timeout > 0 {
+               conn.SetDeadline(time.Now().Add(config.Timeout))
+       }
        c, chans, reqs, err := NewClientConn(conn, addr, config)
        if err != nil {
                return nil, err
        }
+       if config.Timeout > 0 {
+               conn.SetDeadline(time.Time{})
+       }
        return NewClient(c, chans, reqs), nil
 }
shoce commented 2 years ago

As i see this one is stale for few years already. Does it mean you people found some another solution to this problem? If you have one please share with us. Very much interested.

georgmu commented 1 year ago

For my part, I implented a work-around by wrapping the fix from my comment above into a new DialWithDeadline(). If this would make it into the official repo, I could remove this wrapper.