From f4749cba326ed8754af0bc35042d8c92b46c23d7 Mon Sep 17 00:00:00 2001
From: Dave Cheney <dave@cheney.net>
Date: Thu, 9 Aug 2012 10:22:00 +1000
Subject: [PATCH] go.crypto/ssh: never send more data than maxpacket

RFC 4254 s5.2 is clear that a client must never send a data
packet larger than the value of maximum packet supplied by the
remote side during channel setup. The client was not honoring
this value, in fact it wasn't even recording it.

Thanks to Albert Strasheim for the bug report.

R=agl, fullung
CC=golang-dev
https://golang.org/cl/6448128
---
 ssh/channel.go      |  8 ++++----
 ssh/client.go       |  9 ++++++---
 ssh/server.go       | 15 ++++++++-------
 ssh/session_test.go | 31 +++++++++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/ssh/channel.go b/ssh/channel.go
index 40f0aa0..53a2d8c 100644
--- a/ssh/channel.go
+++ b/ssh/channel.go
@@ -74,6 +74,7 @@ type channel struct {
 	conn              // the underlying transport
 	localId, remoteId uint32
 	remoteWin         window
+	maxPacketSize     uint32
 
 	theyClosed  bool // indicates the close msg has been received from the remote side
 	weClosed    bool // incidates the close msg has been sent from our side
@@ -119,10 +120,9 @@ type serverChan struct {
 	chanType  string
 	extraData []byte
 
-	serverConn    *ServerConn
-	myWindow      uint32
-	maxPacketSize uint32
-	err           error
+	serverConn *ServerConn
+	myWindow   uint32
+	err        error
 
 	pendingRequests []ChannelRequest
 	pendingData     []byte
diff --git a/ssh/client.go b/ssh/client.go
index e5748e3..b266f0d 100644
--- a/ssh/client.go
+++ b/ssh/client.go
@@ -499,6 +499,8 @@ func (c *clientChan) waitForChannelOpenResponse() error {
 	case *channelOpenConfirmMsg:
 		// fixup remoteId field
 		c.remoteId = msg.MyId
+		// TODO(dfc) asset this is < 2^31.
+		c.maxPacketSize = msg.MaxPacketSize
 		c.remoteWin.add(msg.MyWindow)
 		return nil
 	case *channelOpenFailureMsg:
@@ -582,9 +584,10 @@ type chanWriter struct {
 // Write writes data to the remote process's standard input.
 func (w *chanWriter) Write(data []byte) (written int, err error) {
 	for len(data) > 0 {
-		// n cannot be larger than 2^31 as len(data) cannot
-		// be larger than 2^31
-		n := int(w.remoteWin.reserve(uint32(len(data))))
+		// never send more data than maxPacketSize even if
+		// there is sufficent window.
+		n := min(int(w.maxPacketSize), len(data))
+		n = int(w.remoteWin.reserve(uint32(n)))
 		remoteId := w.remoteId
 		packet := []byte{
 			msgChannelData,
diff --git a/ssh/server.go b/ssh/server.go
index 5c640a7..2825a3d 100644
--- a/ssh/server.go
+++ b/ssh/server.go
@@ -569,14 +569,15 @@ func (s *ServerConn) Accept() (Channel, error) {
 						conn:      s,
 						remoteId:  msg.PeersId,
 						remoteWin: window{Cond: newCond()},
+						// TODO(dfc) assert this param is < 2^31.
+						maxPacketSize: msg.MaxPacketSize,
 					},
-					chanType:      msg.ChanType,
-					maxPacketSize: msg.MaxPacketSize,
-					extraData:     msg.TypeSpecificData,
-					myWindow:      defaultWindowSize,
-					serverConn:    s,
-					cond:          newCond(),
-					pendingData:   make([]byte, defaultWindowSize),
+					chanType:    msg.ChanType,
+					extraData:   msg.TypeSpecificData,
+					myWindow:    defaultWindowSize,
+					serverConn:  s,
+					cond:        newCond(),
+					pendingData: make([]byte, defaultWindowSize),
 				}
 				c.remoteWin.add(msg.PeersWindow)
 				s.lock.Lock()
diff --git a/ssh/session_test.go b/ssh/session_test.go
index aaeae48..a5aafb1 100644
--- a/ssh/session_test.go
+++ b/ssh/session_test.go
@@ -9,6 +9,7 @@ package ssh
 import (
 	"bytes"
 	"io"
+	"io/ioutil"
 	"net"
 	"testing"
 
@@ -338,6 +339,26 @@ func TestServerZeroWindowAdjust(t *testing.T) {
 	}
 }
 
+// Verify that we never send a packet larger than maxpacket.
+func TestClientStdinRespectsMaxPacketSize(t *testing.T) {
+	conn := dial(discardHandler, t)
+	defer conn.Close()
+	session, err := conn.NewSession()
+	if err != nil {
+		t.Fatalf("Unable to request new session: %s", err)
+	}
+	defer session.Close()
+	if err := session.Shell(); err != nil {
+		t.Fatalf("Unable to execute command: %s", err)
+	}
+	// try to stuff 128k of data into a 32k hole.
+	const size = 128 * 1024
+	n, err := session.clientChan.stdin.Write(make([]byte, size))
+	if n != size || err != nil {
+		t.Fatalf("failed to write: %d, %v", n, err)
+	}
+}
+
 type exitStatusMsg struct {
 	PeersId   uint32
 	Request   string
@@ -456,3 +477,13 @@ func sendZeroWindowAdjust(ch *serverChan) {
 	shell.ReadLine()
 	sendStatus(0, ch)
 }
+
+func discardHandler(ch *serverChan) {
+	defer ch.Close()
+	// grow the window to avoid being fooled by
+	// the initial 1 << 14 window.
+	ch.sendWindowAdj(1024 * 1024)
+	shell := newServerShell(ch, "> ")
+	shell.ReadLine()
+	io.Copy(ioutil.Discard, ch.serverConn)
+}