CVE-2023-48795 (Go, published 2023-12-18T16:15Z)
Description: 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.
Severity: nan (CVSS v3 base score: 5.9)
Weakness: CWE-354 \u2014 Improper Validation of Integrity Check Value
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.
Repository: https://github.com/golang/crypto
Files changed:
ssh/handshake.go
ssh/handshake_test.go
ssh/transport.go
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>


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
			}