GUACAMOLE-2233: Make guacd client timeout configurable

README

@@ -166,6 +166,12 @@ guacd currently takes several command-line options:
         if running in the foreground, the console.  Legal values are debug,
         info, warning, and error.  The default value is info.
 
+    -T MILLISECONDS
+
+        Sets the maximum number of milliseconds that guacd will wait for the
+        next message from a connected client before closing the connection.
+        The value must be a positive integer and defaults to 15000.
+
     -f
         Causes guacd to run in the foreground, rather than automatically
         forking into the background. 
@@ -182,4 +188,3 @@ Please report any bugs encountered by opening a new issue in the JIRA system
 hosted at:
 
     https://issues.apache.org/jira/browse/GUACAMOLE
-

configure.ac

@@ -1492,6 +1492,7 @@ AC_CONFIG_FILES([Makefile
                  src/libguac/Makefile
                  src/libguac/tests/Makefile
                  src/guacd/Makefile
+                 src/guacd/tests/Makefile
                  src/guacd/man/guacd.8
                  src/guacd/man/guacd.conf.5
                  src/guacenc/Makefile

src/guacd-docker/README.md

@@ -38,9 +38,19 @@ Connecting to guacd from an application
 
     docker run --name some-app --link some-guacd:guacd -d application-that-uses-guacd
 
+Configuring the client timeout
+------------------------------
+
+By default, guacd waits up to 15000 milliseconds for the next message from a
+connected client before closing the connection. Set `CLIENT_TIMEOUT` to a
+positive number of milliseconds to override this value:
+
+    docker run --name some-guacd -d -e CLIENT_TIMEOUT=45000 guacamole/guacd
+
+An explicit `-T` command-line option takes precedence over `CLIENT_TIMEOUT`.
+
 Reporting issues
 ================
 
 Please report any bugs encountered by opening a new issue in
 [our JIRA](https://issues.apache.org/jira/browse/GUACAMOLE/).
-

src/guacd-docker/bin/entrypoint.sh

@@ -5,6 +5,12 @@ if [ -n "$GUACD_LOG_LEVEL" ]; then
     echo "WARNING: The GUACD_LOG_LEVEL environment variable has been deprecated in favor of the LOG_LEVEL environment variable. Please migrate your configuration when possible." >&2
 fi
 
+# Allow the client timeout to be overridden with CLIENT_TIMEOUT. Explicit
+# command-line options take precedence over this environment variable.
+if [ -n "$CLIENT_TIMEOUT" ]; then
+    set -- -T "$CLIENT_TIMEOUT" "$@"
+fi
+
 # Listen on 0.0.0.0:4822, logging messages at the info level. Allow log level
 # to be overridden with LOG_LEVEL, and other behavior to be overridden with
 # additional command-line options passed to Docker.

src/guacd/Makefile.am

@@ -28,6 +28,7 @@ AUTOMAKE_OPTIONS = foreign
 AM_CPPFLAGS = -include config.h
 
 sbin_PROGRAMS = guacd
+SUBDIRS = . tests
 
 man_MANS =           \
     man/guacd.8      \
@@ -97,4 +98,3 @@ systemd/guacd.service: systemd/guacd.service.in
 	 -e 's,[@]systemduser[@],$(systemduser),g' \
 	 < systemd/guacd.service.in > systemd/guacd.service
 endif
-

src/guacd/conf-args.c

@@ -33,7 +33,7 @@ int guacd_conf_parse_args(guacd_config* config, int argc, char** argv) {
 
     /* Parse arguments */
     int opt;
-    while ((opt = getopt(argc, argv, "l:b:p:L:C:K:fv")) != -1) {
+    while ((opt = getopt(argc, argv, "l:b:p:L:T:C:K:fv")) != -1) {
 
         /* -l: Bind port */
         if (opt == 'l') {
@@ -77,6 +77,19 @@ int guacd_conf_parse_args(guacd_config* config, int argc, char** argv) {
 
         }
 
+        /* -T: Client timeout */
+        else if (opt == 'T') {
+
+            int timeout = guacd_parse_client_timeout(optarg);
+            if (timeout < 0) {
+                fprintf(stderr, "Client timeout must be a positive integer number of milliseconds no greater than %i.\n", GUACD_MAX_CLIENT_TIMEOUT);
+                return 1;
+            }
+
+            config->client_timeout = timeout;
+
+        }
+
 #ifdef ENABLE_SSL
         /* -C SSL certificate */
         else if (opt == 'C') {
@@ -107,6 +120,7 @@ int guacd_conf_parse_args(guacd_config* config, int argc, char** argv) {
                     " [-b LISTENADDRESS]"
                     " [-p PIDFILE]"
                     " [-L LEVEL]"
+                    " [-T MILLISECONDS]"
 #ifdef ENABLE_SSL
                     " [-C CERTIFICATE_FILE]"
                     " [-K PEM_FILE]"
@@ -122,4 +136,3 @@ int guacd_conf_parse_args(guacd_config* config, int argc, char** argv) {
     return 0;
 
 }
-

src/guacd/conf-file.c

@@ -61,6 +61,22 @@ static int guacd_conf_callback(const char* section, const char* param, const cha
             return 0;
         }
 
+        /* Client timeout */
+        else if (strcmp(param, "client_timeout") == 0) {
+
+            int timeout = guacd_parse_client_timeout(value);
+
+            /* Invalid client timeout */
+            if (timeout < 0) {
+                guacd_conf_parse_error = "Client timeout must be a positive integer number of milliseconds within the supported range";
+                return 1;
+            }
+
+            config->client_timeout = timeout;
+            return 0;
+
+        }
+
     }
 
     /* Options related to daemon startup */
@@ -183,6 +199,7 @@ guacd_config* guacd_conf_load(void) {
     /* Load defaults */
     conf->bind_host = guac_strdup(GUACD_DEFAULT_BIND_HOST);
     conf->bind_port = guac_strdup(GUACD_DEFAULT_BIND_PORT);
+    conf->client_timeout = GUACD_DEFAULT_CLIENT_TIMEOUT;
     conf->pidfile = NULL;
     conf->foreground = 0;
     conf->print_version = 0;
@@ -223,4 +240,3 @@ guacd_config* guacd_conf_load(void) {
     return conf;
 
 }
-

src/guacd/conf-parse.c

@@ -23,8 +23,36 @@
 #include <guacamole/client.h>
 
 #include <ctype.h>
+#include <errno.h>
+#include <stdlib.h>
 #include <string.h>
 
+int guacd_parse_client_timeout(const char* value) {
+
+    char* end;
+
+    /* Reject empty values, signs, whitespace, and all other non-digits */
+    if (*value == '\0')
+        return -1;
+
+    for (const char* current = value; *current != '\0'; current++) {
+        if (!isdigit((unsigned char) *current))
+            return -1;
+    }
+
+    errno = 0;
+    long timeout = strtol(value, &end, 10);
+
+    /* Value must be a positive integer that can be safely converted to the
+     * microsecond timeout expected by libguac */
+    if (errno == ERANGE || end == value || *end != '\0'
+            || timeout <= 0 || timeout > GUACD_MAX_CLIENT_TIMEOUT)
+        return -1;
+
+    return (int) timeout;
+
+}
+
 /*
  * Simple recursive descent parser for an INI-like conf file grammar.  The
  * grammar is, roughly:
@@ -532,4 +560,3 @@ int guacd_parse_log_level(const char* name) {
     return -1;
 
 }
-

src/guacd/conf-parse.h

@@ -53,6 +53,19 @@ int guacd_parse_conf(guacd_param_callback* callback, char* buffer, int length, v
  */
 int guacd_parse_log_level(const char* name);
 
+/**
+ * Parses the given client timeout, returning the corresponding number of
+ * milliseconds, or -1 if the value is invalid or cannot safely be converted
+ * to the microsecond timeout expected by libguac.
+ *
+ * @param value
+ *     The timeout value to parse, in milliseconds.
+ *
+ * @return
+ *     The parsed positive timeout in milliseconds, or -1 if invalid.
+ */
+int guacd_parse_client_timeout(const char* value);
+
 /**
  * Human-readable description of the current error, if any.
  */
@@ -65,4 +78,3 @@ extern char* guacd_conf_parse_error;
 extern char* guacd_conf_parse_error_location;
 
 #endif
-

src/guacd/conf.h

@@ -22,6 +22,8 @@
 
 #include <guacamole/client.h>
 
+#include <limits.h>
+
 /**
  * The default host that guacd should bind to, if no other host is explicitly
  * specified.
@@ -34,6 +36,18 @@
  */
 #define GUACD_DEFAULT_BIND_PORT "4822"
 
+/**
+ * The default number of milliseconds that guacd should wait for messages from
+ * a connected client before closing the connection.
+ */
+#define GUACD_DEFAULT_CLIENT_TIMEOUT 15000
+
+/**
+ * The maximum client timeout, in milliseconds. The corresponding timeout in
+ * microseconds must fit within the signed integer accepted by libguac.
+ */
+#define GUACD_MAX_CLIENT_TIMEOUT (INT_MAX / 1000)
+
 /**
  * The contents of a guacd configuration file.
  */
@@ -49,6 +63,12 @@ typedef struct guacd_config {
      */
     char* bind_port;
 
+    /**
+     * The number of milliseconds to wait for messages from a connected client
+     * before closing the connection.
+     */
+    int client_timeout;
+
     /**
      * The file to write the PID in, if any.
      */
@@ -84,4 +104,3 @@ typedef struct guacd_config {
 } guacd_config;
 
 #endif
-

src/guacd/connection.c

@@ -256,11 +256,16 @@ static int guacd_add_user(guacd_proc* proc, guac_parser* parser, guac_socket* so
  *     The socket associated with the new connection that must be routed to
  *     a new or existing process within the given map.
  *
+ * @param usec_timeout
+ *     The number of microseconds to wait for the next message from the
+ *     connected client before closing the connection.
+ *
  * @return
  *     Zero if the connection was successfully routed, non-zero if routing has
  *     failed.
  */
-static int guacd_route_connection(guacd_proc_map* map, guac_socket* socket) {
+static int guacd_route_connection(guacd_proc_map* map, guac_socket* socket,
+        int usec_timeout) {
 
     guac_parser* parser = guac_parser_alloc();
 
@@ -269,7 +274,7 @@ static int guacd_route_connection(guacd_proc_map* map, guac_socket* socket) {
     guac_error_message = NULL;
 
     /* Get protocol from select instruction */
-    if (guac_parser_expect(parser, socket, GUACD_USEC_TIMEOUT, "select")) {
+    if (guac_parser_expect(parser, socket, usec_timeout, "select")) {
 
         /* Log error */
         guacd_log_handshake_failure();
@@ -323,7 +328,7 @@ static int guacd_route_connection(guacd_proc_map* map, guac_socket* socket) {
                 identifier);
 
         /* Create new process */
-        proc = guacd_create_proc(identifier);
+        proc = guacd_create_proc(identifier, usec_timeout);
         new_process = 1;
 
     }
@@ -397,6 +402,7 @@ void* guacd_connection_thread(void* data) {
 
     guacd_proc_map* map = params->map;
     int connected_socket_fd = params->connected_socket_fd;
+    int usec_timeout = params->usec_timeout;
 
     guac_socket* socket;
 
@@ -423,7 +429,7 @@ void* guacd_connection_thread(void* data) {
 #endif
 
     /* Route connection according to Guacamole, creating a new process if needed */
-    if (guacd_route_connection(map, socket))
+    if (guacd_route_connection(map, socket, usec_timeout))
         guac_socket_free(socket);
 
     guac_mem_free(params);

src/guacd/connection.h

@@ -49,6 +49,12 @@ typedef struct guacd_connection_thread_params {
      */
     int connected_socket_fd;
 
+    /**
+     * The number of microseconds to wait for messages from the connected
+     * client before closing the connection.
+     */
+    int usec_timeout;
+
 } guacd_connection_thread_params;
 
 /**
@@ -117,4 +123,3 @@ typedef struct guacd_connection_io_thread_params {
 void* guacd_connection_io_thread(void* data);
 
 #endif
-

src/guacd/daemon.c

@@ -352,6 +352,7 @@ int main(int argc, char* argv[]) {
 
     /* Log start */
     guacd_log(GUAC_LOG_INFO, "Guacamole proxy daemon (guacd) version " VERSION " started");
+    guacd_log(GUAC_LOG_INFO, "Client timeout is %i milliseconds", config->client_timeout);
 
     /* Get addresses for binding */
     if ((retval = getaddrinfo(config->bind_host, config->bind_port,
@@ -576,6 +577,7 @@ int main(int argc, char* argv[]) {
 
         params->map = map;
         params->connected_socket_fd = connected_socket_fd;
+        params->usec_timeout = config->client_timeout * 1000;
 
 #ifdef ENABLE_SSL
         params->ssl_context = ssl_context;

src/guacd/man/guacd.8.in

@@ -27,6 +27,7 @@ guacd \- Guacamole proxy daemon
 [\fB-l\fR \fIPORT\fR]
 [\fB-p\fR \fIPID FILE\fR]
 [\fB-L\fR \fILOG LEVEL\fR]
+[\fB-T\fR \fIMILLISECONDS\fR]
 [\fB-C\fR \fICERTIFICATE FILE\fR]
 [\fB-K\fR \fIKEY FILE\fR]
 [\fB-f\fR]
@@ -76,6 +77,12 @@ and
 The default value is
 .B info.
 .TP
+\fB\-T\fR \fIMILLISECONDS\fR
+Sets the maximum number of milliseconds that
+.B guacd
+will wait for the next message from a connected client before closing the
+connection. The value must be a positive integer and defaults to 15000.
+.TP
 \fB\-f\fR
 Causes
 .B guacd

src/guacd/man/guacd.conf.5.in

@@ -101,6 +101,12 @@ Requires
 to bind to a specific port when listening for connections. By default,
 .B guacd
 will bind to port 4822.
+.TP
+\fBclient_timeout\fR \fB=\fR \fIMILLISECONDS\fR
+Sets the maximum number of milliseconds that
+.B guacd
+will wait for the next message from a connected client before closing the
+connection. The value must be a positive integer and defaults to 15000.
 .
 .SH DAEMON PARAMETERS
 .TP
@@ -169,6 +175,7 @@ pid_file = /var/run/guacd.pid
 
 bind_host = localhost
 bind_port = 4822
+client_timeout = 15000
 
 [ssl]