Date: Fri, 18 Dec 2020 00:38:48 +0000 (UTC) From: Peter Grehan <grehan@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r368747 - head/usr.sbin/bhyve Message-ID: <202012180038.0BI0cmhN056571@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: grehan Date: Fri Dec 18 00:38:48 2020 New Revision: 368747 URL: https://svnweb.freebsd.org/changeset/base/368747 Log: Fix issues with various VNC clients. - support VNC version 3.3 (macos "Screen Sharing" builtin client) - wait until client has requested an update prior to sending framebuffer data - don't send an update if no framebuffer updates detected - increase framebuffer poll frequency to 30Hz, and double that when kbd/mouse input detected - zero uninitialized array elements in rfb_send_server_init_msg() - fix overly large allocation in rfb_init() - use atomics for flags shared between input and output threads - use #defines for constants This work was contributed by Marko Kiiskila, with reuse of some earlier work by Henrik Gulbrandsen. Clients tested : FreeBSD-current - tightvnc - tigervnc - krdc - vinagre Linux (Ubuntu) - krdc - vinagre - tigervnc - xtightvncviewer - remmina MacOS - VNC Viewer - TigerVNC - Screen Sharing (builtin client) Windows 10 - Tiger VNC - VNC Viewer (cursor lag) - UltraVNC (cursor lag) o/s independent - noVNC (browser) using websockify relay PR: 250795 Submitted by: Marko Kiiskila <marko@apache.org> Reviewed by: jhb (bhyve) MFC after: 3 weeks Relnotes: yes Differential Revision: https://reviews.freebsd.org/D27605 Modified: head/usr.sbin/bhyve/rfb.c Modified: head/usr.sbin/bhyve/rfb.c ============================================================================== --- head/usr.sbin/bhyve/rfb.c Thu Dec 17 23:35:18 2020 (r368746) +++ head/usr.sbin/bhyve/rfb.c Fri Dec 18 00:38:48 2020 (r368747) @@ -39,6 +39,7 @@ __FBSDID("$FreeBSD$"); #include <sys/select.h> #include <sys/time.h> #include <arpa/inet.h> +#include <stdatomic.h> #include <machine/cpufunc.h> #include <machine/specialreg.h> #include <netinet/in.h> @@ -72,6 +73,11 @@ __FBSDID("$FreeBSD$"); #include <openssl/des.h> #endif +/* Delays in microseconds */ +#define CFD_SEL_DELAY 10000 +#define SCREEN_REFRESH_DELAY 33300 /* 30Hz */ +#define SCREEN_POLL_DELAY (SCREEN_REFRESH_DELAY / 2) + static int rfb_debug = 0; #define DPRINTF(params) if (rfb_debug) PRINTLN params #define WPRINTF(params) PRINTLN params @@ -80,6 +86,19 @@ static int rfb_debug = 0; #define AUTH_LENGTH 16 #define PASSWD_LENGTH 8 +/* Protocol versions */ +#define CVERS_3_3 '3' +#define CVERS_3_7 '7' +#define CVERS_3_8 '8' + +/* Client-to-server msg types */ +#define CS_SET_PIXEL_FORMAT 0 +#define CS_SET_ENCODINGS 2 +#define CS_UPDATE_MSG 3 +#define CS_KEY_EVENT 4 +#define CS_POINTER_EVENT 5 +#define CS_CUT_TEXT 6 + #define SECURITY_TYPE_NONE 1 #define SECURITY_TYPE_VNC_AUTH 2 @@ -96,16 +115,22 @@ struct rfb_softc { char *password; - bool enc_raw_ok; - bool enc_zlib_ok; - bool enc_resize_ok; + bool enc_raw_ok; + bool enc_zlib_ok; + bool enc_resize_ok; z_stream zstream; uint8_t *zbuf; int zbuflen; int conn_wait; - int sending; + int wrcount; + + atomic_bool sending; + atomic_bool pending; + atomic_bool update_all; + atomic_bool input_detected; + pthread_mutex_t mtx; pthread_cond_t cond; @@ -202,7 +227,6 @@ struct rfb_cuttext_msg { uint32_t length; }; - static void rfb_send_server_init_msg(int cfd) { @@ -223,6 +247,9 @@ rfb_send_server_init_msg(int cfd) sinfo.pixfmt.red_shift = 16; sinfo.pixfmt.green_shift = 8; sinfo.pixfmt.blue_shift = 0; + sinfo.pixfmt.pad[0] = 0; + sinfo.pixfmt.pad[1] = 0; + sinfo.pixfmt.pad[2] = 0; sinfo.namelen = htonl(strlen("bhyve")); (void)stream_write(cfd, &sinfo, sizeof(sinfo)); (void)stream_write(cfd, "bhyve", strlen("bhyve")); @@ -265,7 +292,6 @@ rfb_recv_set_encodings_msg(struct rfb_softc *rc, int c int i; uint32_t encoding; - assert((sizeof(enc_msg) - 1) == 3); (void)stream_read(cfd, ((void *)&enc_msg)+1, sizeof(enc_msg)-1); for (i = 0; i < htons(enc_msg.numencs); i++) { @@ -308,12 +334,23 @@ fast_crc32(void *buf, int len, uint32_t crcval) return (crcval); } +static int +rfb_send_update_header(struct rfb_softc *rc, int cfd, int numrects) +{ + struct rfb_srvr_updt_msg supdt_msg; + supdt_msg.type = 0; + supdt_msg.pad = 0; + supdt_msg.numrects = htons(numrects); + + return stream_write(cfd, &supdt_msg, + sizeof(struct rfb_srvr_updt_msg)); +} + static int rfb_send_rect(struct rfb_softc *rc, int cfd, struct bhyvegc_image *gc, int x, int y, int w, int h) { - struct rfb_srvr_updt_msg supdt_msg; struct rfb_srvr_rect_hdr srect_hdr; unsigned long zlen; ssize_t nwrite, total; @@ -325,16 +362,6 @@ rfb_send_rect(struct rfb_softc *rc, int cfd, struct bh * Send a single rectangle of the given x, y, w h dimensions. */ - /* Number of rectangles: 1 */ - supdt_msg.type = 0; - supdt_msg.pad = 0; - supdt_msg.numrects = htons(1); - nwrite = stream_write(cfd, &supdt_msg, - sizeof(struct rfb_srvr_updt_msg)); - if (nwrite <= 0) - return (nwrite); - - /* Rectangle header */ srect_hdr.x = htons(x); srect_hdr.y = htons(y); @@ -479,7 +506,7 @@ doraw: #define PIXCELL_MASK 0x1F static int -rfb_send_screen(struct rfb_softc *rc, int cfd, int all) +rfb_send_screen(struct rfb_softc *rc, int cfd) { struct bhyvegc_image *gc_image; ssize_t nwrite; @@ -492,35 +519,54 @@ rfb_send_screen(struct rfb_softc *rc, int cfd, int all int retval; uint32_t *crc_p, *orig_crc; int changes; + bool expected; + /* Return if another thread sending */ + expected = false; + if (atomic_compare_exchange_strong(&rc->sending, &expected, true) == false) + return (1); + + retval = 1; + + /* Updates require a preceding update request */ + if (atomic_exchange(&rc->pending, false) == false) + goto done; + console_refresh(); gc_image = console_get_image(); - pthread_mutex_lock(&rc->mtx); - if (rc->sending) { - pthread_mutex_unlock(&rc->mtx); - return (1); + /* Clear old CRC values when the size changes */ + if (rc->crc_width != gc_image->width || + rc->crc_height != gc_image->height) { + memset(rc->crc, 0, sizeof(uint32_t) * + howmany(RFB_MAX_WIDTH, PIX_PER_CELL) * + howmany(RFB_MAX_HEIGHT, PIX_PER_CELL)); + rc->crc_width = gc_image->width; + rc->crc_height = gc_image->height; } - rc->sending = 1; - pthread_mutex_unlock(&rc->mtx); - retval = 0; + /* A size update counts as an update in itself */ + if (rc->width != gc_image->width || + rc->height != gc_image->height) { + rc->width = gc_image->width; + rc->height = gc_image->height; + if (rc->enc_resize_ok) { + rfb_send_resize_update_msg(rc, cfd); + rc->update_all = true; + goto done; + } + } - if (all) { - retval = rfb_send_all(rc, cfd, gc_image); - goto done; - } + if (atomic_exchange(&rc->update_all, false) == true) { + retval = rfb_send_all(rc, cfd, gc_image); + goto done; + } /* * Calculate the checksum for each 32x32 cell. Send each that * has changed since the last scan. */ - /* Resolution changed */ - - rc->crc_width = gc_image->width; - rc->crc_height = gc_image->height; - w = rc->crc_width; h = rc->crc_height; xcells = howmany(rc->crc_width, PIX_PER_CELL); @@ -580,12 +626,24 @@ rfb_send_screen(struct rfb_softc *rc, int cfd, int all } } + /* + * We only send the update if there are changes. + * Restore the pending flag since it was unconditionally cleared + * above. + */ + if (!changes) { + rc->pending = true; + goto done; + } + /* If number of changes is > THRESH percent, send the whole screen */ if (((changes * 100) / (xcells * ycells)) >= RFB_SEND_ALL_THRESH) { retval = rfb_send_all(rc, cfd, gc_image); goto done; } - + + rfb_send_update_header(rc, cfd, changes); + /* Go through all cells, and send only changed ones */ crc_p = rc->crc_tmp; for (y = 0; y < h; y += PIX_PER_CELL) { @@ -613,45 +671,24 @@ rfb_send_screen(struct rfb_softc *rc, int cfd, int all } } } - retval = 1; done: - pthread_mutex_lock(&rc->mtx); - rc->sending = 0; - pthread_mutex_unlock(&rc->mtx); - + rc->sending = false; + return (retval); } static void -rfb_recv_update_msg(struct rfb_softc *rc, int cfd, int discardonly) +rfb_recv_update_msg(struct rfb_softc *rc, int cfd) { struct rfb_updt_msg updt_msg; - struct bhyvegc_image *gc_image; (void)stream_read(cfd, ((void *)&updt_msg) + 1 , sizeof(updt_msg) - 1); - console_refresh(); - gc_image = console_get_image(); - - updt_msg.x = htons(updt_msg.x); - updt_msg.y = htons(updt_msg.y); - updt_msg.width = htons(updt_msg.width); - updt_msg.height = htons(updt_msg.height); - - if (updt_msg.width != gc_image->width || - updt_msg.height != gc_image->height) { - rc->width = gc_image->width; - rc->height = gc_image->height; - if (rc->enc_resize_ok) - rfb_send_resize_update_msg(rc, cfd); - } - - if (discardonly) - return; - - rfb_send_screen(rc, cfd, 1); + rc->pending = true; + if (!updt_msg.incremental) + rc->update_all = true; } static void @@ -662,6 +699,7 @@ rfb_recv_key_msg(struct rfb_softc *rc, int cfd) (void)stream_read(cfd, ((void *)&key_msg) + 1, sizeof(key_msg) - 1); console_key_event(key_msg.down, htonl(key_msg.code)); + rc->input_detected = true; } static void @@ -672,6 +710,7 @@ rfb_recv_ptr_msg(struct rfb_softc *rc, int cfd) (void)stream_read(cfd, ((void *)&ptr_msg) + 1, sizeof(ptr_msg) - 1); console_ptr_event(ptr_msg.button, htons(ptr_msg.x), htons(ptr_msg.y)); + rc->input_detected = true; } static void @@ -719,7 +758,7 @@ rfb_wr_thr(void *arg) FD_ZERO(&rfds); FD_SET(cfd, &rfds); tv.tv_sec = 0; - tv.tv_usec = 10000; + tv.tv_usec = CFD_SEL_DELAY; err = select(cfd+1, &rfds, NULL, NULL, &tv); if (err < 0) @@ -728,15 +767,23 @@ rfb_wr_thr(void *arg) /* Determine if its time to push screen; ~24hz */ gettimeofday(&tv, NULL); tdiff = timeval_delta(&prev_tv, &tv); - if (tdiff > 40000) { + if (tdiff >= SCREEN_POLL_DELAY) { + bool input; prev_tv.tv_sec = tv.tv_sec; prev_tv.tv_usec = tv.tv_usec; - if (rfb_send_screen(rc, cfd, 0) <= 0) { - return (NULL); + input = atomic_exchange(&rc->input_detected, false); + /* + * Refresh the screen on every second trip through the loop, + * or if keyboard/mouse input has been detected. + */ + if ((++rc->wrcount & 1) || input) { + if (rfb_send_screen(rc, cfd) <= 0) { + return (NULL); + } } } else { /* sleep */ - usleep(40000 - tdiff); + usleep(SCREEN_POLL_DELAY - tdiff); } } @@ -758,7 +805,8 @@ rfb_handle(struct rfb_softc *rc, int cfd) DES_key_schedule ks; int i; #endif - + uint8_t client_ver; + uint8_t auth_type; pthread_t tid; uint32_t sres = 0; int len; @@ -771,27 +819,55 @@ rfb_handle(struct rfb_softc *rc, int cfd) /* 1b. Read client version */ len = stream_read(cfd, buf, VERSION_LENGTH); + if (len == VERSION_LENGTH && !strncmp(vbuf, buf, VERSION_LENGTH - 2)) { + client_ver = buf[VERSION_LENGTH - 2]; + } + if (client_ver != CVERS_3_8 && client_ver != CVERS_3_7) { + /* only recognize 3.3, 3.7 & 3.8. Others dflt to 3.3 */ + client_ver = CVERS_3_3; + } /* 2a. Send security type */ buf[0] = 1; + + /* In versions 3.7 & 3.8, it's 2-way handshake */ + /* For version 3.3, server says what the authentication type must be */ #ifndef NO_OPENSSL - if (rc->password) - buf[1] = SECURITY_TYPE_VNC_AUTH; - else - buf[1] = SECURITY_TYPE_NONE; + if (rc->password) { + auth_type = SECURITY_TYPE_VNC_AUTH; + } else { + auth_type = SECURITY_TYPE_NONE; + } #else - buf[1] = SECURITY_TYPE_NONE; + auth_type = SECURITY_TYPE_NONE; #endif - stream_write(cfd, buf, 2); + switch (client_ver) { + case CVERS_3_7: + case CVERS_3_8: + buf[0] = 1; + buf[1] = auth_type; + stream_write(cfd, buf, 2); - /* 2b. Read agreed security type */ - len = stream_read(cfd, buf, 1); + /* 2b. Read agreed security type */ + len = stream_read(cfd, buf, 1); + if (buf[0] != auth_type) { + /* deny */ + sres = htonl(1); + message = "Auth failed: authentication type mismatch"; + goto report_and_done; + } + break; + case CVERS_3_3: + default: + be32enc(buf, auth_type); + stream_write(cfd, buf, 4); + break; + } /* 2c. Do VNC authentication */ - switch (buf[0]) { + switch (auth_type) { case SECURITY_TYPE_NONE: - sres = 0; break; case SECURITY_TYPE_VNC_AUTH: /* @@ -839,26 +915,46 @@ rfb_handle(struct rfb_softc *rc, int cfd) if (memcmp(crypt_expected, buf, AUTH_LENGTH) != 0) { message = "Auth Failed: Invalid Password."; sres = htonl(1); - } else + } else { sres = 0; + } #else - sres = 0; + sres = htonl(1); WPRINTF(("Auth not supported, no OpenSSL in your system")); #endif break; } - /* 2d. Write back a status */ - stream_write(cfd, &sres, 4); + switch (client_ver) { + case CVERS_3_7: + case CVERS_3_8: +report_and_done: + /* 2d. Write back a status */ + stream_write(cfd, &sres, 4); - if (sres) { - be32enc(buf, strlen(message)); - stream_write(cfd, buf, 4); - stream_write(cfd, message, strlen(message)); - goto done; + if (sres) { + /* 3.7 does not want string explaining cause */ + if (client_ver == CVERS_3_8) { + be32enc(buf, strlen(message)); + stream_write(cfd, buf, 4); + stream_write(cfd, message, strlen(message)); + } + goto done; + } + break; + case CVERS_3_3: + default: + /* for VNC auth case send status */ + if (auth_type == SECURITY_TYPE_VNC_AUTH) { + /* 2d. Write back a status */ + stream_write(cfd, &sres, 4); + } + if (sres) { + goto done; + } + break; } - /* 3a. Read client shared-flag byte */ len = stream_read(cfd, buf, 1); @@ -870,8 +966,6 @@ rfb_handle(struct rfb_softc *rc, int cfd) assert(rc->zbuf != NULL); } - rfb_send_screen(rc, cfd, 1); - perror = pthread_create(&tid, NULL, rfb_wr_thr, rc); if (perror == 0) pthread_set_name_np(tid, "rfbout"); @@ -885,22 +979,22 @@ rfb_handle(struct rfb_softc *rc, int cfd) } switch (buf[0]) { - case 0: + case CS_SET_PIXEL_FORMAT: rfb_recv_set_pixfmt_msg(rc, cfd); break; - case 2: + case CS_SET_ENCODINGS: rfb_recv_set_encodings_msg(rc, cfd); break; - case 3: - rfb_recv_update_msg(rc, cfd, 1); + case CS_UPDATE_MSG: + rfb_recv_update_msg(rc, cfd); break; - case 4: + case CS_KEY_EVENT: rfb_recv_key_msg(rc, cfd); break; - case 5: + case CS_POINTER_EVENT: rfb_recv_ptr_msg(rc, cfd); break; - case 6: + case CS_CUT_TEXT: rfb_recv_cuttext_msg(rc, cfd); break; default: @@ -974,16 +1068,17 @@ rfb_init(char *hostname, int port, int wait, char *pas struct addrinfo *ai = NULL; struct addrinfo hints; int on = 1; + int cnt; #ifndef WITHOUT_CAPSICUM cap_rights_t rights; #endif rc = calloc(1, sizeof(struct rfb_softc)); - rc->crc = calloc(howmany(RFB_MAX_WIDTH * RFB_MAX_HEIGHT, 32), - sizeof(uint32_t)); - rc->crc_tmp = calloc(howmany(RFB_MAX_WIDTH * RFB_MAX_HEIGHT, 32), - sizeof(uint32_t)); + cnt = howmany(RFB_MAX_WIDTH, PIX_PER_CELL) * + howmany(RFB_MAX_HEIGHT, PIX_PER_CELL); + rc->crc = calloc(cnt, sizeof(uint32_t)); + rc->crc_tmp = calloc(cnt, sizeof(uint32_t)); rc->crc_width = RFB_MAX_WIDTH; rc->crc_height = RFB_MAX_HEIGHT; rc->sfd = -1;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202012180038.0BI0cmhN056571>