diff --git a/libfreerdp/core/fastpath.c b/libfreerdp/core/fastpath.c index 86e55db49..9faf6afe1 100644 --- a/libfreerdp/core/fastpath.c +++ b/libfreerdp/core/fastpath.c @@ -983,6 +983,10 @@ BOOL fastpath_send_multiple_input_pdu(rdpFastPath* fastpath, wStream* s, size_t if (rdp->sec_flags & SEC_ENCRYPT) { + BOOL status = FALSE; + if (!security_lock(rdp)) + goto fail; + int sec_bytes = fastpath_get_sec_bytes(fastpath->rdp); BYTE* fpInputEvents = Stream_Pointer(s) + sec_bytes; UINT16 fpInputEvents_length = length - 3 - sec_bytes; @@ -1001,30 +1005,36 @@ BOOL fastpath_send_multiple_input_pdu(rdpFastPath* fastpath, wStream* s, size_t if (!security_hmac_signature(fpInputEvents, fpInputEvents_length, Stream_Pointer(s), rdp)) - goto fail; + goto unlock; if (pad) memset(fpInputEvents + fpInputEvents_length, 0, pad); if (!security_fips_encrypt(fpInputEvents, fpInputEvents_length + pad, rdp)) - goto fail; + goto unlock; length += pad; } else { - BOOL status; - + BOOL res; if (rdp->sec_flags & SEC_SECURE_CHECKSUM) - status = security_salted_mac_signature(rdp, fpInputEvents, fpInputEvents_length, - TRUE, Stream_Pointer(s)); + res = security_salted_mac_signature(rdp, fpInputEvents, fpInputEvents_length, TRUE, + Stream_Pointer(s)); else - status = security_mac_signature(rdp, fpInputEvents, fpInputEvents_length, - Stream_Pointer(s)); + res = security_mac_signature(rdp, fpInputEvents, fpInputEvents_length, + Stream_Pointer(s)); - if (!status || !security_encrypt(fpInputEvents, fpInputEvents_length, rdp)) - goto fail; + if (!res || !security_encrypt(fpInputEvents, fpInputEvents_length, rdp)) + goto unlock; } + + status = TRUE; + unlock: + if (!security_unlock(rdp)) + goto fail; + if (!status) + goto fail; } rdp->sec_flags = 0; @@ -1220,15 +1230,19 @@ BOOL fastpath_send_update_pdu(rdpFastPath* fastpath, BYTE updateCode, wStream* s if (rdp->sec_flags & SEC_ENCRYPT) { + BOOL res = FALSE; + if (!security_lock(rdp)) + return FALSE; UINT32 dataSize = fpUpdateHeaderSize + DstSize + pad; BYTE* data = Stream_Pointer(fs) - dataSize; if (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS) { if (!security_hmac_signature(data, dataSize - pad, pSignature, rdp)) - return FALSE; + goto unlock; - security_fips_encrypt(data, dataSize, rdp); + if (!security_fips_encrypt(data, dataSize, rdp)) + goto unlock; } else { @@ -1238,8 +1252,15 @@ BOOL fastpath_send_update_pdu(rdpFastPath* fastpath, BYTE updateCode, wStream* s status = security_mac_signature(rdp, data, dataSize, pSignature); if (!status || !security_encrypt(data, dataSize, rdp)) - return FALSE; + goto unlock; } + res = TRUE; + + unlock: + if (!security_unlock(rdp)) + return FALSE; + if (!res) + return FALSE; } Stream_SealLength(fs); diff --git a/libfreerdp/core/rdp.c b/libfreerdp/core/rdp.c index 61b394562..0fe06ccc3 100644 --- a/libfreerdp/core/rdp.c +++ b/libfreerdp/core/rdp.c @@ -635,6 +635,10 @@ static BOOL rdp_security_stream_out(rdpRdp* rdp, wStream* s, int length, UINT32 if (sec_flags & SEC_ENCRYPT) { + BOOL res = FALSE; + if (!security_lock(rdp)) + return FALSE; + if (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS) { data = Stream_Pointer(s) + 12; @@ -653,10 +657,11 @@ static BOOL rdp_security_stream_out(rdpRdp* rdp, wStream* s, int length, UINT32 Stream_Write_UINT8(s, *pad); if (!security_hmac_signature(data, length, Stream_Pointer(s), rdp)) - return FALSE; + goto unlock; Stream_Seek(s, 8); - security_fips_encrypt(data, length + *pad, rdp); + if (!security_fips_encrypt(data, length + *pad, rdp)) + goto unlock; } else { @@ -670,13 +675,21 @@ static BOOL rdp_security_stream_out(rdpRdp* rdp, wStream* s, int length, UINT32 status = security_mac_signature(rdp, data, length, Stream_Pointer(s)); if (!status) - return FALSE; + goto unlock; Stream_Seek(s, 8); if (!security_encrypt(Stream_Pointer(s), length, rdp)) - return FALSE; + goto unlock; } + res = TRUE; + + unlock: + + if (!security_unlock(rdp)) + return FALSE; + if (!res) + return FALSE; } rdp->sec_flags = 0; @@ -1318,6 +1331,9 @@ BOOL rdp_decrypt(rdpRdp* rdp, wStream* s, UINT16* pLength, UINT16 securityFlags) WINPR_ASSERT(s); WINPR_ASSERT(pLength); + if (!security_lock(rdp)) + return FALSE; + length = *pLength; if (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_NONE) return TRUE; @@ -1397,6 +1413,8 @@ BOOL rdp_decrypt(rdpRdp* rdp, wStream* s, UINT16* pLength, UINT16 securityFlags) } res = TRUE; unlock: + if (!security_unlock(rdp)) + return FALSE; return res; } diff --git a/libfreerdp/core/security.c b/libfreerdp/core/security.c index 98f0b701f..39e5afd16 100644 --- a/libfreerdp/core/security.c +++ b/libfreerdp/core/security.c @@ -409,7 +409,6 @@ BOOL security_salted_mac_signature(rdpRdp* rdp, const BYTE* data, UINT32 length, WINPR_ASSERT(data || (length == 0)); WINPR_ASSERT(output); - EnterCriticalSection(&rdp->critical); security_UINT32_le(length_le, length); /* length must be little-endian */ if (encryption) @@ -474,7 +473,7 @@ BOOL security_salted_mac_signature(rdpRdp* rdp, const BYTE* data, UINT32 length, out: if (!result) WLog_WARN(TAG, "security mac signature generation failed"); - LeaveCriticalSection(&rdp->critical); + winpr_Digest_Free(sha1); winpr_Digest_Free(md5); return result; @@ -661,15 +660,16 @@ BOOL security_establish_keys(rdpRdp* rdp) rdp->rc4_key_len = 16; } - EnterCriticalSection(&rdp->critical); + if (!security_lock(rdp)) + return FALSE; memcpy(rdp->decrypt_update_key, rdp->decrypt_key, 16); memcpy(rdp->encrypt_update_key, rdp->encrypt_key, 16); rdp->decrypt_use_count = 0; rdp->decrypt_checksum_use_count = 0; rdp->encrypt_use_count = 0; rdp->encrypt_checksum_use_count = 0; - LeaveCriticalSection(&rdp->critical); - return TRUE; + + return security_unlock(rdp); } static BOOL security_key_update(BYTE* key, BYTE* update_key, size_t key_len, rdpRdp* rdp) @@ -740,7 +740,8 @@ out: BOOL security_encrypt(BYTE* data, size_t length, rdpRdp* rdp) { BOOL rc = FALSE; - EnterCriticalSection(&rdp->critical); + + WINPR_ASSERT(rdp); if (!rdp->rc4_encrypt_key) { WLog_ERR(TAG, "rdp->rc4_encrypt_key=%p", rdp->rc4_encrypt_key); @@ -763,7 +764,6 @@ BOOL security_encrypt(BYTE* data, size_t length, rdpRdp* rdp) rdp->encrypt_checksum_use_count++; rc = TRUE; fail: - LeaveCriticalSection(&rdp->critical); return rc; } @@ -774,7 +774,6 @@ BOOL security_decrypt(BYTE* data, size_t length, rdpRdp* rdp) WINPR_ASSERT(data || (length == 0)); WINPR_ASSERT(rdp); - EnterCriticalSection(&rdp->critical); if (!rdp->rc4_decrypt_key) { WLog_ERR(TAG, "rdp->rc4_decrypt_key=%p", rdp->rc4_decrypt_key); @@ -799,7 +798,6 @@ BOOL security_decrypt(BYTE* data, size_t length, rdpRdp* rdp) fail: if (!rc) WLog_WARN(TAG, "Failed to decrypt security"); - LeaveCriticalSection(&rdp->critical); return rc; } @@ -809,9 +807,10 @@ BOOL security_hmac_signature(const BYTE* data, size_t length, BYTE* output, rdpR BYTE use_count_le[4]; WINPR_HMAC_CTX* hmac; BOOL result = FALSE; - EnterCriticalSection(&rdp->critical); + + WINPR_ASSERT(rdp); + security_UINT32_le(use_count_le, rdp->encrypt_use_count); - LeaveCriticalSection(&rdp->critical); if (!(hmac = winpr_HMAC_New())) return FALSE; @@ -840,14 +839,12 @@ BOOL security_fips_encrypt(BYTE* data, size_t length, rdpRdp* rdp) BOOL rc = FALSE; size_t olen; - EnterCriticalSection(&rdp->critical); if (!winpr_Cipher_Update(rdp->fips_encrypt, data, length, data, &olen)) goto fail; rdp->encrypt_use_count++; rc = TRUE; fail: - LeaveCriticalSection(&rdp->critical); return rc; } @@ -856,7 +853,10 @@ BOOL security_fips_decrypt(BYTE* data, size_t length, rdpRdp* rdp) size_t olen; if (!rdp || !rdp->fips_decrypt) + { + WLog_ERR(TAG, "rdp=%p, rdp->fips_decrypt=%p", rdp, rdp->fips_decrypt); return FALSE; + } if (!winpr_Cipher_Update(rdp->fips_decrypt, data, length, data, &olen)) return FALSE; @@ -870,9 +870,8 @@ BOOL security_fips_check_signature(const BYTE* data, size_t length, const BYTE* BYTE use_count_le[4] = { 0 }; WINPR_HMAC_CTX* hmac = NULL; BOOL result = FALSE; - EnterCriticalSection(&rdp->critical); + security_UINT32_le(use_count_le, rdp->decrypt_use_count++); - LeaveCriticalSection(&rdp->critical); if (!(hmac = winpr_HMAC_New())) return FALSE; @@ -898,3 +897,17 @@ out: winpr_HMAC_Free(hmac); return result; } + +BOOL security_lock(rdpRdp* rdp) +{ + WINPR_ASSERT(rdp); + EnterCriticalSection(&rdp->critical); + return TRUE; +} + +BOOL security_unlock(rdpRdp* rdp) +{ + WINPR_ASSERT(rdp); + LeaveCriticalSection(&rdp->critical); + return TRUE; +} diff --git a/libfreerdp/core/security.h b/libfreerdp/core/security.h index 09804ab6e..037b3c60d 100644 --- a/libfreerdp/core/security.h +++ b/libfreerdp/core/security.h @@ -45,6 +45,10 @@ FREERDP_LOCAL BOOL security_mac_signature(rdpRdp* rdp, const BYTE* data, UINT32 FREERDP_LOCAL BOOL security_salted_mac_signature(rdpRdp* rdp, const BYTE* data, UINT32 length, BOOL encryption, BYTE* output); FREERDP_LOCAL BOOL security_establish_keys(rdpRdp* rdp); + +FREERDP_LOCAL BOOL security_lock(rdpRdp* rdp); +FREERDP_LOCAL BOOL security_unlock(rdpRdp* rdp); + FREERDP_LOCAL BOOL security_encrypt(BYTE* data, size_t length, rdpRdp* rdp); FREERDP_LOCAL BOOL security_decrypt(BYTE* data, size_t length, rdpRdp* rdp); FREERDP_LOCAL BOOL security_hmac_signature(const BYTE* data, size_t length, BYTE* output,