GUACAMOLE-221: Fix up lots of comments, streamline code, and fix SSH mutex lock.

This commit is contained in:
Nick Couchman 2020-07-01 16:37:55 -04:00
parent bc8ed4e104
commit 0db61198e9
7 changed files with 89 additions and 28 deletions

View File

@ -20,8 +20,6 @@
#ifndef _GUAC_PROTOCOL_TYPES_H #ifndef _GUAC_PROTOCOL_TYPES_H
#define _GUAC_PROTOCOL_TYPES_H #define _GUAC_PROTOCOL_TYPES_H
#include <stddef.h>
/** /**
* Type definitions related to the Guacamole protocol. * Type definitions related to the Guacamole protocol.
* *

View File

@ -96,9 +96,10 @@ ssize_t __guac_socket_write_length_double(guac_socket* socket, double d) {
} }
/** /**
* Loop through the provided NULL-terminated array, writing the values * Loop through the provided NULL-terminated array, writing the values in the
* and lengths of the values in the array to the given socket. Return * array to the given socket. Values are written as a series of Guacamole
* zero on success, non-zero on error. * protocol elements, including the leading comma and the value length in
* addition to the value itself. Returns zero on success, non-zero on error.
* *
* @param socket * @param socket
* The socket to which the data should be written. * The socket to which the data should be written.
@ -1010,13 +1011,9 @@ int guac_protocol_send_required(guac_socket* socket, const char** required) {
guac_socket_instruction_begin(socket); guac_socket_instruction_begin(socket);
if (guac_socket_write_string(socket, "8.required")) ret_val = guac_socket_write_string(socket, "8.required")
return -1; || guac_socket_write_array(socket, required)
|| guac_socket_write_string(socket, ";");
if (guac_socket_write_array(socket, required))
return -1;
ret_val = guac_socket_write_string(socket, ";");
guac_socket_instruction_end(socket); guac_socket_instruction_end(socket);

View File

@ -183,7 +183,12 @@ typedef struct guac_rdp_client {
/** /**
* Flags for tracking events related to the rdp_credential_cond * Flags for tracking events related to the rdp_credential_cond
* pthread condition. * 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
* information is required.
*
* *
* @see GUAC_RDP_CRED_FLAG_USERNAME * @see GUAC_RDP_CRED_FLAG_USERNAME
* @see GUAC_RDP_CRED_FLAG_PASSWORD * @see GUAC_RDP_CRED_FLAG_PASSWORD

View File

@ -120,6 +120,10 @@ int guac_ssh_client_free_handler(guac_client* client) {
guac_common_clipboard_free(ssh_client->clipboard); guac_common_clipboard_free(ssh_client->clipboard);
free(ssh_client); free(ssh_client);
/* Destroy the pthread conditional handler */
pthread_cond_destroy(&(ssh_client->ssh_credential_cond));
pthread_mutex_destroy(&(ssh_client->ssh_credential_lock));
guac_common_ssh_uninit(); guac_common_ssh_uninit();
return 0; return 0;
} }

View File

@ -58,6 +58,35 @@
#include <sys/socket.h> #include <sys/socket.h>
#include <sys/time.h> #include <sys/time.h>
/**
* A data structure that maps parameter names of credentials that will be
* prompted to the actual text that will be displayed on the screen.
*/
typedef struct guac_ssh_credential_prompt_map {
/**
* The parameter name of the credential that is being prompted for.
*/
char* name;
/**
* The text that will display to the user during the prompt.
*/
char* prompt;
} guac_ssh_credential_prompt_map;
/**
* The map of credentials the user can be prompted for to the text displayed
* on the screen.
*/
guac_ssh_credential_prompt_map ssh_credential_prompt_map[] = {
{ GUAC_SSH_PARAMETER_NAME_USERNAME, "Login as: " },
{ GUAC_SSH_PARAMETER_NAME_PASSWORD, "Password: " },
{ GUAC_SSH_PARAMETER_NAME_PASSPHRASE, "Key passphrase: " },
{ NULL, NULL}
};
/** /**
* This function generates a prompt to the specified instance of guac_client * This function generates a prompt to the specified instance of guac_client
* for the credential specified in the cred_name parameter, which should * for the credential specified in the cred_name parameter, which should
@ -74,19 +103,15 @@ static void guac_ssh_get_credential(guac_client *client, char* cred_name) {
guac_ssh_client* ssh_client = (guac_ssh_client*) client->data; guac_ssh_client* ssh_client = (guac_ssh_client*) client->data;
/* If the client does not support the "required" instruction, just return. */
if (!guac_client_owner_supports_required(client))
return;
/* Lock the terminal thread while prompting for the credential. */ /* Lock the terminal thread while prompting for the credential. */
pthread_mutex_lock(&(ssh_client->term_channel_lock)); pthread_mutex_lock(&(ssh_client->ssh_credential_lock));
/* Let the owner know what we require. */ /* Let the owner know what we require. */
guac_client_owner_send_required(client, (const char* []) {cred_name, NULL}); guac_client_owner_send_required(client, (const char* []) {cred_name, NULL});
/* Wait for the response, and then unlock the thread. */ /* Wait for the response, and then unlock the thread. */
pthread_cond_wait(&(ssh_client->ssh_credential_cond), &(ssh_client->term_channel_lock)); pthread_cond_wait(&(ssh_client->ssh_credential_cond), &(ssh_client->ssh_credential_lock));
pthread_mutex_unlock(&(ssh_client->term_channel_lock)); pthread_mutex_unlock(&(ssh_client->ssh_credential_lock));
} }
@ -110,8 +135,20 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) {
guac_common_ssh_user* user; guac_common_ssh_user* user;
/* Get username */ /* Get username */
while (settings->username == NULL) while (settings->username == NULL) {
/* Client owner supports required instruction, so send prompt(s) that way. */
if (guac_client_owner_supports_required(client)) {
guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_USERNAME); guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_USERNAME);
}
/* Fall back to terminal prompting. */
else {
settings->username = guac_terminal_prompt(ssh_client->term,
"Login as: ", true);
}
}
/* Create user object from username */ /* Create user object from username */
user = guac_common_ssh_create_user(settings->username); user = guac_common_ssh_create_user(settings->username);
@ -135,9 +172,19 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) {
"Re-attempting private key import (WITH passphrase)"); "Re-attempting private key import (WITH passphrase)");
/* Prompt for passphrase if missing */ /* Prompt for passphrase if missing */
while (settings->key_passphrase == NULL) while (settings->key_passphrase == NULL) {
/* Send prompt via required instruction, if supported */
if (guac_client_owner_supports_required(client))
guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_PASSPHRASE); guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_PASSPHRASE);
/* Fall back to terminal prompt */
else
settings->key_passphrase = guac_terminal_prompt(ssh_client->term,
"Key passphrase: ", true);
}
/* Reattempt import with passphrase */ /* Reattempt import with passphrase */
if (guac_common_ssh_user_import_key(user, if (guac_common_ssh_user_import_key(user,
settings->key_base64, settings->key_base64,
@ -285,6 +332,7 @@ void* ssh_client_thread(void* data) {
} }
pthread_mutex_init(&ssh_client->term_channel_lock, NULL); 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 */ /* Open channel for terminal */

View File

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

View File

@ -158,7 +158,10 @@ typedef struct guac_vnc_client {
/** /**
* A field to track flags related to retrieving required credentials * A field to track flags related to retrieving required credentials
* from the client. * from the client. These flags will be set when credentials are required
* that have not been provided, and cleared when those credentials are
* provided. All flags are cleared at the start of the connection, and
* then set as the client determines which are required.
* *
* @see GUAC_VNC_COND_FLAG_USERNAME * @see GUAC_VNC_COND_FLAG_USERNAME
* @see GUAC_VNC_COND_FLAG_PASSWORD * @see GUAC_VNC_COND_FLAG_PASSWORD