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

@@ -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,

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

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)
+	}
+}

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)
 	}