From 2d394e30250de5bcca4998d72fdf78689b4c85fe Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Fri, 26 Jul 2013 14:37:37 -0400 Subject: [PATCH] go.crypto/ssh: add workaround for broken port forwarding in OpenSSH 5. Tested with OpenSSH_5.9 R=agl, dave CC=golang-dev https://golang.org/cl/11921043 --- ssh/client.go | 3 ++ ssh/tcpip.go | 57 ++++++++++++++++++++++++++++++++--- ssh/tcpip_test.go | 16 ++++++++++ ssh/test/forward_unix_test.go | 30 ++---------------- 4 files changed, 74 insertions(+), 32 deletions(-) create mode 100644 ssh/tcpip_test.go diff --git a/ssh/client.go b/ssh/client.go index a506c77..a97a0fb 100644 --- a/ssh/client.go +++ b/ssh/client.go @@ -29,6 +29,8 @@ type ClientConn struct { // Address as passed to the Dial function. dialAddress string + + serverVersion string } type globalRequest struct { @@ -75,6 +77,7 @@ func (c *ClientConn) handshake() error { return err } magics.serverVersion = version + c.serverVersion = string(version) clientKexInit := kexInitMsg{ KexAlgos: supportedKexAlgos, ServerHostKeyAlgos: supportedHostKeyAlgos, diff --git a/ssh/tcpip.go b/ssh/tcpip.go index ad92b43..5abd690 100644 --- a/ssh/tcpip.go +++ b/ssh/tcpip.go @@ -8,7 +8,10 @@ import ( "errors" "fmt" "io" + "math/rand" "net" + "strconv" + "strings" "sync" "time" ) @@ -32,10 +35,60 @@ type channelForwardMsg struct { rport uint32 } +// Automatic port allocation is broken with OpenSSH before 6.0. See +// also https://bugzilla.mindrot.org/show_bug.cgi?id=2017. In +// particular, OpenSSH 5.9 sends a channelOpenMsg with port number 0, +// rather than the actual port number. This means you can never open +// two different listeners with auto allocated ports. We work around +// this by trying explicit ports until we succeed. + +const openSSHPrefix = "OpenSSH_" + +// isBrokenOpenSSHVersion returns true if the given version string +// specifies a version of OpenSSH that is known to have a bug in port +// forwarding. +func isBrokenOpenSSHVersion(versionStr string) bool { + i := strings.Index(versionStr, openSSHPrefix) + if i < 0 { + return false + } + i += len(openSSHPrefix) + j := i + for ; j < len(versionStr); j++ { + if versionStr[j] < '0' || versionStr[j] > '9' { + break + } + } + version, _ := strconv.Atoi(versionStr[i:j]) + return version < 6 +} + +// autoPortListenWorkaround simulates automatic port allocation by +// trying random ports repeatedly. +func (c *ClientConn) autoPortListenWorkaround(laddr *net.TCPAddr) (net.Listener, error) { + var sshListener net.Listener + var err error + const tries = 10 + for i := 0; i < tries; i++ { + addr := *laddr + addr.Port = 1024 + rand.Intn(60000) + sshListener, err = c.ListenTCP(&addr) + if err == nil { + laddr.Port = addr.Port + return sshListener, err + } + } + return nil, fmt.Errorf("ssh: listen on random port failed after %d tries: %v", tries, err) +} + // ListenTCP requests the remote peer open a listening socket // on laddr. Incoming connections will be available by calling // Accept on the returned net.Listener. func (c *ClientConn) ListenTCP(laddr *net.TCPAddr) (net.Listener, error) { + if laddr.Port == 0 && isBrokenOpenSSHVersion(c.serverVersion) { + return c.autoPortListenWorkaround(laddr) + } + m := channelForwardMsg{ "tcpip-forward", true, // sendGlobalRequest waits for a reply @@ -59,10 +112,6 @@ func (c *ClientConn) ListenTCP(laddr *net.TCPAddr) (net.Listener, error) { } // Register this forward, using the port number we obtained. - // - // This does not work on OpenSSH < 6.0, which will send a - // channelOpenMsg with port number 0, rather than the actual - // port number. ch := c.forwardList.add(*laddr) return &tcpListener{laddr, c, ch}, nil diff --git a/ssh/tcpip_test.go b/ssh/tcpip_test.go new file mode 100644 index 0000000..7fa9fc4 --- /dev/null +++ b/ssh/tcpip_test.go @@ -0,0 +1,16 @@ +package ssh + +import ( + "testing" +) + +func TestAutoPortListenBroken(t *testing.T) { + broken := "SSH-2.0-OpenSSH_5.9hh11" + works := "SSH-2.0-OpenSSH_6.1" + if !isBrokenOpenSSHVersion(broken) { + t.Errorf("version %q not marked as broken", broken) + } + if isBrokenOpenSSHVersion(works) { + t.Errorf("version %q marked as broken", works) + } +} diff --git a/ssh/test/forward_unix_test.go b/ssh/test/forward_unix_test.go index e15fae5..0a4758c 100644 --- a/ssh/test/forward_unix_test.go +++ b/ssh/test/forward_unix_test.go @@ -8,47 +8,21 @@ package test import ( "bytes" - "fmt" "io" "io/ioutil" "math/rand" "net" "testing" "time" - - "code.google.com/p/go.crypto/ssh" ) -func listenSSHAuto(conn *ssh.ClientConn) (net.Listener, error) { - var sshListener net.Listener - var err error - tries := 10 - for i := 0; i < tries; i++ { - port := 1024 + rand.Intn(50000) - - // We can't reliably test dynamic port allocation, as it does - // not work correctly with OpenSSH before 6.0. See also - // https://bugzilla.mindrot.org/show_bug.cgi?id=2017 - sshListener, err = conn.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) - if err == nil { - break - } - } - - if err != nil { - return nil, fmt.Errorf("conn.Listen failed: %v (after %d tries)", err, tries) - } - - return sshListener, nil -} - func TestPortForward(t *testing.T) { server := newServer(t) defer server.Shutdown() conn := server.Dial(clientConfig()) defer conn.Close() - sshListener, err := listenSSHAuto(conn) + sshListener, err := conn.Listen("tcp", "localhost:0") if err != nil { t.Fatal(err) } @@ -124,7 +98,7 @@ func TestAcceptClose(t *testing.T) { defer server.Shutdown() conn := server.Dial(clientConfig()) - sshListener, err := listenSSHAuto(conn) + sshListener, err := conn.Listen("tcp", "localhost:0") if err != nil { t.Fatal(err) }