From 0fbf846671c7a0fa339f145aeaa4215d22d11f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Thu, 10 Jan 2013 11:19:57 -0500 Subject: [PATCH] libwinpr-sspi: NTLM extended protection cleanup --- libfreerdp/core/nego.c | 10 +++--- libfreerdp/core/nego.h | 2 +- libfreerdp/core/transport.c | 4 +-- libfreerdp/crypto/tls.c | 44 ------------------------ winpr/include/winpr/security.h | 11 ++++++ winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c | 43 +++++++++++++---------- 6 files changed, 43 insertions(+), 71 deletions(-) diff --git a/libfreerdp/core/nego.c b/libfreerdp/core/nego.c index 770c6da8c..d38a0675e 100644 --- a/libfreerdp/core/nego.c +++ b/libfreerdp/core/nego.c @@ -85,7 +85,7 @@ BOOL nego_connect(rdpNego* nego) nego->state = NEGO_STATE_FAIL; } - if (!nego->NegotiateSecurityLayer_enabled) + if (!nego->NegotiateSecurityLayer) { DEBUG_NEGO("Security Layer Negotiation is disabled"); /* attempt only the highest enabled protocol (see nego_attempt_*) */ @@ -222,7 +222,7 @@ BOOL nego_transport_connect(rdpNego* nego) { nego_tcp_connect(nego); - if (nego->tcp_connected && !nego->NegotiateSecurityLayer_enabled) + if (nego->tcp_connected && !nego->NegotiateSecurityLayer) return nego_security_connect(nego); return nego->tcp_connected; @@ -938,10 +938,10 @@ void nego_set_target(rdpNego* nego, char* hostname, int port) * @param enable_rdp whether to enable security layer negotiation (TRUE for enabled, FALSE for disabled) */ -void nego_set_negotiation_enabled(rdpNego* nego, BOOL NegotiateSecurityLayer_enabled) +void nego_set_negotiation_enabled(rdpNego* nego, BOOL NegotiateSecurityLayer) { - DEBUG_NEGO("Enabling security layer negotiation: %s", NegotiateSecurityLayer_enabled ? "TRUE" : "FALSE"); - nego->NegotiateSecurityLayer_enabled = NegotiateSecurityLayer_enabled; + DEBUG_NEGO("Enabling security layer negotiation: %s", NegotiateSecurityLayer ? "TRUE" : "FALSE"); + nego->NegotiateSecurityLayer = NegotiateSecurityLayer; } /** diff --git a/libfreerdp/core/nego.h b/libfreerdp/core/nego.h index 622f8f33b..0d7f14b40 100644 --- a/libfreerdp/core/nego.h +++ b/libfreerdp/core/nego.h @@ -100,7 +100,7 @@ struct rdp_nego UINT32 selected_protocol; UINT32 requested_protocols; - BOOL NegotiateSecurityLayer_enabled; + BOOL NegotiateSecurityLayer; BYTE enabled_protocols[16]; rdpTransport* transport; diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c index fc2be7b3a..cbdbe89ef 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -153,8 +153,8 @@ BOOL transport_connect_nla(rdpTransport* transport) if (credssp_authenticate(transport->credssp) < 0) { - if (!connectErrorCode) - connectErrorCode = AUTHENTICATIONERROR; + if (!connectErrorCode) + connectErrorCode = AUTHENTICATIONERROR; printf("Authentication failure, check credentials.\n" "If credentials are valid, the NTLMSSP implementation may be to blame.\n"); diff --git a/libfreerdp/crypto/tls.c b/libfreerdp/crypto/tls.c index 7269304fa..af43025c1 100644 --- a/libfreerdp/crypto/tls.c +++ b/libfreerdp/crypto/tls.c @@ -59,50 +59,6 @@ static void tls_free_certificate(CryptoCert cert) free(cert); } -static void tls_md5_update_uint32_be(MD5_CTX* md5, UINT32 num) -{ - BYTE be32[4]; - - be32[0] = (num >> 0) & 0xFF; - be32[1] = (num >> 8) & 0xFF; - be32[2] = (num >> 16) & 0xFF; - be32[3] = (num >> 24) & 0xFF; - - MD5_Update(md5, be32, 4); -} - -BYTE* tls_get_channel_bindings_hash(SecPkgContext_Bindings* Bindings) -{ - MD5_CTX md5; - BYTE* ChannelBindingToken; - UINT32 ChannelBindingTokenLength; - BYTE* ChannelBindingsHash; - UINT32 ChannelBindingsHashLength; - SEC_CHANNEL_BINDINGS* ChannelBindings; - - ChannelBindings = Bindings->Bindings; - ChannelBindingTokenLength = Bindings->BindingsLength - sizeof(SEC_CHANNEL_BINDINGS); - ChannelBindingToken = &((BYTE*) ChannelBindings)[ChannelBindings->dwApplicationDataOffset]; - - ChannelBindingsHashLength = 16; - ChannelBindingsHash = (BYTE*) malloc(ChannelBindingsHashLength); - ZeroMemory(ChannelBindingsHash, ChannelBindingsHashLength); - - MD5_Init(&md5); - - tls_md5_update_uint32_be(&md5, ChannelBindings->dwInitiatorAddrType); - tls_md5_update_uint32_be(&md5, ChannelBindings->cbInitiatorLength); - tls_md5_update_uint32_be(&md5, ChannelBindings->dwAcceptorAddrType); - tls_md5_update_uint32_be(&md5, ChannelBindings->cbAcceptorLength); - tls_md5_update_uint32_be(&md5, ChannelBindings->cbApplicationDataLength); - - MD5_Update(&md5, (void*) ChannelBindingToken, ChannelBindingTokenLength); - - MD5_Final(ChannelBindingsHash, &md5); - - return ChannelBindingsHash; -} - #define TLS_SERVER_END_POINT "tls-server-end-point:" SecPkgContext_Bindings* tls_get_channel_bindings(X509* cert) diff --git a/winpr/include/winpr/security.h b/winpr/include/winpr/security.h index f4b97d8e7..7e1a57944 100644 --- a/winpr/include/winpr/security.h +++ b/winpr/include/winpr/security.h @@ -30,5 +30,16 @@ typedef struct _LSA_UNICODE_STRING PWSTR Buffer; } LSA_UNICODE_STRING, *PLSA_UNICODE_STRING, UNICODE_STRING, *PUNICODE_STRING; +/** + * Windows Integrity Mechanism Design: + * http://msdn.microsoft.com/en-us/library/bb625963.aspx + */ + +#define SECURITY_MANDATORY_UNTRUSTED_RID 0x0000 +#define SECURITY_MANDATORY_LOW_RID 0x1000 +#define SECURITY_MANDATORY_MEDIUM_RID 0x2000 +#define SECURITY_MANDATORY_HIGH_RID 0x3000 +#define SECURITY_MANDATORY_SYSTEM_RID 0x4000 + #endif /* WINPR_SECURITY_H */ diff --git a/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c b/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c index d27a54628..f72117e12 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c +++ b/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c @@ -279,17 +279,22 @@ void ntlm_compute_channel_bindings(NTLM_CONTEXT* context) MD5_Final(context->ChannelBindingsHash, &md5); } -BYTE ntlm_MachineID[32] = - "\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC\xDD\xEE\xFF" - "\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xAA\xBB\xCC\xDD\xEE\xFF"; - void ntlm_compute_single_host_data(NTLM_CONTEXT* context) { + /** + * The Single_Host_Data structure allows a client to send machine-specific information + * within an authentication exchange to services on the same machine. The client can + * produce additional information to be processed in an implementation-specific way when + * the client and server are on the same host. If the server and client platforms are + * different or if they are on different hosts, then the information MUST be ignored. + * Any fields after the MachineID field MUST be ignored on receipt. + */ + context->SingleHostData.Size = 48; context->SingleHostData.Z4 = 0; context->SingleHostData.DataPresent = 1; - context->SingleHostData.CustomData = 0x2000; - CopyMemory(context->SingleHostData.MachineID, ntlm_MachineID, 32); + context->SingleHostData.CustomData = SECURITY_MANDATORY_MEDIUM_RID; + FillMemory(context->SingleHostData.MachineID, 32, 0xAA); } void ntlm_construct_challenge_target_info(NTLM_CONTEXT* context) @@ -396,6 +401,13 @@ void ntlm_construct_authenticate_target_info(NTLM_CONTEXT* context) AvPairsValueLength += 4; } + if (context->SendSingleHostData) + { + AvPairsCount++; /* MsvAvSingleHost */ + ntlm_compute_single_host_data(context); + AvPairsValueLength += context->SingleHostData.Size; + } + /** * Extended Protection for Authentication: * http://blogs.technet.com/b/srd/archive/2009/12/08/extended-protection-for-authentication.aspx @@ -417,13 +429,6 @@ void ntlm_construct_authenticate_target_info(NTLM_CONTEXT* context) AvPairsCount++; /* MsvAvTargetName */ AvPairsValueLength += context->ServicePrincipalName.Length; } - - if (context->SendSingleHostData) - { - AvPairsCount++; /* MsvAvSingleHost */ - ntlm_compute_single_host_data(context); - AvPairsValueLength += context->SingleHostData.Size; - } } size = ntlm_av_pair_list_size(AvPairsCount, AvPairsValueLength); @@ -460,6 +465,12 @@ void ntlm_construct_authenticate_target_info(NTLM_CONTEXT* context) ntlm_av_pair_add(AuthenticateTargetInfo, MsvAvFlags, (PBYTE) &flags, 4); } + if (context->SendSingleHostData) + { + ntlm_av_pair_add(AuthenticateTargetInfo, MsvAvSingleHost, + (PBYTE) &context->SingleHostData, context->SingleHostData.Size); + } + if (!context->SuppressExtendedProtection) { ntlm_av_pair_add(AuthenticateTargetInfo, MsvChannelBindings, context->ChannelBindingsHash, 16); @@ -470,12 +481,6 @@ void ntlm_construct_authenticate_target_info(NTLM_CONTEXT* context) (PBYTE) context->ServicePrincipalName.Buffer, context->ServicePrincipalName.Length); } - - if (context->SendSingleHostData) - { - ntlm_av_pair_add(AuthenticateTargetInfo, MsvAvSingleHost, - (PBYTE) &context->SingleHostData, context->SingleHostData.Size); - } } if (context->NTLMv2)