From 7c6cc321c680f03b9ef0764448e780704f486b51 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Mon, 9 Jan 2017 15:19:37 +0100 Subject: [PATCH] x/ssh: filter debug and ignore messages in transport.readPacket. This prevents these messages from confusing higher layers of the protocol. Fixes #16927. Change-Id: If18d8d02bdde3c0470e29a7280cd355d3e55ad78 Reviewed-on: https://go-review.googlesource.com/34959 Run-TryBot: Han-Wen Nienhuys TryBot-Result: Gobot Gobot Reviewed-by: Han-Wen Nienhuys --- ssh/handshake_test.go | 64 +++++++++++++++++++++++++++++++++++-------- ssh/transport.go | 14 ++++++++-- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/ssh/handshake_test.go b/ssh/handshake_test.go index da53d3a..4cf5368 100644 --- a/ssh/handshake_test.go +++ b/ssh/handshake_test.go @@ -9,6 +9,7 @@ import ( "crypto/rand" "errors" "fmt" + "io" "net" "reflect" "runtime" @@ -58,14 +59,46 @@ func netPipe() (net.Conn, net.Conn, error) { return c1, c2, nil } -func handshakePair(clientConf *ClientConfig, addr string) (client *handshakeTransport, server *handshakeTransport, err error) { +// noiseTransport inserts ignore messages to check that the read loop +// and the key exchange filters out these messages. +type noiseTransport struct { + keyingTransport +} + +func (t *noiseTransport) writePacket(p []byte) error { + ignore := []byte{msgIgnore} + if err := t.keyingTransport.writePacket(ignore); err != nil { + return err + } + debug := []byte{msgDebug, 1, 2, 3} + if err := t.keyingTransport.writePacket(debug); err != nil { + return err + } + + return t.keyingTransport.writePacket(p) +} + +func addNoiseTransport(t keyingTransport) keyingTransport { + return &noiseTransport{t} +} + +// handshakePair creates two handshakeTransports connected with each +// other. If the noise argument is true, both transports will try to +// confuse the other side by sending ignore and debug messages. +func handshakePair(clientConf *ClientConfig, addr string, noise bool) (client *handshakeTransport, server *handshakeTransport, err error) { a, b, err := netPipe() if err != nil { return nil, nil, err } - trC := newTransport(a, rand.Reader, true) - trS := newTransport(b, rand.Reader, false) + var trC, trS keyingTransport + + trC = newTransport(a, rand.Reader, true) + trS = newTransport(b, rand.Reader, false) + if noise { + trC = addNoiseTransport(trC) + trS = addNoiseTransport(trS) + } clientConf.SetDefaults() v := []byte("version") @@ -85,7 +118,7 @@ func TestHandshakeBasic(t *testing.T) { t.Skip("see golang.org/issue/7237") } checker := &testChecker{} - trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr") + trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", true) if err != nil { t.Fatalf("handshakePair: %v", err) } @@ -93,7 +126,9 @@ func TestHandshakeBasic(t *testing.T) { defer trC.Close() defer trS.Close() + clientDone := make(chan int, 0) go func() { + defer close(clientDone) // Client writes a bunch of stuff, and does a key // change in the middle. This should not confuse the // handshake in progress @@ -115,8 +150,10 @@ func TestHandshakeBasic(t *testing.T) { // Server checks that client messages come in cleanly i := 0 + err = nil for { - p, err := trS.readPacket() + var p []byte + p, err = trS.readPacket() if err != nil { break } @@ -129,6 +166,10 @@ func TestHandshakeBasic(t *testing.T) { } i++ } + <-clientDone + if err != nil && err != io.EOF { + t.Fatalf("server error: %v", err) + } if i != 10 { t.Errorf("received %d messages, want 10.", i) } @@ -143,11 +184,12 @@ func TestHandshakeBasic(t *testing.T) { if want != checker.calls[0] { t.Errorf("got %q want %q for host key check", checker.calls[0], want) } + } func TestHandshakeError(t *testing.T) { checker := &testChecker{} - trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "bad") + trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "bad", false) if err != nil { t.Fatalf("handshakePair: %v", err) } @@ -186,7 +228,7 @@ func TestHandshakeError(t *testing.T) { func TestForceFirstKex(t *testing.T) { checker := &testChecker{} - trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr") + trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", false) if err != nil { t.Fatalf("handshakePair: %v", err) } @@ -208,7 +250,7 @@ func TestForceFirstKex(t *testing.T) { func TestHandshakeTwice(t *testing.T) { checker := &testChecker{} - trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr") + trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", false) if err != nil { t.Fatalf("handshakePair: %v", err) } @@ -278,7 +320,7 @@ func TestHandshakeAutoRekeyWrite(t *testing.T) { checker := &testChecker{} clientConf := &ClientConfig{HostKeyCallback: checker.Check} clientConf.RekeyThreshold = 500 - trC, trS, err := handshakePair(clientConf, "addr") + trC, trS, err := handshakePair(clientConf, "addr", false) if err != nil { t.Fatalf("handshakePair: %v", err) } @@ -326,7 +368,7 @@ func TestHandshakeAutoRekeyRead(t *testing.T) { } clientConf.RekeyThreshold = 500 - trC, trS, err := handshakePair(clientConf, "addr") + trC, trS, err := handshakePair(clientConf, "addr", false) if err != nil { t.Fatalf("handshakePair: %v", err) } @@ -448,7 +490,7 @@ func TestDisconnect(t *testing.T) { t.Skip("see golang.org/issue/7237") } checker := &testChecker{} - trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr") + trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", false) if err != nil { t.Fatalf("handshakePair: %v", err) } diff --git a/ssh/transport.go b/ssh/transport.go index 62fba62..7e43a12 100644 --- a/ssh/transport.go +++ b/ssh/transport.go @@ -85,8 +85,18 @@ func (t *transport) prepareKeyChange(algs *algorithms, kexResult *kexResult) err } // Read and decrypt next packet. -func (t *transport) readPacket() ([]byte, error) { - return t.reader.readPacket(t.bufReader) +func (t *transport) readPacket() (p []byte, err error) { + for { + p, err = t.reader.readPacket(t.bufReader) + if err != nil { + break + } + if len(p) == 0 || (p[0] != msgIgnore && p[0] != msgDebug) { + break + } + } + + return p, err } func (s *connectionState) readPacket(r *bufio.Reader) ([]byte, error) {