mirror of
https://github.com/openharmony/third_party_openhitls.git
synced 2026-07-01 10:05:26 -04:00
fix:Fixing uncleaned sensitive information && memory leaks
- The plaintext message is not cleared when the encryption of the app or handshake message fails in the RecConnCbcEncryptThenMac function. - In the SESSMGR_Find function, when retrieving sess from the sess hash table, the retrieved sess is not subject to reference counting. This may lead to other threads freeing this sess, posing a risk of dangling pointers when using this sess subsequently. Signed-off-by: balabala-123 <guozhang4@huawei.com> Cherry-picked from: https://gitcode.com/openHiTLS/openhitls/merge_requests/1389 Signed-off-by: Dongjianwei001 <dongjianwei1@huawei.com>
This commit is contained in:
committed by
Dongjianwei001
parent
1af3bc3e44
commit
7d425ddf96
@@ -532,7 +532,9 @@ static int32_t g_copyParamValue = 1;
|
||||
static void ClearExternalCache(void)
|
||||
{
|
||||
for (uint32_t i = 0; i < g_externalSessionCount; i++) {
|
||||
HITLS_SESS_Free(g_externalSessionCache[i]);
|
||||
if (g_copyParamValue == 1) {
|
||||
HITLS_SESS_Free(g_externalSessionCache[i]);
|
||||
}
|
||||
g_externalSessionCache[i] = NULL;
|
||||
}
|
||||
g_externalSessionCount = 0;
|
||||
@@ -647,7 +649,7 @@ EXIT:
|
||||
* 2. Session resumption succeeds
|
||||
@ */
|
||||
/* BEGIN_CASE */
|
||||
void UT_EXTERNAL_CACHE_GET_CB_FUNCTION_TC003()
|
||||
void UT_EXTERNAL_CACHE_GET_CB_FUNCTION_TC003(int copyParamValue)
|
||||
{
|
||||
FRAME_Init();
|
||||
HITLS_Config *clientConfig = HITLS_CFG_NewTLS12Config();
|
||||
@@ -655,6 +657,7 @@ void UT_EXTERNAL_CACHE_GET_CB_FUNCTION_TC003()
|
||||
ASSERT_TRUE(clientConfig != NULL && serverConfig != NULL);
|
||||
|
||||
ClearExternalCache();
|
||||
g_copyParamValue = copyParamValue;
|
||||
|
||||
/* Set external lookup mode on server */
|
||||
uint32_t mode = HITLS_SESS_CACHE_SERVER | HITLS_SESS_DISABLE_INTERNAL_LOOKUP;
|
||||
@@ -677,7 +680,6 @@ void UT_EXTERNAL_CACHE_GET_CB_FUNCTION_TC003()
|
||||
|
||||
/* Store session in external cache manually */
|
||||
StoreSessionInExternalCache(clientSession);
|
||||
HITLS_SESS_Free(clientSession);
|
||||
FRAME_FreeLink(client);
|
||||
FRAME_FreeLink(server);
|
||||
|
||||
@@ -779,7 +781,7 @@ EXIT:
|
||||
* 2. Session resumption works through external cache
|
||||
@ */
|
||||
/* BEGIN_CASE */
|
||||
void UT_EXTERNAL_CACHE_FULL_EXTERNAL_MODE_TC005()
|
||||
void UT_EXTERNAL_CACHE_FULL_EXTERNAL_MODE_TC005(int copyParamValue)
|
||||
{
|
||||
FRAME_Init();
|
||||
HITLS_Config *clientConfig = HITLS_CFG_NewTLS12Config();
|
||||
@@ -787,6 +789,7 @@ void UT_EXTERNAL_CACHE_FULL_EXTERNAL_MODE_TC005()
|
||||
ASSERT_TRUE(clientConfig != NULL && serverConfig != NULL);
|
||||
|
||||
ClearExternalCache();
|
||||
g_copyParamValue = copyParamValue;
|
||||
|
||||
/* Set complete external mode on server */
|
||||
uint32_t mode = HITLS_SESS_CACHE_SERVER | HITLS_SESS_DISABLE_INTERNAL_STORE | HITLS_SESS_DISABLE_INTERNAL_LOOKUP;
|
||||
@@ -812,7 +815,6 @@ void UT_EXTERNAL_CACHE_FULL_EXTERNAL_MODE_TC005()
|
||||
|
||||
/* Manually store in external cache (simulating application behavior) */
|
||||
StoreSessionInExternalCache(clientSession);
|
||||
HITLS_SESS_Free(clientSession);
|
||||
FRAME_FreeLink(client);
|
||||
FRAME_FreeLink(server);
|
||||
|
||||
|
||||
@@ -34,13 +34,19 @@ UT_EXTERNAL_CACHE_REMOVE_CB_BASIC_TC002
|
||||
UT_EXTERNAL_CACHE_REMOVE_CB_BASIC_TC002:
|
||||
|
||||
UT_EXTERNAL_CACHE_GET_CB_FUNCTION_TC003
|
||||
UT_EXTERNAL_CACHE_GET_CB_FUNCTION_TC003:
|
||||
UT_EXTERNAL_CACHE_GET_CB_FUNCTION_TC003:0
|
||||
|
||||
UT_EXTERNAL_CACHE_GET_CB_FUNCTION_TC003
|
||||
UT_EXTERNAL_CACHE_GET_CB_FUNCTION_TC003:1
|
||||
|
||||
UT_EXTERNAL_CACHE_REMOVE_CB_FUNCTION_TC004
|
||||
UT_EXTERNAL_CACHE_REMOVE_CB_FUNCTION_TC004:
|
||||
|
||||
UT_EXTERNAL_CACHE_FULL_EXTERNAL_MODE_TC005
|
||||
UT_EXTERNAL_CACHE_FULL_EXTERNAL_MODE_TC005:
|
||||
UT_EXTERNAL_CACHE_FULL_EXTERNAL_MODE_TC005:0
|
||||
|
||||
UT_EXTERNAL_CACHE_FULL_EXTERNAL_MODE_TC005
|
||||
UT_EXTERNAL_CACHE_FULL_EXTERNAL_MODE_TC005:1
|
||||
|
||||
# Custom Session Ticket Extension Test Cases
|
||||
UT_CUSTOM_SESSION_TICKET_EXT_DATA_BASIC_TC001
|
||||
|
||||
@@ -294,7 +294,7 @@ void SESSMGR_InsertSession(TLS_SessionMgr *mgr, HITLS_Session *sess, bool isStor
|
||||
}
|
||||
|
||||
#ifdef HITLS_TLS_FEATURE_SESSION_ID
|
||||
/* Find the matching session */
|
||||
/* Find a matching session, verify its validity (time), and only return it after duping it. */
|
||||
HITLS_Session *SESSMGR_Find(TLS_Ctx *ctx, uint8_t *sessionId, uint8_t sessionIdSize)
|
||||
{
|
||||
if (ctx == NULL || ctx->globalConfig == NULL || ctx->globalConfig->sessMgr == NULL || sessionId == NULL ||
|
||||
@@ -312,8 +312,9 @@ HITLS_Session *SESSMGR_Find(TLS_Ctx *ctx, uint8_t *sessionId, uint8_t sessionIdS
|
||||
if (BSL_HASH_At(ctx->globalConfig->sessMgr->hash, (uintptr_t)&key, (uintptr_t *)&sess) != BSL_SUCCESS) {
|
||||
BSL_LOG_BINLOG_FIXLEN(
|
||||
BINLOG_ID15353, BSL_LOG_LEVEL_DEBUG, BSL_LOG_BINLOG_TYPE_RUN, "not find sess", 0, 0, 0, 0);
|
||||
sess = NULL;
|
||||
sess = NULL;
|
||||
}
|
||||
sess = HITLS_SESS_Dup(sess);
|
||||
BSL_SAL_ThreadUnlock(ctx->globalConfig->sessMgr->lock);
|
||||
}
|
||||
|
||||
@@ -322,6 +323,7 @@ HITLS_Session *SESSMGR_Find(TLS_Ctx *ctx, uint8_t *sessionId, uint8_t sessionIdS
|
||||
if (SESS_CheckValidity(sess, curTime) == false) {
|
||||
BSL_LOG_BINLOG_FIXLEN(BINLOG_ID16707, BSL_LOG_LEVEL_INFO, BSL_LOG_BINLOG_TYPE_RUN, "sess time out", 0, 0, 0,
|
||||
0);
|
||||
HITLS_SESS_Free(sess);
|
||||
sess = NULL;
|
||||
}
|
||||
}
|
||||
@@ -330,13 +332,13 @@ HITLS_Session *SESSMGR_Find(TLS_Ctx *ctx, uint8_t *sessionId, uint8_t sessionIdS
|
||||
int32_t copy = 1;
|
||||
sess = ctx->globalConfig->sessionGetCb(ctx, sessionId, sessionIdSize, ©);
|
||||
if (sess != NULL) {
|
||||
if (copy != 0) {
|
||||
sess = HITLS_SESS_Dup(sess);
|
||||
}
|
||||
if (!HITLS_SESS_IsResumable(sess)) {
|
||||
HITLS_SESS_Free(sess);
|
||||
return NULL;
|
||||
}
|
||||
if (copy != 0) {
|
||||
sess = HITLS_SESS_Dup(sess);
|
||||
}
|
||||
if ((SESSMGR_GetCacheMode(ctx->globalConfig->sessMgr) & HITLS_SESS_DISABLE_INTERNAL_STORE) == 0) {
|
||||
// Store the session in the internal cache, unlock the lock first,
|
||||
// and then lock it again in SESSMGR_InsertSession
|
||||
|
||||
@@ -1063,7 +1063,7 @@ static int32_t ServerCheckResume(TLS_Ctx *ctx, const ClientHelloMsg *clientHello
|
||||
if (supportTicket && clientHello->extension.flag.haveTicket) {
|
||||
ctx->negotiatedInfo.isTicket = true;
|
||||
}
|
||||
sess = HITLS_SESS_Dup(SESSMGR_Find(ctx, clientHello->sessionId, clientHello->sessionIdSize));
|
||||
sess = SESSMGR_Find(ctx, clientHello->sessionId, clientHello->sessionIdSize);
|
||||
|
||||
int32_t ret = ResumeCheckExtendedMasterScret(ctx, clientHello, &sess);
|
||||
if (ret != HITLS_SUCCESS) {
|
||||
|
||||
@@ -58,7 +58,7 @@ uint32_t SESSMGR_GetCacheSize(TLS_SessionMgr *mgr);
|
||||
/* add */
|
||||
void SESSMGR_InsertSession(TLS_SessionMgr *mgr, HITLS_Session *sess, bool isStore);
|
||||
|
||||
/* Find the matching session and verify the validity of the session (time) */
|
||||
/* Find a matching session, verify its validity (time), and only return it after duping it. */
|
||||
HITLS_Session *SESSMGR_Find(TLS_Ctx *ctx, uint8_t *sessionId, uint8_t sessionIdSize);
|
||||
|
||||
/* Search for the matching session without checking the validity of the session (time) */
|
||||
|
||||
@@ -154,6 +154,12 @@ static uint32_t GetHmacBLen(HITLS_MacAlgo macAlgo)
|
||||
}
|
||||
}
|
||||
|
||||
static inline uintptr_t UintPtrConstTimeSelect(uint32_t mask, uintptr_t a, uintptr_t b)
|
||||
{
|
||||
uintptr_t wideMask = (uintptr_t)0 - (uintptr_t)((mask >> 31) & 0x1u);
|
||||
return (wideMask & a) | (~wideMask & b);
|
||||
}
|
||||
|
||||
/**
|
||||
* a constant-time implemenation of HMAC to prevent side-channel attacks
|
||||
* reference: https://datatracker.ietf.org/doc/html/rfc2104#autoid-2
|
||||
@@ -209,11 +215,10 @@ static int32_t ConstTimeHmac(RecConnSuitInfo *suiteInfo, HITLS_HASH_Ctx **hashCt
|
||||
(void)SAL_CRYPT_DigestUpdate(obCtx, data, minLen);
|
||||
|
||||
for (uint32_t i = minLen; i < maxLen; i++) {
|
||||
if (i < plainLen) {
|
||||
SAL_CRYPT_DigestUpdate(hashCtx[0], data + i, 1);
|
||||
} else {
|
||||
SAL_CRYPT_DigestUpdate(obCtx, data + i, 1);
|
||||
}
|
||||
uint32_t usePlainCtx = Uint32ConstTimeLt(i, plainLen);
|
||||
HITLS_HASH_Ctx *dstCtx =
|
||||
(HITLS_HASH_Ctx *)(uintptr_t)UintPtrConstTimeSelect(usePlainCtx, (uintptr_t)hashCtx[0], (uintptr_t)obCtx);
|
||||
(void)SAL_CRYPT_DigestUpdate(dstCtx, data + i, 1);
|
||||
}
|
||||
(void)SAL_CRYPT_DigestFinal(hashCtx[0], ihash, &ihashLen);
|
||||
(void)memcpy_s(opad + blen, MAX_DIGEST_SIZE, ihash, ihashLen);
|
||||
@@ -231,7 +236,7 @@ static int32_t ConstTimeHmac(RecConnSuitInfo *suiteInfo, HITLS_HASH_Ctx **hashCt
|
||||
|
||||
static inline uint32_t ConstTimeSelectMemcmp(uint32_t good, uint8_t *a, uint8_t *b, uint32_t l)
|
||||
{
|
||||
uint8_t *t = (good == 0) ? b : a;
|
||||
uint8_t *t = (uint8_t *)(uintptr_t)UintPtrConstTimeSelect(good, (uintptr_t)a, (uintptr_t)b);
|
||||
return ConstTimeMemcmp(t, b, l);
|
||||
}
|
||||
|
||||
@@ -244,7 +249,7 @@ static int32_t RecConnCbcDecMtECheckMacTls(TLS_Ctx *ctx, const REC_TextInput *cr
|
||||
if (hashAlg == HITLS_HASH_BUTT) {
|
||||
return HITLS_CRYPT_ERR_HMAC;
|
||||
}
|
||||
|
||||
|
||||
HITLS_HASH_Ctx *ihashCtx = NULL;
|
||||
HITLS_HASH_Ctx *ohashCtx = NULL;
|
||||
HITLS_HASH_Ctx *obscureHashCtx = NULL;
|
||||
@@ -523,7 +528,7 @@ static int32_t RecConnCbcEncryptThenMac(
|
||||
|
||||
ret = RecConnCopyIV(ctx, state, cipherText, cipherTextLen);
|
||||
if (ret != HITLS_SUCCESS) {
|
||||
BSL_SAL_FREE(plainText);
|
||||
BSL_SAL_ClearFree(plainText, plainTextLen);
|
||||
return ret;
|
||||
}
|
||||
offset += state->suiteInfo->fixedIvLength;
|
||||
@@ -534,7 +539,7 @@ static int32_t RecConnCbcEncryptThenMac(
|
||||
RecConnInitCipherParam(&cipherParam, state);
|
||||
ret = SAL_CRYPT_Encrypt(LIBCTX_FROM_CTX(ctx), ATTRIBUTE_FROM_CTX(ctx),
|
||||
&cipherParam, plainText, plainTextLen, &cipherText[offset], &encLen);
|
||||
BSL_SAL_FREE(plainText);
|
||||
BSL_SAL_ClearFree(plainText, plainTextLen);
|
||||
if (ret != HITLS_SUCCESS) {
|
||||
BSL_LOG_BINLOG_FIXLEN(BINLOG_ID15848, BSL_LOG_LEVEL_ERR, BSL_LOG_BINLOG_TYPE_RUN,
|
||||
"CBC encrypt record error.", 0, 0, 0, 0);
|
||||
|
||||
Reference in New Issue
Block a user