From 32a1406dc4f64542eff4606b7aa51719b70bd12a Mon Sep 17 00:00:00 2001 From: bjcollins Date: Tue, 4 Aug 2015 16:52:44 -0500 Subject: [PATCH 1/4] Return FREERDP_ERROR_AUTHENTICATION_FAILED on an authentication failure when using NLA with xfreerdp. --- client/X11/xf_client.c | 7 +++++-- client/X11/xfreerdp.h | 1 + include/freerdp/freerdp.h | 2 ++ libfreerdp/core/connection.c | 13 ++++++++++--- libfreerdp/core/freerdp.c | 12 ++++++++++++ libfreerdp/core/transport.c | 29 +++++++++++++++++++++++++++++ libfreerdp/core/transport.h | 2 ++ 7 files changed, 61 insertions(+), 5 deletions(-) diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index a788ab2b1..31efca8b7 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -1530,7 +1530,10 @@ void* xf_client_thread(void* param) if (instance->settings->AuthenticationOnly || !status) { WLog_ERR(TAG, "Authentication only, exit status %d", !status); - exit_code = XF_EXIT_CONN_FAILED; + if (freerdp_get_nla_failure(instance)) + exit_code = XF_EXIT_NLA_AUTH_FAILURE; + else + exit_code = XF_EXIT_CONN_FAILED; goto disconnect; } @@ -1641,7 +1644,7 @@ disconnect: DWORD xf_exit_code_from_disconnect_reason(DWORD reason) { - if (reason == 0 || (reason >= XF_EXIT_PARSE_ARGUMENTS && reason <= XF_EXIT_CONN_FAILED)) + if (reason == 0 || (reason >= XF_EXIT_PARSE_ARGUMENTS && reason <= XF_EXIT_NLA_AUTH_FAILURE)) return reason; /* License error set */ else if (reason >= 0x100 && reason <= 0x10A) diff --git a/client/X11/xfreerdp.h b/client/X11/xfreerdp.h index 5e1bdfab1..ec02fd00b 100644 --- a/client/X11/xfreerdp.h +++ b/client/X11/xfreerdp.h @@ -268,6 +268,7 @@ enum XF_EXIT_CODE XF_EXIT_MEMORY = 129, XF_EXIT_PROTOCOL = 130, XF_EXIT_CONN_FAILED = 131, + XF_EXIT_NLA_AUTH_FAILURE = 132, XF_EXIT_UNKNOWN = 255, }; diff --git a/include/freerdp/freerdp.h b/include/freerdp/freerdp.h index d2db002fa..b4c42547a 100644 --- a/include/freerdp/freerdp.h +++ b/include/freerdp/freerdp.h @@ -293,6 +293,8 @@ FREERDP_API const char* getChannelErrorDescription(rdpContext* context); FREERDP_API void setChannelError(rdpContext* context, UINT errorNum, char* description); FREERDP_API BOOL checkChannelErrorEvent(rdpContext* context); +FREERDP_API BOOL freerdp_get_nla_failure(freerdp* instance); + #ifdef __cplusplus } #endif diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c index 295ed9907..c84e0a2e0 100644 --- a/libfreerdp/core/connection.c +++ b/libfreerdp/core/connection.c @@ -299,9 +299,16 @@ BOOL rdp_client_connect(rdpRdp* rdp) { if (rdp_check_fds(rdp) < 0) { - if (!freerdp_get_last_error(rdp->context)) - freerdp_set_last_error(rdp->context, FREERDP_ERROR_CONNECT_TRANSPORT_FAILED); - + if (rdp->transport->nlaFailure == TRUE) + { + if (!freerdp_get_last_error(rdp->context)) + freerdp_set_last_error(rdp->context, FREERDP_ERROR_AUTHENTICATION_FAILED); + } + else + { + if (!freerdp_get_last_error(rdp->context)) + freerdp_set_last_error(rdp->context, FREERDP_ERROR_CONNECT_TRANSPORT_FAILED); + } return FALSE; } } diff --git a/libfreerdp/core/freerdp.c b/libfreerdp/core/freerdp.c index cce07fe3b..00a37b332 100644 --- a/libfreerdp/core/freerdp.c +++ b/libfreerdp/core/freerdp.c @@ -815,3 +815,15 @@ FREERDP_API void setChannelError(rdpContext* context, UINT errorNum, char* descr strncpy(context->errorDescription, description, 499); SetEvent(context->channelErrorEvent); } + +BOOL freerdp_get_nla_failure(freerdp* instance) +{ + rdpRdp* rdp; + + rdp = instance->context->rdp; + + if (transport_get_nla_failure(rdp->transport)) + return TRUE; + + return FALSE; +} diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c index f20a5aa03..18b0b0d61 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -58,6 +58,17 @@ static void* transport_client_thread(void* arg); + +static void test_function(SSL* ssl, int where, int ret) +{ + rdpTransport *transport; + if ((where | SSL_CB_ALERT) && (ret == 561)) + { + transport = (rdpTransport *) SSL_get_app_data(ssl); + transport->nlaFailure = TRUE; + } +} + wStream* transport_send_stream_init(rdpTransport* transport, int size) { wStream* s; @@ -146,6 +157,9 @@ BOOL transport_connect_tls(rdpTransport* transport) transport->frontBio = tls->bio; + BIO_callback_ctrl(tls->bio, BIO_CTRL_SET_CALLBACK, (bio_info_cb*) test_function); + SSL_set_app_data(tls->ssl, transport); + if (!transport->frontBio) { WLog_ERR(TAG, "unable to prepend a filtering TLS bio"); @@ -186,6 +200,7 @@ BOOL transport_connect_nla(rdpTransport* transport) if (nla_client_begin(rdp->nla) < 0) { + transport->nlaFailure = TRUE; if (!freerdp_get_last_error(context)) freerdp_set_last_error(context, FREERDP_ERROR_AUTHENTICATION_FAILED); @@ -343,6 +358,7 @@ BOOL transport_accept_nla(rdpTransport* transport) if (nla_authenticate(transport->nla) < 0) { + transport->nlaFailure = TRUE; WLog_ERR(TAG, "client authentication failure"); transport_set_nla_mode(transport, FALSE); nla_free(transport->nla); @@ -989,6 +1005,7 @@ rdpTransport* transport_new(rdpContext* context) transport->blocking = TRUE; transport->GatewayEnabled = FALSE; transport->layer = TRANSPORT_LAYER_TCP; + transport->nlaFailure = FALSE; if (!InitializeCriticalSectionAndSpinCount(&(transport->ReadLock), 4000)) goto out_free_connectedEvent; @@ -1027,3 +1044,15 @@ void transport_free(rdpTransport* transport) free(transport); } + +BOOL transport_get_nla_failure(rdpTransport* transport) +{ + if (transport != NULL) + { + if (transport->nlaFailure == TRUE) + return TRUE; + } + + return FALSE; +} + diff --git a/libfreerdp/core/transport.h b/libfreerdp/core/transport.h index 1c6c9dcc0..43eabbee4 100644 --- a/libfreerdp/core/transport.h +++ b/libfreerdp/core/transport.h @@ -77,6 +77,7 @@ struct rdp_transport CRITICAL_SECTION ReadLock; CRITICAL_SECTION WriteLock; ULONG written; + BOOL nlaFailure; }; wStream* transport_send_stream_init(rdpTransport* transport, int size); @@ -109,5 +110,6 @@ int transport_receive_pool_return(rdpTransport* transport, wStream* pdu); rdpTransport* transport_new(rdpContext* context); void transport_free(rdpTransport* transport); +BOOL transport_get_nla_failure(rdpTransport* transport); #endif From 7fbc7e45a7275fdc6066e1beb2ac718c858125d8 Mon Sep 17 00:00:00 2001 From: bjcollins Date: Thu, 3 Sep 2015 14:45:40 -0500 Subject: [PATCH 2/4] Clean up NLA authentication failure handling code 1. Make use of freerdp_set_last_error to set authentication failure without the helper functions 2. Rename ssl callback function 3. Break out AuthenticationOnly exit handling from bad connect handling --- client/X11/xf_client.c | 26 +++++++++++++++++++++----- client/X11/xfreerdp.h | 2 +- libfreerdp/core/freerdp.c | 12 ------------ libfreerdp/core/transport.c | 18 ++---------------- 4 files changed, 24 insertions(+), 34 deletions(-) diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index 31efca8b7..9a3bffa1e 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -1526,12 +1526,28 @@ void* xf_client_thread(void* param) xfc = (xfContext*) instance->context; - /* Connection succeeded. --authonly ? */ - if (instance->settings->AuthenticationOnly || !status) + /* --authonly ? */ + if (instance->settings->AuthenticationOnly) { WLog_ERR(TAG, "Authentication only, exit status %d", !status); - if (freerdp_get_nla_failure(instance)) - exit_code = XF_EXIT_NLA_AUTH_FAILURE; + if (!status) + { + if (freerdp_get_last_error(instance->context) == FREERDP_ERROR_AUTHENTICATION_FAILED) + exit_code = XF_EXIT_AUTH_FAILURE; + else + exit_code = XF_EXIT_CONN_FAILED; + } + else + exit_code = XF_EXIT_SUCCESS; + goto disconnect; + } + + if (!status) + { + WLog_ERR(TAG, "Freerdp connect error exit status %d", !status); + exit_code = freerdp_error_info(instance); + if (freerdp_get_last_error(instance->context) == FREERDP_ERROR_AUTHENTICATION_FAILED) + exit_code = XF_EXIT_AUTH_FAILURE; else exit_code = XF_EXIT_CONN_FAILED; goto disconnect; @@ -1644,7 +1660,7 @@ disconnect: DWORD xf_exit_code_from_disconnect_reason(DWORD reason) { - if (reason == 0 || (reason >= XF_EXIT_PARSE_ARGUMENTS && reason <= XF_EXIT_NLA_AUTH_FAILURE)) + if (reason == 0 || (reason >= XF_EXIT_PARSE_ARGUMENTS && reason <= XF_EXIT_AUTH_FAILURE)) return reason; /* License error set */ else if (reason >= 0x100 && reason <= 0x10A) diff --git a/client/X11/xfreerdp.h b/client/X11/xfreerdp.h index ec02fd00b..af5f9505c 100644 --- a/client/X11/xfreerdp.h +++ b/client/X11/xfreerdp.h @@ -268,7 +268,7 @@ enum XF_EXIT_CODE XF_EXIT_MEMORY = 129, XF_EXIT_PROTOCOL = 130, XF_EXIT_CONN_FAILED = 131, - XF_EXIT_NLA_AUTH_FAILURE = 132, + XF_EXIT_AUTH_FAILURE = 132, XF_EXIT_UNKNOWN = 255, }; diff --git a/libfreerdp/core/freerdp.c b/libfreerdp/core/freerdp.c index 00a37b332..cce07fe3b 100644 --- a/libfreerdp/core/freerdp.c +++ b/libfreerdp/core/freerdp.c @@ -815,15 +815,3 @@ FREERDP_API void setChannelError(rdpContext* context, UINT errorNum, char* descr strncpy(context->errorDescription, description, 499); SetEvent(context->channelErrorEvent); } - -BOOL freerdp_get_nla_failure(freerdp* instance) -{ - rdpRdp* rdp; - - rdp = instance->context->rdp; - - if (transport_get_nla_failure(rdp->transport)) - return TRUE; - - return FALSE; -} diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c index 18b0b0d61..adf8a93d7 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -59,7 +59,7 @@ static void* transport_client_thread(void* arg); -static void test_function(SSL* ssl, int where, int ret) +static void transport_ssl_cb(SSL* ssl, int where, int ret) { rdpTransport *transport; if ((where | SSL_CB_ALERT) && (ret == 561)) @@ -157,7 +157,7 @@ BOOL transport_connect_tls(rdpTransport* transport) transport->frontBio = tls->bio; - BIO_callback_ctrl(tls->bio, BIO_CTRL_SET_CALLBACK, (bio_info_cb*) test_function); + BIO_callback_ctrl(tls->bio, BIO_CTRL_SET_CALLBACK, (bio_info_cb*) transport_ssl_cb); SSL_set_app_data(tls->ssl, transport); if (!transport->frontBio) @@ -200,7 +200,6 @@ BOOL transport_connect_nla(rdpTransport* transport) if (nla_client_begin(rdp->nla) < 0) { - transport->nlaFailure = TRUE; if (!freerdp_get_last_error(context)) freerdp_set_last_error(context, FREERDP_ERROR_AUTHENTICATION_FAILED); @@ -358,7 +357,6 @@ BOOL transport_accept_nla(rdpTransport* transport) if (nla_authenticate(transport->nla) < 0) { - transport->nlaFailure = TRUE; WLog_ERR(TAG, "client authentication failure"); transport_set_nla_mode(transport, FALSE); nla_free(transport->nla); @@ -1044,15 +1042,3 @@ void transport_free(rdpTransport* transport) free(transport); } - -BOOL transport_get_nla_failure(rdpTransport* transport) -{ - if (transport != NULL) - { - if (transport->nlaFailure == TRUE) - return TRUE; - } - - return FALSE; -} - From be47c6f782c223aa2bbfc101805c0987848f754b Mon Sep 17 00:00:00 2001 From: bjcollins Date: Thu, 3 Sep 2015 15:27:58 -0500 Subject: [PATCH 3/4] Remove unused functions from initial code to handle NLA authentication failures. --- include/freerdp/freerdp.h | 2 -- libfreerdp/core/transport.h | 1 - 2 files changed, 3 deletions(-) diff --git a/include/freerdp/freerdp.h b/include/freerdp/freerdp.h index b4c42547a..d2db002fa 100644 --- a/include/freerdp/freerdp.h +++ b/include/freerdp/freerdp.h @@ -293,8 +293,6 @@ FREERDP_API const char* getChannelErrorDescription(rdpContext* context); FREERDP_API void setChannelError(rdpContext* context, UINT errorNum, char* description); FREERDP_API BOOL checkChannelErrorEvent(rdpContext* context); -FREERDP_API BOOL freerdp_get_nla_failure(freerdp* instance); - #ifdef __cplusplus } #endif diff --git a/libfreerdp/core/transport.h b/libfreerdp/core/transport.h index 43eabbee4..7a025c403 100644 --- a/libfreerdp/core/transport.h +++ b/libfreerdp/core/transport.h @@ -110,6 +110,5 @@ int transport_receive_pool_return(rdpTransport* transport, wStream* pdu); rdpTransport* transport_new(rdpContext* context); void transport_free(rdpTransport* transport); -BOOL transport_get_nla_failure(rdpTransport* transport); #endif From ee3b39d70f7b9104134aa65c1bfb4a9d527ef1f8 Mon Sep 17 00:00:00 2001 From: bjcollins Date: Tue, 15 Sep 2015 14:17:13 -0500 Subject: [PATCH 4/4] Remove unnecessary variable to keep track of nlaFailure, instead just set the NLA authentication error in the callback where it is detected. --- libfreerdp/core/connection.c | 12 ++---------- libfreerdp/core/transport.c | 4 ++-- libfreerdp/core/transport.h | 1 - 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c index c84e0a2e0..5047aadfe 100644 --- a/libfreerdp/core/connection.c +++ b/libfreerdp/core/connection.c @@ -299,16 +299,8 @@ BOOL rdp_client_connect(rdpRdp* rdp) { if (rdp_check_fds(rdp) < 0) { - if (rdp->transport->nlaFailure == TRUE) - { - if (!freerdp_get_last_error(rdp->context)) - freerdp_set_last_error(rdp->context, FREERDP_ERROR_AUTHENTICATION_FAILED); - } - else - { - if (!freerdp_get_last_error(rdp->context)) - freerdp_set_last_error(rdp->context, FREERDP_ERROR_CONNECT_TRANSPORT_FAILED); - } + if (!freerdp_get_last_error(rdp->context)) + freerdp_set_last_error(rdp->context, FREERDP_ERROR_CONNECT_TRANSPORT_FAILED); return FALSE; } } diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c index adf8a93d7..60ee13d22 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -65,7 +65,8 @@ static void transport_ssl_cb(SSL* ssl, int where, int ret) if ((where | SSL_CB_ALERT) && (ret == 561)) { transport = (rdpTransport *) SSL_get_app_data(ssl); - transport->nlaFailure = TRUE; + if (!freerdp_get_last_error(transport->context)) + freerdp_set_last_error(transport->context, FREERDP_ERROR_AUTHENTICATION_FAILED); } } @@ -1003,7 +1004,6 @@ rdpTransport* transport_new(rdpContext* context) transport->blocking = TRUE; transport->GatewayEnabled = FALSE; transport->layer = TRANSPORT_LAYER_TCP; - transport->nlaFailure = FALSE; if (!InitializeCriticalSectionAndSpinCount(&(transport->ReadLock), 4000)) goto out_free_connectedEvent; diff --git a/libfreerdp/core/transport.h b/libfreerdp/core/transport.h index 7a025c403..1c6c9dcc0 100644 --- a/libfreerdp/core/transport.h +++ b/libfreerdp/core/transport.h @@ -77,7 +77,6 @@ struct rdp_transport CRITICAL_SECTION ReadLock; CRITICAL_SECTION WriteLock; ULONG written; - BOOL nlaFailure; }; wStream* transport_send_stream_init(rdpTransport* transport, int size);