From d8618b0682e69ca5c99f0608368f21188d16fce3 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 26 Sep 2018 21:50:19 -0700 Subject: [PATCH 1/5] GUACAMOLE-623: Support older libwebsockets SSL initialization. --- configure.ac | 22 ++++++++++++++++++++-- src/protocols/kubernetes/kubernetes.c | 6 ++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index d26db39f..bb23f62c 100644 --- a/configure.ac +++ b/configure.ac @@ -1198,14 +1198,32 @@ then have_libwebsockets=no]) fi -# Check for client-specific closed event, which must be used in favor of the -# generic closed event if libwebsockets is recent enough to provide this if test "x$with_websockets" != "xno" then + + # Check for client-specific closed event, which must be used in favor of + # the generic closed event if libwebsockets is recent enough to provide + # this AC_CHECK_DECL([LWS_CALLBACK_CLIENT_CLOSED], [AC_DEFINE([HAVE_LWS_CALLBACK_CLIENT_CLOSED],, [Whether LWS_CALLBACK_CLIENT_CLOSED is defined])],, [#include ]) + + # Older versions of libwebsockets may not define a flag for requesting + # global initialization of OpenSSL, instead performing that initialization + # by default + AC_CHECK_DECL([LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT], + [AC_DEFINE([HAVE_LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT],, + [Whether LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT is defined])],, + [#include ]) + + # Older versions of libwebsockets do not define special macros for SSL + # connection flags, instead relying on documented integer values + AC_CHECK_DECL([LCCSCF_USE_SSL], + [AC_DEFINE([HAVE_LCCSCF_USE_SSL],, + [Whether LCCSCF_USE_SSL is defined])],, + [#include ]) + fi AM_CONDITIONAL([ENABLE_WEBSOCKETS], diff --git a/src/protocols/kubernetes/kubernetes.c b/src/protocols/kubernetes/kubernetes.c index f314c597..9cb0b13b 100644 --- a/src/protocols/kubernetes/kubernetes.c +++ b/src/protocols/kubernetes/kubernetes.c @@ -268,9 +268,15 @@ void* guac_kubernetes_client_thread(void* data) { * do our own validation - libwebsockets does not validate properly if * IP addresses are used. */ if (settings->use_ssl) { +#ifdef HAVE_LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT context_info.options = LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT; +#endif +#ifdef HAVE_LCCSCF_USE_SSL connection_info.ssl_connection = LCCSCF_USE_SSL | LCCSCF_SKIP_SERVER_CERT_HOSTNAME_CHECK; +#else + connection_info.ssl_connection = 2; /* SSL + no hostname check */ +#endif } /* Create libwebsockets context */ From b48a1b3a5d15e6c95fa64222342033eb584dc0a3 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 26 Sep 2018 21:51:07 -0700 Subject: [PATCH 2/5] GUACAMOLE-623: Use libwebsockets' dummy callback only if defined. --- configure.ac | 8 ++++++++ src/protocols/kubernetes/kubernetes.c | 11 ++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index bb23f62c..672d19eb 100644 --- a/configure.ac +++ b/configure.ac @@ -1224,6 +1224,14 @@ then [Whether LCCSCF_USE_SSL is defined])],, [#include ]) + # Older versions of libwebsockets do not define a dummy callback which + # must be invoked after the main event callback is invoked; the main event + # callback must instead manually return zero + AC_CHECK_DECL([lws_callback_http_dummy], + [AC_DEFINE([HAVE_LWS_CALLBACK_HTTP_DUMMY],, + [Whether lws_callback_http_dummy() is defined])],, + [#include ]) + fi AM_CONDITIONAL([ENABLE_WEBSOCKETS], diff --git a/src/protocols/kubernetes/kubernetes.c b/src/protocols/kubernetes/kubernetes.c index 9cb0b13b..fb38d680 100644 --- a/src/protocols/kubernetes/kubernetes.c +++ b/src/protocols/kubernetes/kubernetes.c @@ -66,8 +66,13 @@ static int guac_kubernetes_lws_callback(struct lws* wsi, guac_client* client = guac_kubernetes_lws_current_client; /* Do not handle any further events if connection is closing */ - if (client->state != GUAC_CLIENT_RUNNING) + if (client->state != GUAC_CLIENT_RUNNING) { +#ifdef HAVE_LWS_CALLBACK_HTTP_DUMMY return lws_callback_http_dummy(wsi, reason, user, in, length); +#else + return 0; +#endif + } switch (reason) { @@ -127,7 +132,11 @@ static int guac_kubernetes_lws_callback(struct lws* wsi, } +#ifdef HAVE_LWS_CALLBACK_HTTP_DUMMY return lws_callback_http_dummy(wsi, reason, user, in, length); +#else + return 0; +#endif } From 7ee624844a2ef577bfd5af6c9559919e1ba77846 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 26 Sep 2018 21:51:46 -0700 Subject: [PATCH 3/5] GUACAMOLE-623: Remove unnecessary initialization of pwsi. The pwsi member was previously used to ensure the lws structure was made available to invocations of the event callback early in the connection lifecycle such that the underlyin guac_client could always be retrieved. Since the migration to guac_kubernetes_lws_current_client, this is not necessary, and isn't supported in older versions of libwebsockets anyway. --- src/protocols/kubernetes/kubernetes.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/protocols/kubernetes/kubernetes.c b/src/protocols/kubernetes/kubernetes.c index fb38d680..66fc27d4 100644 --- a/src/protocols/kubernetes/kubernetes.c +++ b/src/protocols/kubernetes/kubernetes.c @@ -268,7 +268,6 @@ void* guac_kubernetes_client_thread(void* data) { .origin = settings->hostname, .port = settings->port, .protocol = GUAC_KUBERNETES_LWS_PROTOCOL, - .pwsi = &kubernetes_client->wsi, .userdata = client }; From 44d3433ea92de6de7d127f93335c0a5be47c735c Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 26 Sep 2018 22:01:43 -0700 Subject: [PATCH 4/5] GUACAMOLE-623: Explicitly bypass certificate checks if requested. For older versions of libwebsockets, simply requesting that OpenSSL ignore the verification result is insufficient, as libwebsockets manually checks and confirms the verification result, producing an error in all but specific cases. --- src/protocols/kubernetes/ssl.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/protocols/kubernetes/ssl.c b/src/protocols/kubernetes/ssl.c index 6ebafc61..520ce8cb 100644 --- a/src/protocols/kubernetes/ssl.c +++ b/src/protocols/kubernetes/ssl.c @@ -110,6 +110,27 @@ static EVP_PKEY* guac_kubernetes_read_key(char* pem) { } +/** + * OpenSSL certificate verification callback which universally accepts all + * certificates without performing any verification at all. + * + * @param x509_ctx + * The current context of the certificate verification process. This + * parameter is ignored by this particular implementation of the callback. + * + * @param arg + * The arbitrary value passed to SSL_CTX_set_cert_verify_callback(). This + * parameter is ignored by this particular implementation of the callback. + * + * @return + * Strictly 0 if certificate verification fails, 1 if the certificate is + * verified. No other values are legal return values for this callback as + * documented by OpenSSL. + */ +static int guac_kubernetes_assume_cert_ok(X509_STORE_CTX* x509_ctx, void* arg) { + return 1; +} + void guac_kubernetes_init_ssl(guac_client* client, SSL_CTX* context) { guac_kubernetes_client* kubernetes_client = @@ -118,8 +139,11 @@ void guac_kubernetes_init_ssl(guac_client* client, SSL_CTX* context) { guac_kubernetes_settings* settings = kubernetes_client->settings; /* Bypass certificate checks if requested */ - if (settings->ignore_cert) - SSL_CTX_set_verify(context, SSL_VERIFY_NONE, NULL); + if (settings->ignore_cert) { + SSL_CTX_set_verify(context, SSL_VERIFY_PEER, NULL); + SSL_CTX_set_cert_verify_callback(context, + guac_kubernetes_assume_cert_ok, NULL); + } /* Otherwise use the given CA certificate to validate (if any) */ else if (settings->ca_cert != NULL) { From 9c593bde89891522324fe84034e4a9bd415aae2c Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 26 Sep 2018 22:30:08 -0700 Subject: [PATCH 5/5] GUACAMOLE-623: Kill connection if libwebsockets is destroying the underlying WebSocket. Older versions of libwebsockets will not necessarily invoke close events under all circumstances, and will instead sometimes summarily destroy the WebSocket. Thankfully there is another event for that, and newer versions of libwebsockets continue to define that event. We can hook into both to handle disconnect. --- src/protocols/kubernetes/kubernetes.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/protocols/kubernetes/kubernetes.c b/src/protocols/kubernetes/kubernetes.c index 66fc27d4..e115fc57 100644 --- a/src/protocols/kubernetes/kubernetes.c +++ b/src/protocols/kubernetes/kubernetes.c @@ -120,6 +120,7 @@ static int guac_kubernetes_lws_callback(struct lws* wsi, #endif /* Connection closed */ + case LWS_CALLBACK_WSI_DESTROY: case LWS_CALLBACK_CLOSED: guac_client_stop(client); guac_client_log(client, GUAC_LOG_DEBUG, "WebSocket connection to "