GUACAMOLE-1140: Address RDP pipe svc race condition.

.github/workflows/pr-build.yml

@@ -0,0 +1,29 @@
+name: Pull request CI build
+
+# Run build for all pull requests
+on:
+  pull_request:
+
+# Limit to only one build for a given PR source branch at a time,
+# cancelling any in-progress builds
+concurrency:
+  group: guacamole-server-pr-${{ github.head_ref }}
+  cancel-in-progress: true
+
+jobs:
+
+  docker_build:
+    name: Run docker build
+    runs-on: ubuntu-latest
+    steps:
+
+      - name: Check out code
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+
+      - name: Build Docker container
+        shell: sh
+        run: |
+          docker build --pull --no-cache --force-rm .

Dockerfile

@@ -32,6 +32,7 @@ RUN apk add --no-cache                \
         build-base                    \
         cairo-dev                     \
         cmake                         \
+        cunit-dev                     \
         git                           \
         grep                          \
         libjpeg-turbo-dev             \

configure.ac

@@ -1243,7 +1243,7 @@ $PACKAGE_NAME version $PACKAGE_VERSION
      freerdp2 ............ ${have_freerdp2}
      pango ............... ${have_pango}
      libavcodec .......... ${have_libavcodec}
-     libavformat.......... ${have_libavformat}
+     libavformat ......... ${have_libavformat}
      libavutil ........... ${have_libavutil}
      libssh2 ............. ${have_libssh2}
      libssl .............. ${have_ssl}

src/common-ssh/common-ssh/user.h

@@ -44,6 +44,12 @@ typedef struct guac_common_ssh_user {
      */
     guac_common_ssh_key* private_key;
 
+    /**
+     * The public key which should be used to authenticate this user, if any,
+     * or NULL if a password or just a private key will be used instead.
+     */
+    char* public_key;
+
 } guac_common_ssh_user;
 
 /**
@@ -104,5 +110,23 @@ void guac_common_ssh_user_set_password(guac_common_ssh_user* user,
 int guac_common_ssh_user_import_key(guac_common_ssh_user* user,
         char* private_key, char* passphrase);
 
+/**
+ * Imports the given public key, associating that key with the given user.
+ * If the public key is imported successfully, it will be used for
+ * future authentication attempts.
+ *
+ * @param user
+ *     The user to associate with the given private key.
+ *
+ * @param public_key
+ *     The base64-encoded public key to import.
+ *
+ * @return
+ *     Zero if public key is successfully imported, or non-zero if the
+ *     public key could not be imported due to an error.
+ */
+int guac_common_ssh_user_import_public_key(guac_common_ssh_user* user,
+        char* public_key);
+
 #endif
 

src/common-ssh/ssh.c

@@ -182,9 +182,11 @@ int guac_common_ssh_init(guac_client* client) {
     CRYPTO_set_locking_callback(guac_common_ssh_openssl_locking_callback);
 #endif
 
-    /* Init OpenSSL */
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+    /* Init OpenSSL - only required for OpenSSL Versions < 1.1.0 */
     SSL_library_init();
     ERR_load_crypto_strings();
+#endif
 
     /* Init libssh2 */
     libssh2_init(0);
@@ -284,6 +286,8 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session)
     /* Get user credentials */
     guac_common_ssh_key* key = user->private_key;
 
+    char* public_key = user->public_key;
+
     /* Validate username provided */
     if (user->username == NULL) {
         guac_client_abort(client, GUAC_PROTOCOL_STATUS_CLIENT_UNAUTHORIZED,
@@ -317,9 +321,11 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session)
             return 1;
         }
 
+        int public_key_length = public_key == NULL ? 0 : strlen(public_key);
+
         /* Attempt public key auth */
         if (libssh2_userauth_publickey_frommemory(session, user->username,
-                    username_len, NULL, 0, key->private_key,
+                    username_len, public_key, public_key_length, key->private_key,
                     key->private_key_length, key->passphrase)) {
 
             /* Abort on failure */

src/common-ssh/user.c

@@ -34,6 +34,7 @@ guac_common_ssh_user* guac_common_ssh_create_user(const char* username) {
     user->username = guac_strdup(username);
     user->password = NULL;
     user->private_key = NULL;
+    user->public_key = NULL;
 
     return user;
 
@@ -48,6 +49,7 @@ void guac_common_ssh_destroy_user(guac_common_ssh_user* user) {
     /* Free all other data */
     guac_mem_free(user->password);
     guac_mem_free(user->username);
+    guac_mem_free(user->public_key);
     guac_mem_free(user);
 
 }
@@ -83,3 +85,15 @@ int guac_common_ssh_user_import_key(guac_common_ssh_user* user,
 
 }
 
+int guac_common_ssh_user_import_public_key(guac_common_ssh_user* user,
+        char* public_key) {
+
+    /* Free existing public key, if present */
+    guac_mem_free(user->public_key);
+    user->public_key = guac_strdup(public_key);
+
+    /* Fail if key could not be read */
+    return user->public_key == NULL;
+
+}
+

src/common/display.c

@@ -180,6 +180,9 @@ void guac_common_display_dup(
     guac_common_display_dup_layers(display->layers, client, socket);
     guac_common_display_dup_layers(display->buffers, client, socket);
 
+    /* Sends a sync instruction to mark the boundary of the first frame */
+    guac_protocol_send_sync(socket, client->last_sent_timestamp, 1);
+
     pthread_mutex_unlock(&display->_lock);
 
 }
@@ -386,4 +389,3 @@ void guac_common_display_free_buffer(guac_common_display* display,
     pthread_mutex_unlock(&display->_lock);
 
 }
-

src/guacd-docker/bin/build-all.sh

@@ -111,5 +111,5 @@ install_from_git "https://github.com/warmcat/libwebsockets" "$WITH_LIBWEBSOCKETS
 
 cd "$BUILD_DIR"
 autoreconf -fi && ./configure --prefix="$PREFIX_DIR" $GUACAMOLE_SERVER_OPTS
-make && make install
+make && make check && make install
 

src/guacd/daemon.c

@@ -426,10 +426,15 @@ int main(int argc, char* argv[]) {
         CRYPTO_set_locking_callback(guacd_openssl_locking_callback);
 #endif
 
-        /* Init SSL */
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+        /* Init OpenSSL for OpenSSL Versions < 1.1.0 */
         SSL_library_init();
         SSL_load_error_strings();
         ssl_context = SSL_CTX_new(SSLv23_server_method());
+#else
+        /* Set up OpenSSL for OpenSSL Versions >= 1.1.0 */
+        ssl_context = SSL_CTX_new(TLS_server_method());
+#endif
 
         /* Load key */
         if (config->key_file != NULL) {

src/guacenc/ffmpeg-compat.c

@@ -215,7 +215,7 @@ int guacenc_avcodec_encode_video(guacenc_video* video, AVFrame* frame) {
 #endif
 }
 
-AVCodecContext* guacenc_build_avcodeccontext(AVStream* stream, AVCodec* codec, 
+AVCodecContext* guacenc_build_avcodeccontext(AVStream* stream, const AVCodec* codec,
         int bitrate, int width, int height, int gop_size, int qmax, int qmin,
         int pix_fmt, AVRational time_base) {
 
@@ -251,7 +251,7 @@ AVCodecContext* guacenc_build_avcodeccontext(AVStream* stream, AVCodec* codec,
 }
 
 int guacenc_open_avcodec(AVCodecContext *avcodec_context,
-        AVCodec *codec, AVDictionary **options,
+        const AVCodec *codec, AVDictionary **options,
         AVStream* stream) {
 
     int ret = avcodec_open2(avcodec_context, codec, options);

src/guacenc/ffmpeg-compat.h

@@ -128,7 +128,7 @@ int guacenc_avcodec_encode_video(guacenc_video* video, AVFrame* frame);
  *     The pointer to the configured AVCodecContext.
  *
  */
-AVCodecContext* guacenc_build_avcodeccontext(AVStream* stream, AVCodec* codec,
+AVCodecContext* guacenc_build_avcodeccontext(AVStream* stream, const AVCodec* codec,
         int bitrate, int width, int height, int gop_size, int qmax, int qmin,
         int pix_fmt, AVRational time_base);
 
@@ -158,7 +158,7 @@ AVCodecContext* guacenc_build_avcodeccontext(AVStream* stream, AVCodec* codec,
  *     Zero on success, a negative value on error.
  */
 int guacenc_open_avcodec(AVCodecContext *avcodec_context,
-        AVCodec *codec, AVDictionary **options,
+        const AVCodec *codec, AVDictionary **options,
         AVStream* stream);
 
 #endif

src/guacenc/video.c

@@ -48,7 +48,7 @@
 guacenc_video* guacenc_video_alloc(const char* path, const char* codec_name,
         int width, int height, int bitrate) {
 
-    AVOutputFormat *container_format;
+    const AVOutputFormat *container_format;
     AVFormatContext *container_format_context;
     AVStream *video_stream;
     int ret;
@@ -64,7 +64,7 @@ guacenc_video* guacenc_video_alloc(const char* path, const char* codec_name,
     container_format = container_format_context->oformat;
 
     /* Pull codec based on name */
-    AVCodec* codec = avcodec_find_encoder_by_name(codec_name);
+    const AVCodec* codec = avcodec_find_encoder_by_name(codec_name);
     if (codec == NULL) {
         guacenc_log(GUAC_LOG_ERROR, "Failed to locate codec \"%s\".",
                 codec_name);

src/libguac/client.c

@@ -702,15 +702,19 @@ void* guac_client_for_user(guac_client* client, guac_user* user,
 }
 
 int guac_client_end_frame(guac_client* client) {
+    return guac_client_end_multiple_frames(client, 0);
+}
+
+int guac_client_end_multiple_frames(guac_client* client, int frames) {
 
     /* Update and send timestamp */
     client->last_sent_timestamp = guac_timestamp_current();
 
     /* Log received timestamp and calculated lag (at TRACE level only) */
     guac_client_log(client, GUAC_LOG_TRACE, "Server completed "
-            "frame %" PRIu64 "ms.", client->last_sent_timestamp);
+            "frame %" PRIu64 "ms (%i logical frames)", client->last_sent_timestamp, frames);
 
-    return guac_protocol_send_sync(client->socket, client->last_sent_timestamp);
+    return guac_protocol_send_sync(client->socket, client->last_sent_timestamp, frames);
 
 }
 

src/libguac/encode-webp.c

@@ -198,16 +198,22 @@ int guac_webp_write(guac_socket* socket, guac_stream* stream,
     config.method = 2; /* Compression method (0=fast/larger, 6=slow/smaller) */
 
     /* Validate configuration */
-    WebPValidateConfig(&config);
+    if (!WebPValidateConfig(&config)) {
+        return -1;
+    }
 
     /* Set up WebP picture */
-    WebPPictureInit(&picture);
+    if (!WebPPictureInit(&picture)) {
+        return -1;
+    }
     picture.use_argb = 1;
     picture.width = width;
     picture.height = height;
 
     /* Allocate and init writer */
-    WebPPictureAlloc(&picture);
+    if (!WebPPictureAlloc(&picture)) {
+        return -1;
+    }
     picture.writer = guac_webp_stream_write;
     picture.custom_ptr = &writer;
     guac_webp_stream_writer_init(&writer, socket, stream);
@@ -244,15 +250,15 @@ int guac_webp_write(guac_socket* socket, guac_stream* stream,
     }
 
     /* Encode image */
-    WebPEncode(&config, &picture);
+    const int result = WebPEncode(&config, &picture) ? 0 : -1;
 
     /* Free picture */
     WebPPictureFree(&picture);
 
     /* Ensure all data is written */
     guac_webp_flush_data(&writer);
 
-    return 0;
+    return result;
 
 }
 

src/libguac/guacamole/client.h

@@ -586,18 +586,47 @@ void* guac_client_for_user(guac_client* client, guac_user* user,
         guac_user_callback* callback, void* data);
 
 /**
- * Marks the end of the current frame by sending a "sync" instruction to
- * all connected users. This instruction will contain the current timestamp.
- * The last_sent_timestamp member of guac_client will be updated accordingly.
+ * Marks the end of the current frame by sending a "sync" instruction to all
+ * connected users, where the number of input frames that were considered in
+ * creating this frame is either unknown or inapplicable. This instruction will
+ * contain the current timestamp. The last_sent_timestamp member of guac_client
+ * will be updated accordingly.
  *
  * If an error occurs sending the instruction, a non-zero value is
  * returned, and guac_error is set appropriately.
  *
- * @param client The guac_client which has finished a frame.
- * @return Zero on success, non-zero on error.
+ * @param client
+ *     The guac_client which has finished a frame.
+ *
+ * @return
+ *     Zero on success, non-zero on error.
  */
 int guac_client_end_frame(guac_client* client);
 
+/**
+ * Marks the end of the current frame by sending a "sync" instruction to all
+ * connected users, where that frame may combine or otherwise represent the
+ * changes of an arbitrary number of input frames. This instruction will
+ * contain the current timestamp, as well as the number of frames that were
+ * considered in creating that frame.  The last_sent_timestamp member of
+ * guac_client will be updated accordingly.
+ *
+ * If an error occurs sending the instruction, a non-zero value is
+ * returned, and guac_error is set appropriately.
+ *
+ * @param client
+ *     The guac_client which has finished a frame.
+ *
+ * @param frames
+ *     The number of distinct frames that were considered or combined when
+ *     generating the current frame, or zero if the boundaries of relevant
+ *     frames are unknown.
+ *
+ * @return
+ *     Zero on success, non-zero on error.
+ */
+int guac_client_end_multiple_frames(guac_client* client, int frames);
+
 /**
  * Initializes the given guac_client using the initialization routine provided
  * by the plugin corresponding to the named protocol. This will automatically

src/libguac/guacamole/protocol.h

@@ -384,11 +384,22 @@ int guac_protocol_send_select(guac_socket* socket, const char* protocol);
  * If an error occurs sending the instruction, a non-zero value is
  * returned, and guac_error is set appropriately.
  *
- * @param socket The guac_socket connection to use.
- * @param timestamp The current timestamp (in milliseconds).
- * @return Zero on success, non-zero on error.
+ * @param socket
+ *     The guac_socket connection to use.
+ *
+ * @param timestamp
+ *     The current timestamp (in milliseconds).
+ *
+ * @param frames
+ *     The number of distinct frames that were considered or combined when
+ *     generating the frame terminated by this instruction, or zero if the
+ *     boundaries of relevant frames are unknown.
+ *
+ * @return
+ *     Zero on success, non-zero on error.
  */
-int guac_protocol_send_sync(guac_socket* socket, guac_timestamp timestamp);
+int guac_protocol_send_sync(guac_socket* socket, guac_timestamp timestamp,
+        int frames);
 
 /* OBJECT INSTRUCTIONS */