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: session remains open if Close is called without sending data #31401

Closed
ciys opened this issue Apr 11, 2019 · 5 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@ciys
Copy link

ciys commented Apr 11, 2019

What version of Go are you using (go version)?

$ go version
1.11.2

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
linux centos and windows

What did you do?

I create a new ssh client,dial and run is ok,but when i close the client i got the EOF error.
I set the debugMux is true to log the ssh detail and found that the server always send the msgChannelClose msg after the commond is successly run.
But the client only set sentClose true,not do anything.So when i call the close method it result an EOF error,but the client didnot send close msg to the server.And use the linux commond

lsof -c process name

the ssh connection is still alive. It results after a long time ,my process cannot open any file because of "too many files".
I tried to notes the code if ch.sentClose { ch.writeMu.Unlock() return io.EOF },and it can work.

What did you expect to see?

Is that a bug?

What did you see instead?

@agnivade
Copy link
Contributor

@hanwen

@bcmills
Copy link
Contributor

bcmills commented Apr 11, 2019

I create a new ssh client

How? (If you're having a problem with a specific package, please provide specific steps that we can follow exactly to reproduce the problem with that package.)

@bcmills bcmills added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. Question labels Apr 11, 2019
@ciys
Copy link
Author

ciys commented Apr 12, 2019

@bcmills ,ok,like this:

import (
	"golang.org/x/crypto/ssh"
	"time"
	"net"
	"fmt"
)

func main() {
	sess, err := Sshconnect("xxx", "xxx", "170.0.44.1", 22, 10)
	fmt.Println("open :", err)
	err = sess.Run("pwd")
	fmt.Println("run :", err)
	err = sess.Close()
	fmt.Println("close :", err)
	time.Sleep(time.Minute)
}

func Sshconnect(user, password, host string, port int, timeout int) (*ssh.Session, error) {
	var (
		auth         []ssh.AuthMethod
		addr         string
		clientConfig *ssh.ClientConfig
		client       *ssh.Client
		session      *ssh.Session
		err          error
	)
	auth = make([]ssh.AuthMethod, 0)
	auth = append(auth, ssh.Password(password))
	clientConfig = &ssh.ClientConfig{
		User:    user,
		Auth:    auth,
		Timeout: time.Duration(timeout) * time.Second,
		HostKeyCallback: func(hostname string, remote net.Addr, key ssh.PublicKey) error {
			return nil
		},
	}
	// connet to ssh
	addr = fmt.Sprintf("%s:%d", host, port)
	if client, err = ssh.Dial("tcp", addr, clientConfig); err != nil {
		return nil, err
	}
	// create session
	if session, err = client.NewSession(); err != nil {
		return nil, err
	}
	return session, nil
}

@bcmills bcmills changed the title ssh client connection close EOF x/crypto/ssh: ssh client connection close EOF Apr 12, 2019
@gopherbot gopherbot added this to the Unreleased milestone Apr 12, 2019
@bcmills bcmills changed the title x/crypto/ssh: ssh client connection close EOF x/crypto/ssh: session remains open if Close is called without sending data Apr 12, 2019
@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed Question WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Apr 12, 2019
@wiml
Copy link

wiml commented Jul 18, 2019

I think that this is probably not a bug, but is tangentially related to #32453 etc.

The error is returned because the session has already been closed (by Run()). The SSH connection leaks because it is never closed — the client object returned from Dial() is discarded, but that's the object that needs to be closed to clean up the underlying TCP connection.

(As an aside, this seems to be a subtle API to use correctly. The session can be closed asynchronously at any time by the other side, and so it's always possible for correctly-written code to get an EOF from Close().)

For reference, turning on the debugMux prints:

2019/07/17 18:31:04 decoding(1): data packet - 21 bytes
2019/07/17 18:31:04 send(1): ssh.windowAdjustMsg{PeersID:0x0, AdditionalBytes:0xc}
2019/07/17 18:31:04 decoding(1): 96 &ssh.channelEOFMsg{PeersID:0x1} - 5 bytes
2019/07/17 18:31:04 decoding(1): 98 &ssh.channelRequestMsg{PeersID:0x1, Request:"exit-status", WantReply:false, RequestSpecificData:[]uint8{0x0, 0x0, 0x0, 0x0}} - 25 bytes
2019/07/17 18:31:04 decoding(1): 97 &ssh.channelCloseMsg{PeersID:0x1} - 5 bytes
2019/07/17 18:31:04 send(1): ssh.channelCloseMsg{PeersID:0x0}
run : <nil>
2019/07/17 18:31:04 send(1): ssh.channelCloseMsg{PeersID:0x0}
close : EOF

@ciys
Copy link
Author

ciys commented Aug 12, 2019

@wiml yes,you are right. I tried to close the client not session, and there is no connection leak.Thank you for your reply!
But i think the api design is not reasonable.

@ciys ciys closed this as completed Aug 13, 2019
@golang golang locked and limited conversation to collaborators Aug 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants