From 6a8d21cab9a3adb5785c5feb7b79c27dd9e3ce3e Mon Sep 17 00:00:00 2001
From: David FORT <contact@hardening-consulting.com>
Date: Wed, 11 Feb 2015 21:38:32 +0100
Subject: [PATCH] Fix server-side protocol negociation

Before this patch, RDP security was (wrongly) the fallback when negociating a
security protocol between the client and the server. For example when a client
was claiming TLS-only when connecting to a FreeRDP based-server with RDP security only,
the result of the negociation was that the server started to do RDP security.
The expected behaviour is to send a nego failure packet with error code
SSL_NOT_ALLOWED_BY_SERVER. This patch fixes this.

We also try to handle all cases of failed negociation and return the corresponding
error code.
---
 libfreerdp/core/connection.c | 40 ++++++++++++++++++++++++++++++------
 libfreerdp/core/nego.c       | 10 +++------
 libfreerdp/core/nego.h       |  4 +++-
 3 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c
index 5841b3dee..fd4d8a091 100644
--- a/libfreerdp/core/connection.c
+++ b/libfreerdp/core/connection.c
@@ -1011,20 +1011,48 @@ BOOL rdp_server_accept_nego(rdpRdp* rdp, wStream* s)
 	{
 		nego->SelectedProtocol = PROTOCOL_TLS;
 	}
-	else if ((settings->RdpSecurity) && (nego->SelectedProtocol == PROTOCOL_RDP))
+	else if ((settings->RdpSecurity) && (nego->RequestedProtocols == PROTOCOL_RDP))
 	{
 		nego->SelectedProtocol = PROTOCOL_RDP;
 	}
 	else
 	{
+		/*
+		 * when here client and server aren't compatible, we select the right
+		 * error message to return to the client in the nego failure packet
+		 */
+		nego->SelectedProtocol = PROTOCOL_FAILED_NEGO;
+
+		if (settings->RdpSecurity)
+		{
+			WLog_ERR(TAG, "server supports only Standard RDP Security");
+			nego->SelectedProtocol |= SSL_NOT_ALLOWED_BY_SERVER;
+		}
+		else
+		{
+			if (settings->NlaSecurity && !settings->TlsSecurity)
+			{
+				WLog_ERR(TAG, "server supports only NLA Security");
+				nego->SelectedProtocol |= HYBRID_REQUIRED_BY_SERVER;
+			}
+			else
+			{
+				WLog_ERR(TAG, "server supports only a SSL based Security (TLS or NLA)");
+				nego->SelectedProtocol |= SSL_REQUIRED_BY_SERVER;
+			}
+		}
+
 		WLog_ERR(TAG, "Protocol security negotiation failure");
 	}
 
-	WLog_INFO(TAG, "Negotiated Security: NLA:%d TLS:%d RDP:%d",
-			 (nego->SelectedProtocol & PROTOCOL_NLA) ? 1 : 0,
-			 (nego->SelectedProtocol & PROTOCOL_TLS) ? 1 : 0,
-			 (nego->SelectedProtocol == PROTOCOL_RDP) ? 1: 0
-			);
+	if (!(nego->SelectedProtocol & PROTOCOL_FAILED_NEGO))
+	{
+		WLog_INFO(TAG, "Negotiated Security: NLA:%d TLS:%d RDP:%d",
+				 (nego->SelectedProtocol & PROTOCOL_NLA) ? 1 : 0,
+				 (nego->SelectedProtocol & PROTOCOL_TLS) ? 1 : 0,
+				 (nego->SelectedProtocol == PROTOCOL_RDP) ? 1: 0
+				);
+	}
 
 	if (!nego_send_negotiation_response(nego))
 		return FALSE;
diff --git a/libfreerdp/core/nego.c b/libfreerdp/core/nego.c
index 6655292d2..9c4e117ce 100644
--- a/libfreerdp/core/nego.c
+++ b/libfreerdp/core/nego.c
@@ -918,20 +918,16 @@ BOOL nego_send_negotiation_response(rdpNego* nego)
 	bm = Stream_GetPosition(s);
 	Stream_Seek(s, length);
 
-	if ((nego->SelectedProtocol == PROTOCOL_RDP) && !settings->RdpSecurity)
+	if (nego->SelectedProtocol & PROTOCOL_FAILED_NEGO)
 	{
+		UINT32 errorCode = (nego->SelectedProtocol & ~PROTOCOL_FAILED_NEGO);
 		flags = 0;
 
 		Stream_Write_UINT8(s, TYPE_RDP_NEG_FAILURE);
 		Stream_Write_UINT8(s, flags); /* flags */
 		Stream_Write_UINT16(s, 8); /* RDP_NEG_DATA length (8) */
 
-		/*
-		 * TODO: Check for other possibilities,
-		 *       like SSL_NOT_ALLOWED_BY_SERVER.
-		 */
-		WLog_ERR(TAG, "client supports only Standard RDP Security");
-		Stream_Write_UINT32(s, SSL_REQUIRED_BY_SERVER);
+		Stream_Write_UINT32(s, errorCode);
 		length += 8;
 		status = FALSE;
 	}
diff --git a/libfreerdp/core/nego.h b/libfreerdp/core/nego.h
index 30b082de5..61684e3d9 100644
--- a/libfreerdp/core/nego.h
+++ b/libfreerdp/core/nego.h
@@ -34,7 +34,9 @@ enum RDP_NEG_PROTOCOLS
 	PROTOCOL_RDP = 0x00000000,
 	PROTOCOL_TLS = 0x00000001,
 	PROTOCOL_NLA = 0x00000002,
-	PROTOCOL_EXT = 0x00000008
+	PROTOCOL_EXT = 0x00000008,
+
+	PROTOCOL_FAILED_NEGO = 0x80000000 /* only used internally, not on the wire */
 };
 
 /* Protocol Security Negotiation Failure Codes */