|
 |
5d360b |
From 52fe55e2bf9df408ebe127a670ee698642d3fcb4 Mon Sep 17 00:00:00 2001
|
|
|
5d360b |
From: "Daniel P. Berrange" <berrange@redhat.com>
|
|
|
5d360b |
Date: Thu, 8 Feb 2018 17:50:38 +0100
|
|
|
5d360b |
Subject: [PATCH 24/27] ui: place a hard cap on VNC server output buffer size
|
|
|
5d360b |
MIME-Version: 1.0
|
|
|
5d360b |
Content-Type: text/plain; charset=UTF-8
|
|
|
5d360b |
Content-Transfer-Encoding: 8bit
|
|
|
5d360b |
|
|
|
5d360b |
RH-Author: Daniel P. Berrange <berrange@redhat.com>
|
|
|
5d360b |
Message-id: <20180208175041.5634-25-berrange@redhat.com>
|
|
|
5d360b |
Patchwork-id: 78957
|
|
|
5d360b |
O-Subject: [RHEL-7.5 qemu-kvm PATCH v1 24/27] ui: place a hard cap on VNC server output buffer size
|
|
|
5d360b |
Bugzilla: 1527405
|
|
|
5d360b |
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
|
|
|
5d360b |
RH-Acked-by: Gerd Hoffmann <kraxel@redhat.com>
|
|
|
5d360b |
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
5d360b |
|
|
|
5d360b |
From: "Daniel P. Berrange" <berrange@redhat.com>
|
|
|
5d360b |
|
|
|
5d360b |
The previous patches fix problems with throttling of forced framebuffer updates
|
|
|
5d360b |
and audio data capture that would cause the QEMU output buffer size to grow
|
|
|
5d360b |
without bound. Those fixes are graceful in that once the client catches up with
|
|
|
5d360b |
reading data from the server, everything continues operating normally.
|
|
|
5d360b |
|
|
|
5d360b |
There is some data which the server sends to the client that is impractical to
|
|
|
5d360b |
throttle. Specifically there are various pseudo framebuffer update encodings to
|
|
|
5d360b |
inform the client of things like desktop resizes, pointer changes, audio
|
|
|
5d360b |
playback start/stop, LED state and so on. These generally only involve sending
|
|
|
5d360b |
a very small amount of data to the client, but a malicious guest might be able
|
|
|
5d360b |
to do things that trigger these changes at a very high rate. Throttling them is
|
|
|
5d360b |
not practical as missed or delayed events would cause broken behaviour for the
|
|
|
5d360b |
client.
|
|
|
5d360b |
|
|
|
5d360b |
This patch thus takes a more forceful approach of setting an absolute upper
|
|
|
5d360b |
bound on the amount of data we permit to be present in the output buffer at
|
|
|
5d360b |
any time. The previous patch set a threshold for throttling the output buffer
|
|
|
5d360b |
by allowing an amount of data equivalent to one complete framebuffer update and
|
|
|
5d360b |
one seconds worth of audio data. On top of this it allowed for one further
|
|
|
5d360b |
forced framebuffer update to be queued.
|
|
|
5d360b |
|
|
|
5d360b |
To be conservative, we thus take that throttling threshold and multiply it by
|
|
|
5d360b |
5 to form an absolute upper bound. If this bound is hit during vnc_write() we
|
|
|
5d360b |
forceably disconnect the client, refusing to queue further data. This limit is
|
|
|
5d360b |
high enough that it should never be hit unless a malicious client is trying to
|
|
|
5d360b |
exploit the sever, or the network is completely saturated preventing any sending
|
|
|
5d360b |
of data on the socket.
|
|
|
5d360b |
|
|
|
5d360b |
This completes the fix for CVE-2017-15124 started in the previous patches.
|
|
|
5d360b |
|
|
|
5d360b |
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
|
|
|
5d360b |
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
|
|
|
5d360b |
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
|
|
|
5d360b |
Message-id: 20171218191228.31018-12-berrange@redhat.com
|
|
|
5d360b |
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
|
|
|
5d360b |
(cherry picked from commit f887cf165db20f405cb8805c716bd363aaadf815)
|
|
|
5d360b |
|
|
|
5d360b |
Conflicts:
|
|
|
5d360b |
ui/vnc.c - context differences and no 'vs->disconnecting' flag.
|
|
|
5d360b |
Using share_mode as a better check for the disconnecting state
|
|
|
5d360b |
than csock == -1, because the worker thread calls vnc_write()
|
|
|
5d360b |
with a fake VncState that has csock == -1.
|
|
|
5d360b |
|
|
|
5d360b |
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
5d360b |
---
|
|
|
5d360b |
ui/vnc.c | 29 +++++++++++++++++++++++++++++
|
|
|
5d360b |
1 file changed, 29 insertions(+)
|
|
|
5d360b |
|
|
|
5d360b |
diff --git a/ui/vnc.c b/ui/vnc.c
|
|
|
5d360b |
index 96b6caf..61fbec2 100644
|
|
|
5d360b |
--- a/ui/vnc.c
|
|
|
5d360b |
+++ b/ui/vnc.c
|
|
|
5d360b |
@@ -1460,8 +1460,37 @@ void vnc_client_read(void *opaque)
|
|
|
5d360b |
}
|
|
|
5d360b |
}
|
|
|
5d360b |
|
|
|
5d360b |
+/*
|
|
|
5d360b |
+ * Scale factor to apply to vs->throttle_output_offset when checking for
|
|
|
5d360b |
+ * hard limit. Worst case normal usage could be x2, if we have a complete
|
|
|
5d360b |
+ * incremental update and complete forced update in the output buffer.
|
|
|
5d360b |
+ * So x3 should be good enough, but we pick x5 to be conservative and thus
|
|
|
5d360b |
+ * (hopefully) never trigger incorrectly.
|
|
|
5d360b |
+ */
|
|
|
5d360b |
+#define VNC_THROTTLE_OUTPUT_LIMIT_SCALE 5
|
|
|
5d360b |
+
|
|
|
5d360b |
void vnc_write(VncState *vs, const void *data, size_t len)
|
|
|
5d360b |
{
|
|
|
5d360b |
+ if (vs->share_mode == VNC_SHARE_MODE_DISCONNECTED) {
|
|
|
5d360b |
+ return;
|
|
|
5d360b |
+ }
|
|
|
5d360b |
+ /* Protection against malicious client/guest to prevent our output
|
|
|
5d360b |
+ * buffer growing without bound if client stops reading data. This
|
|
|
5d360b |
+ * should rarely trigger, because we have earlier throttling code
|
|
|
5d360b |
+ * which stops issuing framebuffer updates and drops audio data
|
|
|
5d360b |
+ * if the throttle_output_offset value is exceeded. So we only reach
|
|
|
5d360b |
+ * this higher level if a huge number of pseudo-encodings get
|
|
|
5d360b |
+ * triggered while data can't be sent on the socket.
|
|
|
5d360b |
+ *
|
|
|
5d360b |
+ * NB throttle_output_offset can be zero during early protocol
|
|
|
5d360b |
+ * handshake, or from the job thread's VncState clone
|
|
|
5d360b |
+ */
|
|
|
5d360b |
+ if (vs->throttle_output_offset != 0 &&
|
|
|
5d360b |
+ vs->output.offset > (vs->throttle_output_offset *
|
|
|
5d360b |
+ VNC_THROTTLE_OUTPUT_LIMIT_SCALE)) {
|
|
|
5d360b |
+ vnc_disconnect_start(vs);
|
|
|
5d360b |
+ return;
|
|
|
5d360b |
+ }
|
|
|
5d360b |
buffer_reserve(&vs->output, len);
|
|
|
5d360b |
|
|
|
5d360b |
if (vs->csock != -1 && buffer_empty(&vs->output)) {
|
|
|
5d360b |
--
|
|
|
5d360b |
1.8.3.1
|
|
|
5d360b |
|