Improved error mapping and tightened checks in rdg_process_packet

libfreerdp/core/gateway/rdg.c

@@ -37,6 +37,7 @@
 #include "../proxy.h"
 #include "../rdp.h"
 #include "../../crypto/opensslcompat.h"
+#include "rpc_fault.h"
 
 #define TAG FREERDP_TAG("core.gateway.rdg")
 
@@ -147,6 +148,70 @@ typedef struct rdg_packet_header
 
 #pragma pack(pop)
 
+typedef struct
+{
+	UINT32 code;
+	const char* name;
+} t_err_mapping;
+
+static const t_err_mapping fields_present[] =
+{
+	{HTTP_CHANNEL_RESPONSE_FIELD_CHANNELID,   "HTTP_CHANNEL_RESPONSE_FIELD_CHANNELID"},
+	{HTTP_CHANNEL_RESPONSE_OPTIONAL, "HTTP_CHANNEL_RESPONSE_OPTIONAL"},
+	{HTTP_CHANNEL_RESPONSE_FIELD_UDPPORT,     "HTTP_CHANNEL_RESPONSE_FIELD_UDPPORT"}
+};
+
+static const t_err_mapping extended_auth[] =
+{
+	{HTTP_EXTENDED_AUTH_NONE, "HTTP_EXTENDED_AUTH_NONE"},
+	{HTTP_EXTENDED_AUTH_SC, "HTTP_EXTENDED_AUTH_SC"},
+	{HTTP_EXTENDED_AUTH_PAA, "HTTP_EXTENDED_AUTH_PAA"},
+	{HTTP_EXTENDED_AUTH_SSPI_NTLM, "HTTP_EXTENDED_AUTH_SSPI_NTLM"}
+};
+
+static const char* fields_present_to_string(UINT16 fieldsPresent)
+{
+	size_t x = 0;
+	static char buffer[1024] = { 0 };
+	char fields[12];
+
+	for (x = 0; x < ARRAYSIZE(fields_present); x++)
+	{
+		if (strnlen(buffer, ARRAYSIZE(buffer)) > 0)
+			strcat(buffer, "|");
+
+		if ((fields_present[x].code & fieldsPresent) != 0)
+			strcat(buffer, fields_present[x].name);
+	}
+
+	sprintf_s(fields, ARRAYSIZE(fields), " [%04"PRIx16"]", fieldsPresent);
+	strcat(buffer, fields);
+	return buffer;
+}
+
+static const char* extended_auth_to_string(UINT16 auth)
+{
+	size_t x = 0;
+	static char buffer[1024] = { 0 };
+	char fields[12];
+
+	if (auth == HTTP_EXTENDED_AUTH_NONE)
+		return "HTTP_EXTENDED_AUTH_NONE [0x0000]";
+
+	for (x = 0; x < ARRAYSIZE(extended_auth); x++)
+	{
+		if (strnlen(buffer, ARRAYSIZE(buffer)) > 0)
+			strcat(buffer, "|");
+
+		if ((extended_auth[x].code & auth) != 0)
+			strcat(buffer, extended_auth[x].name);
+	}
+
+	sprintf_s(fields, ARRAYSIZE(fields), " [%04"PRIx16"]", auth);
+	strcat(buffer, fields);
+	return buffer;
+}
+
 static BOOL rdg_write_packet(rdpRdg* rdg, wStream* sPacket)
 {
 	size_t s;
@@ -546,6 +611,9 @@ static BOOL rdg_skip_seed_payload(rdpTls* tls, SSIZE_T lastResponseLength)
 static BOOL rdg_process_handshake_response(rdpRdg* rdg, wStream* s)
 {
 	UINT32 errorCode;
+	UINT16 serverVersion, extendedAuth;
+	BYTE verMajor, verMinor;
+	const char* error;
 	WLog_DBG(TAG, "Handshake response received");
 
 	if (rdg->state != RDG_CLIENT_STATE_HANDSHAKE)
@@ -553,15 +621,22 @@ static BOOL rdg_process_handshake_response(rdpRdg* rdg, wStream* s)
 		return FALSE;
 	}
 
-	if (Stream_GetRemainingLength(s) < 12)
+	if (Stream_GetRemainingLength(s) < 10)
 		return FALSE;
 
-	Stream_Seek(s, 8);
 	Stream_Read_UINT32(s, errorCode);
+	Stream_Read_UINT8(s, verMajor);
+	Stream_Read_UINT8(s, verMinor);
+	Stream_Read_UINT16(s, serverVersion);
+	Stream_Read_UINT16(s, extendedAuth);
+	error = rpc_error_to_string(errorCode);
+	WLog_DBG(TAG,
+	         "errorCode=%s, verMajor=%"PRId8", verMinor=%"PRId8", serverVersion=%"PRId16", extendedAuth=%s",
+	         error, verMajor, verMinor, serverVersion, extended_auth_to_string(extendedAuth));
 
 	if (FAILED(errorCode))
 	{
-		WLog_DBG(TAG, "Handshake error %"PRIx32, (HRESULT)errorCode);
+		WLog_DBG(TAG, "Handshake error %s", error);
 		return FALSE;
 	}
 
@@ -570,7 +645,9 @@ static BOOL rdg_process_handshake_response(rdpRdg* rdg, wStream* s)
 
 static BOOL rdg_process_tunnel_response(rdpRdg* rdg, wStream* s)
 {
+	UINT16 serverVersion, fieldsPresent;
 	UINT32 errorCode;
+	const char* error;
 	WLog_DBG(TAG, "Tunnel response received");
 
 	if (rdg->state != RDG_CLIENT_STATE_TUNNEL_CREATE)
@@ -578,15 +655,20 @@ static BOOL rdg_process_tunnel_response(rdpRdg* rdg, wStream* s)
 		return FALSE;
 	}
 
-	if (Stream_GetRemainingLength(s) < 14)
+	if (Stream_GetRemainingLength(s) < 10)
 		return FALSE;
 
-	Stream_Seek(s, 10);
+	Stream_Read_UINT16(s, serverVersion);
 	Stream_Read_UINT32(s, errorCode);
+	Stream_Read_UINT16(s, fieldsPresent);
+	Stream_Seek_UINT16(s); /* reserved */
+	error = rpc_error_to_string(errorCode);
+	WLog_DBG(TAG, "serverVersion=%"PRId16", errorCode=%s, fieldsPresent=%s",
+	         serverVersion, error, fields_present_to_string(fieldsPresent));
 
 	if (FAILED(errorCode))
 	{
-		WLog_DBG(TAG, "Tunnel creation error %"PRIx32, (HRESULT)errorCode);
+		WLog_DBG(TAG, "Tunnel creation error %s", error);
 		return FALSE;
 	}
 
@@ -596,6 +678,8 @@ static BOOL rdg_process_tunnel_response(rdpRdg* rdg, wStream* s)
 static BOOL rdg_process_tunnel_authorization_response(rdpRdg* rdg, wStream* s)
 {
 	UINT32 errorCode;
+	UINT16 fieldsPresent;
+	const char* error;
 	WLog_DBG(TAG, "Tunnel authorization received");
 
 	if (rdg->state != RDG_CLIENT_STATE_TUNNEL_AUTHORIZE)
@@ -603,15 +687,19 @@ static BOOL rdg_process_tunnel_authorization_response(rdpRdg* rdg, wStream* s)
 		return FALSE;
 	}
 
-	if (Stream_GetRemainingLength(s) < 12)
+	if (Stream_GetRemainingLength(s) < 8)
 		return FALSE;
 
-	Stream_Seek(s, 8);
 	Stream_Read_UINT32(s, errorCode);
+	Stream_Read_UINT16(s, fieldsPresent);
+	Stream_Seek_UINT16(s); /* reserved */
+	error = rpc_error_to_string(errorCode);
+	WLog_DBG(TAG, "errorCode=%s, fieldsPresent=%s",
+	         error, fields_present_to_string(fieldsPresent));
 
 	if (FAILED(errorCode))
 	{
-		WLog_DBG(TAG, "Tunnel authorization error %"PRIx32, (HRESULT)errorCode);
+		WLog_DBG(TAG, "Tunnel authorization error %s", error);
 		return FALSE;
 	}
 
@@ -620,7 +708,9 @@ static BOOL rdg_process_tunnel_authorization_response(rdpRdg* rdg, wStream* s)
 
 static BOOL rdg_process_channel_response(rdpRdg* rdg, wStream* s)
 {
+	UINT16 fieldsPresent;
 	UINT32 errorCode;
+	const char* error;
 	WLog_DBG(TAG, "Channel response received");
 
 	if (rdg->state != RDG_CLIENT_STATE_CHANNEL_CREATE)
@@ -628,15 +718,20 @@ static BOOL rdg_process_channel_response(rdpRdg* rdg, wStream* s)
 		return FALSE;
 	}
 
-	if (Stream_GetRemainingLength(s) < 12)
+	if (Stream_GetRemainingLength(s) < 8)
 		return FALSE;
 
-	Stream_Seek(s, 8);
 	Stream_Read_UINT32(s, errorCode);
+	Stream_Read_UINT16(s, fieldsPresent);
+	Stream_Seek_UINT16(s); /* reserved */
+	error = rpc_error_to_string(errorCode);
+	WLog_DBG(TAG, "channel response errorCode=%s, fieldsPresent=%s",
+	         error, fields_present_to_string(fieldsPresent));
 
 	if (FAILED(errorCode))
 	{
-		WLog_DBG(TAG, "Channel error %"PRIx32, (HRESULT)errorCode);
+		WLog_ERR(TAG, "channel response errorCode=%s, fieldsPresent=%s",
+		         error, fields_present_to_string(fieldsPresent));
 		return FALSE;
 	}
 
@@ -648,12 +743,18 @@ static BOOL rdg_process_packet(rdpRdg* rdg, wStream* s)
 {
 	BOOL status = TRUE;
 	UINT16 type;
+	UINT32 packetLength;
 	Stream_SetPosition(s, 0);
 
-	if (Stream_GetRemainingLength(s) < 2)
+	if (Stream_GetRemainingLength(s) < 8)
 		return FALSE;
 
-	Stream_Peek_UINT16(s, type);
+	Stream_Read_UINT16(s, type);
+	Stream_Seek_UINT16(s); /* reserved */
+	Stream_Read_UINT32(s, packetLength);
+
+	if (Stream_GetRemainingLength(s) < packetLength)
+		return FALSE;
 
 	switch (type)
 	{

libfreerdp/core/gateway/rpc_client.c

@@ -446,7 +446,7 @@ static int rpc_client_recv_fragment(rdpRpc* rpc, wStream* fragment)
 	}
 	else if (header->common.ptype == PTYPE_FAULT)
 	{
-		rpc_recv_fault_pdu(header);
+		rpc_recv_fault_pdu(header->fault.status);
 		return -1;
 	}
 	else

libfreerdp/core/gateway/rpc_fault.c

@@ -25,6 +25,8 @@
 
 #include "rpc_fault.h"
 
+#include "rpc.h"
+
 #define TAG FREERDP_TAG("core.gateway.rpc")
 
 static const RPC_FAULT_CODE RPC_FAULT_CODES[] =
@@ -359,19 +361,17 @@ static UINT32 rpc_map_status_code_to_win32_error_code(UINT32 code)
 	return code;
 }
 
-int rpc_recv_fault_pdu(rpcconn_hdr_t* header)
+const char* rpc_error_to_string(UINT32 code)
 {
-	int index;
-	UINT32 code;
-	WLog_ERR(TAG,  "RPC Fault PDU:");
-	code = rpc_map_status_code_to_win32_error_code(header->fault.status);
+	size_t index;
+	static char buffer[1024];
 
 	for (index = 0; RPC_FAULT_CODES[index].name != NULL; index++)
 	{
 		if (RPC_FAULT_CODES[index].code == code)
 		{
-			WLog_ERR(TAG,  "status: %s (0x%08"PRIX32")", RPC_FAULT_CODES[index].name, code);
-			return 0;
+			sprintf_s(buffer, ARRAYSIZE(buffer), "%s [0x%08"PRIX32"]", RPC_FAULT_CODES[index].name, code);
+			goto out;
 		}
 	}
 
@@ -379,11 +379,20 @@ int rpc_recv_fault_pdu(rpcconn_hdr_t* header)
 	{
 		if (RPC_TSG_FAULT_CODES[index].code == code)
 		{
-			WLog_ERR(TAG,  "status: %s (0x%08"PRIX32")", RPC_TSG_FAULT_CODES[index].name, code);
-			return 0;
+			sprintf_s(buffer, ARRAYSIZE(buffer), "%s [0x%08"PRIX32"]", RPC_TSG_FAULT_CODES[index].name, code);
+			goto out;
 		}
 	}
 
-	WLog_ERR(TAG,  "status: %s (0x%08"PRIX32")", "UNKNOWN", code);
+	sprintf_s(buffer, ARRAYSIZE(buffer), "%s [0x%08"PRIX32"]", "UNKNOWN", code);
+out:
+	return buffer;
+}
+
+
+int rpc_recv_fault_pdu(UINT32 status)
+{
+	UINT32 code = rpc_map_status_code_to_win32_error_code(status);
+	WLog_ERR(TAG,  "RPC Fault PDU: status=%s", rpc_error_to_string(code));
 	return 0;
 }

libfreerdp/core/gateway/rpc_fault.h

@@ -20,11 +20,10 @@
 #ifndef FREERDP_LIB_CORE_GATEWAY_RPC_FAULT_H
 #define FREERDP_LIB_CORE_GATEWAY_RPC_FAULT_H
 
-#include "rpc.h"
-
 #include <winpr/wtypes.h>
 #include <freerdp/api.h>
 
-FREERDP_LOCAL int rpc_recv_fault_pdu(rpcconn_hdr_t* header);
+FREERDP_LOCAL int rpc_recv_fault_pdu(UINT32 status);
+FREERDP_LOCAL const char* rpc_error_to_string(UINT32 error);
 
 #endif /* FREERDP_LIB_CORE_GATEWAY_RPC_FAULT_H */