From 171bae1f5c13144f1574db11ac9a19732ec567cc Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sun, 25 Mar 2018 17:34:29 -0400 Subject: [PATCH 01/20] GUACAMOLE-527: Add basic check for known hosts file for SSH connections. --- src/common-ssh/ssh.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 5ea6feab..5099de25 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -509,6 +510,47 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, return NULL; } + /* Check known_hosts */ + size_t len; + int type; + struct passwd *pw = getpwuid(getuid()); + char *homedir = pw->pw_dir; + char *known_hosts = strcat(homedir, "/.ssh/known_hosts"); + LIBSSH2_KNOWNHOSTS *ssh_known_hosts = libssh2_knownhost_init(session); + + libssh2_knownhost_readfile(ssh_known_hosts, known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); + const char *fingerprint = libssh2_session_hostkey(session, &len, &type); + + if (fingerprint) { + struct libssh2_knownhost *host; + int check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, atoi(port), + fingerprint, len, + LIBSSH2_KNOWNHOST_TYPE_PLAIN| + LIBSSH2_KNOWNHOST_KEYENC_RAW, + &host); + + switch (check) { + case LIBSSH2_KNOWNHOST_CHECK_MATCH: + guac_client_log(client, GUAC_LOG_DEBUG, + "Host key match found."); + break; + case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host key not found in known hosts file."); + break; + case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host key does not match host entry."); + break; + case LIBSSH2_KNOWNHOST_CHECK_FAILURE: + default: + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host could not be checked against known hosts."); + } + + + } + /* Perform handshake */ if (libssh2_session_handshake(session, fd)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, From 0d82cd1e6c78606a6b4d65b6d2441b02479fda88 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 5 Apr 2018 07:36:37 -0400 Subject: [PATCH 02/20] GUACAMOLE-527: Add host key and type settings. --- src/common-ssh/common-ssh/ssh.h | 3 +- src/common-ssh/ssh.c | 85 +++++++++++++++++++++------------ src/protocols/ssh/settings.c | 22 +++++++++ src/protocols/ssh/settings.h | 10 ++++ 4 files changed, 89 insertions(+), 31 deletions(-) diff --git a/src/common-ssh/common-ssh/ssh.h b/src/common-ssh/common-ssh/ssh.h index 8026549d..e25b6263 100644 --- a/src/common-ssh/common-ssh/ssh.h +++ b/src/common-ssh/common-ssh/ssh.h @@ -98,7 +98,8 @@ void guac_common_ssh_uninit(); * if the connection or authentication were not successful. */ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, - const char* hostname, const char* port, guac_common_ssh_user* user, int keepalive); + const char* hostname, const char* port, guac_common_ssh_user* user, int keepalive, + const char* host_key_type, const char* host_key); /** * Disconnects and destroys the given SSH session, freeing all associated diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 5099de25..a1a62f60 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -415,7 +415,8 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) } guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, - const char* hostname, const char* port, guac_common_ssh_user* user, int keepalive) { + const char* hostname, const char* port, guac_common_ssh_user* user, int keepalive, + const char* host_key_type, const char* host_key) { int retval; @@ -511,44 +512,68 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, } /* Check known_hosts */ - size_t len; - int type; + /* Get known hosts file from user running guacd */ struct passwd *pw = getpwuid(getuid()); char *homedir = pw->pw_dir; char *known_hosts = strcat(homedir, "/.ssh/known_hosts"); LIBSSH2_KNOWNHOSTS *ssh_known_hosts = libssh2_knownhost_init(session); - libssh2_knownhost_readfile(ssh_known_hosts, known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); - const char *fingerprint = libssh2_session_hostkey(session, &len, &type); - if (fingerprint) { - struct libssh2_knownhost *host; - int check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, atoi(port), - fingerprint, len, - LIBSSH2_KNOWNHOST_TYPE_PLAIN| - LIBSSH2_KNOWNHOST_KEYENC_RAW, - &host); + /* Add host key provided from settings */ + if (strcmp(host_key, "") > 0) { - switch (check) { - case LIBSSH2_KNOWNHOST_CHECK_MATCH: - guac_client_log(client, GUAC_LOG_DEBUG, - "Host key match found."); - break; - case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host key not found in known hosts file."); - break; - case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host key does not match host entry."); - break; - case LIBSSH2_KNOWNHOST_CHECK_FAILURE: - default: - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host could not be checked against known hosts."); - } + int kh_key_type = 0; + if (strcmp(host_key_type, "ssh-rsa") == 0) + kh_key_type = LIBSSH2_KNOWNHOST_KEY_SSHRSA; + else if(strcmp(host_key_type, "ssh-dss") == 0) + kh_key_type = LIBSSH2_KNOWNHOST_KEY_SSHDSS; + else if(strcmp(host_key_type, "rsa1") == 0) + kh_key_type = LIBSSH2_KNOWNHOST_KEY_RSA1; + else + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Invalid SSH host key type %s", host_key_type); + if (libssh2_knownhost_addc(ssh_known_hosts, hostname, NULL, host_key, strlen(host_key), + NULL, 0, LIBSSH2_KNOWNHOST_TYPE_PLAIN|LIBSSH2_KNOWNHOST_KEYENC_BASE64| + kh_key_type, NULL)) + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Failed to add host key to known hosts store for %s", hostname); + } + /* Get fingerprint of host we're connecting to */ + size_t fp_len; + int fp_type; + const char *fingerprint = libssh2_session_hostkey(session, &fp_len, &fp_type); + + if (!fingerprint || strcmp(fingerprint, "") == 0) { + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Could not retrieve fingerprint of SSH server %s", hostname); + } + + struct libssh2_knownhost *host; + int kh_check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, atoi(port), + fingerprint, fp_len, + LIBSSH2_KNOWNHOST_TYPE_PLAIN| + LIBSSH2_KNOWNHOST_KEYENC_RAW, + &host); + + switch (kh_check) { + case LIBSSH2_KNOWNHOST_CHECK_MATCH: + guac_client_log(client, GUAC_LOG_DEBUG, + "Host key match found."); + break; + case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host key not found in known hosts file."); + break; + case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host key does not match host entry."); + break; + case LIBSSH2_KNOWNHOST_CHECK_FAILURE: + default: + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host could not be checked against known hosts."); } /* Perform handshake */ diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 2ce97903..bc650e99 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -31,6 +31,8 @@ /* Client plugin arguments */ const char* GUAC_SSH_CLIENT_ARGS[] = { "hostname", + "host_key_type", + "host_key", "port", "username", "password", @@ -68,6 +70,16 @@ enum SSH_ARGS_IDX { */ IDX_HOSTNAME, + /** + * The type of public SSH host key provided. Optional. + */ + IDX_HOST_KEY_TYPE, + + /** + * The Base64-encoded public SSH host key. Optional. + */ + IDX_HOST_KEY, + /** * The port to connect to. Optional. */ @@ -247,6 +259,14 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user, guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, IDX_HOSTNAME, ""); + settings->host_key_type = + guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, + IDX_HOST_KEY_TYPE, "ssh-rsa"); + + settings->host_key = + guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, + IDX_HOST_KEY, NULL); + settings->username = guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, IDX_USERNAME, NULL); @@ -384,6 +404,8 @@ void guac_ssh_settings_free(guac_ssh_settings* settings) { /* Free network connection information */ free(settings->hostname); + free(settings->host_key_type); + free(settings->host_key); free(settings->port); /* Free credentials */ diff --git a/src/protocols/ssh/settings.h b/src/protocols/ssh/settings.h index 393cfc0c..ac8400ec 100644 --- a/src/protocols/ssh/settings.h +++ b/src/protocols/ssh/settings.h @@ -70,6 +70,16 @@ typedef struct guac_ssh_settings { */ char* hostname; + /** + * The type of public SSH host key. + */ + char* host_key_type; + + /** + * The public SSH host key. + */ + char* host_key; + /** * The port of the SSH server to connect to. */ From 9112c4f32f719ca1b4ae5f301e36f96c6190ba9d Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 5 Apr 2018 08:52:16 -0400 Subject: [PATCH 03/20] GUACAMOLE-527: Enable host key setting for SFTP connections. --- src/common-ssh/common-ssh/ssh.h | 2 +- src/common-ssh/ssh.c | 16 ++----------- src/protocols/rdp/rdp.c | 3 ++- src/protocols/rdp/rdp_settings.c | 40 ++++++++++++++++++++++++++++++++ src/protocols/rdp/rdp_settings.h | 10 ++++++++ src/protocols/ssh/settings.c | 23 +++++++++++++----- src/protocols/ssh/settings.h | 2 +- src/protocols/ssh/ssh.c | 6 +++-- src/protocols/vnc/settings.c | 37 +++++++++++++++++++++++++++++ src/protocols/vnc/settings.h | 10 ++++++++ src/protocols/vnc/vnc.c | 3 ++- 11 files changed, 126 insertions(+), 26 deletions(-) diff --git a/src/common-ssh/common-ssh/ssh.h b/src/common-ssh/common-ssh/ssh.h index e25b6263..8f6f6893 100644 --- a/src/common-ssh/common-ssh/ssh.h +++ b/src/common-ssh/common-ssh/ssh.h @@ -99,7 +99,7 @@ void guac_common_ssh_uninit(); */ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, const char* hostname, const char* port, guac_common_ssh_user* user, int keepalive, - const char* host_key_type, const char* host_key); + const int host_key_type, const char* host_key); /** * Disconnects and destroys the given SSH session, freeing all associated diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index a1a62f60..0eb9fa10 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -416,7 +416,7 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, const char* hostname, const char* port, guac_common_ssh_user* user, int keepalive, - const char* host_key_type, const char* host_key) { + const int host_key_type, const char* host_key) { int retval; @@ -522,20 +522,9 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, /* Add host key provided from settings */ if (strcmp(host_key, "") > 0) { - int kh_key_type = 0; - if (strcmp(host_key_type, "ssh-rsa") == 0) - kh_key_type = LIBSSH2_KNOWNHOST_KEY_SSHRSA; - else if(strcmp(host_key_type, "ssh-dss") == 0) - kh_key_type = LIBSSH2_KNOWNHOST_KEY_SSHDSS; - else if(strcmp(host_key_type, "rsa1") == 0) - kh_key_type = LIBSSH2_KNOWNHOST_KEY_RSA1; - else - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Invalid SSH host key type %s", host_key_type); - if (libssh2_knownhost_addc(ssh_known_hosts, hostname, NULL, host_key, strlen(host_key), NULL, 0, LIBSSH2_KNOWNHOST_TYPE_PLAIN|LIBSSH2_KNOWNHOST_KEYENC_BASE64| - kh_key_type, NULL)) + host_key_type, NULL)) guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Failed to add host key to known hosts store for %s", hostname); } @@ -627,4 +616,3 @@ void guac_common_ssh_destroy_session(guac_common_ssh_session* session) { free(session); } - diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 45082bb0..30b19326 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -974,7 +974,8 @@ void* guac_rdp_client_thread(void* data) { /* Attempt SSH connection */ rdp_client->sftp_session = guac_common_ssh_create_session(client, settings->sftp_hostname, - settings->sftp_port, rdp_client->sftp_user, settings->sftp_server_alive_interval); + settings->sftp_port, rdp_client->sftp_user, settings->sftp_server_alive_interval, + settings->sftp_host_key_type, settings->sftp_host_key); /* Fail if SSH connection does not succeed */ if (rdp_client->sftp_session == NULL) { diff --git a/src/protocols/rdp/rdp_settings.c b/src/protocols/rdp/rdp_settings.c index 35f7f202..78afe374 100644 --- a/src/protocols/rdp/rdp_settings.c +++ b/src/protocols/rdp/rdp_settings.c @@ -35,6 +35,9 @@ #include "compat/winpr-wtypes.h" #endif +#ifdef ENABLE_COMMON_SSH +#include +#endif #include #include @@ -81,6 +84,8 @@ const char* GUAC_RDP_CLIENT_ARGS[] = { #ifdef ENABLE_COMMON_SSH "enable-sftp", "sftp-hostname", + "sftp-host-key-type", + "sftp-host-key", "sftp-port", "sftp-username", "sftp-password", @@ -355,6 +360,17 @@ enum RDP_ARGS_IDX { */ IDX_SFTP_HOSTNAME, + /** + * The type of public SSH host key provided. If not specified, it defaults + * to SSH-RSA. + */ + IDX_SFTP_HOST_KEY_TYPE, + + /** + * The public SSH host key of the SFTP server. Optional. + */ + IDX_SFTP_HOST_KEY, + /** * The port of the SSH server to connect to for SFTP. If blank, the default * SSH port of "22" will be used. @@ -822,6 +838,30 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user, guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv, IDX_SFTP_HOSTNAME, settings->hostname); + /* The public SSH host key. */ + settings->sftp_host_key = + guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv, + IDX_SFTP_HOST_KEY, NULL); + + if(settings->sftp_host_key) { + /* Type of public SSH host key. */ + char* str_host_key_type = guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv, + IDX_SFTP_HOST_KEY_TYPE, "ssh-rsa"); + + if (strcmp(str_host_key_type, "ssh-rsa") == 0) + settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHRSA; + else if (strcmp(str_host_key_type, "ssh-dss") == 0) + settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHDSS; + else if (strcmp(str_host_key_type, "rsa1") == 0) + settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_RSA1; + else { + guac_user_log(user, GUAC_LOG_WARNING, "Invalid host key type specified %s. " + "Ignoring host key.", str_host_key_type); + settings->sftp_host_key = NULL; + } + + } + /* Port for SFTP connection */ settings->sftp_port = guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv, diff --git a/src/protocols/rdp/rdp_settings.h b/src/protocols/rdp/rdp_settings.h index ad71ca06..0a442799 100644 --- a/src/protocols/rdp/rdp_settings.h +++ b/src/protocols/rdp/rdp_settings.h @@ -342,6 +342,16 @@ typedef struct guac_rdp_settings { */ char* sftp_hostname; + /** + * The type of the public SSH hos key. + */ + int sftp_host_key_type; + + /** + * The public SSH host key. + */ + char* sftp_host_key; + /** * The port of the SSH server to connect to for SFTP. */ diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index bc650e99..4653e96f 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -24,6 +24,7 @@ #include +#include #include #include #include @@ -259,14 +260,26 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user, guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, IDX_HOSTNAME, ""); - settings->host_key_type = - guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, - IDX_HOST_KEY_TYPE, "ssh-rsa"); - settings->host_key = guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, IDX_HOST_KEY, NULL); + if (settings->host_key) { + char* str_host_key_type = guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, + IDX_HOST_KEY_TYPE, "ssh-rsa"); + if (strcmp(str_host_key_type, "ssh-rsa") == 0) + settings->host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHRSA; + else if (strcmp(str_host_key_type, "ssh-dss") == 0) + settings->host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHDSS; + else if (strcmp(str_host_key_type, "rsa1") == 0) + settings->host_key_type = LIBSSH2_KNOWNHOST_KEY_RSA1; + else { + guac_user_log(user, GUAC_LOG_WARNING, "Invalid host key type specified %s. " + "Ignoring host key.", str_host_key_type); + settings->host_key = NULL; + } + } + settings->username = guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, IDX_USERNAME, NULL); @@ -404,7 +417,6 @@ void guac_ssh_settings_free(guac_ssh_settings* settings) { /* Free network connection information */ free(settings->hostname); - free(settings->host_key_type); free(settings->host_key); free(settings->port); @@ -439,4 +451,3 @@ void guac_ssh_settings_free(guac_ssh_settings* settings) { free(settings); } - diff --git a/src/protocols/ssh/settings.h b/src/protocols/ssh/settings.h index ac8400ec..e47a8161 100644 --- a/src/protocols/ssh/settings.h +++ b/src/protocols/ssh/settings.h @@ -73,7 +73,7 @@ typedef struct guac_ssh_settings { /** * The type of public SSH host key. */ - char* host_key_type; + int host_key_type; /** * The public SSH host key. diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index a614f813..63765559 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -235,7 +235,8 @@ void* ssh_client_thread(void* data) { /* Open SSH session */ ssh_client->session = guac_common_ssh_create_session(client, - settings->hostname, settings->port, ssh_client->user, settings->server_alive_interval); + settings->hostname, settings->port, ssh_client->user, settings->server_alive_interval, + settings->host_key_type, settings->host_key); if (ssh_client->session == NULL) { /* Already aborted within guac_common_ssh_create_session() */ return NULL; @@ -275,7 +276,8 @@ void* ssh_client_thread(void* data) { guac_client_log(client, GUAC_LOG_DEBUG, "Reconnecting for SFTP..."); ssh_client->sftp_session = guac_common_ssh_create_session(client, settings->hostname, - settings->port, ssh_client->user, settings->server_alive_interval); + settings->port, ssh_client->user, settings->server_alive_interval, + settings->host_key_type, settings->host_key); if (ssh_client->sftp_session == NULL) { /* Already aborted within guac_common_ssh_create_session() */ return NULL; diff --git a/src/protocols/vnc/settings.c b/src/protocols/vnc/settings.c index 509921a5..509a067b 100644 --- a/src/protocols/vnc/settings.c +++ b/src/protocols/vnc/settings.c @@ -24,6 +24,9 @@ #include +#ifdef ENABLE_COMMON_SSH +#include +#endif #include #include #include @@ -208,6 +211,16 @@ enum VNC_ARGS_IDX { */ IDX_SFTP_USERNAME, + /** + * The type of public SSH host key provided to identify the SFTP server. + */ + IDX_SFTP_HOST_KEY_TYPE, + + /** + * The public SSH host key to identify the SFTP server. + */ + IDX_SFTP_HOST_KEY, + /** * The password to provide when authenticating with the SSH server for * SFTP (if not using a private key). @@ -411,6 +424,30 @@ guac_vnc_settings* guac_vnc_parse_args(guac_user* user, guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, IDX_SFTP_HOSTNAME, settings->hostname); + /* The public SSH host key. */ + settings->sftp_host_key = + guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, + IDX_SFTP_HOST_KEY, NULL); + + if(settings->sftp_host_key) { + /* Type of public SSH host key. */ + char* str_host_key_type = guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, + IDX_SFTP_HOST_KEY_TYPE, "ssh-rsa"); + + if (strcmp(str_host_key_type, "ssh-rsa") == 0) + settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHRSA; + else if (strcmp(str_host_key_type, "ssh-dss") == 0) + settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHDSS; + else if (strcmp(str_host_key_type, "rsa1") == 0) + settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_RSA1; + else { + guac_user_log(user, GUAC_LOG_WARNING, "Invalid host key type specified %s. " + "Ignoring host key.", str_host_key_type); + settings->sftp_host_key = NULL; + } + + } + /* Port for SFTP connection */ settings->sftp_port = guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, diff --git a/src/protocols/vnc/settings.h b/src/protocols/vnc/settings.h index 85e6478c..35809f83 100644 --- a/src/protocols/vnc/settings.h +++ b/src/protocols/vnc/settings.h @@ -138,6 +138,16 @@ typedef struct guac_vnc_settings { */ char* sftp_hostname; + /** + * The type of public SSH host key provided. + */ + int sftp_host_key_type; + + /** + * The public SSH host key. + */ + char* sftp_host_key; + /** * The port of the SSH server to connect to for SFTP. */ diff --git a/src/protocols/vnc/vnc.c b/src/protocols/vnc/vnc.c index db1e2180..1146ad43 100644 --- a/src/protocols/vnc/vnc.c +++ b/src/protocols/vnc/vnc.c @@ -261,7 +261,8 @@ void* guac_vnc_client_thread(void* data) { /* Attempt SSH connection */ vnc_client->sftp_session = guac_common_ssh_create_session(client, settings->sftp_hostname, - settings->sftp_port, vnc_client->sftp_user, settings->sftp_server_alive_interval); + settings->sftp_port, vnc_client->sftp_user, settings->sftp_server_alive_interval, + settings->sftp_host_key_type, settings->sftp_host_key); /* Fail if SSH connection does not succeed */ if (vnc_client->sftp_session == NULL) { From 2f0c6dcfa3647960fa2b64c149b410a395edb6a9 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 5 Apr 2018 11:26:50 -0400 Subject: [PATCH 04/20] GUACAMOLE-527: Add error logging for known host checks. --- src/common-ssh/ssh.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 0eb9fa10..ee60ccf9 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -534,11 +534,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, int fp_type; const char *fingerprint = libssh2_session_hostkey(session, &fp_len, &fp_type); - if (!fingerprint || strcmp(fingerprint, "") == 0) { - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Could not retrieve fingerprint of SSH server %s", hostname); - } - + /* Check fingerprint against known hosts */ struct libssh2_knownhost *host; int kh_check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, atoi(port), fingerprint, fp_len, @@ -552,15 +548,23 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, "Host key match found."); break; case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: + guac_client_log(client, GUAC_LOG_ERROR, + "Host key not found in known hosts entries for %s.", hostname); guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host key not found in known hosts file."); + "Host key not found in known hosts entries."); break; case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: + guac_client_log(client, GUAC_LOG_ERROR, + "Host entry found, but host key does not match for %s", + hostname); guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Host key does not match host entry."); break; case LIBSSH2_KNOWNHOST_CHECK_FAILURE: default: + guac_client_log(client, GUAC_LOG_ERROR, + "Error checking host key against known hosts for %s", + hostname); guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Host could not be checked against known hosts."); } From c080569cacc6c3c31d339e234e6e69966b696397 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 5 Apr 2018 11:28:32 -0400 Subject: [PATCH 05/20] GUACAMOLE-527: Fix issue with null host_key variable. --- src/common-ssh/ssh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index ee60ccf9..01d50003 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -520,7 +520,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, libssh2_knownhost_readfile(ssh_known_hosts, known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); /* Add host key provided from settings */ - if (strcmp(host_key, "") > 0) { + if (host_key && strcmp(host_key, "") > 0) { if (libssh2_knownhost_addc(ssh_known_hosts, hostname, NULL, host_key, strlen(host_key), NULL, 0, LIBSSH2_KNOWNHOST_TYPE_PLAIN|LIBSSH2_KNOWNHOST_KEYENC_BASE64| From 5bb616832e34bc207cff142f24c1a7732f003d47 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 6 Apr 2018 05:45:14 -0400 Subject: [PATCH 06/20] GUACAMOLE-527: Order SSH handshake correctly, and remove unnecessary logging. --- src/common-ssh/ssh.c | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 01d50003..1327b03e 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -511,6 +511,15 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, return NULL; } + /* Perform handshake */ + if (libssh2_session_handshake(session, fd)) { + guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, + "SSH handshake failed."); + free(common_session); + close(fd); + return NULL; + } + /* Check known_hosts */ /* Get known hosts file from user running guacd */ struct passwd *pw = getpwuid(getuid()); @@ -527,6 +536,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, host_key_type, NULL)) guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Failed to add host key to known hosts store for %s", hostname); + } /* Get fingerprint of host we're connecting to */ @@ -534,6 +544,10 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, int fp_type; const char *fingerprint = libssh2_session_hostkey(session, &fp_len, &fp_type); + if (!fingerprint) + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Failed to get fingerprint for host %s", hostname); + /* Check fingerprint against known hosts */ struct libssh2_knownhost *host; int kh_check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, atoi(port), @@ -545,37 +559,21 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, switch (kh_check) { case LIBSSH2_KNOWNHOST_CHECK_MATCH: guac_client_log(client, GUAC_LOG_DEBUG, - "Host key match found."); + "Host key match found for %s", hostname); break; case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: - guac_client_log(client, GUAC_LOG_ERROR, - "Host key not found in known hosts entries for %s.", hostname); guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host key not found in known hosts entries."); + "Host key not found for %s.", hostname); break; case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: - guac_client_log(client, GUAC_LOG_ERROR, - "Host entry found, but host key does not match for %s", - hostname); guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host key does not match host entry."); + "Host key does not match host entry for %s", hostname); break; case LIBSSH2_KNOWNHOST_CHECK_FAILURE: default: - guac_client_log(client, GUAC_LOG_ERROR, - "Error checking host key against known hosts for %s", - hostname); guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host could not be checked against known hosts."); - } - - /* Perform handshake */ - if (libssh2_session_handshake(session, fd)) { - guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, - "SSH handshake failed."); - free(common_session); - close(fd); - return NULL; + "Host %s could not be checked against known hosts.", + hostname); } /* Store basic session data */ From ec4315dfbe9a648a5cb18c74f4dda7f0380a162a Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 6 Apr 2018 06:33:24 -0400 Subject: [PATCH 07/20] GUACAMOLE-527: Correct names of parameters coming from client. --- src/protocols/ssh/settings.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 4653e96f..d49a16c1 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -32,8 +32,8 @@ /* Client plugin arguments */ const char* GUAC_SSH_CLIENT_ARGS[] = { "hostname", - "host_key_type", - "host_key", + "host-key-type", + "host-key", "port", "username", "password", From 42044e42799402898c00ace8ca770c94f6799843 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Fri, 6 Apr 2018 06:47:35 -0400 Subject: [PATCH 08/20] GUACAMOLE-527: Clean up memory and logging. --- src/common-ssh/ssh.c | 18 ++++++++++-------- src/protocols/rdp/rdp_settings.c | 3 +++ src/protocols/ssh/settings.c | 2 ++ src/protocols/vnc/settings.c | 3 +++ 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 1327b03e..03c4d125 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -520,22 +520,22 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, return NULL; } - /* Check known_hosts */ - /* Get known hosts file from user running guacd */ + /* Check known_hosts, start by getting known_hosts file of user running guacd */ struct passwd *pw = getpwuid(getuid()); - char *homedir = pw->pw_dir; - char *known_hosts = strcat(homedir, "/.ssh/known_hosts"); + const char *known_hosts = strcat(pw->pw_dir, "/.ssh/known_hosts"); LIBSSH2_KNOWNHOSTS *ssh_known_hosts = libssh2_knownhost_init(session); libssh2_knownhost_readfile(ssh_known_hosts, known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); /* Add host key provided from settings */ if (host_key && strcmp(host_key, "") > 0) { - if (libssh2_knownhost_addc(ssh_known_hosts, hostname, NULL, host_key, strlen(host_key), + int kh_add = libssh2_knownhost_addc(ssh_known_hosts, hostname, NULL, host_key, strlen(host_key), NULL, 0, LIBSSH2_KNOWNHOST_TYPE_PLAIN|LIBSSH2_KNOWNHOST_KEYENC_BASE64| - host_key_type, NULL)) - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Failed to add host key to known hosts store for %s", hostname); + host_key_type, NULL); + + if (kh_add) + guac_client_log(client, GUAC_LOG_WARNING, "Failed to add provided host key" + " to known hosts store for %s. Error was %d", hostname, kh_add); } @@ -556,6 +556,8 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, LIBSSH2_KNOWNHOST_KEYENC_RAW, &host); + libssh2_knownhost_free(ssh_known_hosts); + switch (kh_check) { case LIBSSH2_KNOWNHOST_CHECK_MATCH: guac_client_log(client, GUAC_LOG_DEBUG, diff --git a/src/protocols/rdp/rdp_settings.c b/src/protocols/rdp/rdp_settings.c index 78afe374..65e79e6b 100644 --- a/src/protocols/rdp/rdp_settings.c +++ b/src/protocols/rdp/rdp_settings.c @@ -860,6 +860,8 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user, settings->sftp_host_key = NULL; } + free(str_host_key_type); + } /* Port for SFTP connection */ @@ -1039,6 +1041,7 @@ void guac_rdp_settings_free(guac_rdp_settings* settings) { /* Free SFTP settings */ free(settings->sftp_directory); free(settings->sftp_root_directory); + free(settings->sftp_host_key); free(settings->sftp_hostname); free(settings->sftp_passphrase); free(settings->sftp_password); diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index d49a16c1..119d725d 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -278,6 +278,8 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user, "Ignoring host key.", str_host_key_type); settings->host_key = NULL; } + + free(str_host_key_type); } settings->username = diff --git a/src/protocols/vnc/settings.c b/src/protocols/vnc/settings.c index 509a067b..5f82b39b 100644 --- a/src/protocols/vnc/settings.c +++ b/src/protocols/vnc/settings.c @@ -446,6 +446,8 @@ guac_vnc_settings* guac_vnc_parse_args(guac_user* user, settings->sftp_host_key = NULL; } + free(str_host_key_type); + } /* Port for SFTP connection */ @@ -541,6 +543,7 @@ void guac_vnc_settings_free(guac_vnc_settings* settings) { /* Free SFTP settings */ free(settings->sftp_directory); free(settings->sftp_root_directory); + free(settings->sftp_host_key); free(settings->sftp_hostname); free(settings->sftp_passphrase); free(settings->sftp_password); From 551598e0a4fcea547fcd050b278e0c4e6114842f Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 22 May 2018 22:59:51 -0400 Subject: [PATCH 09/20] GUACAMOLE-527: Use libssh2_knownhost_readline and remove host key type. --- src/common-ssh/common-ssh/ssh.h | 2 +- src/common-ssh/ssh.c | 9 ++++----- src/protocols/rdp/rdp.c | 2 +- src/protocols/rdp/rdp_settings.c | 27 --------------------------- src/protocols/rdp/rdp_settings.h | 5 ----- src/protocols/ssh/settings.c | 23 ----------------------- src/protocols/ssh/settings.h | 5 ----- src/protocols/ssh/ssh.c | 4 ++-- src/protocols/vnc/settings.c | 26 -------------------------- src/protocols/vnc/settings.h | 5 ----- src/protocols/vnc/vnc.c | 2 +- 11 files changed, 9 insertions(+), 101 deletions(-) diff --git a/src/common-ssh/common-ssh/ssh.h b/src/common-ssh/common-ssh/ssh.h index 8f6f6893..672e7767 100644 --- a/src/common-ssh/common-ssh/ssh.h +++ b/src/common-ssh/common-ssh/ssh.h @@ -99,7 +99,7 @@ void guac_common_ssh_uninit(); */ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, const char* hostname, const char* port, guac_common_ssh_user* user, int keepalive, - const int host_key_type, const char* host_key); + const char* host_key); /** * Disconnects and destroys the given SSH session, freeing all associated diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 03c4d125..54eae3b0 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -416,7 +416,7 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session) guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, const char* hostname, const char* port, guac_common_ssh_user* user, int keepalive, - const int host_key_type, const char* host_key) { + const char* host_key) { int retval; @@ -529,9 +529,8 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, /* Add host key provided from settings */ if (host_key && strcmp(host_key, "") > 0) { - int kh_add = libssh2_knownhost_addc(ssh_known_hosts, hostname, NULL, host_key, strlen(host_key), - NULL, 0, LIBSSH2_KNOWNHOST_TYPE_PLAIN|LIBSSH2_KNOWNHOST_KEYENC_BASE64| - host_key_type, NULL); + int kh_add = libssh2_knownhost_readline(ssh_known_hosts, host_key, strlen(host_key), + LIBSSH2_KNOWNHOST_FILE_OPENSSH); if (kh_add) guac_client_log(client, GUAC_LOG_WARNING, "Failed to add provided host key" @@ -564,7 +563,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, "Host key match found for %s", hostname); break; case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + guac_client_log(client, GUAC_LOG_WARNING, "Host key not found for %s.", hostname); break; case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 30b19326..4d484320 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -975,7 +975,7 @@ void* guac_rdp_client_thread(void* data) { rdp_client->sftp_session = guac_common_ssh_create_session(client, settings->sftp_hostname, settings->sftp_port, rdp_client->sftp_user, settings->sftp_server_alive_interval, - settings->sftp_host_key_type, settings->sftp_host_key); + settings->sftp_host_key); /* Fail if SSH connection does not succeed */ if (rdp_client->sftp_session == NULL) { diff --git a/src/protocols/rdp/rdp_settings.c b/src/protocols/rdp/rdp_settings.c index 65e79e6b..2fe261ce 100644 --- a/src/protocols/rdp/rdp_settings.c +++ b/src/protocols/rdp/rdp_settings.c @@ -360,12 +360,6 @@ enum RDP_ARGS_IDX { */ IDX_SFTP_HOSTNAME, - /** - * The type of public SSH host key provided. If not specified, it defaults - * to SSH-RSA. - */ - IDX_SFTP_HOST_KEY_TYPE, - /** * The public SSH host key of the SFTP server. Optional. */ @@ -843,27 +837,6 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user, guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv, IDX_SFTP_HOST_KEY, NULL); - if(settings->sftp_host_key) { - /* Type of public SSH host key. */ - char* str_host_key_type = guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv, - IDX_SFTP_HOST_KEY_TYPE, "ssh-rsa"); - - if (strcmp(str_host_key_type, "ssh-rsa") == 0) - settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHRSA; - else if (strcmp(str_host_key_type, "ssh-dss") == 0) - settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHDSS; - else if (strcmp(str_host_key_type, "rsa1") == 0) - settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_RSA1; - else { - guac_user_log(user, GUAC_LOG_WARNING, "Invalid host key type specified %s. " - "Ignoring host key.", str_host_key_type); - settings->sftp_host_key = NULL; - } - - free(str_host_key_type); - - } - /* Port for SFTP connection */ settings->sftp_port = guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv, diff --git a/src/protocols/rdp/rdp_settings.h b/src/protocols/rdp/rdp_settings.h index 0a442799..4f3839e6 100644 --- a/src/protocols/rdp/rdp_settings.h +++ b/src/protocols/rdp/rdp_settings.h @@ -342,11 +342,6 @@ typedef struct guac_rdp_settings { */ char* sftp_hostname; - /** - * The type of the public SSH hos key. - */ - int sftp_host_key_type; - /** * The public SSH host key. */ diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 119d725d..4a1c3710 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -71,11 +71,6 @@ enum SSH_ARGS_IDX { */ IDX_HOSTNAME, - /** - * The type of public SSH host key provided. Optional. - */ - IDX_HOST_KEY_TYPE, - /** * The Base64-encoded public SSH host key. Optional. */ @@ -264,24 +259,6 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user, guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, IDX_HOST_KEY, NULL); - if (settings->host_key) { - char* str_host_key_type = guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, - IDX_HOST_KEY_TYPE, "ssh-rsa"); - if (strcmp(str_host_key_type, "ssh-rsa") == 0) - settings->host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHRSA; - else if (strcmp(str_host_key_type, "ssh-dss") == 0) - settings->host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHDSS; - else if (strcmp(str_host_key_type, "rsa1") == 0) - settings->host_key_type = LIBSSH2_KNOWNHOST_KEY_RSA1; - else { - guac_user_log(user, GUAC_LOG_WARNING, "Invalid host key type specified %s. " - "Ignoring host key.", str_host_key_type); - settings->host_key = NULL; - } - - free(str_host_key_type); - } - settings->username = guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv, IDX_USERNAME, NULL); diff --git a/src/protocols/ssh/settings.h b/src/protocols/ssh/settings.h index e47a8161..761239c7 100644 --- a/src/protocols/ssh/settings.h +++ b/src/protocols/ssh/settings.h @@ -70,11 +70,6 @@ typedef struct guac_ssh_settings { */ char* hostname; - /** - * The type of public SSH host key. - */ - int host_key_type; - /** * The public SSH host key. */ diff --git a/src/protocols/ssh/ssh.c b/src/protocols/ssh/ssh.c index 63765559..f98f2228 100644 --- a/src/protocols/ssh/ssh.c +++ b/src/protocols/ssh/ssh.c @@ -236,7 +236,7 @@ void* ssh_client_thread(void* data) { /* Open SSH session */ ssh_client->session = guac_common_ssh_create_session(client, settings->hostname, settings->port, ssh_client->user, settings->server_alive_interval, - settings->host_key_type, settings->host_key); + settings->host_key); if (ssh_client->session == NULL) { /* Already aborted within guac_common_ssh_create_session() */ return NULL; @@ -277,7 +277,7 @@ void* ssh_client_thread(void* data) { ssh_client->sftp_session = guac_common_ssh_create_session(client, settings->hostname, settings->port, ssh_client->user, settings->server_alive_interval, - settings->host_key_type, settings->host_key); + settings->host_key); if (ssh_client->sftp_session == NULL) { /* Already aborted within guac_common_ssh_create_session() */ return NULL; diff --git a/src/protocols/vnc/settings.c b/src/protocols/vnc/settings.c index 5f82b39b..130c97d5 100644 --- a/src/protocols/vnc/settings.c +++ b/src/protocols/vnc/settings.c @@ -211,11 +211,6 @@ enum VNC_ARGS_IDX { */ IDX_SFTP_USERNAME, - /** - * The type of public SSH host key provided to identify the SFTP server. - */ - IDX_SFTP_HOST_KEY_TYPE, - /** * The public SSH host key to identify the SFTP server. */ @@ -429,27 +424,6 @@ guac_vnc_settings* guac_vnc_parse_args(guac_user* user, guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, IDX_SFTP_HOST_KEY, NULL); - if(settings->sftp_host_key) { - /* Type of public SSH host key. */ - char* str_host_key_type = guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, - IDX_SFTP_HOST_KEY_TYPE, "ssh-rsa"); - - if (strcmp(str_host_key_type, "ssh-rsa") == 0) - settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHRSA; - else if (strcmp(str_host_key_type, "ssh-dss") == 0) - settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_SSHDSS; - else if (strcmp(str_host_key_type, "rsa1") == 0) - settings->sftp_host_key_type = LIBSSH2_KNOWNHOST_KEY_RSA1; - else { - guac_user_log(user, GUAC_LOG_WARNING, "Invalid host key type specified %s. " - "Ignoring host key.", str_host_key_type); - settings->sftp_host_key = NULL; - } - - free(str_host_key_type); - - } - /* Port for SFTP connection */ settings->sftp_port = guac_user_parse_args_string(user, GUAC_VNC_CLIENT_ARGS, argv, diff --git a/src/protocols/vnc/settings.h b/src/protocols/vnc/settings.h index 35809f83..3e2ebd5e 100644 --- a/src/protocols/vnc/settings.h +++ b/src/protocols/vnc/settings.h @@ -138,11 +138,6 @@ typedef struct guac_vnc_settings { */ char* sftp_hostname; - /** - * The type of public SSH host key provided. - */ - int sftp_host_key_type; - /** * The public SSH host key. */ diff --git a/src/protocols/vnc/vnc.c b/src/protocols/vnc/vnc.c index 1146ad43..d9f9dbbb 100644 --- a/src/protocols/vnc/vnc.c +++ b/src/protocols/vnc/vnc.c @@ -262,7 +262,7 @@ void* guac_vnc_client_thread(void* data) { vnc_client->sftp_session = guac_common_ssh_create_session(client, settings->sftp_hostname, settings->sftp_port, vnc_client->sftp_user, settings->sftp_server_alive_interval, - settings->sftp_host_key_type, settings->sftp_host_key); + settings->sftp_host_key); /* Fail if SSH connection does not succeed */ if (vnc_client->sftp_session == NULL) { From 2bebb96804f1bf6d0f573de53999f8ee8e368ef3 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 24 May 2018 09:24:48 -0400 Subject: [PATCH 10/20] GUACAMOLE-527: Fix host key options in the protocol settings. --- src/protocols/rdp/rdp_settings.c | 1 - src/protocols/ssh/settings.c | 1 - src/protocols/vnc/settings.c | 1 + 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/protocols/rdp/rdp_settings.c b/src/protocols/rdp/rdp_settings.c index 2fe261ce..0af64437 100644 --- a/src/protocols/rdp/rdp_settings.c +++ b/src/protocols/rdp/rdp_settings.c @@ -84,7 +84,6 @@ const char* GUAC_RDP_CLIENT_ARGS[] = { #ifdef ENABLE_COMMON_SSH "enable-sftp", "sftp-hostname", - "sftp-host-key-type", "sftp-host-key", "sftp-port", "sftp-username", diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 4a1c3710..7820cf63 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -32,7 +32,6 @@ /* Client plugin arguments */ const char* GUAC_SSH_CLIENT_ARGS[] = { "hostname", - "host-key-type", "host-key", "port", "username", diff --git a/src/protocols/vnc/settings.c b/src/protocols/vnc/settings.c index 130c97d5..27524e13 100644 --- a/src/protocols/vnc/settings.c +++ b/src/protocols/vnc/settings.c @@ -63,6 +63,7 @@ const char* GUAC_VNC_CLIENT_ARGS[] = { #ifdef ENABLE_COMMON_SSH "enable-sftp", "sftp-hostname", + "sftp-host-key", "sftp-port", "sftp-username", "sftp-password", From aec2be6da28fb0e58474e2c2bdd32989330bedb9 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Thu, 24 May 2018 09:41:44 -0400 Subject: [PATCH 11/20] GUACAMOLE-527: Remove unnecessary includes. --- src/protocols/rdp/rdp_settings.c | 3 --- src/protocols/ssh/settings.c | 1 - src/protocols/vnc/settings.c | 3 --- 3 files changed, 7 deletions(-) diff --git a/src/protocols/rdp/rdp_settings.c b/src/protocols/rdp/rdp_settings.c index 0af64437..e1358303 100644 --- a/src/protocols/rdp/rdp_settings.c +++ b/src/protocols/rdp/rdp_settings.c @@ -35,9 +35,6 @@ #include "compat/winpr-wtypes.h" #endif -#ifdef ENABLE_COMMON_SSH -#include -#endif #include #include diff --git a/src/protocols/ssh/settings.c b/src/protocols/ssh/settings.c index 7820cf63..7cfe404f 100644 --- a/src/protocols/ssh/settings.c +++ b/src/protocols/ssh/settings.c @@ -24,7 +24,6 @@ #include -#include #include #include #include diff --git a/src/protocols/vnc/settings.c b/src/protocols/vnc/settings.c index 27524e13..587864a4 100644 --- a/src/protocols/vnc/settings.c +++ b/src/protocols/vnc/settings.c @@ -24,9 +24,6 @@ #include -#ifdef ENABLE_COMMON_SSH -#include -#endif #include #include #include From ac2b4f8d12e89cc6accc6b0f8649dd66075be64a Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Sun, 27 May 2018 16:46:36 -0400 Subject: [PATCH 12/20] GUACAMOLE-527: Check either provided key or key file, if it exists. --- src/common-ssh/ssh.c | 86 ++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 39 deletions(-) diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 54eae3b0..4bd3a3dc 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -520,17 +520,16 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, return NULL; } - /* Check known_hosts, start by getting known_hosts file of user running guacd */ - struct passwd *pw = getpwuid(getuid()); - const char *known_hosts = strcat(pw->pw_dir, "/.ssh/known_hosts"); + /* SSH known host key checking. */ LIBSSH2_KNOWNHOSTS *ssh_known_hosts = libssh2_knownhost_init(session); - libssh2_knownhost_readfile(ssh_known_hosts, known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); + int num_known_hosts = 0; /* Add host key provided from settings */ - if (host_key && strcmp(host_key, "") > 0) { + if (host_key && strcmp(host_key, "") != 0) { int kh_add = libssh2_knownhost_readline(ssh_known_hosts, host_key, strlen(host_key), LIBSSH2_KNOWNHOST_FILE_OPENSSH); + num_known_hosts++; if (kh_add) guac_client_log(client, GUAC_LOG_WARNING, "Failed to add provided host key" @@ -538,43 +537,52 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, } - /* Get fingerprint of host we're connecting to */ - size_t fp_len; - int fp_type; - const char *fingerprint = libssh2_session_hostkey(session, &fp_len, &fp_type); + /* Otherwise, we look for a ssh_known_hosts file within GUACAMOLE_HOME and read that in. */ + else { + const char *known_hosts = "/etc/guacamole/ssh_known_hosts"; + num_known_hosts = libssh2_knownhost_readfile(ssh_known_hosts, known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); + } - if (!fingerprint) - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Failed to get fingerprint for host %s", hostname); + /* If we've found a provided set of host keys, check against them. */ + if (num_known_hosts > 0) { + /* Get fingerprint of host we're connecting to */ + size_t fp_len; + int fp_type; + const char *fingerprint = libssh2_session_hostkey(session, &fp_len, &fp_type); - /* Check fingerprint against known hosts */ - struct libssh2_knownhost *host; - int kh_check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, atoi(port), - fingerprint, fp_len, - LIBSSH2_KNOWNHOST_TYPE_PLAIN| - LIBSSH2_KNOWNHOST_KEYENC_RAW, - &host); - - libssh2_knownhost_free(ssh_known_hosts); - - switch (kh_check) { - case LIBSSH2_KNOWNHOST_CHECK_MATCH: - guac_client_log(client, GUAC_LOG_DEBUG, - "Host key match found for %s", hostname); - break; - case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: - guac_client_log(client, GUAC_LOG_WARNING, - "Host key not found for %s.", hostname); - break; - case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: + if (!fingerprint) guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host key does not match host entry for %s", hostname); - break; - case LIBSSH2_KNOWNHOST_CHECK_FAILURE: - default: - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host %s could not be checked against known hosts.", - hostname); + "Failed to get fingerprint for host %s", hostname); + + /* Check fingerprint against known hosts */ + struct libssh2_knownhost *host; + int kh_check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, atoi(port), + fingerprint, fp_len, + LIBSSH2_KNOWNHOST_TYPE_PLAIN| + LIBSSH2_KNOWNHOST_KEYENC_RAW, + &host); + + libssh2_knownhost_free(ssh_known_hosts); + + switch (kh_check) { + case LIBSSH2_KNOWNHOST_CHECK_MATCH: + guac_client_log(client, GUAC_LOG_DEBUG, + "Host key match found for %s", hostname); + break; + case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host key not found for %s.", hostname); + break; + case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host key does not match known hosts entry for %s", hostname); + break; + case LIBSSH2_KNOWNHOST_CHECK_FAILURE: + default: + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host %s could not be checked against known hosts.", + hostname); + } } /* Store basic session data */ From 428243bb783ac58ecf228a5704b5709c26d9239b Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 28 May 2018 10:12:07 -0400 Subject: [PATCH 13/20] GUACAMOLE-527: Move host key checking to a separate function. --- src/common-ssh/common-ssh/key.h | 50 ++++++++++++++++++++ src/common-ssh/key.c | 78 +++++++++++++++++++++++++++++++ src/common-ssh/ssh.c | 83 +++++++++++---------------------- 3 files changed, 154 insertions(+), 57 deletions(-) diff --git a/src/common-ssh/common-ssh/key.h b/src/common-ssh/common-ssh/key.h index 1754deae..576ba1ba 100644 --- a/src/common-ssh/common-ssh/key.h +++ b/src/common-ssh/common-ssh/key.h @@ -22,6 +22,9 @@ #include "config.h" +#include +#include + #include /** @@ -166,5 +169,52 @@ void guac_common_ssh_key_free(guac_common_ssh_key* key); int guac_common_ssh_key_sign(guac_common_ssh_key* key, const char* data, int length, unsigned char* sig); +/** + * Verifies the fingerprint for the given hostname/port combination against + * one or more known_hosts entries. The known_host entries can either be a + * single host_key, provided by the client, or a set of known_hosts entries + * provided in the /etc/guacamole/ssh_known_hosts file. Failure to correctly + * load the known_hosts entries will result in a connection abort and a returned + * error code. A return code of zero indiciates that either no known_hosts entries + * were provided, or that the verification succeeded (match). Negative values + * indicate internal libssh2 error codes; positive values indicate a failure + * during verification of the fingerprint against the known hosts. + * + * @param session + * A pointer to the LIBSSH2_SESSION structure of the SSH connection already + * in progress. + * + * @param client + * The current guac_client instance for which the known_hosts checking is + * being performed. + * + * @param host_key + * The known host entry provided by the client. If this is non-null and not + * empty, it will be the only host key loaded and used for verification. If + * this is null or empty an attempt will be made to read the + * /etc/guacamole/ssh_known_hosts file and load entries from it. + * + * @param hostname + * The hostname or IP of the server that is being verified. + * + * @param port + * The port number of the server being verified. + * + * @param fingerprint + * The fingering of the server being verified. + * + * @param fp_len + * The length of the fingerprint being verified + * + * @return + * The status of the known_hosts check. This will be zero if no entries + * are provided or if the match succeeds, negative to indicate internal + * libssh2 errors, or positive to indicate failures during host key + * checking. + */ +int guac_common_ssh_verify_host_key(LIBSSH2_SESSION* session, guac_client* client, + const char* host_key, const char* hostname, int port, const char* fingerprint, + const size_t fp_len); + #endif diff --git a/src/common-ssh/key.c b/src/common-ssh/key.c index a05d696d..570da8e8 100644 --- a/src/common-ssh/key.c +++ b/src/common-ssh/key.c @@ -245,3 +245,81 @@ int guac_common_ssh_key_sign(guac_common_ssh_key* key, const char* data, } +int guac_common_ssh_verify_host_key(LIBSSH2_SESSION* session, guac_client* client, + const char* host_key, const char* hostname, int port, const char* fingerprint, + const size_t fp_len) { + + LIBSSH2_KNOWNHOSTS* ssh_known_hosts = libssh2_knownhost_init(session); + int known_hosts = 0; + + /* Add host key provided from settings */ + if (host_key && strcmp(host_key, "") != 0) { + + known_hosts = libssh2_knownhost_readline(ssh_known_hosts, host_key, strlen(host_key), + LIBSSH2_KNOWNHOST_FILE_OPENSSH); + + /* readline function returns 0 on success, so we increment to indicate a valid entry */ + if (known_hosts == 0) + known_hosts++; + + } + + /* Otherwise, we look for a ssh_known_hosts file within GUACAMOLE_HOME and read that in. */ + else { + + const char *guac_known_hosts = "/etc/guacamole/ssh_known_hosts"; + known_hosts = libssh2_knownhost_readfile(ssh_known_hosts, guac_known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); + + } + + /* If there's an error provided, abort connection and return that. */ + if (known_hosts < 0) { + + guac_client_log(client, GUAC_LOG_ERROR, + "Failure trying to load SSH host keys."); + + libssh2_knownhost_free(ssh_known_hosts); + return known_hosts; + + } + + /* No host keys were loaded, so we bail out checking and continue the connection. */ + else if (known_hosts == 0) { + libssh2_knownhost_free(ssh_known_hosts); + return known_hosts; + } + + + /* Check fingerprint against known hosts */ + int kh_check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, port, + fingerprint, fp_len, + LIBSSH2_KNOWNHOST_TYPE_PLAIN| + LIBSSH2_KNOWNHOST_KEYENC_RAW, + NULL); + + /* Deal with the return of the host key check */ + switch (kh_check) { + case LIBSSH2_KNOWNHOST_CHECK_MATCH: + guac_client_log(client, GUAC_LOG_DEBUG, + "Host key match found for %s", hostname); + break; + case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: + guac_client_log(client, GUAC_LOG_ERROR, + "Host key not found for %s.", hostname); + break; + case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: + guac_client_log(client, GUAC_LOG_ERROR, + "Host key does not match known hosts entry for %s", hostname); + break; + case LIBSSH2_KNOWNHOST_CHECK_FAILURE: + default: + guac_client_log(client, GUAC_LOG_ERROR, + "Host %s could not be checked against known hosts.", + hostname); + } + + /* Return the check value */ + libssh2_knownhost_free(ssh_known_hosts); + return kh_check; + +} diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 4bd3a3dc..efdef838 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -520,69 +520,38 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, return NULL; } + /* Get fingerprint of host we're connecting to */ + size_t fp_len; + int fp_type; + const char *fingerprint = libssh2_session_hostkey(session, &fp_len, &fp_type); + + /* Failure to generate a fingerprint means we should abort */ + if (!fingerprint) { + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Failed to get fingerprint for host %s", hostname); + return NULL; + } + /* SSH known host key checking. */ - LIBSSH2_KNOWNHOSTS *ssh_known_hosts = libssh2_knownhost_init(session); - int num_known_hosts = 0; + int known_host_check = guac_common_ssh_verify_host_key(session, client, host_key, + hostname, atoi(port), fingerprint, + fp_len); - /* Add host key provided from settings */ - if (host_key && strcmp(host_key, "") != 0) { + /* Abort on any error codes */ + if (known_host_check != 0) { + char* err_msg; + int err_len; + libssh2_session_last_error(session, &err_msg, &err_len, 0); - int kh_add = libssh2_knownhost_readline(ssh_known_hosts, host_key, strlen(host_key), - LIBSSH2_KNOWNHOST_FILE_OPENSSH); - num_known_hosts++; - - if (kh_add) - guac_client_log(client, GUAC_LOG_WARNING, "Failed to add provided host key" - " to known hosts store for %s. Error was %d", hostname, kh_add); - - } - - /* Otherwise, we look for a ssh_known_hosts file within GUACAMOLE_HOME and read that in. */ - else { - const char *known_hosts = "/etc/guacamole/ssh_known_hosts"; - num_known_hosts = libssh2_knownhost_readfile(ssh_known_hosts, known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); - } - - /* If we've found a provided set of host keys, check against them. */ - if (num_known_hosts > 0) { - /* Get fingerprint of host we're connecting to */ - size_t fp_len; - int fp_type; - const char *fingerprint = libssh2_session_hostkey(session, &fp_len, &fp_type); - - if (!fingerprint) + if (known_host_check < 0) guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Failed to get fingerprint for host %s", hostname); + "Error occurred attempting to check host key: %s", err_msg); - /* Check fingerprint against known hosts */ - struct libssh2_knownhost *host; - int kh_check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, atoi(port), - fingerprint, fp_len, - LIBSSH2_KNOWNHOST_TYPE_PLAIN| - LIBSSH2_KNOWNHOST_KEYENC_RAW, - &host); + if (known_host_check > 0) + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Host fingerprint did not match any provided known host keys: %s", err_msg); - libssh2_knownhost_free(ssh_known_hosts); - - switch (kh_check) { - case LIBSSH2_KNOWNHOST_CHECK_MATCH: - guac_client_log(client, GUAC_LOG_DEBUG, - "Host key match found for %s", hostname); - break; - case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host key not found for %s.", hostname); - break; - case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host key does not match known hosts entry for %s", hostname); - break; - case LIBSSH2_KNOWNHOST_CHECK_FAILURE: - default: - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host %s could not be checked against known hosts.", - hostname); - } + return NULL; } /* Store basic session data */ From 27c977adb2a78e861a4fe5272ab8a41d8c3cd8c0 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 28 May 2018 10:23:47 -0400 Subject: [PATCH 14/20] GUACAMOLE-527: Make sure ssh_known_hosts exists before trying to load. --- src/common-ssh/key.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common-ssh/key.c b/src/common-ssh/key.c index 570da8e8..66b8386b 100644 --- a/src/common-ssh/key.c +++ b/src/common-ssh/key.c @@ -35,6 +35,7 @@ #include #include +#include guac_common_ssh_key* guac_common_ssh_key_alloc(char* data, int length, char* passphrase) { @@ -268,7 +269,8 @@ int guac_common_ssh_verify_host_key(LIBSSH2_SESSION* session, guac_client* clien else { const char *guac_known_hosts = "/etc/guacamole/ssh_known_hosts"; - known_hosts = libssh2_knownhost_readfile(ssh_known_hosts, guac_known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); + if (access(guac_known_hosts, F_OK) != -1) + known_hosts = libssh2_knownhost_readfile(ssh_known_hosts, guac_known_hosts, LIBSSH2_KNOWNHOST_FILE_OPENSSH); } From ebbb7492e77e1d0e9f9f6e12269e723a330e8f63 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 28 May 2018 10:27:44 -0400 Subject: [PATCH 15/20] GUACAMOLE-527: Add warning if no known host keys are provided. --- src/common-ssh/key.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/common-ssh/key.c b/src/common-ssh/key.c index 66b8386b..b7c73f08 100644 --- a/src/common-ssh/key.c +++ b/src/common-ssh/key.c @@ -287,6 +287,8 @@ int guac_common_ssh_verify_host_key(LIBSSH2_SESSION* session, guac_client* clien /* No host keys were loaded, so we bail out checking and continue the connection. */ else if (known_hosts == 0) { + guac_client_log(client, GUAC_LOG_WARNING, + "No known host keys provided, host identity will not be verified."); libssh2_knownhost_free(ssh_known_hosts); return known_hosts; } From 7e254955e8dc58f5d3d487ef45c05d6991a3792a Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 28 May 2018 10:31:34 -0400 Subject: [PATCH 16/20] GUACAMOLE-527: Slight tweak to error message. --- src/common-ssh/ssh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index efdef838..4ed6d1d1 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -549,7 +549,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, if (known_host_check > 0) guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host fingerprint did not match any provided known host keys: %s", err_msg); + "Host fingerprint did not match any provided known host keys. %s", err_msg); return NULL; } From f9379dc6bb9d2dbdc280b99604ce0520f9b426ec Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 25 Jun 2018 08:31:00 -0400 Subject: [PATCH 17/20] GUACAMOLE-527: Get full error message when key verification fails. --- src/common-ssh/key.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common-ssh/key.c b/src/common-ssh/key.c index b7c73f08..4d569db7 100644 --- a/src/common-ssh/key.c +++ b/src/common-ssh/key.c @@ -277,8 +277,10 @@ int guac_common_ssh_verify_host_key(LIBSSH2_SESSION* session, guac_client* clien /* If there's an error provided, abort connection and return that. */ if (known_hosts < 0) { + char* errmsg; + int errval = libssh2_session_last_error(session, &errmsg, NULL, 0); guac_client_log(client, GUAC_LOG_ERROR, - "Failure trying to load SSH host keys."); + "Error %d trying to load SSH host keys: %s", errval, errmsg); libssh2_knownhost_free(ssh_known_hosts); return known_hosts; From ba684962b6f19e4ddd0ffa1435f0fe8563b978a9 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 25 Jun 2018 13:50:19 -0400 Subject: [PATCH 18/20] GUACAMOLE-527: Plug some memory leaks before returning NULL. --- src/common-ssh/ssh.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 4ed6d1d1..1b13520c 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -529,6 +529,8 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, if (!fingerprint) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Failed to get fingerprint for host %s", hostname); + free(common_session); + close(fd); return NULL; } @@ -551,6 +553,8 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Host fingerprint did not match any provided known host keys. %s", err_msg); + free(common_session); + close(fd); return NULL; } From 7bc6a623650b8b0d3c2e24d6b78cf288a81923a6 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 25 Jun 2018 13:57:01 -0400 Subject: [PATCH 19/20] GUACAMOLE-527: Do not call a remote host key a fingerprint. --- src/common-ssh/common-ssh/key.h | 16 ++++++++-------- src/common-ssh/key.c | 8 ++++---- src/common-ssh/ssh.c | 19 +++++++++---------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/common-ssh/common-ssh/key.h b/src/common-ssh/common-ssh/key.h index 576ba1ba..897555a3 100644 --- a/src/common-ssh/common-ssh/key.h +++ b/src/common-ssh/common-ssh/key.h @@ -170,7 +170,7 @@ int guac_common_ssh_key_sign(guac_common_ssh_key* key, const char* data, int length, unsigned char* sig); /** - * Verifies the fingerprint for the given hostname/port combination against + * Verifies the host key for the given hostname/port combination against * one or more known_hosts entries. The known_host entries can either be a * single host_key, provided by the client, or a set of known_hosts entries * provided in the /etc/guacamole/ssh_known_hosts file. Failure to correctly @@ -178,7 +178,7 @@ int guac_common_ssh_key_sign(guac_common_ssh_key* key, const char* data, * error code. A return code of zero indiciates that either no known_hosts entries * were provided, or that the verification succeeded (match). Negative values * indicate internal libssh2 error codes; positive values indicate a failure - * during verification of the fingerprint against the known hosts. + * during verification of the host key against the known hosts. * * @param session * A pointer to the LIBSSH2_SESSION structure of the SSH connection already @@ -200,11 +200,11 @@ int guac_common_ssh_key_sign(guac_common_ssh_key* key, const char* data, * @param port * The port number of the server being verified. * - * @param fingerprint - * The fingering of the server being verified. + * @param remote_hostkey + * The host key of the remote system being verified. * - * @param fp_len - * The length of the fingerprint being verified + * @param remote_hostkey_len + * The length of the remote host key being verified * * @return * The status of the known_hosts check. This will be zero if no entries @@ -213,8 +213,8 @@ int guac_common_ssh_key_sign(guac_common_ssh_key* key, const char* data, * checking. */ int guac_common_ssh_verify_host_key(LIBSSH2_SESSION* session, guac_client* client, - const char* host_key, const char* hostname, int port, const char* fingerprint, - const size_t fp_len); + const char* host_key, const char* hostname, int port, const char* remote_hostkey, + const size_t remote_hostkey_len); #endif diff --git a/src/common-ssh/key.c b/src/common-ssh/key.c index 4d569db7..f835e4cc 100644 --- a/src/common-ssh/key.c +++ b/src/common-ssh/key.c @@ -247,8 +247,8 @@ int guac_common_ssh_key_sign(guac_common_ssh_key* key, const char* data, } int guac_common_ssh_verify_host_key(LIBSSH2_SESSION* session, guac_client* client, - const char* host_key, const char* hostname, int port, const char* fingerprint, - const size_t fp_len) { + const char* host_key, const char* hostname, int port, const char* remote_hostkey, + const size_t remote_hostkey_len) { LIBSSH2_KNOWNHOSTS* ssh_known_hosts = libssh2_knownhost_init(session); int known_hosts = 0; @@ -296,9 +296,9 @@ int guac_common_ssh_verify_host_key(LIBSSH2_SESSION* session, guac_client* clien } - /* Check fingerprint against known hosts */ + /* Check remote host key against known hosts */ int kh_check = libssh2_knownhost_checkp(ssh_known_hosts, hostname, port, - fingerprint, fp_len, + remote_hostkey, remote_hostkey_len, LIBSSH2_KNOWNHOST_TYPE_PLAIN| LIBSSH2_KNOWNHOST_KEYENC_RAW, NULL); diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 1b13520c..831245db 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -520,15 +520,14 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, return NULL; } - /* Get fingerprint of host we're connecting to */ - size_t fp_len; - int fp_type; - const char *fingerprint = libssh2_session_hostkey(session, &fp_len, &fp_type); + /* Get host key of remote system we're connecting to */ + size_t remote_hostkey_len; + const char *remote_hostkey = libssh2_session_hostkey(session, &remote_hostkey_len, NULL); - /* Failure to generate a fingerprint means we should abort */ - if (!fingerprint) { + /* Failure to retrieve a host key means we should abort */ + if (!remote_hostkey) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Failed to get fingerprint for host %s", hostname); + "Failed to get host key for %s", hostname); free(common_session); close(fd); return NULL; @@ -536,8 +535,8 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, /* SSH known host key checking. */ int known_host_check = guac_common_ssh_verify_host_key(session, client, host_key, - hostname, atoi(port), fingerprint, - fp_len); + hostname, atoi(port), remote_hostkey, + remote_hostkey_len); /* Abort on any error codes */ if (known_host_check != 0) { @@ -551,7 +550,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, if (known_host_check > 0) guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, - "Host fingerprint did not match any provided known host keys. %s", err_msg); + "Host key did not match any provided known host keys. %s", err_msg); free(common_session); close(fd); From fe44fd7c3b57cf74c7182eb1e9538baed5d238f8 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Mon, 25 Jun 2018 20:04:09 -0400 Subject: [PATCH 20/20] GUACAMOLE-527: Remove unused error message length variable. --- src/common-ssh/ssh.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/common-ssh/ssh.c b/src/common-ssh/ssh.c index 831245db..9dde5111 100644 --- a/src/common-ssh/ssh.c +++ b/src/common-ssh/ssh.c @@ -541,8 +541,7 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client, /* Abort on any error codes */ if (known_host_check != 0) { char* err_msg; - int err_len; - libssh2_session_last_error(session, &err_msg, &err_len, 0); + libssh2_session_last_error(session, &err_msg, NULL, 0); if (known_host_check < 0) guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR,