|
| 1 | +From 9d2ee975ef9fe627bf0a6f01c1f69e8ef1d4f05d Mon Sep 17 00:00:00 2001 |
| 2 | +From: Roland Shoemaker <bracewell@google.com> |
| 3 | +Date: Mon, 20 Nov 2023 12:06:18 -0800 |
| 4 | +Subject: [PATCH] ssh: implement strict KEX protocol changes |
| 5 | +MIME-Version: 1.0 |
| 6 | +Content-Type: text/plain; charset=UTF-8 |
| 7 | +Content-Transfer-Encoding: 8bit |
| 8 | + |
| 9 | +Implement the "strict KEX" protocol changes, as described in section |
| 10 | +1.9 of the OpenSSH PROTOCOL file (as of OpenSSH version 9.6/9.6p1). |
| 11 | + |
| 12 | +Namely this makes the following changes: |
| 13 | + * Both the server and the client add an additional algorithm to the |
| 14 | + initial KEXINIT message, indicating support for the strict KEX mode. |
| 15 | + * When one side of the connection sees the strict KEX extension |
| 16 | + algorithm, the strict KEX mode is enabled for messages originating |
| 17 | + from the other side of the connection. If the sequence number for |
| 18 | + the side which requested the extension is not 1 (indicating that it |
| 19 | + has already received non-KEXINIT packets), the connection is |
| 20 | + terminated. |
| 21 | + * When strict kex mode is enabled, unexpected messages during the |
| 22 | + handshake are considered fatal. Additionally when a key change |
| 23 | + occurs (on the receipt of the NEWKEYS message) the message sequence |
| 24 | + numbers are reset. |
| 25 | + |
| 26 | +Thanks to Fabian Bäumer, Marcus Brinkmann, and Jörg Schwenk from Ruhr |
| 27 | +University Bochum for reporting this issue. |
| 28 | + |
| 29 | +Fixes CVE-2023-48795 |
| 30 | +Fixes golang/go#64784 |
| 31 | + |
| 32 | +Change-Id: I96b53afd2bd2fb94d2b6f2a46a5dacf325357604 |
| 33 | +Reviewed-on: https://go-review.googlesource.com/c/crypto/+/550715 |
| 34 | +Reviewed-by: Nicola Murino <nicola.murino@gmail.com> |
| 35 | +Reviewed-by: Tatiana Bradley <tatianabradley@google.com> |
| 36 | +TryBot-Result: Gopher Robot <gobot@golang.org> |
| 37 | +Run-TryBot: Roland Shoemaker <roland@golang.org> |
| 38 | +Reviewed-by: Damien Neil <dneil@google.com> |
| 39 | +LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> |
| 40 | +--- |
| 41 | + vendor/golang.org/x/crypto/ssh/handshake.go | 59 +++++++++++++++++++-- |
| 42 | + vendor/golang.org/x/crypto/ssh/transport.go | 32 +++++++++-- |
| 43 | + 2 files changed, 81 insertions(+), 10 deletions(-) |
| 44 | + |
| 45 | +diff --git a/vendor/golang.org/x/crypto/ssh/handshake.go b/vendor/golang.org/x/crypto/ssh/handshake.go |
| 46 | +index 653dc4d..e7d4545 100644 |
| 47 | +--- a/vendor/golang.org/x/crypto/ssh/handshake.go |
| 48 | ++++ b/vendor/golang.org/x/crypto/ssh/handshake.go |
| 49 | +@@ -34,6 +34,16 @@ type keyingTransport interface { |
| 50 | + // direction will be effected if a msgNewKeys message is sent |
| 51 | + // or received. |
| 52 | + prepareKeyChange(*algorithms, *kexResult) error |
| 53 | ++ |
| 54 | ++ // setStrictMode sets the strict KEX mode, notably triggering |
| 55 | ++ // sequence number resets on sending or receiving msgNewKeys. |
| 56 | ++ // If the sequence number is already > 1 when setStrictMode |
| 57 | ++ // is called, an error is returned. |
| 58 | ++ setStrictMode() error |
| 59 | ++ |
| 60 | ++ // setInitialKEXDone indicates to the transport that the initial key exchange |
| 61 | ++ // was completed |
| 62 | ++ setInitialKEXDone() |
| 63 | + } |
| 64 | + |
| 65 | + // handshakeTransport implements rekeying on top of a keyingTransport |
| 66 | +@@ -94,6 +104,10 @@ type handshakeTransport struct { |
| 67 | + |
| 68 | + // The session ID or nil if first kex did not complete yet. |
| 69 | + sessionID []byte |
| 70 | ++ |
| 71 | ++ // strictMode indicates if the other side of the handshake indicated |
| 72 | ++ // that we should be following the strict KEX protocol restrictions. |
| 73 | ++ strictMode bool |
| 74 | + } |
| 75 | + |
| 76 | + type pendingKex struct { |
| 77 | +@@ -201,7 +215,10 @@ func (t *handshakeTransport) readLoop() { |
| 78 | + close(t.incoming) |
| 79 | + break |
| 80 | + } |
| 81 | +- if p[0] == msgIgnore || p[0] == msgDebug { |
| 82 | ++ // If this is the first kex, and strict KEX mode is enabled, |
| 83 | ++ // we don't ignore any messages, as they may be used to manipulate |
| 84 | ++ // the packet sequence numbers. |
| 85 | ++ if !(t.sessionID == nil && t.strictMode) && (p[0] == msgIgnore || p[0] == msgDebug) { |
| 86 | + continue |
| 87 | + } |
| 88 | + t.incoming <- p |
| 89 | +@@ -432,6 +449,11 @@ func (t *handshakeTransport) readOnePacket(first bool) ([]byte, error) { |
| 90 | + return successPacket, nil |
| 91 | + } |
| 92 | + |
| 93 | ++const ( |
| 94 | ++ kexStrictClient = "kex-strict-c-v00@openssh.com" |
| 95 | ++ kexStrictServer = "kex-strict-s-v00@openssh.com" |
| 96 | ++) |
| 97 | ++ |
| 98 | + // sendKexInit sends a key change message. |
| 99 | + func (t *handshakeTransport) sendKexInit() error { |
| 100 | + t.mu.Lock() |
| 101 | +@@ -445,7 +467,6 @@ func (t *handshakeTransport) sendKexInit() error { |
| 102 | + } |
| 103 | + |
| 104 | + msg := &kexInitMsg{ |
| 105 | +- KexAlgos: t.config.KeyExchanges, |
| 106 | + CiphersClientServer: t.config.Ciphers, |
| 107 | + CiphersServerClient: t.config.Ciphers, |
| 108 | + MACsClientServer: t.config.MACs, |
| 109 | +@@ -455,6 +476,13 @@ func (t *handshakeTransport) sendKexInit() error { |
| 110 | + } |
| 111 | + io.ReadFull(rand.Reader, msg.Cookie[:]) |
| 112 | + |
| 113 | ++ // We mutate the KexAlgos slice, in order to add the kex-strict extension algorithm, |
| 114 | ++ // and possibly to add the ext-info extension algorithm. Since the slice may be the |
| 115 | ++ // user owned KeyExchanges, we create our own slice in order to avoid using user |
| 116 | ++ // owned memory by mistake. |
| 117 | ++ msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+2) // room for kex-strict and ext-info |
| 118 | ++ msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...) |
| 119 | ++ |
| 120 | + isServer := len(t.hostKeys) > 0 |
| 121 | + if isServer { |
| 122 | + for _, k := range t.hostKeys { |
| 123 | +@@ -474,17 +502,24 @@ func (t *handshakeTransport) sendKexInit() error { |
| 124 | + msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, keyFormat) |
| 125 | + } |
| 126 | + } |
| 127 | ++ |
| 128 | ++ if t.sessionID == nil { |
| 129 | ++ msg.KexAlgos = append(msg.KexAlgos, kexStrictServer) |
| 130 | ++ } |
| 131 | + } else { |
| 132 | + msg.ServerHostKeyAlgos = t.hostKeyAlgorithms |
| 133 | + |
| 134 | + // As a client we opt in to receiving SSH_MSG_EXT_INFO so we know what |
| 135 | + // algorithms the server supports for public key authentication. See RFC |
| 136 | + // 8308, Section 2.1. |
| 137 | ++ // |
| 138 | ++ // We also send the strict KEX mode extension algorithm, in order to opt |
| 139 | ++ // into the strict KEX mode. |
| 140 | + if firstKeyExchange := t.sessionID == nil; firstKeyExchange { |
| 141 | +- msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+1) |
| 142 | +- msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...) |
| 143 | + msg.KexAlgos = append(msg.KexAlgos, "ext-info-c") |
| 144 | ++ msg.KexAlgos = append(msg.KexAlgos, kexStrictClient) |
| 145 | + } |
| 146 | ++ |
| 147 | + } |
| 148 | + |
| 149 | + packet := Marshal(msg) |
| 150 | +@@ -581,6 +616,13 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { |
| 151 | + return err |
| 152 | + } |
| 153 | + |
| 154 | ++ if t.sessionID == nil && ((isClient && contains(serverInit.KexAlgos, kexStrictServer)) || (!isClient && contains(clientInit.KexAlgos, kexStrictClient))) { |
| 155 | ++ t.strictMode = true |
| 156 | ++ if err := t.conn.setStrictMode(); err != nil { |
| 157 | ++ return err |
| 158 | ++ } |
| 159 | ++ } |
| 160 | ++ |
| 161 | + // We don't send FirstKexFollows, but we handle receiving it. |
| 162 | + // |
| 163 | + // RFC 4253 section 7 defines the kex and the agreement method for |
| 164 | +@@ -615,7 +657,8 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { |
| 165 | + return err |
| 166 | + } |
| 167 | + |
| 168 | +- if t.sessionID == nil { |
| 169 | ++ firstKeyExchange := t.sessionID == nil |
| 170 | ++ if firstKeyExchange { |
| 171 | + t.sessionID = result.H |
| 172 | + } |
| 173 | + result.SessionID = t.sessionID |
| 174 | +@@ -632,6 +675,12 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { |
| 175 | + return unexpectedMessageError(msgNewKeys, packet[0]) |
| 176 | + } |
| 177 | + |
| 178 | ++ if firstKeyExchange { |
| 179 | ++ // Indicates to the transport that the first key exchange is completed |
| 180 | ++ // after receiving SSH_MSG_NEWKEYS. |
| 181 | ++ t.conn.setInitialKEXDone() |
| 182 | ++ } |
| 183 | ++ |
| 184 | + return nil |
| 185 | + } |
| 186 | + |
| 187 | +diff --git a/vendor/golang.org/x/crypto/ssh/transport.go b/vendor/golang.org/x/crypto/ssh/transport.go |
| 188 | +index acf5a21..4df45fc 100644 |
| 189 | +--- a/vendor/golang.org/x/crypto/ssh/transport.go |
| 190 | ++++ b/vendor/golang.org/x/crypto/ssh/transport.go |
| 191 | +@@ -48,6 +48,9 @@ type transport struct { |
| 192 | + rand io.Reader |
| 193 | + isClient bool |
| 194 | + io.Closer |
| 195 | ++ |
| 196 | ++ strictMode bool |
| 197 | ++ initialKEXDone bool |
| 198 | + } |
| 199 | + |
| 200 | + // packetCipher represents a combination of SSH encryption/MAC |
| 201 | +@@ -73,6 +76,18 @@ type connectionState struct { |
| 202 | + pendingKeyChange chan packetCipher |
| 203 | + } |
| 204 | + |
| 205 | ++func (t *transport) setStrictMode() error { |
| 206 | ++ if t.reader.seqNum != 1 { |
| 207 | ++ return errors.New("ssh: sequence number != 1 when strict KEX mode requested") |
| 208 | ++ } |
| 209 | ++ t.strictMode = true |
| 210 | ++ return nil |
| 211 | ++} |
| 212 | ++ |
| 213 | ++func (t *transport) setInitialKEXDone() { |
| 214 | ++ t.initialKEXDone = true |
| 215 | ++} |
| 216 | ++ |
| 217 | + // prepareKeyChange sets up key material for a keychange. The key changes in |
| 218 | + // both directions are triggered by reading and writing a msgNewKey packet |
| 219 | + // respectively. |
| 220 | +@@ -111,11 +126,12 @@ func (t *transport) printPacket(p []byte, write bool) { |
| 221 | + // Read and decrypt next packet. |
| 222 | + func (t *transport) readPacket() (p []byte, err error) { |
| 223 | + for { |
| 224 | +- p, err = t.reader.readPacket(t.bufReader) |
| 225 | ++ p, err = t.reader.readPacket(t.bufReader, t.strictMode) |
| 226 | + if err != nil { |
| 227 | + break |
| 228 | + } |
| 229 | +- if len(p) == 0 || (p[0] != msgIgnore && p[0] != msgDebug) { |
| 230 | ++ // in strict mode we pass through DEBUG and IGNORE packets only during the initial KEX |
| 231 | ++ if len(p) == 0 || (t.strictMode && !t.initialKEXDone) || (p[0] != msgIgnore && p[0] != msgDebug) { |
| 232 | + break |
| 233 | + } |
| 234 | + } |
| 235 | +@@ -126,7 +142,7 @@ func (t *transport) readPacket() (p []byte, err error) { |
| 236 | + return p, err |
| 237 | + } |
| 238 | + |
| 239 | +-func (s *connectionState) readPacket(r *bufio.Reader) ([]byte, error) { |
| 240 | ++func (s *connectionState) readPacket(r *bufio.Reader, strictMode bool) ([]byte, error) { |
| 241 | + packet, err := s.packetCipher.readCipherPacket(s.seqNum, r) |
| 242 | + s.seqNum++ |
| 243 | + if err == nil && len(packet) == 0 { |
| 244 | +@@ -139,6 +155,9 @@ func (s *connectionState) readPacket(r *bufio.Reader) ([]byte, error) { |
| 245 | + select { |
| 246 | + case cipher := <-s.pendingKeyChange: |
| 247 | + s.packetCipher = cipher |
| 248 | ++ if strictMode { |
| 249 | ++ s.seqNum = 0 |
| 250 | ++ } |
| 251 | + default: |
| 252 | + return nil, errors.New("ssh: got bogus newkeys message") |
| 253 | + } |
| 254 | +@@ -169,10 +188,10 @@ func (t *transport) writePacket(packet []byte) error { |
| 255 | + if debugTransport { |
| 256 | + t.printPacket(packet, true) |
| 257 | + } |
| 258 | +- return t.writer.writePacket(t.bufWriter, t.rand, packet) |
| 259 | ++ return t.writer.writePacket(t.bufWriter, t.rand, packet, t.strictMode) |
| 260 | + } |
| 261 | + |
| 262 | +-func (s *connectionState) writePacket(w *bufio.Writer, rand io.Reader, packet []byte) error { |
| 263 | ++func (s *connectionState) writePacket(w *bufio.Writer, rand io.Reader, packet []byte, strictMode bool) error { |
| 264 | + changeKeys := len(packet) > 0 && packet[0] == msgNewKeys |
| 265 | + |
| 266 | + err := s.packetCipher.writeCipherPacket(s.seqNum, w, rand, packet) |
| 267 | +@@ -187,6 +206,9 @@ func (s *connectionState) writePacket(w *bufio.Writer, rand io.Reader, packet [] |
| 268 | + select { |
| 269 | + case cipher := <-s.pendingKeyChange: |
| 270 | + s.packetCipher = cipher |
| 271 | ++ if strictMode { |
| 272 | ++ s.seqNum = 0 |
| 273 | ++ } |
| 274 | + default: |
| 275 | + panic("ssh: no key material for msgNewKeys") |
| 276 | + } |
| 277 | +-- |
| 278 | +2.25.1 |
| 279 | + |
0 commit comments