-
Notifications
You must be signed in to change notification settings - Fork 551
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
node-problem-detector: Add patch to resolve CVE-2023-48795
- Loading branch information
Showing
2 changed files
with
277 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,269 @@ | ||
From 9d2ee975ef9fe627bf0a6f01c1f69e8ef1d4f05d Mon Sep 17 00:00:00 2001 | ||
From: Roland Shoemaker <[email protected]> | ||
Date: Mon, 20 Nov 2023 12:06:18 -0800 | ||
Subject: [PATCH] ssh: implement strict KEX protocol changes | ||
MIME-Version: 1.0 | ||
Content-Type: text/plain; charset=UTF-8 | ||
Content-Transfer-Encoding: 8bit | ||
|
||
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 <[email protected]> | ||
Reviewed-by: Tatiana Bradley <[email protected]> | ||
TryBot-Result: Gopher Robot <[email protected]> | ||
Run-TryBot: Roland Shoemaker <[email protected]> | ||
Reviewed-by: Damien Neil <[email protected]> | ||
LUCI-TryBot-Result: Go LUCI <[email protected]> | ||
--- | ||
.../golang.org/x/crypto/ssh/handshake.go | 56 +++++++++++++++++-- | ||
.../golang.org/x/crypto/ssh/transport.go | 32 +++++++++-- | ||
2 files changed, 79 insertions(+), 9 deletions(-) | ||
|
||
diff --git a/test/vendor/golang.org/x/crypto/ssh/handshake.go b/test/vendor/golang.org/x/crypto/ssh/handshake.go | ||
index 70a7369..59cef62 100644 | ||
--- a/test/vendor/golang.org/x/crypto/ssh/handshake.go | ||
+++ b/test/vendor/golang.org/x/crypto/ssh/handshake.go | ||
@@ -34,6 +34,16 @@ type keyingTransport interface { | ||
// 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 | ||
@@ -95,6 +105,10 @@ type handshakeTransport struct { | ||
|
||
// 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 { | ||
@@ -203,7 +217,10 @@ func (t *handshakeTransport) readLoop() { | ||
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 | ||
@@ -435,6 +452,11 @@ func (t *handshakeTransport) readOnePacket(first bool) ([]byte, error) { | ||
return successPacket, nil | ||
} | ||
|
||
+const ( | ||
+ kexStrictClient = "[email protected]" | ||
+ kexStrictServer = "[email protected]" | ||
+) | ||
+ | ||
// sendKexInit sends a key change message. | ||
func (t *handshakeTransport) sendKexInit() error { | ||
t.mu.Lock() | ||
@@ -448,7 +470,6 @@ func (t *handshakeTransport) sendKexInit() error { | ||
} | ||
|
||
msg := &kexInitMsg{ | ||
- KexAlgos: t.config.KeyExchanges, | ||
CiphersClientServer: t.config.Ciphers, | ||
CiphersServerClient: t.config.Ciphers, | ||
MACsClientServer: t.config.MACs, | ||
@@ -458,6 +479,13 @@ func (t *handshakeTransport) sendKexInit() error { | ||
} | ||
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 { | ||
@@ -482,17 +510,24 @@ func (t *handshakeTransport) sendKexInit() error { | ||
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) | ||
@@ -598,6 +633,13 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { | ||
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 | ||
@@ -672,6 +714,12 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { | ||
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 | ||
} | ||
|
||
diff --git a/test/vendor/golang.org/x/crypto/ssh/transport.go b/test/vendor/golang.org/x/crypto/ssh/transport.go | ||
index da01580..0424d2d 100644 | ||
--- a/test/vendor/golang.org/x/crypto/ssh/transport.go | ||
+++ b/test/vendor/golang.org/x/crypto/ssh/transport.go | ||
@@ -49,6 +49,9 @@ type transport struct { | ||
rand io.Reader | ||
isClient bool | ||
io.Closer | ||
+ | ||
+ strictMode bool | ||
+ initialKEXDone bool | ||
} | ||
|
||
// packetCipher represents a combination of SSH encryption/MAC | ||
@@ -74,6 +77,18 @@ type connectionState struct { | ||
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. | ||
@@ -112,11 +127,12 @@ 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 | ||
} | ||
} | ||
@@ -127,7 +143,7 @@ func (t *transport) readPacket() (p []byte, err error) { | ||
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 { | ||
@@ -140,6 +156,9 @@ func (s *connectionState) readPacket(r *bufio.Reader) ([]byte, error) { | ||
select { | ||
case cipher := <-s.pendingKeyChange: | ||
s.packetCipher = cipher | ||
+ if strictMode { | ||
+ s.seqNum = 0 | ||
+ } | ||
default: | ||
return nil, errors.New("ssh: got bogus newkeys message") | ||
} | ||
@@ -170,10 +189,10 @@ 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) | ||
@@ -188,6 +207,9 @@ func (s *connectionState) writePacket(w *bufio.Writer, rand io.Reader, packet [] | ||
select { | ||
case cipher := <-s.pendingKeyChange: | ||
s.packetCipher = cipher | ||
+ if strictMode { | ||
+ s.seqNum = 0 | ||
+ } | ||
default: | ||
panic("ssh: no key material for msgNewKeys") | ||
} | ||
-- | ||
2.25.1 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
Summary: Kubernetes daemon to detect and report node issues | ||
Name: node-problem-detector | ||
Version: 0.8.15 | ||
Release: 1%{?dist} | ||
Release: 2%{?dist} | ||
License: ASL 2.0 | ||
Vendor: Microsoft Corporation | ||
Distribution: Azure Linux | ||
|
@@ -39,6 +39,7 @@ Source1: %{name}-%{version}-vendor.tar.gz | |
Source2: %{name}-%{version}-test-vendor.tar.gz | ||
Patch0: 0001-remove-arch-specific-logic-from-makefile.patch | ||
Patch1: 0001-add-Mariner-and-Azure-Linux-OS-Versions.patch | ||
Patch2: CVE-2023-48795.patch | ||
BuildRequires: golang | ||
BuildRequires: systemd-devel | ||
Requires: azurelinux-release | ||
|
@@ -59,15 +60,15 @@ Requires: node-problem-detector | |
Default configuration files for node-problem-detector | ||
|
||
%prep | ||
%autosetup -p1 | ||
|
||
%build | ||
%autosetup -N | ||
# create vendor folder from the vendor tarball | ||
tar -xf %{SOURCE1} --no-same-owner | ||
pushd test | ||
tar -xf %{SOURCE2} --no-same-owner | ||
popd | ||
%autopatch -p1 | ||
|
||
%build | ||
%make_build build-binaries VERSION=%{version} | ||
|
||
%install | ||
|
@@ -99,6 +100,9 @@ make test | |
%config(noreplace) %{_sysconfdir}/node-problem-detector.d/* | ||
|
||
%changelog | ||
* Thu Nov 28 2024 Sumedh Sharma <[email protected]> - 0.8.15-2 | ||
- Add patch to resolve CVE-2023-48795 | ||
|
||
* Fri Feb 16 2024 Sean Dougherty <[email protected]> - 0.8.15-1 | ||
- Upgrade to 0.8.15 for Azure Linux 3.0 | ||
|
||
|