CVE-2023-48795: Improper Validation of Integrity Check Value (CWE-354)
value: The SSH transport protocol with certain OpenSSH extensions, found in OpenSSH before 9.6 and other products, allows remote attackers to bypass integrity checks such that some packets are omitted (from the extension negotiation message), and a client and server may consequently end up with a connection for which some security features have been downgraded or disabled, aka a Terrapin attack. This occurs because the SSH Binary Packet Protocol (BPP), implemented by these extensions, mishandles the handshake phase and mishandles use of sequence numbers. For example, there is an effective attack against SSH's use of ChaCha20-Poly1305 (and CBC with Encrypt-then-MAC). The bypass occurs in chacha20-poly1305@openssh.com and (if CBC is used) the -etm@openssh.com MAC algorithms. This also affects Maverick Synergy Java SSH API before 3.1.0-SNAPSHOT, Dropbear through 2022.83, Ssh before 5.1.1 in Erlang/OTP, PuTTY before 0.80, AsyncSSH before 2.14.2, golang.org/x/crypto before 0.17.0, libssh before 0.10.6, libssh2 through 1.11.0, Thorn Tech SFTP Gateway before 3.4.6, Tera Term before 5.1, Paramiko before 3.4.0, jsch before 0.2.15, SFTPGo before 2.5.6, Netgate pfSense Plus through 23.09.1, Netgate pfSense CE through 2.7.2, HPN-SSH through 18.2.0, ProFTPD before 1.3.8b (and before 1.3.9rc2), ORYX CycloneSSH before 2.3.4, NetSarang XShell 7 before Build 0144, CrushFTP before 10.6.0, ConnectBot SSH library before 2.2.22, Apache MINA sshd through 2.11.0, sshj through 0.37.0, TinySSH through 20230101, trilead-ssh2 6401, LANCOM LCOS and LANconfig, FileZilla before 3.66.4, Nova before 11.8, PKIX-SSH before 14.4, SecureCRT before 9.4.3, Transmit5 before 5.10.4, Win32-OpenSSH before 9.5.0.0p1-Beta, WinSCP before 6.2.2, Bitvise SSH Server before 9.32, Bitvise SSH Client before 9.33, KiTTY through 0.76.1.13, the net-ssh gem 7.2.0 for Ruby, the mscdex ssh2 module before 1.15.0 for Node.js, the thrussh library before 0.35.1 for Rust, and the Russh crate before 0.40.2 for Rust.
CWE description: The product does not validate or incorrectly validates the integrity check values or "checksums" of a message. This may prevent it from detecting if the data has been modified or corrupted in transmission.
CVSS v3 base score: 5.9
Published: 2023-12-18T16:15Z
Repository: https://github.com/golang/crypto
Language: Go
Files changed: ssh/handshake.go, ssh/handshake_test.go, ssh/transport.go
Commit date: 2023-12-18 16:33:08 +0000
Commit message: ssh: implement strict KEX protocol changes

Implement the "strict KEX" protocol changes, as described in section
1.9 of the OpenSSH PROTOCOL file (as of OpenSSH version 9.6/9.6p1).

Namely this makes the following changes:
  * Both the server and the client add an additional algorithm to the
    initial KEXINIT message, indicating support for the strict KEX mode.
  * When one side of the connection sees the strict KEX extension
    algorithm, the strict KEX mode is enabled for messages originating
    from the other side of the connection. If the sequence number for
    the side which requested the extension is not 1 (indicating that it
    has already received non-KEXINIT packets), the connection is
    terminated.
  * When strict kex mode is enabled, unexpected messages during the
    handshake are considered fatal. Additionally when a key change
    occurs (on the receipt of the NEWKEYS message) the message sequence
    numbers are reset.

Thanks to Fabian Bäumer, Marcus Brinkmann, and Jörg Schwenk from Ruhr
University Bochum for reporting this issue.

Fixes CVE-2023-48795
Fixes golang/go#64784

Change-Id: I96b53afd2bd2fb94d2b6f2a46a5dacf325357604
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/550715
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

Diff: diff --git a/ssh/handshake.go b/ssh/handshake.go
index 49bbba7..56cdc7c 100644
--- a/ssh/handshake.go
+++ b/ssh/handshake.go
@@ -33,10 +33,20 @@ type keyingTransport interface {
 
 	// prepareKeyChange sets up a key change. The key change for a
 	// direction will be effected if a msgNewKeys message is sent
 	// or received.
 	prepareKeyChange(*algorithms, *kexResult) error
+
+	// setStrictMode sets the strict KEX mode, notably triggering
+	// sequence number resets on sending or receiving msgNewKeys.
+	// If the sequence number is already > 1 when setStrictMode
+	// is called, an error is returned.
+	setStrictMode() error
+
+	// setInitialKEXDone indicates to the transport that the initial key exchange
+	// was completed
+	setInitialKEXDone()
 }
 
 // handshakeTransport implements rekeying on top of a keyingTransport
 // and offers a thread-safe writePacket() interface.
 type handshakeTransport struct {
@@ -98,10 +108,14 @@ type handshakeTransport struct {
 	readPacketsLeft uint32
 	readBytesLeft   int64
 
 	// The session ID or nil if first kex did not complete yet.
 	sessionID []byte
+
+	// strictMode indicates if the other side of the handshake indicated
+	// that we should be following the strict KEX protocol restrictions.
+	strictMode bool
 }
 
 type pendingKex struct {
 	otherInit []byte
 	done      chan error
@@ -207,11 +221,14 @@ func (t *handshakeTransport) readLoop() {
 		if err != nil {
 			t.readError = err
 			close(t.incoming)
 			break
 		}
-		if p[0] == msgIgnore || p[0] == msgDebug {
+		// If this is the first kex, and strict KEX mode is enabled,
+		// we don't ignore any messages, as they may be used to manipulate
+		// the packet sequence numbers.
+		if !(t.sessionID == nil && t.strictMode) && (p[0] == msgIgnore || p[0] == msgDebug) {
 			continue
 		}
 		t.incoming <- p
 	}
 
@@ -439,10 +456,15 @@ func (t *handshakeTransport) readOnePacket(first bool) ([]byte, error) {
 	}
 
 	return successPacket, nil
 }
 
+const (
+	kexStrictClient = "kex-strict-c-v00@openssh.com"
+	kexStrictServer = "kex-strict-s-v00@openssh.com"
+)
+
 // sendKexInit sends a key change message.
 func (t *handshakeTransport) sendKexInit() error {
 	t.mu.Lock()
 	defer t.mu.Unlock()
 	if t.sentInitMsg != nil {
@@ -452,20 +474,26 @@ func (t *handshakeTransport) sendKexInit() error {
 		// second kexInit.
 		return nil
 	}
 
 	msg := &kexInitMsg{
-		KexAlgos:                t.config.KeyExchanges,
 		CiphersClientServer:     t.config.Ciphers,
 		CiphersServerClient:     t.config.Ciphers,
 		MACsClientServer:        t.config.MACs,
 		MACsServerClient:        t.config.MACs,
 		CompressionClientServer: supportedCompressions,
 		CompressionServerClient: supportedCompressions,
 	}
 	io.ReadFull(rand.Reader, msg.Cookie[:])
 
+	// We mutate the KexAlgos slice, in order to add the kex-strict extension algorithm,
+	// and possibly to add the ext-info extension algorithm. Since the slice may be the
+	// user owned KeyExchanges, we create our own slice in order to avoid using user
+	// owned memory by mistake.
+	msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+2) // room for kex-strict and ext-info
+	msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...)
+
 	isServer := len(t.hostKeys) > 0
 	if isServer {
 		for _, k := range t.hostKeys {
 			// If k is a MultiAlgorithmSigner, we restrict the signature
 			// algorithms. If k is a AlgorithmSigner, presume it supports all
@@ -486,21 +514,28 @@ func (t *handshakeTransport) sendKexInit() error {
 				msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, algorithmsForKeyFormat(keyFormat)...)
 			default:
 				msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, keyFormat)
 			}
 		}
+
+		if t.sessionID == nil {
+			msg.KexAlgos = append(msg.KexAlgos, kexStrictServer)
+		}
 	} else {
 		msg.ServerHostKeyAlgos = t.hostKeyAlgorithms
 
 		// As a client we opt in to receiving SSH_MSG_EXT_INFO so we know what
 		// algorithms the server supports for public key authentication. See RFC
 		// 8308, Section 2.1.
+		//
+		// We also send the strict KEX mode extension algorithm, in order to opt
+		// into the strict KEX mode.
 		if firstKeyExchange := t.sessionID == nil; firstKeyExchange {
-			msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+1)
-			msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...)
 			msg.KexAlgos = append(msg.KexAlgos, "ext-info-c")
+			msg.KexAlgos = append(msg.KexAlgos, kexStrictClient)
 		}
+
 	}
 
 	packet := Marshal(msg)
 
 	// writePacket destroys the contents, so save a copy.
@@ -602,10 +637,17 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error {
 	t.algorithms, err = findAgreedAlgorithms(isClient, clientInit, serverInit)
 	if err != nil {
 		return err
 	}
 
+	if t.sessionID == nil && ((isClient && contains(serverInit.KexAlgos, kexStrictServer)) || (!isClient && contains(clientInit.KexAlgos, kexStrictClient))) {
+		t.strictMode = true
+		if err := t.conn.setStrictMode(); err != nil {
+			return err
+		}
+	}
+
 	// We don't send FirstKexFollows, but we handle receiving it.
 	//
 	// RFC 4253 section 7 defines the kex and the agreement method for
 	// first_kex_packet_follows. It states that the guessed packet
 	// should be ignored if the "kex algorithm and/or the host
@@ -677,10 +719,16 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error {
 		return err
 	} else if packet[0] != msgNewKeys {
 		return unexpectedMessageError(msgNewKeys, packet[0])
 	}
 
+	if firstKeyExchange {
+		// Indicates to the transport that the first key exchange is completed
+		// after receiving SSH_MSG_NEWKEYS.
+		t.conn.setInitialKEXDone()
+	}
+
 	return nil
 }
 
 // algorithmSignerWrapper is an AlgorithmSigner that only supports the default
 // key format algorithm.
diff --git a/ssh/handshake_test.go b/ssh/handshake_test.go
index 65afc20..2bc607b 100644
--- a/ssh/handshake_test.go
+++ b/ssh/handshake_test.go
@@ -393,10 +393,14 @@ func (n *errorKeyingTransport) readPacket() ([]byte, error) {
 
 	n.readLeft--
 	return n.packetConn.readPacket()
 }
 
+func (n *errorKeyingTransport) setStrictMode() error { return nil }
+
+func (n *errorKeyingTransport) setInitialKEXDone() {}
+
 func TestHandshakeErrorHandlingRead(t *testing.T) {
 	for i := 0; i < 20; i++ {
 		testHandshakeErrorHandlingN(t, i, -1, false)
 	}
 }
@@ -708,5 +712,310 @@ func TestPickIncompatibleHostKeyAlgo(t *testing.T) {
 	signer := pickHostKey([]Signer{multiAlgoSigner}, KeyAlgoRSA)
 	if signer != nil {
 		t.Fatal("incompatible signer returned")
 	}
 }
+
+func TestStrictKEXResetSeqFirstKEX(t *testing.T) {
+	if runtime.GOOS == "plan9" {
+		t.Skip("see golang.org/issue/7237")
+	}
+
+	checker := &syncChecker{
+		waitCall: make(chan int, 10),
+		called:   make(chan int, 10),
+	}
+
+	checker.waitCall <- 1
+	trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", false)
+	if err != nil {
+		t.Fatalf("handshakePair: %v", err)
+	}
+	<-checker.called
+
+	t.Cleanup(func() {
+		trC.Close()
+		trS.Close()
+	})
+
+	// Throw away the msgExtInfo packet sent during the handshake by the server
+	_, err = trC.readPacket()
+	if err != nil {
+		t.Fatalf("readPacket failed: %s", err)
+	}
+
+	// close the handshake transports before checking the sequence number to
+	// avoid races.
+	trC.Close()
+	trS.Close()
+
+	// check that the sequence number counters. We reset after msgNewKeys, but
+	// then the server immediately writes msgExtInfo, and we close the
+	// transports so we expect read 2, write 0 on the client and read 1, write 1
+	// on the server.
+	if trC.conn.(*transport).reader.seqNum != 2 || trC.conn.(*transport).writer.seqNum != 0 ||
+		trS.conn.(*transport).reader.seqNum != 1 || trS.conn.(*transport).writer.seqNum != 1 {
+		t.Errorf(
+			"unexpected sequence counters:\nclient: reader %d (expected 2), writer %d (expected 0)\nserver: reader %d (expected 1), writer %d (expected 1)",
+			trC.conn.(*transport).reader.seqNum,
+			trC.conn.(*transport).writer.seqNum,
+			trS.conn.(*transport).reader.seqNum,
+			trS.conn.(*transport).writer.seqNum,
+		)
+	}
+}
+
+func TestStrictKEXResetSeqSuccessiveKEX(t *testing.T) {
+	if runtime.GOOS == "plan9" {
+		t.Skip("see golang.org/issue/7237")
+	}
+
+	checker := &syncChecker{
+		waitCall: make(chan int, 10),
+		called:   make(chan int, 10),
+	}
+
+	checker.waitCall <- 1
+	trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", false)
+	if err != nil {
+		t.Fatalf("handshakePair: %v", err)
+	}
+	<-checker.called
+
+	t.Cleanup(func() {
+		trC.Close()
+		trS.Close()
+	})
+
+	// Throw away the msgExtInfo packet sent during the handshake by the server
+	_, err = trC.readPacket()
+	if err != nil {
+		t.Fatalf("readPacket failed: %s", err)
+	}
+
+	// write and read five packets on either side to bump the sequence numbers
+	for i := 0; i < 5; i++ {
+		if err := trC.writePacket([]byte{msgRequestSuccess}); err != nil {
+			t.Fatalf("writePacket failed: %s", err)
+		}
+		if _, err := trS.readPacket(); err != nil {
+			t.Fatalf("readPacket failed: %s", err)
+		}
+		if err := trS.writePacket([]byte{msgRequestSuccess}); err != nil {
+			t.Fatalf("writePacket failed: %s", err)
+		}
+		if _, err := trC.readPacket(); err != nil {
+			t.Fatalf("readPacket failed: %s", err)
+		}
+	}
+
+	// Request a key exchange, which should cause the sequence numbers to reset
+	checker.waitCall <- 1
+	trC.requestKeyExchange()
+	<-checker.called
+
+	// write a packet on the client, and then read it, to verify the key change has actually happened, since
+	// the HostKeyCallback is called _during_ the handshake, so isn't actually indicative of the handshake
+	// finishing.
+	dummyPacket := []byte{99}
+	if err := trS.writePacket(dummyPacket); err != nil {
+		t.Fatalf("writePacket failed: %s", err)
+	}
+	if p, err := trC.readPacket(); err != nil {
+		t.Fatalf("readPacket failed: %s", err)
+	} else if !bytes.Equal(p, dummyPacket) {
+		t.Fatalf("unexpected packet: got %x, want %x", p, dummyPacket)
+	}
+
+	// close the handshake transports before checking the sequence number to
+	// avoid races.
+	trC.Close()
+	trS.Close()
+
+	if trC.conn.(*transport).reader.seqNum != 2 || trC.conn.(*transport).writer.seqNum != 0 ||
+		trS.conn.(*transport).reader.seqNum != 1 || trS.conn.(*transport).writer.seqNum != 1 {
+		t.Errorf(
+			"unexpected sequence counters:\nclient: reader %d (expected 2), writer %d (expected 0)\nserver: reader %d (expected 1), writer %d (expected 1)",
+			trC.conn.(*transport).reader.seqNum,
+			trC.conn.(*transport).writer.seqNum,
+			trS.conn.(*transport).reader.seqNum,
+			trS.conn.(*transport).writer.seqNum,
+		)
+	}
+}
+
+func TestSeqNumIncrease(t *testing.T) {
+	if runtime.GOOS == "plan9" {
+		t.Skip("see golang.org/issue/7237")
+	}
+
+	checker := &syncChecker{
+		waitCall: make(chan int, 10),
+		called:   make(chan int, 10),
+	}
+
+	checker.waitCall <- 1
+	trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", false)
+	if err != nil {
+		t.Fatalf("handshakePair: %v", err)
+	}
+	<-checker.called
+
+	t.Cleanup(func() {
+		trC.Close()
+		trS.Close()
+	})
+
+	// Throw away the msgExtInfo packet sent during the handshake by the server
+	_, err = trC.readPacket()
+	if err != nil {
+		t.Fatalf("readPacket failed: %s", err)
+	}
+
+	// write and read five packets on either side to bump the sequence numbers
+	for i := 0; i < 5; i++ {
+		if err := trC.writePacket([]byte{msgRequestSuccess}); err != nil {
+			t.Fatalf("writePacket failed: %s", err)
+		}
+		if _, err := trS.readPacket(); err != nil {
+			t.Fatalf("readPacket failed: %s", err)
+		}
+		if err := trS.writePacket([]byte{msgRequestSuccess}); err != nil {
+			t.Fatalf("writePacket failed: %s", err)
+		}
+		if _, err := trC.readPacket(); err != nil {
+			t.Fatalf("readPacket failed: %s", err)
+		}
+	}
+
+	// close the handshake transports before checking the sequence number to
+	// avoid races.
+	trC.Close()
+	trS.Close()
+
+	if trC.conn.(*transport).reader.seqNum != 7 || trC.conn.(*transport).writer.seqNum != 5 ||
+		trS.conn.(*transport).reader.seqNum != 6 || trS.conn.(*transport).writer.seqNum != 6 {
+		t.Errorf(
+			"unexpected sequence counters:\nclient: reader %d (expected 7), writer %d (expected 5)\nserver: reader %d (expected 6), writer %d (expected 6)",
+			trC.conn.(*transport).reader.seqNum,
+			trC.conn.(*transport).writer.seqNum,
+			trS.conn.(*transport).reader.seqNum,
+			trS.conn.(*transport).writer.seqNum,
+		)
+	}
+}
+
+func TestStrictKEXUnexpectedMsg(t *testing.T) {
+	if runtime.GOOS == "plan9" {
+		t.Skip("see golang.org/issue/7237")
+	}
+
+	// Check that unexpected messages during the handshake cause failure
+	_, _, err := handshakePair(&ClientConfig{HostKeyCallback: func(hostname string, remote net.Addr, key PublicKey) error { return nil }}, "addr", true)
+	if err == nil {
+		t.Fatal("handshake should fail when there are unexpected messages during the handshake")
+	}
+
+	trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: func(hostname string, remote net.Addr, key PublicKey) error { return nil }}, "addr", false)
+	if err != nil {
+		t.Fatalf("handshake failed: %s", err)
+	}
+
+	// Check that ignore/debug pacekts are still ignored outside of the handshake
+	if err := trC.writePacket([]byte{msgIgnore}); err != nil {
+		t.Fatalf("writePacket failed: %s", err)
+	}
+	if err := trC.writePacket([]byte{msgDebug}); err != nil {
+		t.Fatalf("writePacket failed: %s", err)
+	}
+	dummyPacket := []byte{99}
+	if err := trC.writePacket(dummyPacket); err != nil {
+		t.Fatalf("writePacket failed: %s", err)
+	}
+
+	if p, err := trS.readPacket(); err != nil {
+		t.Fatalf("readPacket failed: %s", err)
+	} else if !bytes.Equal(p, dummyPacket) {
+		t.Fatalf("unexpected packet: got %x, want %x", p, dummyPacket)
+	}
+}
+
+func TestStrictKEXMixed(t *testing.T) {
+	// Test that we still support a mixed connection, where one side sends kex-strict but the other
+	// side doesn't.
+
+	a, b, err := netPipe()
+	if err != nil {
+		t.Fatalf("netPipe failed: %s", err)
+	}
+
+	var trC, trS keyingTransport
+
+	trC = newTransport(a, rand.Reader, true)
+	trS = newTransport(b, rand.Reader, false)
+	trS = addNoiseTransport(trS)
+
+	clientConf := &ClientConfig{HostKeyCallback: func(hostname string, remote net.Addr, key PublicKey) error { return nil }}
+	clientConf.SetDefaults()
+
+	v := []byte("version")
+	client := newClientTransport(trC, v, v, clientConf, "addr", a.RemoteAddr())
+
+	serverConf := &ServerConfig{}
+	serverConf.AddHostKey(testSigners["ecdsa"])
+	serverConf.AddHostKey(testSigners["rsa"])
+	serverConf.SetDefaults()
+
+	transport := newHandshakeTransport(trS, &serverConf.Config, []byte("version"), []byte("version"))
+	transport.hostKeys = serverConf.hostKeys
+	transport.publicKeyAuthAlgorithms = serverConf.PublicKeyAuthAlgorithms
+
+	readOneFailure := make(chan error, 1)
+	go func() {
+		if _, err := transport.readOnePacket(true); err != nil {
+			readOneFailure <- err
+		}
+	}()
+
+	// Basically sendKexInit, but without the kex-strict extension algorithm
+	msg := &kexInitMsg{
+		KexAlgos:                transport.config.KeyExchanges,
+		CiphersClientServer:     transport.config.Ciphers,
+		CiphersServerClient:     transport.config.Ciphers,
+		MACsClientServer:        transport.config.MACs,
+		MACsServerClient:        transport.config.MACs,
+		CompressionClientServer: supportedCompressions,
+		CompressionServerClient: supportedCompressions,
+		ServerHostKeyAlgos:      []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512, KeyAlgoRSA},
+	}
+	packet := Marshal(msg)
+	// writePacket destroys the contents, so save a copy.
+	packetCopy := make([]byte, len(packet))
+	copy(packetCopy, packet)
+	if err := transport.pushPacket(packetCopy); err != nil {
+		t.Fatalf("pushPacket: %s", err)
+	}
+	transport.sentInitMsg = msg
+	transport.sentInitPacket = packet
+
+	if err := transport.getWriteError(); err != nil {
+		t.Fatalf("getWriteError failed: %s", err)
+	}
+	var request *pendingKex
+	select {
+	case err = <-readOneFailure:
+		t.Fatalf("server readOnePacket failed: %s", err)
+	case request = <-transport.startKex:
+		break
+	}
+
+	// We expect the following calls to fail if the side which does not support
+	// kex-strict sends unexpected/ignored packets during the handshake, even if
+	// the other side does support kex-strict.
+
+	if err := transport.enterKeyExchange(request.otherInit); err != nil {
+		t.Fatalf("enterKeyExchange failed: %s", err)
+	}
+	if err := client.waitSession(); err != nil {
+		t.Fatalf("client.waitSession: %v", err)
+	}
+}
diff --git a/ssh/transport.go b/ssh/transport.go
index da01580..0424d2d 100644
--- a/ssh/transport.go
+++ b/ssh/transport.go
@@ -47,10 +47,13 @@ type transport struct {
 	bufReader *bufio.Reader
 	bufWriter *bufio.Writer
 	rand      io.Reader
 	isClient  bool
 	io.Closer
+
+	strictMode     bool
+	initialKEXDone bool
 }
 
 // packetCipher represents a combination of SSH encryption/MAC
 // protocol.  A single instance should be used for one direction only.
 type packetCipher interface {
@@ -72,10 +75,22 @@ type connectionState struct {
 	seqNum           uint32
 	dir              direction
 	pendingKeyChange chan packetCipher
 }
 
+func (t *transport) setStrictMode() error {
+	if t.reader.seqNum != 1 {
+		return errors.New("ssh: sequence number != 1 when strict KEX mode requested")
+	}
+	t.strictMode = true
+	return nil
+}
+
+func (t *transport) setInitialKEXDone() {
+	t.initialKEXDone = true
+}
+
 // prepareKeyChange sets up key material for a keychange. The key changes in
 // both directions are triggered by reading and writing a msgNewKey packet
 // respectively.
 func (t *transport) prepareKeyChange(algs *algorithms, kexResult *kexResult) error {
 	ciph, err := newPacketCipher(t.reader.dir, algs.r, kexResult)
@@ -110,26 +125,27 @@ func (t *transport) printPacket(p []byte, write bool) {
 }
 
 // Read and decrypt next packet.
 func (t *transport) readPacket() (p []byte, err error) {
 	for {
-		p, err = t.reader.readPacket(t.bufReader)
+		p, err = t.reader.readPacket(t.bufReader, t.strictMode)
 		if err != nil {
 			break
 		}
-		if len(p) == 0 || (p[0] != msgIgnore && p[0] != msgDebug) {
+		// in strict mode we pass through DEBUG and IGNORE packets only during the initial KEX
+		if len(p) == 0 || (t.strictMode && !t.initialKEXDone) || (p[0] != msgIgnore && p[0] != msgDebug) {
 			break
 		}
 	}
 	if debugTransport {
 		t.printPacket(p, false)
 	}
 
 	return p, err
 }
 
-func (s *connectionState) readPacket(r *bufio.Reader) ([]byte, error) {
+func (s *connectionState) readPacket(r *bufio.Reader, strictMode bool) ([]byte, error) {
 	packet, err := s.packetCipher.readCipherPacket(s.seqNum, r)
 	s.seqNum++
 	if err == nil && len(packet) == 0 {
 		err = errors.New("ssh: zero length packet")
 	}
@@ -138,10 +154,13 @@ func (s *connectionState) readPacket(r *bufio.Reader) ([]byte, error) {
 		switch packet[0] {
 		case msgNewKeys:
 			select {
 			case cipher := <-s.pendingKeyChange:
 				s.packetCipher = cipher
+				if strictMode {
+					s.seqNum = 0
+				}
 			default:
 				return nil, errors.New("ssh: got bogus newkeys message")
 			}
 
 		case msgDisconnect:
@@ -168,14 +187,14 @@ func (s *connectionState) readPacket(r *bufio.Reader) ([]byte, error) {
 
 func (t *transport) writePacket(packet []byte) error {
 	if debugTransport {
 		t.printPacket(packet, true)
 	}
-	return t.writer.writePacket(t.bufWriter, t.rand, packet)
+	return t.writer.writePacket(t.bufWriter, t.rand, packet, t.strictMode)
 }
 
-func (s *connectionState) writePacket(w *bufio.Writer, rand io.Reader, packet []byte) error {
+func (s *connectionState) writePacket(w *bufio.Writer, rand io.Reader, packet []byte, strictMode bool) error {
 	changeKeys := len(packet) > 0 && packet[0] == msgNewKeys
 
 	err := s.packetCipher.writeCipherPacket(s.seqNum, w, rand, packet)
 	if err != nil {
 		return err
@@ -186,10 +205,13 @@ func (s *connectionState) writePacket(w *bufio.Writer, rand io.Reader, packet []
 	s.seqNum++
 	if changeKeys {
 		select {
 		case cipher := <-s.pendingKeyChange:
 			s.packetCipher = cipher
+			if strictMode {
+				s.seqNum = 0
+			}
 		default:
 			panic("ssh: no key material for msgNewKeys")
 		}
 	}
 	return err

Vulnerable code: 		if p[0] == msgIgnore || p[0] == msgDebug {
		KexAlgos:                t.config.KeyExchanges,
			msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+1)
			msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...)
		p, err = t.reader.readPacket(t.bufReader)
		if len(p) == 0 || (p[0] != msgIgnore && p[0] != msgDebug) {
func (s *connectionState) readPacket(r *bufio.Reader) ([]byte, error) {
	return t.writer.writePacket(t.bufWriter, t.rand, packet)
func (s *connectionState) writePacket(w *bufio.Writer, rand io.Reader, packet []byte) error {
Fixed code: 
	// setStrictMode sets the strict KEX mode, notably triggering
	// sequence number resets on sending or receiving msgNewKeys.
	// If the sequence number is already > 1 when setStrictMode
	// is called, an error is returned.
	setStrictMode() error

	// setInitialKEXDone indicates to the transport that the initial key exchange
	// was completed
	setInitialKEXDone()

	// strictMode indicates if the other side of the handshake indicated
	// that we should be following the strict KEX protocol restrictions.
	strictMode bool
		// If this is the first kex, and strict KEX mode is enabled,
		// we don't ignore any messages, as they may be used to manipulate
		// the packet sequence numbers.
		if !(t.sessionID == nil && t.strictMode) && (p[0] == msgIgnore || p[0] == msgDebug) {
const (
	kexStrictClient = "kex-strict-c-v00@openssh.com"
	kexStrictServer = "kex-strict-s-v00@openssh.com"
)

	// We mutate the KexAlgos slice, in order to add the kex-strict extension algorithm,
	// and possibly to add the ext-info extension algorithm. Since the slice may be the
	// user owned KeyExchanges, we create our own slice in order to avoid using user
	// owned memory by mistake.
	msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+2) // room for kex-strict and ext-info
	msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...)


		if t.sessionID == nil {
			msg.KexAlgos = append(msg.KexAlgos, kexStrictServer)
		}
		//
		// We also send the strict KEX mode extension algorithm, in order to opt
		// into the strict KEX mode.
			msg.KexAlgos = append(msg.KexAlgos, kexStrictClient)

	if t.sessionID == nil && ((isClient && contains(serverInit.KexAlgos, kexStrictServer)) || (!isClient && contains(clientInit.KexAlgos, kexStrictClient))) {
		t.strictMode = true
		if err := t.conn.setStrictMode(); err != nil {
			return err
		}
	}

	if firstKeyExchange {
		// Indicates to the transport that the first key exchange is completed
		// after receiving SSH_MSG_NEWKEYS.
		t.conn.setInitialKEXDone()
	}

func (n *errorKeyingTransport) setStrictMode() error { return nil }

func (n *errorKeyingTransport) setInitialKEXDone() {}


func TestStrictKEXResetSeqFirstKEX(t *testing.T) {
	if runtime.GOOS == "plan9" {
		t.Skip("see golang.org/issue/7237")
	}

	checker := &syncChecker{
		waitCall: make(chan int, 10),
		called:   make(chan int, 10),
	}

	checker.waitCall <- 1
	trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", false)
	if err != nil {
		t.Fatalf("handshakePair: %v", err)
	}
	<-checker.called

	t.Cleanup(func() {
		trC.Close()
		trS.Close()
	})

	// Throw away the msgExtInfo packet sent during the handshake by the server
	_, err = trC.readPacket()
	if err != nil {
		t.Fatalf("readPacket failed: %s", err)
	}

	// close the handshake transports before checking the sequence number to
	// avoid races.
	trC.Close()
	trS.Close()

	// check that the sequence number counters. We reset after msgNewKeys, but
	// then the server immediately writes msgExtInfo, and we close the
	// transports so we expect read 2, write 0 on the client and read 1, write 1
	// on the server.
	if trC.conn.(*transport).reader.seqNum != 2 || trC.conn.(*transport).writer.seqNum != 0 ||
		trS.conn.(*transport).reader.seqNum != 1 || trS.conn.(*transport).writer.seqNum != 1 {
		t.Errorf(
			"unexpected sequence counters:\nclient: reader %d (expected 2), writer %d (expected 0)\nserver: reader %d (expected 1), writer %d (expected 1)",
			trC.conn.(*transport).reader.seqNum,
			trC.conn.(*transport).writer.seqNum,
			trS.conn.(*transport).reader.seqNum,
			trS.conn.(*transport).writer.seqNum,
		)
	}
}

func TestStrictKEXResetSeqSuccessiveKEX(t *testing.T) {
	if runtime.GOOS == "plan9" {
		t.Skip("see golang.org/issue/7237")
	}

	checker := &syncChecker{
		waitCall: make(chan int, 10),
		called:   make(chan int, 10),
	}

	checker.waitCall <- 1
	trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", false)
	if err != nil {
		t.Fatalf("handshakePair: %v", err)
	}
	<-checker.called

	t.Cleanup(func() {
		trC.Close()
		trS.Close()
	})

	// Throw away the msgExtInfo packet sent during the handshake by the server
	_, err = trC.readPacket()
	if err != nil {
		t.Fatalf("readPacket failed: %s", err)
	}

	// write and read five packets on either side to bump the sequence numbers
	for i := 0; i < 5; i++ {
		if err := trC.writePacket([]byte{msgRequestSuccess}); err != nil {
			t.Fatalf("writePacket failed: %s", err)
		}
		if _, err := trS.readPacket(); err != nil {
			t.Fatalf("readPacket failed: %s", err)
		}
		if err := trS.writePacket([]byte{msgRequestSuccess}); err != nil {
			t.Fatalf("writePacket failed: %s", err)
		}
		if _, err := trC.readPacket(); err != nil {
			t.Fatalf("readPacket failed: %s", err)
		}
	}

	// Request a key exchange, which should cause the sequence numbers to reset
	checker.waitCall <- 1
	trC.requestKeyExchange()
	<-checker.called

	// write a packet on the client, and then read it, to verify the key change has actually happened, since
	// the HostKeyCallback is called _during_ the handshake, so isn't actually indicative of the handshake
	// finishing.
	dummyPacket := []byte{99}
	if err := trS.writePacket(dummyPacket); err != nil {
		t.Fatalf("writePacket failed: %s", err)
	}
	if p, err := trC.readPacket(); err != nil {
		t.Fatalf("readPacket failed: %s", err)
	} else if !bytes.Equal(p, dummyPacket) {
		t.Fatalf("unexpected packet: got %x, want %x", p, dummyPacket)
	}

	// close the handshake transports before checking the sequence number to
	// avoid races.
	trC.Close()
	trS.Close()

	if trC.conn.(*transport).reader.seqNum != 2 || trC.conn.(*transport).writer.seqNum != 0 ||
		trS.conn.(*transport).reader.seqNum != 1 || trS.conn.(*transport).writer.seqNum != 1 {
		t.Errorf(
			"unexpected sequence counters:\nclient: reader %d (expected 2), writer %d (expected 0)\nserver: reader %d (expected 1), writer %d (expected 1)",
			trC.conn.(*transport).reader.seqNum,
			trC.conn.(*transport).writer.seqNum,
			trS.conn.(*transport).reader.seqNum,
			trS.conn.(*transport).writer.seqNum,
		)
	}
}

func TestSeqNumIncrease(t *testing.T) {
	if runtime.GOOS == "plan9" {
		t.Skip("see golang.org/issue/7237")
	}

	checker := &syncChecker{
		waitCall: make(chan int, 10),
		called:   make(chan int, 10),
	}

	checker.waitCall <- 1
	trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: checker.Check}, "addr", false)
	if err != nil {
		t.Fatalf("handshakePair: %v", err)
	}
	<-checker.called

	t.Cleanup(func() {
		trC.Close()
		trS.Close()
	})

	// Throw away the msgExtInfo packet sent during the handshake by the server
	_, err = trC.readPacket()
	if err != nil {
		t.Fatalf("readPacket failed: %s", err)
	}

	// write and read five packets on either side to bump the sequence numbers
	for i := 0; i < 5; i++ {
		if err := trC.writePacket([]byte{msgRequestSuccess}); err != nil {
			t.Fatalf("writePacket failed: %s", err)
		}
		if _, err := trS.readPacket(); err != nil {
			t.Fatalf("readPacket failed: %s", err)
		}
		if err := trS.writePacket([]byte{msgRequestSuccess}); err != nil {
			t.Fatalf("writePacket failed: %s", err)
		}
		if _, err := trC.readPacket(); err != nil {
			t.Fatalf("readPacket failed: %s", err)
		}
	}

	// close the handshake transports before checking the sequence number to
	// avoid races.
	trC.Close()
	trS.Close()

	if trC.conn.(*transport).reader.seqNum != 7 || trC.conn.(*transport).writer.seqNum != 5 ||
		trS.conn.(*transport).reader.seqNum != 6 || trS.conn.(*transport).writer.seqNum != 6 {
		t.Errorf(
			"unexpected sequence counters:\nclient: reader %d (expected 7), writer %d (expected 5)\nserver: reader %d (expected 6), writer %d (expected 6)",
			trC.conn.(*transport).reader.seqNum,
			trC.conn.(*transport).writer.seqNum,
			trS.conn.(*transport).reader.seqNum,
			trS.conn.(*transport).writer.seqNum,
		)
	}
}

func TestStrictKEXUnexpectedMsg(t *testing.T) {
	if runtime.GOOS == "plan9" {
		t.Skip("see golang.org/issue/7237")
	}

	// Check that unexpected messages during the handshake cause failure
	_, _, err := handshakePair(&ClientConfig{HostKeyCallback: func(hostname string, remote net.Addr, key PublicKey) error { return nil }}, "addr", true)
	if err == nil {
		t.Fatal("handshake should fail when there are unexpected messages during the handshake")
	}

	trC, trS, err := handshakePair(&ClientConfig{HostKeyCallback: func(hostname string, remote net.Addr, key PublicKey) error { return nil }}, "addr", false)
	if err != nil {
		t.Fatalf("handshake failed: %s", err)
	}

	// Check that ignore/debug pacekts are still ignored outside of the handshake
	if err := trC.writePacket([]byte{msgIgnore}); err != nil {
		t.Fatalf("writePacket failed: %s", err)
	}
	if err := trC.writePacket([]byte{msgDebug}); err != nil {
		t.Fatalf("writePacket failed: %s", err)
	}
	dummyPacket := []byte{99}
	if err := trC.writePacket(dummyPacket); err != nil {
		t.Fatalf("writePacket failed: %s", err)
	}

	if p, err := trS.readPacket(); err != nil {
		t.Fatalf("readPacket failed: %s", err)
	} else if !bytes.Equal(p, dummyPacket) {
		t.Fatalf("unexpected packet: got %x, want %x", p, dummyPacket)
	}
}

func TestStrictKEXMixed(t *testing.T) {
	// Test that we still support a mixed connection, where one side sends kex-strict but the other
	// side doesn't.

	a, b, err := netPipe()
	if err != nil {
		t.Fatalf("netPipe failed: %s", err)
	}

	var trC, trS keyingTransport

	trC = newTransport(a, rand.Reader, true)
	trS = newTransport(b, rand.Reader, false)
	trS = addNoiseTransport(trS)

	clientConf := &ClientConfig{HostKeyCallback: func(hostname string, remote net.Addr, key PublicKey) error { return nil }}
	clientConf.SetDefaults()

	v := []byte("version")
	client := newClientTransport(trC, v, v, clientConf, "addr", a.RemoteAddr())

	serverConf := &ServerConfig{}
	serverConf.AddHostKey(testSigners["ecdsa"])
	serverConf.AddHostKey(testSigners["rsa"])
	serverConf.SetDefaults()

	transport := newHandshakeTransport(trS, &serverConf.Config, []byte("version"), []byte("version"))
	transport.hostKeys = serverConf.hostKeys
	transport.publicKeyAuthAlgorithms = serverConf.PublicKeyAuthAlgorithms

	readOneFailure := make(chan error, 1)
	go func() {
		if _, err := transport.readOnePacket(true); err != nil {
			readOneFailure <- err
		}
	}()

	// Basically sendKexInit, but without the kex-strict extension algorithm
	msg := &kexInitMsg{
		KexAlgos:                transport.config.KeyExchanges,
		CiphersClientServer:     transport.config.Ciphers,
		CiphersServerClient:     transport.config.Ciphers,
		MACsClientServer:        transport.config.MACs,
		MACsServerClient:        transport.config.MACs,
		CompressionClientServer: supportedCompressions,
		CompressionServerClient: supportedCompressions,
		ServerHostKeyAlgos:      []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512, KeyAlgoRSA},
	}
	packet := Marshal(msg)
	// writePacket destroys the contents, so save a copy.
	packetCopy := make([]byte, len(packet))
	copy(packetCopy, packet)
	if err := transport.pushPacket(packetCopy); err != nil {
		t.Fatalf("pushPacket: %s", err)
	}
	transport.sentInitMsg = msg
	transport.sentInitPacket = packet

	if err := transport.getWriteError(); err != nil {
		t.Fatalf("getWriteError failed: %s", err)
	}
	var request *pendingKex
	select {
	case err = <-readOneFailure:
		t.Fatalf("server readOnePacket failed: %s", err)
	case request = <-transport.startKex:
		break
	}

	// We expect the following calls to fail if the side which does not support
	// kex-strict sends unexpected/ignored packets during the handshake, even if
	// the other side does support kex-strict.

	if err := transport.enterKeyExchange(request.otherInit); err != nil {
		t.Fatalf("enterKeyExchange failed: %s", err)
	}
	if err := client.waitSession(); err != nil {
		t.Fatalf("client.waitSession: %v", err)
	}
}

	strictMode     bool
	initialKEXDone bool
func (t *transport) setStrictMode() error {
	if t.reader.seqNum != 1 {
		return errors.New("ssh: sequence number != 1 when strict KEX mode requested")
	}
	t.strictMode = true
	return nil
}

func (t *transport) setInitialKEXDone() {
	t.initialKEXDone = true
}

		p, err = t.reader.readPacket(t.bufReader, t.strictMode)
		// in strict mode we pass through DEBUG and IGNORE packets only during the initial KEX
		if len(p) == 0 || (t.strictMode && !t.initialKEXDone) || (p[0] != msgIgnore && p[0] != msgDebug) {
func (s *connectionState) readPacket(r *bufio.Reader, strictMode bool) ([]byte, error) {
				if strictMode {
					s.seqNum = 0
				}
	return t.writer.writePacket(t.bufWriter, t.rand, packet, t.strictMode)
func (s *connectionState) writePacket(w *bufio.Writer, rand io.Reader, packet []byte, strictMode bool) error {
			if strictMode {
				s.seqNum = 0
			}