fix:Fixing some issues,See details for the list.

List of issues:
1: The certificate time check is not considered an error if there is no callback return value of 0 at the lower level.
2: Some comment issues.
3: The provider initialization is not protected by a lock.
4: Some functions lack parameter checks.

Cherry-picked from: https://gitcode.com/openHiTLS/openhitls/merge_requests/1471

Signed-off-by: Dongjianwei001 <dongjianwei1@huawei.com>

codecs/src/decode_chain.c

@@ -110,7 +110,7 @@ void CRYPT_DECODE_PoolFreeCtx(CRYPT_DECODER_PoolCtx *poolCtx)
 
 static int32_t SetDecodeType(void *val, int32_t valLen, const char **targetValue)
 {
-    if (valLen == 0 || valLen > MAX_CRYPT_DECODE_FORMAT_TYPE_SIZE) {
+    if (valLen <= 0 || valLen > MAX_CRYPT_DECODE_FORMAT_TYPE_SIZE) {
         BSL_ERR_PUSH_ERROR(CRYPT_INVALID_ARG);
         return CRYPT_INVALID_ARG;
     }

crypto/provider/src/cmvp/iso_prov/cmvp_iso19790.c

@@ -249,7 +249,7 @@ ERR:
 static bool EcdhParamCheck(const CRYPT_EAL_PkeyC2Data *data)
 {
     // https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar2.pdf Chapters 3 and 5
-    // Requires curve specified using SP 800-56A
+    // Requires curve specified using SP 800-56A sm2
     static const uint32_t list[] = {
         CRYPT_ECC_NISTP224,
         CRYPT_ECC_NISTP256,

crypto/provider/src/mgr/crypt_provider.c

@@ -105,6 +105,24 @@ static int32_t ListCompareProvider(const void *a, const void *b)
     return (strcmp(ctx->providerName, providerName) == 0) ? 0 : 1;
 }
 
+static int32_t CheckProviderLoadedNoLock(CRYPT_EAL_LibCtx *libCtx, const char *providerName, bool increaseRef,
+    CRYPT_EAL_ProvMgrCtx **providerMgr)
+{
+    CRYPT_EAL_ProvMgrCtx *tempProviderMgr =
+        (CRYPT_EAL_ProvMgrCtx *)BSL_LIST_SearchDataConst(libCtx->providers, providerName, ListCompareProvider, NULL);
+    if (tempProviderMgr != NULL && increaseRef) {
+        int32_t tempCount = 0;
+        int32_t ret = BSL_SAL_AtomicUpReferences(&tempProviderMgr->ref, &tempCount);
+        if (ret != BSL_SUCCESS) {
+            BSL_ERR_PUSH_ERROR(ret);
+            return ret;
+        }
+    }
+
+    *providerMgr = tempProviderMgr;
+    return CRYPT_SUCCESS;
+}
+
 static int32_t CheckProviderLoaded(CRYPT_EAL_LibCtx *libCtx, const char *providerName, bool increaseRef,
     CRYPT_EAL_ProvMgrCtx **providerMgr)
 {
@@ -114,22 +132,9 @@ static int32_t CheckProviderLoaded(CRYPT_EAL_LibCtx *libCtx, const char *provide
         return ret;
     }
 
-    CRYPT_EAL_ProvMgrCtx *tempProviderMgr =
-        (CRYPT_EAL_ProvMgrCtx *)BSL_LIST_SearchDataConst(libCtx->providers, providerName, ListCompareProvider, NULL);
-    if (tempProviderMgr != NULL && increaseRef) {
-        // Provider is already loaded, increase the reference count
-        int32_t tempCount = 0;
-        ret = BSL_SAL_AtomicUpReferences(&tempProviderMgr->ref, &tempCount);
-        if (ret != BSL_SUCCESS) {
-            BSL_ERR_PUSH_ERROR(ret);
-            (void)BSL_SAL_ThreadUnlock(libCtx->lock);
-            return ret;
-        }
-    }
+    ret = CheckProviderLoadedNoLock(libCtx, providerName, increaseRef, providerMgr);
     (void)BSL_SAL_ThreadUnlock(libCtx->lock);
-
-    *providerMgr = tempProviderMgr;
-    return CRYPT_SUCCESS;
+    return ret;
 }
 
 static bool IsEalPreDefinedProvider(const char *providerName)
@@ -214,18 +219,24 @@ int32_t CRYPT_EAL_ProviderLoad(CRYPT_EAL_LibCtx *libCtx, BSL_SAL_LibFmtCmd cmd,
         return ret;
     }
 
-    // Check if the provider is already loaded
-    ret = CheckProviderLoaded(localCtx, providerFullName, true, &providerMgr);
-    if (ret != CRYPT_SUCCESS) {
+    ret = BSL_SAL_ThreadWriteLock(localCtx->lock);
+    if (ret != BSL_SUCCESS) {
+        BSL_ERR_PUSH_ERROR(ret);
         BSL_SAL_Free(providerFullName);
         return ret;
     }
+
+    // Check if the provider is already loaded
+    ret = CheckProviderLoadedNoLock(localCtx, providerFullName, true, &providerMgr);
+    if (ret != CRYPT_SUCCESS) {
+        goto EXIT;
+    }
     if (providerMgr != NULL) {
-        BSL_SAL_Free(providerFullName);
         if (mgrCtx != NULL) {
             *mgrCtx = providerMgr;
         }
-        return CRYPT_SUCCESS;
+        ret = CRYPT_SUCCESS;
+        goto EXIT;
     }
 
     CRYPT_EAL_ImplProviderInit initFunc = NULL;
@@ -235,22 +246,25 @@ int32_t CRYPT_EAL_ProviderLoad(CRYPT_EAL_LibCtx *libCtx, BSL_SAL_LibFmtCmd cmd,
 #ifdef HITLS_BSL_SAL_DL
         ret = FindProviderInitFunc(localCtx, providerFullName, (void **)&initFunc, &handle);
         if (ret != CRYPT_SUCCESS) {
-            BSL_SAL_Free(providerFullName);
-            return ret;
+            goto EXIT;
         }
 #else
-        return CRYPT_NOT_SUPPORT;
+        ret = CRYPT_NOT_SUPPORT;
+        goto EXIT;
 #endif
     }
 #ifndef HITLS_BSL_SAL_DL
     ret = CRYPT_EAL_AddNewProvMgrCtx(localCtx, providerFullName, NULL, initFunc, handle, param, mgrCtx);
 #else
     ret = CRYPT_EAL_AddNewProvMgrCtx(localCtx, providerFullName, localCtx->searchProviderPath, initFunc, handle, param,
-    mgrCtx);
+        mgrCtx);
     if (ret != CRYPT_SUCCESS) {
         (void)BSL_SAL_UnLoadLib(handle);
     }
 #endif
+
+EXIT:
+    (void)BSL_SAL_ThreadUnlock(localCtx->lock);
     BSL_SAL_Free(providerFullName);
     return ret;
 }
@@ -274,25 +288,36 @@ int32_t CRYPT_EAL_ProviderRegister(CRYPT_EAL_LibCtx *libCtx, const char *provide
         BSL_ERR_PUSH_ERROR(CRYPT_PROVIDER_INVALID_LIB_CTX);
         return CRYPT_PROVIDER_INVALID_LIB_CTX;
     }
-    int32_t ret = CheckProviderLoaded(localCtx, providerName, true, &providerMgr);
-    if (ret != CRYPT_SUCCESS) {
+    int32_t ret = BSL_SAL_ThreadWriteLock(localCtx->lock);
+    if (ret != BSL_SUCCESS) {
+        BSL_ERR_PUSH_ERROR(ret);
         return ret;
     }
+    ret = CheckProviderLoadedNoLock(localCtx, providerName, true, &providerMgr);
+    if (ret != CRYPT_SUCCESS) {
+        goto EXIT;
+    }
     if (providerMgr != NULL) {
         if (mgrCtx != NULL) {
             *mgrCtx = providerMgr;
         }
-        return CRYPT_SUCCESS;
+        ret = CRYPT_SUCCESS;
+        goto EXIT;
     }
     if (IsEalPreDefinedProvider(providerName)) {
         initFunc = CRYPT_EAL_DefaultProvInit;
     } else {
         if (initFunc == NULL) {
             BSL_ERR_PUSH_ERROR(CRYPT_NULL_INPUT);
-            return CRYPT_NULL_INPUT;
+            ret = CRYPT_NULL_INPUT;
+            goto EXIT;
         }
     }
-    return CRYPT_EAL_AddNewProvMgrCtx(localCtx, providerName, NULL, initFunc, NULL, param, mgrCtx);
+    ret = CRYPT_EAL_AddNewProvMgrCtx(localCtx, providerName, NULL, initFunc, NULL, param, mgrCtx);
+
+EXIT:
+    (void)BSL_SAL_ThreadUnlock(localCtx->lock);
+    return ret;
 }
 
 int32_t CRYPT_EAL_ProviderIsLoaded(CRYPT_EAL_LibCtx *libCtx, BSL_SAL_LibFmtCmd cmd, const char *providerName,

crypto/provider/src/mgr/crypt_provider_common.c

@@ -239,38 +239,6 @@ void CRYPT_EAL_ProviderMgrCtxFree(CRYPT_EAL_ProvMgrCtx  *ctx)
     BSL_SAL_Free(ctx);
 }
 
-// Add provider to the list
-static int32_t AddProviderToList(CRYPT_EAL_LibCtx *libCtx, CRYPT_EAL_ProvMgrCtx *providerMgr)
-{
-    int32_t ret = BSL_SAL_ThreadWriteLock(libCtx->lock);
-    if (ret != BSL_SUCCESS) {
-        BSL_ERR_PUSH_ERROR(ret);
-        return ret;
-    }
-
-    ret = BSL_LIST_AddElement(libCtx->providers, providerMgr, BSL_LIST_POS_END);
-    (void)BSL_SAL_ThreadUnlock(libCtx->lock);
-    if (ret != BSL_SUCCESS) {
-        BSL_ERR_PUSH_ERROR(ret);
-    }
-
-    return ret;
-}
-
-static void DeleteProviderFromList(CRYPT_EAL_LibCtx *libCtx, CRYPT_EAL_ProvMgrCtx *providerMgr)
-{
-    (void)BSL_SAL_ThreadWriteLock(libCtx->lock);
-    for (BslListNode *node = BSL_LIST_FirstNode(libCtx->providers); node != NULL;
-        node = BSL_LIST_GetNextNode(libCtx->providers, node)) {
-        if (BSL_LIST_GetData(node) == providerMgr) {
-            BSL_LIST_DeleteNode(libCtx->providers, node, (BSL_LIST_PFUNC_FREE)CRYPT_EAL_ProviderMgrCtxFree);
-            break;
-        }
-    }
-
-    (void)BSL_SAL_ThreadUnlock(libCtx->lock);
-}
-
 int32_t CRYPT_EAL_AddNewProvMgrCtx(CRYPT_EAL_LibCtx *libCtx, const char *providerName, const char *providerPath,
     CRYPT_EAL_ImplProviderInit init, void *handle, BSL_Param *param, CRYPT_EAL_ProvMgrCtx **ctx)
 {
@@ -303,14 +271,15 @@ int32_t CRYPT_EAL_AddNewProvMgrCtx(CRYPT_EAL_LibCtx *libCtx, const char *provide
         BSL_ERR_PUSH_ERROR(ret);
         return ret;
     }
-    ret = AddProviderToList(libCtx, mgrCtx);
-    if (ret != CRYPT_SUCCESS) {
+    ret = CRYPT_EAL_InitProviderMethod(mgrCtx, param, init);
+    if (ret != BSL_SUCCESS) {
         CRYPT_EAL_ProviderMgrCtxFree(mgrCtx);
         return ret;
     }
-    ret = CRYPT_EAL_InitProviderMethod(mgrCtx, param, init);
-    if (ret != BSL_SUCCESS) {
-        DeleteProviderFromList(libCtx, mgrCtx);
+    ret = BSL_LIST_AddElement(libCtx->providers, mgrCtx, BSL_LIST_POS_END);
+    if (ret != CRYPT_SUCCESS) {
+        CRYPT_EAL_ProviderMgrCtxFree(mgrCtx);
+        BSL_ERR_PUSH_ERROR(ret);
         return ret;
     }
     if (ctx != NULL) {

pki/x509_verify/src/hitls_x509_verify.c

@@ -226,7 +226,7 @@ HITLS_X509_StoreCtx *HITLS_X509_StoreCtxNew(void)
 #endif
 
     ctx->verifyParam.maxDepth = HITLS_X509_MAX_DEPTH;
-    ctx->verifyParam.securityBits = 128; // 128: The default number of secure bits.
+    ctx->verifyParam.securityBits = 0; // 0: The default number of secure bits.
     ctx->certChain = NULL; // Initialize to NULL, will be created when needed
 #ifdef HITLS_PKI_X509_VFY_CB
     ctx->verifyCb = VerifyCbDefault;
@@ -1664,7 +1664,7 @@ static int32_t GetCheckTime(HITLS_X509_StoreCtx *storeCtx, bool *isCheckTime, in
         *checkTime = storeCtx->verifyParam.time;
     } else if ((storeCtx->verifyParam.flags & HITLS_X509_VFY_FLAG_DISABLE_TIME_CHECK) == 0) {
         *checkTime = BSL_SAL_CurrentSysTimeGet();
-        if (*checkTime < 0) {
+        if (*checkTime <= 0) {
             return BSL_SAL_TIME_SYS_ERROR;
         }
     } else {

testcode/framework/include/helper.h

@@ -27,7 +27,7 @@
 extern "C" {
 #endif
 
-#define MAX_TEST_FUCNTION_COUNT 100
+#define MAX_TEST_FUCNTION_COUNT 120
 #define MAX_TEST_FUNCTION_NAME 500
 #define MAX_ARGUMENT_COUNT 50
 #define MAX_EXPRESSION_COUNT 100

testcode/sdv/testcase/codecs/decode/test_suite_sdv_decode.c

@@ -516,10 +516,18 @@ void SDV_CRYPT_DECODE_POOL_CTRL_API_TC001(void)
         sizeof(bool)), CRYPT_INVALID_ARG);
 
     /* Test with invalid format length */
+    ASSERT_EQ(CRYPT_DECODE_PoolCtrl(poolCtx, CRYPT_DECODE_POOL_CMD_SET_TARGET_FORMAT, (void *)targetFormat, 0),
+        CRYPT_INVALID_ARG);
+    ASSERT_EQ(CRYPT_DECODE_PoolCtrl(poolCtx, CRYPT_DECODE_POOL_CMD_SET_TARGET_FORMAT, (void *)targetFormat, -1),
+        CRYPT_INVALID_ARG);
     ASSERT_EQ(CRYPT_DECODE_PoolCtrl(poolCtx, CRYPT_DECODE_POOL_CMD_SET_TARGET_FORMAT, (void *)targetFormat,
         MAX_CRYPT_DECODE_FORMAT_TYPE_SIZE + 1), CRYPT_INVALID_ARG);
 
     /* Test with invalid type length */
+    ASSERT_EQ(CRYPT_DECODE_PoolCtrl(poolCtx, CRYPT_DECODE_POOL_CMD_SET_TARGET_TYPE, (void *)targetType, 0),
+        CRYPT_INVALID_ARG);
+    ASSERT_EQ(CRYPT_DECODE_PoolCtrl(poolCtx, CRYPT_DECODE_POOL_CMD_SET_TARGET_TYPE, (void *)targetType, -1),
+        CRYPT_INVALID_ARG);
     ASSERT_EQ(CRYPT_DECODE_PoolCtrl(poolCtx, CRYPT_DECODE_POOL_CMD_SET_TARGET_TYPE, (void *)targetType,
         MAX_CRYPT_DECODE_FORMAT_TYPE_SIZE + 1), CRYPT_INVALID_ARG);
 

testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.c

@@ -39,6 +39,7 @@
 #include "bsl_uio.h"
 #include "hitls_pki_utils.h"
 #include "bsl_asn1.h"
+#include "bsl_obj.h"
 #include "stub_utils.h"
 #include "bsl_err_internal.h"
 
@@ -50,6 +51,7 @@
 STUB_DEFINE_RET4(int32_t, HITLS_X509_CheckCertTime, HITLS_X509_StoreCtx *, HITLS_X509_Cert *, int32_t, int64_t *);
 STUB_DEFINE_RET3(int32_t, BSL_LIST_AddElement, BslList *, void *, BslListPosition);
 STUB_DEFINE_VOID1(HITLS_X509_CertFree, HITLS_X509_Cert *);
+STUB_DEFINE_RET0(BslUnixTime, BSL_SAL_CurrentSysTimeGet);
 
 /* ============================================================================
  * Helper Macros for Verification Callback
@@ -523,6 +525,11 @@ static int32_t HITLS_AddCrlToStoreTest(char *path, HITLS_X509_StoreCtx *store, H
     return HITLS_X509_StoreCtxCtrl(store, HITLS_X509_STORECTX_SET_CRL, *crl, sizeof(HITLS_X509_Crl));
 }
 
+static BslUnixTime STUB_BSL_SAL_CurrentSysTimeGet_Zero(void)
+{
+    return 0;
+}
+
 /* BEGIN_CASE */
 void SDV_X509_BUILD_CERT_CHAIN_FUNC_TC001(char *rootPath, char *caPath, char *cert, char *crlPath)
 {
@@ -2706,6 +2713,52 @@ EXIT:
 }
 /* END_CASE */
 
+/**
+ * Mock the system time source to return Unix epoch, and the default certificate
+ * time check should treat it as an invalid current time.
+ */
+/* BEGIN_CASE */
+void SDV_X509_VFY_CERT_TIME_SYS_TIME_ZERO_FAIL_TC001(void)
+{
+    HITLS_X509_Cert *root = NULL;
+    HITLS_X509_Cert *inter = NULL;
+    HITLS_X509_Cert *leaf = NULL;
+    HITLS_X509_StoreCtx *store = NULL;
+    HITLS_X509_List *chain = NULL;
+    bool isStubbed = false;
+
+    TestMemInit();
+    store = HITLS_X509_StoreCtxNew();
+    chain = BSL_LIST_New(sizeof(HITLS_X509_Cert *));
+    ASSERT_TRUE(store != NULL && chain != NULL);
+
+    ASSERT_EQ(HITLS_X509_CertParseFile(BSL_FORMAT_ASN1, "../testdata/cert/chain/time/root_current.der", &root), 0);
+    ASSERT_EQ(HITLS_X509_CertParseFile(BSL_FORMAT_ASN1, "../testdata/cert/chain/time/inter_current.der", &inter), 0);
+    ASSERT_EQ(HITLS_X509_CertParseFile(BSL_FORMAT_ASN1, "../testdata/cert/chain/time/leaf_current.der", &leaf), 0);
+    ASSERT_EQ(BSL_LIST_AddElement(chain, leaf, BSL_LIST_POS_END), BSL_SUCCESS);
+    ASSERT_EQ(BSL_LIST_AddElement(chain, inter, BSL_LIST_POS_END), BSL_SUCCESS);
+    ASSERT_EQ(BSL_LIST_AddElement(chain, root, BSL_LIST_POS_END), BSL_SUCCESS);
+    ASSERT_EQ(HITLS_X509_StoreCtxCtrl(store, HITLS_X509_STORECTX_DEEP_COPY_SET_CA, root, sizeof(HITLS_X509_Cert)), 0);
+
+    STUB_REPLACE(BSL_SAL_CurrentSysTimeGet, STUB_BSL_SAL_CurrentSysTimeGet_Zero);
+    isStubbed = true;
+    ASSERT_EQ(HITLS_X509_CertVerify(store, chain), BSL_SAL_TIME_SYS_ERROR);
+    STUB_RESTORE(BSL_SAL_CurrentSysTimeGet);
+    isStubbed = false;
+    TestErrClear();
+
+    ASSERT_EQ(HITLS_X509_CertVerify(store, chain), HITLS_PKI_SUCCESS);
+    ASSERT_TRUE(TestIsErrStackEmpty());
+
+EXIT:
+    if (isStubbed) {
+        STUB_RESTORE(BSL_SAL_CurrentSysTimeGet);
+    }
+    HITLS_X509_StoreCtxFree(store);
+    BSL_LIST_FREE(chain, (BSL_LIST_PFUNC_FREE)HITLS_X509_CertFree);
+}
+/* END_CASE */
+
 /**
  * Construct an expired certificate chain and set the verification parameter verifyParam.
  * time to a historical moment within the certificate's validity period. Verification should succeed.

testcode/sdv/testcase/pki/verify/test_suite_sdv_x509_vfy.data

@@ -208,6 +208,9 @@ SDV_X509_VFY_KU_NOEKU_PURPOSE_TC001:"../testdata/cert/chain/ku_noeku_suite/ku_no
 SDV_X509_VFY_CERT_TIME_CURRENT_PASS_TC001
 SDV_X509_VFY_CERT_TIME_CURRENT_PASS_TC001:
 
+SDV_X509_VFY_CERT_TIME_SYS_TIME_ZERO_FAIL_TC001 system time returns zero fail
+SDV_X509_VFY_CERT_TIME_SYS_TIME_ZERO_FAIL_TC001:
+
 SDV_X509_VFY_CERT_TIME_HISTORY_PASS_TC001
 SDV_X509_VFY_CERT_TIME_HISTORY_PASS_TC001: