Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect RSA verify data length check #182

Merged
merged 5 commits into from
Oct 11, 2023
+103 −7
Merged

Fix incorrect RSA verify data length check #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c521479
Fix incorrect RSA verify data length check
chinglee-iot Sep 29, 2023
2e08df6
Merge branch 'main' into fix-rsa-verify
chinglee-iot Sep 29, 2023
034ea53
Update size table
chinglee-iot Sep 29, 2023
ae96745
Merge branch 'main' into fix-rsa-verify
moninom1 Oct 6, 2023
adbec42
Update unit test
chinglee-iot Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/doxygen/include/size_table.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
</tr>
<tr>
<td>core_pkcs11_mbedtls.c</td>
<td><center>8.9K</center></td>
<td><center>9.0K</center></td>
<td><center>7.5K</center></td>
</tr>
<tr>
<td><b>Total estimates</b></td>
<td><b><center>10.2K</center></b></td>
<td><b><center>10.3K</center></b></td>
<td><b><center>8.6K</center></b></td>
</tr>
</table>
21 changes: 19 additions & 2 deletions source/portable/mbedtls/core_pkcs11_mbedtls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4734,7 +4734,7 @@ static CK_RV prvVerifyInitEC_RSAKeys( P11Session_t * pxSession,
{
xKeyType = mbedtls_pk_get_type( &pxSession->xVerifyKey );

if( ( pMechanism->mechanism == CKM_RSA_X_509 ) && ( xKeyType == MBEDTLS_PK_RSA ) )
if( ( ( pMechanism->mechanism == CKM_RSA_PKCS ) || ( pMechanism->mechanism == CKM_RSA_X_509 ) ) && ( xKeyType == MBEDTLS_PK_RSA ) )
{
/* Mechanisms align with the port. */
}
Expand Down Expand Up @@ -4851,6 +4851,7 @@ CK_DECLARE_FUNCTION( CK_RV, C_VerifyInit )( CK_SESSION_HANDLE hSession,
{
switch( pMechanism->mechanism )
{
case CKM_RSA_PKCS:
case CKM_RSA_X_509:
case CKM_ECDSA:

Expand Down Expand Up @@ -4979,10 +4980,26 @@ CK_DECLARE_FUNCTION( CK_RV, C_Verify )( CK_SESSION_HANDLE hSession,
{
if( pxSessionObj->xOperationVerifyMechanism == CKM_RSA_X_509 )
{
if( ulDataLen != pkcs11RSA_2048_SIGNATURE_LENGTH )
if( ulDataLen != pkcs11SHA256_DIGEST_LENGTH )
{
LogError( ( "Failed verify operation. Data Length was too "
"short for pkcs11SHA256_DIGEST_LENGTH." ) );
xResult = CKR_DATA_LEN_RANGE;
}

if( ulSignatureLen != pkcs11RSA_2048_SIGNATURE_LENGTH )
{
LogError( ( "Failed verify operation. Signature Length was too "
"short for pkcs11RSA_2048_SIGNATURE_LENGTH." ) );
xResult = CKR_SIGNATURE_LEN_RANGE;
}
}
else if( pxSessionObj->xOperationVerifyMechanism == CKM_RSA_PKCS )
{
if( ulDataLen != pkcs11SHA256_DIGEST_LENGTH )
{
LogError( ( "Failed verify operation. Data Length was too "
"short for pkcs11SHA256_DIGEST_LENGTH." ) );
xResult = CKR_DATA_LEN_RANGE;
}

Expand Down
85 changes: 82 additions & 3 deletions test/pkcs11_mbedtls_utest/core_pkcs11_mbedtls_utest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5371,7 +5371,7 @@ void test_pkcs11_C_VerifyRSA( void )
CK_SESSION_HANDLE xSession = CK_INVALID_HANDLE;
CK_OBJECT_HANDLE xObject = CK_INVALID_HANDLE;
CK_MECHANISM xMechanism = { 0 };
CK_BYTE pxDummyData[ pkcs11RSA_2048_SIGNATURE_LENGTH ] = { 0xAA };
CK_BYTE pxDummyData[ pkcs11SHA256_DIGEST_LENGTH ] = { 0xAA };
CK_ULONG ulDummyDataLen = sizeof( pxDummyData );
CK_BYTE pxDummySignature[ pkcs11RSA_2048_SIGNATURE_LENGTH ] = { 0xAA };
CK_ULONG ulDummySignatureLen = sizeof( pxDummySignature );
Expand Down Expand Up @@ -5420,6 +5420,65 @@ void test_pkcs11_C_VerifyRSA( void )
}
}

/*!
* @brief C_Verify RSA happy path with CKM_RSA_PKCS.
*
*/
void test_pkcs11_C_VerifyRSA_PKCS( void )
{
CK_RV xResult = CKR_OK;
CK_SESSION_HANDLE xSession = CK_INVALID_HANDLE;
CK_OBJECT_HANDLE xObject = CK_INVALID_HANDLE;
CK_MECHANISM xMechanism = { 0 };
CK_BYTE pxDummyData[ pkcs11SHA256_DIGEST_LENGTH ] = { 0xAA };
CK_ULONG ulDummyDataLen = sizeof( pxDummyData );
CK_BYTE pxDummySignature[ pkcs11RSA_2048_SIGNATURE_LENGTH ] = { 0xAA };
CK_ULONG ulDummySignatureLen = sizeof( pxDummySignature );
mbedtls_pk_context xMbedContext = { 0 };
mbedtls_pk_info_t xPkInfo = { 0 };

/* These just have to be not NULL so we can hit the proper path. */
xMbedContext.pk_ctx = &xPkInfo;
xMbedContext.pk_info = &xPkInfo;

xMechanism.mechanism = CKM_RSA_PKCS;
CK_BBOOL xIsPrivate = CK_FALSE;

prvCommonInitStubs( &xSession );

if( TEST_PROTECT() )
{
xResult = prvCreateRSAPub( &xSession, &xObject );
TEST_ASSERT_EQUAL( CKR_OK, xResult );

mock_osal_mutex_lock_IgnoreAndReturn( 0 );
mock_osal_mutex_unlock_IgnoreAndReturn( 0 );
xResult = C_Verify( xSession, pxDummyData, 0, pxDummySignature, ulDummySignatureLen );
TEST_ASSERT_EQUAL( CKR_OPERATION_NOT_INITIALIZED, xResult );

PKCS11_PAL_GetObjectValue_ExpectAnyArgsAndReturn( CKR_OK );
PKCS11_PAL_GetObjectValue_ReturnThruPtr_pIsPrivate( &xIsPrivate );
mbedtls_pk_init_StopIgnore();
mbedtls_pk_init_ExpectAnyArgs();
mbedtls_pk_init_ReturnThruPtr_ctx( &xMbedContext );
mbedtls_pk_parse_public_key_IgnoreAndReturn( 0 );
PKCS11_PAL_GetObjectValueCleanup_CMockIgnore();
xPkType = MBEDTLS_PK_RSA;
xResult = C_VerifyInit( xSession, &xMechanism, xObject );
TEST_ASSERT_EQUAL( CKR_OK, xResult );

mbedtls_pk_verify_IgnoreAndReturn( 0 );
mbedtls_pk_free_CMockIgnore();
xResult = C_Verify( xSession, pxDummyData, ulDummyDataLen, pxDummySignature, ulDummySignatureLen );
TEST_ASSERT_EQUAL( CKR_OK, xResult );
}

if( TEST_PROTECT() )
{
prvCommonDeinitStubs( &xSession );
}
}

/*!
* @brief C_Verify Bad args.
*
Expand Down Expand Up @@ -5491,14 +5550,34 @@ void test_pkcs11_C_VerifyBadArgs( void )
xResult = C_Verify( xSession, pxDummyData, pkcs11RSA_2048_SIGNATURE_LENGTH, pxDummySignature, 0 );
TEST_ASSERT_EQUAL( CKR_SIGNATURE_LEN_RANGE, xResult );

/* CKM_RSA_PKCS invalid data length range. */
xMechanism.mechanism = CKM_RSA_PKCS;
PKCS11_PAL_GetObjectValue_ExpectAnyArgsAndReturn( CKR_OK );
PKCS11_PAL_GetObjectValue_ReturnThruPtr_pIsPrivate( &xIsPrivate );
xResult = C_VerifyInit( xSession, &xMechanism, xObject );
TEST_ASSERT_EQUAL( CKR_OK, xResult );

xResult = C_Verify( xSession, pxDummyData, 0, pxDummySignature, pkcs11RSA_2048_SIGNATURE_LENGTH );
TEST_ASSERT_EQUAL( CKR_DATA_LEN_RANGE, xResult );

/* CKM_RSA_PKCS invalid signature length range. */
xMechanism.mechanism = CKM_RSA_PKCS;
PKCS11_PAL_GetObjectValue_ExpectAnyArgsAndReturn( CKR_OK );
PKCS11_PAL_GetObjectValue_ReturnThruPtr_pIsPrivate( &xIsPrivate );
xResult = C_VerifyInit( xSession, &xMechanism, xObject );
TEST_ASSERT_EQUAL( CKR_OK, xResult );

xResult = C_Verify( xSession, pxDummyData, pkcs11SHA256_DIGEST_LENGTH, pxDummySignature, 0 );
TEST_ASSERT_EQUAL( CKR_SIGNATURE_LEN_RANGE, xResult );

xMechanism.mechanism = CKM_RSA_X_509;
PKCS11_PAL_GetObjectValue_ExpectAnyArgsAndReturn( CKR_OK );
PKCS11_PAL_GetObjectValue_ReturnThruPtr_pIsPrivate( &xIsPrivate );
xResult = C_VerifyInit( xSession, &xMechanism, xObject );
TEST_ASSERT_EQUAL( CKR_OK, xResult );

mock_osal_mutex_lock_IgnoreAndReturn( 1 );
xResult = C_Verify( xSession, pxDummyData, pkcs11RSA_2048_SIGNATURE_LENGTH, pxDummySignature, pkcs11RSA_2048_SIGNATURE_LENGTH );
xResult = C_Verify( xSession, pxDummyData, pkcs11SHA256_DIGEST_LENGTH, pxDummySignature, pkcs11RSA_2048_SIGNATURE_LENGTH );
TEST_ASSERT_EQUAL( CKR_CANT_LOCK, xResult );
mock_osal_mutex_lock_IgnoreAndReturn( 0 );

Expand All @@ -5516,7 +5595,7 @@ void test_pkcs11_C_VerifyBadArgs( void )
TEST_ASSERT_EQUAL( CKR_OK, xResult );

mbedtls_pk_verify_IgnoreAndReturn( -1 );
xResult = C_Verify( xSession, pxDummyData, pkcs11RSA_2048_SIGNATURE_LENGTH, pxDummySignature, pkcs11RSA_2048_SIGNATURE_LENGTH );
xResult = C_Verify( xSession, pxDummyData, pkcs11SHA256_DIGEST_LENGTH, pxDummySignature, pkcs11RSA_2048_SIGNATURE_LENGTH );
TEST_ASSERT_EQUAL( CKR_SIGNATURE_INVALID, xResult );

/* patch */
Expand Down