9feed69 moved the async reader code to RedsStream so that it can be used
for the SASL authentication code. In particular, it introduced a
RedsStream::async_read member which is used by the SASL authentication code
for its async operations.
However, what was not done is to remove the now redundant
RedLinkInfo::async_read field. This causes failures when using SASL
authentication as the async read error callback is getting set
on the RedLinkInfo::async_read structure, but then the SASL code is trying
to use the RedeStream::async_read structure for its async IOs, which do not
have the needed error callback set.
This commit makes use of the newly introduced reds_stream_async_read()
helper in order to make use of RedsStream::async_read.
This replaces async_read_set_error_handler() which was unused. This sets a
callback to be called when an async operation fails.
We could pass the error_handler to each reds_stream_async_read() call, but as
we will be using the same one for all async calls, it's more convenient to set it
once and for all.
AsyncRead is going to be private to reds_stream.c in one of the next
commits, and the error handler will need to be set from reds.c,
hence the move to a public RedsStream method.
There are 2 SASL-related function prototypes which are unused in the
--without-sasl case. They cause a warning, and a build failure
when using -Werror. Wrapping them in #if HAVE_SASL avoids this issue.
This can fail in fips mode for example. If we ignore the failure, we'll get
a crash:
#0 0x00007f38d63728a0 in BN_num_bits () from /lib64/libcrypto.so.10
#1 0x00007f38d639661d in RSA_size () from /lib64/libcrypto.so.10
#2 0x00007f38d7991762 in reds_handle_read_link_done () from /lib64/libspice-server.so.1
#3 0x00007f38d7990c06 in spice_server_add_client () from /lib64/libspice-server.so.1
#4 0x00007f38d7990c6a in reds_accept () from /lib64/libspice-server.so.1
#5 0x00007f38dc0d2946 in qemu_iohandler_poll (pollfds=0x7f38dedce200, ret=755449965, ret@entry=1) at iohandler.c:143
#6 0x00007f38dc0d6ea8 in main_loop_wait (nonblocking=<optimized out>) at main-loop.c:465
#7 0x00007f38dbffd7c0 in main_loop () at vl.c:1988
#8 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4357
This commit will cause the client connection to fail but qemu won't
segfault.
static void _spice_timer_set(SpiceTimer *timer, uint32_t ms, uint32_t now)
The _spice_timer_set() function takes a 32-bit integer for the "now" value.
The now value passed in however, can exceed 2^32 (it's in ms and derived
from CLOCK_MONOTONIC, which will wrap around a 32-bit integer in around 46
days).
If the now value passed in exceeds 2^32, this will mean timers are inserted
into the active list with expiry values before the current time, they will
immediately trigger, and (if they don't make themselves inactive) be
reinserted still before the current time.
This leads to an infinite loop in spice_timer_queue_cb().
https://bugzilla.redhat.com/show_bug.cgi?id=1072700
For example, with qemu, a webdav channel can be created this way:
-chardev spiceport,name=org.spice-space.webdav.0,...
And redirected to a virtio port:
-device virtserialport,...,name=org.spice-space.webdav.0
When trying to start mjpeg compression mode, mjpeg_encoder_start_frame()
tests the image format as its only able to compress 24/32bpp images. On
images with lower bit depths, we return MJPEG_ENCODER_FRAME_UNSUPPORTED to
indicate this is not a format we can compress. However, this return goes
with a spice_warning("unsupported format"). As the rest of the code can
cope with this unsupported format by not doing mjpeg compression, it's
nicer to downgrade this spice_warning() to spice_debug().
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1070028
The private data is allocated at the same time as RedsStream and
goes immediatly after the main RedsStream data.
This private member will allow to hide internal RedsStream
implementation details from the rest of spice-server.
SASL authentication mostly use members from RedsStream to do its work, so
it makes sense to have its code in reds_stream.c. This should allow to make
RedsStream::sasl private in the future.
The AsyncRead structure in reds.h wraps an async read + callback to
be done on a stream. Moving it to reds_stream.h is needed in order
to move SASL authentication there.
Now that stream creation and SSL enabling are done by helpers
in reds_stream.c, we can move the initialization of the vfunc
read/write pointers there too.
Initializing a new stream means initializing quite a few fields.
This commit factors this initialization in a dedicated reds_stream_new
helper. This also helps moving more code from reds.c to reds_stream.c
test-display-streaming is calling malloc() without checking its return
value. Coverity warns about this. This commit switches to g_malloc() to
sidestep this warning (g_malloc() never returns NULL but aborts instead).
When creating a TLS socket, both spice-server and spice-gtk currently
call SSL_CTX_new(TLSv1_method()). The TLSv1_method() function set the
protocol version to TLS 1.0 exclusively. The correct way to support
multiple protocol versions is to call SSLv23_method() in spite of its
scary name. This method will enable all SSL/TLS protocol versions. The
protocol suite may be further narrowed down by setting respective
SSL_OP_NO_<version_code> options of SSL context. This possibility is
used in this patch in order to block use of SSLv3 that is enabled by
default in openssl for client sockets as of now but spice has never used
it.
This file was added in bc50ff076 a few months ago, but is not listed
in Makefile.am, and thus not part of tarballs. However, it's being included
from other C files, so not having it causes compilation breakage.
reds_handle_ticket uses a fixed size 'password' buffer for the decrypted
password whose size is SPICE_MAX_PASSWORD_LENGTH. However,
RSA_private_decrypt which we call for the decryption expects the
destination buffer to be at least RSA_size(link->tiTicketing.rsa)
bytes long. On my spice-server build, SPICE_MAX_PASSWORD_LENGTH
is 60 while RSA_size() is 128, so we end up overflowing 'password'
when using long passwords (this was reproduced using the string:
'fullscreen=1proxy=#enter proxy here; e.g spice_proxy = http://[proxy]:[port]'
as a password).
When the overflow occurs, QEMU dies with:
*** stack smashing detected ***: qemu-system-x86_64 terminated
This commit ensures we use a corectly sized 'password' buffer,
and that it's correctly nul-terminated so that we can use strcmp
instead of strncmp. To keep using strncmp, we'd need to figure out
which one of 'password' and 'taTicket.password' is the smaller buffer,
and use that size.
This fixes rhbz#999839
It's depending on an unmaintained package (slirp), and I don't
think anyone uses that code. It's not tested upstream nor in fedora,
so let's remove it.
Some versions of gcc warn about:
red_channel.c: In function 'red_channel_client_wait_outgoing_item':
red_channel.c:2331: error: 'end_time' may be used uninitialized in this function [-Wuninitialized]
red_channel.c: In function 'red_channel_client_wait_pipe_item_sent':
red_channel.c:2363: error: 'end_time' may be used uninitialized in this function [-Wuninitialized]
red_channel.c: In function 'red_channel_wait_all_sent':
red_channel.c:2401: error: 'end_time' may be used uninitialized in this function [-Wuninitialized]
This is a false positive as end_time is unitialized when timeout is -1, and
we will only try to use end_time if timeout is not -1.
This commit initializes end_time to UINT64_MAX to avoid that warning. As
the test involving end_time will never be reached, we ensure it's always
TRUE so that it would be a noop even if it was reached.
This commit reuse several macros from libvirt to test for
support for "-Wl,-z -Wl,relro", "-Wl,-z -Wl,now" and
"-Wl,--no-copy-dt-needed-entries", and use them if available.
Releasing modifiers keys unconditionally on disconnect leads to
unexpected guest wakeups. To improve the situation, the server can
release only the pressed keys, which will prevent the wakeup in most
cases.
Furthermore, it's not sufficient to release only the modifiers keys.
Any key should be released on client disconnect to avoid sticky key
press across connections.
https://bugzilla.redhat.com/show_bug.cgi?id=871240
