code review 6099050: ssh: handle bad servers better.

ssh: handle bad servers better.

This change prevents bad servers from crashing a client by sending an
invalid channel ID. It also makes the client disconnect in more cases
of invalid messages from a server and cleans up the client channels
in the event of a disconnect.

[agl1, 2012-04-23 22:29:48]

Hello dave@cheney.net (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://agl%40golang.org@code.google.com/p/go.crypto/

[agl1, 2012-04-23 22:30:22]

http://codereview.appspot.com/6099050/diff/2001/ssh/client.go
File ssh/client.go (right):

http://codereview.appspot.com/6099050/diff/2001/ssh/client.go#newcode276
ssh/client.go:276: return
It wasn't clear here whether you wanted to tear down the connection, or whether
it's best to ignore disconnect messages from the server.

[dave_cheney.net, 2012-04-24 09:04:27]

LGTM. Thank you for closing this long standing TODO.

http://codereview.appspot.com/6099050/diff/2001/ssh/client.go
File ssh/client.go (right):

http://codereview.appspot.com/6099050/diff/2001/ssh/client.go#newcode276
ssh/client.go:276: return
RFC 4254 s 11 says all impls MUST accept and process this message. Certainly it
could be cleaner, but closeAll gets the job done. A future enhancement might be
to place the disconnection message text into an error and pass it back to any
readers or writers of a channel. 

On 2012/04/23 22:30:22, agl1 wrote:
> It wasn't clear here whether you wanted to tear down the connection, or
whether
> it's best to ignore disconnect messages from the server.

http://codereview.appspot.com/6099050/diff/4001/ssh/client.go
File ssh/client.go (right):

http://codereview.appspot.com/6099050/diff/4001/ssh/client.go#newcode187
ssh/client.go:187: defer func() {
Nice use of the defer.

http://codereview.appspot.com/6099050/diff/4001/ssh/client.go#newcode437
ssh/client.go:437: func (c *chanlist) closeAll() {
Thanks for implementing this.

[agl1, 2012-04-24 17:46:29]

*** Submitted as
http://code.google.com/p/go/source/detail?r=af839935547f&repo=crypto ***

ssh: handle bad servers better.

This change prevents bad servers from crashing a client by sending an
invalid channel ID. It also makes the client disconnect in more cases
of invalid messages from a server and cleans up the client channels
in the event of a disconnect.

R=dave
CC=golang-dev
http://codereview.appspot.com/6099050