GUACAMOLE-221: Add back in SSH credential argv support; fix style and comments.

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

@@ -36,8 +36,8 @@
  *     The connection parameter that is being requested from the client.
  * 
  * @return
- *     A newly-allocated string containing the credentials request from the
+ *     A newly-allocated string containing the credentials to be requested from
- *     client, or NULL if the credentials will be updated via the required
+ *     the client, or NULL if the credentials will be updated via the required
  *     instruction.
  */
 typedef char* guac_ssh_credential_handler(guac_client* client, char* cred_name);

src/common-ssh/ssh.c

@@ -363,7 +363,7 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session)
         
         char* password = common_session->credential_handler(client, "password");
         if (password != NULL)
-            user->password = password;
+            guac_common_ssh_user_set_password(user, password);
         
     }
     

src/libguac/client.c

@@ -479,7 +479,7 @@ int guac_client_load_plugin(guac_client* client, const char* protocol) {
 }
 
 /**
- * Callback function which is invoked by guac_client_owner_send_required() to
+ * A callback function which is invoked by guac_client_owner_send_required() to
  * send the required parameters to the specified user, who is the owner of the
  * client session.
  * 
@@ -488,7 +488,7 @@ int guac_client_load_plugin(guac_client* client, const char* protocol) {
  *     of the client.
  * 
  * @param data
- *     Pointer to a NULL-terminated array of required parameters that will be
+ *     A pointer to a NULL-terminated array of required parameters that will be
  *     passed on to the owner to continue the connection.
  * 
  * @return
@@ -670,7 +670,7 @@ static void* __webp_support_callback(guac_user* user, void* data) {
 #endif
 
 /**
- * Callback function which is invoked by guac_client_owner_supports_required()
+ * A callback function which is invoked by guac_client_owner_supports_required()
  * to determine if the owner of a client supports the "required" instruction,
  * updating the flag to indicate support.
  * 
@@ -678,7 +678,7 @@ static void* __webp_support_callback(guac_user* user, void* data) {
  *     The guac_user that will be checked for "required" instruction support.
  * 
  * @param data
- *     Pointer to an int containing the status for support of the "required"
+ *     A pointer to an int containing the status for support of the "required"
  *     instruction.  This will be 0 if the owner does not support this
  *     instruction, or 1 if the owner does support it.
  * 

src/libguac/guacamole/protocol-types.h

@@ -277,8 +277,8 @@ typedef enum guac_line_join_style {
 } guac_line_join_style;
 
 /**
- * Track the various protocol versions supported by guacd to help negotiate
+ * The set of protocol versions known to guacd to help negotiate features that
- * features that may not be supported by various versions of the client.
+ * may not be supported by various versions of the client.
  */
 typedef enum guac_protocol_version {
     
@@ -289,7 +289,8 @@ typedef enum guac_protocol_version {
     
     /**
      * Original protocol version 1.0.0, which lacks support for negotiating
-     * parameters and protocol version.
+     * parameters and protocol version, and requires that parameters in the
+     * client/server handshake be delivered in order.
      */
     GUAC_PROTOCOL_VERSION_1_0_0 = 0x010000,
             

src/libguac/protocol.c

@@ -41,13 +41,13 @@
 #include <sys/types.h>
 
 /**
- * Structure mapping the enum value of a protocol version to the string
+ * A structure mapping the enum value of a Guacamole protocol version to the
- * representation of the version.
+ * string representation of the version.
  */
 typedef struct guac_protocol_version_mapping {
     
     /**
-     * The enum value representing the selected protocol version.
+     * The enum value of the protocol version.
      */
     guac_protocol_version version;
     
@@ -59,7 +59,7 @@ typedef struct guac_protocol_version_mapping {
 } guac_protocol_version_mapping;
 
 /**
- * The map of known protocol version enum to the corresponding string value.
+ * The map of known protocol versions to the corresponding string value.
  */
 guac_protocol_version_mapping guac_protocol_version_table[] = {
     { GUAC_PROTOCOL_VERSION_1_0_0,   "VERSION_1_0_0" },

src/protocols/rdp/argv.c

@@ -155,11 +155,11 @@ int guac_rdp_argv_handler(guac_user* user, guac_stream* stream,
     guac_rdp_argv_setting setting;
 
     /* Allow users to update authentication details */
-    if (strcmp(name, GUAC_RDP_PARAMETER_NAME_USERNAME) == 0)
+    if (strcmp(name, GUAC_RDP_ARGV_USERNAME) == 0)
         setting = GUAC_RDP_ARGV_SETTING_USERNAME;
-    else if (strcmp(name, GUAC_RDP_PARAMETER_NAME_PASSWORD) == 0)
+    else if (strcmp(name, GUAC_RDP_ARGV_PASSWORD) == 0)
         setting = GUAC_RDP_ARGV_SETTING_PASSWORD;
-    else if (strcmp(name, GUAC_RDP_PARAMETER_NAME_DOMAIN) == 0)
+    else if (strcmp(name, GUAC_RDP_ARGV_DOMAIN) == 0)
         setting = GUAC_RDP_ARGV_SETTING_DOMAIN;
 
     /* No other connection parameters may be updated */

src/protocols/rdp/argv.h

@@ -37,5 +37,23 @@
  */
 guac_user_argv_handler guac_rdp_argv_handler;
 
+/**
+ * The name of the parameter that specifies/updates the username that will be
+ * sent to the RDP server during authentication.
+ */
+#define GUAC_RDP_ARGV_USERNAME "username"
+
+/**
+ * The name of the parameter that specifies/updates the password that will be
+ * sent to the RDP server during authentication.
+ */
+#define GUAC_RDP_ARGV_PASSWORD "password"
+
+/**
+ * The name of the parameter that specifies/updates the domain name that will be
+ * sent to the RDP server during authentication.
+ */
+#define GUAC_RDP_ARGV_DOMAIN "domain"
+
 #endif
 

src/protocols/rdp/rdp.c

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+#include "argv.h"
 #include "beep.h"
 #include "bitmap.h"
 #include "channels/audio-input/audio-buffer.h"
@@ -200,10 +201,15 @@ BOOL rdp_freerdp_pre_connect(freerdp* instance) {
 }
 
 /**
- * Callback invoked by FreeRDP when authentication is required but a username
+ * Callback invoked by FreeRDP when authentication is required but the required
- * and password has not already been given. In the case of Guacamole, this
+ * parameters have not been provided. In the case of Guacamole clients that
- * function always succeeds but does not populate the username or password. The
+ * support the "required" instruction, this function will send any of the three
- * username/password must be given within the connection parameters.
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information.  If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true.
  *
  * @param instance
  *     The FreeRDP instance associated with the RDP session requesting
@@ -232,25 +238,33 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
     char* params[4] = {};
     int i = 0;
     
-    /* If the client does not support the "required" instruction, just
+    /* If the client does not support the "required" instruction, warn and
-        exit. */
+     * quit.
-    if (!guac_client_owner_supports_required(client))
+     */
+    if (!guac_client_owner_supports_required(client)) {
+        guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+                "\"required\" instruction. No authentication parameters will "
+                "be requested.");
         return TRUE;
+    }
     
+    /* If the username is undefined, add it to the requested parameters. */
     if (settings->username == NULL) {
-        params[i] = GUAC_RDP_PARAMETER_NAME_USERNAME;
+        params[i] = GUAC_RDP_ARGV_USERNAME;
         rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_USERNAME;
         i++;
     }
     
+    /* If the password is undefined, add it to the requested parameters. */
     if (settings->password == NULL) {
-        params[i] = GUAC_RDP_PARAMETER_NAME_PASSWORD;
+        params[i] = GUAC_RDP_ARGV_PASSWORD;
         rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_PASSWORD;
         i++;
     }
     
+    /* If the domain is undefined, add it to the requested parameters. */
     if (settings->domain == NULL) {
-        params[i] = GUAC_RDP_PARAMETER_NAME_DOMAIN;
+        params[i] = GUAC_RDP_ARGV_DOMAIN;
         rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_DOMAIN;
         i++;
     }

src/protocols/rdp/rdp.h

@@ -186,7 +186,7 @@ typedef struct guac_rdp_client {
      * pthread condition. These flags will be set when credential parameters
      * are required by the connection, and cleared when those have been
      * provided by the client. All flags are cleared at the start of the
-     * connection, and then set as the client determines that further
+     * connection, and then set as the RDP client determines that further
      * information is required.
      * 
      * 

src/protocols/rdp/settings.c

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+#include "argv.h"
 #include "common/defaults.h"
 #include "common/string.h"
 #include "config.h"
@@ -42,9 +43,9 @@
 const char* GUAC_RDP_CLIENT_ARGS[] = {
     "hostname",
     "port",
-    GUAC_RDP_PARAMETER_NAME_DOMAIN,
+    GUAC_RDP_ARGV_DOMAIN,
-    GUAC_RDP_PARAMETER_NAME_USERNAME,
+    GUAC_RDP_ARGV_USERNAME,
-    GUAC_RDP_PARAMETER_NAME_PASSWORD,
+    GUAC_RDP_ARGV_PASSWORD,
     "width",
     "height",
     "dpi",

src/protocols/rdp/settings.h

@@ -73,24 +73,6 @@
  */
 #define GUAC_RDP_ORDER_SUPPORT_LENGTH 32
 
-/**
- * The name of the parameter that is used by Guacamole to collect the username
- * from the Guacamole client and send it to the RDP server.
- */
-#define GUAC_RDP_PARAMETER_NAME_USERNAME "username"
-
-/**
- * The name of the parameter that is used by Guacamole to collect the password
- * from the Guacamole client and send it to the RDP server.
- */
-#define GUAC_RDP_PARAMETER_NAME_PASSWORD "password"
-
-/**
- * The name of the parameter that is used by Guacamole to collect the domain
- * name from the Guacamole client and send it to the RDP server.
- */
-#define GUAC_RDP_PARAMETER_NAME_DOMAIN "domain"
-
 /**
  * All supported combinations of security types.
  */

src/protocols/ssh/argv.c

@@ -18,7 +18,10 @@
  */
 
 #include "config.h"
+
 #include "argv.h"
+#include "common-ssh/user.h"
+#include "settings.h"
 #include "ssh.h"
 #include "terminal/terminal.h"
 
@@ -36,9 +39,30 @@ int guac_ssh_argv_callback(guac_user* user, const char* mimetype,
     guac_client* client = user->client;
     guac_ssh_client* ssh_client = (guac_ssh_client*) client->data;
     guac_terminal* terminal = ssh_client->term;
+    guac_ssh_settings* settings = ssh_client->settings;
+
+    /* Update username */
+    if (strcmp(name, GUAC_SSH_ARGV_USERNAME) == 0) {
+        free(settings->username);
+        settings->username = strdup(value);
+        pthread_cond_signal(&ssh_client->ssh_credential_cond);
+    }
+    
+    /* Update password */
+    else if (strcmp(name, GUAC_SSH_ARGV_PASSWORD) == 0) {
+        guac_common_ssh_user_set_password(ssh_client->user, value);
+        pthread_cond_signal(&ssh_client->ssh_credential_cond);
+    }
+    
+    /* Update private key passphrase */
+    else if (strcmp(name, GUAC_SSH_ARGV_PASSPHRASE) == 0) {
+        free(settings->key_passphrase);
+        settings->key_passphrase = strdup(value);
+        pthread_cond_signal(&ssh_client->ssh_credential_cond);
+    }
     
     /* Update color scheme */
-    if (strcmp(name, GUAC_SSH_ARGV_COLOR_SCHEME) == 0)
+    else if (strcmp(name, GUAC_SSH_ARGV_COLOR_SCHEME) == 0)
         guac_terminal_apply_color_scheme(terminal, value);
 
     /* Update font name */

src/protocols/ssh/argv.h

@@ -57,7 +57,7 @@
 #define GUAC_SSH_ARGV_PASSWORD "password"
 
 /**
- * The name of the parameter that specifies/updates the private ky passphrase
+ * The name of the parameter that specifies/updates the private key passphrase
  * used by the connection.
  */
 #define GUAC_SSH_ARGV_PASSPHRASE "passphrase"

src/protocols/ssh/client.c

@@ -53,6 +53,9 @@ int guac_client_init(guac_client* client) {
     client->free_handler = guac_ssh_client_free_handler;
 
     /* Register handlers for argument values that may be sent after the handshake */
+    guac_argv_register(GUAC_SSH_ARGV_USERNAME, guac_ssh_argv_callback, NULL, 0);
+    guac_argv_register(GUAC_SSH_ARGV_PASSWORD, guac_ssh_argv_callback, NULL, 0);
+    guac_argv_register(GUAC_SSH_ARGV_PASSPHRASE, guac_ssh_argv_callback, NULL, 0);
     guac_argv_register(GUAC_SSH_ARGV_COLOR_SCHEME, guac_ssh_argv_callback, NULL, GUAC_ARGV_OPTION_ECHO);
     guac_argv_register(GUAC_SSH_ARGV_FONT_NAME, guac_ssh_argv_callback, NULL, GUAC_ARGV_OPTION_ECHO);
     guac_argv_register(GUAC_SSH_ARGV_FONT_SIZE, guac_ssh_argv_callback, NULL, GUAC_ARGV_OPTION_ECHO);

src/protocols/ssh/ssh.c

@@ -87,9 +87,13 @@ guac_ssh_credential_prompt_map ssh_credential_prompt_map[] = {
 };
 
 /**
- * This function generates a prompt to the specified instance of guac_client
+ * Prompts the user for the credential specified in the cred_name parameter in
- * for the credential specified in the cred_name parameter, which should
+ * order to continue a SSH connection. If the owner of the connection is running
- * be a valid SSH connection parameter.
+ * a client that supports the "required" instruction this credential will be
+ * sent to the client with that instruction and updated directly via the argv
+ * handler. If the client does not support the "required" instruction the prompt
+ * will be generated directly on the terminal and the value returned through
+ * this function.
  * 
  * @param client
  *     The guac_client object associated with the current connection
@@ -97,6 +101,13 @@ guac_ssh_credential_prompt_map ssh_credential_prompt_map[] = {
  * 
  * @param cred_name
  *     The name of the parameter to prompt for in the client.
+ * 
+ * @return
+ *     If the client does not support the "required" instruction, the value
+ *     provided by the user on the terminal will be returned. NULL will be
+ *     returned if either the client supports the "required" instruction and
+ *     the parameter will be updated directly, or if the parameter specified
+ *     is invalid.
  */
 static char* guac_ssh_get_credential(guac_client *client, char* cred_name) {
 
@@ -118,7 +129,7 @@ static char* guac_ssh_get_credential(guac_client *client, char* cred_name) {
         return NULL;
     }
     
-    /* Lock the terminal thread while prompting for the credential. */
+    /* Lock the SSH client thread while prompting for the credential. */
     pthread_mutex_lock(&(ssh_client->ssh_credential_lock));
     
     /* Let the owner know what we require. */
@@ -152,7 +163,7 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) {
     guac_common_ssh_user* user;
 
     /* Get username */
-    while (settings->username == NULL) {
+    if (settings->username == NULL) {
         
         char* username = guac_ssh_get_credential(client, GUAC_SSH_ARGV_USERNAME);
         if (username != NULL)
@@ -182,7 +193,7 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) {
                     "Re-attempting private key import (WITH passphrase)");
 
             /* Prompt for passphrase if missing */
-            while (settings->key_passphrase == NULL) {
+            if (settings->key_passphrase == NULL) {
                 
                 char* passphrase = guac_ssh_get_credential(client, GUAC_SSH_ARGV_PASSPHRASE);
                 if (passphrase != NULL)
@@ -338,7 +349,7 @@ void* ssh_client_thread(void* data) {
 
     pthread_mutex_init(&ssh_client->term_channel_lock, NULL);
     pthread_mutex_init(&ssh_client->ssh_credential_lock, NULL);
-    pthread_cond_init(&(ssh_client->ssh_credential_cond), NULL);
+    pthread_cond_init(&ssh_client->ssh_credential_cond, NULL);
 
     /* Open channel for terminal */
     ssh_client->term_channel =
@@ -553,7 +564,7 @@ void* ssh_client_thread(void* data) {
     guac_client_stop(client);
     pthread_join(input_thread, NULL);
 
-    pthread_cond_destroy(&(ssh_client->ssh_credential_cond));
+    pthread_cond_destroy(&ssh_client->ssh_credential_cond);
     pthread_mutex_destroy(&ssh_client->term_channel_lock);
 
     guac_client_log(client, GUAC_LOG_INFO, "SSH connection ended.");

src/protocols/ssh/ssh.h

@@ -91,13 +91,13 @@ typedef struct guac_ssh_client {
     pthread_mutex_t term_channel_lock;
     
     /**
-     * Lock that will be locked when retrieving required credentials from the
+     * Lock that controls access to updating credential parameters for the
-     * user, and unlocked when those requirements are satisfied.
+     * SSH connection.
      */
     pthread_mutex_t ssh_credential_lock;
     
     /**
-     * Condition used when SSH client thread needs to wait for Guacamole
+     * Condition used when the SSH client thread needs to wait for Guacamole
      * client to pass additional credentials before continuing the connection.
      */
     pthread_cond_t ssh_credential_cond;

src/protocols/vnc/argv.c

@@ -153,9 +153,9 @@ int guac_vnc_argv_handler(guac_user* user, guac_stream* stream, char* mimetype,
     guac_vnc_argv_setting setting;
 
     /* Allow users to update authentication information */
-    if (strcmp(name, GUAC_VNC_PARAMETER_NAME_USERNAME) == 0)
+    if (strcmp(name, GUAC_VNC_ARGV_USERNAME) == 0)
         setting = GUAC_VNC_ARGV_SETTING_USERNAME;
-    else if (strcmp(name, GUAC_VNC_PARAMETER_NAME_PASSWORD) == 0)
+    else if (strcmp(name, GUAC_VNC_ARGV_PASSWORD) == 0)
         setting = GUAC_VNC_ARGV_SETTING_PASSWORD;
 
     /* No other connection parameters may be updated */

src/protocols/vnc/argv.h

@@ -37,5 +37,17 @@
  */
 guac_user_argv_handler guac_vnc_argv_handler;
 
+/**
+ * The name of the parameter Guacamole will use to specify/update the username
+ * for the VNC connection.
+ */
+#define GUAC_VNC_ARGV_USERNAME "username"
+
+/**
+ * The name of the parameter Guacamole will use to specify/update the password
+ * for the VNC connection.
+ */
+#define GUAC_VNC_ARGV_PASSWORD "password"
+
 #endif /* ARGV_H */
 

src/protocols/vnc/auth.c

@@ -19,6 +19,7 @@
 
 #include "config.h"
 
+#include "argv.h"
 #include "auth.h"
 #include "vnc.h"
 
@@ -48,7 +49,7 @@ char* guac_vnc_get_password(rfbClient* client) {
         
         /* Send the request for password to the owner. */
         guac_client_owner_send_required(gc,
-                (const char* []) {GUAC_VNC_PARAMETER_NAME_PASSWORD, NULL});
+                (const char* []) {GUAC_VNC_ARGV_PASSWORD, NULL});
         
         /* Set the conditional flag. */
         vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_PASSWORD;
@@ -87,14 +88,14 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) {
         
         /* Check if username is null or empty. */
         if (settings->username == NULL) {
-            params[i] = GUAC_VNC_PARAMETER_NAME_USERNAME;
+            params[i] = GUAC_VNC_ARGV_USERNAME;
             i++;
             vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_USERNAME;
         }
         
         /* Check if password is null or empty. */
         if (settings->password == NULL) {
-            params[i] = GUAC_VNC_PARAMETER_NAME_PASSWORD;
+            params[i] = GUAC_VNC_ARGV_PASSWORD;
             i++;
             vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_PASSWORD;
         }

src/protocols/vnc/client.c

@@ -51,12 +51,12 @@ int guac_client_init(guac_client* client) {
 
 #ifdef ENABLE_VNC_TLS_LOCKING
     /* Initialize the TLS write lock */
-    pthread_mutex_init(&(vnc_client->tls_lock), NULL);
+    pthread_mutex_init(&vnc_client->tls_lock, NULL);
 #endif
     
     /* Initialize credential lock, cond, and flags */
-    pthread_mutex_init(&(vnc_client->vnc_credential_lock), NULL);
+    pthread_mutex_init(&vnc_client->vnc_credential_lock, NULL);
-    pthread_cond_init(&(vnc_client->vnc_credential_cond), NULL);
+    pthread_cond_init(&vnc_client->vnc_credential_cond, NULL);
     vnc_client->vnc_credential_flags = 0;
 
     /* Init clipboard */
@@ -142,8 +142,8 @@ int guac_vnc_client_free_handler(guac_client* client) {
 #endif
     
     /* Clean up credential mutex */
-    pthread_cond_destroy(&(vnc_client->vnc_credential_cond));
+    pthread_cond_destroy(&vnc_client->vnc_credential_cond);
-    pthread_mutex_destroy(&(vnc_client->vnc_credential_lock));
+    pthread_mutex_destroy(&vnc_client->vnc_credential_lock);
 
     /* Free generic data struct */
     free(client->data);

src/protocols/vnc/settings.c

@@ -19,6 +19,7 @@
 
 #include "config.h"
 
+#include "argv.h"
 #include "client.h"
 #include "common/defaults.h"
 #include "settings.h"
@@ -37,8 +38,8 @@ const char* GUAC_VNC_CLIENT_ARGS[] = {
     "port",
     "read-only",
     "encodings",
-    GUAC_VNC_PARAMETER_NAME_USERNAME,
+    GUAC_VNC_ARGV_USERNAME,
-    GUAC_VNC_PARAMETER_NAME_PASSWORD,
+    GUAC_VNC_ARGV_PASSWORD,
     "swap-red-blue",
     "color-depth",
     "cursor",

src/protocols/vnc/settings.h

@@ -30,18 +30,6 @@
  */
 #define GUAC_VNC_DEFAULT_RECORDING_NAME "recording"
 
-/**
- * The name of the parameter Guacamole will use to collect the username from the
- * Guacamole client to send to the VNC server.
- */
-#define GUAC_VNC_PARAMETER_NAME_USERNAME "username"
-
-/**
- * The name of the parameter Guacamole will use to collect the password from the
- * Guacamole client to send to the VNC server.
- */
-#define GUAC_VNC_PARAMETER_NAME_PASSWORD "password"
-
 /**
  * VNC-specific client data.
  */

src/protocols/vnc/vnc.h

@@ -46,12 +46,12 @@
 #include <pthread.h>
 
 /**
- * A flag for tracking status of requesting username from client.
+ * A flag for tracking the status of requesting username from client.
  */
 #define GUAC_VNC_COND_FLAG_USERNAME 1
 
 /**
- * A flag for tracking status of requesting password from client.
+ * A flag for tracking the status of requesting password from client.
  */
 #define GUAC_VNC_COND_FLAG_PASSWORD 2
 
@@ -145,13 +145,13 @@ typedef struct guac_vnc_client {
     guac_iconv_write* clipboard_writer;
     
     /**
-     * A lock that will be locked when retrieving required credentials from
+     * A lock that controls access to updating credentials when connecting
-     * the client, and unlocked when credentials have been retrieved.
+     * to a VNC server.
      */
     pthread_mutex_t vnc_credential_lock;
     
     /**
-     * A condition to use for signaling the thread when credentials have been
+     * A condition for signaling the thread when credentials have been
      * retrieved from the client.
      */
     pthread_cond_t vnc_credential_cond;