Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GUACAMOLE-2012: Fix SSH connection to FIPS servers which only offer AES GCM #572

Open
wants to merge 1 commit into
base: patch
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 63 additions & 1 deletion src/common-ssh/ssh.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
* under the License.
*/

#include "common/string.h"

#include "common-ssh/key.h"
#include "common-ssh/ssh.h"
#include "common-ssh/user.h"
Expand All @@ -39,6 +41,7 @@
#include <netinet/in.h>
#include <pthread.h>
#include <pwd.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdlib.h>
#include <string.h>
Expand All @@ -61,7 +64,7 @@ GCRY_THREAD_OPTION_PTHREAD_IMPL;
/**
* A list of ciphers that are both FIPS-compliant, and OpenSSL-supported.
*/
#define FIPS_COMPLIANT_CIPHERS "aes128-ctr,aes192-ctr,aes256-ctr,aes128-cbc,aes192-cbc,aes256-cbc"
#define FIPS_COMPLIANT_CIPHERS "[email protected],[email protected],aes128-ctr,aes192-ctr,aes256-ctr,aes128-cbc,aes192-cbc,aes256-cbc"

#ifdef OPENSSL_REQUIRES_THREADING_CALLBACKS
/**
Expand Down Expand Up @@ -403,6 +406,57 @@ static int guac_common_ssh_authenticate(guac_common_ssh_session* common_session)

}

/**
* Verifies if given algorithms are supported by libssh2.
* Writes log messages if an algorithm is not supported or
* could not get the list of supported algorithms from libssh2.
*
* @param client
* The Guacamole client that is using SSH.
*
* @param session
* The session associated with the user to be authenticated.
*
* @param method_type
* One of the libssh2 Method Type constants for libssh2_session_method_pref().
*
* @param algs
* A string with preferred list of algorithms, for example FIPS_COMPLIANT_CIPHERS.
*
*/
static void check_if_algs_are_supported(guac_client* client, LIBSSH2_SESSION* session,
int method_type, const char* algs) {

/* Request the list of supported algorithms/cyphers from libssh2. */
const char** supported_algs;
int supported_algs_count =
libssh2_session_supported_algs(session, method_type, &supported_algs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the example at https://libssh2.org/libssh2_session_supported_algs.html, the memory pointed to by supported_algs after a successful call to libssh2_session_supported_algs() must eventually be freed by a call to libssh2_free().


if (supported_algs_count > 0) {
char** preferred_algs = guac_split(algs, ',');
for (int i = 0; preferred_algs[i]; i++) {
bool found = false;
/* Check if the algorithm is found in the libssh2 supported list. */
for (int j = 0; j < supported_algs_count; j++) {
if (strcmp(preferred_algs[i], supported_algs[j]) == 0) {
found = true;
break;
}
}
if (!found) {
guac_client_log(client, GUAC_LOG_WARNING,
"Preferred algorithm/cipher '%s' is not supported by libssh2", preferred_algs[i]);
}
}
guac_mem_free(preferred_algs);
}
else {
guac_client_log(client, GUAC_LOG_WARNING,
"libssh2 reports that no ciphers/algorithms are supported. This may be a bug in libssh2."
"If the SSH connection fails, it may not be possible to log the cause here.");
Comment on lines +455 to +456
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A space is needed at the end of the first string literal here. Without that, the relevant portion of the resulting string will be libssh2.If and not the intended libssh2. If.

}
}

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,
Expand Down Expand Up @@ -507,8 +561,16 @@ guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client,
* https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp2906.pdf
*/
if (guac_fips_enabled()) {
/*
* The following algorithm check is only to simplify debugging.
* libssh2_session_method_pref() ignores unsupported methods.
* So they are not sent to the remote host during protocol negotiation anyways.
*/
check_if_algs_are_supported(client, session, LIBSSH2_METHOD_KEX, FIPS_COMPLIANT_KEX_ALGORITHMS);
libssh2_session_method_pref(session, LIBSSH2_METHOD_KEX, FIPS_COMPLIANT_KEX_ALGORITHMS);
check_if_algs_are_supported(client, session, LIBSSH2_METHOD_CRYPT_CS, FIPS_COMPLIANT_CIPHERS);
libssh2_session_method_pref(session, LIBSSH2_METHOD_CRYPT_CS, FIPS_COMPLIANT_CIPHERS);
check_if_algs_are_supported(client, session, LIBSSH2_METHOD_CRYPT_SC, FIPS_COMPLIANT_CIPHERS);
libssh2_session_method_pref(session, LIBSSH2_METHOD_CRYPT_SC, FIPS_COMPLIANT_CIPHERS);
}

Expand Down