From f1bf70a4a20dcbf5264b559b94e7c8e9d839b525 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 22 Feb 2018 14:56:40 -0500 Subject: [PATCH 01/22] GUACAMOLE-269: Add basic support for sending TTY mode encoding. --- src/protocols/ssh/Makefile.am | 1 + src/protocols/ssh/ssh.c | 5 +++-- src/protocols/ssh/ttymode.h | 36 +++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 src/protocols/ssh/ttymode.h diff --git a/src/protocols/ssh/Makefile.am b/src/protocols/ssh/Makefile.am index 3a5c7f73..7306b8bc 100644 --- a/src/protocols/ssh/Makefile.am +++ b/src/protocols/ssh/Makefile.am @@ -38,6 +38,7 @@ noinst_HEADERS = \ settings.h \ sftp.h \ ssh.h \ + ttymode.h \ user.h # Add agent sources if enabled diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 0ea60bc4..88614feb 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -26,6 +26,7 @@ #include "sftp.h" #include "ssh.h" #include "terminal/terminal.h" +#include "ttymode.h" #ifdef ENABLE_SSH_AGENT #include "ssh_agent.h" @@ -297,8 +298,8 @@ void* ssh_client_thread(void* data) { } /* Request PTY */ - if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, NULL, 0, - ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { + if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, guac_tty_modes, + sizeof(guac_tty_modes), ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Unable to allocate PTY."); return NULL; } diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h new file mode 100644 index 00000000..f248ed82 --- /dev/null +++ b/src/protocols/ssh/ttymode.h @@ -0,0 +1,36 @@ +/* + * 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_SSH_TTYMODE_H +#define GUAC_SSH_TTYMODE_H + +#include "config.h" + +#define TTY_OP_END 0 +#define TTY_OP_VERASE 3 + +#define GUAC_SSH_TERM_DEFAULT_BACKSPACE 127 + +char guac_tty_modes[] = { + TTY_OP_VERASE, + 0, 0, 0, GUAC_SSH_TERM_DEFAULT_BACKSPACE, + TTY_OP_END +}; + +#endif From 5583748b548c8d4a9be5865251d956c290e1338a Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 22 Feb 2018 19:56:05 -0500 Subject: [PATCH 02/22] GUACAMOLE-269: Move constant declaration to ttymode.c --- src/protocols/ssh/Makefile.am | 1 + src/protocols/ssh/ssh.c | 4 ++-- src/protocols/ssh/ttymode.c | 27 +++++++++++++++++++++++++++ src/protocols/ssh/ttymode.h | 10 +++------- 4 files changed, 33 insertions(+), 9 deletions(-) create mode 100644 src/protocols/ssh/ttymode.c diff --git a/src/protocols/ssh/Makefile.am b/src/protocols/ssh/Makefile.am index 7306b8bc..6a6de034 100644 --- a/src/protocols/ssh/Makefile.am +++ b/src/protocols/ssh/Makefile.am @@ -29,6 +29,7 @@ libguac_client_ssh_la_SOURCES = \ settings.c \ sftp.c \ ssh.c \ + ttymode.c \ user.c noinst_HEADERS = \ diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 88614feb..62bfbb25 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -298,8 +298,8 @@ void* ssh_client_thread(void* data) { } /* Request PTY */ - if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, guac_tty_modes, - sizeof(guac_tty_modes), ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { + if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, GUAC_SSH_TTY_MODES, + sizeof(GUAC_SSH_TTY_MODES), ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Unable to allocate PTY."); return NULL; } diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c new file mode 100644 index 00000000..e5baff4f --- /dev/null +++ b/src/protocols/ssh/ttymode.c @@ -0,0 +1,27 @@ +/* + * 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 "ttymode.h" + +const char GUAC_SSH_TTY_MODES[6] = { + GUAC_SSH_TTY_OP_VERASE, + 0, 0, 0, GUAC_SSH_TERM_DEFAULT_BACKSPACE, + GUAC_SSH_TTY_OP_END +}; diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index f248ed82..524051e6 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -22,15 +22,11 @@ #include "config.h" -#define TTY_OP_END 0 -#define TTY_OP_VERASE 3 +#define GUAC_SSH_TTY_OP_END 0 +#define GUAC_SSH_TTY_OP_VERASE 3 #define GUAC_SSH_TERM_DEFAULT_BACKSPACE 127 -char guac_tty_modes[] = { - TTY_OP_VERASE, - 0, 0, 0, GUAC_SSH_TERM_DEFAULT_BACKSPACE, - TTY_OP_END -}; +extern const char GUAC_SSH_TTY_MODES[6]; #endif From 2ace9385a27d77e3fbf1f987b3bf3f0ce6af7ebc Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 22 Feb 2018 20:10:16 -0500 Subject: [PATCH 03/22] GUACAMOLE-269: Add documentation for the defines and variables. --- src/protocols/ssh/ttymode.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index 524051e6..e1a0101f 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -22,11 +22,31 @@ #include "config.h" +/** + * The SSH TTY mode encoding opcode that terminates + * the list of TTY modes. + */ #define GUAC_SSH_TTY_OP_END 0 + +/** + * The SSH tty mode encoding opcode that configures + * the TTY erase code to configure the server + * backspace key. + */ #define GUAC_SSH_TTY_OP_VERASE 3 +/** + * The default ASCII code to send for the backspace + * key that will be sent to the SSH server. + */ #define GUAC_SSH_TERM_DEFAULT_BACKSPACE 127 +/** + * The array of TTY mode encoding data to send to the + * SSH server. These consist of pairs of byte codes + * and uint32 (4-byte) values, with a 0 to terminate + * the list. + */ extern const char GUAC_SSH_TTY_MODES[6]; #endif From 46e908c06eae26d419f72bd3bd071287ca4c02f6 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 23 Feb 2018 22:14:11 -0500 Subject: [PATCH 04/22] GUACAMOLE-269: Allow backspace key to be configured. --- src/protocols/ssh/settings.c | 13 +++++++++++++ src/protocols/ssh/settings.h | 5 +++++ src/protocols/ssh/ssh.c | 2 +- src/protocols/telnet/settings.c | 11 +++++++++++ src/protocols/telnet/settings.h | 7 +++++++ src/protocols/telnet/telnet.c | 2 +- src/terminal/terminal.c | 9 +++++++-- src/terminal/terminal/terminal.h | 12 +++++++++++- 8 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 7c803eaa..89b6e82b 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -56,6 +56,7 @@ const char* GUAC_SSH_CLIENT_ARGS[] = { "create-recording-path", "read-only", "server-alive-interval", + "backspace", NULL }; @@ -209,6 +210,13 @@ enum SSH_ARGS_IDX { */ IDX_SERVER_ALIVE_INTERVAL, + /** + * The ASCII code, in decimal, to send for the backspace key, as configured + * by the SSH connection from the client. By default this will be 0x7f, + * the ASCII DELETE code. + */ + IDX_BACKSPACE, + SSH_ARGS_COUNT }; @@ -348,6 +356,11 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user, guac_user_parse_args_int(user, GUAC_SSH_CLIENT_ARGS, argv, IDX_SERVER_ALIVE_INTERVAL, 0); + /* Parse backspace key setting */ + settings->backspace = + guac_user_parse_args_int(user, GUAC_SSH_CLIENT_ARGS, argv, + IDX_BACKSPACE, 127); + /* Parsing was successful */ return settings; diff --git a/src/protocols/ssh/settings.h b/src/protocols/ssh/settings.h index 689d4258..65f60fa8 100644 --- a/src/protocols/ssh/settings.h +++ b/src/protocols/ssh/settings.h @@ -223,6 +223,11 @@ typedef struct guac_ssh_settings { */ int server_alive_interval; + /** + * The decismal ASCII code of the command to send for backspace. + */ + int backspace; + } guac_ssh_settings; /** diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 62bfbb25..86ebddbc 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -207,7 +207,7 @@ void* ssh_client_thread(void* data) { ssh_client->term = guac_terminal_create(client, settings->font_name, settings->font_size, settings->resolution, settings->width, settings->height, - settings->color_scheme); + settings->color_scheme, settings->backspace); /* Fail if terminal init failed */ if (ssh_client->term == NULL) { diff --git a/src/protocols/telnet/settings.c b/src/protocols/telnet/settings.c index 082cff48..acb1f905 100644 --- a/src/protocols/telnet/settings.c +++ b/src/protocols/telnet/settings.c @@ -50,6 +50,7 @@ const char* GUAC_TELNET_CLIENT_ARGS[] = { "recording-include-keys", "create-recording-path", "read-only", + "backspace", NULL }; @@ -174,6 +175,11 @@ enum TELNET_ARGS_IDX { */ IDX_READ_ONLY, + /** + * ASCII code to use for the backspace key, or 127 if not specified. + */ + IDX_BACKSPACE, + TELNET_ARGS_COUNT }; @@ -328,6 +334,11 @@ guac_telnet_settings* guac_telnet_parse_args(guac_user* user, guac_user_parse_args_boolean(user, GUAC_TELNET_CLIENT_ARGS, argv, IDX_CREATE_RECORDING_PATH, false); + /* Parse backspae key code */ + settings->backspace = + guac_user_parse_args_int(user, GUAC_TELNET_CLIENT_ARGS, argv, + IDX_BACKSPACE, 127); + /* Parsing was successful */ return settings; diff --git a/src/protocols/telnet/settings.h b/src/protocols/telnet/settings.h index 11761c63..71c807a3 100644 --- a/src/protocols/telnet/settings.h +++ b/src/protocols/telnet/settings.h @@ -207,6 +207,13 @@ typedef struct guac_telnet_settings { */ bool recording_include_keys; + /** + * The ASCII code, in decimal, that the telnet client will use when the + * backspace key is pressed. By default, this is 0x7f, ASCII delete, + * but can be configured in the client settings. + */ + int backspace; + } guac_telnet_settings; /** diff --git a/src/protocols/telnet/telnet.c b/src/protocols/telnet/telnet.c index 2a4000d8..040d10be 100644 --- a/src/protocols/telnet/telnet.c +++ b/src/protocols/telnet/telnet.c @@ -480,7 +480,7 @@ void* guac_telnet_client_thread(void* data) { telnet_client->term = guac_terminal_create(client, settings->font_name, settings->font_size, settings->resolution, settings->width, settings->height, - settings->color_scheme); + settings->color_scheme, settings->backspace); /* Fail if terminal init failed */ if (telnet_client->term == NULL) { diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c index 4d2171a0..0dae5ccd 100644 --- a/src/terminal/terminal.c +++ b/src/terminal/terminal.c @@ -257,7 +257,8 @@ void* guac_terminal_thread(void* data) { guac_terminal* guac_terminal_create(guac_client* client, const char* font_name, int font_size, int dpi, - int width, int height, const char* color_scheme) { + int width, int height, const char* color_scheme, + const int backspace) { int default_foreground; int default_background; @@ -406,6 +407,10 @@ guac_terminal* guac_terminal_create(guac_client* client, return NULL; } + /* Configure backspace */ + term->backspace = backspace; + guac_client_log(client, GUAC_LOG_DEBUG, "Backspace has been set to %d", term->backspace); + return term; } @@ -1594,7 +1599,7 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed /* Non-printable keys */ else { - if (keysym == 0xFF08) return guac_terminal_send_string(term, "\x7F"); /* Backspace */ + if (keysym == 0xFF08) return guac_terminal_send_string(term, &term->backspace); /* Backspace */ if (keysym == 0xFF09 || keysym == 0xFF89) return guac_terminal_send_string(term, "\x09"); /* Tab */ if (keysym == 0xFF0D || keysym == 0xFF8D) return guac_terminal_send_string(term, "\x0D"); /* Enter */ if (keysym == 0xFF1B) return guac_terminal_send_string(term, "\x1B"); /* Esc */ diff --git a/src/terminal/terminal/terminal.h b/src/terminal/terminal/terminal.h index 37b07484..90cf06ac 100644 --- a/src/terminal/terminal/terminal.h +++ b/src/terminal/terminal/terminal.h @@ -432,6 +432,11 @@ struct guac_terminal { */ guac_common_clipboard* clipboard; + /** + * Hexidecimal ASCII code sent when backspace is pressed. + */ + char backspace; + }; /** @@ -464,13 +469,18 @@ struct guac_terminal { * invalid, a warning will be logged, and the terminal will fall back on * GUAC_TERMINAL_SCHEME_GRAY_BLACK. * + * @param backspace + * The decimal ASCII code to send when backspace is pressed in + * this terminal. + * * @return * A new guac_terminal having the given font, dimensions, and attributes * which renders all text to the given client. */ guac_terminal* guac_terminal_create(guac_client* client, const char* font_name, int font_size, int dpi, - int width, int height, const char* color_scheme); + int width, int height, const char* color_scheme, + const int backspace); /** * Frees all resources associated with the given terminal. From fd58d31eeae1857f33d3d679da782b465c289026 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sun, 25 Feb 2018 08:41:14 -0500 Subject: [PATCH 05/22] GUACAMOLE-269: Use backspace config to set up tty modes. --- src/protocols/ssh/ssh.c | 10 ++++-- src/protocols/ssh/ttymode.c | 61 ++++++++++++++++++++++++++++++++++--- src/protocols/ssh/ttymode.h | 52 ++++++++++++++++++++++++++----- 3 files changed, 108 insertions(+), 15 deletions(-) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 86ebddbc..6b90a197 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -192,6 +192,9 @@ void* ssh_client_thread(void* data) { return NULL; } + /* Initialize a ttymode array */ + guac_ssh_ttymodes ssh_ttymodes = init_ttymodes(); + /* Set up screen recording, if requested */ if (settings->recording_path != NULL) { ssh_client->recording = guac_common_recording_create(client, @@ -209,6 +212,9 @@ void* ssh_client_thread(void* data) { settings->resolution, settings->width, settings->height, settings->color_scheme, settings->backspace); + /* Add the backspace key to the ttymode array */ + add_ttymode(&ssh_ttymodes, GUAC_SSH_TTY_OP_VERASE, settings->backspace); + /* Fail if terminal init failed */ if (ssh_client->term == NULL) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, @@ -298,8 +304,8 @@ void* ssh_client_thread(void* data) { } /* Request PTY */ - if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, GUAC_SSH_TTY_MODES, - sizeof(GUAC_SSH_TTY_MODES), ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { + if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, ttymodes_to_array(&ssh_ttymodes), + sizeof_ttymodes(&ssh_ttymodes), ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Unable to allocate PTY."); return NULL; } diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index e5baff4f..286f124f 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -20,8 +20,59 @@ #include "config.h" #include "ttymode.h" -const char GUAC_SSH_TTY_MODES[6] = { - GUAC_SSH_TTY_OP_VERASE, - 0, 0, 0, GUAC_SSH_TERM_DEFAULT_BACKSPACE, - GUAC_SSH_TTY_OP_END -}; +#include +#include + +guac_ssh_ttymodes init_ttymodes() { + // Simple allocation for a placeholder + guac_ssh_ttymode* ttymode_array = malloc(1); + + // Set up the initial data structure. + guac_ssh_ttymodes empty_modes = { + ttymode_array, + 0 + }; + + return empty_modes; +} + +void add_ttymode(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value) { + tty_modes->num_opcodes++; + tty_modes->ttymode_array = realloc(tty_modes->ttymode_array, sizeof(guac_ssh_ttymode) * tty_modes->num_opcodes); + tty_modes->ttymode_array[tty_modes->num_opcodes - 1].opcode = opcode; + tty_modes->ttymode_array[tty_modes->num_opcodes - 1].value = value; + +} + +int sizeof_ttymodes(guac_ssh_ttymodes *tty_modes) { + // Each opcode pair is 5 bytes (1 opcode, 4 value) + // Add one for the ending opcode + return (tty_modes->num_opcodes * 5) + 1; +} + +char* ttymodes_to_array(guac_ssh_ttymodes *tty_modes) { + // Total data size should be number of tracked opcodes + // plus one final byte for the TTY_OP_END code. + int data_size = (tty_modes->num_opcodes * 5) + 1; + char* temp = malloc(data_size); + + // Loop through the array based on number of tracked + // opcodes and convert each one. + for (int i = 0; i < tty_modes->num_opcodes; i++) { + int idx = i * 5; + uint32_t value = tty_modes->ttymode_array[i].value; + // Opcode goes in first byte. + temp[idx] = tty_modes->ttymode_array[i].opcode; + + // Convert 32-bit int to individual bytes. + temp[idx+1] = (value >> 24) & 0xFF; + temp[idx+2] = (value >> 16) & 0xFF; + temp[idx+3] = (value >> 8) & 0xFF; + temp[idx+4] = value & 0xFF; + } + + // Add the ending opcode + temp[data_size - 1] = GUAC_SSH_TTY_OP_END; + + return temp; +} diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index e1a0101f..93f85bf1 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -22,6 +22,8 @@ #include "config.h" +#include + /** * The SSH TTY mode encoding opcode that terminates * the list of TTY modes. @@ -36,17 +38,51 @@ #define GUAC_SSH_TTY_OP_VERASE 3 /** - * The default ASCII code to send for the backspace - * key that will be sent to the SSH server. + * A data type which holds a single opcode + * and the value for that opcode. */ -#define GUAC_SSH_TERM_DEFAULT_BACKSPACE 127 +typedef struct guac_ssh_ttymode { + char opcode; + uint32_t value; +} guac_ssh_ttymode; /** - * The array of TTY mode encoding data to send to the - * SSH server. These consist of pairs of byte codes - * and uint32 (4-byte) values, with a 0 to terminate - * the list. + * A data type which holds an array of + * guac_ssh_ttymode data, along with the count of + * the number of opcodes currently in the array. */ -extern const char GUAC_SSH_TTY_MODES[6]; +typedef struct guac_ssh_ttymodes { + guac_ssh_ttymode* ttymode_array; + int num_opcodes; +} guac_ssh_ttymodes; + + +/** + * Initialize an empty guac_ssh_ttymodes data structure, + * with a null array of guac_ssh_ttymode and opcodes + * set to zero. + */ +guac_ssh_ttymodes init_ttymodes(); + +/** + * Add an item to the opcode array. This resizes the + * array, increments the number of opcodes, and adds + * the specified opcode and value pair to the data + * structure. + */ +void add_ttymode(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value); + +/** + * Retrieve the size, in bytes, of the ttymode_array + * in the given guac_ssh_ttymodes data structure. + */ +int sizeof_ttymodes(guac_ssh_ttymodes *tty_modes); + +/** + * Convert the ttymodes data structure into a char + * pointer array suitable for passing into the + * libssh2_channel_request_pty_ex() function. + */ +char* ttymodes_to_array(guac_ssh_ttymodes *tty_modes); #endif From 9bd28321e5d4b900765539422a9baae917542d1a Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 27 Feb 2018 09:13:01 -0500 Subject: [PATCH 06/22] GUACAMOLE-269: Fix up style in comments. --- src/protocols/ssh/settings.h | 2 +- src/protocols/ssh/ttymode.c | 24 +++++++++++++----------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/protocols/ssh/settings.h b/src/protocols/ssh/settings.h index 65f60fa8..175ece93 100644 --- a/src/protocols/ssh/settings.h +++ b/src/protocols/ssh/settings.h @@ -224,7 +224,7 @@ typedef struct guac_ssh_settings { int server_alive_interval; /** - * The decismal ASCII code of the command to send for backspace. + * The integer ASCII code of the command to send for backspace. */ int backspace; diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index 286f124f..f1e21d3f 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -24,10 +24,10 @@ #include guac_ssh_ttymodes init_ttymodes() { - // Simple allocation for a placeholder + /* Simple allocation for a placeholder */ guac_ssh_ttymode* ttymode_array = malloc(1); - // Set up the initial data structure. + /* Set up the initial data structure. */ guac_ssh_ttymodes empty_modes = { ttymode_array, 0 @@ -45,33 +45,35 @@ void add_ttymode(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value) { } int sizeof_ttymodes(guac_ssh_ttymodes *tty_modes) { - // Each opcode pair is 5 bytes (1 opcode, 4 value) - // Add one for the ending opcode + /* Each opcode pair is 5 bytes (1 opcode, 4 value) + Add one for the ending opcode */ + return (tty_modes->num_opcodes * 5) + 1; } char* ttymodes_to_array(guac_ssh_ttymodes *tty_modes) { - // Total data size should be number of tracked opcodes - // plus one final byte for the TTY_OP_END code. + /* Total data size should be number of tracked opcodes + plus one final byte for the TTY_OP_END code. */ int data_size = (tty_modes->num_opcodes * 5) + 1; char* temp = malloc(data_size); - // Loop through the array based on number of tracked - // opcodes and convert each one. + /* Loop through the array based on number of tracked + opcodes and convert each one. */ for (int i = 0; i < tty_modes->num_opcodes; i++) { int idx = i * 5; uint32_t value = tty_modes->ttymode_array[i].value; - // Opcode goes in first byte. + + /* Opcode goes in first byte. */ temp[idx] = tty_modes->ttymode_array[i].opcode; - // Convert 32-bit int to individual bytes. + /* Convert 32-bit int to individual bytes. */ temp[idx+1] = (value >> 24) & 0xFF; temp[idx+2] = (value >> 16) & 0xFF; temp[idx+3] = (value >> 8) & 0xFF; temp[idx+4] = value & 0xFF; } - // Add the ending opcode + /* Add the ending opcode */ temp[data_size - 1] = GUAC_SSH_TTY_OP_END; return temp; From c286668b797a149c77ae609d654d3e7899a9c36b Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 27 Feb 2018 09:23:30 -0500 Subject: [PATCH 07/22] GUACAMOLE-269: Name functions per Guacamole standards. --- src/protocols/ssh/ssh.c | 9 +++++---- src/protocols/ssh/ttymode.c | 8 ++++---- src/protocols/ssh/ttymode.h | 8 ++++---- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 6b90a197..6aecde0c 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -193,7 +193,7 @@ void* ssh_client_thread(void* data) { } /* Initialize a ttymode array */ - guac_ssh_ttymodes ssh_ttymodes = init_ttymodes(); + guac_ssh_ttymodes ssh_ttymodes = guac_ssh_ttymodes_init(); /* Set up screen recording, if requested */ if (settings->recording_path != NULL) { @@ -213,7 +213,7 @@ void* ssh_client_thread(void* data) { settings->color_scheme, settings->backspace); /* Add the backspace key to the ttymode array */ - add_ttymode(&ssh_ttymodes, GUAC_SSH_TTY_OP_VERASE, settings->backspace); + guac_ssh_ttymodes_add(&ssh_ttymodes, GUAC_SSH_TTY_OP_VERASE, settings->backspace); /* Fail if terminal init failed */ if (ssh_client->term == NULL) { @@ -304,8 +304,9 @@ void* ssh_client_thread(void* data) { } /* Request PTY */ - if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, ttymodes_to_array(&ssh_ttymodes), - sizeof_ttymodes(&ssh_ttymodes), ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { + if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, + guac_ssh_ttymodes_to_array(&ssh_ttymodes), guac_ssh_ttymodes_size(&ssh_ttymodes), + ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Unable to allocate PTY."); return NULL; } diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index f1e21d3f..f21e9f7f 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -23,7 +23,7 @@ #include #include -guac_ssh_ttymodes init_ttymodes() { +guac_ssh_ttymodes guac_ssh_ttymodes_init() { /* Simple allocation for a placeholder */ guac_ssh_ttymode* ttymode_array = malloc(1); @@ -36,7 +36,7 @@ guac_ssh_ttymodes init_ttymodes() { return empty_modes; } -void add_ttymode(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value) { +void guac_ssh_ttymodes_add(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value) { tty_modes->num_opcodes++; tty_modes->ttymode_array = realloc(tty_modes->ttymode_array, sizeof(guac_ssh_ttymode) * tty_modes->num_opcodes); tty_modes->ttymode_array[tty_modes->num_opcodes - 1].opcode = opcode; @@ -44,14 +44,14 @@ void add_ttymode(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value) { } -int sizeof_ttymodes(guac_ssh_ttymodes *tty_modes) { +int guac_ssh_ttymodes_size(guac_ssh_ttymodes *tty_modes) { /* Each opcode pair is 5 bytes (1 opcode, 4 value) Add one for the ending opcode */ return (tty_modes->num_opcodes * 5) + 1; } -char* ttymodes_to_array(guac_ssh_ttymodes *tty_modes) { +char* guac_ssh_ttymodes_to_array(guac_ssh_ttymodes *tty_modes) { /* Total data size should be number of tracked opcodes plus one final byte for the TTY_OP_END code. */ int data_size = (tty_modes->num_opcodes * 5) + 1; diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index 93f85bf1..ca5569d4 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -62,7 +62,7 @@ typedef struct guac_ssh_ttymodes { * with a null array of guac_ssh_ttymode and opcodes * set to zero. */ -guac_ssh_ttymodes init_ttymodes(); +guac_ssh_ttymodes guac_ssh_ttymodes_init(); /** * Add an item to the opcode array. This resizes the @@ -70,19 +70,19 @@ guac_ssh_ttymodes init_ttymodes(); * the specified opcode and value pair to the data * structure. */ -void add_ttymode(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value); +void guac_ssh_ttymodes_add(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value); /** * Retrieve the size, in bytes, of the ttymode_array * in the given guac_ssh_ttymodes data structure. */ -int sizeof_ttymodes(guac_ssh_ttymodes *tty_modes); +int guac_ssh_ttymodes_size(guac_ssh_ttymodes *tty_modes); /** * Convert the ttymodes data structure into a char * pointer array suitable for passing into the * libssh2_channel_request_pty_ex() function. */ -char* ttymodes_to_array(guac_ssh_ttymodes *tty_modes); +char* guac_ssh_ttymodes_to_array(guac_ssh_ttymodes *tty_modes); #endif From dd78d230eae38f8063683f2c9335abdb6af664d3 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 27 Feb 2018 09:35:29 -0500 Subject: [PATCH 08/22] GUACAMOLE-269: Backspace key should send null-terminated string. --- src/terminal/terminal.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c index 0dae5ccd..35810dcc 100644 --- a/src/terminal/terminal.c +++ b/src/terminal/terminal.c @@ -1599,7 +1599,12 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed /* Non-printable keys */ else { - if (keysym == 0xFF08) return guac_terminal_send_string(term, &term->backspace); /* Backspace */ + if (keysym == 0xFF08) { + char* backspace_str = malloc(sizeof(char) * 2); + backspace_str[0] = term->backspace; + backspace_str[1] = '\0'; + return guac_terminal_send_string(term, backspace_str); /* Backspace */ + } if (keysym == 0xFF09 || keysym == 0xFF89) return guac_terminal_send_string(term, "\x09"); /* Tab */ if (keysym == 0xFF0D || keysym == 0xFF8D) return guac_terminal_send_string(term, "\x0D"); /* Enter */ if (keysym == 0xFF1B) return guac_terminal_send_string(term, "\x1B"); /* Esc */ From 33cca463462604669c40b235c6f2cd8371841147 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 27 Feb 2018 09:41:03 -0500 Subject: [PATCH 09/22] GUACAMOLE-269: Remove debug code. --- src/terminal/terminal.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c index 35810dcc..70a2640a 100644 --- a/src/terminal/terminal.c +++ b/src/terminal/terminal.c @@ -409,7 +409,6 @@ guac_terminal* guac_terminal_create(guac_client* client, /* Configure backspace */ term->backspace = backspace; - guac_client_log(client, GUAC_LOG_DEBUG, "Backspace has been set to %d", term->backspace); return term; From 64ca77f3a59ea69ac42812705744ebece235ddae Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 27 Feb 2018 09:59:18 -0500 Subject: [PATCH 10/22] GUACAMOLE-269: Change struct to struct pointer. --- src/protocols/ssh/ssh.c | 6 +++--- src/protocols/ssh/ttymode.c | 9 ++++----- src/protocols/ssh/ttymode.h | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 6aecde0c..eac4eced 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -193,7 +193,7 @@ void* ssh_client_thread(void* data) { } /* Initialize a ttymode array */ - guac_ssh_ttymodes ssh_ttymodes = guac_ssh_ttymodes_init(); + guac_ssh_ttymodes* ssh_ttymodes = guac_ssh_ttymodes_init(); /* Set up screen recording, if requested */ if (settings->recording_path != NULL) { @@ -213,7 +213,7 @@ void* ssh_client_thread(void* data) { settings->color_scheme, settings->backspace); /* Add the backspace key to the ttymode array */ - guac_ssh_ttymodes_add(&ssh_ttymodes, GUAC_SSH_TTY_OP_VERASE, settings->backspace); + guac_ssh_ttymodes_add(ssh_ttymodes, GUAC_SSH_TTY_OP_VERASE, settings->backspace); /* Fail if terminal init failed */ if (ssh_client->term == NULL) { @@ -305,7 +305,7 @@ void* ssh_client_thread(void* data) { /* Request PTY */ if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, - guac_ssh_ttymodes_to_array(&ssh_ttymodes), guac_ssh_ttymodes_size(&ssh_ttymodes), + guac_ssh_ttymodes_to_array(ssh_ttymodes), guac_ssh_ttymodes_size(ssh_ttymodes), ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Unable to allocate PTY."); return NULL; diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index f21e9f7f..02fb54ba 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -23,15 +23,14 @@ #include #include -guac_ssh_ttymodes guac_ssh_ttymodes_init() { +guac_ssh_ttymodes* guac_ssh_ttymodes_init() { /* Simple allocation for a placeholder */ guac_ssh_ttymode* ttymode_array = malloc(1); /* Set up the initial data structure. */ - guac_ssh_ttymodes empty_modes = { - ttymode_array, - 0 - }; + guac_ssh_ttymodes* empty_modes = malloc(sizeof(guac_ssh_ttymodes)); + empty_modes->ttymode_array = ttymode_array; + empty_modes->num_opcodes = 0; return empty_modes; } diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index ca5569d4..ecc57de0 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -62,7 +62,7 @@ typedef struct guac_ssh_ttymodes { * with a null array of guac_ssh_ttymode and opcodes * set to zero. */ -guac_ssh_ttymodes guac_ssh_ttymodes_init(); +guac_ssh_ttymodes* guac_ssh_ttymodes_init(); /** * Add an item to the opcode array. This resizes the From c3e1b2afefb76a12a8c21361c15195e05fecfc84 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 27 Feb 2018 15:49:57 -0500 Subject: [PATCH 11/22] GUACAMOLE-269: Fix minor style issues and update comments. --- src/protocols/ssh/settings.c | 4 ++-- src/protocols/ssh/ttymode.c | 7 +++++-- src/protocols/telnet/settings.c | 3 ++- src/protocols/telnet/settings.h | 6 +++--- src/terminal/terminal.c | 3 ++- src/terminal/terminal/terminal.h | 4 ++-- 6 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 89b6e82b..983f7f08 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -211,8 +211,8 @@ enum SSH_ARGS_IDX { IDX_SERVER_ALIVE_INTERVAL, /** - * The ASCII code, in decimal, to send for the backspace key, as configured - * by the SSH connection from the client. By default this will be 0x7f, + * The ASCII code, as an integer, to send for the backspace key, as configured + * by the SSH connection from the client. By default this will be 127, * the ASCII DELETE code. */ IDX_BACKSPACE, diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index 02fb54ba..39830904 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -36,17 +36,20 @@ guac_ssh_ttymodes* guac_ssh_ttymodes_init() { } void guac_ssh_ttymodes_add(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value) { + /* Increment number of opcodes */ tty_modes->num_opcodes++; + + /* Increase size of the pointer array */ tty_modes->ttymode_array = realloc(tty_modes->ttymode_array, sizeof(guac_ssh_ttymode) * tty_modes->num_opcodes); + + /* Add new values */ tty_modes->ttymode_array[tty_modes->num_opcodes - 1].opcode = opcode; tty_modes->ttymode_array[tty_modes->num_opcodes - 1].value = value; - } int guac_ssh_ttymodes_size(guac_ssh_ttymodes *tty_modes) { /* Each opcode pair is 5 bytes (1 opcode, 4 value) Add one for the ending opcode */ - return (tty_modes->num_opcodes * 5) + 1; } diff --git a/src/protocols/telnet/settings.c b/src/protocols/telnet/settings.c index acb1f905..b49b773e 100644 --- a/src/protocols/telnet/settings.c +++ b/src/protocols/telnet/settings.c @@ -176,7 +176,8 @@ enum TELNET_ARGS_IDX { IDX_READ_ONLY, /** - * ASCII code to use for the backspace key, or 127 if not specified. + * ASCII code, as an integer to use for the backspace key, or 127 + * if not specified. */ IDX_BACKSPACE, diff --git a/src/protocols/telnet/settings.h b/src/protocols/telnet/settings.h index 71c807a3..9bc3dc37 100644 --- a/src/protocols/telnet/settings.h +++ b/src/protocols/telnet/settings.h @@ -208,9 +208,9 @@ typedef struct guac_telnet_settings { bool recording_include_keys; /** - * The ASCII code, in decimal, that the telnet client will use when the - * backspace key is pressed. By default, this is 0x7f, ASCII delete, - * but can be configured in the client settings. + * The ASCII code, as an integer, that the telnet client will use when the + * backspace key is pressed. By default, this is 127, ASCII delete, if + * not specified in the client settings. */ int backspace; diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c index 70a2640a..c19bd835 100644 --- a/src/terminal/terminal.c +++ b/src/terminal/terminal.c @@ -1598,11 +1598,12 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed /* Non-printable keys */ else { + /* Backspace can vary based on configuration of terminal by client. */ if (keysym == 0xFF08) { char* backspace_str = malloc(sizeof(char) * 2); backspace_str[0] = term->backspace; backspace_str[1] = '\0'; - return guac_terminal_send_string(term, backspace_str); /* Backspace */ + return guac_terminal_send_string(term, backspace_str); } if (keysym == 0xFF09 || keysym == 0xFF89) return guac_terminal_send_string(term, "\x09"); /* Tab */ if (keysym == 0xFF0D || keysym == 0xFF8D) return guac_terminal_send_string(term, "\x0D"); /* Enter */ diff --git a/src/terminal/terminal/terminal.h b/src/terminal/terminal/terminal.h index 90cf06ac..c071b402 100644 --- a/src/terminal/terminal/terminal.h +++ b/src/terminal/terminal/terminal.h @@ -433,7 +433,7 @@ struct guac_terminal { guac_common_clipboard* clipboard; /** - * Hexidecimal ASCII code sent when backspace is pressed. + * ASCII character to send when backspace is pressed. */ char backspace; @@ -470,7 +470,7 @@ struct guac_terminal { * GUAC_TERMINAL_SCHEME_GRAY_BLACK. * * @param backspace - * The decimal ASCII code to send when backspace is pressed in + * The integer ASCII code to send when backspace is pressed in * this terminal. * * @return From dd7522bd9fb8c23ea5f64309741b69a2764d20b4 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 3 Mar 2018 21:15:04 -0500 Subject: [PATCH 12/22] GUACAMOLE-269: Get rid of dynamic allocation and properly free up data structures. --- src/protocols/ssh/ssh.c | 14 +++++++++--- src/protocols/ssh/ttymode.c | 15 +++++-------- src/protocols/ssh/ttymode.h | 44 +++++++++++++++++++++++++++++++++++-- 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index eac4eced..35d40134 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -193,7 +193,7 @@ void* ssh_client_thread(void* data) { } /* Initialize a ttymode array */ - guac_ssh_ttymodes* ssh_ttymodes = guac_ssh_ttymodes_init(); + guac_ssh_ttymodes* ssh_ttymodes = guac_ssh_ttymodes_init(1); /* Set up screen recording, if requested */ if (settings->recording_path != NULL) { @@ -303,14 +303,22 @@ void* ssh_client_thread(void* data) { } + /* Get char pointer array for TTY Mode Encoding */ + int ttymode_size = guac_ssh_ttymodes_size(ssh_ttymodes); + char* ttymode_array = guac_ssh_ttymodes_to_array(ssh_ttymodes, ttymode_size); + /* Request PTY */ if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, - guac_ssh_ttymodes_to_array(ssh_ttymodes), guac_ssh_ttymodes_size(ssh_ttymodes), - ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { + ttymode_array, ttymode_size, ssh_client->term->term_width, + ssh_client->term->term_height, 0, 0)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Unable to allocate PTY."); return NULL; } + /* We're done with TTY Mode Encoding, so free structures. */ + free(ttymode_array); + free(ssh_ttymodes); + /* If a command is specified, run that instead of a shell */ if (settings->command != NULL) { if (libssh2_channel_exec(ssh_client->term_channel, settings->command)) { diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index 39830904..1541da68 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -23,9 +23,9 @@ #include #include -guac_ssh_ttymodes* guac_ssh_ttymodes_init() { - /* Simple allocation for a placeholder */ - guac_ssh_ttymode* ttymode_array = malloc(1); +guac_ssh_ttymodes* guac_ssh_ttymodes_init(int max_opcodes) { + /* Allocate enough space for the max opcodes */ + guac_ssh_ttymode* ttymode_array = malloc(sizeof(guac_ssh_ttymode) * max_opcodes); /* Set up the initial data structure. */ guac_ssh_ttymodes* empty_modes = malloc(sizeof(guac_ssh_ttymodes)); @@ -39,9 +39,6 @@ void guac_ssh_ttymodes_add(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t v /* Increment number of opcodes */ tty_modes->num_opcodes++; - /* Increase size of the pointer array */ - tty_modes->ttymode_array = realloc(tty_modes->ttymode_array, sizeof(guac_ssh_ttymode) * tty_modes->num_opcodes); - /* Add new values */ tty_modes->ttymode_array[tty_modes->num_opcodes - 1].opcode = opcode; tty_modes->ttymode_array[tty_modes->num_opcodes - 1].value = value; @@ -53,10 +50,8 @@ int guac_ssh_ttymodes_size(guac_ssh_ttymodes *tty_modes) { return (tty_modes->num_opcodes * 5) + 1; } -char* guac_ssh_ttymodes_to_array(guac_ssh_ttymodes *tty_modes) { - /* Total data size should be number of tracked opcodes - plus one final byte for the TTY_OP_END code. */ - int data_size = (tty_modes->num_opcodes * 5) + 1; +char* guac_ssh_ttymodes_to_array(guac_ssh_ttymodes *tty_modes, int data_size) { + char* temp = malloc(data_size); /* Loop through the array based on number of tracked diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index ecc57de0..30f36e61 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -61,20 +61,47 @@ typedef struct guac_ssh_ttymodes { * Initialize an empty guac_ssh_ttymodes data structure, * with a null array of guac_ssh_ttymode and opcodes * set to zero. + * + * @param max_opcodes + * The maximum number of opcodes that will be allocated. + * + * @return + * The pointer array for the gauc_ssh_ttymodes data + * structure generated by the function. */ -guac_ssh_ttymodes* guac_ssh_ttymodes_init(); +guac_ssh_ttymodes* guac_ssh_ttymodes_init(int max_opcodes); /** * Add an item to the opcode array. This resizes the * array, increments the number of opcodes, and adds * the specified opcode and value pair to the data * structure. + * + * @param tty_modes + * The pointer to the guac_ssh_ttymodes data structure + * to add the opcode to. + * + * @param opcode + * The single byte opcode as documented in section 8 + * of the SSH RFC. + * + * @param value + * The four byte value of the opcodeto add to the + * guac_ssh_ttymodes data structure. */ void guac_ssh_ttymodes_add(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value); /** * Retrieve the size, in bytes, of the ttymode_array * in the given guac_ssh_ttymodes data structure. + * + * @param tty_modes + * The pointer to the guac_ssh_ttymodes structure + * whose size we are curious about. + * + * @return + * The number of bytes of the opcodes and value paris + * in the data structure. */ int guac_ssh_ttymodes_size(guac_ssh_ttymodes *tty_modes); @@ -82,7 +109,20 @@ int guac_ssh_ttymodes_size(guac_ssh_ttymodes *tty_modes); * Convert the ttymodes data structure into a char * pointer array suitable for passing into the * libssh2_channel_request_pty_ex() function. + * + * @param tty_modes + * The pointer to the guac_ssh_ttymodes structure + * that contains the opcode and value pairs + * we want to convert to a character pointer array. + * + * @param data_size + * The size of the resulting character pointer + * array. + * + * @return + * The character pointer array of opcode and + * value pairs to be passed to a SSH session. */ -char* guac_ssh_ttymodes_to_array(guac_ssh_ttymodes *tty_modes); +char* guac_ssh_ttymodes_to_array(guac_ssh_ttymodes *tty_modes, int data_size); #endif From 45b832bfdc7ced13a8aa5a4a090a60dae818dffe Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 8 Mar 2018 09:32:19 -0500 Subject: [PATCH 13/22] GUACAMOLE-269: Remove all dynamic allocation and simplify implementation. --- src/protocols/ssh/ssh.c | 19 +++---- src/protocols/ssh/ttymode.c | 70 +++++++++--------------- src/protocols/ssh/ttymode.h | 104 ++++++++++-------------------------- 3 files changed, 61 insertions(+), 132 deletions(-) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 35d40134..9881c304 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -193,7 +193,8 @@ void* ssh_client_thread(void* data) { } /* Initialize a ttymode array */ - guac_ssh_ttymodes* ssh_ttymodes = guac_ssh_ttymodes_init(1); + const int num_tty_opcodes = 1; + char ssh_ttymodes[(GUAC_SSH_TTY_OPCODE_SIZE * num_tty_opcodes) + 1]; /* Set up screen recording, if requested */ if (settings->recording_path != NULL) { @@ -212,9 +213,6 @@ void* ssh_client_thread(void* data) { settings->resolution, settings->width, settings->height, settings->color_scheme, settings->backspace); - /* Add the backspace key to the ttymode array */ - guac_ssh_ttymodes_add(ssh_ttymodes, GUAC_SSH_TTY_OP_VERASE, settings->backspace); - /* Fail if terminal init failed */ if (ssh_client->term == NULL) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, @@ -303,22 +301,19 @@ void* ssh_client_thread(void* data) { } - /* Get char pointer array for TTY Mode Encoding */ - int ttymode_size = guac_ssh_ttymodes_size(ssh_ttymodes); - char* ttymode_array = guac_ssh_ttymodes_to_array(ssh_ttymodes, ttymode_size); + /* Set up the ttymode array prior to requesting the PTY */ + if (guac_ssh_ttymodes_init(ssh_ttymodes, sizeof(ssh_ttymodes), + num_tty_opcodes, (guac_ssh_ttymode){ GUAC_SSH_TTY_OP_VERASE, settings->backspace})) + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Error configuring TTY mode encoding."); /* Request PTY */ if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, - ttymode_array, ttymode_size, ssh_client->term->term_width, + ssh_ttymodes, sizeof(ssh_ttymodes), ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Unable to allocate PTY."); return NULL; } - /* We're done with TTY Mode Encoding, so free structures. */ - free(ttymode_array); - free(ssh_ttymodes); - /* If a command is specified, run that instead of a shell */ if (settings->command != NULL) { if (libssh2_channel_exec(ssh_client->term_channel, settings->command)) { diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index 1541da68..4c6091e5 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -20,58 +20,40 @@ #include "config.h" #include "ttymode.h" +#include #include #include -guac_ssh_ttymodes* guac_ssh_ttymodes_init(int max_opcodes) { - /* Allocate enough space for the max opcodes */ - guac_ssh_ttymode* ttymode_array = malloc(sizeof(guac_ssh_ttymode) * max_opcodes); - - /* Set up the initial data structure. */ - guac_ssh_ttymodes* empty_modes = malloc(sizeof(guac_ssh_ttymodes)); - empty_modes->ttymode_array = ttymode_array; - empty_modes->num_opcodes = 0; +int guac_ssh_ttymodes_init(char opcode_array[], const int array_size, + const int num_opcodes, ...) { - return empty_modes; -} + /* Initialize the variable argument list. */ + va_list args; + va_start(args, num_opcodes); -void guac_ssh_ttymodes_add(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value) { - /* Increment number of opcodes */ - tty_modes->num_opcodes++; + /* Check to make sure the array has enough space. + We need one extra byte at the end for the ending opcode. */ + if ((num_opcodes * GUAC_SSH_TTY_OPCODE_SIZE) >= (array_size)) + return 1; - /* Add new values */ - tty_modes->ttymode_array[tty_modes->num_opcodes - 1].opcode = opcode; - tty_modes->ttymode_array[tty_modes->num_opcodes - 1].value = value; -} + for (int i = 0; i < num_opcodes; i++) { + /* Calculate offset in array */ + int offset = i * GUAC_SSH_TTY_OPCODE_SIZE; -int guac_ssh_ttymodes_size(guac_ssh_ttymodes *tty_modes) { - /* Each opcode pair is 5 bytes (1 opcode, 4 value) - Add one for the ending opcode */ - return (tty_modes->num_opcodes * 5) + 1; -} + /* Get the next argument to this function */ + guac_ssh_ttymode ttymode = va_arg(args, guac_ssh_ttymode); -char* guac_ssh_ttymodes_to_array(guac_ssh_ttymodes *tty_modes, int data_size) { - - char* temp = malloc(data_size); - - /* Loop through the array based on number of tracked - opcodes and convert each one. */ - for (int i = 0; i < tty_modes->num_opcodes; i++) { - int idx = i * 5; - uint32_t value = tty_modes->ttymode_array[i].value; - - /* Opcode goes in first byte. */ - temp[idx] = tty_modes->ttymode_array[i].opcode; - - /* Convert 32-bit int to individual bytes. */ - temp[idx+1] = (value >> 24) & 0xFF; - temp[idx+2] = (value >> 16) & 0xFF; - temp[idx+3] = (value >> 8) & 0xFF; - temp[idx+4] = value & 0xFF; + /* Place opcode and value in array */ + opcode_array[offset] = ttymode.opcode; + opcode_array[offset + 1] = (ttymode.value >> 24) & 0xFF; + opcode_array[offset + 2] = (ttymode.value >> 16) & 0xFF; + opcode_array[offset + 3] = (ttymode.value >> 8) & 0xFF; + opcode_array[offset + 4] = ttymode.value & 0xFF; } - - /* Add the ending opcode */ - temp[data_size - 1] = GUAC_SSH_TTY_OP_END; - return temp; + /* Put the end opcode in the last opcode space */ + opcode_array[num_opcodes * GUAC_SSH_TTY_OPCODE_SIZE] = GUAC_SSH_TTY_OP_END; + + return 0; + } diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index 30f36e61..be40b84c 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -24,6 +24,14 @@ #include +/** + * The size of a TTY mode encoding opcode and + * value pair. As defined in the SSH RFC, this + * is 5 bytes - a single byte for the opcode, and + * 4 bytes for the value. + */ +#define GUAC_SSH_TTY_OPCODE_SIZE 5 + /** * The SSH TTY mode encoding opcode that terminates * the list of TTY modes. @@ -31,15 +39,15 @@ #define GUAC_SSH_TTY_OP_END 0 /** - * The SSH tty mode encoding opcode that configures + * The SSH TTY mode encoding opcode that configures * the TTY erase code to configure the server * backspace key. */ #define GUAC_SSH_TTY_OP_VERASE 3 /** - * A data type which holds a single opcode - * and the value for that opcode. + * Simple structure that defines a opcode and + * value pair. */ typedef struct guac_ssh_ttymode { char opcode; @@ -47,82 +55,26 @@ typedef struct guac_ssh_ttymode { } guac_ssh_ttymode; /** - * A data type which holds an array of - * guac_ssh_ttymode data, along with the count of - * the number of opcodes currently in the array. - */ -typedef struct guac_ssh_ttymodes { - guac_ssh_ttymode* ttymode_array; - int num_opcodes; -} guac_ssh_ttymodes; - - -/** - * Initialize an empty guac_ssh_ttymodes data structure, - * with a null array of guac_ssh_ttymode and opcodes - * set to zero. + * Initialize an array with the specified opcode/value + * pairs, and return the size, in bytes, of the array. * - * @param max_opcodes - * The maximum number of opcodes that will be allocated. + * @param opcode_array + * Pointer to the opcode array. + * + * @param array_size + * Size, in bytes, of the array. + * + * @param num_opcodes + * Number of opcodes to expect. + * + * @params ... + * A variable number of guac_ssh_ttymodes + * to place in the array. * * @return - * The pointer array for the gauc_ssh_ttymodes data - * structure generated by the function. + * The size, in bytes, of the array. */ -guac_ssh_ttymodes* guac_ssh_ttymodes_init(int max_opcodes); - -/** - * Add an item to the opcode array. This resizes the - * array, increments the number of opcodes, and adds - * the specified opcode and value pair to the data - * structure. - * - * @param tty_modes - * The pointer to the guac_ssh_ttymodes data structure - * to add the opcode to. - * - * @param opcode - * The single byte opcode as documented in section 8 - * of the SSH RFC. - * - * @param value - * The four byte value of the opcodeto add to the - * guac_ssh_ttymodes data structure. - */ -void guac_ssh_ttymodes_add(guac_ssh_ttymodes *tty_modes, char opcode, uint32_t value); - -/** - * Retrieve the size, in bytes, of the ttymode_array - * in the given guac_ssh_ttymodes data structure. - * - * @param tty_modes - * The pointer to the guac_ssh_ttymodes structure - * whose size we are curious about. - * - * @return - * The number of bytes of the opcodes and value paris - * in the data structure. - */ -int guac_ssh_ttymodes_size(guac_ssh_ttymodes *tty_modes); - -/** - * Convert the ttymodes data structure into a char - * pointer array suitable for passing into the - * libssh2_channel_request_pty_ex() function. - * - * @param tty_modes - * The pointer to the guac_ssh_ttymodes structure - * that contains the opcode and value pairs - * we want to convert to a character pointer array. - * - * @param data_size - * The size of the resulting character pointer - * array. - * - * @return - * The character pointer array of opcode and - * value pairs to be passed to a SSH session. - */ -char* guac_ssh_ttymodes_to_array(guac_ssh_ttymodes *tty_modes, int data_size); +int guac_ssh_ttymodes_init(char opcode_array[], const int array_size, + const int num_opcodes, ...); #endif From 86dde85b2d0f63268942a7a9ba01b6d2e3bfbb73 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 24 Mar 2018 14:53:22 -0400 Subject: [PATCH 14/22] GUACAMOLE-269: Comment and spelling updates. --- src/protocols/ssh/ttymode.h | 6 ++++-- src/protocols/telnet/settings.c | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index be40b84c..869d0523 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -56,7 +56,8 @@ typedef struct guac_ssh_ttymode { /** * Initialize an array with the specified opcode/value - * pairs, and return the size, in bytes, of the array. + * pairs, returning 0 if successful, or a non-zero value + * if a failure occurs. * * @param opcode_array * Pointer to the opcode array. @@ -72,7 +73,8 @@ typedef struct guac_ssh_ttymode { * to place in the array. * * @return - * The size, in bytes, of the array. + * Zero of the function is successful, non-zero + * if a failure occurs. */ int guac_ssh_ttymodes_init(char opcode_array[], const int array_size, const int num_opcodes, ...); diff --git a/src/protocols/telnet/settings.c b/src/protocols/telnet/settings.c index b49b773e..8f802915 100644 --- a/src/protocols/telnet/settings.c +++ b/src/protocols/telnet/settings.c @@ -335,7 +335,7 @@ guac_telnet_settings* guac_telnet_parse_args(guac_user* user, guac_user_parse_args_boolean(user, GUAC_TELNET_CLIENT_ARGS, argv, IDX_CREATE_RECORDING_PATH, false); - /* Parse backspae key code */ + /* Parse backspace key code */ settings->backspace = guac_user_parse_args_int(user, GUAC_TELNET_CLIENT_ARGS, argv, IDX_BACKSPACE, 127); From 112ce5299e6534ef0dac6ac8280b0fd7c512eeeb Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 24 Mar 2018 14:54:27 -0400 Subject: [PATCH 15/22] GUACAMOLE-269: Remove unnecessary dynamic allocation. --- src/terminal/terminal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c index c19bd835..bfc36d1a 100644 --- a/src/terminal/terminal.c +++ b/src/terminal/terminal.c @@ -1600,7 +1600,7 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed /* Backspace can vary based on configuration of terminal by client. */ if (keysym == 0xFF08) { - char* backspace_str = malloc(sizeof(char) * 2); + char backspace_str[2]; backspace_str[0] = term->backspace; backspace_str[1] = '\0'; return guac_terminal_send_string(term, backspace_str); From 11136f7d7bd879eb10838506b8d40ca69ad4806b Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 24 Mar 2018 15:09:34 -0400 Subject: [PATCH 16/22] GUACAMOLE-269: More documentation updates. --- src/protocols/ssh/ttymode.h | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index 869d0523..59c5415d 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -46,21 +46,43 @@ #define GUAC_SSH_TTY_OP_VERASE 3 /** - * Simple structure that defines a opcode and - * value pair. + * The SSH protocol attempts to configure the remote + * terminal by sending pairs of opcodes and values, as + * described in Section 8 of RFC 4254. These are + * comprised of a single byte opcode and a 4-byte + * value. This data structure stores a single opcode + * and value pair. */ typedef struct guac_ssh_ttymode { + + /** + * The single byte opcode for defining the TTY + * encoding setting for the remote terminal. The + * stadard codes are defined in Section 8 of + * the SSH RFC (4254). + */ char opcode; + + /** + * The four byte value of the setting for the + * defined opcode. + */ uint32_t value; + } guac_ssh_ttymode; /** - * Initialize an array with the specified opcode/value - * pairs, returning 0 if successful, or a non-zero value - * if a failure occurs. + * Opcodes and value pairs are passed to the SSH connection + * in a single array, beginning with the opcode and followed + * by a four byte value, repeating until the end opcode is + * encountered. This function takes the array, the array + * size, expected number of opcodes, and that number of + * guac_ssh_ttymode arguments and puts them in the array + * exepcted by the SSH connection. * * @param opcode_array - * Pointer to the opcode array. + * Pointer to the opcode array that will ultimately + * be passed to the SSH connection. * * @param array_size * Size, in bytes, of the array. From e16bfd783755fe7cc8019f28f0eb6e997678d8db Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sat, 24 Mar 2018 15:50:11 -0400 Subject: [PATCH 17/22] GAUCAMOLE-269: Memory effeciency updates. --- src/protocols/ssh/ttymode.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index 4c6091e5..4526cc1f 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -36,23 +36,22 @@ int guac_ssh_ttymodes_init(char opcode_array[], const int array_size, if ((num_opcodes * GUAC_SSH_TTY_OPCODE_SIZE) >= (array_size)) return 1; + char *current = opcode_array; for (int i = 0; i < num_opcodes; i++) { - /* Calculate offset in array */ - int offset = i * GUAC_SSH_TTY_OPCODE_SIZE; /* Get the next argument to this function */ - guac_ssh_ttymode ttymode = va_arg(args, guac_ssh_ttymode); + guac_ssh_ttymode* ttymode = va_arg(args, guac_ssh_ttymode*); /* Place opcode and value in array */ - opcode_array[offset] = ttymode.opcode; - opcode_array[offset + 1] = (ttymode.value >> 24) & 0xFF; - opcode_array[offset + 2] = (ttymode.value >> 16) & 0xFF; - opcode_array[offset + 3] = (ttymode.value >> 8) & 0xFF; - opcode_array[offset + 4] = ttymode.value & 0xFF; + *(current++) = ttymode->opcode; + *(current++) = (ttymode->value >> 24) & 0xFF; + *(current++) = (ttymode->value >> 16) & 0xFF; + *(current++) = (ttymode->value >> 8) & 0xFF; + *(current++) = ttymode->value & 0xFF; } /* Put the end opcode in the last opcode space */ - opcode_array[num_opcodes * GUAC_SSH_TTY_OPCODE_SIZE] = GUAC_SSH_TTY_OP_END; + *(current) = GUAC_SSH_TTY_OP_END; return 0; From c898f35959099b45f85dda72d31a7f59693833c6 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 2 Apr 2018 07:47:49 -0400 Subject: [PATCH 18/22] GUACAMOLE-269: Clean up terminal backspace initialization. --- src/terminal/terminal.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c index bfc36d1a..076305a9 100644 --- a/src/terminal/terminal.c +++ b/src/terminal/terminal.c @@ -1600,9 +1600,7 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed /* Backspace can vary based on configuration of terminal by client. */ if (keysym == 0xFF08) { - char backspace_str[2]; - backspace_str[0] = term->backspace; - backspace_str[1] = '\0'; + char backspace_str[] = { term->backspace, '\0' }; return guac_terminal_send_string(term, backspace_str); } if (keysym == 0xFF09 || keysym == 0xFF89) return guac_terminal_send_string(term, "\x09"); /* Tab */ From ea946f2492ade3245b7ad19c9111a846c738e927 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 2 Apr 2018 09:10:11 -0400 Subject: [PATCH 19/22] GUACAMOLE-269: Changes to initializing opcode array. --- src/protocols/ssh/ssh.c | 11 +++++---- src/protocols/ssh/ttymode.c | 49 +++++++++++++++++++++++-------------- src/protocols/ssh/ttymode.h | 15 +++++------- 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 9881c304..ace5d6fa 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -192,7 +192,6 @@ void* ssh_client_thread(void* data) { return NULL; } - /* Initialize a ttymode array */ const int num_tty_opcodes = 1; char ssh_ttymodes[(GUAC_SSH_TTY_OPCODE_SIZE * num_tty_opcodes) + 1]; @@ -302,13 +301,15 @@ void* ssh_client_thread(void* data) { } /* Set up the ttymode array prior to requesting the PTY */ - if (guac_ssh_ttymodes_init(ssh_ttymodes, sizeof(ssh_ttymodes), - num_tty_opcodes, (guac_ssh_ttymode){ GUAC_SSH_TTY_OP_VERASE, settings->backspace})) - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Error configuring TTY mode encoding."); + int ttymodeBytes = guac_ssh_ttymodes_init(ssh_ttymodes, sizeof(ssh_ttymodes), + GUAC_SSH_TTY_OP_VERASE, settings->backspace, GUAC_SSH_TTY_OP_END); + if (ttymodeBytes < 1) + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Error storing TTY mode encoding \ + opcodes and values in array."); /* Request PTY */ if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, - ssh_ttymodes, sizeof(ssh_ttymodes), ssh_client->term->term_width, + ssh_ttymodes, ttymodeBytes, ssh_client->term->term_width, ssh_client->term->term_height, 0, 0)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Unable to allocate PTY."); return NULL; diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index 4526cc1f..9f60859d 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -21,38 +21,49 @@ #include "ttymode.h" #include +#include #include #include int guac_ssh_ttymodes_init(char opcode_array[], const int array_size, - const int num_opcodes, ...) { + ...) { /* Initialize the variable argument list. */ va_list args; - va_start(args, num_opcodes); - - /* Check to make sure the array has enough space. - We need one extra byte at the end for the ending opcode. */ - if ((num_opcodes * GUAC_SSH_TTY_OPCODE_SIZE) >= (array_size)) - return 1; + va_start(args, array_size); + /* Initialize array pointer and byte counter. */ char *current = opcode_array; - for (int i = 0; i < num_opcodes; i++) { + int bytes = 0; - /* Get the next argument to this function */ - guac_ssh_ttymode* ttymode = va_arg(args, guac_ssh_ttymode*); + /* Loop through variable argument list. */ + while (true) { + + /* Check to make sure we don't overrun array. */ + if (bytes >= array_size) + return -1; - /* Place opcode and value in array */ - *(current++) = ttymode->opcode; - *(current++) = (ttymode->value >> 24) & 0xFF; - *(current++) = (ttymode->value >> 16) & 0xFF; - *(current++) = (ttymode->value >> 8) & 0xFF; - *(current++) = ttymode->value & 0xFF; + /* Next argument should be an opcode. */ + char opcode = (char)va_arg(args, int); + *(current++) = opcode; + bytes += sizeof(char); + + /* If it's the end opcode, we're done. */ + if (opcode == GUAC_SSH_TTY_OP_END) + break; + + /* Next argument should be 4-byte value. */ + uint32_t value = va_arg(args, uint32_t); + *(current++) = (value >> 24) & 0xFF; + *(current++) = (value >> 16) & 0xFF; + *(current++) = (value >> 8) & 0xFF; + *(current++) = value & 0xFF; + bytes += sizeof(uint32_t); } - /* Put the end opcode in the last opcode space */ - *(current) = GUAC_SSH_TTY_OP_END; + /* We're done processing arguments. */ + va_end(args); - return 0; + return bytes; } diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index 59c5415d..96bf0b49 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -76,9 +76,9 @@ typedef struct guac_ssh_ttymode { * in a single array, beginning with the opcode and followed * by a four byte value, repeating until the end opcode is * encountered. This function takes the array, the array - * size, expected number of opcodes, and that number of - * guac_ssh_ttymode arguments and puts them in the array - * exepcted by the SSH connection. + * size, and a variable number of opcode and value pair + * arguments and puts them in the array expected by the + * SSH connection. * * @param opcode_array * Pointer to the opcode array that will ultimately @@ -87,18 +87,15 @@ typedef struct guac_ssh_ttymode { * @param array_size * Size, in bytes, of the array. * - * @param num_opcodes - * Number of opcodes to expect. - * * @params ... - * A variable number of guac_ssh_ttymodes + * A variable number of opcode and value pairs * to place in the array. * * @return - * Zero of the function is successful, non-zero + * Number of bytes written to the array, or zero * if a failure occurs. */ int guac_ssh_ttymodes_init(char opcode_array[], const int array_size, - const int num_opcodes, ...); + ...); #endif From b441181c189e3ac8baa226f2022e82d8739532e3 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 2 Apr 2018 09:22:51 -0400 Subject: [PATCH 20/22] GUACAMOLE-269: Remove unnecessary data structure and array size, and update comments. --- src/protocols/ssh/ssh.c | 5 ++-- src/protocols/ssh/ttymode.c | 9 ++------ src/protocols/ssh/ttymode.h | 46 +++++++++++-------------------------- 3 files changed, 17 insertions(+), 43 deletions(-) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index ace5d6fa..dca62084 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -192,8 +192,7 @@ void* ssh_client_thread(void* data) { return NULL; } - const int num_tty_opcodes = 1; - char ssh_ttymodes[(GUAC_SSH_TTY_OPCODE_SIZE * num_tty_opcodes) + 1]; + char ssh_ttymodes[GUAC_SSH_TTYMODES_SIZE(1)]; /* Set up screen recording, if requested */ if (settings->recording_path != NULL) { @@ -301,7 +300,7 @@ void* ssh_client_thread(void* data) { } /* Set up the ttymode array prior to requesting the PTY */ - int ttymodeBytes = guac_ssh_ttymodes_init(ssh_ttymodes, sizeof(ssh_ttymodes), + int ttymodeBytes = guac_ssh_ttymodes_init(ssh_ttymodes, GUAC_SSH_TTY_OP_VERASE, settings->backspace, GUAC_SSH_TTY_OP_END); if (ttymodeBytes < 1) guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Error storing TTY mode encoding \ diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index 9f60859d..686cbbd3 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -25,12 +25,11 @@ #include #include -int guac_ssh_ttymodes_init(char opcode_array[], const int array_size, - ...) { +int guac_ssh_ttymodes_init(char opcode_array[], ...) { /* Initialize the variable argument list. */ va_list args; - va_start(args, array_size); + va_start(args, opcode_array); /* Initialize array pointer and byte counter. */ char *current = opcode_array; @@ -39,10 +38,6 @@ int guac_ssh_ttymodes_init(char opcode_array[], const int array_size, /* Loop through variable argument list. */ while (true) { - /* Check to make sure we don't overrun array. */ - if (bytes >= array_size) - return -1; - /* Next argument should be an opcode. */ char opcode = (char)va_arg(args, int); *(current++) = opcode; diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index 96bf0b49..648d13e4 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -46,46 +46,27 @@ #define GUAC_SSH_TTY_OP_VERASE 3 /** - * The SSH protocol attempts to configure the remote - * terminal by sending pairs of opcodes and values, as - * described in Section 8 of RFC 4254. These are - * comprised of a single byte opcode and a 4-byte - * value. This data structure stores a single opcode - * and value pair. + * Macro for calculating the number of bytes required + * to pass a given number of opcodes, which calculates + * the size of the number of opcodes plus the single byte + * end opcode. */ -typedef struct guac_ssh_ttymode { - - /** - * The single byte opcode for defining the TTY - * encoding setting for the remote terminal. The - * stadard codes are defined in Section 8 of - * the SSH RFC (4254). - */ - char opcode; - - /** - * The four byte value of the setting for the - * defined opcode. - */ - uint32_t value; - -} guac_ssh_ttymode; +#define GUAC_SSH_TTYMODES_SIZE(num_opcodes) ((GUAC_SSH_TTY_OPCODE_SIZE * num_opcodes) + 1) /** * Opcodes and value pairs are passed to the SSH connection * in a single array, beginning with the opcode and followed * by a four byte value, repeating until the end opcode is - * encountered. This function takes the array, the array - * size, and a variable number of opcode and value pair - * arguments and puts them in the array expected by the - * SSH connection. + * encountered. This function takes the array that will be + * sent and a variable number of opcode and value pair + * arguments and places the opcode and values in the array + * as expected by the SSH connection. * * @param opcode_array * Pointer to the opcode array that will ultimately - * be passed to the SSH connection. - * - * @param array_size - * Size, in bytes, of the array. + * be passed to the SSH connection. The array must + * be size to handle 5 bytes for each opcode and value + * pair, plus one additional byte for the end opcode. * * @params ... * A variable number of opcode and value pairs @@ -95,7 +76,6 @@ typedef struct guac_ssh_ttymode { * Number of bytes written to the array, or zero * if a failure occurs. */ -int guac_ssh_ttymodes_init(char opcode_array[], const int array_size, - ...); +int guac_ssh_ttymodes_init(char opcode_array[], ...); #endif From 7453bc8f4472cd41e90cf72cfdbbce3d5fa10140 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 2 Apr 2018 15:04:03 -0400 Subject: [PATCH 21/22] GUACAMOLE-269: Clean up logging and comments, and simplify code. --- src/protocols/ssh/ssh.c | 4 ++-- src/protocols/ssh/ttymode.c | 5 +---- src/protocols/ssh/ttymode.h | 8 ++++++++ 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index dca62084..1a7e6ef1 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -303,8 +303,8 @@ void* ssh_client_thread(void* data) { int ttymodeBytes = guac_ssh_ttymodes_init(ssh_ttymodes, GUAC_SSH_TTY_OP_VERASE, settings->backspace, GUAC_SSH_TTY_OP_END); if (ttymodeBytes < 1) - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Error storing TTY mode encoding \ - opcodes and values in array."); + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Unable to set TTY modes." + " Backspace may not work as expected."); /* Request PTY */ if (libssh2_channel_request_pty_ex(ssh_client->term_channel, "linux", sizeof("linux")-1, diff --git a/src/protocols/ssh/ttymode.c b/src/protocols/ssh/ttymode.c index 686cbbd3..e6731b4a 100644 --- a/src/protocols/ssh/ttymode.c +++ b/src/protocols/ssh/ttymode.c @@ -33,7 +33,6 @@ int guac_ssh_ttymodes_init(char opcode_array[], ...) { /* Initialize array pointer and byte counter. */ char *current = opcode_array; - int bytes = 0; /* Loop through variable argument list. */ while (true) { @@ -41,7 +40,6 @@ int guac_ssh_ttymodes_init(char opcode_array[], ...) { /* Next argument should be an opcode. */ char opcode = (char)va_arg(args, int); *(current++) = opcode; - bytes += sizeof(char); /* If it's the end opcode, we're done. */ if (opcode == GUAC_SSH_TTY_OP_END) @@ -53,12 +51,11 @@ int guac_ssh_ttymodes_init(char opcode_array[], ...) { *(current++) = (value >> 16) & 0xFF; *(current++) = (value >> 8) & 0xFF; *(current++) = value & 0xFF; - bytes += sizeof(uint32_t); } /* We're done processing arguments. */ va_end(args); - return bytes; + return current - opcode_array; } diff --git a/src/protocols/ssh/ttymode.h b/src/protocols/ssh/ttymode.h index 648d13e4..92e081b3 100644 --- a/src/protocols/ssh/ttymode.h +++ b/src/protocols/ssh/ttymode.h @@ -50,6 +50,14 @@ * to pass a given number of opcodes, which calculates * the size of the number of opcodes plus the single byte * end opcode. + * + * @param num_opcodes + * The number of opcodes for which a size in bytes + * should be calculated. + * + * @returns + * The number of bytes that the given number of + * opcodes will require. */ #define GUAC_SSH_TTYMODES_SIZE(num_opcodes) ((GUAC_SSH_TTY_OPCODE_SIZE * num_opcodes) + 1) From dc1918b2176a90ce10a6e15584f8462c6a042ba7 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 2 Apr 2018 15:05:56 -0400 Subject: [PATCH 22/22] GUACAMOLE-269: Don't abort on ttymode issue, just log a warning. --- src/protocols/ssh/ssh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 1a7e6ef1..7c76037a 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -303,7 +303,7 @@ void* ssh_client_thread(void* data) { int ttymodeBytes = guac_ssh_ttymodes_init(ssh_ttymodes, GUAC_SSH_TTY_OP_VERASE, settings->backspace, GUAC_SSH_TTY_OP_END); if (ttymodeBytes < 1) - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Unable to set TTY modes." + guac_client_log(client, GUAC_LOG_WARNING, "Unable to set TTY modes." " Backspace may not work as expected."); /* Request PTY */