From 7cbb17fecb34db5291cde2f20061eaf746fb8e17 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Tue, 18 Jun 2013 12:43:42 -0400 Subject: [PATCH] go.crypto/ssh: fix test breakages introduced by 125:40246d2ae2eb * Remove special handling for dynamically allocated ports. This was a bug in OpenSSH 5.x sshd. * Run the test with a preselected port number. * Run TestPortForward only on unix platforms. R=dave, agl CC=golang-dev https://golang.org/cl/10049045 --- ssh/tcpip.go | 30 ++++++++----------- .../{forward_test.go => forward_unix_test.go} | 25 ++++++++++++++-- 2 files changed, 35 insertions(+), 20 deletions(-) rename ssh/test/{forward_test.go => forward_unix_test.go} (70%) diff --git a/ssh/tcpip.go b/ssh/tcpip.go index 498e341..8ebe262 100644 --- a/ssh/tcpip.go +++ b/ssh/tcpip.go @@ -48,15 +48,8 @@ func (c *ClientConn) ListenTCP(laddr *net.TCPAddr) (net.Listener, error) { return nil, err } - // Register this forward, using the port number we requested. - // If we requested port 0 (auto allocated port), we have to - // register under 0, since the channelOpenMsg will list 0 - // rather than the allocated port number. - ch := c.forwardList.add(*laddr) - // If the original port was 0, then the remote side will // supply a real port number in the response. - origPort := uint32(laddr.Port) if laddr.Port == 0 { port, _, ok := parseUint32(resp.Data) if !ok { @@ -65,7 +58,14 @@ func (c *ClientConn) ListenTCP(laddr *net.TCPAddr) (net.Listener, error) { laddr.Port = int(port) } - return &tcpListener{laddr, origPort, c, ch}, nil + // 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 } // forwardList stores a mapping between remote @@ -126,10 +126,8 @@ func (l *forwardList) lookup(addr net.TCPAddr) (chan forward, bool) { type tcpListener struct { laddr *net.TCPAddr - // The port with which we made the request, which can be 0. - origPort uint32 - conn *ClientConn - in <-chan forward + conn *ClientConn + in <-chan forward } // Accept waits for and returns the next connection to the listener. @@ -155,13 +153,9 @@ func (l *tcpListener) Close() error { "cancel-tcpip-forward", true, l.laddr.IP.String(), - l.origPort, + uint32(l.laddr.Port), } - origAddr := net.TCPAddr{ - IP: l.laddr.IP, - Port: int(l.origPort), - } - l.conn.forwardList.remove(origAddr) + l.conn.forwardList.remove(*l.laddr) if _, err := l.conn.sendGlobalRequest(m); err != nil { return err } diff --git a/ssh/test/forward_test.go b/ssh/test/forward_unix_test.go similarity index 70% rename from ssh/test/forward_test.go rename to ssh/test/forward_unix_test.go index 9f57bb0..dc64241 100644 --- a/ssh/test/forward_test.go +++ b/ssh/test/forward_unix_test.go @@ -1,7 +1,14 @@ +// Copyright 2012 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build darwin freebsd linux netbsd openbsd + package test import ( "bytes" + "fmt" "io" "io/ioutil" "math/rand" @@ -15,9 +22,23 @@ func TestPortForward(t *testing.T) { conn := server.Dial(clientConfig()) defer conn.Close() - sshListener, err := conn.Listen("tcp", "127.0.0.1:0") + 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 { - t.Fatalf("conn.Listen failed: %v", err) + t.Fatalf("conn.Listen failed: %v (after %d tries)", err, tries) } go func() {