diff --git a/source/portable/mbedtls/core_pkcs11_mbedtls.c b/source/portable/mbedtls/core_pkcs11_mbedtls.c index a09172cd..31354fc1 100644 --- a/source/portable/mbedtls/core_pkcs11_mbedtls.c +++ b/source/portable/mbedtls/core_pkcs11_mbedtls.c @@ -1143,7 +1143,7 @@ static CK_RV prvDeleteObjectFromList( CK_OBJECT_HANDLE xPalHandle ) { CK_RV xResult = CKR_OK; int32_t lGotSemaphore = ( int32_t ) 0; - uint32_t ulIndex = 0; + CK_ULONG ulIndex; lGotSemaphore = mbedtls_mutex_lock( &xP11Context.xObjectList.xMutex ); @@ -1187,49 +1187,57 @@ static CK_RV prvAddObjectToList( CK_OBJECT_HANDLE xPalHandle, { CK_RV xResult = CKR_HOST_MEMORY; - /* MISRA Ref 10.5.1 [Essential type casting] */ - /* More details at: https://github.com/FreeRTOS/corePKCS11/blob/main/MISRA.md#rule-105 */ - /* coverity[misra_c_2012_rule_10_5_violation] */ - CK_BBOOL xObjectFound = ( CK_BBOOL ) CK_FALSE; - uint32_t ulSearchIndex = 0; - - if( 0 == mbedtls_mutex_lock( &xP11Context.xObjectList.xMutex ) ) + if( xLabelLength > pkcs11configMAX_LABEL_LENGTH ) + { + xResult = CKR_ARGUMENTS_BAD; + LogError( ( "Failed to add object to internal object list: " + "xLabelLength exceeds pkcs11configMAX_LABEL_LENGTH." ) ); + } + else if( 0 == mbedtls_mutex_lock( &xP11Context.xObjectList.xMutex ) ) { + CK_ULONG ulSearchIndex; + CK_ULONG ulEmptyIndex = 0; + P11Object_t * pxEmptyP11Object = NULL; + + /* Iterate over list to find an existing entry containing xPalHandle */ for( ulSearchIndex = 0; ulSearchIndex < pkcs11configMAX_NUM_OBJECTS; ulSearchIndex++ ) { - if( xResult == CKR_OK ) - { - break; - } + P11Object_t * pxP11Object = &( xP11Context.xObjectList.xObjects[ ulSearchIndex ] ); - if( xP11Context.xObjectList.xObjects[ ulSearchIndex ].xHandle == xPalHandle ) - { - /* Object already exists in list. */ - /* MISRA Ref 10.5.1 [Essential type casting] */ - /* More details at: https://github.com/FreeRTOS/corePKCS11/blob/main/MISRA.md#rule-105 */ - /* coverity[misra_c_2012_rule_10_5_violation] */ - xResult = CKR_OK; - xObjectFound = ( CK_BBOOL ) CK_TRUE; - } - else if( xP11Context.xObjectList.xObjects[ ulSearchIndex ].xHandle == CK_INVALID_HANDLE ) + /* Update an existing entry with the desired xPalHandle */ + if( pxP11Object->xHandle == xPalHandle ) { + ( void ) memcpy( pxP11Object->xLabel, pcLabel, xLabelLength ); + pxP11Object->xLabelSize = xLabelLength; + xResult = CKR_OK; + *pxAppHandle = ulSearchIndex + 1; + + /* Entry updated, so exit the loop. */ + break; } else { - /* Cannot find a free object. */ + if( ( pxP11Object->xHandle == CK_INVALID_HANDLE ) && + ( pxEmptyP11Object == NULL ) ) + { + pxEmptyP11Object = pxP11Object; + ulEmptyIndex = ulSearchIndex; + } } } - /* MISRA Ref 10.5.1 [Essential type casting] */ - /* More details at: https://github.com/FreeRTOS/corePKCS11/blob/main/MISRA.md#rule-105 */ - /* coverity[misra_c_2012_rule_10_5_violation] */ - if( ( xResult == CKR_OK ) && ( xObjectFound == ( CK_BBOOL ) CK_FALSE ) && ( xLabelLength <= pkcs11configMAX_LABEL_LENGTH ) ) + /* Check if we have reached the end of the list without writing */ + if( ( xResult != CKR_OK ) && + ( pxEmptyP11Object != NULL ) ) { - xP11Context.xObjectList.xObjects[ ulSearchIndex - 1UL ].xHandle = xPalHandle; - ( void ) memcpy( xP11Context.xObjectList.xObjects[ ulSearchIndex - 1UL ].xLabel, pcLabel, xLabelLength ); - xP11Context.xObjectList.xObjects[ ulSearchIndex - 1UL ].xLabelSize = xLabelLength; - *pxAppHandle = ulSearchIndex; + pxEmptyP11Object->xHandle = xPalHandle; + pxEmptyP11Object->xLabelSize = xLabelLength; + + ( void ) memcpy( pxEmptyP11Object->xLabel, pcLabel, xLabelLength ); + + *pxAppHandle = ulEmptyIndex + 1; + xResult = CKR_OK; } ( void ) mbedtls_mutex_unlock( &xP11Context.xObjectList.xMutex ); diff --git a/test/mbedtls_integration/CMakeLists.txt b/test/mbedtls_integration/CMakeLists.txt index 37706401..455cd878 100644 --- a/test/mbedtls_integration/CMakeLists.txt +++ b/test/mbedtls_integration/CMakeLists.txt @@ -19,11 +19,9 @@ target_link_libraries(target_lib INTERFACE pkcs11_api) add_library(target_lib_mb2 STATIC) target_link_libraries(target_lib_mb2 PRIVATE target_lib MbedTLS2::mbedcrypto) -target_enable_gcov(target_lib_mb2 PRIVATE) add_library(target_lib_mb3 STATIC) target_link_libraries(target_lib_mb3 PRIVATE target_lib MbedTLS3::mbedcrypto) -target_enable_gcov(target_lib_mb3 PRIVATE) add_library(mbedtls_test INTERFACE) target_sources(mbedtls_test INTERFACE mbedtls_integration_test.c INTERFACE "${PKCS_PAL_POSIX_SOURCES}") diff --git a/test/pkcs11_mbedtls_utest/core_pkcs11_mbedtls_utest.c b/test/pkcs11_mbedtls_utest/core_pkcs11_mbedtls_utest.c index 4870e6e8..234a6c46 100644 --- a/test/pkcs11_mbedtls_utest/core_pkcs11_mbedtls_utest.c +++ b/test/pkcs11_mbedtls_utest/core_pkcs11_mbedtls_utest.c @@ -256,12 +256,12 @@ void setUp( void ) mbedtls_pk_info_from_type_Stub( pk_info_from_type_stub ); } -/* called before each testcase */ +/* called after each testcase */ void tearDown( void ) { TEST_ASSERT_EQUAL_INT_MESSAGE( 0, usMallocFreeCalls, - "free is not called the same number of times as malloc, \ - you might have a memory leak!!" ); + "free was not called the same number of times as malloc, \ + you might have a memory leak!!" ); usMallocFreeCalls = 0; } @@ -6650,6 +6650,47 @@ void test_pkcs11_C_DestroyObject( void ) } } +/*! + * @brief C_DestroyObject where the call to PKCS11_PAL_DestroyObject fails + * + */ +void test_pkcs11_C_DestroyObjectPalFailure( void ) +{ + CK_RV xResult = CKR_OK; + CK_SESSION_HANDLE xSession = 0; + CK_OBJECT_HANDLE xObject = 0; + + if( TEST_PROTECT() ) + { + prvCommonInitStubs( &xSession ); + + xResult = prvCreateEcPub( &xSession, &xObject ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + xResult = prvCreateEcPriv( &xSession, &xObject ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + PKCS11_PAL_GetObjectValue_IgnoreAndReturn( CKR_OK ); + + mbedtls_calloc_Stub( pvPkcs11CallocCb ); + vPkcs11Free_Stub( vPkcs11FreeCb ); + mbedtls_free_Stub( vPkcs11FreeCb ); + + PKCS11_PAL_SaveObject_IgnoreAndReturn( 2 ); + + PKCS11_PAL_GetObjectValueCleanup_CMockIgnore(); + PKCS11_PAL_DestroyObject_IgnoreAndReturn( CKR_FUNCTION_FAILED ); + + xResult = C_DestroyObject( xSession, xObject ); + TEST_ASSERT_EQUAL( CKR_FUNCTION_FAILED, xResult ); + } + + if( TEST_PROTECT() ) + { + prvCommonDeinitStubs( &xSession ); + } +} + /*! * @brief C_DestroyObject failed to get mutex when removing object from internal list. * @@ -6739,3 +6780,227 @@ void test_pkcs11_C_DestroyObjectBadHandle( void ) prvCommonDeinitStubs( &xSession ); } } + +void test_pkcs11_prvAddObjectToList( void ) +{ + CK_RV xResult = CKR_OK; + CK_SESSION_HANDLE xSession = 0; + + vPkcs11Free_Stub( vPkcs11FreeCb ); + mbedtls_free_Stub( vPkcs11FreeCb ); + + prvCommonInitStubs( &xSession ); + + if( TEST_PROTECT() ) + { + CK_OBJECT_HANDLE xCreatedObjectHandle = 0; + CK_OBJECT_CLASS xCertificateClass = CKO_CERTIFICATE; + CK_CERTIFICATE_TYPE xCertificateType = CKC_X_509; + CK_BBOOL xTokenStorage = CK_TRUE; + CK_BYTE pxSubject[] = "TestSubject"; + CK_BYTE pxCertData[] = "Empty Cert"; + + CK_BYTE pxCertLabel1[] = "test_cert_label_1"; + CK_OBJECT_HANDLE xObjectPalHandle1 = 0x12345678; + + CK_ATTRIBUTE pCertificateTemplate1[ 6 ] = + { + { + CKA_CLASS, + &xCertificateClass, + sizeof( xCertificateClass ) + }, + { + CKA_SUBJECT, + pxSubject, + sizeof( pxSubject ) - 1 + }, + { + CKA_VALUE, + pxCertData, + ( CK_ULONG ) sizeof( pxCertData ) + }, + { + CKA_LABEL, + pxCertLabel1, + strlen( ( const char * ) pxCertLabel1 ) + }, + { + CKA_CERTIFICATE_TYPE, + &xCertificateType, + sizeof( CK_CERTIFICATE_TYPE ) + }, + { + CKA_TOKEN, + &xTokenStorage, + sizeof( xTokenStorage ) + } + }; + + PKCS11_PAL_SaveObject_IgnoreAndReturn( xObjectPalHandle1 ); + + xResult = C_CreateObject( xSession, + pCertificateTemplate1, + sizeof( pCertificateTemplate1 ) / sizeof( CK_ATTRIBUTE ), + &xCreatedObjectHandle ); + + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + /* First Object is always handle 1 */ + TEST_ASSERT_EQUAL( 1, xCreatedObjectHandle ); + + CK_OBJECT_HANDLE xObjectPalHandle2 = 0xABCDEF; + CK_BYTE pxCertLabel2[] = "test_cert_label_2"; + CK_ATTRIBUTE pCertificateTemplate2[ 6 ] = + { + { + CKA_CLASS, + &xCertificateClass, + sizeof( xCertificateClass ) + }, + { + CKA_SUBJECT, + pxSubject, + sizeof( pxSubject ) - 1 + }, + { + CKA_VALUE, + pxCertData, + ( CK_ULONG ) sizeof( pxCertData ) + }, + { + CKA_LABEL, + pxCertLabel2, + strlen( ( const char * ) pxCertLabel2 ) + }, + { + CKA_CERTIFICATE_TYPE, + &xCertificateType, + sizeof( CK_CERTIFICATE_TYPE ) + }, + { + CKA_TOKEN, + &xTokenStorage, + sizeof( xTokenStorage ) + } + }; + + PKCS11_PAL_SaveObject_IgnoreAndReturn( xObjectPalHandle2 ); + + xResult = C_CreateObject( xSession, + pCertificateTemplate2, + sizeof( pCertificateTemplate2 ) / sizeof( CK_ATTRIBUTE ), + &xCreatedObjectHandle ); + + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + /* Second Object is always handle 2 */ + TEST_ASSERT_EQUAL( 2, xCreatedObjectHandle ); + + /* Lookup the first object we created */ + CK_OBJECT_HANDLE xFoundObjectHandle = CK_INVALID_HANDLE; + CK_ULONG ulObjectCount = 0; + CK_ATTRIBUTE xFindObjectTemplate = + { + CKA_LABEL, + pxCertLabel1, + sizeof( pxCertLabel1 ) - 1 + }; + + xResult = C_FindObjectsInit( xSession, &xFindObjectTemplate, 1 ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + xResult = C_FindObjects( xSession, &xFoundObjectHandle, 1, &ulObjectCount ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + TEST_ASSERT_EQUAL( 1, ulObjectCount ); + TEST_ASSERT_EQUAL( 1, xFoundObjectHandle ); + + xResult = C_FindObjectsFinal( xSession ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + /* Lookup the second object we created */ + CK_ATTRIBUTE xFindObjectTemplate2 = + { + CKA_LABEL, + pxCertLabel2, + sizeof( pxCertLabel2 ) - 1 + }; + + xResult = C_FindObjectsInit( xSession, &xFindObjectTemplate2, 1 ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + xResult = C_FindObjects( xSession, &xFoundObjectHandle, 1, &ulObjectCount ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + TEST_ASSERT_EQUAL( 1, ulObjectCount ); + TEST_ASSERT_EQUAL( 2, xFoundObjectHandle ); + + xResult = C_FindObjectsFinal( xSession ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + /* Replace the first object with a copy of the second object */ + CK_ATTRIBUTE pCertificateTemplate3[ 6 ] = + { + { + CKA_CLASS, + &xCertificateClass, + sizeof( xCertificateClass ) + }, + { + CKA_SUBJECT, + pxSubject, + sizeof( pxSubject ) - 1 + }, + { + CKA_VALUE, + pxCertData, + ( CK_ULONG ) sizeof( pxCertData ) + }, + { + CKA_LABEL, + pxCertLabel2, + strlen( ( const char * ) pxCertLabel2 ) + }, + { + CKA_CERTIFICATE_TYPE, + &xCertificateType, + sizeof( CK_CERTIFICATE_TYPE ) + }, + { + CKA_TOKEN, + &xTokenStorage, + sizeof( xTokenStorage ) + } + }; + + PKCS11_PAL_SaveObject_IgnoreAndReturn( xObjectPalHandle1 ); + xResult = C_CreateObject( xSession, + pCertificateTemplate3, + sizeof( pCertificateTemplate3 ) / sizeof( CK_ATTRIBUTE ), + &xCreatedObjectHandle ); + + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + /* Check that the first object was replaced */ + TEST_ASSERT_EQUAL( 1, xCreatedObjectHandle ); + + xResult = C_FindObjectsInit( xSession, &xFindObjectTemplate2, 1 ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + + /* Find objects with pxCertLabel2 labels */ + xFoundObjectHandle = CK_INVALID_HANDLE; + + /* Object in first slot in list was found */ + xResult = C_FindObjects( xSession, &xFoundObjectHandle, 1, &ulObjectCount ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + TEST_ASSERT_EQUAL( 1, ulObjectCount ); + TEST_ASSERT_EQUAL( 1, xFoundObjectHandle ); + + xResult = C_FindObjectsFinal( xSession ); + TEST_ASSERT_EQUAL( CKR_OK, xResult ); + } + + if( TEST_PROTECT() ) + { + prvCommonDeinitStubs( &xSession ); + } +}