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.

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

Jump to bottom

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

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

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

dsnet opened this issue Mar 1, 2017 · 7 comments
Milestone
Unreleased

Comments

@dsnet
Copy link
Member

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 commented Mar 1, 2017

\cc @hanwen

@hanwen
Copy link
Contributor

hanwen commented Mar 1, 2017

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

@dsnet dsnet mentioned this issue Aug 16, 2017
Closed
@nhooyr
Copy link
Contributor

nhooyr commented Aug 16, 2017

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

@georgmu
Copy link

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 commented Oct 7, 2019 •
edited

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

shoce commented Nov 11, 2022

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

georgmu commented Dec 14, 2022 •
edited

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.

@leehinman leehinman mentioned this issue Mar 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@hanwen @shoce @georgmu @dsnet @nhooyr