[pve-devel] [PATCH corosync-pve 1/3] cherry-pick CVE-2018-1084 fix

Fabian Grünbichler f.gruenbichler at proxmox.com
Fri Apr 13 10:54:06 CEST 2018


DoS via malformed packet.

Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
 ...essary-and-problematic-corosync-qdevice.i.patch |  78 ------------
 ...08-totemcrypto-Check-length-of-the-packet.patch |  53 ++++++++
 ...-totemudp-Check-lenght-of-message-to-sent.patch | 125 +++++++++++++++++++
 ...-msgio-Fix-reading-of-msg-longer-than-i32.patch | 137 +++++++++++++++++++++
 patches/series                                     |   4 +-
 5 files changed, 318 insertions(+), 79 deletions(-)
 delete mode 100644 patches/0008-remove-unecessary-and-problematic-corosync-qdevice.i.patch
 create mode 100644 patches/0008-totemcrypto-Check-length-of-the-packet.patch
 create mode 100644 patches/0012-totemudp-Check-lenght-of-message-to-sent.patch
 create mode 100644 patches/0013-qdevice-msgio-Fix-reading-of-msg-longer-than-i32.patch

diff --git a/patches/0008-remove-unecessary-and-problematic-corosync-qdevice.i.patch b/patches/0008-remove-unecessary-and-problematic-corosync-qdevice.i.patch
deleted file mode 100644
index c37216c..0000000
--- a/patches/0008-remove-unecessary-and-problematic-corosync-qdevice.i.patch
+++ /dev/null
@@ -1,78 +0,0 @@
-From f1b91ad6c1659477b72853666cde930932279d6c Mon Sep 17 00:00:00 2001
-From: Thomas Lamprecht <t.lamprecht at proxmox.com>
-Date: Wed, 29 Mar 2017 09:35:19 +0200
-Subject: [PATCH 8/8] remove unecessary and problematic corosync-qdevice.init
-MIME-Version: 1.0
-Content-Type: text/plain; charset=UTF-8
-Content-Transfer-Encoding: 8bit
-
-Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
----
- debian/corosync-qdevice.init | 55 --------------------------------------------
- 1 file changed, 55 deletions(-)
- delete mode 100755 debian/corosync-qdevice.init
-
-diff --git a/debian/corosync-qdevice.init b/debian/corosync-qdevice.init
-deleted file mode 100755
-index 344666a..0000000
---- a/debian/corosync-qdevice.init
-+++ /dev/null
-@@ -1,55 +0,0 @@
--#!/bin/sh
--# kFreeBSD do not accept scripts as interpreters, using #!/bin/sh and sourcing.
--if [ true != "$INIT_D_SCRIPT_SOURCED" ] ; then
--    set "$0" "$@"; INIT_D_SCRIPT_SOURCED=true . /lib/init/init-d-script
--fi
--### BEGIN INIT INFO
--# Provides:		corosync-qdevice
--# Required-Start:	$remote_fs $syslog corosync
--# Required-Stop:	$remote_fs $syslog corosync
--# Default-Start:	
--# Default-Stop:		0 1 6
--# Short-Description:	Corosync Qdevice daemon
--# Description:		Starts and stops Corosync Qdevice daemon.
--### END INIT INFO
--
--NAME="corosync-qdevice"
--DESC="Corosync Qdevice daemon"
--DAEMON="/usr/sbin/$NAME"
--PIDFILE="/run/$NAME/$NAME.pid"
--
--CONFIG="/etc/default/$NAME"
--[ -f "$CONFIG" ] && . "$CONFIG"
--
--DAEMON_ARGS="$COROSYNC_QDEVICE_OPTIONS"
--
--do_start_prepare() {
--    if grep -q nocluster /proc/cmdline; then
--        log_failure_msg "not configured to run at boot"
--        exit 1
--    fi
--}
--
--# do_{start,stop}_cmd from init-d-script, but without the --name option.
--# corosync-qdevice is too long for a process name, it gets truncated,
--# which makes it incompatible with the --name option.  See #843419.
--do_start_cmd() {
--	start-stop-daemon --start --quiet ${PIDFILE:+--pidfile ${PIDFILE}} \
--	    $START_ARGS \
--	    --startas $DAEMON --exec $DAEMON --test > /dev/null \
--	    || return 1
--	start-stop-daemon --start --quiet ${PIDFILE:+--pidfile ${PIDFILE}} \
--	    $START_ARGS \
--	    --startas $DAEMON --exec $DAEMON -- $DAEMON_ARGS \
--	    || return 2
--}
--do_stop_cmd() {
--	start-stop-daemon --stop --quiet --retry=TERM/30/KILL/5 \
--	    $STOP_ARGS \
--	    ${PIDFILE:+--pidfile ${PIDFILE}} --exec $DAEMON
--	RETVAL="$?"
--	[ "$RETVAL" = 2 ] && return 2
--	# Many daemons don't delete their pidfiles when they exit.
--	rm -f $PIDFILE
--	return $RETVAL
--}
--- 
-2.1.4
-
diff --git a/patches/0008-totemcrypto-Check-length-of-the-packet.patch b/patches/0008-totemcrypto-Check-length-of-the-packet.patch
new file mode 100644
index 0000000..42650b8
--- /dev/null
+++ b/patches/0008-totemcrypto-Check-length-of-the-packet.patch
@@ -0,0 +1,53 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jan Friesse <jfriesse at redhat.com>
+Date: Mon, 19 Mar 2018 16:59:41 +0100
+Subject: [PATCH] totemcrypto: Check length of the packet
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Packet has to be longer than crypto_config_header and hash_len,
+otherwise unallocated memory is passed into calculate_nss_hash function,
+what may result in crash.
+
+Signed-off-by: Jan Friesse <jfriesse at redhat.com>
+Reviewed-by: Raphael Sanchez Prudencio <rasanche at redhat.com>
+Reviewed-by: Christine Caulfield <ccaulfie at redhat.com>
+(cherry picked from commit fc1d5418533c1faf21616b282c2559bed7d361c4)
+Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
+---
+ exec/totemcrypto.c | 11 +++++++++++
+ 1 file changed, 11 insertions(+)
+
+diff --git a/exec/totemcrypto.c b/exec/totemcrypto.c
+index a97ba62f..bf30ffc2 100644
+--- a/exec/totemcrypto.c
++++ b/exec/totemcrypto.c
+@@ -627,6 +627,11 @@ static int authenticate_nss_2_3 (
+ 		unsigned char	tmp_hash[hash_len[instance->crypto_hash_type]];
+ 		int             datalen = *buf_len - hash_len[instance->crypto_hash_type];
+ 
++		if (*buf_len <= hash_len[instance->crypto_hash_type]) {
++			log_printf(instance->log_level_security, "Received message is too short...  ignoring");
++			return -1;
++		}
++
+ 		if (calculate_nss_hash(instance, buf, datalen, tmp_hash) < 0) {
+ 			return -1;
+ 		}
+@@ -736,6 +741,12 @@ int crypto_authenticate_and_decrypt (struct crypto_instance *instance,
+ {
+ 	struct crypto_config_header *cch = (struct crypto_config_header *)buf;
+ 
++	if (*buf_len <= sizeof(struct crypto_config_header)) {
++		log_printf(instance->log_level_security, "Received message is too short...  ignoring");
++
++		return (-1);
++	}
++
+ 	if (cch->crypto_cipher_type != CRYPTO_CIPHER_TYPE_2_3) {
+ 		log_printf(instance->log_level_security,
+ 			   "Incoming packet has different crypto type. Rejecting");
+-- 
+2.14.2
+
diff --git a/patches/0012-totemudp-Check-lenght-of-message-to-sent.patch b/patches/0012-totemudp-Check-lenght-of-message-to-sent.patch
new file mode 100644
index 0000000..ec883d7
--- /dev/null
+++ b/patches/0012-totemudp-Check-lenght-of-message-to-sent.patch
@@ -0,0 +1,125 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jan Friesse <jfriesse at redhat.com>
+Date: Fri, 23 Mar 2018 16:47:42 +0100
+Subject: [PATCH] totemudp: Check lenght of message to sent
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+If message to sent is too long, encrypt and authentificate may overwrite
+stack (buf_out). Check this condition and throw message if this happens.
+
+Signed-off-by: Jan Friesse <jfriesse at redhat.com>
+Reviewed-by: Christine Caulfield <ccaulfie at redhat.com>
+(cherry picked from commit 08cb2372cd3bd63a910c2618a2cc86cad8885d78)
+Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
+---
+ exec/totemcrypto.h |  3 +++
+ exec/totemcrypto.c |  8 +++++++-
+ exec/totemudp.c    | 12 ++++++++++++
+ exec/totemudpu.c   | 12 ++++++++++++
+ 4 files changed, 34 insertions(+), 1 deletion(-)
+
+diff --git a/exec/totemcrypto.h b/exec/totemcrypto.h
+index 7c06c391..b6941055 100644
+--- a/exec/totemcrypto.h
++++ b/exec/totemcrypto.h
+@@ -44,6 +44,9 @@ extern size_t crypto_sec_header_size(
+ 	const char *crypto_cipher_type,
+ 	const char *crypto_hash_type);
+ 
++extern size_t crypto_get_current_sec_header_size(
++	const struct crypto_instance *instance);
++
+ extern int crypto_authenticate_and_decrypt (
+ 	struct crypto_instance *instance,
+ 	unsigned char *buf,
+diff --git a/exec/totemcrypto.c b/exec/totemcrypto.c
+index bf30ffc2..122f15fe 100644
+--- a/exec/totemcrypto.c
++++ b/exec/totemcrypto.c
+@@ -699,6 +699,13 @@ size_t crypto_sec_header_size(
+ 	return hdr_size;
+ }
+ 
++size_t crypto_get_current_sec_header_size(
++	const struct crypto_instance *instance)
++{
++
++	return (instance->crypto_header_size);
++}
++
+ /*
+  * 2.0 packet format:
+  *   crypto_cipher_type | crypto_hash_type | __pad0 | __pad1 | hash | salt | data
+@@ -780,7 +787,6 @@ int crypto_authenticate_and_decrypt (struct crypto_instance *instance,
+ 	/*
+ 	 * decrypt
+ 	 */
+-
+ 	if (decrypt_nss_2_3(instance, buf, buf_len) != 0) {
+ 		return -1;
+ 	}
+diff --git a/exec/totemudp.c b/exec/totemudp.c
+index 31d05704..dbc6ee97 100644
+--- a/exec/totemudp.c
++++ b/exec/totemudp.c
+@@ -270,6 +270,12 @@ static inline void ucast_sendmsg (
+ 	struct iovec iovec;
+ 	int addrlen;
+ 
++	if (msg_len + crypto_get_current_sec_header_size(instance->crypto_inst) > sizeof(buf_out)) {
++		log_printf(LOGSYS_LEVEL_CRIT, "UDP message for ucast is too big. Ignoring message");
++
++		return ;
++	}
++
+ 	/*
+ 	 * Encrypt and digest the message
+ 	 */
+@@ -338,6 +344,12 @@ static inline void mcast_sendmsg (
+ 	struct sockaddr_storage sockaddr;
+ 	int addrlen;
+ 
++	if (msg_len + crypto_get_current_sec_header_size(instance->crypto_inst) > sizeof(buf_out)) {
++		log_printf(LOGSYS_LEVEL_CRIT, "UDP message for mcast is too big. Ignoring message");
++
++		return ;
++	}
++
+ 	/*
+ 	 * Encrypt and digest the message
+ 	 */
+diff --git a/exec/totemudpu.c b/exec/totemudpu.c
+index 037f82b4..78005c48 100644
+--- a/exec/totemudpu.c
++++ b/exec/totemudpu.c
+@@ -271,6 +271,12 @@ static inline void ucast_sendmsg (
+ 	struct iovec iovec;
+ 	int addrlen;
+ 
++	if (msg_len + crypto_get_current_sec_header_size(instance->crypto_inst) > sizeof(buf_out)) {
++		log_printf(LOGSYS_LEVEL_CRIT, "UDPU message for ucast is too big. Ignoring message");
++
++		return ;
++	}
++
+ 	/*
+ 	 * Encrypt and digest the message
+ 	 */
+@@ -341,6 +347,12 @@ static inline void mcast_sendmsg (
+         struct list_head *list;
+ 	struct totemudpu_member *member;
+ 
++	if (msg_len + crypto_get_current_sec_header_size(instance->crypto_inst) > sizeof(buf_out)) {
++		log_printf(LOGSYS_LEVEL_CRIT, "UDPU message for mcast is too big. Ignoring message");
++
++		return ;
++	}
++
+ 	/*
+ 	 * Encrypt and digest the message
+ 	 */
+-- 
+2.14.2
+
diff --git a/patches/0013-qdevice-msgio-Fix-reading-of-msg-longer-than-i32.patch b/patches/0013-qdevice-msgio-Fix-reading-of-msg-longer-than-i32.patch
new file mode 100644
index 0000000..034194a
--- /dev/null
+++ b/patches/0013-qdevice-msgio-Fix-reading-of-msg-longer-than-i32.patch
@@ -0,0 +1,137 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jan Friesse <jfriesse at redhat.com>
+Date: Fri, 23 Mar 2018 17:19:09 +0100
+Subject: [PATCH] qdevice msgio: Fix reading of msg longer than i32
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+And also remove the unused msgio_send_blocking which was used in
+the very early phases of qdevice development and it's not used
+any longer, so it make sense to delete it.
+
+Signed-off-by: Jan Friesse <jfriesse at redhat.com>
+Reviewed-by: Christine Caulfield <ccaulfie at redhat.com>
+(cherry picked from commit b25b029fe186bacf089ab8136da58390945eb35c)
+Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
+---
+ qdevices/msgio.c | 71 +++++++++++---------------------------------------------
+ 1 file changed, 14 insertions(+), 57 deletions(-)
+
+diff --git a/qdevices/msgio.c b/qdevices/msgio.c
+index cedff34a..21158c16 100644
+--- a/qdevices/msgio.c
++++ b/qdevices/msgio.c
+@@ -37,55 +37,6 @@
+ 
+ #define MSGIO_LOCAL_BUF_SIZE			(1 << 10)
+ 
+-ssize_t
+-msgio_send(PRFileDesc *sock, const char *msg, size_t msg_len, size_t *start_pos)
+-{
+-	ssize_t sent_bytes;
+-
+-	if ((sent_bytes = PR_Send(sock, msg + *start_pos,
+-	    msg_len - *start_pos, 0, PR_INTERVAL_NO_TIMEOUT)) != -1) {
+-		*start_pos += sent_bytes;
+-	}
+-
+-	return (sent_bytes);
+-}
+-
+-ssize_t
+-msgio_send_blocking(PRFileDesc *sock, const char *msg, size_t msg_len)
+-{
+-	PRPollDesc pfd;
+-	size_t already_sent_bytes;
+-	PRInt32 res;
+-	ssize_t ret;
+-
+-	already_sent_bytes = 0;
+-	ret = 0;
+-
+-	while (ret != -1 && already_sent_bytes < msg_len) {
+-		pfd.fd = sock;
+-		pfd.in_flags = PR_POLL_WRITE;
+-		pfd.out_flags = 0;
+-
+-		if ((res = PR_Poll(&pfd, 1, PR_INTERVAL_NO_TIMEOUT)) > 0) {
+-			if (pfd.out_flags & PR_POLL_WRITE) {
+-				if ((msgio_send(sock, msg, msg_len, &already_sent_bytes) == -1) &&
+-				    PR_GetError() != PR_WOULD_BLOCK_ERROR) {
+-					ret = -1;
+-				} else {
+-					ret = already_sent_bytes;
+-				}
+-			} else if (pfd.out_flags & (PR_POLL_ERR | PR_POLL_NVAL | PR_POLL_HUP)) {
+-				PR_SetError(PR_IO_ERROR, 0);
+-				ret = -1;
+-			}
+-		} else {
+-			ret = -1;
+-		}
+-	}
+-
+-	return (ret);
+-}
+-
+ /*
+  * -1 = send returned 0,
+  * -2 = unhandled error.
+@@ -96,18 +47,21 @@ int
+ msgio_write(PRFileDesc *sock, const struct dynar *msg, size_t *already_sent_bytes)
+ {
+ 	PRInt32 sent;
+-	PRInt32 to_send;
++	PRInt32 to_send_i32;
++	size_t to_send;
+ 
+ 	to_send = dynar_size(msg) - *already_sent_bytes;
+ 	if (to_send > MSGIO_LOCAL_BUF_SIZE) {
+-		to_send = MSGIO_LOCAL_BUF_SIZE;
++		to_send_i32 = MSGIO_LOCAL_BUF_SIZE;
++	} else {
++		to_send_i32 = (PRInt32)to_send;
+ 	}
+ 
+-	sent = PR_Send(sock, dynar_data(msg) + *already_sent_bytes, to_send, 0,
++	sent = PR_Send(sock, dynar_data(msg) + *already_sent_bytes, to_send_i32, 0,
+ 	    PR_INTERVAL_NO_TIMEOUT);
+ 
+ 	if (sent > 0) {
+-		*already_sent_bytes += sent;
++		*already_sent_bytes += (size_t)sent;
+ 
+ 		if (*already_sent_bytes == dynar_size(msg)) {
+ 			/*
+@@ -143,7 +97,8 @@ msgio_read(PRFileDesc *sock, struct dynar *msg, size_t *already_received_bytes,
+ {
+ 	char local_read_buffer[MSGIO_LOCAL_BUF_SIZE];
+ 	PRInt32 readed;
+-	PRInt32 to_read;
++	size_t to_read;
++	PRInt32 to_read_i32;
+ 	int ret;
+ 
+ 	ret = 0;
+@@ -161,12 +116,14 @@ msgio_read(PRFileDesc *sock, struct dynar *msg, size_t *already_received_bytes,
+ 	}
+ 
+ 	if (to_read > MSGIO_LOCAL_BUF_SIZE) {
+-		to_read = MSGIO_LOCAL_BUF_SIZE;
++		to_read_i32 = MSGIO_LOCAL_BUF_SIZE;
++	} else {
++		to_read_i32 = (PRInt32)to_read;
+ 	}
+ 
+-	readed = PR_Recv(sock, local_read_buffer, to_read, 0, PR_INTERVAL_NO_TIMEOUT);
++	readed = PR_Recv(sock, local_read_buffer, to_read_i32, 0, PR_INTERVAL_NO_TIMEOUT);
+ 	if (readed > 0) {
+-		*already_received_bytes += readed;
++		*already_received_bytes += (size_t)readed;
+ 
+ 		if (!*skipping_msg) {
+ 			if (dynar_cat(msg, local_read_buffer, readed) == -1) {
+-- 
+2.14.2
+
diff --git a/patches/series b/patches/series
index b6afd80..5fbab33 100644
--- a/patches/series
+++ b/patches/series
@@ -5,4 +5,6 @@
 0005-add-corosync-pve-postinst-for-restart-on-dist-upgrad.patch
 0006-add-libcorosync4-pve-transitional-package.patch
 0007-only-start-corosync.service-if-conf-exists.patch
-0008-remove-unecessary-and-problematic-corosync-qdevice.i.patch
+0008-totemcrypto-Check-length-of-the-packet.patch
+0012-totemudp-Check-lenght-of-message-to-sent.patch
+0013-qdevice-msgio-Fix-reading-of-msg-longer-than-i32.patch
-- 
2.14.2




More information about the pve-devel mailing list