diff --git a/ssh/streamlocal.go b/ssh/streamlocal.go index 152470f..8e997da 100644 --- a/ssh/streamlocal.go +++ b/ssh/streamlocal.go @@ -58,6 +58,7 @@ func (c *Client) dialStreamLocal(socketPath string) (Channel, error) { return nil, err } go DiscardRequests(in) + go io.Copy(io.Discard, ch.Stderr()) return ch, err } @@ -79,6 +80,7 @@ func (l *unixListener) Accept() (net.Conn, error) { return nil, err } go DiscardRequests(incoming) + go io.Copy(io.Discard, ch.Stderr()) return &chanConn{ Channel: ch, diff --git a/ssh/tcpip.go b/ssh/tcpip.go index 78c41fe..213d8a6 100644 --- a/ssh/tcpip.go +++ b/ssh/tcpip.go @@ -332,6 +332,7 @@ func (l *tcpListener) Accept() (net.Conn, error) { return nil, err } go DiscardRequests(incoming) + go io.Copy(io.Discard, ch.Stderr()) return &chanConn{ Channel: ch, @@ -495,6 +496,7 @@ func (c *Client) dial(laddr string, lport int, raddr string, rport int) (Channel return nil, err } go DiscardRequests(in) + go io.Copy(io.Discard, ch.Stderr()) return ch, nil } diff --git a/ssh/tcpip_test.go b/ssh/tcpip_test.go index 4d85114..5ec7ed9 100644 --- a/ssh/tcpip_test.go +++ b/ssh/tcpip_test.go @@ -6,6 +6,7 @@ package ssh import ( "context" + "io" "net" "testing" "time" @@ -51,3 +52,213 @@ func TestClientDialContextWithDeadline(t *testing.T) { t.Errorf("DialContext: got nil error, expected %v", context.DeadlineExceeded) } } + +// forwardingPair establishes a client/server connection pair over an +// in-memory pipe. The server grants all global requests and accepts all +// channels opened by the client, delivering them on the returned channel. +func forwardingPair(t *testing.T) (*Client, *ServerConn, <-chan Channel) { + t.Helper() + + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + serverDone := make(chan struct{}) + t.Cleanup(func() { <-serverDone }) + t.Cleanup(func() { + c1.Close() + c2.Close() + }) + + serverConf := ServerConfig{ + NoClientAuth: true, + } + serverConf.AddHostKey(testSigners["rsa"]) + incoming := make(chan *ServerConn, 1) + serverChans := make(chan Channel, 1) + go func() { + defer close(serverDone) + conn, chans, reqs, err := NewServerConn(c1, &serverConf) + incoming <- conn + if err != nil { + t.Errorf("NewServerConn error: %v", err) + return + } + go func() { + for r := range reqs { + r.Reply(true, nil) + } + }() + for newCh := range chans { + ch, chanReqs, err := newCh.Accept() + if err != nil { + t.Errorf("Accept: %v", err) + continue + } + go DiscardRequests(chanReqs) + serverChans <- ch + } + }() + + conf := ClientConfig{ + HostKeyCallback: InsecureIgnoreHostKey(), + } + conn, chans, reqs, err := NewClientConn(c2, "", &conf) + if err != nil { + t.Fatalf("NewClientConn: %v", err) + } + client := NewClient(conn, chans, reqs) + t.Cleanup(func() { client.Close() }) + + server := <-incoming + if server == nil { + t.Fatal("Unable to get server") + } + t.Cleanup(func() { server.Close() }) + + return client, server, serverChans +} + +// floodStderr writes more than the default channel receive window (2 MiB) +// to w, which must be the stderr stream of an SSH channel. Stderr data is +// buffered and its window credit is only returned when the stream is +// read, so if the peer does not drain the stream the write blocks forever +// once the remote window is exhausted. +func floodStderr(t *testing.T, w io.Writer) { + t.Helper() + + const payload = 4 * 1024 * 1024 + done := make(chan error, 1) + go func() { + buf := make([]byte, 32*1024) + remaining := payload + for remaining > 0 { + n := min(len(buf), remaining) + nw, err := w.Write(buf[:n]) + if err != nil { + done <- err + return + } + remaining -= nw + } + done <- nil + }() + + select { + case err := <-done: + if err != nil { + t.Fatalf("write to stderr: %v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("write to stderr blocked: the stderr stream is not drained") + } +} + +// verifyMainStream checks that the main stream is still usable after +// stderr traffic by sending a payload from the server channel and +// reading it back from the client connection. +func verifyMainStream(t *testing.T, serverCh Channel, clientConn net.Conn) { + t.Helper() + + want := []byte("main stream after stderr flood") + if _, err := serverCh.Write(want); err != nil { + t.Fatalf("write to main stream: %v", err) + } + got := make([]byte, len(want)) + if _, err := io.ReadFull(clientConn, got); err != nil { + t.Fatalf("read from main stream: %v", err) + } + if string(got) != string(want) { + t.Fatalf("read %q from main stream, want %q", got, want) + } +} + +func TestClientDialDiscardsStderr(t *testing.T) { + for _, network := range []string{"tcp", "unix"} { + t.Run(network, func(t *testing.T) { + client, _, serverChans := forwardingPair(t) + + addr := "127.0.0.1:8022" + if network == "unix" { + addr = "/tmp/forwarded.sock" + } + conn, err := client.Dial(network, addr) + if err != nil { + t.Fatalf("Dial(%q, %q): %v", network, addr, err) + } + defer conn.Close() + serverCh := <-serverChans + defer serverCh.Close() + + floodStderr(t, serverCh.Stderr()) + verifyMainStream(t, serverCh, conn) + }) + } +} + +func TestClientListenerDiscardsStderr(t *testing.T) { + for _, network := range []string{"tcp", "unix"} { + t.Run(network, func(t *testing.T) { + client, server, _ := forwardingPair(t) + + var ( + ln net.Listener + payload []byte + chType string + err error + ) + if network == "tcp" { + ln, err = client.Listen("tcp", "127.0.0.1:8022") + if err != nil { + t.Fatalf("Listen: %v", err) + } + chType = "forwarded-tcpip" + payload = Marshal(&forwardedTCPPayload{ + Addr: "127.0.0.1", + Port: 8022, + OriginAddr: "127.0.0.5", + OriginPort: 1234, + }) + } else { + ln, err = client.ListenUnix("/tmp/forwarded.sock") + if err != nil { + t.Fatalf("ListenUnix: %v", err) + } + chType = "forwarded-streamlocal@openssh.com" + payload = Marshal(&forwardedStreamLocalPayload{ + SocketPath: "/tmp/forwarded.sock", + }) + } + defer ln.Close() + + // OpenChannel does not return until the channel is + // accepted, so run Accept concurrently. + accepted := make(chan net.Conn, 1) + go func() { + conn, err := ln.Accept() + if err != nil { + t.Errorf("Accept: %v", err) + close(accepted) + return + } + accepted <- conn + }() + + serverCh, reqs, err := server.OpenChannel(chType, payload) + if err != nil { + t.Fatalf("OpenChannel(%q): %v", chType, err) + } + go DiscardRequests(reqs) + defer serverCh.Close() + + conn := <-accepted + if conn == nil { + t.Fatal("no connection accepted") + } + defer conn.Close() + + floodStderr(t, serverCh.Stderr()) + verifyMainStream(t, serverCh, conn) + }) + } +}