go.crypto/ssh: sanity check incoming packet length

The check for a sensible packet length was removed a while ago
when the window size and channel packet size checks were moved
into channel.go. While the RFC suggests that any packet of size
less than uint32 -1 is valid, most implmentations limit the size
to a smaller value. OpenSSH chose 256kb, so that sounds like a
sensible default.

R=agl, huin, kardianos
CC=golang-dev
https://golang.org/cl/6490098

ssh/session_test.go

@@ -435,6 +435,16 @@ func TestClientCannotSendAfterClose(t *testing.T) {
 	}
 }
 
+func TestClientCannotSendHugePacket(t *testing.T) {
+	// client and server use the same transport write code so this
+	// test suffices for both.
+	conn := dial(shellHandler, t)
+	defer conn.Close()
+	if err := conn.transport.writePacket(make([]byte, maxPacket*2)); err == nil {
+		t.Fatalf("huge packet write should fail")
+	}
+}
+
 // windowTestBytes is the number of bytes that we'll send to the SSH server.
 const windowTestBytes = 16000 * 200
 

ssh/transport.go

@@ -19,6 +19,14 @@ import (
 
 const (
 	packetSizeMultiple = 16 // TODO(huin) this should be determined by the cipher.
+
+	// RFC 4253 section 6.1 defines a minimum packet size of 32768 that implementations
+	// MUST be able to process (plus a few more kilobytes for padding and mac). The RFC
+	// indicates implementations SHOULD be able to handle larger packet sizes, but then
+	// waffles on about reasonable limits. 
+	//	
+	// OpenSSH caps their maxPacket at 256kb so we choose to do the same.
+	maxPacket = 256 * 1024
 )
 
 // conn represents an ssh transport that implements packet based
@@ -92,7 +100,11 @@ func (r *reader) readOnePacket() ([]byte, error) {
 	paddingLength := uint32(lengthBytes[4])
 
 	if length <= paddingLength+1 {
-		return nil, errors.New("ssh: invalid packet length")
+		return nil, errors.New("ssh: invalid packet length, packet too small")
+	}
+
+	if length > maxPacket {
+		return nil, errors.New("ssh: invalid packet length, packet too large")
 	}
 
 	packet := make([]byte, length-1+macSize)
@@ -132,6 +144,9 @@ func (t *transport) readPacket() ([]byte, error) {
 
 // Encrypt and send a packet of data to the remote peer.
 func (w *writer) writePacket(packet []byte) error {
+	if len(packet) > maxPacket {
+		return errors.New("ssh: packet too large")
+	}
 	w.Mutex.Lock()
 	defer w.Mutex.Unlock()