GUACAMOLE-482: Merge allow encoding to proceed despite invalid instructions.
diff --git a/Dockerfile b/Dockerfile
index 21b98d8..4d463a3 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -22,9 +22,9 @@
#
-# Use CentOS as base for the build
-ARG CENTOS_VERSION=centos7
-FROM centos:${CENTOS_VERSION} AS builder
+# Use Debian as base for the build
+ARG DEBIAN_VERSION=stable
+FROM debian:${DEBIAN_VERSION} AS builder
# Base directory for installed build artifacts.
# Due to limitations of the Docker image build process, this value is
@@ -37,41 +37,44 @@
ARG BUILD_DEPENDENCIES=" \
autoconf \
automake \
- cairo-devel \
- freerdp-devel \
gcc \
- libjpeg-turbo-devel \
- libssh2-devel \
+ libcairo2-dev \
+ libfreerdp-dev \
+ libjpeg62-turbo-dev \
+ libossp-uuid-dev \
+ libpango1.0-dev \
+ libpulse-dev \
+ libssh2-1-dev \
+ libssl-dev \
+ libtelnet-dev \
libtool \
- libtelnet-devel \
- libvorbis-devel \
- libvncserver-devel \
- libwebp-devel \
- make \
- pango-devel \
- pulseaudio-libs-devel \
- uuid-devel"
-
-# Build time environment
-ENV LC_ALL=en_US.UTF-8
+ libvncserver-dev \
+ libwebp-dev \
+ make"
# Bring build environment up to date and install build dependencies
-RUN yum -y update && \
- yum -y install epel-release && \
- yum -y install $BUILD_DEPENDENCIES && \
- yum clean all
+RUN apt-get update && \
+ apt-get install -y $BUILD_DEPENDENCIES && \
+ rm -rf /var/lib/apt/lists/*
# Add configuration scripts
-COPY src/guacd-docker/bin /opt/guacd/bin/
+COPY src/guacd-docker/bin "${PREFIX_DIR}/bin/"
# Copy source to container for sake of build
COPY . "$BUILD_DIR"
# Build guacamole-server from local source
-RUN /opt/guacd/bin/build-guacd.sh "$BUILD_DIR" "$PREFIX_DIR"
+RUN ${PREFIX_DIR}/bin/build-guacd.sh "$BUILD_DIR" "$PREFIX_DIR"
-# Use same CentOS as the base for the runtime image
-FROM centos:${CENTOS_VERSION}
+# Record the packages of all runtime library dependencies
+RUN ${PREFIX_DIR}/bin/list-dependencies.sh \
+ ${PREFIX_DIR}/sbin/guacd \
+ ${PREFIX_DIR}/lib/libguac-client-*.so \
+ ${PREFIX_DIR}/lib/freerdp/guac*.so \
+ > ${PREFIX_DIR}/DEPENDENCIES
+
+# Use same Debian as the base for the runtime image
+FROM debian:${DEBIAN_VERSION}
# Base directory for installed build artifacts.
# Due to limitations of the Docker image build process, this value is
@@ -81,43 +84,29 @@
ARG PREFIX_DIR=/usr/local/guacamole
# Runtime environment
-ENV LC_ALL=en_US.UTF-8
+ENV LC_ALL=C.UTF-8
+ENV LD_LIBRARY_PATH=${PREFIX_DIR}/lib
ENV GUACD_LOG_LEVEL=info
ARG RUNTIME_DEPENDENCIES=" \
- cairo \
- dejavu-sans-mono-fonts \
- freerdp \
- freerdp-plugins \
ghostscript \
- libjpeg-turbo \
- libssh2 \
- liberation-mono-fonts \
- libtelnet \
- libvorbis \
- libvncserver \
- libwebp \
- pango \
- pulseaudio-libs \
- terminus-fonts \
- uuid"
-
-# Bring runtime environment up to date and install runtime dependencies
-RUN yum -y update && \
- yum -y install epel-release && \
- yum -y install $RUNTIME_DEPENDENCIES && \
- yum clean all && \
- rm -rf /var/cache/yum
+ libfreerdp-plugins-standard \
+ fonts-liberation \
+ fonts-dejavu \
+ xfonts-terminus"
# Copy build artifacts into this stage
COPY --from=builder ${PREFIX_DIR} ${PREFIX_DIR}
+# Bring runtime environment up to date and install runtime dependencies
+RUN apt-get update && \
+ apt-get install -y $RUNTIME_DEPENDENCIES && \
+ apt-get install -y $(cat "${PREFIX_DIR}"/DEPENDENCIES) && \
+ rm -rf /var/lib/apt/lists/*
+
# Link FreeRDP plugins into proper path
-RUN FREERDP_DIR=$(dirname \
- $(rpm -ql freerdp-libs | grep 'libfreerdp.*\.so' | head -n1)) && \
- FREERDP_PLUGIN_DIR="${FREERDP_DIR}/freerdp" && \
- mkdir -p "$FREERDP_PLUGIN_DIR" && \
- ln -s "$PREFIX_DIR"/lib/freerdp/*.so "$FREERDP_PLUGIN_DIR"
+RUN ${PREFIX_DIR}/bin/link-freerdp-plugins.sh \
+ ${PREFIX_DIR}/lib/freerdp/guac*.so
# Expose the default listener port
EXPOSE 4822
diff --git a/src/guacd-docker/bin/build-guacd.sh b/src/guacd-docker/bin/build-guacd.sh
index 9e8c3a3..4714743 100755
--- a/src/guacd-docker/bin/build-guacd.sh
+++ b/src/guacd-docker/bin/build-guacd.sh
@@ -43,7 +43,7 @@
cd "$BUILD_DIR"
autoreconf -fi
-./configure --prefix="$PREFIX_DIR"
+./configure --prefix="$PREFIX_DIR" --disable-guaclog
make
make install
ldconfig
diff --git a/src/guacd-docker/bin/link-freerdp-plugins.sh b/src/guacd-docker/bin/link-freerdp-plugins.sh
new file mode 100755
index 0000000..332d4c0
--- /dev/null
+++ b/src/guacd-docker/bin/link-freerdp-plugins.sh
@@ -0,0 +1,86 @@
+#!/bin/sh -e
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+##
+## @fn link-freerdp-plugins.sh
+##
+## Automatically creates any required symbolic links for the proper loading of
+## the given FreeRDP plugins. If a given plugin is already in the correct
+## directory, no link is created for that plugin.
+##
+## @param ...
+## The FreeRDP plugins to add links for.
+##
+
+##
+## Given the full path to a FreeRDP plugin, locates the base directory of the
+## associated FreeRDP installation (where the FreeRDP library .so files are
+## located), printing the result to STDOUT. If the directory cannot be
+## determined, an error is printed.
+##
+## @param PLUGIN_FILE
+## The full path to the FreeRDP plugin to check.
+##
+where_is_freerdp() {
+
+ PLUGIN_FILE="$1"
+
+ # Determine the location of all libfreerdp* libraries explicitly linked
+ # to given file
+ PATHS="$(ldd "$PLUGIN_FILE" \
+ | awk '/=>/{print $(NF-1)}' \
+ | grep 'libfreerdp' \
+ | xargs -r dirname \
+ | xargs -r realpath \
+ | sort -u)"
+
+ # Verify that exactly one location was found
+ if [ "$(echo "$PATHS" | wc -l)" != 1 ]; then
+ echo "$1: Unable to locate FreeRDP install location." >&2
+ return 1
+ fi
+
+ echo "$PATHS"
+
+}
+
+#
+# Create symbolic links as necessary to include all given plugins within the
+# search path of FreeRDP
+#
+
+while [ -n "$1" ]; do
+
+ # Determine correct install location for FreeRDP plugins
+ FREERDP_DIR="$(where_is_freerdp "$1")"
+ FREERDP_PLUGIN_DIR="${FREERDP_DIR}/freerdp"
+
+ # Add symbolic link if necessary
+ if [ ! -e "$FREERDP_PLUGIN_DIR/$(basename "$1")" ]; then
+ mkdir -p "$FREERDP_PLUGIN_DIR"
+ ln -s "$1" "$FREERDP_PLUGIN_DIR"
+ else
+ echo "$1: Already in correct directory." >&2
+ fi
+
+ shift
+
+done
+
diff --git a/src/guacd-docker/bin/list-dependencies.sh b/src/guacd-docker/bin/list-dependencies.sh
new file mode 100755
index 0000000..090185e
--- /dev/null
+++ b/src/guacd-docker/bin/list-dependencies.sh
@@ -0,0 +1,48 @@
+#!/bin/sh -e
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+##
+## @fn list-dependencies.sh
+##
+## Lists the Debian/Ubuntu package names for all library dependencies of the
+## given binaries. Each package is only listed once, even if multiple binaries
+## provided by the same package are given.
+##
+## @param ...
+## The full paths to all binaries being checked.
+##
+
+while [ -n "$1" ]; do
+
+ # For all non-Guacamole library dependencies
+ ldd "$1" | grep -v 'libguac' | awk '/=>/{print $(NF-1)}' \
+ | while read LIBRARY; do
+
+ # Determine the Debian package which is associated with that
+ # library, if any
+ dpkg-query -S "$LIBRARY" 2> /dev/null || true
+
+ done
+
+ # Next binary
+ shift
+
+done | cut -f1 -d: | sort -u
+
diff --git a/src/guacd/proc.c b/src/guacd/proc.c
index 0a35910..27ad69c 100644
--- a/src/guacd/proc.c
+++ b/src/guacd/proc.c
@@ -33,11 +33,15 @@
#include <guacamole/user.h>
#include <errno.h>
+#include <pthread.h>
+#include <signal.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <sys/time.h>
#include <sys/types.h>
#include <sys/socket.h>
+#include <sys/wait.h>
/**
* Parameters for the user thread.
@@ -139,6 +143,151 @@
}
/**
+ * Forcibly kills all processes within the current process group, including the
+ * current process and all child processes. This function is only safe to call
+ * if the process group ID has been correctly set. Calling this function within
+ * a process which does not have a PGID separate from the main guacd process
+ * can result in guacd itself being terminated.
+ */
+static void guacd_kill_current_proc_group() {
+
+ /* Forcibly kill all children within process group */
+ if (kill(0, SIGKILL))
+ guacd_log(GUAC_LOG_WARNING, "Unable to forcibly terminate "
+ "client process: %s ", strerror(errno));
+
+}
+
+/**
+ * The current status of a background attempt to free a guac_client instance.
+ */
+typedef struct guacd_client_free {
+
+ /**
+ * The guac_client instance being freed.
+ */
+ guac_client* client;
+
+ /**
+ * The condition which is signalled whenever changes are made to the
+ * completed flag. The completed flag only changes from zero (not yet
+ * freed) to non-zero (successfully freed).
+ */
+ pthread_cond_t completed_cond;
+
+ /**
+ * Mutex which must be acquired before any changes are made to the
+ * completed flag.
+ */
+ pthread_mutex_t completed_mutex;
+
+ /**
+ * Whether the guac_client has been successfully freed. Initially, this
+ * will be zero, indicating that the free operation has not yet been
+ * attempted. If the client is eventually successfully freed, this will be
+ * set to a non-zero value. Changes to this flag are signalled through
+ * the completed_cond condition.
+ */
+ int completed;
+
+} guacd_client_free;
+
+/**
+ * Thread which frees a given guac_client instance in the background. If the
+ * free operation succeeds, a flag is set on the provided structure, and the
+ * change in that flag is signalled with a pthread condition.
+ *
+ * At the time this function is provided to a pthread_create() call, the
+ * completed flag of the associated guacd_client_free structure MUST be
+ * initialized to zero, the pthread mutex and condition MUST both be
+ * initialized, and the client pointer must point to the guac_client being
+ * freed.
+ *
+ * @param data
+ * A pointer to a guacd_client_free structure describing the free
+ * operation.
+ *
+ * @return
+ * Always NULL.
+ */
+static void* guacd_client_free_thread(void* data) {
+
+ guacd_client_free* free_operation = (guacd_client_free*) data;
+
+ /* Attempt to free client (this may never return if the client is
+ * malfunctioning) */
+ guac_client_free(free_operation->client);
+
+ /* Signal that the client was successfully freed */
+ pthread_mutex_lock(&free_operation->completed_mutex);
+ free_operation->completed = 1;
+ pthread_cond_broadcast(&free_operation->completed_cond);
+ pthread_mutex_unlock(&free_operation->completed_mutex);
+
+ return NULL;
+
+}
+
+/**
+ * Attempts to free the given guac_client, restricting the time taken by the
+ * free handler of the guac_client to a finite number of seconds. If the free
+ * handler does not complete within the time alotted, this function returns
+ * and the intended free operation is left in an undefined state.
+ *
+ * @param client
+ * The guac_client instance to free.
+ *
+ * @param timeout
+ * The maximum amount of time to wait for the guac_client to be freed,
+ * in seconds.
+ *
+ * @return
+ * Zero if the guac_client was successfully freed within the time alotted,
+ * non-zero otherwise.
+ */
+static int guacd_timed_client_free(guac_client* client, int timeout) {
+
+ pthread_t client_free_thread;
+
+ guacd_client_free free_operation = {
+ .client = client,
+ .completed_cond = PTHREAD_COND_INITIALIZER,
+ .completed_mutex = PTHREAD_MUTEX_INITIALIZER,
+ .completed = 0
+ };
+
+ /* Get current time */
+ struct timeval current_time;
+ if (gettimeofday(¤t_time, NULL))
+ return 1;
+
+ /* Calculate exact time that the free operation MUST complete by */
+ struct timespec deadline = {
+ .tv_sec = current_time.tv_sec + timeout,
+ .tv_nsec = current_time.tv_usec * 1000
+ };
+
+ /* Free the client in a separate thread, so we can time the free operation */
+ if (pthread_create(&client_free_thread, NULL,
+ guacd_client_free_thread, &free_operation))
+ return 1;
+
+ /* The mutex associated with the pthread conditional and flag MUST be
+ * acquired before attempting to wait for the condition */
+ if (pthread_mutex_lock(&free_operation.completed_mutex))
+ return 1;
+
+ /* Wait a finite amount of time for the free operation to finish */
+ if (pthread_cond_timedwait(&free_operation.completed_cond,
+ &free_operation.completed_mutex, &deadline))
+ return 1;
+
+ /* Return status of free operation */
+ return !free_operation.completed;
+
+}
+
+/**
* Starts protocol-specific handling on the given process by loading the client
* plugin for that protocol. This function does NOT return. It initializes the
* process with protocol-specific handlers and then runs until the guacd_proc's
@@ -154,8 +303,18 @@
*/
static void guacd_exec_proc(guacd_proc* proc, const char* protocol) {
+ int result = 1;
+
+ /* Set process group ID to match PID */
+ if (setpgid(0, 0)) {
+ guacd_log(GUAC_LOG_ERROR, "Cannot set PGID for connection process: %s",
+ strerror(errno));
+ goto cleanup_process;
+ }
+
/* Init client for selected protocol */
- if (guac_client_load_plugin(proc->client, protocol)) {
+ guac_client* client = proc->client;
+ if (guac_client_load_plugin(client, protocol)) {
/* Log error */
if (guac_error == GUAC_STATUS_NOT_FOUND)
@@ -165,10 +324,7 @@
guacd_log_guac_error(GUAC_LOG_ERROR,
"Unable to load client plugin");
- guac_client_free(proc->client);
- close(proc->fd_socket);
- free(proc);
- exit(1);
+ goto cleanup_client;
}
/* The first file descriptor is the owner */
@@ -185,14 +341,47 @@
}
- /* Stop and free client */
- guac_client_stop(proc->client);
- guac_client_free(proc->client);
+cleanup_client:
- /* Child is finished */
+ /* Request client to stop/disconnect */
+ guac_client_stop(client);
+
+ /* Attempt to free client cleanly */
+ guacd_log(GUAC_LOG_DEBUG, "Requesting termination of client...");
+ result = guacd_timed_client_free(client, GUACD_CLIENT_FREE_TIMEOUT);
+
+ /* If client was unable to be freed, warn and forcibly kill */
+ if (result) {
+ guacd_log(GUAC_LOG_WARNING, "Client did not terminate in a timely "
+ "manner. Forcibly terminating client and any child "
+ "processes.");
+ guacd_kill_current_proc_group();
+ }
+ else
+ guacd_log(GUAC_LOG_DEBUG, "Client terminated successfully.");
+
+ /* Verify whether children were all properly reaped */
+ pid_t child_pid;
+ while ((child_pid = waitpid(0, NULL, WNOHANG)) > 0) {
+ guacd_log(GUAC_LOG_DEBUG, "Automatically reaped unreaped "
+ "(zombie) child process with PID %i.", child_pid);
+ }
+
+ /* If running children remain, warn and forcibly kill */
+ if (child_pid == 0) {
+ guacd_log(GUAC_LOG_WARNING, "Client reported successful termination, "
+ "but child processes remain. Forcibly terminating client and "
+ "child processes.");
+ guacd_kill_current_proc_group();
+ }
+
+cleanup_process:
+
+ /* Free up all internal resources outside the client */
close(proc->fd_socket);
free(proc);
- exit(0);
+
+ exit(result);
}
diff --git a/src/guacd/proc.h b/src/guacd/proc.h
index 13bd9f8..d13ae10 100644
--- a/src/guacd/proc.h
+++ b/src/guacd/proc.h
@@ -41,6 +41,14 @@
#define GUACD_USEC_TIMEOUT (GUACD_TIMEOUT*1000)
/**
+ * The number of seconds to wait for any particular guac_client instance
+ * to be freed following disconnect. If the free operation does not complete
+ * within this period of time, the associated process will be forcibly
+ * terminated.
+ */
+#define GUACD_CLIENT_FREE_TIMEOUT 5
+
+/**
* Process information of the internal remote desktop client.
*/
typedef struct guacd_proc {
