microsoft
/
SymCrypt-OpenSSL
зеркало из https://github.com/microsoft/SymCrypt-OpenSSL.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Fix memory leak in DH when falling back to OpenSSL (#36) 

+ Add SslPlay test case which uses default OpenSSL implementation
+ Call DH_OpenSSL finish method in scossl_dh_finish to free anything
  which may have been allocated in fallback to OpenSSL
+ Free BNs in cleanup in DH (technically had some memory leaks in
  failure cases before)
This commit is contained in:
Samuel Lee 2022-01-19 19:34:33 +00:00 коммит произвёл GitHub
Родитель 358aa635b6
Коммит 1c142b0fa8
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
2 изменённых файлов: 55 добавлений и 26 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

49
SslPlay/SslPlay.cpp
Просмотреть файл

@ -177,6 +177,8 @@ void TestDhDlgroup(const BIGNUM* p)
BIGNUM* g1 = NULL;
BIGNUM* p2 = NULL;
BIGNUM* g2 = NULL;
const BIGNUM* cp1 = NULL;
const BIGNUM* cg1 = NULL;
const BIGNUM* pubkey1 = NULL;
const BIGNUM* pubkey2 = NULL;
@ -189,24 +191,38 @@ void TestDhDlgroup(const BIGNUM* p)
goto end;
}
p1 = BN_dup(p);
p2 = BN_dup(p);
g1 = BN_new();
g2 = BN_new();
if( p1==NULL || p2==NULL || g1==NULL || g2==NULL || !BN_set_word(g1, 2) || !BN_set_word(g2, 2) )
if( p )
{
handleOpenSSLError("BN_dup, BN_new, or BN_set_word failed\n");
goto end;
p1 = BN_dup(p);
p2 = BN_dup(p);
g1 = BN_new();
g2 = BN_new();
if( p1==NULL || p2==NULL || g1==NULL || g2==NULL || !BN_set_word(g1, 2) || !BN_set_word(g2, 2) )
{
handleOpenSSLError("BN_dup, BN_new, or BN_set_word failed\n");
goto end;
}
printf("Command DH_set0_pqg\n");
if( DH_set0_pqg(key1, p1, NULL, g1) != 1 )
{
handleOpenSSLError("DH_set0_pqg failed\n");
goto end;
}
p1 = NULL; // key1 manages p1 and g1 BIGNUMs now
g1 = NULL;
} else {
if( DH_generate_parameters_ex(key1, 1024, 3, NULL) != 1 )
{
handleOpenSSLError("DH_generate_parameters_ex failed\n");
goto end;
}
DH_get0_pqg(key1, &cp1, NULL, &cg1);
p2 = BN_dup(cp1);
g2 = BN_dup(cg1);
}
printf("Command DH_set0_pqg\n");
if( DH_set0_pqg(key1, p1, NULL, g1) != 1 )
{
handleOpenSSLError("DH_set0_pqg failed\n");
goto end;
}
p1 = NULL; // key1 manages p1 and g1 BIGNUMs now
g1 = NULL;
if( DH_set0_pqg(key2, p2, NULL, g2) != 1 )
{
handleOpenSSLError("DH_set0_pqg failed\n");
@ -290,6 +306,9 @@ void TestDh()
TestDhDlgroup(p_ffdhe);
DH_free(dh_ffdhe);
// Test an unsupported DH group
TestDhDlgroup(NULL);
printf("%s", SeparatorLine);
}

32
SymCryptEngine/src/scossl_dh.c
Просмотреть файл

@ -131,11 +131,11 @@ SCOSSL_STATUS scossl_dh_generate_keypair(
{
SCOSSL_LOG_ERROR(SCOSSL_ERR_F_DH_GENERATE_KEYPAIR, ERR_R_OPERATION_FAIL,
"DH_set0_key failed.");
BN_clear_free(dh_privkey);
BN_free(dh_pubkey);
goto cleanup;
}
// Do not free the temporary BIGNUMs now, as DH manages them after success
dh_privkey = NULL;
dh_pubkey = NULL;
pKeyCtx->initialized = 1;
res = SCOSSL_SUCCESS;
@ -147,6 +147,9 @@ cleanup:
scossl_dh_free_key_context(pKeyCtx);
}
BN_clear_free(dh_privkey);
BN_free(dh_pubkey);
if( pbData )
{
OPENSSL_clear_free(pbData, cbData);
@ -285,10 +288,10 @@ SCOSSL_STATUS scossl_dh_import_keypair(
{
SCOSSL_LOG_ERROR(SCOSSL_ERR_F_DH_IMPORT_KEYPAIR, ERR_R_OPERATION_FAIL,
"DH_set0_key failed.");
BN_free(generated_dh_pubkey);
goto cleanup;
}
// Do not free the temporary BIGNUM now, as DH manages them after success
// Do not free the temporary BIGNUM now, as DH manages it after success
generated_dh_pubkey = NULL;
}
pKeyCtx->initialized = 1;
@ -301,6 +304,8 @@ cleanup:
scossl_dh_free_key_context(pKeyCtx);
}
BN_free(generated_dh_pubkey);
if( pbData )
{
OPENSSL_clear_free( pbData, cbData );
@ -466,7 +471,7 @@ SCOSSL_STATUS scossl_get_dh_context(_Inout_ DH* dh, _Out_ PSCOSSL_DH_KEY_CONTEXT
SCOSSL_STATUS scossl_dh_generate_key(_Inout_ DH* dh)
{
const DH_METHOD* ossl_dh_meth = NULL;
PFN_DH_meth_generate_key pfn_dh_meth_generate_key = NULL;
PSCOSSL_DH_KEY_CONTEXT pKeyCtx = NULL;
switch( scossl_get_dh_context_ex(dh, &pKeyCtx, TRUE) )
@ -476,8 +481,7 @@ SCOSSL_STATUS scossl_dh_generate_key(_Inout_ DH* dh)
"scossl_get_dh_context_ex failed.");
return SCOSSL_FAILURE;
case SCOSSL_FALLBACK:
ossl_dh_meth = DH_OpenSSL();
PFN_DH_meth_generate_key pfn_dh_meth_generate_key = DH_meth_get_generate_key(ossl_dh_meth);
pfn_dh_meth_generate_key = DH_meth_get_generate_key(DH_OpenSSL());
if (pfn_dh_meth_generate_key == NULL)
{
return SCOSSL_FAILURE;
@ -496,7 +500,7 @@ SCOSSL_RETURNLENGTH scossl_dh_compute_key(_Out_writes_bytes_(DH_size(dh)) unsign
_In_ const BIGNUM* pub_key,
_In_ DH* dh)
{
const DH_METHOD* ossl_dh_meth = NULL;
PFN_DH_meth_compute_key pfn_dh_meth_compute_key = NULL;
SYMCRYPT_ERROR scError = SYMCRYPT_NO_ERROR;
PSCOSSL_DH_KEY_CONTEXT pKeyCtx = NULL;
BYTE buf[SCOSSL_DH_MAX_PUBLIC_KEY_LEN] = { 0 };
@ -513,8 +517,7 @@ SCOSSL_RETURNLENGTH scossl_dh_compute_key(_Out_writes_bytes_(DH_size(dh)) unsign
"scossl_get_dh_context failed.");
return res;
case SCOSSL_FALLBACK:
ossl_dh_meth = DH_OpenSSL();
PFN_DH_meth_compute_key pfn_dh_meth_compute_key = DH_meth_get_compute_key(ossl_dh_meth);
pfn_dh_meth_compute_key = DH_meth_get_compute_key(DH_OpenSSL());
if (pfn_dh_meth_compute_key == NULL)
{
return res;
@ -587,6 +590,7 @@ cleanup:
SCOSSL_STATUS scossl_dh_finish(_Inout_ DH* dh)
{
PFN_DH_meth_finish pfn_dh_meth_finish = DH_meth_get_finish(DH_OpenSSL());
PSCOSSL_DH_KEY_CONTEXT pKeyCtx = DH_get_ex_data(dh, scossl_dh_idx);
if( pKeyCtx )
{
@ -597,7 +601,13 @@ SCOSSL_STATUS scossl_dh_finish(_Inout_ DH* dh)
OPENSSL_free(pKeyCtx);
DH_set_ex_data(dh, scossl_dh_idx, NULL);
}
return SCOSSL_SUCCESS;
// Ensure any buffers initialized by DH_OpenSSL are freed
if( pfn_dh_meth_finish == NULL )
{
return SCOSSL_FAILURE;
}
return pfn_dh_meth_finish(dh);
}
void scossl_destroy_safeprime_dlgroups(void)