From 54021b50b0fd936fa530a48927c4698630c247f3 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Aug 2023 17:21:49 +0200 Subject: [PATCH] [core,peer] fix initial state transitions update initial state transitions according to [MS-RDPBCGR] the diagram is misleading, some of the text below ambigious, but 1.3.1.1 Connection Sequence phase 10 description lists the dependencies of server initiated messages. --- libfreerdp/core/activation.c | 20 +------------------- libfreerdp/core/activation.h | 1 + libfreerdp/core/connection.c | 23 ++++++++++++++++++++--- libfreerdp/core/peer.c | 16 ++++++++++------ 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/libfreerdp/core/activation.c b/libfreerdp/core/activation.c index 3658954de..335a5e66e 100644 --- a/libfreerdp/core/activation.c +++ b/libfreerdp/core/activation.c @@ -185,7 +185,7 @@ BOOL rdp_send_server_control_cooperate_pdu(rdpRdp* rdp) return rdp_send_data_pdu(rdp, s, DATA_PDU_TYPE_CONTROL, rdp->mcs->userId); } -static BOOL rdp_send_server_control_granted_pdu(rdpRdp* rdp) +BOOL rdp_send_server_control_granted_pdu(rdpRdp* rdp) { wStream* s = rdp_data_pdu_init(rdp); if (!s) @@ -749,24 +749,6 @@ BOOL rdp_server_accept_client_font_list_pdu(rdpRdp* rdp, wStream* s) return FALSE; rdp_finalize_set_flag(rdp, FINALIZE_CS_FONT_LIST_PDU); - if (!rdp_server_transition_to_state(rdp, CONNECTION_STATE_FINALIZATION_CLIENT_SYNC)) - return FALSE; - - if (!rdp_send_server_synchronize_pdu(rdp)) - return FALSE; - - if (!rdp_server_transition_to_state(rdp, CONNECTION_STATE_FINALIZATION_CLIENT_COOPERATE)) - return FALSE; - - if (!rdp_send_server_control_cooperate_pdu(rdp)) - return FALSE; - - if (!rdp_server_transition_to_state(rdp, CONNECTION_STATE_FINALIZATION_CLIENT_GRANTED_CONTROL)) - return FALSE; - - if (!rdp_send_server_control_granted_pdu(rdp)) - return FALSE; - if (!rdp_server_transition_to_state(rdp, CONNECTION_STATE_FINALIZATION_CLIENT_FONT_MAP)) return FALSE; diff --git a/libfreerdp/core/activation.h b/libfreerdp/core/activation.h index 2f90fd945..4d7d95273 100644 --- a/libfreerdp/core/activation.h +++ b/libfreerdp/core/activation.h @@ -71,6 +71,7 @@ FREERDP_LOCAL BOOL rdp_send_client_synchronize_pdu(rdpRdp* rdp); FREERDP_LOCAL BOOL rdp_recv_server_control_pdu(rdpRdp* rdp, wStream* s); FREERDP_LOCAL BOOL rdp_send_server_control_cooperate_pdu(rdpRdp* rdp); FREERDP_LOCAL BOOL rdp_send_client_control_pdu(rdpRdp* rdp, UINT16 action); +FREERDP_LOCAL BOOL rdp_send_server_control_granted_pdu(rdpRdp* rdp); FREERDP_LOCAL BOOL rdp_send_client_persistent_key_list_pdu(rdpRdp* rdp); FREERDP_LOCAL BOOL rdp_send_client_font_list_pdu(rdpRdp* rdp, UINT16 flags); FREERDP_LOCAL BOOL rdp_recv_font_map_pdu(rdpRdp* rdp, wStream* s); diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c index 6b3bcf7fe..dfa10a717 100644 --- a/libfreerdp/core/connection.c +++ b/libfreerdp/core/connection.c @@ -1331,8 +1331,6 @@ BOOL rdp_client_transition_to_state(rdpRdp* rdp, CONNECTION_STATE state) case CONNECTION_STATE_FINALIZATION_PERSISTENT_KEY_LIST: case CONNECTION_STATE_FINALIZATION_FONT_LIST: update_reset_state(rdp->update); - if (!rdp_finalize_reset_flags(rdp, FALSE)) - return FALSE; break; case CONNECTION_STATE_CAPABILITIES_EXCHANGE_CONFIRM_ACTIVE: @@ -1624,6 +1622,23 @@ BOOL rdp_server_accept_mcs_channel_join_request(rdpRdp* rdp, wStream* s) return rdp_server_transition_to_state(rdp, rc); } +static BOOL rdp_server_send_sync(rdpRdp* rdp) +{ + WINPR_ASSERT(rdp); + + if (!rdp_server_transition_to_state(rdp, CONNECTION_STATE_FINALIZATION_CLIENT_SYNC)) + return FALSE; + if (!rdp_send_server_synchronize_pdu(rdp)) + return FALSE; + if (!rdp_server_transition_to_state(rdp, CONNECTION_STATE_FINALIZATION_CLIENT_COOPERATE)) + return FALSE; + if (!rdp_send_server_control_cooperate_pdu(rdp)) + return FALSE; + if (!rdp_finalize_reset_flags(rdp, FALSE)) + return FALSE; + return rdp_server_transition_to_state(rdp, CONNECTION_STATE_FINALIZATION_SYNC); +} + BOOL rdp_server_accept_confirm_active(rdpRdp* rdp, wStream* s, UINT16 pduLength) { freerdp_peer* peer; @@ -1657,7 +1672,7 @@ BOOL rdp_server_accept_confirm_active(rdpRdp* rdp, wStream* s, UINT16 pduLength) if (rdp->settings->SaltedChecksum) rdp->do_secure_checksum = TRUE; - return rdp_server_transition_to_state(rdp, CONNECTION_STATE_FINALIZATION_SYNC); + return rdp_server_send_sync(rdp); } BOOL rdp_server_reactivate(rdpRdp* rdp) @@ -1954,5 +1969,7 @@ state_run_t rdp_client_connect_confirm_active(rdpRdp* rdp, wStream* s) CONNECTION_STATE_CAPABILITIES_EXCHANGE_MONITOR_LAYOUT)) status = STATE_RUN_FAILED; } + if (!rdp_finalize_reset_flags(rdp, FALSE)) + status = STATE_RUN_FAILED; return status; } diff --git a/libfreerdp/core/peer.c b/libfreerdp/core/peer.c index f9d19849b..839c5e945 100644 --- a/libfreerdp/core/peer.c +++ b/libfreerdp/core/peer.c @@ -1044,9 +1044,12 @@ static state_run_t peer_recv_callback_internal(rdpTransport* transport, wStream* else ret = STATE_RUN_SUCCESS; } - if (!rdp_server_transition_to_state( - rdp, CONNECTION_STATE_CAPABILITIES_EXCHANGE_CONFIRM_ACTIVE)) - ret = STATE_RUN_FAILED; + if (state_run_success(ret)) + { + if (!rdp_server_transition_to_state( + rdp, CONNECTION_STATE_CAPABILITIES_EXCHANGE_CONFIRM_ACTIVE)) + ret = STATE_RUN_FAILED; + } break; case CONNECTION_STATE_CAPABILITIES_EXCHANGE_CONFIRM_ACTIVE: @@ -1058,7 +1061,6 @@ static state_run_t peer_recv_callback_internal(rdpTransport* transport, wStream* break; case CONNECTION_STATE_FINALIZATION_SYNC: - rdp_finalize_reset_flags(rdp, FALSE); ret = peer_recv_pdu(client, s); if (rdp_finalize_is_flag_set(rdp, FINALIZE_CS_SYNCHRONIZE_PDU)) { @@ -1083,8 +1085,10 @@ static state_run_t peer_recv_callback_internal(rdpTransport* transport, wStream* ret = peer_recv_pdu(client, s); if (rdp_finalize_is_flag_set(rdp, FINALIZE_CS_CONTROL_REQUEST_PDU)) { - if (!rdp_server_transition_to_state( - rdp, CONNECTION_STATE_FINALIZATION_PERSISTENT_KEY_LIST)) + if (!rdp_send_server_control_granted_pdu(rdp)) + ret = STATE_RUN_FAILED; + else if (!rdp_server_transition_to_state( + rdp, CONNECTION_STATE_FINALIZATION_PERSISTENT_KEY_LIST)) ret = STATE_RUN_FAILED; } else