From 1eac8fa15eb5fa68fc3a1c09a05d640ea1f84b2f Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 28 Oct 2024 11:47:58 +0100 Subject: [PATCH 1/6] [winpr,crypto] deprecate winpr_Cipher_New Deprecate function in favor of new winpr_CipherNewEx with explict key and IV length parameters, adjust test cases to test both. --- winpr/include/winpr/custom-crypto.h | 29 +++- winpr/libwinpr/crypto/cipher.c | 162 ++++++++++++------ winpr/libwinpr/crypto/crypto.c | 10 +- winpr/libwinpr/crypto/test/TestCryptoCipher.c | 20 ++- 4 files changed, 153 insertions(+), 68 deletions(-) diff --git a/winpr/include/winpr/custom-crypto.h b/winpr/include/winpr/custom-crypto.h index cc1c37332..e1d779ff7 100644 --- a/winpr/include/winpr/custom-crypto.h +++ b/winpr/include/winpr/custom-crypto.h @@ -265,9 +265,32 @@ extern "C" WINPR_API void winpr_Cipher_Free(WINPR_CIPHER_CTX* ctx); WINPR_ATTR_MALLOC(winpr_Cipher_Free, 1) - WINPR_API WINPR_CIPHER_CTX* winpr_Cipher_New(WINPR_CIPHER_TYPE cipher, - WINPR_CRYPTO_OPERATION op, const void* key, - const void* iv); + WINPR_API WINPR_DEPRECATED_VAR("[since 3.10.0] use winpr_Cipher_NewEx", + WINPR_CIPHER_CTX* winpr_Cipher_New(WINPR_CIPHER_TYPE cipher, + WINPR_CRYPTO_OPERATION op, + const void* key, + const void* iv)); + + /** @brief Create a new \b WINPR_CIPHER_CTX + * + * creates a new stream cipher. Only the ciphers supported by your SSL library are available, + * fallback to WITH_INTERNAL_RC4 is not possible. + * + * @param cipher The cipher to create the context for + * @param op Operation \b WINPR_ENCRYPT or \b WINPR_DECRYPT + * @param key A pointer to the key material (size must match expectations for the cipher used) + * @param keylen The length in bytes of key material + * @param iv A pointer to the IV material (size must match expectations for the cipher used) + * @param ivlen The length in bytes of the IV + * + * @return A newly allocated context or \b NULL + * + * @since version 3.10.0 + */ + WINPR_ATTR_MALLOC(winpr_Cipher_Free, 1) + WINPR_API WINPR_CIPHER_CTX* winpr_Cipher_NewEx(WINPR_CIPHER_TYPE cipher, + WINPR_CRYPTO_OPERATION op, const void* key, + size_t keylen, const void* iv, size_t ivlen); WINPR_API BOOL winpr_Cipher_SetPadding(WINPR_CIPHER_CTX* ctx, BOOL enabled); WINPR_API BOOL winpr_Cipher_Update(WINPR_CIPHER_CTX* ctx, const void* input, size_t ilen, void* output, size_t* olen); diff --git a/winpr/libwinpr/crypto/cipher.c b/winpr/libwinpr/crypto/cipher.c index ee3df9c30..64f63f638 100644 --- a/winpr/libwinpr/crypto/cipher.c +++ b/winpr/libwinpr/crypto/cipher.c @@ -47,6 +47,19 @@ #endif #endif +struct winpr_cipher_ctx_private_st +{ + WINPR_CIPHER_TYPE cipher; + WINPR_CRYPTO_OPERATION op; + +#ifdef WITH_OPENSSL + EVP_CIPHER_CTX* ectx; +#endif +#ifdef WITH_MBEDTLS + mbedtls_cipher_context_t* mctx; +#endif +}; + /** * RC4 */ @@ -562,69 +575,92 @@ mbedtls_cipher_type_t winpr_mbedtls_get_cipher_type(int cipher) WINPR_CIPHER_CTX* winpr_Cipher_New(WINPR_CIPHER_TYPE cipher, WINPR_CRYPTO_OPERATION op, const void* key, const void* iv) { - WINPR_CIPHER_CTX* ctx = NULL; -#if defined(WITH_OPENSSL) - int operation = 0; - const EVP_CIPHER* evp = NULL; - EVP_CIPHER_CTX* octx = NULL; + return winpr_Cipher_NewEx(cipher, op, key, 0, iv, 0); +} - if (!(evp = winpr_openssl_get_evp_cipher(cipher))) - return NULL; - - if (!(octx = EVP_CIPHER_CTX_new())) - return NULL; - - operation = (op == WINPR_ENCRYPT) ? 1 : 0; - - if (EVP_CipherInit_ex(octx, evp, NULL, key, iv, operation) != 1) +WINPR_API WINPR_CIPHER_CTX* winpr_Cipher_NewEx(WINPR_CIPHER_TYPE cipher, WINPR_CRYPTO_OPERATION op, + const void* key, size_t keylen, const void* iv, + size_t ivlen) +{ + if (cipher == WINPR_CIPHER_ARC4_128) { - EVP_CIPHER_CTX_free(octx); + WLog_ERR(TAG, + "WINPR_CIPHER_ARC4_128 (RC4) cipher not supported, use winpr_RC4_new instead"); return NULL; } - EVP_CIPHER_CTX_set_padding(octx, 0); - ctx = (WINPR_CIPHER_CTX*)octx; + WINPR_CIPHER_CTX* ctx = calloc(1, sizeof(WINPR_CIPHER_CTX)); + if (!ctx) + return NULL; + + ctx->cipher = cipher; + ctx->op = op; + +#if defined(WITH_OPENSSL) + const EVP_CIPHER* evp = winpr_openssl_get_evp_cipher(cipher); + if (!evp) + goto fail; + + ctx->ectx = EVP_CIPHER_CTX_new(); + if (!ctx->ectx) + goto fail; + + if (keylen != 0) + { + WINPR_ASSERT(keylen <= INT32_MAX); + const int len = EVP_CIPHER_CTX_key_length(ctx->ectx); + if ((len > 0) && (len != keylen)) + { + if (EVP_CIPHER_CTX_set_key_length(ctx->ectx, (int)keylen) != 1) + goto fail; + } + } + + if (ivlen != 0) + { + WINPR_ASSERT(ivlen <= INT32_MAX); + const int len = EVP_CIPHER_CTX_iv_length(ctx->ectx); + if ((len > 0) && (ivlen != len)) + goto fail; + } + const int operation = (op == WINPR_ENCRYPT) ? 1 : 0; + + if (EVP_CipherInit_ex(ctx->ectx, evp, NULL, key, iv, operation) != 1) + goto fail; + + EVP_CIPHER_CTX_set_padding(ctx->ectx, 0); + #elif defined(WITH_MBEDTLS) - int key_bitlen; - mbedtls_operation_t operation; - mbedtls_cipher_context_t* mctx; mbedtls_cipher_type_t cipher_type = winpr_mbedtls_get_cipher_type(cipher); const mbedtls_cipher_info_t* cipher_info = mbedtls_cipher_info_from_type(cipher_type); if (!cipher_info) - return NULL; + goto fail; - if (!(mctx = (mbedtls_cipher_context_t*)calloc(1, sizeof(mbedtls_cipher_context_t)))) - return NULL; + ctx->mctx = calloc(1, sizeof(mbedtls_cipher_context_t)); + if (!ctx->mctx) + goto fail; - operation = (op == WINPR_ENCRYPT) ? MBEDTLS_ENCRYPT : MBEDTLS_DECRYPT; - mbedtls_cipher_init(mctx); + const mbedtls_operation_t operation = (op == WINPR_ENCRYPT) ? MBEDTLS_ENCRYPT : MBEDTLS_DECRYPT; + mbedtls_cipher_init(ctx->mctx); - if (mbedtls_cipher_setup(mctx, cipher_info) != 0) - { - free(mctx); - return NULL; - } + if (mbedtls_cipher_setup(ctx->mctx, cipher_info) != 0) + goto fail; - key_bitlen = mbedtls_cipher_get_key_bitlen(mctx); + const int key_bitlen = mbedtls_cipher_get_key_bitlen(ctx->mctx); - if (mbedtls_cipher_setkey(mctx, key, key_bitlen, operation) != 0) - { - mbedtls_cipher_free(mctx); - free(mctx); - return NULL; - } + if (mbedtls_cipher_setkey(ctx->mctx, key, key_bitlen, operation) != 0) + goto fail; - if (mbedtls_cipher_set_padding_mode(mctx, MBEDTLS_PADDING_NONE) != 0) - { - mbedtls_cipher_free(mctx); - free(mctx); - return NULL; - } + if (mbedtls_cipher_set_padding_mode(ctx->mctx, MBEDTLS_PADDING_NONE) != 0) + goto fail; - ctx = (WINPR_CIPHER_CTX*)mctx; #endif return ctx; + +fail: + winpr_Cipher_Free(ctx); + return NULL; } BOOL winpr_Cipher_SetPadding(WINPR_CIPHER_CTX* ctx, BOOL enabled) @@ -632,7 +668,9 @@ BOOL winpr_Cipher_SetPadding(WINPR_CIPHER_CTX* ctx, BOOL enabled) WINPR_ASSERT(ctx); #if defined(WITH_OPENSSL) - EVP_CIPHER_CTX_set_padding((EVP_CIPHER_CTX*)ctx, enabled); + if (!ctx->ectx) + return FALSE; + EVP_CIPHER_CTX_set_padding(ctx->ectx, enabled); #elif defined(WITH_MBEDTLS) mbedtls_cipher_padding_t option = enabled ? MBEDTLS_PADDING_PKCS7 : MBEDTLS_PADDING_NONE; if (mbedtls_cipher_set_padding_mode((mbedtls_cipher_context_t*)ctx, option) != 0) @@ -646,6 +684,9 @@ BOOL winpr_Cipher_SetPadding(WINPR_CIPHER_CTX* ctx, BOOL enabled) BOOL winpr_Cipher_Update(WINPR_CIPHER_CTX* ctx, const void* input, size_t ilen, void* output, size_t* olen) { + WINPR_ASSERT(ctx); + WINPR_ASSERT(olen); + #if defined(WITH_OPENSSL) int outl = (int)*olen; @@ -655,16 +696,16 @@ BOOL winpr_Cipher_Update(WINPR_CIPHER_CTX* ctx, const void* input, size_t ilen, return FALSE; } - WINPR_ASSERT(ctx); - if (EVP_CipherUpdate((EVP_CIPHER_CTX*)ctx, output, &outl, input, (int)ilen) == 1) + WINPR_ASSERT(ctx->ectx); + if (EVP_CipherUpdate(ctx->ectx, output, &outl, input, (int)ilen) == 1) { *olen = (size_t)outl; return TRUE; } #elif defined(WITH_MBEDTLS) - - if (mbedtls_cipher_update((mbedtls_cipher_context_t*)ctx, input, ilen, output, olen) == 0) + WINPR_ASSERT(ctx->mctx); + if (mbedtls_cipher_update(ctx->mctx, input, ilen, output, olen) == 0) return TRUE; #endif @@ -675,10 +716,13 @@ BOOL winpr_Cipher_Update(WINPR_CIPHER_CTX* ctx, const void* input, size_t ilen, BOOL winpr_Cipher_Final(WINPR_CIPHER_CTX* ctx, void* output, size_t* olen) { + WINPR_ASSERT(ctx); + #if defined(WITH_OPENSSL) int outl = (int)*olen; - if (EVP_CipherFinal_ex((EVP_CIPHER_CTX*)ctx, output, &outl) == 1) + WINPR_ASSERT(ctx->ectx); + if (EVP_CipherFinal_ex(ctx->ectx, output, &outl) == 1) { *olen = (size_t)outl; return TRUE; @@ -686,10 +730,12 @@ BOOL winpr_Cipher_Final(WINPR_CIPHER_CTX* ctx, void* output, size_t* olen) #elif defined(WITH_MBEDTLS) - if (mbedtls_cipher_finish((mbedtls_cipher_context_t*)ctx, output, olen) == 0) + WINPR_ASSERT(ctx->mctx); + if (mbedtls_cipher_finish(ctx->mctx, output, olen) == 0) return TRUE; #endif + return FALSE; } @@ -699,11 +745,17 @@ void winpr_Cipher_Free(WINPR_CIPHER_CTX* ctx) return; #if defined(WITH_OPENSSL) - EVP_CIPHER_CTX_free((EVP_CIPHER_CTX*)ctx); + if (ctx->ectx) + EVP_CIPHER_CTX_free(ctx->ectx); #elif defined(WITH_MBEDTLS) - mbedtls_cipher_free((mbedtls_cipher_context_t*)ctx); - free(ctx); + if (ctx->mctx) + { + mbedtls_cipher_free(ctx->mctx); + free(ctx->mctx); + } #endif + + free(ctx); } /** @@ -789,7 +841,7 @@ int winpr_Cipher_BytesToKey(int cipher, WINPR_MD_TYPE md, const void* salt, cons goto err; } - i = 0; + unsigned int i = 0; if (nkey) { diff --git a/winpr/libwinpr/crypto/crypto.c b/winpr/libwinpr/crypto/crypto.c index 26d371fa1..9a0cc8773 100644 --- a/winpr/libwinpr/crypto/crypto.c +++ b/winpr/libwinpr/crypto/crypto.c @@ -186,8 +186,9 @@ BOOL CryptProtectMemory(LPVOID pData, DWORD cbData, DWORD dwFlags) if (!pCipherText) goto out; - if ((enc = winpr_Cipher_New(WINPR_CIPHER_AES_256_CBC, WINPR_ENCRYPT, pMemBlock->key, - pMemBlock->iv)) == NULL) + if ((enc = winpr_Cipher_NewEx(WINPR_CIPHER_AES_256_CBC, WINPR_ENCRYPT, pMemBlock->key, + sizeof(pMemBlock->key), pMemBlock->iv, sizeof(pMemBlock->iv))) == + NULL) goto out; if (!winpr_Cipher_Update(enc, pMemBlock->pData, pMemBlock->cbData, pCipherText, &cbOut)) goto out; @@ -234,8 +235,9 @@ BOOL CryptUnprotectMemory(LPVOID pData, DWORD cbData, DWORD dwFlags) if (!pPlainText) goto out; - if ((dec = winpr_Cipher_New(WINPR_CIPHER_AES_256_CBC, WINPR_DECRYPT, pMemBlock->key, - pMemBlock->iv)) == NULL) + if ((dec = winpr_Cipher_NewEx(WINPR_CIPHER_AES_256_CBC, WINPR_DECRYPT, pMemBlock->key, + sizeof(pMemBlock->key), pMemBlock->iv, sizeof(pMemBlock->iv))) == + NULL) goto out; if (!winpr_Cipher_Update(dec, pMemBlock->pData, pMemBlock->cbData, pPlainText, &cbOut)) goto out; diff --git a/winpr/libwinpr/crypto/test/TestCryptoCipher.c b/winpr/libwinpr/crypto/test/TestCryptoCipher.c index da58eec6e..afa661ddc 100644 --- a/winpr/libwinpr/crypto/test/TestCryptoCipher.c +++ b/winpr/libwinpr/crypto/test/TestCryptoCipher.c @@ -4,12 +4,11 @@ #include #include -static BOOL test_crypto_cipher_aes_128_cbc(void) +static BOOL test_crypto_cipher_aes_128_cbc(BOOL ex) { - WINPR_CIPHER_CTX* ctx = NULL; BOOL result = FALSE; - BYTE key[] = "0123456789abcdeF"; - BYTE iv[] = "1234567887654321"; + BYTE key[16] = "0123456789abcdeF"; + BYTE iv[16] = "1234567887654321"; BYTE ibuf[1024] = { 0 }; BYTE obuf[1024] = { 0 }; size_t ilen = 0; @@ -20,7 +19,13 @@ static BOOL test_crypto_cipher_aes_128_cbc(void) /* encrypt */ - if (!(ctx = winpr_Cipher_New(WINPR_CIPHER_AES_128_CBC, WINPR_ENCRYPT, key, iv))) + WINPR_CIPHER_CTX* ctx = NULL; + if (ex) + ctx = winpr_Cipher_NewEx(WINPR_CIPHER_AES_128_CBC, WINPR_ENCRYPT, key, sizeof(key), iv, + sizeof(iv)); + else + ctx = winpr_Cipher_New(WINPR_CIPHER_AES_128_CBC, WINPR_ENCRYPT, key, iv); + if (!ctx) { (void)fprintf(stderr, "%s: winpr_Cipher_New (encrypt) failed\n", __func__); return FALSE; @@ -219,7 +224,10 @@ int TestCryptoCipher(int argc, char* argv[]) winpr_InitializeSSL(WINPR_SSL_INIT_DEFAULT); - if (!test_crypto_cipher_aes_128_cbc()) + if (!test_crypto_cipher_aes_128_cbc(TRUE)) + return -1; + + if (!test_crypto_cipher_aes_128_cbc(FALSE)) return -1; if (!test_crypto_cipher_rc4()) From 2b0b52be9ea6c5308f0e1584d2c776ae1613c7d1 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 28 Oct 2024 11:49:17 +0100 Subject: [PATCH 2/6] [core,rdp] adjust warning for RC4 RC4 is not supported by winpr_Cipher_New but only by explicitly calling winpr_RC4_New. Adjust the availability test to use the correct functions. --- libfreerdp/core/rdp.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/libfreerdp/core/rdp.c b/libfreerdp/core/rdp.c index 4c7e412b8..f2267d1f7 100644 --- a/libfreerdp/core/rdp.c +++ b/libfreerdp/core/rdp.c @@ -2967,14 +2967,27 @@ static void log_build_warn_cipher(rdpRdp* rdp, log_line_t* firstLine, WINPR_CIPH char key[WINPR_CIPHER_MAX_KEY_LENGTH] = { 0 }; char iv[WINPR_CIPHER_MAX_IV_LENGTH] = { 0 }; - WINPR_CIPHER_CTX* enc = winpr_Cipher_New(md, WINPR_ENCRYPT, key, iv); - WINPR_CIPHER_CTX* dec = winpr_Cipher_New(md, WINPR_DECRYPT, key, iv); - if (enc && dec) + + /* RC4 only exists in the compatiblity functions winpr_RC4_* + * winpr_Cipher_* does not support that. */ + if (md == WINPR_CIPHER_ARC4_128) { - haveCipher = TRUE; + WINPR_RC4_CTX* enc = winpr_RC4_New(key, sizeof(key)); + haveCipher = enc != NULL; + winpr_RC4_Free(enc); + } + else + { + WINPR_CIPHER_CTX* enc = + winpr_Cipher_NewEx(md, WINPR_ENCRYPT, key, sizeof(key), iv, sizeof(iv)); + WINPR_CIPHER_CTX* dec = + winpr_Cipher_NewEx(md, WINPR_DECRYPT, key, sizeof(key), iv, sizeof(iv)); + if (enc && dec) + haveCipher = TRUE; + + winpr_Cipher_Free(enc); + winpr_Cipher_Free(dec); } - winpr_Cipher_Free(enc); - winpr_Cipher_Free(dec); if (!haveCipher) { From 3ae0a10142400f1471bee8bf489ff1bcc2b28452 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 28 Oct 2024 11:50:27 +0100 Subject: [PATCH 3/6] [core] replace usage of winpr_Cipher_New prefer winpr_CipherNewEx to be on the safe side. --- libfreerdp/core/connection.c | 20 ++++++++++++-------- libfreerdp/core/gateway/arm.c | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c index 8506b797d..38bbfdd9a 100644 --- a/libfreerdp/core/connection.c +++ b/libfreerdp/core/connection.c @@ -799,8 +799,9 @@ static BOOL rdp_client_establish_keys(rdpRdp* rdp) if (settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS) { - rdp->fips_encrypt = winpr_Cipher_New(WINPR_CIPHER_DES_EDE3_CBC, WINPR_ENCRYPT, - rdp->fips_encrypt_key, fips_ivec); + rdp->fips_encrypt = + winpr_Cipher_NewEx(WINPR_CIPHER_DES_EDE3_CBC, WINPR_ENCRYPT, rdp->fips_encrypt_key, + sizeof(rdp->fips_encrypt_key), fips_ivec, sizeof(fips_ivec)); if (!rdp->fips_encrypt) { @@ -808,8 +809,9 @@ static BOOL rdp_client_establish_keys(rdpRdp* rdp) goto end; } - rdp->fips_decrypt = winpr_Cipher_New(WINPR_CIPHER_DES_EDE3_CBC, WINPR_DECRYPT, - rdp->fips_decrypt_key, fips_ivec); + rdp->fips_decrypt = + winpr_Cipher_NewEx(WINPR_CIPHER_DES_EDE3_CBC, WINPR_DECRYPT, rdp->fips_decrypt_key, + sizeof(rdp->fips_decrypt_key), fips_ivec, sizeof(fips_ivec)); if (!rdp->fips_decrypt) { @@ -927,8 +929,9 @@ BOOL rdp_server_establish_keys(rdpRdp* rdp, wStream* s) if (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS) { - rdp->fips_encrypt = winpr_Cipher_New(WINPR_CIPHER_DES_EDE3_CBC, WINPR_ENCRYPT, - rdp->fips_encrypt_key, fips_ivec); + rdp->fips_encrypt = + winpr_Cipher_NewEx(WINPR_CIPHER_DES_EDE3_CBC, WINPR_ENCRYPT, rdp->fips_encrypt_key, + sizeof(rdp->fips_encrypt_key), fips_ivec, sizeof(fips_ivec)); if (!rdp->fips_encrypt) { @@ -936,8 +939,9 @@ BOOL rdp_server_establish_keys(rdpRdp* rdp, wStream* s) goto end; } - rdp->fips_decrypt = winpr_Cipher_New(WINPR_CIPHER_DES_EDE3_CBC, WINPR_DECRYPT, - rdp->fips_decrypt_key, fips_ivec); + rdp->fips_decrypt = + winpr_Cipher_NewEx(WINPR_CIPHER_DES_EDE3_CBC, WINPR_DECRYPT, rdp->fips_decrypt_key, + sizeof(rdp->fips_decrypt_key), fips_ivec, sizeof(fips_ivec)); if (!rdp->fips_decrypt) { diff --git a/libfreerdp/core/gateway/arm.c b/libfreerdp/core/gateway/arm.c index 55d4186c1..a9493389f 100644 --- a/libfreerdp/core/gateway/arm.c +++ b/libfreerdp/core/gateway/arm.c @@ -412,7 +412,7 @@ static WINPR_CIPHER_CTX* treatAuthBlob(const BYTE* pbInput, size_t cbInput) return NULL; } - ret = winpr_Cipher_New(cipherType, WINPR_ENCRYPT, Stream_Pointer(s), NULL); + ret = winpr_Cipher_NewEx(cipherType, WINPR_ENCRYPT, Stream_Pointer(s), cbKeyData, NULL, 0); if (!ret) { WLog_ERR(TAG, "error creating cipher"); From 2a41730f70cdd0c88d83aa3fe9a02d5b764dc734 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 28 Oct 2024 11:51:13 +0100 Subject: [PATCH 4/6] [common,assistance] replace winpr_Cipher_New * use winpr_Cipher_NewEx where possible * use winpr_RC4_New where RC4 is used --- libfreerdp/common/assistance.c | 50 +++++++++++++--------------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/libfreerdp/common/assistance.c b/libfreerdp/common/assistance.c index e0a368758..8275e76c8 100644 --- a/libfreerdp/common/assistance.c +++ b/libfreerdp/common/assistance.c @@ -514,7 +514,7 @@ static BOOL freerdp_assistance_parse_attr_uint32(UINT32* opt, const char* key, c return FALSE; } - *opt = val; + *opt = (UINT32)val; return TRUE; } @@ -851,27 +851,25 @@ BYTE* freerdp_assistance_encrypt_pass_stub(const char* password, const char* pas BOOL rc = 0; size_t cbPasswordW = 0; size_t cbPassStubW = 0; - size_t EncryptedSize = 0; - BYTE PasswordHash[WINPR_MD5_DIGEST_LENGTH]; - WINPR_CIPHER_CTX* rc4Ctx = NULL; + BYTE PasswordHash[WINPR_MD5_DIGEST_LENGTH] = { 0 }; + WINPR_RC4_CTX* rc4Ctx = NULL; BYTE* pbIn = NULL; BYTE* pbOut = NULL; - size_t cbOut = 0; - size_t cbIn = 0; - size_t cbFinal = 0; + BYTE* res = NULL; WCHAR* PasswordW = ConvertUtf8ToWCharAlloc(password, &cbPasswordW); WCHAR* PassStubW = ConvertUtf8ToWCharAlloc(passStub, &cbPassStubW); + cbPasswordW = (cbPasswordW) * sizeof(WCHAR); + cbPassStubW = (cbPassStubW) * sizeof(WCHAR); + const size_t EncryptedSize = cbPassStubW + 4; + if (!PasswordW || !PassStubW) goto fail; - cbPasswordW = (cbPasswordW) * sizeof(WCHAR); - cbPassStubW = (cbPassStubW) * sizeof(WCHAR); if (!winpr_Digest(WINPR_MD_MD5, (BYTE*)PasswordW, cbPasswordW, (BYTE*)PasswordHash, sizeof(PasswordHash))) goto fail; - EncryptedSize = cbPassStubW + 4; pbIn = (BYTE*)calloc(1, EncryptedSize); pbOut = (BYTE*)calloc(1, EncryptedSize); @@ -880,7 +878,7 @@ BYTE* freerdp_assistance_encrypt_pass_stub(const char* password, const char* pas *((UINT32*)pbIn) = (UINT32)cbPassStubW; CopyMemory(&pbIn[4], PassStubW, cbPassStubW); - rc4Ctx = winpr_Cipher_New(WINPR_CIPHER_ARC4_128, WINPR_ENCRYPT, PasswordHash, NULL); + rc4Ctx = winpr_RC4_New(PasswordHash, sizeof(PasswordHash)); if (!rc4Ctx) { @@ -888,35 +886,24 @@ BYTE* freerdp_assistance_encrypt_pass_stub(const char* password, const char* pas goto fail; } - cbOut = cbFinal = 0; - cbIn = EncryptedSize; - rc = winpr_Cipher_Update(rc4Ctx, pbIn, cbIn, pbOut, &cbOut); + rc = winpr_RC4_Update(rc4Ctx, EncryptedSize, pbIn, pbOut); if (!rc) { WLog_ERR(TAG, "winpr_Cipher_Update failure"); goto fail; } - - if (!winpr_Cipher_Final(rc4Ctx, pbOut + cbOut, &cbFinal)) - { - WLog_ERR(TAG, "winpr_Cipher_Final failure"); - goto fail; - } - - winpr_Cipher_Free(rc4Ctx); - free(pbIn); - free(PasswordW); - free(PassStubW); - *pEncryptedSize = EncryptedSize; - return pbOut; + res = pbOut; fail: - winpr_Cipher_Free(rc4Ctx); + winpr_RC4_Free(rc4Ctx); free(PasswordW); free(PassStubW); free(pbIn); - free(pbOut); - return NULL; + if (!res) + free(pbOut); + else + *pEncryptedSize = EncryptedSize; + return res; } static BOOL freerdp_assistance_decrypt2(rdpAssistanceFile* file) @@ -959,7 +946,8 @@ static BOOL freerdp_assistance_decrypt2(rdpAssistanceFile* file) goto fail; aesDec = - winpr_Cipher_New(WINPR_CIPHER_AES_128_CBC, WINPR_DECRYPT, DerivedKey, InitializationVector); + winpr_Cipher_NewEx(WINPR_CIPHER_AES_128_CBC, WINPR_DECRYPT, DerivedKey, sizeof(DerivedKey), + InitializationVector, sizeof(InitializationVector)); if (!aesDec) goto fail; From 555d46f7e2f4e202a9b8af9390e360ee1ac8402c Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 28 Oct 2024 12:27:28 +0100 Subject: [PATCH 5/6] [winpr,crypto] deactivate key and IV lenght checks OpenSSL does not support that in older versions, deactivate for the time being. --- winpr/libwinpr/crypto/cipher.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/winpr/libwinpr/crypto/cipher.c b/winpr/libwinpr/crypto/cipher.c index 64f63f638..c852a3010 100644 --- a/winpr/libwinpr/crypto/cipher.c +++ b/winpr/libwinpr/crypto/cipher.c @@ -605,6 +605,7 @@ WINPR_API WINPR_CIPHER_CTX* winpr_Cipher_NewEx(WINPR_CIPHER_TYPE cipher, WINPR_C if (!ctx->ectx) goto fail; +#if 0 if (keylen != 0) { WINPR_ASSERT(keylen <= INT32_MAX); @@ -623,6 +624,8 @@ WINPR_API WINPR_CIPHER_CTX* winpr_Cipher_NewEx(WINPR_CIPHER_TYPE cipher, WINPR_C if ((len > 0) && (ivlen != len)) goto fail; } +#endif + const int operation = (op == WINPR_ENCRYPT) ? 1 : 0; if (EVP_CipherInit_ex(ctx->ectx, evp, NULL, key, iv, operation) != 1) From 8e33854c62ab284fcaf56cc0728bae408ddd9f31 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 29 Oct 2024 08:03:05 +0100 Subject: [PATCH 6/6] [tests] improve TestCommonAssistance Compare encrypted passwords against reference values to ensure the encryption routines work as expected --- libfreerdp/common/test/TestCommonAssistance.c | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/libfreerdp/common/test/TestCommonAssistance.c b/libfreerdp/common/test/TestCommonAssistance.c index 35fe86311..0dba986cb 100644 --- a/libfreerdp/common/test/TestCommonAssistance.c +++ b/libfreerdp/common/test/TestCommonAssistance.c @@ -7,6 +7,10 @@ #include static const char TEST_MSRC_INCIDENT_PASSWORD_TYPE1[] = "Password1"; +static const BYTE TEST_MSRC_INCIDENT_PASSWORD_TYPE1_ENC[32] = { + 0x3c, 0x9c, 0xae, 0xb, 0xce, 0x7a, 0xb1, 0x5c, 0x8a, 0xac, 0x1, 0xd6, 0x76, 0x4, 0x5e, 0xdf, + 0x3f, 0xfa, 0xf0, 0x92, 0xe2, 0xde, 0x36, 0x8a, 0x20, 0x17, 0xe6, 0x8a, 0xd, 0xed, 0x7c, 0x90 +}; static const char TEST_MSRC_INCIDENT_FILE_TYPE1[] = "" @@ -30,6 +34,10 @@ static const BYTE TEST_MSRC_INCIDENT_EXPERT_BLOB_TYPE1[32] = WINPR_PRAGMA_DIAG_POP static const char TEST_MSRC_INCIDENT_PASSWORD_TYPE2[] = "48BJQ853X3B4"; +static const BYTE TEST_MSRC_INCIDENT_PASSWORD_TYPE2_ENC[32] = { + 0x77, 0x7d, 0xfa, 0xae, 0x90, 0x28, 0x12, 0x4d, 0xd0, 0x2e, 0xde, 0x80, 0x14, 0x22, 0x1b, 0x4a, + 0xd1, 0xf4, 0xec, 0x13, 0x85, 0x39, 0xd7, 0x33, 0xac, 0x76, 0x78, 0x95, 0xb2, 0xd8, 0x57, 0xd9 +}; static const char TEST_MSRC_INCIDENT_FILE_TYPE2[] = "" @@ -167,6 +175,10 @@ static const char connectionstr3[] = "L=\"0\"/>" ""; static const char connectionpwd3[] = "4X638PTVZTKZ"; +static const BYTE connectionpwd3_enc[32] = { 0x15, 0x20, 0x04, 0x96, 0xaf, 0x33, 0xc6, 0xe0, + 0x1b, 0xbf, 0x4a, 0x15, 0xc9, 0xc1, 0xb8, 0x71, + 0x44, 0x3f, 0x2e, 0x93, 0xa8, 0x82, 0x35, 0x2b, + 0x24, 0x08, 0x06, 0x55, 0x16, 0x4e, 0x9d, 0x3b }; static BOOL run_test_parse(wLog* log, const char* input, size_t len, const char* password, BOOL expect) @@ -193,7 +205,8 @@ static BOOL test_file_to_settings(wLog* log, rdpAssistanceFile* file) return rc; } -static BOOL test_file_from_buffer(wLog* log, const char* data, size_t size, const char* pwd) +static BOOL test_file_from_buffer(wLog* log, const char* data, size_t size, const char* pwd, + const BYTE* enc, size_t encsize) { BOOL rc = FALSE; int status = 0; @@ -231,6 +244,14 @@ static BOOL test_file_from_buffer(wLog* log, const char* data, size_t size, cons WLog_Print(log, WLOG_INFO, "expertBlob='%s'", expertBlob); } + if (encsize != EncryptedPassStubLength) + goto fail; + if (encsize > 0) + { + if (memcmp(EncryptedPassStub, enc, encsize) != 0) + goto fail; + } + rc = test_file_to_settings(log, file); fail: freerdp_assistance_file_free(file); @@ -241,21 +262,24 @@ fail: static BOOL test_msrsc_incident_file_type1(wLog* log) { - return test_file_from_buffer(log, TEST_MSRC_INCIDENT_FILE_TYPE1, - sizeof(TEST_MSRC_INCIDENT_FILE_TYPE1), - TEST_MSRC_INCIDENT_PASSWORD_TYPE1); + return test_file_from_buffer( + log, TEST_MSRC_INCIDENT_FILE_TYPE1, sizeof(TEST_MSRC_INCIDENT_FILE_TYPE1), + TEST_MSRC_INCIDENT_PASSWORD_TYPE1, TEST_MSRC_INCIDENT_PASSWORD_TYPE1_ENC, + sizeof(TEST_MSRC_INCIDENT_PASSWORD_TYPE1_ENC)); } static BOOL test_msrsc_incident_file_type2(wLog* log) { if (!test_file_from_buffer(log, connectionstr2, sizeof(connectionstr2), - TEST_MSRC_INCIDENT_PASSWORD_TYPE2)) + TEST_MSRC_INCIDENT_PASSWORD_TYPE2, NULL, 0)) return FALSE; - if (!test_file_from_buffer(log, connectionstr3, sizeof(connectionstr3), connectionpwd3)) + if (!test_file_from_buffer(log, connectionstr3, sizeof(connectionstr3), connectionpwd3, + connectionpwd3_enc, sizeof(connectionpwd3_enc))) return FALSE; - if (!test_file_from_buffer(log, TEST_MSRC_INCIDENT_FILE_TYPE2, - sizeof(TEST_MSRC_INCIDENT_FILE_TYPE2), - TEST_MSRC_INCIDENT_PASSWORD_TYPE2)) + if (!test_file_from_buffer( + log, TEST_MSRC_INCIDENT_FILE_TYPE2, sizeof(TEST_MSRC_INCIDENT_FILE_TYPE2), + TEST_MSRC_INCIDENT_PASSWORD_TYPE2, TEST_MSRC_INCIDENT_PASSWORD_TYPE2_ENC, + sizeof(TEST_MSRC_INCIDENT_PASSWORD_TYPE2_ENC))) return FALSE; return TRUE; }