Skip to content
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.

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

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

Open
dsnet opened this issue Mar 1, 2017 · 5 comments
Open

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

dsnet opened this issue Mar 1, 2017 · 5 comments
Milestone

Comments

@dsnet
Copy link
Member

@dsnet dsnet commented Mar 1, 2017

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:

  • AliveInterval time.Duration: Sets a timeout interval after which, if no data has been received, sends an alive message through the encrypted channel to invoke a response from the remote end. The default is 0, indicating that such messages will not be sent.
  • AliveCountMax int: Sets the number of alive messages which may be sent without receiving a response from the remote end. If this threshold is reached while alive messages are being sent, the SSH session will be terminated.

If this is reasonable, I can implement this.

@dsnet dsnet added this to the Unreleased milestone Mar 1, 2017
@dsnet
Copy link
Member Author

@dsnet dsnet commented Mar 1, 2017

\cc @hanwen

@hanwen
Copy link
Contributor

@hanwen hanwen commented Mar 1, 2017

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

@nhooyr
Copy link
Contributor

@nhooyr nhooyr commented Aug 16, 2017

@hanwen see #21478 for why that's not efficient.

@georgmu
Copy link

@georgmu georgmu commented Oct 7, 2019

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
Copy link

@georgmu georgmu commented Oct 7, 2019

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
 }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.