From 21a5d9ee62da27130ff2193547cb176392c33d22 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 2 Mar 2019 11:04:48 -0500 Subject: [PATCH 01/26] GUACAMOLE-221: Add protocol functions for sending prompt to client. --- src/common-ssh/common-ssh/ssh.h | 11 +- src/common-ssh/ssh.c | 2 +- src/libguac/guacamole/protocol.h | 16 +++ src/libguac/protocol.c | 15 +++ src/protocols/rdp/Makefile.am | 2 + src/protocols/rdp/argv.c | 189 +++++++++++++++++++++++++++++++ src/protocols/rdp/argv.h | 41 +++++++ src/protocols/rdp/client.c | 7 ++ src/protocols/rdp/rdp.c | 33 +++++- src/protocols/rdp/rdp.h | 12 ++ src/protocols/rdp/user.c | 3 + src/protocols/ssh/argv.c | 1 + src/protocols/ssh/ssh.c | 62 +++++----- src/protocols/ssh/ssh.h | 6 + src/protocols/vnc/Makefile.am | 2 + src/protocols/vnc/argv.c | 163 ++++++++++++++++++++++++++ src/protocols/vnc/argv.h | 41 +++++++ src/protocols/vnc/auth.c | 19 +++- src/protocols/vnc/client.c | 10 +- src/protocols/vnc/vnc.h | 10 ++ 20 files changed, 600 insertions(+), 45 deletions(-) create mode 100644 src/protocols/rdp/argv.c create mode 100644 src/protocols/rdp/argv.h create mode 100644 src/protocols/vnc/argv.c create mode 100644 src/protocols/vnc/argv.h diff --git a/src/common-ssh/common-ssh/ssh.h b/src/common-ssh/common-ssh/ssh.h index 346d8abc..698fac17 100644 --- a/src/common-ssh/common-ssh/ssh.h +++ b/src/common-ssh/common-ssh/ssh.h @@ -26,21 +26,16 @@ #include /** - * Handler for retrieving additional credentials. + * A handler function for retrieving additional credentials for the client. * * @param client * The Guacamole Client associated with this need for additional * credentials. * * @param cred_name - * The name of the credential being requested, which will be shared - * with the client in order to generate a meaningful prompt. - * - * @return - * A newly-allocated string containing the credentials provided by - * the user, which must be freed by a call to free(). + * The connection parameter that is being requested from the client. */ -typedef char* guac_ssh_credential_handler(guac_client* client, char* cred_name); +typedef void guac_ssh_credential_handler(guac_client* client, char* cred_name); /** * An SSH session, backed by libssh2 and associated with a particular diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index c03e984d..eed4c003 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -360,7 +360,7 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) /* Attempt authentication with username + password. */ if (user->password == NULL && common_session->credential_handler) - user->password = common_session->credential_handler(client, "Password: "); + common_session->credential_handler(client, "password"); /* Authenticate with password, if provided */ if (user->password != NULL) { diff --git a/src/libguac/guacamole/protocol.h b/src/libguac/guacamole/protocol.h index f4e02880..54f7b072 100644 --- a/src/libguac/guacamole/protocol.h +++ b/src/libguac/guacamole/protocol.h @@ -794,6 +794,22 @@ int guac_protocol_send_push(guac_socket* socket, const guac_layer* layer); int guac_protocol_send_rect(guac_socket* socket, const guac_layer* layer, int x, int y, int width, int height); +/** + * Sends a "required" instruction over the given guac_socket connection. This + * instruction indicates to the client that one or more additional parameters + * is needed to continue the connection. + * + * @param socket + * The guac_socket connection to use. + * + * @param required + * The name of the parameter that is required. + * + * @return + * Zero on success, non-zero on error. + */ +int guac_protocol_send_required(guac_socket* socket, const char* required); + /** * Sends a reset instruction over the given guac_socket connection. * diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index ff73a558..972ac547 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -961,6 +961,21 @@ int guac_protocol_send_rect(guac_socket* socket, } +int guac_protocol_send_required(guac_socket* socket, const char* required) { + + int ret_val; + + guac_socket_instruction_begin(socket); + ret_val = + guac_socket_write_string(socket, "8.required,") + || __guac_socket_write_length_string(socket, required) + || guac_socket_write_string(socket, ";"); + + guac_socket_instruction_end(socket); + return ret_val; + +} + int guac_protocol_send_reset(guac_socket* socket, const guac_layer* layer) { int ret_val; diff --git a/src/protocols/rdp/Makefile.am b/src/protocols/rdp/Makefile.am index ad949b89..fcee5422 100644 --- a/src/protocols/rdp/Makefile.am +++ b/src/protocols/rdp/Makefile.am @@ -38,6 +38,7 @@ nodist_libguac_client_rdp_la_SOURCES = \ _generated_keymaps.c libguac_client_rdp_la_SOURCES = \ + argv.c \ beep.c \ bitmap.c \ channels/audio-input/audio-buffer.c \ @@ -82,6 +83,7 @@ libguac_client_rdp_la_SOURCES = \ user.c noinst_HEADERS = \ + argv.h \ beep.h \ bitmap.h \ channels/audio-input/audio-buffer.h \ diff --git a/src/protocols/rdp/argv.c b/src/protocols/rdp/argv.c new file mode 100644 index 00000000..a79a64c4 --- /dev/null +++ b/src/protocols/rdp/argv.c @@ -0,0 +1,189 @@ +/* + * 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. + */ + +#include "config.h" +#include "argv.h" +#include "rdp.h" + +#include +#include +#include + +#include +#include +#include + +/** + * All RDP connection settings which may be updated by unprivileged users + * through "argv" streams. + */ +typedef enum guac_rdp_argv_setting { + + /** + * The username of the connection. + */ + GUAC_RDP_ARGV_SETTING_USERNAME, + + /** + * The password to authenticate the connection. + */ + GUAC_RDP_ARGV_SETTING_PASSWORD, + + /** + * The domain to use for connection authentication. + */ + GUAC_RDP_ARGV_SETTING_DOMAIN + +} guac_rdp_argv_setting; + +/** + * The value or current status of a connection parameter received over an + * "argv" stream. + */ +typedef struct guac_rdp_argv { + + /** + * The specific setting being updated. + */ + guac_rdp_argv_setting setting; + + /** + * Buffer space for containing the received argument value. + */ + char buffer[GUAC_RDP_ARGV_MAX_LENGTH]; + + /** + * The number of bytes received so far. + */ + int length; + +} guac_rdp_argv; + +/** + * Handler for "blob" instructions which appends the data from received blobs + * to the end of the in-progress argument value buffer. + * + * @see guac_user_blob_handler + */ +static int guac_rdp_argv_blob_handler(guac_user* user, + guac_stream* stream, void* data, int length) { + + guac_rdp_argv* argv = (guac_rdp_argv*) stream->data; + + /* Calculate buffer size remaining, including space for null terminator, + * adjusting received length accordingly */ + int remaining = sizeof(argv->buffer) - argv->length - 1; + if (length > remaining) + length = remaining; + + /* Append received data to end of buffer */ + memcpy(argv->buffer + argv->length, data, length); + argv->length += length; + + return 0; + +} + +/** + * Handler for "end" instructions which applies the changes specified by the + * argument value buffer associated with the stream. + * + * @see guac_user_end_handler + */ +static int guac_rdp_argv_end_handler(guac_user* user, + guac_stream* stream) { + + guac_client* client = user->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + guac_rdp_settings* settings = rdp_client->settings; + + /* Append null terminator to value */ + guac_rdp_argv* argv = (guac_rdp_argv*) stream->data; + argv->buffer[argv->length] = '\0'; + + /* Apply changes to chosen setting */ + switch (argv->setting) { + + /* Update RDP username. */ + case GUAC_RDP_ARGV_SETTING_USERNAME: + free(settings->username); + settings->username = malloc(strlen(argv->buffer) * sizeof(char)); + strcpy(settings->username, argv->buffer); + pthread_cond_broadcast(&(rdp_client->rdp_cond)); + break; + + case GUAC_RDP_ARGV_SETTING_PASSWORD: + free(settings->password); + settings->password = malloc(strlen(argv->buffer) * sizeof(char)); + strcpy(settings->password, argv->buffer); + pthread_cond_broadcast(&(rdp_client->rdp_cond)); + break; + + case GUAC_RDP_ARGV_SETTING_DOMAIN: + free(settings->domain); + settings->domain = malloc(strlen(argv->buffer) * sizeof(char)); + strcpy(settings->domain, argv->buffer); + pthread_cond_broadcast(&(rdp_client->rdp_cond)); + break; + + } + + free(argv); + return 0; + +} + +int guac_rdp_argv_handler(guac_user* user, guac_stream* stream, + char* mimetype, char* name) { + + guac_rdp_argv_setting setting; + + /* Allow users to update authentication details */ + if (strcmp(name, "username") == 0) + setting = GUAC_RDP_ARGV_SETTING_USERNAME; + else if (strcmp(name, "password") == 0) + setting = GUAC_RDP_ARGV_SETTING_PASSWORD; + else if (strcmp(name, "domain") == 0) + setting = GUAC_RDP_ARGV_SETTING_DOMAIN; + + /* No other connection parameters may be updated */ + else { + guac_protocol_send_ack(user->socket, stream, "Not allowed.", + GUAC_PROTOCOL_STATUS_CLIENT_FORBIDDEN); + guac_socket_flush(user->socket); + return 0; + } + + guac_rdp_argv* argv = malloc(sizeof(guac_rdp_argv)); + argv->setting = setting; + argv->length = 0; + + /* Prepare stream to receive argument value */ + stream->blob_handler = guac_rdp_argv_blob_handler; + stream->end_handler = guac_rdp_argv_end_handler; + stream->data = argv; + + /* Signal stream is ready */ + guac_protocol_send_ack(user->socket, stream, "Ready for updated " + "parameter.", GUAC_PROTOCOL_STATUS_SUCCESS); + guac_socket_flush(user->socket); + return 0; + +} + diff --git a/src/protocols/rdp/argv.h b/src/protocols/rdp/argv.h new file mode 100644 index 00000000..b265517b --- /dev/null +++ b/src/protocols/rdp/argv.h @@ -0,0 +1,41 @@ +/* + * 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. + */ + + +#ifndef GUAC_RDP_ARGV_H +#define GUAC_RDP_ARGV_H + +#include "config.h" + +#include + +/** + * The maximum number of bytes to allow for any argument value received via an + * argv stream, including null terminator. + */ +#define GUAC_RDP_ARGV_MAX_LENGTH 16384 + +/** + * Handles an incoming stream from a Guacamole "argv" instruction, updating the + * given connection parameter if that parameter is allowed to be updated. + */ +guac_user_argv_handler guac_rdp_argv_handler; + +#endif + diff --git a/src/protocols/rdp/client.c b/src/protocols/rdp/client.c index 07c90a33..14b4c7f2 100644 --- a/src/protocols/rdp/client.c +++ b/src/protocols/rdp/client.c @@ -158,6 +158,10 @@ int guac_client_init(guac_client* client, int argc, char** argv) { /* Initalize the lock */ pthread_rwlock_init(&(rdp_client->lock), NULL); + /* Init RDP credential lock and condition */ + pthread_mutex_init(&(rdp_client->rdp_credential_lock), &(rdp_client->attributes)); + pthread_cond_init(&(rdp_client->rdp_credential_cond), NULL);; + /* Set handlers */ client->join_handler = guac_rdp_user_join_handler; client->free_handler = guac_rdp_client_free_handler; @@ -218,6 +222,9 @@ int guac_rdp_client_free_handler(guac_client* client) { /* Clean up audio input buffer, if allocated */ if (rdp_client->audio_input != NULL) guac_rdp_audio_buffer_free(rdp_client->audio_input); + + /* Destroy the pthread conditional handler */ + pthread_cond_destroy(&(rdp_client->rdp_cond)); pthread_rwlock_destroy(&(rdp_client->lock)); diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 3d7702c6..8b3bdebe 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -66,6 +66,7 @@ #include #include #include +#include #include #include #include @@ -227,10 +228,34 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, rdpContext* context = instance->context; guac_client* client = ((rdp_freerdp_context*) context)->client; - - /* Warn if connection is likely to fail due to lack of credentials */ - guac_client_log(client, GUAC_LOG_INFO, - "Authentication requested but username or password not given"); + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + guac_rdp_settings* settings = rdp_client->settings; + + pthread_mutex_lock(&(rdp_client->rdp_lock)); + + while (settings->username == NULL || strcmp(settings->username, "") == 0) { + guac_protocol_send_required(client->socket, "username"); + guac_socket_flush(client->socket); + pthread_cond_wait(&(rdp_client->rdp_cond), &(rdp_client->rdp_lock)); + *username = settings->username; + } + + while (settings->password == NULL || strcmp(settings->password, "") == 0) { + guac_protocol_send_required(client->socket, "password"); + guac_socket_flush(client->socket); + pthread_cond_wait(&(rdp_client->rdp_cond), &(rdp_client->rdp_lock)); + *password = settings->password; + } + + while (settings->domain == NULL || strcmp(settings->domain, "") == 0) { + guac_protocol_send_required(client->socket, "domain"); + guac_socket_flush(client->socket); + pthread_cond_wait(&(rdp_client->rdp_cond), &(rdp_client->rdp_lock)); + *domain = settings->domain; + } + + pthread_mutex_unlock(&(rdp_client->rdp_lock)); + return TRUE; } diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index e65f702c..68c4cb00 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -153,6 +153,18 @@ typedef struct guac_rdp_client { */ guac_common_list* available_svc; + /** + * Lock which is locked and unlocked for each credential required + * during the connection process. + */ + pthread_mutex_t rdp_credential_lock; + + /** + * Condition which is used when the pthread needs to wait for a + * particular credential to be provided. + */ + pthread_cond_t rdp_credential_cond; + /** * Common attributes for locks. */ diff --git a/src/protocols/rdp/user.c b/src/protocols/rdp/user.c index ef401965..f5eed822 100644 --- a/src/protocols/rdp/user.c +++ b/src/protocols/rdp/user.c @@ -116,6 +116,9 @@ int guac_rdp_user_join_handler(guac_user* user, int argc, char** argv) { /* Inbound arbitrary named pipes */ user->pipe_handler = guac_rdp_pipe_svc_pipe_handler; + + /* Handler for updating parameters during connection. */ + user->argv_handler = guac_rdp_argv_handler; } diff --git a/src/protocols/ssh/argv.c b/src/protocols/ssh/argv.c index 9b3cf653..ff32afde 100644 --- a/src/protocols/ssh/argv.c +++ b/src/protocols/ssh/argv.c @@ -36,6 +36,7 @@ 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 color scheme */ if (strcmp(name, GUAC_SSH_ARGV_COLOR_SCHEME) == 0) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 03f5b0b6..7ee18c4b 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -57,6 +57,32 @@ #include #include +/** + * A function used to generate a prompt to gather additional credentials from + * the guac_client during a connection, and using the specified parameter to + * generate the prompt to the client. + * + * @param client + * The guac_client object associated with the current connection + * where additional credentials are required. + * + * @param cred_name + * The name of the parameter to prompt for in the client. + */ +static void guac_ssh_get_credential(guac_client *client, char* cred_name) { + + guac_ssh_client* ssh_client = (guac_ssh_client*) client->data; + + pthread_mutex_lock(&(ssh_client->term_channel_lock)); + + guac_protocol_send_required(client->socket, cred_name); + guac_socket_flush(client->socket); + + pthread_cond_wait(&(ssh_client->ssh_cond), &(ssh_client->term_channel_lock)); + pthread_mutex_unlock(&(ssh_client->term_channel_lock)); + +} + /** * Produces a new user object containing a username and password or private * key, prompting the user as necessary to obtain that information. @@ -77,9 +103,8 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { guac_common_ssh_user* user; /* Get username */ - if (settings->username == NULL) - settings->username = guac_terminal_prompt(ssh_client->term, - "Login as: ", true); + while (settings->username == NULL) + guac_ssh_get_credential(client, "username"); /* Create user object from username */ user = guac_common_ssh_create_user(settings->username); @@ -103,10 +128,8 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { "Re-attempting private key import (WITH passphrase)"); /* Prompt for passphrase if missing */ - if (settings->key_passphrase == NULL) - settings->key_passphrase = - guac_terminal_prompt(ssh_client->term, - "Key passphrase: ", false); + while (settings->key_passphrase == NULL) + guac_ssh_get_credential(client, "passphrase"); /* Reattempt import with passphrase */ if (guac_common_ssh_user_import_key(user, @@ -144,29 +167,6 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { } -/** - * A function used to generate a terminal prompt to gather additional - * credentials from the guac_client during a connection, and using - * the specified string to generate the prompt for the user. - * - * @param client - * The guac_client object associated with the current connection - * where additional credentials are required. - * - * @param cred_name - * The prompt text to display to the screen when prompting for the - * additional credentials. - * - * @return - * The string of credentials gathered from the user. - */ -static char* guac_ssh_get_credential(guac_client *client, char* cred_name) { - - guac_ssh_client* ssh_client = (guac_ssh_client*) client->data; - return guac_terminal_prompt(ssh_client->term, cred_name, false); - -} - void* ssh_input_thread(void* data) { guac_client* client = (guac_client*) data; @@ -281,6 +281,7 @@ void* ssh_client_thread(void* data) { } pthread_mutex_init(&ssh_client->term_channel_lock, NULL); + pthread_cond_init(&(ssh_client->ssh_cond), NULL); /* Open channel for terminal */ ssh_client->term_channel = @@ -495,6 +496,7 @@ void* ssh_client_thread(void* data) { guac_client_stop(client); pthread_join(input_thread, NULL); + pthread_cond_destroy(&(ssh_client->ssh_cond)); pthread_mutex_destroy(&ssh_client->term_channel_lock); guac_client_log(client, GUAC_LOG_INFO, "SSH connection ended."); diff --git a/src/protocols/ssh/ssh.h b/src/protocols/ssh/ssh.h index cb5d8bc5..47189d24 100644 --- a/src/protocols/ssh/ssh.h +++ b/src/protocols/ssh/ssh.h @@ -89,6 +89,12 @@ typedef struct guac_ssh_client { * Lock dictating access to the SSH terminal channel. */ pthread_mutex_t term_channel_lock; + + /** + * Condition used when SSH client thread needs to wait for Guacamole + * client response. + */ + pthread_cond_t ssh_cond; /** * The current clipboard contents. diff --git a/src/protocols/vnc/Makefile.am b/src/protocols/vnc/Makefile.am index fcba33f1..419e643e 100644 --- a/src/protocols/vnc/Makefile.am +++ b/src/protocols/vnc/Makefile.am @@ -29,6 +29,7 @@ ACLOCAL_AMFLAGS = -I m4 lib_LTLIBRARIES = libguac-client-vnc.la libguac_client_vnc_la_SOURCES = \ + argv.c \ auth.c \ client.c \ clipboard.c \ @@ -41,6 +42,7 @@ libguac_client_vnc_la_SOURCES = \ vnc.c noinst_HEADERS = \ + argv.h \ auth.h \ client.h \ clipboard.h \ diff --git a/src/protocols/vnc/argv.c b/src/protocols/vnc/argv.c new file mode 100644 index 00000000..3b147c7f --- /dev/null +++ b/src/protocols/vnc/argv.c @@ -0,0 +1,163 @@ +/* + * 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. + */ + +#include "config.h" +#include "argv.h" +#include "vnc.h" + +#include +#include +#include + +#include +#include +#include + +/** + * All VNC connection settings which may be updated by unprivileged users + * through "argv" streams. + */ +typedef enum guac_vnc_argv_setting { + + /** + * The password for the connection. + */ + GUAC_VNC_ARGV_SETTING_PASSWORD + +} guac_vnc_argv_setting; + +/** + * The value or current status of a connection parameter received over an + * "argv" stream. + */ +typedef struct guac_vnc_argv { + + /** + * The specific setting being updated. + */ + guac_vnc_argv_setting setting; + + /** + * Buffer space for containing the received argument value. + */ + char buffer[GUAC_VNC_ARGV_MAX_LENGTH]; + + /** + * The number of bytes received so far. + */ + int length; + +} guac_vnc_argv; + +/** + * Handler for "blob" instructions which appends the data from received blobs + * to the end of the in-progress argument value buffer. + * + * @see guac_user_blob_handler + */ +static int guac_vnc_argv_blob_handler(guac_user* user, guac_stream* stream, + void* data, int length) { + + guac_vnc_argv* argv = (guac_vnc_argv*) stream->data; + + /* Calculate buffer size remaining, including space for null terminator, + * adjusting received length accordingly */ + int remaining = sizeof(argv->buffer) - argv->length - 1; + if (length > remaining) + length = remaining; + + /* Append received data to end of buffer */ + memcpy(argv->buffer + argv->length, data, length); + argv->length += length; + + return 0; + +} + +/** + * Handler for "end" instructions which applies the changes specified by the + * argument value buffer associated with the stream. + * + * @see guac_user_end_handler + */ +static int guac_vnc_argv_end_handler(guac_user* user, guac_stream* stream) { + + guac_client* client = user->client; + guac_vnc_client* vnc_client = (guac_vnc_client*) client->data; + guac_vnc_settings* settings = vnc_client->settings; + + /* Append null terminator to value */ + guac_vnc_argv* argv = (guac_vnc_argv*) stream->data; + argv->buffer[argv->length] = '\0'; + + /* Apply changes to chosen setting */ + switch (argv->setting) { + + /* Update password */ + case GUAC_VNC_ARGV_SETTING_PASSWORD: + + /* Update password in settings */ + if (settings->password != NULL) + free(settings->password); + settings->password = malloc(strlen(argv->buffer) * sizeof(char)); + strcpy(settings->password, argv->buffer); + + pthread_cond_broadcast(&(vnc_client->argv_cond)); + break; + + } + + free(argv); + return 0; + +} + +int guac_vnc_argv_handler(guac_user* user, guac_stream* stream, char* mimetype, + char* name) { + + guac_vnc_argv_setting setting; + + /* Allow users to update authentication information */ + if (strcmp(name, "password") == 0) + setting = GUAC_VNC_ARGV_SETTING_PASSWORD; + + /* No other connection parameters may be updated */ + else { + guac_protocol_send_ack(user->socket, stream, "Not allowed.", + GUAC_PROTOCOL_STATUS_CLIENT_FORBIDDEN); + guac_socket_flush(user->socket); + return 0; + } + + guac_vnc_argv* argv = malloc(sizeof(guac_vnc_argv)); + argv->setting = setting; + argv->length = 0; + + /* Prepare stream to receive argument value */ + stream->blob_handler = guac_vnc_argv_blob_handler; + stream->end_handler = guac_vnc_argv_end_handler; + stream->data = argv; + + /* Signal stream is ready */ + guac_protocol_send_ack(user->socket, stream, "Ready for updated " + "parameter.", GUAC_PROTOCOL_STATUS_SUCCESS); + guac_socket_flush(user->socket); + return 0; + +} diff --git a/src/protocols/vnc/argv.h b/src/protocols/vnc/argv.h new file mode 100644 index 00000000..599eaaa7 --- /dev/null +++ b/src/protocols/vnc/argv.h @@ -0,0 +1,41 @@ +/* + * 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. + */ + +#ifndef ARGV_H +#define ARGV_H + +#include "config.h" + +#include + + +/** + * The maximum number of bytes to allow for any argument value received via an + * argv stream, including null terminator. + */ +#define GUAC_VNC_ARGV_MAX_LENGTH 16384 + +/** + * Handles an incoming stream from a Guacamole "argv" instruction, updating the + * given connection parameter if that parameter is allowed to be updated. + */ +guac_user_argv_handler guac_vnc_argv_handler; + +#endif /* ARGV_H */ + diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index 8cc6b1d7..54d2f2e7 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -23,12 +23,29 @@ #include "vnc.h" #include +#include +#include #include #include +#include +#include + char* guac_vnc_get_password(rfbClient* client) { guac_client* gc = rfbClientGetClientData(client, GUAC_VNC_CLIENT_KEY); - return ((guac_vnc_client*) gc->data)->settings->password; + guac_vnc_client* vnc_client = ((guac_vnc_client*) gc->data); + guac_vnc_settings* settings = vnc_client->settings; + + if (settings->password == NULL || strcmp(settings->password, "") == 0) { + pthread_mutex_lock(&(vnc_client->argv_lock)); + guac_protocol_send_required(gc->socket, "password"); + guac_socket_flush(gc->socket); + pthread_cond_wait(&(vnc_client->argv_cond), &(vnc_client->argv_lock)); + pthread_mutex_unlock(&(vnc_client->argv_lock)); + } + + return settings->password; + } rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { diff --git a/src/protocols/vnc/client.c b/src/protocols/vnc/client.c index cee1d4d5..691c490c 100644 --- a/src/protocols/vnc/client.c +++ b/src/protocols/vnc/client.c @@ -50,9 +50,13 @@ int guac_client_init(guac_client* client) { client->data = vnc_client; #ifdef ENABLE_VNC_TLS_LOCKING - /* Initialize the write lock */ + /* Initialize the TLS write lock */ pthread_mutex_init(&(vnc_client->tls_lock), NULL); #endif + + /* Initialize argv lock and condition */ + pthread_mutex_init(&(vnc_client->argv_lock), NULL); + pthread_cond_init(&(vnc_client->argv_cond), NULL); /* Init clipboard */ vnc_client->clipboard = guac_common_clipboard_alloc(GUAC_VNC_CLIPBOARD_MAX_LENGTH); @@ -135,6 +139,10 @@ int guac_vnc_client_free_handler(guac_client* client) { /* Clean up TLS lock mutex. */ pthread_mutex_destroy(&(vnc_client->tls_lock)); #endif + + /* Clean up argv mutex */ + pthread_cond_destroy(&(vnc_client->argv_cond)); + pthread_mutex_destroy(&(vnc_client->argv_lock)); /* Free generic data struct */ free(client->data); diff --git a/src/protocols/vnc/vnc.h b/src/protocols/vnc/vnc.h index 7c189cfb..e36ac66d 100644 --- a/src/protocols/vnc/vnc.h +++ b/src/protocols/vnc/vnc.h @@ -61,6 +61,16 @@ typedef struct guac_vnc_client { */ pthread_mutex_t tls_lock; #endif + + /** + * The lock for tracking changes via argv. + */ + pthread_mutex_t argv_lock; + + /** + * The condition for signaling argv updates. + */ + pthread_cond_t argv_cond; /** * The underlying VNC client. From 7369bed22c49ad54fd44dd300efd4a8ec5d7a9d3 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 9 Aug 2019 21:09:11 -0400 Subject: [PATCH 02/26] GUACAMOLE-221: Add support for sending multiple params in required. --- src/libguac/guacamole/protocol.h | 4 +- src/libguac/protocol.c | 28 +++++++++++--- src/protocols/rdp/argv.c | 13 ++++--- src/protocols/rdp/client.c | 1 + src/protocols/rdp/rdp.c | 53 ++++++++++++++++++--------- src/protocols/rdp/rdp.h | 21 +++++++++++ src/protocols/ssh/ssh.c | 2 +- src/protocols/vnc/argv.c | 29 ++++++++++++++- src/protocols/vnc/auth.c | 63 +++++++++++++++++++++++++++++--- src/protocols/vnc/client.c | 1 + src/protocols/vnc/vnc.h | 15 ++++++++ 11 files changed, 191 insertions(+), 39 deletions(-) diff --git a/src/libguac/guacamole/protocol.h b/src/libguac/guacamole/protocol.h index 54f7b072..54e8d6f6 100644 --- a/src/libguac/guacamole/protocol.h +++ b/src/libguac/guacamole/protocol.h @@ -803,12 +803,12 @@ int guac_protocol_send_rect(guac_socket* socket, const guac_layer* layer, * The guac_socket connection to use. * * @param required - * The name of the parameter that is required. + * A NULL-terminated array of required parameters. * * @return * Zero on success, non-zero on error. */ -int guac_protocol_send_required(guac_socket* socket, const char* required); +int guac_protocol_send_required(guac_socket* socket, const char** required); /** * Sends a reset instruction over the given guac_socket connection. diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index 972ac547..b592ebc3 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -961,17 +961,33 @@ int guac_protocol_send_rect(guac_socket* socket, } -int guac_protocol_send_required(guac_socket* socket, const char* required) { +static int __guac_protocol_send_required(guac_socket* socket, + const char** required) { + + if (guac_socket_write_string(socket, "8.required")) return -1; + + for (int i=0; required[i] != NULL; i++) { + + if (guac_socket_write_string(socket, ",")) + return -1; + + if (__guac_socket_write_length_string(socket, required[i])) + return -1; + + } + + return guac_socket_write_string(socket, ";"); + +} + +int guac_protocol_send_required(guac_socket* socket, const char** required) { int ret_val; guac_socket_instruction_begin(socket); - ret_val = - guac_socket_write_string(socket, "8.required,") - || __guac_socket_write_length_string(socket, required) - || guac_socket_write_string(socket, ";"); - + ret_val = __guac_protocol_send_required(socket, required); guac_socket_instruction_end(socket); + return ret_val; } diff --git a/src/protocols/rdp/argv.c b/src/protocols/rdp/argv.c index a79a64c4..96db4b9c 100644 --- a/src/protocols/rdp/argv.c +++ b/src/protocols/rdp/argv.c @@ -48,8 +48,8 @@ typedef enum guac_rdp_argv_setting { /** * The domain to use for connection authentication. */ - GUAC_RDP_ARGV_SETTING_DOMAIN - + GUAC_RDP_ARGV_SETTING_DOMAIN, + } guac_rdp_argv_setting; /** @@ -125,24 +125,27 @@ static int guac_rdp_argv_end_handler(guac_user* user, free(settings->username); settings->username = malloc(strlen(argv->buffer) * sizeof(char)); strcpy(settings->username, argv->buffer); - pthread_cond_broadcast(&(rdp_client->rdp_cond)); + rdp_client->rdp_cond_flags ^= GUAC_RDP_COND_FLAG_USERNAME; break; case GUAC_RDP_ARGV_SETTING_PASSWORD: free(settings->password); settings->password = malloc(strlen(argv->buffer) * sizeof(char)); strcpy(settings->password, argv->buffer); - pthread_cond_broadcast(&(rdp_client->rdp_cond)); + rdp_client->rdp_cond_flags ^= GUAC_RDP_COND_FLAG_PASSWORD; break; case GUAC_RDP_ARGV_SETTING_DOMAIN: free(settings->domain); settings->domain = malloc(strlen(argv->buffer) * sizeof(char)); strcpy(settings->domain, argv->buffer); - pthread_cond_broadcast(&(rdp_client->rdp_cond)); + rdp_client->rdp_cond_flags ^= GUAC_RDP_COND_FLAG_DOMAIN; break; } + + if (!rdp_client->rdp_cond_flags) + pthread_cond_signal(&(rdp_client->rdp_cond)); free(argv); return 0; diff --git a/src/protocols/rdp/client.c b/src/protocols/rdp/client.c index 14b4c7f2..f232e6a8 100644 --- a/src/protocols/rdp/client.c +++ b/src/protocols/rdp/client.c @@ -161,6 +161,7 @@ int guac_client_init(guac_client* client, int argc, char** argv) { /* Init RDP credential lock and condition */ pthread_mutex_init(&(rdp_client->rdp_credential_lock), &(rdp_client->attributes)); pthread_cond_init(&(rdp_client->rdp_credential_cond), NULL);; + rdp_client->rdp_credential_flags = 0; /* Set handlers */ client->join_handler = guac_rdp_user_join_handler; diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 8b3bdebe..4441d66c 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -230,32 +230,51 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, guac_client* client = ((rdp_freerdp_context*) context)->client; guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; guac_rdp_settings* settings = rdp_client->settings; + char* params[4] = {}; + int i = 0; - pthread_mutex_lock(&(rdp_client->rdp_lock)); + if (settings->username == NULL || strcmp(settings->username, "") == 0) { + params[i] = "username"; + rdp_client->rdp_cond_flags |= GUAC_RDP_COND_FLAG_USERNAME; + i++; + } - while (settings->username == NULL || strcmp(settings->username, "") == 0) { - guac_protocol_send_required(client->socket, "username"); + if (settings->password == NULL || strcmp(settings->password, "") == 0) { + params[i] = "password"; + rdp_client->rdp_cond_flags |= GUAC_RDP_COND_FLAG_PASSWORD; + i++; + } + + if (settings->domain == NULL || strcmp(settings->domain, "") == 0) { + params[i] = "domain"; + rdp_client->rdp_cond_flags |= GUAC_RDP_COND_FLAG_DOMAIN; + i++; + } + + /* NULL-terminate the array. */ + params[i] = NULL; + + if (i > 0) { + /* Lock the client thread. */ + pthread_mutex_lock(&(rdp_client->rdp_lock)); + + /* Send require params and flush socket. */ + guac_protocol_send_required(client->socket, (const char**) params); guac_socket_flush(client->socket); + + /* Wait for condition. */ pthread_cond_wait(&(rdp_client->rdp_cond), &(rdp_client->rdp_lock)); + + /* Get new values from settings. */ *username = settings->username; - } - - while (settings->password == NULL || strcmp(settings->password, "") == 0) { - guac_protocol_send_required(client->socket, "password"); - guac_socket_flush(client->socket); - pthread_cond_wait(&(rdp_client->rdp_cond), &(rdp_client->rdp_lock)); *password = settings->password; - } - - while (settings->domain == NULL || strcmp(settings->domain, "") == 0) { - guac_protocol_send_required(client->socket, "domain"); - guac_socket_flush(client->socket); - pthread_cond_wait(&(rdp_client->rdp_cond), &(rdp_client->rdp_lock)); *domain = settings->domain; + + /* Unlock the thread. */ + pthread_mutex_unlock(&(rdp_client->rdp_lock)); } - pthread_mutex_unlock(&(rdp_client->rdp_lock)); - + /* Always return TRUE allowing connection to retry. */ return TRUE; } diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index 68c4cb00..418c908a 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -49,6 +49,21 @@ #include #include +/** + * A flag for tracking if we are waiting conditionally on a username. + */ +#define GUAC_RDP_COND_FLAG_USERNAME 1 + +/** + * A flag for tracking if we are waiting conditionally on a password. + */ +#define GUAC_RDP_COND_FLAG_PASSWORD 2 + +/** + * A flag for tracking if we are waiting conditionally on a domain. + */ +#define GUAC_RDP_COND_FLAG_DOMAIN 3 + /** * RDP-specific client data. */ @@ -164,6 +179,12 @@ typedef struct guac_rdp_client { * particular credential to be provided. */ pthread_cond_t rdp_credential_cond; + + /** + * Flags for tracking events related to the rdp_credential_cond + * pthread condition. + */ + unsigned rdp_credential_flags; /** * Common attributes for locks. diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 7ee18c4b..f3acedb3 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -75,7 +75,7 @@ static void guac_ssh_get_credential(guac_client *client, char* cred_name) { pthread_mutex_lock(&(ssh_client->term_channel_lock)); - guac_protocol_send_required(client->socket, cred_name); + guac_protocol_send_required(client->socket, (const char* []) {cred_name, NULL}); guac_socket_flush(client->socket); pthread_cond_wait(&(ssh_client->ssh_cond), &(ssh_client->term_channel_lock)); diff --git a/src/protocols/vnc/argv.c b/src/protocols/vnc/argv.c index 3b147c7f..24ac33ed 100644 --- a/src/protocols/vnc/argv.c +++ b/src/protocols/vnc/argv.c @@ -35,6 +35,11 @@ */ typedef enum guac_vnc_argv_setting { + /** + * The username for the connection. + */ + GUAC_VNC_ARGV_SETTING_USERNAME, + /** * The password for the connection. */ @@ -109,6 +114,19 @@ static int guac_vnc_argv_end_handler(guac_user* user, guac_stream* stream) { /* Apply changes to chosen setting */ switch (argv->setting) { + /* Update username */ + case GUAC_VNC_ARGV_SETTING_USERNAME: + + /* Update username in settings. */ + if (settings->username != NULL) + free(settings->username); + settings->username = malloc(strlen(argv->buffer) * sizeof(char)); + strcpy(settings->username, argv->buffer); + + /* Remove the username conditional flag. */ + vnc_client->argv_cond_flags ^= GUAC_VNC_COND_FLAG_USERNAME; + break; + /* Update password */ case GUAC_VNC_ARGV_SETTING_PASSWORD: @@ -118,10 +136,15 @@ static int guac_vnc_argv_end_handler(guac_user* user, guac_stream* stream) { settings->password = malloc(strlen(argv->buffer) * sizeof(char)); strcpy(settings->password, argv->buffer); - pthread_cond_broadcast(&(vnc_client->argv_cond)); + /* Remove the password conditional flag. */ + vnc_client->argv_cond_flags ^= GUAC_VNC_COND_FLAG_PASSWORD; break; } + + /* If no flags are set, signal the conditional. */ + if (!vnc_client->argv_cond_flags) + pthread_cond_broadcast(&(vnc_client->argv_cond)); free(argv); return 0; @@ -134,7 +157,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, "password") == 0) + if (strcmp(name, "username") == 0) + setting = GUAC_VNC_ARGV_SETTING_USERNAME; + else if (strcmp(name, "password") == 0) setting = GUAC_VNC_ARGV_SETTING_PASSWORD; /* No other connection parameters may be updated */ diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index 54d2f2e7..c0ae2b98 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -36,11 +36,23 @@ char* guac_vnc_get_password(rfbClient* client) { guac_vnc_client* vnc_client = ((guac_vnc_client*) gc->data); guac_vnc_settings* settings = vnc_client->settings; + /* If password isn't around, prompt for it. */ if (settings->password == NULL || strcmp(settings->password, "") == 0) { + /* Lock the thread. */ pthread_mutex_lock(&(vnc_client->argv_lock)); - guac_protocol_send_required(gc->socket, "password"); + + /* Send the request for password and flush the socket. */ + guac_protocol_send_required(gc->socket, + (const char* []) {"password", NULL}); guac_socket_flush(gc->socket); + + /* Set the conditional flag. */ + vnc_client->argv_cond_flags |= GUAC_VNC_COND_FLAG_PASSWORD; + + /* Wait for the condition. */ pthread_cond_wait(&(vnc_client->argv_cond), &(vnc_client->argv_lock)); + + /* Unlock the thread. */ pthread_mutex_unlock(&(vnc_client->argv_lock)); } @@ -50,13 +62,52 @@ char* guac_vnc_get_password(rfbClient* client) { rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { guac_client* gc = rfbClientGetClientData(client, GUAC_VNC_CLIENT_KEY); - guac_vnc_settings* settings = ((guac_vnc_client*) gc->data)->settings; - + guac_vnc_client* vnc_client = ((guac_vnc_client*) gc->data); + guac_vnc_settings* settings = vnc_client->settings; + + /* Handle request for Username/Password credentials */ if (credentialType == rfbCredentialTypeUser) { rfbCredential *creds = malloc(sizeof(rfbCredential)); - creds->userCredential.username = settings->username; - creds->userCredential.password = settings->password; - return creds; + char* params[2] = {NULL}; + int i = 0; + + /* Check if username is null or empty. */ + if (settings->username == NULL || strcmp(settings->username, "") == 0) { + params[i] = "username"; + i++; + vnc_client->argv_cond_flags |= GUAC_VNC_COND_FLAG_USERNAME; + } + + /* Check if password is null or empty. */ + if (settings->password == NULL || strcmp(settings->password, "") == 0) { + params[i] = "password"; + i++; + vnc_client->argv_cond_flags |= GUAC_VNC_COND_FLAG_PASSWORD; + } + + /* If we have empty parameters, request them. */ + if (i > 0) { + /* Lock the thread. */ + pthread_mutex_lock(&(vnc_client->argv_lock)); + + /* Send required parameters to client and flush the socket. */ + guac_protocol_send_required(gc->socket, (const char**) params); + guac_socket_flush(gc->socket); + + /* Wait for the parameters to be returned. */ + pthread_cond_wait(&(vnc_client->argv_cond), &(vnc_client->argv_lock)); + + /* Pull the credentials from updated settings. */ + creds->userCredential.username = settings->username; + creds->userCredential.password = settings->password; + + /* Unlock the thread. */ + pthread_mutex_unlock(&(vnc_client->argv_lock)); + + return creds; + + } + } guac_client_abort(gc, GUAC_PROTOCOL_STATUS_SERVER_ERROR, diff --git a/src/protocols/vnc/client.c b/src/protocols/vnc/client.c index 691c490c..e52dfc55 100644 --- a/src/protocols/vnc/client.c +++ b/src/protocols/vnc/client.c @@ -57,6 +57,7 @@ int guac_client_init(guac_client* client) { /* Initialize argv lock and condition */ pthread_mutex_init(&(vnc_client->argv_lock), NULL); pthread_cond_init(&(vnc_client->argv_cond), NULL); + vnc_client->argv_cond_flags = 0; /* Init clipboard */ vnc_client->clipboard = guac_common_clipboard_alloc(GUAC_VNC_CLIPBOARD_MAX_LENGTH); diff --git a/src/protocols/vnc/vnc.h b/src/protocols/vnc/vnc.h index e36ac66d..c30205e8 100644 --- a/src/protocols/vnc/vnc.h +++ b/src/protocols/vnc/vnc.h @@ -45,6 +45,16 @@ #include +/** + * A flag for tracking status of requesting username from client. + */ +#define GUAC_VNC_COND_FLAG_USERNAME 1 + +/** + * A flag for tracking status of requesting password from client. + */ +#define GUAC_VNC_COND_FLAG_PASSWORD 2 + /** * VNC-specific client data. */ @@ -71,6 +81,11 @@ typedef struct guac_vnc_client { * The condition for signaling argv updates. */ pthread_cond_t argv_cond; + + /** + * Flags for conditional signaling for argv updates; + */ + unsigned argv_cond_flags; /** * The underlying VNC client. From 76ef6332cce707edc31230e9a9463fdc9cf6bc88 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sun, 26 Jan 2020 03:53:34 -0500 Subject: [PATCH 03/26] GUACAMOLE-221: Make lock, condition, and flags specific to credentials. --- src/protocols/rdp/argv.c | 19 ++++++++----------- src/protocols/rdp/client.c | 2 +- src/protocols/rdp/rdp.c | 12 ++++++------ src/protocols/rdp/rdp.h | 6 +++--- src/protocols/rdp/user.c | 1 + 5 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/protocols/rdp/argv.c b/src/protocols/rdp/argv.c index 96db4b9c..c256f6ea 100644 --- a/src/protocols/rdp/argv.c +++ b/src/protocols/rdp/argv.c @@ -123,29 +123,26 @@ static int guac_rdp_argv_end_handler(guac_user* user, /* Update RDP username. */ case GUAC_RDP_ARGV_SETTING_USERNAME: free(settings->username); - settings->username = malloc(strlen(argv->buffer) * sizeof(char)); - strcpy(settings->username, argv->buffer); - rdp_client->rdp_cond_flags ^= GUAC_RDP_COND_FLAG_USERNAME; + settings->username = strndup(argv->buffer, argv->length); + rdp_client->rdp_credential_flags &= ~GUAC_RDP_CRED_FLAG_USERNAME; break; case GUAC_RDP_ARGV_SETTING_PASSWORD: free(settings->password); - settings->password = malloc(strlen(argv->buffer) * sizeof(char)); - strcpy(settings->password, argv->buffer); - rdp_client->rdp_cond_flags ^= GUAC_RDP_COND_FLAG_PASSWORD; + settings->password = strndup(argv->buffer, argv->length); + rdp_client->rdp_credential_flags &= ~GUAC_RDP_CRED_FLAG_PASSWORD; break; case GUAC_RDP_ARGV_SETTING_DOMAIN: free(settings->domain); - settings->domain = malloc(strlen(argv->buffer) * sizeof(char)); - strcpy(settings->domain, argv->buffer); - rdp_client->rdp_cond_flags ^= GUAC_RDP_COND_FLAG_DOMAIN; + settings->domain = strndup(argv->buffer, argv->length); + rdp_client->rdp_credential_flags &= ~GUAC_RDP_CRED_FLAG_DOMAIN; break; } - if (!rdp_client->rdp_cond_flags) - pthread_cond_signal(&(rdp_client->rdp_cond)); + if (!rdp_client->rdp_credential_flags) + pthread_cond_signal(&(rdp_client->rdp_credential_cond)); free(argv); return 0; diff --git a/src/protocols/rdp/client.c b/src/protocols/rdp/client.c index f232e6a8..94998ddb 100644 --- a/src/protocols/rdp/client.c +++ b/src/protocols/rdp/client.c @@ -225,7 +225,7 @@ int guac_rdp_client_free_handler(guac_client* client) { guac_rdp_audio_buffer_free(rdp_client->audio_input); /* Destroy the pthread conditional handler */ - pthread_cond_destroy(&(rdp_client->rdp_cond)); + pthread_cond_destroy(&(rdp_client->rdp_credential_cond)); pthread_rwlock_destroy(&(rdp_client->lock)); diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 4441d66c..fee73d3b 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -235,19 +235,19 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, if (settings->username == NULL || strcmp(settings->username, "") == 0) { params[i] = "username"; - rdp_client->rdp_cond_flags |= GUAC_RDP_COND_FLAG_USERNAME; + rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_USERNAME; i++; } if (settings->password == NULL || strcmp(settings->password, "") == 0) { params[i] = "password"; - rdp_client->rdp_cond_flags |= GUAC_RDP_COND_FLAG_PASSWORD; + rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_PASSWORD; i++; } if (settings->domain == NULL || strcmp(settings->domain, "") == 0) { params[i] = "domain"; - rdp_client->rdp_cond_flags |= GUAC_RDP_COND_FLAG_DOMAIN; + rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_DOMAIN; i++; } @@ -256,14 +256,14 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, if (i > 0) { /* Lock the client thread. */ - pthread_mutex_lock(&(rdp_client->rdp_lock)); + pthread_mutex_lock(&(rdp_client->rdp_credential_lock)); /* Send require params and flush socket. */ guac_protocol_send_required(client->socket, (const char**) params); guac_socket_flush(client->socket); /* Wait for condition. */ - pthread_cond_wait(&(rdp_client->rdp_cond), &(rdp_client->rdp_lock)); + pthread_cond_wait(&(rdp_client->rdp_credential_cond), &(rdp_client->rdp_credential_lock)); /* Get new values from settings. */ *username = settings->username; @@ -271,7 +271,7 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, *domain = settings->domain; /* Unlock the thread. */ - pthread_mutex_unlock(&(rdp_client->rdp_lock)); + pthread_mutex_unlock(&(rdp_client->rdp_credential_lock)); } /* Always return TRUE allowing connection to retry. */ diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index 418c908a..361f2da7 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -52,17 +52,17 @@ /** * A flag for tracking if we are waiting conditionally on a username. */ -#define GUAC_RDP_COND_FLAG_USERNAME 1 +#define GUAC_RDP_CRED_FLAG_USERNAME 1 /** * A flag for tracking if we are waiting conditionally on a password. */ -#define GUAC_RDP_COND_FLAG_PASSWORD 2 +#define GUAC_RDP_CRED_FLAG_PASSWORD 2 /** * A flag for tracking if we are waiting conditionally on a domain. */ -#define GUAC_RDP_COND_FLAG_DOMAIN 3 +#define GUAC_RDP_CRED_FLAG_DOMAIN 4 /** * RDP-specific client data. diff --git a/src/protocols/rdp/user.c b/src/protocols/rdp/user.c index f5eed822..8be53d3e 100644 --- a/src/protocols/rdp/user.c +++ b/src/protocols/rdp/user.c @@ -17,6 +17,7 @@ * under the License. */ +#include "argv.h" #include "channels/audio-input/audio-input.h" #include "channels/cliprdr.h" #include "channels/pipe-svc.h" From 939d9548105220dc7095b8d88a6ef78025f6b97a Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sun, 26 Jan 2020 04:33:04 -0500 Subject: [PATCH 04/26] GUACAMOLE-221: Extract array writing in protocol into common function and document. --- src/libguac/protocol.c | 84 +++++++++++++++++++++++++--------------- src/protocols/rdp/argv.c | 2 +- 2 files changed, 54 insertions(+), 32 deletions(-) diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index b592ebc3..6379ad37 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -66,6 +66,37 @@ ssize_t __guac_socket_write_length_double(guac_socket* socket, double d) { } +/** + * Loop through the provided NULL-terminated array, writing the values + * and lengths of the values in the array to the given socket. Return + * zero on success, non-zero on error. + * + * @param socket + * The socket to which the data should be written. + * + * @param array + * The NULL-terminated array of values to write. + * + * @return + * Zero on success, non-zero on error. + */ +static int __guac_socket_write_array(guac_socket* socket, const char** array) { + + /* Loop through array, writing provided values to the socket. */ + for (int i=0; array[i] != NULL; i++) { + + if (guac_socket_write_string(socket, ",")) + return -1; + + if (__guac_socket_write_length_string(socket, array[i])) + return -1; + + } + + return 0; + +} + /* Protocol functions */ int guac_protocol_send_ack(guac_socket* socket, guac_stream* stream, @@ -90,8 +121,6 @@ int guac_protocol_send_ack(guac_socket* socket, guac_stream* stream, static int __guac_protocol_send_args(guac_socket* socket, const char** args) { - int i; - if (guac_socket_write_string(socket, "4.args")) return -1; /* Send protocol version ahead of other args. */ @@ -99,15 +128,8 @@ static int __guac_protocol_send_args(guac_socket* socket, const char** args) { || __guac_socket_write_length_string(socket, GUACAMOLE_PROTOCOL_VERSION)) return -1; - for (i=0; args[i] != NULL; i++) { - - if (guac_socket_write_string(socket, ",")) - return -1; - - if (__guac_socket_write_length_string(socket, args[i])) - return -1; - - } + if (__guac_socket_write_array(socket, args)) + return -1; return guac_socket_write_string(socket, ";"); @@ -308,19 +330,10 @@ int guac_protocol_send_close(guac_socket* socket, const guac_layer* layer) { static int __guac_protocol_send_connect(guac_socket* socket, const char** args) { - int i; - if (guac_socket_write_string(socket, "7.connect")) return -1; - for (i=0; args[i] != NULL; i++) { - - if (guac_socket_write_string(socket, ",")) - return -1; - - if (__guac_socket_write_length_string(socket, args[i])) - return -1; - - } + if (__guac_socket_write_array(socket, args)) + return -1; return guac_socket_write_string(socket, ";"); @@ -961,20 +974,29 @@ int guac_protocol_send_rect(guac_socket* socket, } +/** + * Sends the "required" instruction on the given socket, looping + * through all the items provided in the NULL-terminated array, + * and closing out the instruction. Returns zero on success, or + * non-zero on error. + * + * @param socket + * The socket on which to write the instruction. + * + * @param required + * The NULL-terminated array of required parameters to send + * to the client. + * + * @return + * Zero if successful, non-zero on error. + */ static int __guac_protocol_send_required(guac_socket* socket, const char** required) { if (guac_socket_write_string(socket, "8.required")) return -1; - for (int i=0; required[i] != NULL; i++) { - - if (guac_socket_write_string(socket, ",")) - return -1; - - if (__guac_socket_write_length_string(socket, required[i])) - return -1; - - } + if (__guac_socket_write_array(socket, required)) + return -1; return guac_socket_write_string(socket, ";"); diff --git a/src/protocols/rdp/argv.c b/src/protocols/rdp/argv.c index c256f6ea..2d75440d 100644 --- a/src/protocols/rdp/argv.c +++ b/src/protocols/rdp/argv.c @@ -123,7 +123,7 @@ static int guac_rdp_argv_end_handler(guac_user* user, /* Update RDP username. */ case GUAC_RDP_ARGV_SETTING_USERNAME: free(settings->username); - settings->username = strndup(argv->buffer, argv->length); + settings->username = strndup(argv->buffer, argv->length); rdp_client->rdp_credential_flags &= ~GUAC_RDP_CRED_FLAG_USERNAME; break; From 43180835118c8cc3dfcf0a441a6c7d6a86854007 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 19 Mar 2020 14:04:15 -0400 Subject: [PATCH 05/26] GUACAMOLE-221: Fix up style, comments, and variable names. --- src/common-ssh/common-ssh/ssh.h | 2 +- src/libguac/protocol.c | 18 ++++++++++-------- src/protocols/rdp/Makefile.am | 4 ++-- src/protocols/rdp/argv.c | 2 +- src/protocols/rdp/rdp.c | 1 - src/protocols/rdp/rdp.h | 9 +++++---- src/protocols/ssh/ssh.c | 15 +++++++++------ src/protocols/ssh/ssh.h | 4 ++-- src/protocols/vnc/Makefile.am | 4 ++-- src/protocols/vnc/argv.c | 20 ++++++++------------ src/protocols/vnc/auth.c | 20 +++++++++++--------- src/protocols/vnc/client.c | 14 +++++++------- src/protocols/vnc/vnc.h | 33 ++++++++++++++++++--------------- 13 files changed, 76 insertions(+), 70 deletions(-) diff --git a/src/common-ssh/common-ssh/ssh.h b/src/common-ssh/common-ssh/ssh.h index 698fac17..609e1a1e 100644 --- a/src/common-ssh/common-ssh/ssh.h +++ b/src/common-ssh/common-ssh/ssh.h @@ -26,7 +26,7 @@ #include /** - * A handler function for retrieving additional credentials for the client. + * A handler function for retrieving additional credentials from the client. * * @param client * The Guacamole Client associated with this need for additional diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index 6379ad37..fa9e477d 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -128,8 +128,8 @@ static int __guac_protocol_send_args(guac_socket* socket, const char** args) { || __guac_socket_write_length_string(socket, GUACAMOLE_PROTOCOL_VERSION)) return -1; - if (__guac_socket_write_array(socket, args)) - return -1; + if (__guac_socket_write_array(socket, args)) + return -1; return guac_socket_write_string(socket, ";"); @@ -330,10 +330,11 @@ int guac_protocol_send_close(guac_socket* socket, const guac_layer* layer) { static int __guac_protocol_send_connect(guac_socket* socket, const char** args) { - if (guac_socket_write_string(socket, "7.connect")) return -1; + if (guac_socket_write_string(socket, "7.connect")) + return -1; - if (__guac_socket_write_array(socket, args)) - return -1; + if (__guac_socket_write_array(socket, args)) + return -1; return guac_socket_write_string(socket, ";"); @@ -993,10 +994,11 @@ int guac_protocol_send_rect(guac_socket* socket, static int __guac_protocol_send_required(guac_socket* socket, const char** required) { - if (guac_socket_write_string(socket, "8.required")) return -1; + if (guac_socket_write_string(socket, "8.required")) + return -1; - if (__guac_socket_write_array(socket, required)) - return -1; + if (__guac_socket_write_array(socket, required)) + return -1; return guac_socket_write_string(socket, ";"); diff --git a/src/protocols/rdp/Makefile.am b/src/protocols/rdp/Makefile.am index fcee5422..e8ddfeb4 100644 --- a/src/protocols/rdp/Makefile.am +++ b/src/protocols/rdp/Makefile.am @@ -38,7 +38,7 @@ nodist_libguac_client_rdp_la_SOURCES = \ _generated_keymaps.c libguac_client_rdp_la_SOURCES = \ - argv.c \ + argv.c \ beep.c \ bitmap.c \ channels/audio-input/audio-buffer.c \ @@ -83,7 +83,7 @@ libguac_client_rdp_la_SOURCES = \ user.c noinst_HEADERS = \ - argv.h \ + argv.h \ beep.h \ bitmap.h \ channels/audio-input/audio-buffer.h \ diff --git a/src/protocols/rdp/argv.c b/src/protocols/rdp/argv.c index 2d75440d..2d34c57d 100644 --- a/src/protocols/rdp/argv.c +++ b/src/protocols/rdp/argv.c @@ -87,7 +87,7 @@ static int guac_rdp_argv_blob_handler(guac_user* user, guac_rdp_argv* argv = (guac_rdp_argv*) stream->data; /* Calculate buffer size remaining, including space for null terminator, - * adjusting received length accordingly */ + adjusting received length accordingly */ int remaining = sizeof(argv->buffer) - argv->length - 1; if (length > remaining) length = remaining; diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index fee73d3b..fb52dff5 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -66,7 +66,6 @@ #include #include #include -#include #include #include #include diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index 361f2da7..1509913f 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -169,14 +169,15 @@ typedef struct guac_rdp_client { guac_common_list* available_svc; /** - * Lock which is locked and unlocked for each credential required - * during the connection process. + * Lock which is locked when one or more credentials are required to + * complete the connection, and unlocked when credentials have been + * provided by the client. */ pthread_mutex_t rdp_credential_lock; /** - * Condition which is used when the pthread needs to wait for a - * particular credential to be provided. + * Condition which is used when the pthread needs to wait for one or more + * credentials to be provided by the client. */ pthread_cond_t rdp_credential_cond; diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index f3acedb3..74e0e8cb 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -58,9 +58,9 @@ #include /** - * A function used to generate a prompt to gather additional credentials from - * the guac_client during a connection, and using the specified parameter to - * generate the prompt to the client. + * This function generates a prompt to the specified instance of guac_client + * for the credential specified in the cred_name parameter, which should + * be a valid SSH connection parameter. * * @param client * The guac_client object associated with the current connection @@ -73,12 +73,15 @@ static void guac_ssh_get_credential(guac_client *client, char* cred_name) { guac_ssh_client* ssh_client = (guac_ssh_client*) client->data; + /* Lock the terminal thread while prompting for the credential. */ pthread_mutex_lock(&(ssh_client->term_channel_lock)); + /* Let the client know what we require and flush the socket. */ guac_protocol_send_required(client->socket, (const char* []) {cred_name, NULL}); guac_socket_flush(client->socket); - pthread_cond_wait(&(ssh_client->ssh_cond), &(ssh_client->term_channel_lock)); + /* Wait for the response, and then unlock the thread. */ + pthread_cond_wait(&(ssh_client->ssh_credential_cond), &(ssh_client->term_channel_lock)); pthread_mutex_unlock(&(ssh_client->term_channel_lock)); } @@ -281,7 +284,7 @@ void* ssh_client_thread(void* data) { } pthread_mutex_init(&ssh_client->term_channel_lock, NULL); - pthread_cond_init(&(ssh_client->ssh_cond), NULL); + pthread_cond_init(&(ssh_client->ssh_credential_cond), NULL); /* Open channel for terminal */ ssh_client->term_channel = @@ -496,7 +499,7 @@ void* ssh_client_thread(void* data) { guac_client_stop(client); pthread_join(input_thread, NULL); - pthread_cond_destroy(&(ssh_client->ssh_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."); diff --git a/src/protocols/ssh/ssh.h b/src/protocols/ssh/ssh.h index 47189d24..bfd2fd1b 100644 --- a/src/protocols/ssh/ssh.h +++ b/src/protocols/ssh/ssh.h @@ -92,9 +92,9 @@ typedef struct guac_ssh_client { /** * Condition used when SSH client thread needs to wait for Guacamole - * client response. + * client to pass additional credentials before continuing the connection. */ - pthread_cond_t ssh_cond; + pthread_cond_t ssh_credential_cond; /** * The current clipboard contents. diff --git a/src/protocols/vnc/Makefile.am b/src/protocols/vnc/Makefile.am index 419e643e..64e8147e 100644 --- a/src/protocols/vnc/Makefile.am +++ b/src/protocols/vnc/Makefile.am @@ -29,7 +29,7 @@ ACLOCAL_AMFLAGS = -I m4 lib_LTLIBRARIES = libguac-client-vnc.la libguac_client_vnc_la_SOURCES = \ - argv.c \ + argv.c \ auth.c \ client.c \ clipboard.c \ @@ -42,7 +42,7 @@ libguac_client_vnc_la_SOURCES = \ vnc.c noinst_HEADERS = \ - argv.h \ + argv.h \ auth.h \ client.h \ clipboard.h \ diff --git a/src/protocols/vnc/argv.c b/src/protocols/vnc/argv.c index 24ac33ed..0f8caebf 100644 --- a/src/protocols/vnc/argv.c +++ b/src/protocols/vnc/argv.c @@ -118,33 +118,29 @@ static int guac_vnc_argv_end_handler(guac_user* user, guac_stream* stream) { case GUAC_VNC_ARGV_SETTING_USERNAME: /* Update username in settings. */ - if (settings->username != NULL) - free(settings->username); - settings->username = malloc(strlen(argv->buffer) * sizeof(char)); - strcpy(settings->username, argv->buffer); + free(settings->username); + settings->username = strndup(argv->buffer, argv->length); /* Remove the username conditional flag. */ - vnc_client->argv_cond_flags ^= GUAC_VNC_COND_FLAG_USERNAME; + vnc_client->vnc_credential_flags &= ~GUAC_VNC_COND_FLAG_USERNAME; break; /* Update password */ case GUAC_VNC_ARGV_SETTING_PASSWORD: /* Update password in settings */ - if (settings->password != NULL) - free(settings->password); - settings->password = malloc(strlen(argv->buffer) * sizeof(char)); - strcpy(settings->password, argv->buffer); + free(settings->password); + settings->password = strndup(argv->buffer, argv->length); /* Remove the password conditional flag. */ - vnc_client->argv_cond_flags ^= GUAC_VNC_COND_FLAG_PASSWORD; + vnc_client->vnc_credential_flags &= ~GUAC_VNC_COND_FLAG_PASSWORD; break; } /* If no flags are set, signal the conditional. */ - if (!vnc_client->argv_cond_flags) - pthread_cond_broadcast(&(vnc_client->argv_cond)); + if (!vnc_client->vnc_credential_flags) + pthread_cond_broadcast(&(vnc_client->vnc_credential_cond)); free(argv); return 0; diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index c0ae2b98..47aae87f 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -39,7 +39,7 @@ char* guac_vnc_get_password(rfbClient* client) { /* If password isn't around, prompt for it. */ if (settings->password == NULL || strcmp(settings->password, "") == 0) { /* Lock the thread. */ - pthread_mutex_lock(&(vnc_client->argv_lock)); + pthread_mutex_lock(&(vnc_client->vnc_credential_lock)); /* Send the request for password and flush the socket. */ guac_protocol_send_required(gc->socket, @@ -47,13 +47,14 @@ char* guac_vnc_get_password(rfbClient* client) { guac_socket_flush(gc->socket); /* Set the conditional flag. */ - vnc_client->argv_cond_flags |= GUAC_VNC_COND_FLAG_PASSWORD; + vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_PASSWORD; /* Wait for the condition. */ - pthread_cond_wait(&(vnc_client->argv_cond), &(vnc_client->argv_lock)); + pthread_cond_wait(&(vnc_client->vnc_credential_cond), + &(vnc_client->vnc_credential_lock)); /* Unlock the thread. */ - pthread_mutex_unlock(&(vnc_client->argv_lock)); + pthread_mutex_unlock(&(vnc_client->vnc_credential_lock)); } return settings->password; @@ -75,34 +76,35 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { if (settings->username == NULL || strcmp(settings->username, "") == 0) { params[i] = "username"; i++; - vnc_client->argv_cond_flags |= GUAC_VNC_COND_FLAG_USERNAME; + vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_USERNAME; } /* Check if password is null or empty. */ if (settings->password == NULL || strcmp(settings->password, "") == 0) { params[i] = "password"; i++; - vnc_client->argv_cond_flags |= GUAC_VNC_COND_FLAG_PASSWORD; + vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_PASSWORD; } /* If we have empty parameters, request them. */ if (i > 0) { /* Lock the thread. */ - pthread_mutex_lock(&(vnc_client->argv_lock)); + pthread_mutex_lock(&(vnc_client->vnc_credential_lock)); /* Send required parameters to client and flush the socket. */ guac_protocol_send_required(gc->socket, (const char**) params); guac_socket_flush(gc->socket); /* Wait for the parameters to be returned. */ - pthread_cond_wait(&(vnc_client->argv_cond), &(vnc_client->argv_lock)); + pthread_cond_wait(&(vnc_client->vnc_credential_cond), + &(vnc_client->vnc_credential_lock)); /* Pull the credentials from updated settings. */ creds->userCredential.username = settings->username; creds->userCredential.password = settings->password; /* Unlock the thread. */ - pthread_mutex_unlock(&(vnc_client->argv_lock)); + pthread_mutex_unlock(&(vnc_client->vnc_credential_lock)); return creds; diff --git a/src/protocols/vnc/client.c b/src/protocols/vnc/client.c index e52dfc55..99d4410b 100644 --- a/src/protocols/vnc/client.c +++ b/src/protocols/vnc/client.c @@ -54,10 +54,10 @@ int guac_client_init(guac_client* client) { pthread_mutex_init(&(vnc_client->tls_lock), NULL); #endif - /* Initialize argv lock and condition */ - pthread_mutex_init(&(vnc_client->argv_lock), NULL); - pthread_cond_init(&(vnc_client->argv_cond), NULL); - vnc_client->argv_cond_flags = 0; + /* Initialize credential lock, cond, and flags */ + pthread_mutex_init(&(vnc_client->vnc_credential_lock), NULL); + pthread_cond_init(&(vnc_client->vnc_credential_cond), NULL); + vnc_client->vnc_credential_flags = 0; /* Init clipboard */ vnc_client->clipboard = guac_common_clipboard_alloc(GUAC_VNC_CLIPBOARD_MAX_LENGTH); @@ -141,9 +141,9 @@ int guac_vnc_client_free_handler(guac_client* client) { pthread_mutex_destroy(&(vnc_client->tls_lock)); #endif - /* Clean up argv mutex */ - pthread_cond_destroy(&(vnc_client->argv_cond)); - pthread_mutex_destroy(&(vnc_client->argv_lock)); + /* Clean up credential mutex */ + pthread_cond_destroy(&(vnc_client->vnc_credential_cond)); + pthread_mutex_destroy(&(vnc_client->vnc_credential_lock)); /* Free generic data struct */ free(client->data); diff --git a/src/protocols/vnc/vnc.h b/src/protocols/vnc/vnc.h index c30205e8..ead4e6d1 100644 --- a/src/protocols/vnc/vnc.h +++ b/src/protocols/vnc/vnc.h @@ -71,21 +71,6 @@ typedef struct guac_vnc_client { */ pthread_mutex_t tls_lock; #endif - - /** - * The lock for tracking changes via argv. - */ - pthread_mutex_t argv_lock; - - /** - * The condition for signaling argv updates. - */ - pthread_cond_t argv_cond; - - /** - * Flags for conditional signaling for argv updates; - */ - unsigned argv_cond_flags; /** * The underlying VNC client. @@ -158,6 +143,24 @@ typedef struct guac_vnc_client { * Clipboard encoding-specific writer. */ guac_iconv_write* clipboard_writer; + + /** + * A lock that will be locked when retrieving required credentials from + * the client, and unlocked when credentials have been retrieved. + */ + pthread_mutex_t vnc_credential_lock; + + /** + * A condition to use for signaling the thread when credentials have been + * retrieved from the client. + */ + pthread_cond_t vnc_credential_cond; + + /** + * A field to track flags related to retrieving required credentials + * from the client. + */ + unsigned vnc_credential_flags; } guac_vnc_client; From 51b9c9c103e20a9d436b7b3b1beadb939445a36b Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Wed, 8 Apr 2020 22:46:29 -0400 Subject: [PATCH 06/26] GUACAMOLE-221: Remove manual addition of null terminator --- src/protocols/rdp/argv.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/protocols/rdp/argv.c b/src/protocols/rdp/argv.c index 2d34c57d..946c0520 100644 --- a/src/protocols/rdp/argv.c +++ b/src/protocols/rdp/argv.c @@ -115,7 +115,6 @@ static int guac_rdp_argv_end_handler(guac_user* user, /* Append null terminator to value */ guac_rdp_argv* argv = (guac_rdp_argv*) stream->data; - argv->buffer[argv->length] = '\0'; /* Apply changes to chosen setting */ switch (argv->setting) { From 7759f9b1c0bd27cf632e6f9862d7f6f6dbdecff9 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 14 Apr 2020 19:02:38 -0400 Subject: [PATCH 07/26] GUACAMOLE-221: Add socket keepalive when sending required fields. --- src/libguac/protocol.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index fa9e477d..2e2e6b0d 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -994,6 +994,9 @@ int guac_protocol_send_rect(guac_socket* socket, static int __guac_protocol_send_required(guac_socket* socket, const char** required) { + // The socket should be kept alive while waiting for user response. + guac_socket_require_keep_alive(socket); + if (guac_socket_write_string(socket, "8.required")) return -1; From 5c309f5cb12e1bd99118f8554ce3256ee63f4baf Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 26 Jun 2020 15:41:16 -0400 Subject: [PATCH 08/26] GUACAMOLE-221: Move away from reserved function names. --- src/libguac/protocol.c | 43 +++++++++++------------------------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index 2e2e6b0d..284c94b5 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -80,7 +80,7 @@ ssize_t __guac_socket_write_length_double(guac_socket* socket, double d) { * @return * Zero on success, non-zero on error. */ -static int __guac_socket_write_array(guac_socket* socket, const char** array) { +static int guac_socket_write_array(guac_socket* socket, const char** array) { /* Loop through array, writing provided values to the socket. */ for (int i=0; array[i] != NULL; i++) { @@ -128,7 +128,7 @@ static int __guac_protocol_send_args(guac_socket* socket, const char** args) { || __guac_socket_write_length_string(socket, GUACAMOLE_PROTOCOL_VERSION)) return -1; - if (__guac_socket_write_array(socket, args)) + if (guac_socket_write_array(socket, args)) return -1; return guac_socket_write_string(socket, ";"); @@ -333,7 +333,7 @@ static int __guac_protocol_send_connect(guac_socket* socket, const char** args) if (guac_socket_write_string(socket, "7.connect")) return -1; - if (__guac_socket_write_array(socket, args)) + if (guac_socket_write_array(socket, args)) return -1; return guac_socket_write_string(socket, ";"); @@ -975,44 +975,23 @@ int guac_protocol_send_rect(guac_socket* socket, } -/** - * Sends the "required" instruction on the given socket, looping - * through all the items provided in the NULL-terminated array, - * and closing out the instruction. Returns zero on success, or - * non-zero on error. - * - * @param socket - * The socket on which to write the instruction. - * - * @param required - * The NULL-terminated array of required parameters to send - * to the client. - * - * @return - * Zero if successful, non-zero on error. - */ -static int __guac_protocol_send_required(guac_socket* socket, - const char** required) { - +int guac_protocol_send_required(guac_socket* socket, const char** required) { + + int ret_val; + + guac_socket_instruction_begin(socket); + // The socket should be kept alive while waiting for user response. guac_socket_require_keep_alive(socket); if (guac_socket_write_string(socket, "8.required")) return -1; - if (__guac_socket_write_array(socket, required)) + if (guac_socket_write_array(socket, required)) return -1; - return guac_socket_write_string(socket, ";"); - -} - -int guac_protocol_send_required(guac_socket* socket, const char** required) { + ret_val = guac_socket_write_string(socket, ";"); - int ret_val; - - guac_socket_instruction_begin(socket); - ret_val = __guac_protocol_send_required(socket, required); guac_socket_instruction_end(socket); return ret_val; From 5881209f1261e8d45d12acb9eb8af3e8d694f5f0 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 27 Jun 2020 17:55:43 -0400 Subject: [PATCH 09/26] GUACAMOLE-221: Move keep-alives to protocol implementation and only send required instruction to owner. --- src/libguac/protocol.c | 3 --- src/protocols/rdp/rdp.c | 5 +++-- src/protocols/ssh/ssh.c | 7 ++++--- src/protocols/vnc/auth.c | 5 +++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index 284c94b5..ff903c4b 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -981,9 +981,6 @@ int guac_protocol_send_required(guac_socket* socket, const char** required) { guac_socket_instruction_begin(socket); - // The socket should be kept alive while waiting for user response. - guac_socket_require_keep_alive(socket); - if (guac_socket_write_string(socket, "8.required")) return -1; diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index fb52dff5..75a20bda 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -258,8 +258,9 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, pthread_mutex_lock(&(rdp_client->rdp_credential_lock)); /* Send require params and flush socket. */ - guac_protocol_send_required(client->socket, (const char**) params); - guac_socket_flush(client->socket); + guac_socket_require_keep_alive(client->__owner->socket); + guac_protocol_send_required(client->__owner->socket, (const char**) params); + guac_socket_flush(client->__owner->socket); /* Wait for condition. */ pthread_cond_wait(&(rdp_client->rdp_credential_cond), &(rdp_client->rdp_credential_lock)); diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 74e0e8cb..d2bc5b12 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -76,9 +76,10 @@ static void guac_ssh_get_credential(guac_client *client, char* cred_name) { /* Lock the terminal thread while prompting for the credential. */ pthread_mutex_lock(&(ssh_client->term_channel_lock)); - /* Let the client know what we require and flush the socket. */ - guac_protocol_send_required(client->socket, (const char* []) {cred_name, NULL}); - guac_socket_flush(client->socket); + /* Let the owner know what we require and flush the socket. */ + guac_socket_require_keep_alive(client->__owner->socket); + guac_protocol_send_required(client->__owner->socket, (const char* []) {cred_name, NULL}); + guac_socket_flush(client->__owner->socket); /* Wait for the response, and then unlock the thread. */ pthread_cond_wait(&(ssh_client->ssh_credential_cond), &(ssh_client->term_channel_lock)); diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index 47aae87f..6ea21a38 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -42,9 +42,10 @@ char* guac_vnc_get_password(rfbClient* client) { pthread_mutex_lock(&(vnc_client->vnc_credential_lock)); /* Send the request for password and flush the socket. */ - guac_protocol_send_required(gc->socket, + guac_socket_require_keep_alive(gc->__owner->socket); + guac_protocol_send_required(gc->__owner->socket, (const char* []) {"password", NULL}); - guac_socket_flush(gc->socket); + guac_socket_flush(gc->__owner->socket); /* Set the conditional flag. */ vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_PASSWORD; From 5ec25517618533a86bd50fe0cfadf4e332ab615b Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 27 Jun 2020 18:29:38 -0400 Subject: [PATCH 10/26] GUACAMOLE-221: Use constants for parameters updated via argv or required instructions. --- src/common-ssh/common-ssh/ssh-constants.h | 60 ++++++++++++++++++++++ src/common-ssh/ssh.c | 3 +- src/libguac/guacamole/protocol.h | 2 +- src/protocols/rdp/argv.c | 7 +-- src/protocols/rdp/rdp.c | 12 ++--- src/protocols/rdp/settings.c | 6 +-- src/protocols/rdp/settings.h | 18 +++++++ src/protocols/ssh/argv.c | 62 +++++++++++++++++++++++ src/protocols/ssh/argv.h | 18 +++++++ src/protocols/ssh/settings.c | 7 +-- src/protocols/ssh/ssh.c | 5 +- src/protocols/vnc/argv.c | 4 +- src/protocols/vnc/auth.c | 10 ++-- src/protocols/vnc/settings.c | 4 +- src/protocols/vnc/settings.h | 12 +++++ 15 files changed, 202 insertions(+), 28 deletions(-) create mode 100644 src/common-ssh/common-ssh/ssh-constants.h diff --git a/src/common-ssh/common-ssh/ssh-constants.h b/src/common-ssh/common-ssh/ssh-constants.h new file mode 100644 index 00000000..fc7efb80 --- /dev/null +++ b/src/common-ssh/common-ssh/ssh-constants.h @@ -0,0 +1,60 @@ +/* + * 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. + */ + +#ifndef GUAC_COMMON_SSH_CONSTANTS_H +#define GUAC_COMMON_SSH_CONSTANTS_H + +/** + * The name of the parameter that Guacamole uses to collect the username from + * the Guacamole client and send it to the SSH server. + */ +#define GUAC_SSH_PARAMETER_NAME_USERNAME "username" + +/** + * The name of the parameter that Guacamole uses to collect the password from + * the Guacamole client and send it to the SSH server. + */ +#define GUAC_SSH_PARAMETER_NAME_PASSWORD "password" + +/** + * The name of the parameter that Guacamole uses to collect a SSH key passphrase + * from the Guacamole client in order to unlock a private key. + */ +#define GUAC_SSH_PARAMETER_NAME_PASSPHRASE "passphrase" + +/** + * The name of the parameter that Guacamole uses to collect the terminal + * color scheme from the Guacamole client. + */ +#define GUAC_SSH_PARAMETER_NAME_COLOR_SCHEME "color-scheme" + +/** + * The name of the parameter that Guacamole uses to collect the font name + * from the Guacamole client. + */ +#define GUAC_SSH_PARAMETER_NAME_FONT_NAME "font-name" + +/** + * The name of the parameter that Guacamole uses to collect the font size + * from the Guacamole client + */ +#define GUAC_SSH_PARAMETER_NAME_FONT_SIZE "font-size" + +#endif /* GUAC_COMMON_SSH_CONSTANTS_H */ + diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index eed4c003..07188d98 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -19,6 +19,7 @@ #include "common-ssh/key.h" #include "common-ssh/ssh.h" +#include "common-ssh/ssh-constants.h" #include "common-ssh/user.h" #include @@ -360,7 +361,7 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) /* Attempt authentication with username + password. */ if (user->password == NULL && common_session->credential_handler) - common_session->credential_handler(client, "password"); + common_session->credential_handler(client, GUAC_SSH_PARAMETER_NAME_PASSWORD); /* Authenticate with password, if provided */ if (user->password != NULL) { diff --git a/src/libguac/guacamole/protocol.h b/src/libguac/guacamole/protocol.h index 54e8d6f6..6ee1ad4e 100644 --- a/src/libguac/guacamole/protocol.h +++ b/src/libguac/guacamole/protocol.h @@ -800,7 +800,7 @@ int guac_protocol_send_rect(guac_socket* socket, const guac_layer* layer, * is needed to continue the connection. * * @param socket - * The guac_socket connection to use. + * The guac_socket connection to which to send the instruction. * * @param required * A NULL-terminated array of required parameters. diff --git a/src/protocols/rdp/argv.c b/src/protocols/rdp/argv.c index 946c0520..023a98df 100644 --- a/src/protocols/rdp/argv.c +++ b/src/protocols/rdp/argv.c @@ -20,6 +20,7 @@ #include "config.h" #include "argv.h" #include "rdp.h" +#include "settings.h" #include #include @@ -154,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, "username") == 0) + if (strcmp(name, GUAC_RDP_PARAMETER_NAME_USERNAME) == 0) setting = GUAC_RDP_ARGV_SETTING_USERNAME; - else if (strcmp(name, "password") == 0) + else if (strcmp(name, GUAC_RDP_PARAMETER_NAME_PASSWORD) == 0) setting = GUAC_RDP_ARGV_SETTING_PASSWORD; - else if (strcmp(name, "domain") == 0) + else if (strcmp(name, GUAC_RDP_PARAMETER_NAME_DOMAIN) == 0) setting = GUAC_RDP_ARGV_SETTING_DOMAIN; /* No other connection parameters may be updated */ diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 75a20bda..07e670e4 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -232,20 +232,20 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, char* params[4] = {}; int i = 0; - if (settings->username == NULL || strcmp(settings->username, "") == 0) { - params[i] = "username"; + if (settings->username == NULL) { + params[i] = GUAC_RDP_PARAMETER_NAME_USERNAME; rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_USERNAME; i++; } - if (settings->password == NULL || strcmp(settings->password, "") == 0) { - params[i] = "password"; + if (settings->password == NULL) { + params[i] = GUAC_RDP_PARAMETER_NAME_PASSWORD; rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_PASSWORD; i++; } - if (settings->domain == NULL || strcmp(settings->domain, "") == 0) { - params[i] = "domain"; + if (settings->domain == NULL) { + params[i] = GUAC_RDP_PARAMETER_NAME_DOMAIN; rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_DOMAIN; i++; } diff --git a/src/protocols/rdp/settings.c b/src/protocols/rdp/settings.c index 307c83ba..468df416 100644 --- a/src/protocols/rdp/settings.c +++ b/src/protocols/rdp/settings.c @@ -42,9 +42,9 @@ const char* GUAC_RDP_CLIENT_ARGS[] = { "hostname", "port", - "domain", - "username", - "password", + GUAC_RDP_PARAMETER_NAME_DOMAIN, + GUAC_RDP_PARAMETER_NAME_USERNAME, + GUAC_RDP_PARAMETER_NAME_PASSWORD, "width", "height", "dpi", diff --git a/src/protocols/rdp/settings.h b/src/protocols/rdp/settings.h index d8092216..e2257962 100644 --- a/src/protocols/rdp/settings.h +++ b/src/protocols/rdp/settings.h @@ -73,6 +73,24 @@ */ #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. */ diff --git a/src/protocols/ssh/argv.c b/src/protocols/ssh/argv.c index ff32afde..802b6dd6 100644 --- a/src/protocols/ssh/argv.c +++ b/src/protocols/ssh/argv.c @@ -20,6 +20,7 @@ #include "config.h" #include "argv.h" #include "ssh.h" +#include "common-ssh/ssh-constants.h" #include "terminal/terminal.h" #include @@ -42,9 +43,70 @@ int guac_ssh_argv_callback(guac_user* user, const char* mimetype, if (strcmp(name, GUAC_SSH_ARGV_COLOR_SCHEME) == 0) guac_terminal_apply_color_scheme(terminal, value); +<<<<<<< HEAD /* Update font name */ else if (strcmp(name, GUAC_SSH_ARGV_FONT_NAME) == 0) guac_terminal_apply_font(terminal, value, -1, 0); +======= + /* Update username */ + case GUAC_SSH_ARGV_SETTING_USERNAME: + free(settings->username); + settings->username = strndup(argv->buffer, argv->length); + + pthread_cond_broadcast(&(ssh_client->ssh_credential_cond)); + break; + + /* Update password */ + case GUAC_SSH_ARGV_SETTING_PASSWORD: + + /* Update password in settings */ + free(settings->password); + settings->password = strndup(argv->buffer, argv->length); + + /* Update password in ssh user */ + guac_common_ssh_user* ssh_user = (guac_common_ssh_user*) ssh_client->user; + if (ssh_user != NULL) + guac_common_ssh_user_set_password(ssh_user, argv->buffer); + + pthread_cond_broadcast(&(ssh_client->ssh_credential_cond)); + break; + + /* Update private key passphrase */ + case GUAC_SSH_ARGV_SETTING_PASSPHRASE: + free(settings->key_passphrase); + settings->key_passphrase = strndup(argv->buffer, argv->length); + + pthread_cond_broadcast(&(ssh_client->ssh_credential_cond)); + break; + + /* Update color scheme */ + case GUAC_SSH_ARGV_SETTING_COLOR_SCHEME: + guac_terminal_apply_color_scheme(terminal, argv->buffer); + guac_client_stream_argv(client, client->socket, "text/plain", + GUAC_SSH_PARAMETER_NAME_COLOR_SCHEME, argv->buffer); + break; + + /* Update font name */ + case GUAC_SSH_ARGV_SETTING_FONT_NAME: + guac_terminal_apply_font(terminal, argv->buffer, -1, 0); + guac_client_stream_argv(client, client->socket, "text/plain", + GUAC_SSH_PARAMETER_NAME_FONT_NAME, argv->buffer); + break; + + /* Update font size */ + case GUAC_SSH_ARGV_SETTING_FONT_SIZE: + + /* Update only if font size is sane */ + size = atoi(argv->buffer); + if (size > 0) { + guac_terminal_apply_font(terminal, NULL, size, + ssh_client->settings->resolution); + guac_client_stream_argv(client, client->socket, "text/plain", + GUAC_SSH_PARAMETER_NAME_FONT_SIZE, argv->buffer); + } + + break; +>>>>>>> GUACAMOLE-221: Use constants for parameters updated via argv or required instructions. /* Update only if font size is sane */ else if (strcmp(name, GUAC_SSH_ARGV_FONT_SIZE) == 0) { diff --git a/src/protocols/ssh/argv.h b/src/protocols/ssh/argv.h index 4cbdb4c8..4dc24250 100644 --- a/src/protocols/ssh/argv.h +++ b/src/protocols/ssh/argv.h @@ -44,6 +44,24 @@ */ #define GUAC_SSH_ARGV_FONT_SIZE "font-size" +/** + * The name of the parameter that specifies/updates the username used by the + * connection. + */ +#define GUAC_SSH_ARGV_USERNAME "username" + +/** + * The name of the parameter that specifies/updates the password used by the + * connection. + */ +#define GUAC_SSH_ARGV_PASSWORD "password" + +/** + * The name of the parameter that specifies/updates the private ky passphrase + * used by the connection. + */ +#define GUAC_SSH_ARGV_PASSPHRASE "passphrase" + /** * Handles a received argument value from a Guacamole "argv" instruction, * updating the given connection parameter. diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 242e1d29..8b057b6a 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -22,6 +22,7 @@ #include "argv.h" #include "client.h" #include "common/defaults.h" +#include "common-ssh/ssh-constants.h" #include "settings.h" #include @@ -36,8 +37,8 @@ const char* GUAC_SSH_CLIENT_ARGS[] = { "hostname", "host-key", "port", - "username", - "password", + GUAC_SSH_ARGV_USERNAME, + GUAC_SSH_ARGV_PASSWORD, GUAC_SSH_ARGV_FONT_NAME, GUAC_SSH_ARGV_FONT_SIZE, "enable-sftp", @@ -45,7 +46,7 @@ const char* GUAC_SSH_CLIENT_ARGS[] = { "sftp-disable-download", "sftp-disable-upload", "private-key", - "passphrase", + GUAC_SSH_ARGV_PASSPHRASE, #ifdef ENABLE_SSH_AGENT "enable-agent", #endif diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index d2bc5b12..6179694e 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -23,6 +23,7 @@ #include "common/recording.h" #include "common-ssh/sftp.h" #include "common-ssh/ssh.h" +#include "common-ssh/ssh-constants.h" #include "settings.h" #include "sftp.h" #include "ssh.h" @@ -108,7 +109,7 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { /* Get username */ while (settings->username == NULL) - guac_ssh_get_credential(client, "username"); + guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_USERNAME); /* Create user object from username */ user = guac_common_ssh_create_user(settings->username); @@ -133,7 +134,7 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { /* Prompt for passphrase if missing */ while (settings->key_passphrase == NULL) - guac_ssh_get_credential(client, "passphrase"); + guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_PASSPHRASE); /* Reattempt import with passphrase */ if (guac_common_ssh_user_import_key(user, diff --git a/src/protocols/vnc/argv.c b/src/protocols/vnc/argv.c index 0f8caebf..5bbe04fd 100644 --- a/src/protocols/vnc/argv.c +++ b/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, "username") == 0) + if (strcmp(name, GUAC_VNC_PARAMETER_NAME_USERNAME) == 0) setting = GUAC_VNC_ARGV_SETTING_USERNAME; - else if (strcmp(name, "password") == 0) + else if (strcmp(name, GUAC_VNC_PARAMETER_NAME_PASSWORD) == 0) setting = GUAC_VNC_ARGV_SETTING_PASSWORD; /* No other connection parameters may be updated */ diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index 6ea21a38..b5f93221 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -44,7 +44,7 @@ char* guac_vnc_get_password(rfbClient* client) { /* Send the request for password and flush the socket. */ guac_socket_require_keep_alive(gc->__owner->socket); guac_protocol_send_required(gc->__owner->socket, - (const char* []) {"password", NULL}); + (const char* []) {GUAC_VNC_PARAMETER_NAME_PASSWORD, NULL}); guac_socket_flush(gc->__owner->socket); /* Set the conditional flag. */ @@ -74,15 +74,15 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { int i = 0; /* Check if username is null or empty. */ - if (settings->username == NULL || strcmp(settings->username, "") == 0) { - params[i] = "username"; + if (settings->username == NULL) { + params[i] = GUAC_VNC_PARAMETER_NAME_USERNAME; i++; vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_USERNAME; } /* Check if password is null or empty. */ - if (settings->password == NULL || strcmp(settings->password, "") == 0) { - params[i] = "password"; + if (settings->password == NULL) { + params[i] = GUAC_VNC_PARAMETER_NAME_PASSWORD; i++; vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_PASSWORD; } diff --git a/src/protocols/vnc/settings.c b/src/protocols/vnc/settings.c index c0d14859..f4104754 100644 --- a/src/protocols/vnc/settings.c +++ b/src/protocols/vnc/settings.c @@ -37,8 +37,8 @@ const char* GUAC_VNC_CLIENT_ARGS[] = { "port", "read-only", "encodings", - "username", - "password", + GUAC_VNC_PARAMETER_NAME_USERNAME, + GUAC_VNC_PARAMETER_NAME_PASSWORD, "swap-red-blue", "color-depth", "cursor", diff --git a/src/protocols/vnc/settings.h b/src/protocols/vnc/settings.h index 8d5659ef..7a386833 100644 --- a/src/protocols/vnc/settings.h +++ b/src/protocols/vnc/settings.h @@ -30,6 +30,18 @@ */ #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. */ From c579e7337f8ed26e99e700b3fc905789615771a5 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 27 Jun 2020 19:57:12 -0400 Subject: [PATCH 11/26] GUACAMOLE-221: Implement function for sending required to client owner. --- src/libguac/client.c | 19 +++++++++++++++++++ src/libguac/guacamole/client.h | 16 ++++++++++++++++ src/protocols/rdp/rdp.c | 5 ++--- src/protocols/ssh/ssh.c | 5 ++--- src/protocols/vnc/auth.c | 9 ++++----- 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/libguac/client.c b/src/libguac/client.c index 64404534..6f1c595b 100644 --- a/src/libguac/client.c +++ b/src/libguac/client.c @@ -478,6 +478,25 @@ int guac_client_load_plugin(guac_client* client, const char* protocol) { } +int guac_client_owner_send_required(guac_client* client, const char** required) { + + int retval; + + pthread_rwlock_rdlock(&(client->__users_lock)); + + /* Invoke callback with current owner */ + retval = guac_protocol_send_required(client->__owner->socket, required); + + /* Flush the socket */ + guac_socket_flush(client->__owner->socket); + + pthread_rwlock_unlock(&(client->__users_lock)); + + /* Return value from sending required */ + return retval; + +} + /** * Updates the provided approximate processing lag, taking into account the * processing lag of the given user. diff --git a/src/libguac/guacamole/client.h b/src/libguac/guacamole/client.h index 71d48f22..578fc593 100644 --- a/src/libguac/guacamole/client.h +++ b/src/libguac/guacamole/client.h @@ -549,6 +549,22 @@ int guac_client_load_plugin(guac_client* client, const char* protocol); */ int guac_client_get_processing_lag(guac_client* client); +/** + * Sends a request to the owner of the given guac_client for parameters required + * to continue the connection started by the client. The function returns zero + * on success or non-zero on failure. + * + * @param client + * The client where additional connection parameters are required. + * + * @param required + * The NULL-terminated array of required parameters. + * + * @return + * Zero on success, non-zero on failure. + */ +int guac_client_owner_send_required(guac_client* client, const char** required); + /** * Streams the given connection parameter value over an argument value stream * ("argv" instruction), exposing the current value of the named connection diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 07e670e4..9c064c70 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -258,9 +258,8 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, pthread_mutex_lock(&(rdp_client->rdp_credential_lock)); /* Send require params and flush socket. */ - guac_socket_require_keep_alive(client->__owner->socket); - guac_protocol_send_required(client->__owner->socket, (const char**) params); - guac_socket_flush(client->__owner->socket); + guac_socket_require_keep_alive(client->socket); + guac_client_owner_send_required(client, (const char**) params); /* Wait for condition. */ pthread_cond_wait(&(rdp_client->rdp_credential_cond), &(rdp_client->rdp_credential_lock)); diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 6179694e..23fbee0d 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -78,9 +78,8 @@ static void guac_ssh_get_credential(guac_client *client, char* cred_name) { pthread_mutex_lock(&(ssh_client->term_channel_lock)); /* Let the owner know what we require and flush the socket. */ - guac_socket_require_keep_alive(client->__owner->socket); - guac_protocol_send_required(client->__owner->socket, (const char* []) {cred_name, NULL}); - guac_socket_flush(client->__owner->socket); + guac_socket_require_keep_alive(client->socket); + guac_client_owner_send_required(client, (const char* []) {cred_name, NULL}); /* Wait for the response, and then unlock the thread. */ pthread_cond_wait(&(ssh_client->ssh_credential_cond), &(ssh_client->term_channel_lock)); diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index b5f93221..434e1c0a 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -42,10 +42,9 @@ char* guac_vnc_get_password(rfbClient* client) { pthread_mutex_lock(&(vnc_client->vnc_credential_lock)); /* Send the request for password and flush the socket. */ - guac_socket_require_keep_alive(gc->__owner->socket); - guac_protocol_send_required(gc->__owner->socket, + guac_socket_require_keep_alive(gc->socket); + guac_client_owner_send_required(gc, (const char* []) {GUAC_VNC_PARAMETER_NAME_PASSWORD, NULL}); - guac_socket_flush(gc->__owner->socket); /* Set the conditional flag. */ vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_PASSWORD; @@ -93,8 +92,8 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { pthread_mutex_lock(&(vnc_client->vnc_credential_lock)); /* Send required parameters to client and flush the socket. */ - guac_protocol_send_required(gc->socket, (const char**) params); - guac_socket_flush(gc->socket); + guac_socket_require_keep_alive(gc->socket); + guac_client_owner_send_required(gc, (const char**) params); /* Wait for the parameters to be returned. */ pthread_cond_wait(&(vnc_client->vnc_credential_cond), From 0761908a7755a27a2a4c8ea503c1d8a64b849429 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 27 Jun 2020 22:43:25 -0400 Subject: [PATCH 12/26] GUACAMOLE-221: Sockets start keep alive by default. --- src/libguac/guacamole/socket.h | 4 ++++ src/libguac/socket.c | 6 ++++-- src/protocols/rdp/rdp.c | 3 +-- src/protocols/ssh/ssh.c | 6 +----- src/protocols/vnc/auth.c | 6 ++---- src/protocols/vnc/vnc.c | 3 --- 6 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/libguac/guacamole/socket.h b/src/libguac/guacamole/socket.h index 2da697fe..cce4f69e 100644 --- a/src/libguac/guacamole/socket.h +++ b/src/libguac/guacamole/socket.h @@ -136,6 +136,10 @@ void guac_socket_free(guac_socket* socket); * to ensure neither side of the socket times out while the socket is open. * This ping will take the form of a "nop" instruction. * + * @deprecated + * Manually starting the keep alive process on sockets is no longer + * necessary, as the sockets enable the "nop" ping by default. + * * @param socket * The guac_socket to declare as requiring an automatic keep-alive ping. */ diff --git a/src/libguac/socket.c b/src/libguac/socket.c index 66442d0c..8892256e 100644 --- a/src/libguac/socket.c +++ b/src/libguac/socket.c @@ -150,8 +150,10 @@ guac_socket* guac_socket_alloc() { socket->state = GUAC_SOCKET_OPEN; socket->last_write_timestamp = guac_timestamp_current(); - /* No keep alive ping by default */ - socket->__keep_alive_enabled = 0; + /* keep alive ping by default */ + socket->__keep_alive_enabled = 1; + pthread_create(&(socket->__keep_alive_thread), NULL, + __guac_socket_keep_alive_thread, (void*) socket); /* No handlers yet */ socket->read_handler = NULL; diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 9c064c70..c4bfc28b 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -257,8 +257,7 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, /* Lock the client thread. */ pthread_mutex_lock(&(rdp_client->rdp_credential_lock)); - /* Send require params and flush socket. */ - guac_socket_require_keep_alive(client->socket); + /* Send require parameters to the owner. */ guac_client_owner_send_required(client, (const char**) params); /* Wait for condition. */ diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 23fbee0d..9c87380f 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -77,8 +77,7 @@ static void guac_ssh_get_credential(guac_client *client, char* cred_name) { /* Lock the terminal thread while prompting for the credential. */ pthread_mutex_lock(&(ssh_client->term_channel_lock)); - /* Let the owner know what we require and flush the socket. */ - guac_socket_require_keep_alive(client->socket); + /* Let the owner know what we require. */ guac_client_owner_send_required(client, (const char* []) {cred_name, NULL}); /* Wait for the response, and then unlock the thread. */ @@ -272,9 +271,6 @@ void* ssh_client_thread(void* data) { return NULL; } - /* Ensure connection is kept alive during lengthy connects */ - guac_socket_require_keep_alive(client->socket); - /* Open SSH session */ ssh_client->session = guac_common_ssh_create_session(client, settings->hostname, settings->port, ssh_client->user, settings->server_alive_interval, diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index 434e1c0a..fc534573 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -41,8 +41,7 @@ char* guac_vnc_get_password(rfbClient* client) { /* Lock the thread. */ pthread_mutex_lock(&(vnc_client->vnc_credential_lock)); - /* Send the request for password and flush the socket. */ - guac_socket_require_keep_alive(gc->socket); + /* Send the request for password to the owner. */ guac_client_owner_send_required(gc, (const char* []) {GUAC_VNC_PARAMETER_NAME_PASSWORD, NULL}); @@ -91,8 +90,7 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { /* Lock the thread. */ pthread_mutex_lock(&(vnc_client->vnc_credential_lock)); - /* Send required parameters to client and flush the socket. */ - guac_socket_require_keep_alive(gc->socket); + /* Send required parameters to owner. */ guac_client_owner_send_required(gc, (const char**) params); /* Wait for the parameters to be returned. */ diff --git a/src/protocols/vnc/vnc.c b/src/protocols/vnc/vnc.c index 31d97022..33bd5c59 100644 --- a/src/protocols/vnc/vnc.c +++ b/src/protocols/vnc/vnc.c @@ -259,9 +259,6 @@ void* guac_vnc_client_thread(void* data) { "clipboard encoding: '%s'.", settings->clipboard_encoding); } - /* Ensure connection is kept alive during lengthy connects */ - guac_socket_require_keep_alive(client->socket); - /* Set up libvncclient logging */ rfbClientLog = guac_vnc_client_log_info; rfbClientErr = guac_vnc_client_log_error; From b00b629b9628d7354d3305bd24523eac07fd80ca Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 27 Jun 2020 22:57:33 -0400 Subject: [PATCH 13/26] GUACAMOLE-221: Clean up VNC mutex; update comments. --- src/protocols/rdp/client.c | 1 + src/protocols/rdp/rdp.h | 4 ++++ src/protocols/vnc/vnc.h | 3 +++ 3 files changed, 8 insertions(+) diff --git a/src/protocols/rdp/client.c b/src/protocols/rdp/client.c index 94998ddb..f521d6e4 100644 --- a/src/protocols/rdp/client.c +++ b/src/protocols/rdp/client.c @@ -226,6 +226,7 @@ int guac_rdp_client_free_handler(guac_client* client) { /* Destroy the pthread conditional handler */ pthread_cond_destroy(&(rdp_client->rdp_credential_cond)); + pthread_mutex_destroy(&(rdp_client->rdp_credential_lock)); pthread_rwlock_destroy(&(rdp_client->lock)); diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index 1509913f..26684337 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -184,6 +184,10 @@ typedef struct guac_rdp_client { /** * Flags for tracking events related to the rdp_credential_cond * pthread condition. + * + * @see GUAC_RDP_CRED_FLAG_USERNAME + * @see GUAC_RDP_CRED_FLAG_PASSWORD + * @see GUAC_RDP_CRED_FLAG_DOMAIN */ unsigned rdp_credential_flags; diff --git a/src/protocols/vnc/vnc.h b/src/protocols/vnc/vnc.h index ead4e6d1..9a2d48a8 100644 --- a/src/protocols/vnc/vnc.h +++ b/src/protocols/vnc/vnc.h @@ -159,6 +159,9 @@ typedef struct guac_vnc_client { /** * A field to track flags related to retrieving required credentials * from the client. + * + * @see GUAC_VNC_COND_FLAG_USERNAME + * @see GUAC_VNC_COND_FLAG_PASSWORD */ unsigned vnc_credential_flags; From bc8ed4e10471f63431ca30922d762921fdef50fa Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sun, 28 Jun 2020 15:56:01 -0400 Subject: [PATCH 14/26] GUACAMOLE-221: Implement guacd support for verifying that client can accept the required instruction. --- src/guacd/proc.c | 3 + src/libguac/client.c | 81 ++++++++++++++++++---- src/libguac/guacamole/client.h | 15 ++++ src/libguac/guacamole/protocol-constants.h | 2 +- src/libguac/guacamole/protocol-types.h | 35 ++++++++++ src/libguac/guacamole/protocol.h | 27 ++++++++ src/libguac/guacamole/socket.h | 4 -- src/libguac/guacamole/user.h | 17 +++++ src/libguac/protocol.c | 60 ++++++++++++++++ src/libguac/socket.c | 6 +- src/libguac/user-handshake.c | 8 ++- src/libguac/user.c | 9 +++ src/protocols/rdp/rdp.c | 9 ++- src/protocols/ssh/ssh.c | 4 ++ src/protocols/vnc/auth.c | 16 ++++- 15 files changed, 269 insertions(+), 27 deletions(-) diff --git a/src/guacd/proc.c b/src/guacd/proc.c index aacf2b03..9612dd33 100644 --- a/src/guacd/proc.c +++ b/src/guacd/proc.c @@ -340,6 +340,9 @@ static void guacd_exec_proc(guacd_proc* proc, const char* protocol) { owner = 0; } + + /* Enable keep alive on the broadcast socket */ + guac_socket_require_keep_alive(client->socket); cleanup_client: diff --git a/src/libguac/client.c b/src/libguac/client.c index 6f1c595b..cc898371 100644 --- a/src/libguac/client.c +++ b/src/libguac/client.c @@ -478,22 +478,39 @@ int guac_client_load_plugin(guac_client* client, const char* protocol) { } +/** + * 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. + * + * @param user + * The guac_user that will receive the required parameters, who is the owner + * of the client. + * + * @param data + * Pointer to a NULL-terminated array of required parameters that will be + * passed on to the owner to continue the connection. + * + * @return + * A pointer to an integer containing the return status of the send + * operation. + */ +static void* guac_client_owner_send_required_callback(guac_user* user, void* data) { + + const char** required = (const char **) data; + + /* Send required parameters to owner. */ + return (void*) ((intptr_t) guac_protocol_send_required(user->socket, required)); + +} + int guac_client_owner_send_required(guac_client* client, const char** required) { - int retval; - - pthread_rwlock_rdlock(&(client->__users_lock)); - - /* Invoke callback with current owner */ - retval = guac_protocol_send_required(client->__owner->socket, required); + /* Don't send required instruction if client does not support it. */ + if (!guac_client_owner_supports_required(client)) + return -1; - /* Flush the socket */ - guac_socket_flush(client->__owner->socket); - - pthread_rwlock_unlock(&(client->__users_lock)); - - /* Return value from sending required */ - return retval; + return (int) ((intptr_t) guac_client_for_owner(client, guac_client_owner_send_required_callback, required)); } @@ -652,6 +669,44 @@ static void* __webp_support_callback(guac_user* user, void* data) { } #endif +/** + * 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. + * + * @param user + * 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" + * instruction. This will be 0 if the owner does not support this + * instruction, or 1 if the owner does support it. + * + * @return + * Always NULL. + */ +static void* guac_owner_supports_required_callback(guac_user* user, void* data) { + + int* required_supported = (int *) data; + + /* Check if owner supports required */ + if (*required_supported) + *required_supported = guac_user_supports_required(user); + + return NULL; + +} + +int guac_client_owner_supports_required(guac_client* client) { + + int required_supported = 1; + + guac_client_for_owner(client, guac_owner_supports_required_callback, &required_supported); + + return required_supported; + +} + int guac_client_supports_webp(guac_client* client) { #ifdef ENABLE_WEBP diff --git a/src/libguac/guacamole/client.h b/src/libguac/guacamole/client.h index 578fc593..4ffe5cf4 100644 --- a/src/libguac/guacamole/client.h +++ b/src/libguac/guacamole/client.h @@ -708,6 +708,21 @@ void guac_client_stream_webp(guac_client* client, guac_socket* socket, guac_composite_mode mode, const guac_layer* layer, int x, int y, cairo_surface_t* surface, int quality, int lossless); +/** + * Returns whether the owner of the given client supports the "required" + * instruction, returning non-zero if the client owner does support the + * instruction, or zero if the owner does not. + * + * @param client + * The Guacamole client whose owner should be checked for supporting + * the "required" instruction. + * + * @return + * Non-zero if the owner of the given client supports the "required" + * instruction, zero otherwise. + */ +int guac_client_owner_supports_required(guac_client* client); + /** * Returns whether all users of the given client support WebP. If any user does * not support WebP, or the server cannot encode WebP images, zero is returned. diff --git a/src/libguac/guacamole/protocol-constants.h b/src/libguac/guacamole/protocol-constants.h index afa67bb9..5bd91b07 100644 --- a/src/libguac/guacamole/protocol-constants.h +++ b/src/libguac/guacamole/protocol-constants.h @@ -38,7 +38,7 @@ * This version is passed by the __guac_protocol_send_args() function from the * server to the client during the client/server handshake. */ -#define GUACAMOLE_PROTOCOL_VERSION "VERSION_1_1_0" +#define GUACAMOLE_PROTOCOL_VERSION "VERSION_1_3_0" /** * The maximum number of bytes that should be sent in any one blob instruction diff --git a/src/libguac/guacamole/protocol-types.h b/src/libguac/guacamole/protocol-types.h index b85d36e7..46ca4573 100644 --- a/src/libguac/guacamole/protocol-types.h +++ b/src/libguac/guacamole/protocol-types.h @@ -20,6 +20,8 @@ #ifndef _GUAC_PROTOCOL_TYPES_H #define _GUAC_PROTOCOL_TYPES_H +#include + /** * Type definitions related to the Guacamole protocol. * @@ -276,5 +278,38 @@ typedef enum guac_line_join_style { GUAC_LINE_JOIN_ROUND = 0x2 } guac_line_join_style; +/** + * Track the various protocol versions supported by guacd to help negotiate + * features that may not be supported by various versions of the client. + */ +typedef enum guac_protocol_version { + + /** + * An unknown version of the Guacamole protocol. + */ + GUAC_PROTOCOL_VERSION_UNKNOWN = 0x000000, + + /** + * Original protocol version 1.0.0, which lacks support for negotiating + * parameters and protocol version. + */ + GUAC_PROTOCOL_VERSION_1_0_0 = 0x010000, + + /** + * Protocol version 1.1.0, which includes support for parameter and version + * negotiation and for sending timezone information from the client + * to the server. + */ + GUAC_PROTOCOL_VERSION_1_1_0 = 0x010100, + + /** + * Protocol version 1.3.0, which supports the "required" instruction, + * allowing connections in guacd to request information from the client and + * await a response. + */ + GUAC_PROTOCOL_VERSION_1_3_0 = 0x010300 + +} guac_protocol_version; + #endif diff --git a/src/libguac/guacamole/protocol.h b/src/libguac/guacamole/protocol.h index 6ee1ad4e..c4c6f211 100644 --- a/src/libguac/guacamole/protocol.h +++ b/src/libguac/guacamole/protocol.h @@ -1023,5 +1023,32 @@ int guac_protocol_send_name(guac_socket* socket, const char* name); */ int guac_protocol_decode_base64(char* base64); +/** + * Given a string representation of a protocol version, search the mapping + * to find the matching enum version, or GUAC_PROTOCOL_VERSION_UNKNOWN if no + * match is found. + * + * @param version_string + * The string representation of the protocol version. + * + * @return + * The enum value of the protocol version, or GUAC_PROTOCOL_VERSION_UNKNOWN + * if no match is found. + */ +guac_protocol_version guac_protocol_string_to_version(char* version_string); + +/** + * Given the enum value of the protocol version, return a pointer to the string + * representation of the version, or NULL if the version is unknown. + * + * @param version + * The enum value of the protocol version. + * + * @return + * A pointer to the string representation of the protocol version, or NULL + * if the version is unknown. + */ +const char* guac_protocol_version_to_string(guac_protocol_version version); + #endif diff --git a/src/libguac/guacamole/socket.h b/src/libguac/guacamole/socket.h index cce4f69e..2da697fe 100644 --- a/src/libguac/guacamole/socket.h +++ b/src/libguac/guacamole/socket.h @@ -136,10 +136,6 @@ void guac_socket_free(guac_socket* socket); * to ensure neither side of the socket times out while the socket is open. * This ping will take the form of a "nop" instruction. * - * @deprecated - * Manually starting the keep alive process on sockets is no longer - * necessary, as the sockets enable the "nop" ping by default. - * * @param socket * The guac_socket to declare as requiring an automatic keep-alive ping. */ diff --git a/src/libguac/guacamole/user.h b/src/libguac/guacamole/user.h index fac42b98..de04d2a9 100644 --- a/src/libguac/guacamole/user.h +++ b/src/libguac/guacamole/user.h @@ -95,6 +95,12 @@ struct guac_user_info { * is the standard tzdata naming convention. */ const char* timezone; + + /** + * The Guacamole protocol version that the remote system supports, allowing + * for feature support to be negotiated between client and server. + */ + guac_protocol_version protocol_version; }; @@ -823,6 +829,17 @@ void guac_user_stream_webp(guac_user* user, guac_socket* socket, guac_composite_mode mode, const guac_layer* layer, int x, int y, cairo_surface_t* surface, int quality, int lossless); +/** + * Returns whether the given user supports the "required" instruction. + * + * @param user + * The Guacamole user to check for support of the "required" instruction. + * + * @return + * Non-zero if the user supports the "required" instruction, otherwise zero. + */ +int guac_user_supports_required(guac_user* user); + /** * Returns whether the given user supports WebP. If the user does not * support WebP, or the server cannot encode WebP images, zero is returned. diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index ff903c4b..2c1e132b 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -23,6 +23,7 @@ #include "guacamole/layer.h" #include "guacamole/object.h" #include "guacamole/protocol.h" +#include "guacamole/protocol-types.h" #include "guacamole/socket.h" #include "guacamole/stream.h" #include "guacamole/unicode.h" @@ -39,6 +40,34 @@ #include #include +/** + * Structure mapping the enum value of a protocol version to the string + * representation of the version. + */ +typedef struct guac_protocol_version_mapping { + + /** + * The enum value representing the selected protocol version. + */ + guac_protocol_version version; + + /** + * The string value representing the protocol version. + */ + char* version_string; + +} guac_protocol_version_mapping; + +/** + * The map of known protocol version enum to the corresponding string value. + */ +guac_protocol_version_mapping guac_protocol_version_table[] = { + { GUAC_PROTOCOL_VERSION_1_0_0, "VERSION_1_0_0" }, + { GUAC_PROTOCOL_VERSION_1_1_0, "VERSION_1_1_0" }, + { GUAC_PROTOCOL_VERSION_1_3_0, "VERSION_1_3_0" }, + { GUAC_PROTOCOL_VERSION_UNKNOWN, NULL } +}; + /* Output formatting functions */ ssize_t __guac_socket_write_length_string(guac_socket* socket, const char* str) { @@ -1275,3 +1304,34 @@ int guac_protocol_decode_base64(char* base64) { } +guac_protocol_version guac_protocol_string_to_version(char* version_string) { + + guac_protocol_version_mapping* current = guac_protocol_version_table; + while (current->version != GUAC_PROTOCOL_VERSION_UNKNOWN) { + + if (strcmp(current->version_string, version_string) == 0) + return current->version; + + current++; + + } + + return GUAC_PROTOCOL_VERSION_UNKNOWN; + +} + +const char* guac_protocol_version_to_string(guac_protocol_version version) { + + guac_protocol_version_mapping* current = guac_protocol_version_table; + while (current->version) { + + if (current->version == version) { + return (const char*) current->version_string; + } + + } + + return NULL; + +} + diff --git a/src/libguac/socket.c b/src/libguac/socket.c index 8892256e..66442d0c 100644 --- a/src/libguac/socket.c +++ b/src/libguac/socket.c @@ -150,10 +150,8 @@ guac_socket* guac_socket_alloc() { socket->state = GUAC_SOCKET_OPEN; socket->last_write_timestamp = guac_timestamp_current(); - /* keep alive ping by default */ - socket->__keep_alive_enabled = 1; - pthread_create(&(socket->__keep_alive_thread), NULL, - __guac_socket_keep_alive_thread, (void*) socket); + /* No keep alive ping by default */ + socket->__keep_alive_enabled = 0; /* No handlers yet */ socket->read_handler = NULL; diff --git a/src/libguac/user-handshake.c b/src/libguac/user-handshake.c index 89afccdd..bc9992b2 100644 --- a/src/libguac/user-handshake.c +++ b/src/libguac/user-handshake.c @@ -344,12 +344,16 @@ int guac_user_handle_connection(guac_user* user, int usec_timeout) { guac_client_log(client, GUAC_LOG_INFO, "User \"%s\" joined connection " "\"%s\" (%i users now present)", user->user_id, client->connection_id, client->connected_users); - if (strcmp(parser->argv[0],"") != 0) + if (strcmp(parser->argv[0],"") != 0) { guac_client_log(client, GUAC_LOG_DEBUG, "Client is using protocol " "version \"%s\"", parser->argv[0]); - else + user->info.protocol_version = guac_protocol_string_to_version(parser->argv[0]); + } + else { guac_client_log(client, GUAC_LOG_DEBUG, "Client has not defined " "its protocol version."); + user->info.protocol_version = GUAC_PROTOCOL_VERSION_1_0_0; + } /* Handle user I/O, wait for connection to terminate */ guac_user_start(parser, user, usec_timeout); diff --git a/src/libguac/user.c b/src/libguac/user.c index aaa51bb9..4320ca77 100644 --- a/src/libguac/user.c +++ b/src/libguac/user.c @@ -316,6 +316,15 @@ void guac_user_stream_webp(guac_user* user, guac_socket* socket, } +int guac_user_supports_required(guac_user* user) { + + if (user == NULL) + return 0; + + return (user->info.protocol_version >= GUAC_PROTOCOL_VERSION_1_3_0); + +} + int guac_user_supports_webp(guac_user* user) { #ifdef ENABLE_WEBP diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index c4bfc28b..cfba7f50 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -202,7 +202,7 @@ BOOL rdp_freerdp_pre_connect(freerdp* instance) { /** * Callback invoked by FreeRDP when authentication is required but a username * and password has not already been given. In the case of Guacamole, this - * function always succeeds but does not populate the usename or password. The + * function always succeeds but does not populate the username or password. The * username/password must be given within the connection parameters. * * @param instance @@ -232,6 +232,11 @@ 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 + exit. */ + if (!guac_client_owner_supports_required(client)) + return TRUE; + if (settings->username == NULL) { params[i] = GUAC_RDP_PARAMETER_NAME_USERNAME; rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_USERNAME; @@ -257,7 +262,7 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, /* Lock the client thread. */ pthread_mutex_lock(&(rdp_client->rdp_credential_lock)); - /* Send require parameters to the owner. */ + /* Send required parameters to the owner. */ guac_client_owner_send_required(client, (const char**) params); /* Wait for condition. */ diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 9c87380f..40040c01 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -74,6 +74,10 @@ static void guac_ssh_get_credential(guac_client *client, char* cred_name) { 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. */ pthread_mutex_lock(&(ssh_client->term_channel_lock)); diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index fc534573..260acee3 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -36,6 +36,11 @@ char* guac_vnc_get_password(rfbClient* client) { guac_vnc_client* vnc_client = ((guac_vnc_client*) gc->data); guac_vnc_settings* settings = vnc_client->settings; + /* If the client does not support the "required" instruction, just return + the configuration data. */ + if (!guac_client_owner_supports_required(gc)) + return ((guac_vnc_client*) gc->data)->settings->password; + /* If password isn't around, prompt for it. */ if (settings->password == NULL || strcmp(settings->password, "") == 0) { /* Lock the thread. */ @@ -64,10 +69,19 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { guac_client* gc = rfbClientGetClientData(client, GUAC_VNC_CLIENT_KEY); guac_vnc_client* vnc_client = ((guac_vnc_client*) gc->data); guac_vnc_settings* settings = vnc_client->settings; - + /* Handle request for Username/Password credentials */ if (credentialType == rfbCredentialTypeUser) { rfbCredential *creds = malloc(sizeof(rfbCredential)); + + /* If the client does not support the "required" instruction, just return + the parameters already configured. */ + if (!guac_client_owner_supports_required(gc)) { + creds->userCredential.username = settings->username; + creds->userCredential.password = settings->password; + return creds; + } + char* params[2] = {NULL}; int i = 0; From 0db61198e96de757c1cbee4642106371ab9813b5 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Wed, 1 Jul 2020 16:37:55 -0400 Subject: [PATCH 15/26] GUACAMOLE-221: Fix up lots of comments, streamline code, and fix SSH mutex lock. --- src/libguac/guacamole/protocol-types.h | 2 - src/libguac/protocol.c | 21 ++++---- src/protocols/rdp/rdp.h | 7 ++- src/protocols/ssh/client.c | 6 ++- src/protocols/ssh/ssh.c | 70 ++++++++++++++++++++++---- src/protocols/ssh/ssh.h | 6 +++ src/protocols/vnc/vnc.h | 5 +- 7 files changed, 89 insertions(+), 28 deletions(-) diff --git a/src/libguac/guacamole/protocol-types.h b/src/libguac/guacamole/protocol-types.h index 46ca4573..c65bbc29 100644 --- a/src/libguac/guacamole/protocol-types.h +++ b/src/libguac/guacamole/protocol-types.h @@ -20,8 +20,6 @@ #ifndef _GUAC_PROTOCOL_TYPES_H #define _GUAC_PROTOCOL_TYPES_H -#include - /** * Type definitions related to the Guacamole protocol. * diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index 2c1e132b..78e24fb1 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -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 - * and lengths of the values in the array to the given socket. Return - * zero on success, non-zero on error. + * Loop through the provided NULL-terminated array, writing the values in the + * array to the given socket. Values are written as a series of Guacamole + * 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 * The socket to which the data should be written. @@ -111,7 +112,7 @@ ssize_t __guac_socket_write_length_double(guac_socket* socket, double d) { */ static int guac_socket_write_array(guac_socket* socket, const char** array) { - /* Loop through array, writing provided values to the socket. */ + /* Loop through array, writing provided values to the socket. */ for (int i=0; array[i] != NULL; i++) { if (guac_socket_write_string(socket, ",")) @@ -122,7 +123,7 @@ static int guac_socket_write_array(guac_socket* socket, const char** array) { } - return 0; + return 0; } @@ -1009,14 +1010,10 @@ int guac_protocol_send_required(guac_socket* socket, const char** required) { int ret_val; guac_socket_instruction_begin(socket); - - if (guac_socket_write_string(socket, "8.required")) - return -1; - if (guac_socket_write_array(socket, required)) - return -1; - - ret_val = guac_socket_write_string(socket, ";"); + ret_val = guac_socket_write_string(socket, "8.required") + || guac_socket_write_array(socket, required) + || guac_socket_write_string(socket, ";"); guac_socket_instruction_end(socket); diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index 26684337..3b84b8e9 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -183,7 +183,12 @@ typedef struct guac_rdp_client { /** * 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_PASSWORD diff --git a/src/protocols/ssh/client.c b/src/protocols/ssh/client.c index 1bd6647e..986968e2 100644 --- a/src/protocols/ssh/client.c +++ b/src/protocols/ssh/client.c @@ -47,7 +47,7 @@ int guac_client_init(guac_client* client) { /* Init clipboard */ ssh_client->clipboard = guac_common_clipboard_alloc(GUAC_SSH_CLIPBOARD_MAX_LENGTH); - + /* Set handlers */ client->join_handler = guac_ssh_user_join_handler; client->free_handler = guac_ssh_client_free_handler; @@ -120,6 +120,10 @@ int guac_ssh_client_free_handler(guac_client* client) { guac_common_clipboard_free(ssh_client->clipboard); 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(); return 0; } diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 40040c01..57effd54 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -58,6 +58,35 @@ #include #include +/** + * 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 * 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; - /* 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. */ - pthread_mutex_lock(&(ssh_client->term_channel_lock)); + pthread_mutex_lock(&(ssh_client->ssh_credential_lock)); /* Let the owner know what we require. */ guac_client_owner_send_required(client, (const char* []) {cred_name, NULL}); /* Wait for the response, and then unlock the thread. */ - pthread_cond_wait(&(ssh_client->ssh_credential_cond), &(ssh_client->term_channel_lock)); - pthread_mutex_unlock(&(ssh_client->term_channel_lock)); + pthread_cond_wait(&(ssh_client->ssh_credential_cond), &(ssh_client->ssh_credential_lock)); + pthread_mutex_unlock(&(ssh_client->ssh_credential_lock)); } @@ -110,9 +135,21 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { guac_common_ssh_user* user; /* Get username */ - while (settings->username == NULL) - guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_USERNAME); + 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); + } + + /* Fall back to terminal prompting. */ + else { + settings->username = guac_terminal_prompt(ssh_client->term, + "Login as: ", true); + } + } + /* Create user object from username */ user = guac_common_ssh_create_user(settings->username); @@ -135,8 +172,18 @@ 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) - guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_PASSPHRASE); + 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); + + /* Fall back to terminal prompt */ + else + settings->key_passphrase = guac_terminal_prompt(ssh_client->term, + "Key passphrase: ", true); + + } /* Reattempt import with passphrase */ if (guac_common_ssh_user_import_key(user, @@ -285,6 +332,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); /* Open channel for terminal */ diff --git a/src/protocols/ssh/ssh.h b/src/protocols/ssh/ssh.h index bfd2fd1b..1378522e 100644 --- a/src/protocols/ssh/ssh.h +++ b/src/protocols/ssh/ssh.h @@ -90,6 +90,12 @@ typedef struct guac_ssh_client { */ 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 * client to pass additional credentials before continuing the connection. diff --git a/src/protocols/vnc/vnc.h b/src/protocols/vnc/vnc.h index 9a2d48a8..62c3c1a1 100644 --- a/src/protocols/vnc/vnc.h +++ b/src/protocols/vnc/vnc.h @@ -158,7 +158,10 @@ typedef struct guac_vnc_client { /** * 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_PASSWORD From e8feeabfef42f70a05a157d7add9b9ff302cd16b Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Wed, 1 Jul 2020 17:06:31 -0400 Subject: [PATCH 16/26] GUACAMOLE-221: Implement CUnit tests for protocol version comparison and conversion. --- src/libguac/tests/Makefile.am | 1 + .../tests/protocol/guac_protocol_version.c | 69 +++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 src/libguac/tests/protocol/guac_protocol_version.c diff --git a/src/libguac/tests/Makefile.am b/src/libguac/tests/Makefile.am index 4ab3269f..6fd3798f 100644 --- a/src/libguac/tests/Makefile.am +++ b/src/libguac/tests/Makefile.am @@ -40,6 +40,7 @@ test_libguac_SOURCES = \ parser/read.c \ pool/next_free.c \ protocol/base64_decode.c \ + protocol/guac_protocol_version.c \ socket/fd_send_instruction.c \ socket/nested_send_instruction.c \ string/strlcat.c \ diff --git a/src/libguac/tests/protocol/guac_protocol_version.c b/src/libguac/tests/protocol/guac_protocol_version.c new file mode 100644 index 00000000..782b0aa4 --- /dev/null +++ b/src/libguac/tests/protocol/guac_protocol_version.c @@ -0,0 +1,69 @@ +/* + * 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. + */ + +#include +#include +#include + +/** + * Test which verifies that conversion of the guac_protocol_version enum to + * string values succeeds and produces the expected results. + */ +void test_guac_protocol__version_to_string() { + + guac_protocol_version version_a = GUAC_PROTOCOL_VERSION_1_3_0; + guac_protocol_version version_b = GUAC_PROTOCOL_VERSION_1_0_0; + guac_protocol_version version_c = GUAC_PROTOCOL_VERSION_UNKNOWN; + + CU_ASSERT_STRING_EQUAL_FATAL(guac_protocol_version_to_string(version_a), "VERSION_1_3_0"); + CU_ASSERT_STRING_EQUAL_FATAL(guac_protocol_version_to_string(version_b), "VERSION_1_0_0"); + CU_ASSERT_PTR_NULL_FATAL(guac_protocol_version_to_string(version_c)); + +} + +/** + * Test which verifies that the version of String representations of Guacamole + * protocol versions are successfully converted into their matching + * guac_protocol_version enum values, and that versions that do not match + * any version get the correct unknown value. + */ +void test_guac_protocol__string_to_version() { + + char* str_version_a = "VERSION_1_3_0"; + char* str_version_b = "VERSION_1_1_0"; + char* str_version_c = "AVACADO"; + char* str_version_d = "VERSION_31_4_1"; + + CU_ASSERT_EQUAL_FATAL(guac_protocol_string_to_version(str_version_a), GUAC_PROTOCOL_VERSION_1_3_0); + CU_ASSERT_EQUAL_FATAL(guac_protocol_string_to_version(str_version_b), GUAC_PROTOCOL_VERSION_1_1_0); + CU_ASSERT_EQUAL_FATAL(guac_protocol_string_to_version(str_version_c), GUAC_PROTOCOL_VERSION_UNKNOWN); + CU_ASSERT_EQUAL_FATAL(guac_protocol_string_to_version(str_version_d), GUAC_PROTOCOL_VERSION_UNKNOWN); + +} + +/** + * Test which verifies that the comparisons between guac_protocol_version enum + * values produces the expected results. + */ +void test_gauc_protocol__version_comparison() { + + CU_ASSERT_TRUE_FATAL(GUAC_PROTOCOL_VERSION_1_3_0 > GUAC_PROTOCOL_VERSION_1_0_0); + CU_ASSERT_TRUE_FATAL(GUAC_PROTOCOL_VERSION_UNKNOWN < GUAC_PROTOCOL_VERSION_1_1_0); + +} \ No newline at end of file From 98dbf15d0b2127854cca340a0255b94aeb342705 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Wed, 1 Jul 2020 17:33:22 -0400 Subject: [PATCH 17/26] GUACAMOLE-221: Fix up SSH terminal prompt fallback. --- src/common-ssh/common-ssh/ssh.h | 7 +++++- src/common-ssh/ssh.c | 9 +++++-- src/protocols/ssh/ssh.c | 44 +++++++++++++++++++-------------- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/common-ssh/common-ssh/ssh.h b/src/common-ssh/common-ssh/ssh.h index 609e1a1e..e354cf1a 100644 --- a/src/common-ssh/common-ssh/ssh.h +++ b/src/common-ssh/common-ssh/ssh.h @@ -34,8 +34,13 @@ * * @param cred_name * The connection parameter that is being requested from the client. + * + * @return + * A newly-allocated string containing the credentials request from the + * client, or NULL if the credentials will be updated via the required + * instruction. */ -typedef void guac_ssh_credential_handler(guac_client* client, char* cred_name); +typedef char* guac_ssh_credential_handler(guac_client* client, char* cred_name); /** * An SSH session, backed by libssh2 and associated with a particular diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 07188d98..32f8d550 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -360,8 +360,13 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) } /* Attempt authentication with username + password. */ - if (user->password == NULL && common_session->credential_handler) - common_session->credential_handler(client, GUAC_SSH_PARAMETER_NAME_PASSWORD); + if (user->password == NULL && common_session->credential_handler) { + + char* password = common_session->credential_handler(client, GUAC_SSH_PARAMETER_NAME_PASSWORD); + if (password != NULL) + user->password = password; + + } /* Authenticate with password, if provided */ if (user->password != NULL) { diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 57effd54..4b69b8ed 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -99,10 +99,26 @@ guac_ssh_credential_prompt_map ssh_credential_prompt_map[] = { * @param cred_name * The name of the parameter to prompt for in the client. */ -static void guac_ssh_get_credential(guac_client *client, char* cred_name) { +static char* guac_ssh_get_credential(guac_client *client, char* cred_name) { guac_ssh_client* ssh_client = (guac_ssh_client*) client->data; + /* If client owner does not support required, use terminal prompt. */ + if (!guac_client_owner_supports_required(client)) { + + /* Loop to find correct prompt for credential name. */ + guac_ssh_credential_prompt_map* current = ssh_credential_prompt_map; + while (current->name != NULL) { + if (strcmp(current->name, cred_name) == 0) + return guac_terminal_prompt(ssh_client->term, + current->prompt, true); + current++; + } + + /* No matching credential was found, so return NULL. */ + return NULL; + } + /* Lock the terminal thread while prompting for the credential. */ pthread_mutex_lock(&(ssh_client->ssh_credential_lock)); @@ -113,6 +129,8 @@ static void guac_ssh_get_credential(guac_client *client, char* cred_name) { pthread_cond_wait(&(ssh_client->ssh_credential_cond), &(ssh_client->ssh_credential_lock)); pthread_mutex_unlock(&(ssh_client->ssh_credential_lock)); + return NULL; + } /** @@ -137,16 +155,9 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { /* Get username */ 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); - } - - /* Fall back to terminal prompting. */ - else { - settings->username = guac_terminal_prompt(ssh_client->term, - "Login as: ", true); - } + char* username = guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_USERNAME); + if (username != NULL) + settings->username = username; } @@ -174,14 +185,9 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { /* Prompt for passphrase if missing */ 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); - - /* Fall back to terminal prompt */ - else - settings->key_passphrase = guac_terminal_prompt(ssh_client->term, - "Key passphrase: ", true); + char* passphrase = guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_PASSPHRASE); + if (passphrase != NULL) + settings->key_passphrase = passphrase; } From 26b9850d8708b0fd34e5dd5ae6583b23c6ca1b58 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 4 Aug 2020 22:29:52 -0400 Subject: [PATCH 18/26] GUACAMOLE-221: Remove bad rebase code. --- src/common-ssh/common-ssh/ssh-constants.h | 60 --------------------- src/common-ssh/ssh.c | 3 +- src/protocols/ssh/argv.c | 63 ----------------------- src/protocols/ssh/settings.c | 1 - src/protocols/ssh/ssh.c | 11 ++-- 5 files changed, 6 insertions(+), 132 deletions(-) delete mode 100644 src/common-ssh/common-ssh/ssh-constants.h diff --git a/src/common-ssh/common-ssh/ssh-constants.h b/src/common-ssh/common-ssh/ssh-constants.h deleted file mode 100644 index fc7efb80..00000000 --- a/src/common-ssh/common-ssh/ssh-constants.h +++ /dev/null @@ -1,60 +0,0 @@ -/* - * 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. - */ - -#ifndef GUAC_COMMON_SSH_CONSTANTS_H -#define GUAC_COMMON_SSH_CONSTANTS_H - -/** - * The name of the parameter that Guacamole uses to collect the username from - * the Guacamole client and send it to the SSH server. - */ -#define GUAC_SSH_PARAMETER_NAME_USERNAME "username" - -/** - * The name of the parameter that Guacamole uses to collect the password from - * the Guacamole client and send it to the SSH server. - */ -#define GUAC_SSH_PARAMETER_NAME_PASSWORD "password" - -/** - * The name of the parameter that Guacamole uses to collect a SSH key passphrase - * from the Guacamole client in order to unlock a private key. - */ -#define GUAC_SSH_PARAMETER_NAME_PASSPHRASE "passphrase" - -/** - * The name of the parameter that Guacamole uses to collect the terminal - * color scheme from the Guacamole client. - */ -#define GUAC_SSH_PARAMETER_NAME_COLOR_SCHEME "color-scheme" - -/** - * The name of the parameter that Guacamole uses to collect the font name - * from the Guacamole client. - */ -#define GUAC_SSH_PARAMETER_NAME_FONT_NAME "font-name" - -/** - * The name of the parameter that Guacamole uses to collect the font size - * from the Guacamole client - */ -#define GUAC_SSH_PARAMETER_NAME_FONT_SIZE "font-size" - -#endif /* GUAC_COMMON_SSH_CONSTANTS_H */ - diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 32f8d550..90ddeea1 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -19,7 +19,6 @@ #include "common-ssh/key.h" #include "common-ssh/ssh.h" -#include "common-ssh/ssh-constants.h" #include "common-ssh/user.h" #include @@ -362,7 +361,7 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) /* Attempt authentication with username + password. */ if (user->password == NULL && common_session->credential_handler) { - char* password = common_session->credential_handler(client, GUAC_SSH_PARAMETER_NAME_PASSWORD); + char* password = common_session->credential_handler(client, "password"); if (password != NULL) user->password = password; diff --git a/src/protocols/ssh/argv.c b/src/protocols/ssh/argv.c index 802b6dd6..9b3cf653 100644 --- a/src/protocols/ssh/argv.c +++ b/src/protocols/ssh/argv.c @@ -20,7 +20,6 @@ #include "config.h" #include "argv.h" #include "ssh.h" -#include "common-ssh/ssh-constants.h" #include "terminal/terminal.h" #include @@ -37,76 +36,14 @@ 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 color scheme */ if (strcmp(name, GUAC_SSH_ARGV_COLOR_SCHEME) == 0) guac_terminal_apply_color_scheme(terminal, value); -<<<<<<< HEAD /* Update font name */ else if (strcmp(name, GUAC_SSH_ARGV_FONT_NAME) == 0) guac_terminal_apply_font(terminal, value, -1, 0); -======= - /* Update username */ - case GUAC_SSH_ARGV_SETTING_USERNAME: - free(settings->username); - settings->username = strndup(argv->buffer, argv->length); - - pthread_cond_broadcast(&(ssh_client->ssh_credential_cond)); - break; - - /* Update password */ - case GUAC_SSH_ARGV_SETTING_PASSWORD: - - /* Update password in settings */ - free(settings->password); - settings->password = strndup(argv->buffer, argv->length); - - /* Update password in ssh user */ - guac_common_ssh_user* ssh_user = (guac_common_ssh_user*) ssh_client->user; - if (ssh_user != NULL) - guac_common_ssh_user_set_password(ssh_user, argv->buffer); - - pthread_cond_broadcast(&(ssh_client->ssh_credential_cond)); - break; - - /* Update private key passphrase */ - case GUAC_SSH_ARGV_SETTING_PASSPHRASE: - free(settings->key_passphrase); - settings->key_passphrase = strndup(argv->buffer, argv->length); - - pthread_cond_broadcast(&(ssh_client->ssh_credential_cond)); - break; - - /* Update color scheme */ - case GUAC_SSH_ARGV_SETTING_COLOR_SCHEME: - guac_terminal_apply_color_scheme(terminal, argv->buffer); - guac_client_stream_argv(client, client->socket, "text/plain", - GUAC_SSH_PARAMETER_NAME_COLOR_SCHEME, argv->buffer); - break; - - /* Update font name */ - case GUAC_SSH_ARGV_SETTING_FONT_NAME: - guac_terminal_apply_font(terminal, argv->buffer, -1, 0); - guac_client_stream_argv(client, client->socket, "text/plain", - GUAC_SSH_PARAMETER_NAME_FONT_NAME, argv->buffer); - break; - - /* Update font size */ - case GUAC_SSH_ARGV_SETTING_FONT_SIZE: - - /* Update only if font size is sane */ - size = atoi(argv->buffer); - if (size > 0) { - guac_terminal_apply_font(terminal, NULL, size, - ssh_client->settings->resolution); - guac_client_stream_argv(client, client->socket, "text/plain", - GUAC_SSH_PARAMETER_NAME_FONT_SIZE, argv->buffer); - } - - break; ->>>>>>> GUACAMOLE-221: Use constants for parameters updated via argv or required instructions. /* Update only if font size is sane */ else if (strcmp(name, GUAC_SSH_ARGV_FONT_SIZE) == 0) { diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 8b057b6a..9af51fa4 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -22,7 +22,6 @@ #include "argv.h" #include "client.h" #include "common/defaults.h" -#include "common-ssh/ssh-constants.h" #include "settings.h" #include diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 4b69b8ed..ff404f56 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -23,7 +23,6 @@ #include "common/recording.h" #include "common-ssh/sftp.h" #include "common-ssh/ssh.h" -#include "common-ssh/ssh-constants.h" #include "settings.h" #include "sftp.h" #include "ssh.h" @@ -81,9 +80,9 @@ typedef struct guac_ssh_credential_prompt_map { * 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: " }, + { GUAC_SSH_ARGV_USERNAME, "Login as: " }, + { GUAC_SSH_ARGV_PASSWORD, "Password: " }, + { GUAC_SSH_ARGV_PASSPHRASE, "Key passphrase: " }, { NULL, NULL} }; @@ -155,7 +154,7 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { /* Get username */ while (settings->username == NULL) { - char* username = guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_USERNAME); + char* username = guac_ssh_get_credential(client, GUAC_SSH_ARGV_USERNAME); if (username != NULL) settings->username = username; @@ -185,7 +184,7 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { /* Prompt for passphrase if missing */ while (settings->key_passphrase == NULL) { - char* passphrase = guac_ssh_get_credential(client, GUAC_SSH_PARAMETER_NAME_PASSPHRASE); + char* passphrase = guac_ssh_get_credential(client, GUAC_SSH_ARGV_PASSPHRASE); if (passphrase != NULL) settings->key_passphrase = passphrase; From ec3cdfd17b7cd805a132b4216df498eb4f3751c0 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 20 Aug 2020 22:11:09 -0400 Subject: [PATCH 19/26] GUACAMOLE-221: We need to flush the socket after sending required. --- src/libguac/protocol.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index 78e24fb1..16ee3d51 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -1013,7 +1013,8 @@ int guac_protocol_send_required(guac_socket* socket, const char** required) { ret_val = guac_socket_write_string(socket, "8.required") || guac_socket_write_array(socket, required) - || guac_socket_write_string(socket, ";"); + || guac_socket_write_string(socket, ";") + || guac_socket_flush(socket); guac_socket_instruction_end(socket); From f70fdfc6124836f6d3df4ce729f93f45c8476e6e Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 21 Aug 2020 14:45:24 -0400 Subject: [PATCH 20/26] GUACAMOLE-221: Add back in SSH credential argv support; fix style and comments. --- src/common-ssh/common-ssh/ssh.h | 4 +-- src/common-ssh/ssh.c | 2 +- src/libguac/client.c | 8 +++--- src/libguac/guacamole/protocol-types.h | 7 +++--- src/libguac/protocol.c | 8 +++--- src/protocols/rdp/argv.c | 6 ++--- src/protocols/rdp/argv.h | 18 ++++++++++++++ src/protocols/rdp/rdp.c | 34 ++++++++++++++++++-------- src/protocols/rdp/rdp.h | 4 +-- src/protocols/rdp/settings.c | 7 +++--- src/protocols/rdp/settings.h | 18 -------------- src/protocols/ssh/argv.c | 26 +++++++++++++++++++- src/protocols/ssh/argv.h | 2 +- src/protocols/ssh/client.c | 3 +++ src/protocols/ssh/settings.c | 4 +-- src/protocols/ssh/ssh.c | 29 +++++++++++++++------- src/protocols/ssh/ssh.h | 6 ++--- src/protocols/vnc/argv.c | 4 +-- src/protocols/vnc/argv.h | 12 +++++++++ src/protocols/vnc/auth.c | 7 +++--- src/protocols/vnc/client.c | 10 ++++---- src/protocols/vnc/settings.c | 5 ++-- src/protocols/vnc/settings.h | 12 --------- src/protocols/vnc/vnc.h | 10 ++++---- 24 files changed, 151 insertions(+), 95 deletions(-) diff --git a/src/common-ssh/common-ssh/ssh.h b/src/common-ssh/common-ssh/ssh.h index e354cf1a..336c4b54 100644 --- a/src/common-ssh/common-ssh/ssh.h +++ b/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 - * client, or NULL if the credentials will be updated via the required + * A newly-allocated string containing the credentials to be requested from + * 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); diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 90ddeea1..af3711f3 100644 --- a/src/common-ssh/ssh.c +++ b/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); } diff --git a/src/libguac/client.c b/src/libguac/client.c index cc898371..4be0d4de 100644 --- a/src/libguac/client.c +++ b/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. * diff --git a/src/libguac/guacamole/protocol-types.h b/src/libguac/guacamole/protocol-types.h index c65bbc29..d492fe7f 100644 --- a/src/libguac/guacamole/protocol-types.h +++ b/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 - * features that may not be supported by various versions of the client. + * The set of protocol versions known to guacd to help negotiate features that + * 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, diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index 16ee3d51..8002c92a 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -41,13 +41,13 @@ #include /** - * Structure mapping the enum value of a protocol version to the string - * representation of the version. + * A structure mapping the enum value of a Guacamole protocol version to the + * 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" }, diff --git a/src/protocols/rdp/argv.c b/src/protocols/rdp/argv.c index 023a98df..60c706ee 100644 --- a/src/protocols/rdp/argv.c +++ b/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 */ diff --git a/src/protocols/rdp/argv.h b/src/protocols/rdp/argv.h index b265517b..bef5e6ec 100644 --- a/src/protocols/rdp/argv.h +++ b/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 diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index cfba7f50..df06dc8c 100644 --- a/src/protocols/rdp/rdp.c +++ b/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 - * and password has not already been given. In the case of Guacamole, this - * function always succeeds but does not populate the username or password. The - * username/password must be given within the connection parameters. + * Callback invoked by FreeRDP when authentication is required but the required + * parameters have not been provided. In the case of Guacamole clients that + * support the "required" instruction, this function will send any of the three + * 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 - exit. */ - if (!guac_client_owner_supports_required(client)) + /* If the client does not support the "required" instruction, warn and + * quit. + */ + 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++; } diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index 3b84b8e9..4706a4fe 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -185,8 +185,8 @@ typedef struct guac_rdp_client { * Flags for tracking events related to the rdp_credential_cond * 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 + * provided by the client. All flags are cleared at the start of the + * connection, and then set as the RDP client determines that further * information is required. * * diff --git a/src/protocols/rdp/settings.c b/src/protocols/rdp/settings.c index 468df416..bac8af0b 100644 --- a/src/protocols/rdp/settings.c +++ b/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_PARAMETER_NAME_USERNAME, - GUAC_RDP_PARAMETER_NAME_PASSWORD, + GUAC_RDP_ARGV_DOMAIN, + GUAC_RDP_ARGV_USERNAME, + GUAC_RDP_ARGV_PASSWORD, "width", "height", "dpi", diff --git a/src/protocols/rdp/settings.h b/src/protocols/rdp/settings.h index e2257962..d8092216 100644 --- a/src/protocols/rdp/settings.h +++ b/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. */ diff --git a/src/protocols/ssh/argv.c b/src/protocols/ssh/argv.c index 9b3cf653..c6921565 100644 --- a/src/protocols/ssh/argv.c +++ b/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 */ diff --git a/src/protocols/ssh/argv.h b/src/protocols/ssh/argv.h index 4dc24250..0a726c10 100644 --- a/src/protocols/ssh/argv.h +++ b/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" diff --git a/src/protocols/ssh/client.c b/src/protocols/ssh/client.c index 986968e2..2a4ec5c9 100644 --- a/src/protocols/ssh/client.c +++ b/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); diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 9af51fa4..f287c901 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -36,8 +36,8 @@ const char* GUAC_SSH_CLIENT_ARGS[] = { "hostname", "host-key", "port", - GUAC_SSH_ARGV_USERNAME, - GUAC_SSH_ARGV_PASSWORD, + GUAC_SSH_ARGV_USERNAME, + GUAC_SSH_ARGV_PASSWORD, GUAC_SSH_ARGV_FONT_NAME, GUAC_SSH_ARGV_FONT_SIZE, "enable-sftp", diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index ff404f56..1a9e2717 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -83,13 +83,17 @@ guac_ssh_credential_prompt_map ssh_credential_prompt_map[] = { { GUAC_SSH_ARGV_USERNAME, "Login as: " }, { GUAC_SSH_ARGV_PASSWORD, "Password: " }, { GUAC_SSH_ARGV_PASSPHRASE, "Key passphrase: " }, - { NULL, NULL} + { NULL, NULL} }; /** - * This function generates a prompt to the specified instance of guac_client - * for the credential specified in the cred_name parameter, which should - * be a valid SSH connection parameter. + * Prompts the user for the credential specified in the cred_name parameter in + * order to continue a SSH connection. If the owner of the connection is running + * 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."); diff --git a/src/protocols/ssh/ssh.h b/src/protocols/ssh/ssh.h index 1378522e..b86ea48f 100644 --- a/src/protocols/ssh/ssh.h +++ b/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 - * user, and unlocked when those requirements are satisfied. + * Lock that controls access to updating credential parameters for the + * 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; diff --git a/src/protocols/vnc/argv.c b/src/protocols/vnc/argv.c index 5bbe04fd..cdd75f65 100644 --- a/src/protocols/vnc/argv.c +++ b/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 */ diff --git a/src/protocols/vnc/argv.h b/src/protocols/vnc/argv.h index 599eaaa7..c19262ee 100644 --- a/src/protocols/vnc/argv.h +++ b/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 */ diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index 260acee3..13f3cfc7 100644 --- a/src/protocols/vnc/auth.c +++ b/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; } diff --git a/src/protocols/vnc/client.c b/src/protocols/vnc/client.c index 99d4410b..d5613570 100644 --- a/src/protocols/vnc/client.c +++ b/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_cond_init(&(vnc_client->vnc_credential_cond), NULL); + pthread_mutex_init(&vnc_client->vnc_credential_lock, 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_mutex_destroy(&(vnc_client->vnc_credential_lock)); + pthread_cond_destroy(&vnc_client->vnc_credential_cond); + pthread_mutex_destroy(&vnc_client->vnc_credential_lock); /* Free generic data struct */ free(client->data); diff --git a/src/protocols/vnc/settings.c b/src/protocols/vnc/settings.c index f4104754..86207c47 100644 --- a/src/protocols/vnc/settings.c +++ b/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_PARAMETER_NAME_PASSWORD, + GUAC_VNC_ARGV_USERNAME, + GUAC_VNC_ARGV_PASSWORD, "swap-red-blue", "color-depth", "cursor", diff --git a/src/protocols/vnc/settings.h b/src/protocols/vnc/settings.h index 7a386833..8d5659ef 100644 --- a/src/protocols/vnc/settings.h +++ b/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. */ diff --git a/src/protocols/vnc/vnc.h b/src/protocols/vnc/vnc.h index 62c3c1a1..ff1e5486 100644 --- a/src/protocols/vnc/vnc.h +++ b/src/protocols/vnc/vnc.h @@ -46,12 +46,12 @@ #include /** - * 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 - * the client, and unlocked when credentials have been retrieved. + * A lock that controls access to updating credentials when connecting + * 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; From b6d3edb7498ba180839d665c3846691fe80a98a7 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 21 Aug 2020 20:23:52 -0400 Subject: [PATCH 21/26] GUACAMOLE-221: Move VNC and RDP argv to new callback. --- src/protocols/rdp/argv.c | 163 ++++--------------------------------- src/protocols/rdp/argv.h | 13 +-- src/protocols/rdp/client.c | 9 -- src/protocols/rdp/rdp.c | 16 ++-- src/protocols/rdp/rdp.h | 43 ---------- src/protocols/rdp/user.c | 4 +- src/protocols/vnc/argv.c | 153 +++------------------------------- src/protocols/vnc/argv.h | 14 +--- src/protocols/vnc/auth.c | 40 +++------ src/protocols/vnc/client.c | 9 -- src/protocols/vnc/user.c | 4 + src/protocols/vnc/vnc.h | 34 -------- 12 files changed, 61 insertions(+), 441 deletions(-) diff --git a/src/protocols/rdp/argv.c b/src/protocols/rdp/argv.c index 60c706ee..c6c922b9 100644 --- a/src/protocols/rdp/argv.c +++ b/src/protocols/rdp/argv.c @@ -30,160 +30,31 @@ #include #include -/** - * All RDP connection settings which may be updated by unprivileged users - * through "argv" streams. - */ -typedef enum guac_rdp_argv_setting { - - /** - * The username of the connection. - */ - GUAC_RDP_ARGV_SETTING_USERNAME, - - /** - * The password to authenticate the connection. - */ - GUAC_RDP_ARGV_SETTING_PASSWORD, - - /** - * The domain to use for connection authentication. - */ - GUAC_RDP_ARGV_SETTING_DOMAIN, - -} guac_rdp_argv_setting; - -/** - * The value or current status of a connection parameter received over an - * "argv" stream. - */ -typedef struct guac_rdp_argv { - - /** - * The specific setting being updated. - */ - guac_rdp_argv_setting setting; - - /** - * Buffer space for containing the received argument value. - */ - char buffer[GUAC_RDP_ARGV_MAX_LENGTH]; - - /** - * The number of bytes received so far. - */ - int length; - -} guac_rdp_argv; - -/** - * Handler for "blob" instructions which appends the data from received blobs - * to the end of the in-progress argument value buffer. - * - * @see guac_user_blob_handler - */ -static int guac_rdp_argv_blob_handler(guac_user* user, - guac_stream* stream, void* data, int length) { - - guac_rdp_argv* argv = (guac_rdp_argv*) stream->data; - - /* Calculate buffer size remaining, including space for null terminator, - adjusting received length accordingly */ - int remaining = sizeof(argv->buffer) - argv->length - 1; - if (length > remaining) - length = remaining; - - /* Append received data to end of buffer */ - memcpy(argv->buffer + argv->length, data, length); - argv->length += length; - - return 0; - -} - -/** - * Handler for "end" instructions which applies the changes specified by the - * argument value buffer associated with the stream. - * - * @see guac_user_end_handler - */ -static int guac_rdp_argv_end_handler(guac_user* user, - guac_stream* stream) { +int guac_rdp_argv_callback(guac_user* user, const char* mimetype, + const char* name, const char* value, void* data) { guac_client* client = user->client; guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; guac_rdp_settings* settings = rdp_client->settings; - - /* Append null terminator to value */ - guac_rdp_argv* argv = (guac_rdp_argv*) stream->data; - /* Apply changes to chosen setting */ - switch (argv->setting) { - - /* Update RDP username. */ - case GUAC_RDP_ARGV_SETTING_USERNAME: - free(settings->username); - settings->username = strndup(argv->buffer, argv->length); - rdp_client->rdp_credential_flags &= ~GUAC_RDP_CRED_FLAG_USERNAME; - break; - - case GUAC_RDP_ARGV_SETTING_PASSWORD: - free(settings->password); - settings->password = strndup(argv->buffer, argv->length); - rdp_client->rdp_credential_flags &= ~GUAC_RDP_CRED_FLAG_PASSWORD; - break; - - case GUAC_RDP_ARGV_SETTING_DOMAIN: - free(settings->domain); - settings->domain = strndup(argv->buffer, argv->length); - rdp_client->rdp_credential_flags &= ~GUAC_RDP_CRED_FLAG_DOMAIN; - break; - + /* Update username */ + if (strcmp(name, GUAC_RDP_ARGV_USERNAME) == 0) { + free(settings->username); + settings->username = strdup(value); } - if (!rdp_client->rdp_credential_flags) - pthread_cond_signal(&(rdp_client->rdp_credential_cond)); - - free(argv); - return 0; - -} - -int guac_rdp_argv_handler(guac_user* user, guac_stream* stream, - char* mimetype, char* name) { - - guac_rdp_argv_setting setting; - - /* Allow users to update authentication details */ - if (strcmp(name, GUAC_RDP_ARGV_USERNAME) == 0) - setting = GUAC_RDP_ARGV_SETTING_USERNAME; - else if (strcmp(name, GUAC_RDP_ARGV_PASSWORD) == 0) - setting = GUAC_RDP_ARGV_SETTING_PASSWORD; - else if (strcmp(name, GUAC_RDP_ARGV_DOMAIN) == 0) - setting = GUAC_RDP_ARGV_SETTING_DOMAIN; - - /* No other connection parameters may be updated */ - else { - guac_protocol_send_ack(user->socket, stream, "Not allowed.", - GUAC_PROTOCOL_STATUS_CLIENT_FORBIDDEN); - guac_socket_flush(user->socket); - return 0; + /* Update password */ + else if (strcmp(name, GUAC_RDP_ARGV_PASSWORD) == 0) { + free(settings->password); + settings->password = strdup(value); + } + + /* Update domain */ + else if (strcmp(name, GUAC_RDP_ARGV_DOMAIN) == 0) { + free(settings->domain); + settings->domain = strdup(value); } - guac_rdp_argv* argv = malloc(sizeof(guac_rdp_argv)); - argv->setting = setting; - argv->length = 0; - - /* Prepare stream to receive argument value */ - stream->blob_handler = guac_rdp_argv_blob_handler; - stream->end_handler = guac_rdp_argv_end_handler; - stream->data = argv; - - /* Signal stream is ready */ - guac_protocol_send_ack(user->socket, stream, "Ready for updated " - "parameter.", GUAC_PROTOCOL_STATUS_SUCCESS); - guac_socket_flush(user->socket); return 0; -} - +} \ No newline at end of file diff --git a/src/protocols/rdp/argv.h b/src/protocols/rdp/argv.h index bef5e6ec..c15aba92 100644 --- a/src/protocols/rdp/argv.h +++ b/src/protocols/rdp/argv.h @@ -23,19 +23,14 @@ #include "config.h" +#include #include /** - * The maximum number of bytes to allow for any argument value received via an - * argv stream, including null terminator. + * Handles a received argument value from a Guacamole "argv" instruction, + * updating the given connection parameter. */ -#define GUAC_RDP_ARGV_MAX_LENGTH 16384 - -/** - * Handles an incoming stream from a Guacamole "argv" instruction, updating the - * given connection parameter if that parameter is allowed to be updated. - */ -guac_user_argv_handler guac_rdp_argv_handler; +guac_argv_callback guac_rdp_argv_callback; /** * The name of the parameter that specifies/updates the username that will be diff --git a/src/protocols/rdp/client.c b/src/protocols/rdp/client.c index f521d6e4..07c90a33 100644 --- a/src/protocols/rdp/client.c +++ b/src/protocols/rdp/client.c @@ -158,11 +158,6 @@ int guac_client_init(guac_client* client, int argc, char** argv) { /* Initalize the lock */ pthread_rwlock_init(&(rdp_client->lock), NULL); - /* Init RDP credential lock and condition */ - pthread_mutex_init(&(rdp_client->rdp_credential_lock), &(rdp_client->attributes)); - pthread_cond_init(&(rdp_client->rdp_credential_cond), NULL);; - rdp_client->rdp_credential_flags = 0; - /* Set handlers */ client->join_handler = guac_rdp_user_join_handler; client->free_handler = guac_rdp_client_free_handler; @@ -223,10 +218,6 @@ int guac_rdp_client_free_handler(guac_client* client) { /* Clean up audio input buffer, if allocated */ if (rdp_client->audio_input != NULL) guac_rdp_audio_buffer_free(rdp_client->audio_input); - - /* Destroy the pthread conditional handler */ - pthread_cond_destroy(&(rdp_client->rdp_credential_cond)); - pthread_mutex_destroy(&(rdp_client->rdp_credential_lock)); pthread_rwlock_destroy(&(rdp_client->lock)); diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index df06dc8c..8bf1f22e 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -65,6 +65,7 @@ #include #include #include +#include #include #include #include @@ -247,25 +248,25 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, "be requested."); return TRUE; } - + /* If the username is undefined, add it to the requested parameters. */ if (settings->username == NULL) { + guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0); 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) { + guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0); 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) { + guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0); params[i] = GUAC_RDP_ARGV_DOMAIN; - rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_DOMAIN; i++; } @@ -273,22 +274,17 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, params[i] = NULL; if (i > 0) { - /* Lock the client thread. */ - pthread_mutex_lock(&(rdp_client->rdp_credential_lock)); /* Send required parameters to the owner. */ guac_client_owner_send_required(client, (const char**) params); - /* Wait for condition. */ - pthread_cond_wait(&(rdp_client->rdp_credential_cond), &(rdp_client->rdp_credential_lock)); + guac_argv_await((const char**) params); /* Get new values from settings. */ *username = settings->username; *password = settings->password; *domain = settings->domain; - /* Unlock the thread. */ - pthread_mutex_unlock(&(rdp_client->rdp_credential_lock)); } /* Always return TRUE allowing connection to retry. */ diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index 4706a4fe..e65f702c 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -49,21 +49,6 @@ #include #include -/** - * A flag for tracking if we are waiting conditionally on a username. - */ -#define GUAC_RDP_CRED_FLAG_USERNAME 1 - -/** - * A flag for tracking if we are waiting conditionally on a password. - */ -#define GUAC_RDP_CRED_FLAG_PASSWORD 2 - -/** - * A flag for tracking if we are waiting conditionally on a domain. - */ -#define GUAC_RDP_CRED_FLAG_DOMAIN 4 - /** * RDP-specific client data. */ @@ -168,34 +153,6 @@ typedef struct guac_rdp_client { */ guac_common_list* available_svc; - /** - * Lock which is locked when one or more credentials are required to - * complete the connection, and unlocked when credentials have been - * provided by the client. - */ - pthread_mutex_t rdp_credential_lock; - - /** - * Condition which is used when the pthread needs to wait for one or more - * credentials to be provided by the client. - */ - pthread_cond_t rdp_credential_cond; - - /** - * Flags for tracking events related to the rdp_credential_cond - * 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 RDP client determines that further - * information is required. - * - * - * @see GUAC_RDP_CRED_FLAG_USERNAME - * @see GUAC_RDP_CRED_FLAG_PASSWORD - * @see GUAC_RDP_CRED_FLAG_DOMAIN - */ - unsigned rdp_credential_flags; - /** * Common attributes for locks. */ diff --git a/src/protocols/rdp/user.c b/src/protocols/rdp/user.c index 8be53d3e..e7a91d77 100644 --- a/src/protocols/rdp/user.c +++ b/src/protocols/rdp/user.c @@ -17,7 +17,6 @@ * under the License. */ -#include "argv.h" #include "channels/audio-input/audio-input.h" #include "channels/cliprdr.h" #include "channels/pipe-svc.h" @@ -34,6 +33,7 @@ #include "sftp.h" #endif +#include #include #include #include @@ -119,7 +119,7 @@ int guac_rdp_user_join_handler(guac_user* user, int argc, char** argv) { user->pipe_handler = guac_rdp_pipe_svc_pipe_handler; /* Handler for updating parameters during connection. */ - user->argv_handler = guac_rdp_argv_handler; + user->argv_handler = guac_argv_handler; } diff --git a/src/protocols/vnc/argv.c b/src/protocols/vnc/argv.c index cdd75f65..e59b1de7 100644 --- a/src/protocols/vnc/argv.c +++ b/src/protocols/vnc/argv.c @@ -29,156 +29,25 @@ #include #include -/** - * All VNC connection settings which may be updated by unprivileged users - * through "argv" streams. - */ -typedef enum guac_vnc_argv_setting { - - /** - * The username for the connection. - */ - GUAC_VNC_ARGV_SETTING_USERNAME, - - /** - * The password for the connection. - */ - GUAC_VNC_ARGV_SETTING_PASSWORD - -} guac_vnc_argv_setting; - -/** - * The value or current status of a connection parameter received over an - * "argv" stream. - */ -typedef struct guac_vnc_argv { - - /** - * The specific setting being updated. - */ - guac_vnc_argv_setting setting; - - /** - * Buffer space for containing the received argument value. - */ - char buffer[GUAC_VNC_ARGV_MAX_LENGTH]; - - /** - * The number of bytes received so far. - */ - int length; - -} guac_vnc_argv; - -/** - * Handler for "blob" instructions which appends the data from received blobs - * to the end of the in-progress argument value buffer. - * - * @see guac_user_blob_handler - */ -static int guac_vnc_argv_blob_handler(guac_user* user, guac_stream* stream, - void* data, int length) { - - guac_vnc_argv* argv = (guac_vnc_argv*) stream->data; - - /* Calculate buffer size remaining, including space for null terminator, - * adjusting received length accordingly */ - int remaining = sizeof(argv->buffer) - argv->length - 1; - if (length > remaining) - length = remaining; - - /* Append received data to end of buffer */ - memcpy(argv->buffer + argv->length, data, length); - argv->length += length; - - return 0; - -} - -/** - * Handler for "end" instructions which applies the changes specified by the - * argument value buffer associated with the stream. - * - * @see guac_user_end_handler - */ -static int guac_vnc_argv_end_handler(guac_user* user, guac_stream* stream) { +int guac_vnc_argv_callback(guac_user* user, const char* mimetype, + const char* name, const char* value, void* data) { guac_client* client = user->client; guac_vnc_client* vnc_client = (guac_vnc_client*) client->data; guac_vnc_settings* settings = vnc_client->settings; - /* Append null terminator to value */ - guac_vnc_argv* argv = (guac_vnc_argv*) stream->data; - argv->buffer[argv->length] = '\0'; - - /* Apply changes to chosen setting */ - switch (argv->setting) { - - /* Update username */ - case GUAC_VNC_ARGV_SETTING_USERNAME: - - /* Update username in settings. */ - free(settings->username); - settings->username = strndup(argv->buffer, argv->length); - - /* Remove the username conditional flag. */ - vnc_client->vnc_credential_flags &= ~GUAC_VNC_COND_FLAG_USERNAME; - break; - - /* Update password */ - case GUAC_VNC_ARGV_SETTING_PASSWORD: - - /* Update password in settings */ - free(settings->password); - settings->password = strndup(argv->buffer, argv->length); - - /* Remove the password conditional flag. */ - vnc_client->vnc_credential_flags &= ~GUAC_VNC_COND_FLAG_PASSWORD; - break; - + /* Update username */ + if (strcmp(name, GUAC_VNC_ARGV_USERNAME) == 0) { + free(settings->username); + settings->username = strdup(value); } - /* If no flags are set, signal the conditional. */ - if (!vnc_client->vnc_credential_flags) - pthread_cond_broadcast(&(vnc_client->vnc_credential_cond)); - - free(argv); - return 0; - -} - -int guac_vnc_argv_handler(guac_user* user, guac_stream* stream, char* mimetype, - char* name) { - - guac_vnc_argv_setting setting; - - /* Allow users to update authentication information */ - if (strcmp(name, GUAC_VNC_ARGV_USERNAME) == 0) - setting = GUAC_VNC_ARGV_SETTING_USERNAME; - else if (strcmp(name, GUAC_VNC_ARGV_PASSWORD) == 0) - setting = GUAC_VNC_ARGV_SETTING_PASSWORD; - - /* No other connection parameters may be updated */ - else { - guac_protocol_send_ack(user->socket, stream, "Not allowed.", - GUAC_PROTOCOL_STATUS_CLIENT_FORBIDDEN); - guac_socket_flush(user->socket); - return 0; + /* Update password */ + else if (strcmp(name, GUAC_VNC_ARGV_PASSWORD) == 0) { + free(settings->password); + settings->password = strdup(value); } - guac_vnc_argv* argv = malloc(sizeof(guac_vnc_argv)); - argv->setting = setting; - argv->length = 0; - - /* Prepare stream to receive argument value */ - stream->blob_handler = guac_vnc_argv_blob_handler; - stream->end_handler = guac_vnc_argv_end_handler; - stream->data = argv; - - /* Signal stream is ready */ - guac_protocol_send_ack(user->socket, stream, "Ready for updated " - "parameter.", GUAC_PROTOCOL_STATUS_SUCCESS); - guac_socket_flush(user->socket); return 0; -} +} \ No newline at end of file diff --git a/src/protocols/vnc/argv.h b/src/protocols/vnc/argv.h index c19262ee..67777820 100644 --- a/src/protocols/vnc/argv.h +++ b/src/protocols/vnc/argv.h @@ -22,20 +22,14 @@ #include "config.h" +#include #include - /** - * The maximum number of bytes to allow for any argument value received via an - * argv stream, including null terminator. + * Handles a received argument value from a Guacamole "argv" instruction, + * updating the given connection parameter. */ -#define GUAC_VNC_ARGV_MAX_LENGTH 16384 - -/** - * Handles an incoming stream from a Guacamole "argv" instruction, updating the - * given connection parameter if that parameter is allowed to be updated. - */ -guac_user_argv_handler guac_vnc_argv_handler; +guac_argv_callback guac_vnc_argv_callback; /** * The name of the parameter Guacamole will use to specify/update the username diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index 13f3cfc7..12f8361e 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -23,6 +23,7 @@ #include "auth.h" #include "vnc.h" +#include #include #include #include @@ -44,22 +45,17 @@ char* guac_vnc_get_password(rfbClient* client) { /* If password isn't around, prompt for it. */ if (settings->password == NULL || strcmp(settings->password, "") == 0) { - /* Lock the thread. */ - pthread_mutex_lock(&(vnc_client->vnc_credential_lock)); + + guac_argv_register(GUAC_VNC_ARGV_PASSWORD, guac_vnc_argv_callback, NULL, 0); + + const char* params[] = {GUAC_VNC_ARGV_PASSWORD, NULL}; /* Send the request for password to the owner. */ - guac_client_owner_send_required(gc, - (const char* []) {GUAC_VNC_ARGV_PASSWORD, NULL}); - - /* Set the conditional flag. */ - vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_PASSWORD; - - /* Wait for the condition. */ - pthread_cond_wait(&(vnc_client->vnc_credential_cond), - &(vnc_client->vnc_credential_lock)); - - /* Unlock the thread. */ - pthread_mutex_unlock(&(vnc_client->vnc_credential_lock)); + guac_client_owner_send_required(gc, params); + + /* Wait for the arguments to be returned */ + guac_argv_await(params); + } return settings->password; @@ -88,36 +84,26 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { /* Check if username is null or empty. */ if (settings->username == NULL) { + guac_argv_register(GUAC_VNC_ARGV_USERNAME, guac_vnc_argv_callback, NULL, 0); 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) { + guac_argv_register(GUAC_VNC_ARGV_PASSWORD, guac_vnc_argv_callback, NULL, 0); params[i] = GUAC_VNC_ARGV_PASSWORD; i++; - vnc_client->vnc_credential_flags |= GUAC_VNC_COND_FLAG_PASSWORD; } /* If we have empty parameters, request them. */ if (i > 0) { - /* Lock the thread. */ - pthread_mutex_lock(&(vnc_client->vnc_credential_lock)); /* Send required parameters to owner. */ guac_client_owner_send_required(gc, (const char**) params); /* Wait for the parameters to be returned. */ - pthread_cond_wait(&(vnc_client->vnc_credential_cond), - &(vnc_client->vnc_credential_lock)); - - /* Pull the credentials from updated settings. */ - creds->userCredential.username = settings->username; - creds->userCredential.password = settings->password; - - /* Unlock the thread. */ - pthread_mutex_unlock(&(vnc_client->vnc_credential_lock)); + guac_argv_await((const char**) params); return creds; diff --git a/src/protocols/vnc/client.c b/src/protocols/vnc/client.c index d5613570..93371ee6 100644 --- a/src/protocols/vnc/client.c +++ b/src/protocols/vnc/client.c @@ -53,11 +53,6 @@ int guac_client_init(guac_client* client) { /* Initialize the TLS write lock */ 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_cond_init(&vnc_client->vnc_credential_cond, NULL); - vnc_client->vnc_credential_flags = 0; /* Init clipboard */ vnc_client->clipboard = guac_common_clipboard_alloc(GUAC_VNC_CLIPBOARD_MAX_LENGTH); @@ -140,10 +135,6 @@ int guac_vnc_client_free_handler(guac_client* client) { /* Clean up TLS lock mutex. */ pthread_mutex_destroy(&(vnc_client->tls_lock)); #endif - - /* Clean up credential mutex */ - pthread_cond_destroy(&vnc_client->vnc_credential_cond); - pthread_mutex_destroy(&vnc_client->vnc_credential_lock); /* Free generic data struct */ free(client->data); diff --git a/src/protocols/vnc/user.c b/src/protocols/vnc/user.c index a3d38d79..807cd0e8 100644 --- a/src/protocols/vnc/user.c +++ b/src/protocols/vnc/user.c @@ -32,6 +32,7 @@ #include "pulse/pulse.h" #endif +#include #include #include #include @@ -98,6 +99,9 @@ int guac_vnc_user_join_handler(guac_user* user, int argc, char** argv) { /* Inbound (client to server) clipboard transfer */ if (!settings->disable_paste) user->clipboard_handler = guac_vnc_clipboard_handler; + + /* Updates to connection parameters */ + user->argv_handler = guac_argv_handler; #ifdef ENABLE_COMMON_SSH /* Set generic (non-filesystem) file upload handler */ diff --git a/src/protocols/vnc/vnc.h b/src/protocols/vnc/vnc.h index ff1e5486..7c189cfb 100644 --- a/src/protocols/vnc/vnc.h +++ b/src/protocols/vnc/vnc.h @@ -45,16 +45,6 @@ #include -/** - * A flag for tracking the status of requesting username from client. - */ -#define GUAC_VNC_COND_FLAG_USERNAME 1 - -/** - * A flag for tracking the status of requesting password from client. - */ -#define GUAC_VNC_COND_FLAG_PASSWORD 2 - /** * VNC-specific client data. */ @@ -143,30 +133,6 @@ typedef struct guac_vnc_client { * Clipboard encoding-specific writer. */ guac_iconv_write* clipboard_writer; - - /** - * A lock that controls access to updating credentials when connecting - * to a VNC server. - */ - pthread_mutex_t vnc_credential_lock; - - /** - * A condition for signaling the thread when credentials have been - * retrieved from the client. - */ - pthread_cond_t vnc_credential_cond; - - /** - * A field to track flags related to retrieving required credentials - * 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_PASSWORD - */ - unsigned vnc_credential_flags; } guac_vnc_client; From 6605f217c561dee699b919915f2a359df5cc59b0 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 15 Sep 2020 14:50:56 -0400 Subject: [PATCH 22/26] GUACAMOLE-221: Rollback changes to SSH protocol prompting --- src/common-ssh/common-ssh/ssh.h | 10 +-- src/common-ssh/ssh.c | 9 +-- src/protocols/ssh/argv.c | 26 +----- src/protocols/ssh/argv.h | 18 ----- src/protocols/ssh/client.c | 9 +-- src/protocols/ssh/settings.c | 6 +- src/protocols/ssh/ssh.c | 136 ++++++++------------------------ src/protocols/ssh/ssh.h | 12 --- 8 files changed, 45 insertions(+), 181 deletions(-) diff --git a/src/common-ssh/common-ssh/ssh.h b/src/common-ssh/common-ssh/ssh.h index 336c4b54..346d8abc 100644 --- a/src/common-ssh/common-ssh/ssh.h +++ b/src/common-ssh/common-ssh/ssh.h @@ -26,19 +26,19 @@ #include /** - * A handler function for retrieving additional credentials from the client. + * Handler for retrieving additional credentials. * * @param client * The Guacamole Client associated with this need for additional * credentials. * * @param cred_name - * The connection parameter that is being requested from the client. + * The name of the credential being requested, which will be shared + * with the client in order to generate a meaningful prompt. * * @return - * A newly-allocated string containing the credentials to be requested from - * the client, or NULL if the credentials will be updated via the required - * instruction. + * A newly-allocated string containing the credentials provided by + * the user, which must be freed by a call to free(). */ typedef char* guac_ssh_credential_handler(guac_client* client, char* cred_name); diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index af3711f3..c03e984d 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -359,13 +359,8 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) } /* Attempt authentication with username + password. */ - if (user->password == NULL && common_session->credential_handler) { - - char* password = common_session->credential_handler(client, "password"); - if (password != NULL) - guac_common_ssh_user_set_password(user, password); - - } + if (user->password == NULL && common_session->credential_handler) + user->password = common_session->credential_handler(client, "Password: "); /* Authenticate with password, if provided */ if (user->password != NULL) { diff --git a/src/protocols/ssh/argv.c b/src/protocols/ssh/argv.c index c6921565..9b3cf653 100644 --- a/src/protocols/ssh/argv.c +++ b/src/protocols/ssh/argv.c @@ -18,10 +18,7 @@ */ #include "config.h" - #include "argv.h" -#include "common-ssh/user.h" -#include "settings.h" #include "ssh.h" #include "terminal/terminal.h" @@ -39,30 +36,9 @@ 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 */ - else if (strcmp(name, GUAC_SSH_ARGV_COLOR_SCHEME) == 0) + if (strcmp(name, GUAC_SSH_ARGV_COLOR_SCHEME) == 0) guac_terminal_apply_color_scheme(terminal, value); /* Update font name */ diff --git a/src/protocols/ssh/argv.h b/src/protocols/ssh/argv.h index 0a726c10..4cbdb4c8 100644 --- a/src/protocols/ssh/argv.h +++ b/src/protocols/ssh/argv.h @@ -44,24 +44,6 @@ */ #define GUAC_SSH_ARGV_FONT_SIZE "font-size" -/** - * The name of the parameter that specifies/updates the username used by the - * connection. - */ -#define GUAC_SSH_ARGV_USERNAME "username" - -/** - * The name of the parameter that specifies/updates the password used by the - * connection. - */ -#define GUAC_SSH_ARGV_PASSWORD "password" - -/** - * The name of the parameter that specifies/updates the private key passphrase - * used by the connection. - */ -#define GUAC_SSH_ARGV_PASSPHRASE "passphrase" - /** * Handles a received argument value from a Guacamole "argv" instruction, * updating the given connection parameter. diff --git a/src/protocols/ssh/client.c b/src/protocols/ssh/client.c index 2a4ec5c9..1bd6647e 100644 --- a/src/protocols/ssh/client.c +++ b/src/protocols/ssh/client.c @@ -47,15 +47,12 @@ int guac_client_init(guac_client* client) { /* Init clipboard */ ssh_client->clipboard = guac_common_clipboard_alloc(GUAC_SSH_CLIPBOARD_MAX_LENGTH); - + /* Set handlers */ client->join_handler = guac_ssh_user_join_handler; 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); @@ -123,10 +120,6 @@ int guac_ssh_client_free_handler(guac_client* client) { guac_common_clipboard_free(ssh_client->clipboard); 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(); return 0; } diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index f287c901..242e1d29 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -36,8 +36,8 @@ const char* GUAC_SSH_CLIENT_ARGS[] = { "hostname", "host-key", "port", - GUAC_SSH_ARGV_USERNAME, - GUAC_SSH_ARGV_PASSWORD, + "username", + "password", GUAC_SSH_ARGV_FONT_NAME, GUAC_SSH_ARGV_FONT_SIZE, "enable-sftp", @@ -45,7 +45,7 @@ const char* GUAC_SSH_CLIENT_ARGS[] = { "sftp-disable-download", "sftp-disable-upload", "private-key", - GUAC_SSH_ARGV_PASSPHRASE, + "passphrase", #ifdef ENABLE_SSH_AGENT "enable-agent", #endif diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 1a9e2717..03f5b0b6 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -57,92 +57,6 @@ #include #include -/** - * 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_ARGV_USERNAME, "Login as: " }, - { GUAC_SSH_ARGV_PASSWORD, "Password: " }, - { GUAC_SSH_ARGV_PASSPHRASE, "Key passphrase: " }, - { NULL, NULL} -}; - -/** - * Prompts the user for the credential specified in the cred_name parameter in - * order to continue a SSH connection. If the owner of the connection is running - * 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 - * where additional credentials are required. - * - * @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) { - - guac_ssh_client* ssh_client = (guac_ssh_client*) client->data; - - /* If client owner does not support required, use terminal prompt. */ - if (!guac_client_owner_supports_required(client)) { - - /* Loop to find correct prompt for credential name. */ - guac_ssh_credential_prompt_map* current = ssh_credential_prompt_map; - while (current->name != NULL) { - if (strcmp(current->name, cred_name) == 0) - return guac_terminal_prompt(ssh_client->term, - current->prompt, true); - current++; - } - - /* No matching credential was found, so return NULL. */ - return NULL; - } - - /* 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. */ - guac_client_owner_send_required(client, (const char* []) {cred_name, NULL}); - - /* Wait for the response, and then unlock the thread. */ - pthread_cond_wait(&(ssh_client->ssh_credential_cond), &(ssh_client->ssh_credential_lock)); - pthread_mutex_unlock(&(ssh_client->ssh_credential_lock)); - - return NULL; - -} - /** * Produces a new user object containing a username and password or private * key, prompting the user as necessary to obtain that information. @@ -163,14 +77,10 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { guac_common_ssh_user* user; /* Get username */ - if (settings->username == NULL) { - - char* username = guac_ssh_get_credential(client, GUAC_SSH_ARGV_USERNAME); - if (username != NULL) - settings->username = username; + if (settings->username == NULL) + settings->username = guac_terminal_prompt(ssh_client->term, + "Login as: ", true); - } - /* Create user object from username */ user = guac_common_ssh_create_user(settings->username); @@ -193,13 +103,10 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { "Re-attempting private key import (WITH passphrase)"); /* Prompt for passphrase if missing */ - if (settings->key_passphrase == NULL) { - - char* passphrase = guac_ssh_get_credential(client, GUAC_SSH_ARGV_PASSPHRASE); - if (passphrase != NULL) - settings->key_passphrase = passphrase; - - } + if (settings->key_passphrase == NULL) + settings->key_passphrase = + guac_terminal_prompt(ssh_client->term, + "Key passphrase: ", false); /* Reattempt import with passphrase */ if (guac_common_ssh_user_import_key(user, @@ -237,6 +144,29 @@ static guac_common_ssh_user* guac_ssh_get_user(guac_client* client) { } +/** + * A function used to generate a terminal prompt to gather additional + * credentials from the guac_client during a connection, and using + * the specified string to generate the prompt for the user. + * + * @param client + * The guac_client object associated with the current connection + * where additional credentials are required. + * + * @param cred_name + * The prompt text to display to the screen when prompting for the + * additional credentials. + * + * @return + * The string of credentials gathered from the user. + */ +static char* guac_ssh_get_credential(guac_client *client, char* cred_name) { + + guac_ssh_client* ssh_client = (guac_ssh_client*) client->data; + return guac_terminal_prompt(ssh_client->term, cred_name, false); + +} + void* ssh_input_thread(void* data) { guac_client* client = (guac_client*) data; @@ -338,6 +268,9 @@ void* ssh_client_thread(void* data) { return NULL; } + /* Ensure connection is kept alive during lengthy connects */ + guac_socket_require_keep_alive(client->socket); + /* Open SSH session */ ssh_client->session = guac_common_ssh_create_session(client, settings->hostname, settings->port, ssh_client->user, settings->server_alive_interval, @@ -348,8 +281,6 @@ 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); /* Open channel for terminal */ ssh_client->term_channel = @@ -564,7 +495,6 @@ void* ssh_client_thread(void* data) { guac_client_stop(client); pthread_join(input_thread, NULL); - 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."); diff --git a/src/protocols/ssh/ssh.h b/src/protocols/ssh/ssh.h index b86ea48f..cb5d8bc5 100644 --- a/src/protocols/ssh/ssh.h +++ b/src/protocols/ssh/ssh.h @@ -89,18 +89,6 @@ typedef struct guac_ssh_client { * Lock dictating access to the SSH terminal channel. */ pthread_mutex_t term_channel_lock; - - /** - * Lock that controls access to updating credential parameters for the - * SSH connection. - */ - pthread_mutex_t ssh_credential_lock; - - /** - * 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; /** * The current clipboard contents. From bfb54f72a0ebafee2f8dd761e455371b13713720 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Wed, 16 Sep 2020 20:49:19 -0400 Subject: [PATCH 23/26] GUACAMOLE-221: Clean up libguac, protocol changes, and documentation. --- src/libguac/client.c | 34 ++++----- src/libguac/guacamole/protocol-types.h | 4 +- src/libguac/guacamole/protocol.h | 12 +-- src/libguac/guacamole/string.h | 13 ++++ src/libguac/protocol.c | 9 ++- src/libguac/string.c | 11 +++ .../tests/protocol/guac_protocol_version.c | 18 ++--- src/protocols/rdp/rdp.c | 18 +++-- src/protocols/rdp/settings.c | 50 ++++--------- src/protocols/rdp/user.c | 5 +- src/protocols/vnc/auth.c | 74 ++++++++++--------- src/protocols/vnc/user.c | 5 +- 12 files changed, 130 insertions(+), 123 deletions(-) diff --git a/src/libguac/client.c b/src/libguac/client.c index 4be0d4de..9738c0c2 100644 --- a/src/libguac/client.c +++ b/src/libguac/client.c @@ -492,15 +492,17 @@ int guac_client_load_plugin(guac_client* client, const char* protocol) { * passed on to the owner to continue the connection. * * @return - * A pointer to an integer containing the return status of the send - * operation. + * Zero if the operation succeeds or non-zero on failure, cast as a void*. */ static void* guac_client_owner_send_required_callback(guac_user* user, void* data) { const char** required = (const char **) data; /* Send required parameters to owner. */ - return (void*) ((intptr_t) guac_protocol_send_required(user->socket, required)); + if (user != NULL) + return (void*) ((intptr_t) guac_protocol_send_required(user->socket, required)); + + return (void*) ((intptr_t) -1); } @@ -672,38 +674,30 @@ static void* __webp_support_callback(guac_user* user, void* data) { /** * 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. + * returning zero if the user does not support the instruction or non-zero if + * the user supports it. * * @param user * The guac_user that will be checked for "required" instruction support. * * @param data - * 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. + * Data provided to the callback. This value is never used within this + * callback. * * @return - * Always NULL. + * A non-zero integer if the provided user who owns the connection supports + * the "required" instruction, or zero if the user does not. The integer + * is cast as a void*. */ static void* guac_owner_supports_required_callback(guac_user* user, void* data) { - int* required_supported = (int *) data; - - /* Check if owner supports required */ - if (*required_supported) - *required_supported = guac_user_supports_required(user); - - return NULL; + return (void*) ((intptr_t) guac_user_supports_required(user)); } int guac_client_owner_supports_required(guac_client* client) { - int required_supported = 1; - - guac_client_for_owner(client, guac_owner_supports_required_callback, &required_supported); - - return required_supported; + return (int) ((intptr_t) guac_client_for_owner(client, guac_owner_supports_required_callback, NULL)); } diff --git a/src/libguac/guacamole/protocol-types.h b/src/libguac/guacamole/protocol-types.h index d492fe7f..51652d2b 100644 --- a/src/libguac/guacamole/protocol-types.h +++ b/src/libguac/guacamole/protocol-types.h @@ -277,8 +277,8 @@ typedef enum guac_line_join_style { } guac_line_join_style; /** - * The set of protocol versions known to guacd to help negotiate features that - * may not be supported by various versions of the client. + * The set of protocol versions known to guacd to handle negotiation or feature + * support between differing versions of Guacamole clients and guacd. */ typedef enum guac_protocol_version { diff --git a/src/libguac/guacamole/protocol.h b/src/libguac/guacamole/protocol.h index c4c6f211..c6dbed29 100644 --- a/src/libguac/guacamole/protocol.h +++ b/src/libguac/guacamole/protocol.h @@ -797,7 +797,7 @@ int guac_protocol_send_rect(guac_socket* socket, const guac_layer* layer, /** * Sends a "required" instruction over the given guac_socket connection. This * instruction indicates to the client that one or more additional parameters - * is needed to continue the connection. + * are needed to continue the connection. * * @param socket * The guac_socket connection to which to send the instruction. @@ -1024,18 +1024,18 @@ int guac_protocol_send_name(guac_socket* socket, const char* name); int guac_protocol_decode_base64(char* base64); /** - * Given a string representation of a protocol version, search the mapping - * to find the matching enum version, or GUAC_PROTOCOL_VERSION_UNKNOWN if no - * match is found. + * Given a string representation of a protocol version, return the enum value of + * that protocol version, or GUAC_PROTOCOL_VERSION_UNKNOWN if the value is not a + * known version. * * @param version_string * The string representation of the protocol version. * * @return * The enum value of the protocol version, or GUAC_PROTOCOL_VERSION_UNKNOWN - * if no match is found. + * if the provided version is not known. */ -guac_protocol_version guac_protocol_string_to_version(char* version_string); +guac_protocol_version guac_protocol_string_to_version(const char* version_string); /** * Given the enum value of the protocol version, return a pointer to the string diff --git a/src/libguac/guacamole/string.h b/src/libguac/guacamole/string.h index 5e56e330..a648c0c2 100644 --- a/src/libguac/guacamole/string.h +++ b/src/libguac/guacamole/string.h @@ -109,6 +109,19 @@ size_t guac_strlcpy(char* restrict dest, const char* restrict src, size_t n); */ size_t guac_strlcat(char* restrict dest, const char* restrict src, size_t n); +/** + * Simple wrapper for strdup() which behaves identically to standard strdup(), + * except that NULL will be returned if the provided string is NULL. + * + * @param str + * The string to duplicate as a newly-allocated string. + * + * @return + * A newly-allocated string containing identically the same content as the + * given string, or NULL if the given string was NULL. + */ +char* guac_strdup(const char* str); + /** * Concatenates each of the given strings, separated by the given delimiter, * storing the result within a destination buffer. The number of bytes written diff --git a/src/libguac/protocol.c b/src/libguac/protocol.c index 8002c92a..59a46d5b 100644 --- a/src/libguac/protocol.c +++ b/src/libguac/protocol.c @@ -1302,7 +1302,7 @@ int guac_protocol_decode_base64(char* base64) { } -guac_protocol_version guac_protocol_string_to_version(char* version_string) { +guac_protocol_version guac_protocol_string_to_version(const char* version_string) { guac_protocol_version_mapping* current = guac_protocol_version_table; while (current->version != GUAC_PROTOCOL_VERSION_UNKNOWN) { @@ -1321,11 +1321,12 @@ guac_protocol_version guac_protocol_string_to_version(char* version_string) { const char* guac_protocol_version_to_string(guac_protocol_version version) { guac_protocol_version_mapping* current = guac_protocol_version_table; - while (current->version) { + while (current->version != GUAC_PROTOCOL_VERSION_UNKNOWN) { - if (current->version == version) { + if (current->version == version) return (const char*) current->version_string; - } + + current++; } diff --git a/src/libguac/string.c b/src/libguac/string.c index f05c4c06..e75f7c18 100644 --- a/src/libguac/string.c +++ b/src/libguac/string.c @@ -81,6 +81,17 @@ size_t guac_strlcat(char* restrict dest, const char* restrict src, size_t n) { } +char* guac_strdup(const char* str) { + + /* Return NULL if no string provided */ + if (str == NULL) + return NULL; + + /* Otherwise just invoke strdup() */ + return strdup(str); + +} + size_t guac_strljoin(char* restrict dest, const char* restrict const* elements, int nmemb, const char* restrict delim, size_t n) { diff --git a/src/libguac/tests/protocol/guac_protocol_version.c b/src/libguac/tests/protocol/guac_protocol_version.c index 782b0aa4..37f42e0f 100644 --- a/src/libguac/tests/protocol/guac_protocol_version.c +++ b/src/libguac/tests/protocol/guac_protocol_version.c @@ -31,9 +31,9 @@ void test_guac_protocol__version_to_string() { guac_protocol_version version_b = GUAC_PROTOCOL_VERSION_1_0_0; guac_protocol_version version_c = GUAC_PROTOCOL_VERSION_UNKNOWN; - CU_ASSERT_STRING_EQUAL_FATAL(guac_protocol_version_to_string(version_a), "VERSION_1_3_0"); - CU_ASSERT_STRING_EQUAL_FATAL(guac_protocol_version_to_string(version_b), "VERSION_1_0_0"); - CU_ASSERT_PTR_NULL_FATAL(guac_protocol_version_to_string(version_c)); + CU_ASSERT_STRING_EQUAL(guac_protocol_version_to_string(version_a), "VERSION_1_3_0"); + CU_ASSERT_STRING_EQUAL(guac_protocol_version_to_string(version_b), "VERSION_1_0_0"); + CU_ASSERT_PTR_NULL(guac_protocol_version_to_string(version_c)); } @@ -50,10 +50,10 @@ void test_guac_protocol__string_to_version() { char* str_version_c = "AVACADO"; char* str_version_d = "VERSION_31_4_1"; - CU_ASSERT_EQUAL_FATAL(guac_protocol_string_to_version(str_version_a), GUAC_PROTOCOL_VERSION_1_3_0); - CU_ASSERT_EQUAL_FATAL(guac_protocol_string_to_version(str_version_b), GUAC_PROTOCOL_VERSION_1_1_0); - CU_ASSERT_EQUAL_FATAL(guac_protocol_string_to_version(str_version_c), GUAC_PROTOCOL_VERSION_UNKNOWN); - CU_ASSERT_EQUAL_FATAL(guac_protocol_string_to_version(str_version_d), GUAC_PROTOCOL_VERSION_UNKNOWN); + CU_ASSERT_EQUAL(guac_protocol_string_to_version(str_version_a), GUAC_PROTOCOL_VERSION_1_3_0); + CU_ASSERT_EQUAL(guac_protocol_string_to_version(str_version_b), GUAC_PROTOCOL_VERSION_1_1_0); + CU_ASSERT_EQUAL(guac_protocol_string_to_version(str_version_c), GUAC_PROTOCOL_VERSION_UNKNOWN); + CU_ASSERT_EQUAL(guac_protocol_string_to_version(str_version_d), GUAC_PROTOCOL_VERSION_UNKNOWN); } @@ -63,7 +63,7 @@ void test_guac_protocol__string_to_version() { */ void test_gauc_protocol__version_comparison() { - CU_ASSERT_TRUE_FATAL(GUAC_PROTOCOL_VERSION_1_3_0 > GUAC_PROTOCOL_VERSION_1_0_0); - CU_ASSERT_TRUE_FATAL(GUAC_PROTOCOL_VERSION_UNKNOWN < GUAC_PROTOCOL_VERSION_1_1_0); + CU_ASSERT_TRUE(GUAC_PROTOCOL_VERSION_1_3_0 > GUAC_PROTOCOL_VERSION_1_0_0); + CU_ASSERT_TRUE(GUAC_PROTOCOL_VERSION_UNKNOWN < GUAC_PROTOCOL_VERSION_1_1_0); } \ No newline at end of file diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 8bf1f22e..6c80abcd 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -43,6 +43,7 @@ #include "pointer.h" #include "print-job.h" #include "rdp.h" +#include "settings.h" #ifdef ENABLE_COMMON_SSH #include "common-ssh/sftp.h" @@ -70,6 +71,7 @@ #include #include #include +#include #include #include #include @@ -236,7 +238,7 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, guac_client* client = ((rdp_freerdp_context*) context)->client; guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; guac_rdp_settings* settings = rdp_client->settings; - char* params[4] = {}; + char* params[4] = {NULL}; int i = 0; /* If the client does not support the "required" instruction, warn and @@ -275,15 +277,17 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, if (i > 0) { - /* Send required parameters to the owner. */ + /* Send required parameters to the owner and wait for the response. */ guac_client_owner_send_required(client, (const char**) params); - guac_argv_await((const char**) params); - /* Get new values from settings. */ - *username = settings->username; - *password = settings->password; - *domain = settings->domain; + /* Free old values and get new values from settings. */ + free(*username); + free(*password); + free(*domain); + *username = guac_strdup(settings->username); + *password = guac_strdup(settings->password); + *domain = guac_strdup(settings->domain); } diff --git a/src/protocols/rdp/settings.c b/src/protocols/rdp/settings.c index bac8af0b..57760d93 100644 --- a/src/protocols/rdp/settings.c +++ b/src/protocols/rdp/settings.c @@ -1267,47 +1267,25 @@ static int guac_rdp_get_performance_flags(guac_rdp_settings* guac_settings) { } -/** - * Simple wrapper for strdup() which behaves identically to standard strdup(), - * execpt that NULL will be returned if the provided string is NULL. - * - * @param str - * The string to duplicate as a newly-allocated string. - * - * @return - * A newly-allocated string containing identically the same content as the - * given string, or NULL if the given string was NULL. - */ -static char* guac_rdp_strdup(const char* str) { - - /* Return NULL if no string provided */ - if (str == NULL) - return NULL; - - /* Otherwise just invoke strdup() */ - return strdup(str); - -} - void guac_rdp_push_settings(guac_client* client, guac_rdp_settings* guac_settings, freerdp* rdp) { rdpSettings* rdp_settings = rdp->settings; /* Authentication */ - rdp_settings->Domain = guac_rdp_strdup(guac_settings->domain); - rdp_settings->Username = guac_rdp_strdup(guac_settings->username); - rdp_settings->Password = guac_rdp_strdup(guac_settings->password); + rdp_settings->Domain = guac_strdup(guac_settings->domain); + rdp_settings->Username = guac_strdup(guac_settings->username); + rdp_settings->Password = guac_strdup(guac_settings->password); /* Connection */ - rdp_settings->ServerHostname = guac_rdp_strdup(guac_settings->hostname); + rdp_settings->ServerHostname = guac_strdup(guac_settings->hostname); rdp_settings->ServerPort = guac_settings->port; /* Session */ rdp_settings->ColorDepth = guac_settings->color_depth; rdp_settings->DesktopWidth = guac_settings->width; rdp_settings->DesktopHeight = guac_settings->height; - rdp_settings->AlternateShell = guac_rdp_strdup(guac_settings->initial_program); + rdp_settings->AlternateShell = guac_strdup(guac_settings->initial_program); rdp_settings->KeyboardLayout = guac_settings->server_layout->freerdp_keyboard_layout; /* Performance flags */ @@ -1425,9 +1403,9 @@ void guac_rdp_push_settings(guac_client* client, rdp_settings->Workarea = TRUE; rdp_settings->RemoteApplicationMode = TRUE; rdp_settings->RemoteAppLanguageBarSupported = TRUE; - rdp_settings->RemoteApplicationProgram = guac_rdp_strdup(guac_settings->remote_app); - rdp_settings->ShellWorkingDirectory = guac_rdp_strdup(guac_settings->remote_app_dir); - rdp_settings->RemoteApplicationCmdLine = guac_rdp_strdup(guac_settings->remote_app_args); + rdp_settings->RemoteApplicationProgram = guac_strdup(guac_settings->remote_app); + rdp_settings->ShellWorkingDirectory = guac_strdup(guac_settings->remote_app_dir); + rdp_settings->RemoteApplicationCmdLine = guac_strdup(guac_settings->remote_app_args); } /* Preconnection ID */ @@ -1441,7 +1419,7 @@ void guac_rdp_push_settings(guac_client* client, if (guac_settings->preconnection_blob != NULL) { rdp_settings->NegotiateSecurityLayer = FALSE; rdp_settings->SendPreconnectionPdu = TRUE; - rdp_settings->PreconnectionBlob = guac_rdp_strdup(guac_settings->preconnection_blob); + rdp_settings->PreconnectionBlob = guac_strdup(guac_settings->preconnection_blob); } /* Enable use of RD gateway if a gateway hostname is provided */ @@ -1451,20 +1429,20 @@ void guac_rdp_push_settings(guac_client* client, rdp_settings->GatewayEnabled = TRUE; /* RD gateway connection details */ - rdp_settings->GatewayHostname = guac_rdp_strdup(guac_settings->gateway_hostname); + rdp_settings->GatewayHostname = guac_strdup(guac_settings->gateway_hostname); rdp_settings->GatewayPort = guac_settings->gateway_port; /* RD gateway credentials */ rdp_settings->GatewayUseSameCredentials = FALSE; - rdp_settings->GatewayDomain = guac_rdp_strdup(guac_settings->gateway_domain); - rdp_settings->GatewayUsername = guac_rdp_strdup(guac_settings->gateway_username); - rdp_settings->GatewayPassword = guac_rdp_strdup(guac_settings->gateway_password); + rdp_settings->GatewayDomain = guac_strdup(guac_settings->gateway_domain); + rdp_settings->GatewayUsername = guac_strdup(guac_settings->gateway_username); + rdp_settings->GatewayPassword = guac_strdup(guac_settings->gateway_password); } /* Store load balance info (and calculate length) if provided */ if (guac_settings->load_balance_info != NULL) { - rdp_settings->LoadBalanceInfo = (BYTE*) guac_rdp_strdup(guac_settings->load_balance_info); + rdp_settings->LoadBalanceInfo = (BYTE*) guac_strdup(guac_settings->load_balance_info); rdp_settings->LoadBalanceInfoLength = strlen(guac_settings->load_balance_info); } diff --git a/src/protocols/rdp/user.c b/src/protocols/rdp/user.c index e7a91d77..351c67e6 100644 --- a/src/protocols/rdp/user.c +++ b/src/protocols/rdp/user.c @@ -118,8 +118,9 @@ int guac_rdp_user_join_handler(guac_user* user, int argc, char** argv) { /* Inbound arbitrary named pipes */ user->pipe_handler = guac_rdp_pipe_svc_pipe_handler; - /* Handler for updating parameters during connection. */ - user->argv_handler = guac_argv_handler; + /* If we own it, register handler for updating parameters during connection. */ + if (user->owner) + user->argv_handler = guac_argv_handler; } diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index 12f8361e..b5a74b15 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -63,6 +63,7 @@ char* guac_vnc_get_password(rfbClient* client) { } rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { + guac_client* gc = rfbClientGetClientData(client, GUAC_VNC_CLIENT_KEY); guac_vnc_client* vnc_client = ((guac_vnc_client*) gc->data); guac_vnc_settings* settings = vnc_client->settings; @@ -71,43 +72,46 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { if (credentialType == rfbCredentialTypeUser) { rfbCredential *creds = malloc(sizeof(rfbCredential)); - /* If the client does not support the "required" instruction, just return - the parameters already configured. */ - if (!guac_client_owner_supports_required(gc)) { - creds->userCredential.username = settings->username; - creds->userCredential.password = settings->password; - return creds; + /* If the client supports the "required" instruction, prompt for and + update those. */ + if (guac_client_owner_supports_required(gc)) { + char* params[3] = {NULL}; + int i = 0; + + /* Check if username is null or empty. */ + if (settings->username == NULL) { + guac_argv_register(GUAC_VNC_ARGV_USERNAME, guac_vnc_argv_callback, NULL, 0); + params[i] = GUAC_VNC_ARGV_USERNAME; + i++; + } + + /* Check if password is null or empty. */ + if (settings->password == NULL) { + guac_argv_register(GUAC_VNC_ARGV_PASSWORD, guac_vnc_argv_callback, NULL, 0); + params[i] = GUAC_VNC_ARGV_PASSWORD; + i++; + } + + params[i] = NULL; + + /* If we have empty parameters, request them. */ + if (i > 0) { + + /* Send required parameters to owner. */ + guac_client_owner_send_required(gc, (const char**) params); + + /* Wait for the parameters to be returned. */ + guac_argv_await((const char**) params); + + return creds; + + } } - char* params[2] = {NULL}; - int i = 0; - - /* Check if username is null or empty. */ - if (settings->username == NULL) { - guac_argv_register(GUAC_VNC_ARGV_USERNAME, guac_vnc_argv_callback, NULL, 0); - params[i] = GUAC_VNC_ARGV_USERNAME; - i++; - } - - /* Check if password is null or empty. */ - if (settings->password == NULL) { - guac_argv_register(GUAC_VNC_ARGV_PASSWORD, guac_vnc_argv_callback, NULL, 0); - params[i] = GUAC_VNC_ARGV_PASSWORD; - i++; - } - - /* If we have empty parameters, request them. */ - if (i > 0) { - - /* Send required parameters to owner. */ - guac_client_owner_send_required(gc, (const char**) params); - - /* Wait for the parameters to be returned. */ - guac_argv_await((const char**) params); - - return creds; - - } + /* Copy the values and return the credential set. */ + creds->userCredential.username = strdup(settings->username); + creds->userCredential.password = strdup(settings->password); + return creds; } diff --git a/src/protocols/vnc/user.c b/src/protocols/vnc/user.c index 807cd0e8..4e6f473c 100644 --- a/src/protocols/vnc/user.c +++ b/src/protocols/vnc/user.c @@ -100,8 +100,9 @@ int guac_vnc_user_join_handler(guac_user* user, int argc, char** argv) { if (!settings->disable_paste) user->clipboard_handler = guac_vnc_clipboard_handler; - /* Updates to connection parameters */ - user->argv_handler = guac_argv_handler; + /* Updates to connection parameters if we own the connection */ + if (user->owner) + user->argv_handler = guac_argv_handler; #ifdef ENABLE_COMMON_SSH /* Set generic (non-filesystem) file upload handler */ From 3b4007c9fa5ce11bdd974443b8bbd4fbd04117c7 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 18 Sep 2020 22:04:51 -0400 Subject: [PATCH 24/26] GUACAMOLE-221: Tweak logic for when RDP domain is requested. --- src/protocols/rdp/rdp.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 6c80abcd..b31be494 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -256,6 +256,14 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0); params[i] = GUAC_RDP_ARGV_USERNAME; i++; + + /* If username is undefined and domain is also undefined, request domain. */ + if (settings->domain == NULL) { + guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0); + params[i] = GUAC_RDP_ARGV_DOMAIN; + i++; + } + } /* If the password is undefined, add it to the requested parameters. */ @@ -265,13 +273,6 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username, i++; } - /* If the domain is undefined, add it to the requested parameters. */ - if (settings->domain == NULL) { - guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0); - params[i] = GUAC_RDP_ARGV_DOMAIN; - i++; - } - /* NULL-terminate the array. */ params[i] = NULL; From 9e1dada14b059a9edf2cd6ce0986fa4365457a9f Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 18 Sep 2020 22:15:36 -0400 Subject: [PATCH 25/26] GUACAMOLE-221: Add CUnit tests for guac_strdup() --- src/libguac/tests/Makefile.am | 1 + src/libguac/tests/string/strdup.c | 49 +++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 src/libguac/tests/string/strdup.c diff --git a/src/libguac/tests/Makefile.am b/src/libguac/tests/Makefile.am index 6fd3798f..7ee55948 100644 --- a/src/libguac/tests/Makefile.am +++ b/src/libguac/tests/Makefile.am @@ -43,6 +43,7 @@ test_libguac_SOURCES = \ protocol/guac_protocol_version.c \ socket/fd_send_instruction.c \ socket/nested_send_instruction.c \ + string/strdup.c \ string/strlcat.c \ string/strlcpy.c \ string/strljoin.c \ diff --git a/src/libguac/tests/string/strdup.c b/src/libguac/tests/string/strdup.c new file mode 100644 index 00000000..0b1e76b3 --- /dev/null +++ b/src/libguac/tests/string/strdup.c @@ -0,0 +1,49 @@ +/* + * 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. + */ + +#include +#include + +#include +#include + +/** + * Source test string for copying. + */ +const char* source_string = "Mashing avocados."; + +/** + * A NULL string variable for copying to insure that NULL is copied properly. + */ +const char* null_string = NULL; + +/** + * Verify guac_strdup() behavior when the string is both NULL and not NULL. + */ +void test_string__strdup() { + + /* Copy the strings. */ + char* dest_string = guac_strdup(source_string); + char* null_copy = guac_strdup(null_string); + + /* Run the tests. */ + CU_ASSERT_STRING_EQUAL(dest_string, "Mashing avocados."); + CU_ASSERT_PTR_NULL(null_copy); + +} \ No newline at end of file From 3e19583b296539edc42a90b714e1ed15e55c28f9 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 18 Sep 2020 22:23:03 -0400 Subject: [PATCH 26/26] GUACAMOLE-221: Switch VNC credentials to NULL when parameter is not passed --- src/protocols/vnc/auth.c | 24 +++++++++--------------- src/protocols/vnc/settings.c | 6 ++++-- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/protocols/vnc/auth.c b/src/protocols/vnc/auth.c index b5a74b15..c26e4d2e 100644 --- a/src/protocols/vnc/auth.c +++ b/src/protocols/vnc/auth.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -41,10 +42,10 @@ char* guac_vnc_get_password(rfbClient* client) { /* If the client does not support the "required" instruction, just return the configuration data. */ if (!guac_client_owner_supports_required(gc)) - return ((guac_vnc_client*) gc->data)->settings->password; + return guac_strdup(settings->password); /* If password isn't around, prompt for it. */ - if (settings->password == NULL || strcmp(settings->password, "") == 0) { + if (settings->password == NULL) { guac_argv_register(GUAC_VNC_ARGV_PASSWORD, guac_vnc_argv_callback, NULL, 0); @@ -58,7 +59,7 @@ char* guac_vnc_get_password(rfbClient* client) { } - return settings->password; + return guac_strdup(settings->password); } @@ -78,14 +79,14 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { char* params[3] = {NULL}; int i = 0; - /* Check if username is null or empty. */ + /* Check if username is not provided. */ if (settings->username == NULL) { guac_argv_register(GUAC_VNC_ARGV_USERNAME, guac_vnc_argv_callback, NULL, 0); params[i] = GUAC_VNC_ARGV_USERNAME; i++; } - /* Check if password is null or empty. */ + /* Check if password is not provided. */ if (settings->password == NULL) { guac_argv_register(GUAC_VNC_ARGV_PASSWORD, guac_vnc_argv_callback, NULL, 0); params[i] = GUAC_VNC_ARGV_PASSWORD; @@ -94,23 +95,16 @@ rfbCredential* guac_vnc_get_credentials(rfbClient* client, int credentialType) { params[i] = NULL; - /* If we have empty parameters, request them. */ + /* If we have empty parameters, request them and await response. */ if (i > 0) { - - /* Send required parameters to owner. */ guac_client_owner_send_required(gc, (const char**) params); - - /* Wait for the parameters to be returned. */ guac_argv_await((const char**) params); - - return creds; - } } /* Copy the values and return the credential set. */ - creds->userCredential.username = strdup(settings->username); - creds->userCredential.password = strdup(settings->password); + creds->userCredential.username = guac_strdup(settings->username); + creds->userCredential.password = guac_strdup(settings->password); return creds; } diff --git a/src/protocols/vnc/settings.c b/src/protocols/vnc/settings.c index 86207c47..c6f36464 100644 --- a/src/protocols/vnc/settings.c +++ b/src/protocols/vnc/settings.c @@ -393,11 +393,11 @@ guac_vnc_settings* guac_vnc_parse_args(guac_user* user, settings->username = guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, - IDX_USERNAME, ""); /* NOTE: freed by libvncclient */ + IDX_USERNAME, NULL); settings->password = guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, - IDX_PASSWORD, ""); /* NOTE: freed by libvncclient */ + IDX_PASSWORD, NULL); /* Remote cursor */ if (strcmp(argv[IDX_CURSOR], "remote") == 0) { @@ -625,8 +625,10 @@ void guac_vnc_settings_free(guac_vnc_settings* settings) { free(settings->clipboard_encoding); free(settings->encodings); free(settings->hostname); + free(settings->password); free(settings->recording_name); free(settings->recording_path); + free(settings->username); #ifdef ENABLE_VNC_REPEATER /* Free VNC repeater settings */