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: timeout needed when requesting session with PTY #36227

Closed
johnsiilver opened this issue Dec 20, 2019 · 4 comments
Closed

x/crypto/ssh: timeout needed when requesting session with PTY #36227

johnsiilver opened this issue Dec 20, 2019 · 4 comments

Comments

@johnsiilver
Copy link

@johnsiilver johnsiilver commented Dec 20, 2019

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

$ go version go1.13.5 darwin/amd64

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
GO111MODULE="off"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/johnsiilver/Library/Caches/go-build"
GOENV="/Users/johnsiilver/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/johnsiilver/golang"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/44/fghj74fs5_j0mkwqnqs2r9c00000gn/T/go-build248366933=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I am doing two things:

  • Faking an SSH server
  • Using goexpect with the SSH client to get into the server

My server does not provide PTYs and ignores requests for them, only responding to "shell".

I was attempting to use goexpect with goexpect.SpawnSSH() to interact with my fake SSH server, which is pretending to be a router from a vendor.

goexpect.SpawnSSH() calls the ssh.Session.RequestPty()
That in turn calls: s.ch.SendRequest()
Which is a private "channel" type in channel.go that implements Channel, which has a .SendRequest() method

In channel.go around line 594 there is a statement:
m, ok := (<-ch.msg)

This doesn't return.

So what happens is that the client locks up without any indication of what is going on.

I don't know the best way to handle this (context for RequestPty() (probably too late) ) or just a timeout waiting for this based on some value like 30 * time.Second.

An example is included here:

https://play.golang.org/p/jLTIkNRWjLH

What did you expect to see?

Some type of error if the PTY could not be granted. Clients should not break because the server is broken.

What did you see instead?

The program blocks forever, no indication of what is going on.

@gopherbot gopherbot added this to the Unreleased milestone Dec 20, 2019
@dmitshur dmitshur changed the title x/crypto/ssh - Timeout needed when requesting session with PTY x/crypto/ssh: timeout needed when requesting session with PTY Dec 20, 2019
@dmitshur
Copy link
Member

@dmitshur dmitshur commented Dec 20, 2019

@hanwen
Copy link
Contributor

@hanwen hanwen commented Jan 7, 2020

https://play.golang.org/p/jLTIkNRWjLH line 177

of your server is ignoring incoming requests. you should discard them explicitly.

Did you copy & paste this example from somewhere?

@hanwen
Copy link
Contributor

@hanwen hanwen commented Jan 7, 2020

From SSH perspective, this is working as expected.

There is no room in the protocol to react a SendRequest call not getting a response. If you want to do something with timeouts, you can kill the entire connection if it doesn't complete your work within a set time.

@hanwen hanwen closed this Jan 7, 2020
@johnsiilver
Copy link
Author

@johnsiilver johnsiilver commented Jan 7, 2020

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.