Skip to content

Commit

Permalink
Fix MISRA C 2012 deviations (#190)
Browse files Browse the repository at this point in the history
* Fix rule 18.4 deviations. Not using operators with pointer type.
* Suppress rule 20.5 deviations in misra.config to allow use of undef.
* Use parenthesis to specify operator precedence.

---------

Co-authored-by: Ubuntu <[email protected]>
  • Loading branch information
chinglee-iot and Ubuntu authored Feb 26, 2024
1 parent 640dd3f commit e0cd4db
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 74 deletions.
2 changes: 1 addition & 1 deletion source/core_pkcs11.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ CK_RV vAppendSHA256AlgorithmIdentifierSequence( const uint8_t * puc32ByteHashedM
if( xResult == CKR_OK )
{
( void ) memcpy( puc51ByteHashOidBuffer, pucOidSequence, sizeof( pucOidSequence ) );
( void ) memcpy( &puc51ByteHashOidBuffer[ sizeof( pucOidSequence ) ], puc32ByteHashedMessage, 32 );
( void ) memcpy( &( puc51ByteHashOidBuffer[ sizeof( pucOidSequence ) ] ), puc32ByteHashedMessage, 32 );
}

return xResult;
Expand Down
58 changes: 29 additions & 29 deletions source/core_pki_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,19 @@ int8_t PKI_mbedTLSSignatureToPkcs11Signature( uint8_t * pxSignaturePKCS,
if( ucSigComponentLength == 33UL )
{
/* Chop off the leading zero. The first 4 bytes were SEQUENCE, LENGTH, INTEGER, LENGTH, 0x00 padding. */
( void ) memcpy( pxSignaturePKCS, &pxMbedSignature[ 5 ], 32 );
( void ) memcpy( pxSignaturePKCS, &( pxMbedSignature[ 5 ] ), 32 );
/* SEQUENCE, LENGTH, INTEGER, LENGTH, leading zero, R, S's integer tag */
pxNextLength = &pxMbedSignature[ 5U + 32U + 1U ];
pxNextLength = &( pxMbedSignature[ 5U + 32U + 1U ] );
}
else if( ucSigComponentLength <= 32UL )
{
/* The R component is 32 bytes or less. Copy so that it is properly represented as a 32 byte value,
* leaving leading 0 pads at beginning if necessary. */
( void ) memcpy( &pxSignaturePKCS[ 32UL - ucSigComponentLength ], /* If the R component is less than 32 bytes, leave the leading zeros. */
&pxMbedSignature[ 4 ], /* SEQUENCE, LENGTH, INTEGER, LENGTH, (R component begins as the 5th byte) */
( void ) memcpy( &( pxSignaturePKCS[ 32UL - ucSigComponentLength ] ), /* If the R component is less than 32 bytes, leave the leading zeros. */
&( pxMbedSignature[ 4 ] ), /* SEQUENCE, LENGTH, INTEGER, LENGTH, (R component begins as the 5th byte) */
ucSigComponentLength );
pxNextLength = &pxMbedSignature[ 4U + ucSigComponentLength + 1U ]; /* Move the pointer to get rid of
* SEQUENCE, LENGTH, INTEGER, LENGTH, R Component, S integer tag. */
pxNextLength = &( pxMbedSignature[ 4U + ucSigComponentLength + 1U ] ); /* Move the pointer to get rid of
* SEQUENCE, LENGTH, INTEGER, LENGTH, R Component, S integer tag. */
}
else
{
Expand All @@ -103,16 +103,16 @@ int8_t PKI_mbedTLSSignatureToPkcs11Signature( uint8_t * pxSignaturePKCS,

if( ucSigComponentLength == 33UL )
{
( void ) memcpy( &pxSignaturePKCS[ 32 ],
&pxNextLength[ 2 ], /*LENGTH (of S component), 0x00 padding, S component is 3rd byte - we want to skip the leading zero. */
( void ) memcpy( &( pxSignaturePKCS[ 32 ] ),
&( pxNextLength[ 2 ] ), /*LENGTH (of S component), 0x00 padding, S component is 3rd byte - we want to skip the leading zero. */
32 );
}
else if( ucSigComponentLength <= 32UL )
{
/* The S component is 32 bytes or less. Copy so that it is properly represented as a 32 byte value,
* leaving leading 0 pads at beginning if necessary. */
( void ) memcpy( &pxSignaturePKCS[ 64UL - ucSigComponentLength ],
&pxNextLength[ 1 ],
( void ) memcpy( &( pxSignaturePKCS[ 64UL - ucSigComponentLength ] ),
&( pxNextLength[ 1 ] ),
ucSigComponentLength );
}
else
Expand Down Expand Up @@ -166,41 +166,41 @@ int8_t PKI_pkcs11SignatureTombedTLSSignature( uint8_t * pucSig,
* This prevents the number from being interpreted as negative. */
if( ( ucTemp[ 0 ] & 0x80UL ) == 0x80UL )
{
pucSig[ 1 ]++; /* Increment the length of the structure to account for the 0x00 pad. */
pucSig[ 3 ] = 0x21; /* Increment the length of the R value to account for the 0x00 pad. */
pucSig[ 4 ] = 0x0; /* Write the 0x00 pad. */
( void ) memcpy( &pucSig[ 5 ], ucTemp, 32 ); /* Copy the 32-byte R value. */
pucSigPtr = pucSig + 33; /* Increment the pointer to compensate for padded R length. */
pucSig[ 1 ]++; /* Increment the length of the structure to account for the 0x00 pad. */
pucSig[ 3 ] = 0x21; /* Increment the length of the R value to account for the 0x00 pad. */
pucSig[ 4 ] = 0x0; /* Write the 0x00 pad. */
( void ) memcpy( &( pucSig[ 5 ] ), ucTemp, 32 ); /* Copy the 32-byte R value. */
pucSigPtr = &( pucSig[ 33 ] ); /* Increment the pointer to compensate for padded R length. */
}
else
{
pucSig[ 3 ] = 0x20; /* R length with be 32 bytes. */
( void ) memcpy( &pucSig[ 4 ], ucTemp, 32 ); /* Copy 32 bytes of R into the signature buffer. */
pucSigPtr = pucSig + 32; /* Increment the pointer for 32 byte R length. */
pucSig[ 3 ] = 0x20; /* R length with be 32 bytes. */
( void ) memcpy( &( pucSig[ 4 ] ), ucTemp, 32 ); /* Copy 32 bytes of R into the signature buffer. */
pucSigPtr = &( pucSig[ 32 ] ); /* Increment the pointer for 32 byte R length. */
}

pucSigPtr += 4; /* Increment the pointer to offset the SEQUENCE, LENGTH, R-INTEGER, LENGTH. */
pucSigPtr[ 0 ] = 0x02; /* INTEGER tag for S. */
pucSigPtr += 1; /* Increment over S INTEGER tag. */
pucSigPtr = &( pucSigPtr[ 4 ] ); /* Increment the pointer to offset the SEQUENCE, LENGTH, R-INTEGER, LENGTH. */
pucSigPtr[ 0 ] = 0x02; /* INTEGER tag for S. */
pucSigPtr = &( pucSigPtr[ 1 ] ); /* Increment over S INTEGER tag. */

/******************* S Component. ****************/

/* If the first bit is one, pre-append a 00 byte.
* This prevents the number from being interpreted as negative. */
if( ( ucTemp[ 32 ] & 0x80UL ) == 0x80UL )
{
pucSig[ 1 ]++; /* Increment the length of the structure to account for the 0x00 pad. */
pucSigPtr[ 0 ] = 0x21; /* Increment the length of the S value to account for the 0x00 pad. */
pucSigPtr[ 1 ] = 0x00; /* Write the 0x00 pad. */
pucSigPtr += 2; /* pucSigPtr was pointing at the S-length. Increment by 2 to hop over length and 0 padding. */
pucSig[ 1 ]++; /* Increment the length of the structure to account for the 0x00 pad. */
pucSigPtr[ 0 ] = 0x21; /* Increment the length of the S value to account for the 0x00 pad. */
pucSigPtr[ 1 ] = 0x00; /* Write the 0x00 pad. */
pucSigPtr = &( pucSigPtr[ 2 ] ); /* pucSigPtr was pointing at the S-length. Increment by 2 to hop over length and 0 padding. */

( void ) memcpy( pucSigPtr, &ucTemp[ 32 ], 32 ); /* Copy the S value. */
( void ) memcpy( pucSigPtr, &( ucTemp[ 32 ] ), 32 ); /* Copy the S value. */
}
else
{
pucSigPtr[ 0 ] = 0x20; /* S length will be 32 bytes. */
pucSigPtr++; /* Hop pointer over the length byte. */
( void ) memcpy( pucSigPtr, &ucTemp[ 32 ], 32 ); /* Copy the S value. */
pucSigPtr[ 0 ] = 0x20; /* S length will be 32 bytes. */
pucSigPtr++; /* Hop pointer over the length byte. */
( void ) memcpy( pucSigPtr, &( ucTemp[ 32 ] ), 32 ); /* Copy the S value. */
}

/* The total signature length is the length of the R and S integers plus 2 bytes for
Expand Down
26 changes: 13 additions & 13 deletions source/portable/mbedtls/core_pkcs11_mbedtls.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ static P11Session_t * prvSessionPointerFromHandle( CK_SESSION_HANDLE xSession )
if( ( xSession >= 1UL ) && ( xSession <= pkcs11configMAX_SESSIONS ) )
{
/* Decrement by 1, invalid handles in PKCS #11 are defined to be 0. */
pxSession = &pxP11Sessions[ xSession - 1UL ];
pxSession = &( pxP11Sessions[ xSession - 1UL ] );
}
else
{
Expand Down Expand Up @@ -1309,7 +1309,7 @@ static CK_RV prvAppendEmptyECDerKey( uint8_t * pusECPrivateKey,
* array will be appended to the valid private key.
* It must be removed so that we can read the private
* key back at a later time. */
lCompare = memcmp( &pusECPrivateKey[ ulDerBufSize - 6UL ], emptyPubKey, sizeof( emptyPubKey ) );
lCompare = memcmp( &( pusECPrivateKey[ ulDerBufSize - 6UL ] ), emptyPubKey, sizeof( emptyPubKey ) );

if( ( lCompare == 0 ) && ( *pulActualKeyLength >= 6UL ) )
{
Expand Down Expand Up @@ -1912,7 +1912,7 @@ CK_DECLARE_FUNCTION( CK_RV, C_OpenSession )( CK_SLOT_ID slotID,
if( pxP11Sessions[ ulSessionCount ].xOpened == ( CK_BBOOL ) CK_FALSE )
{
xResult = CKR_OK;
pxSessionObj = &pxP11Sessions[ ulSessionCount ];
pxSessionObj = &( pxP11Sessions[ ulSessionCount ] );
/* 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] */
Expand Down Expand Up @@ -2096,7 +2096,7 @@ static CK_RV prvCreateCertificate( CK_ATTRIBUTE * pxTemplate,
/* Search for the pointer to the certificate VALUE. */
for( ulIndex = 0; ulIndex < ulCount; ulIndex++ )
{
xResult = prvCertAttParse( &pxTemplate[ ulIndex ], &xCertificateType,
xResult = prvCertAttParse( &( pxTemplate[ ulIndex ] ), &xCertificateType,
&pxCertificateValue, &xCertificateLength,
&pxLabel );

Expand Down Expand Up @@ -2186,7 +2186,7 @@ static void prvGetLabel( CK_ATTRIBUTE ** ppxLabel,
if( xAttribute.type == CKA_LABEL )
{
LogDebug( ( "Successfully found the label in the template." ) );
*ppxLabel = &pxTemplate[ ulIndex ];
*ppxLabel = &( pxTemplate[ ulIndex ] );
break;
}
}
Expand Down Expand Up @@ -2388,7 +2388,7 @@ static void prvGetLabel( CK_ATTRIBUTE ** ppxLabel,
{
for( ulIndex = 0; ulIndex < ulCount; ulIndex++ )
{
xResult = prvEcKeyAttParse( &pxTemplate[ ulIndex ], &xMbedContext, xIsPrivate );
xResult = prvEcKeyAttParse( &( pxTemplate[ ulIndex ] ), &xMbedContext, xIsPrivate );

if( xResult != CKR_OK )
{
Expand Down Expand Up @@ -2466,7 +2466,7 @@ static CK_RV prvCreateRsaKey( CK_ATTRIBUTE * pxTemplate,
/* Parse template and collect the relevant parts. */
for( ulIndex = 0; ulIndex < ulCount; ulIndex++ )
{
xResult = prvRsaKeyAttParse( &pxTemplate[ ulIndex ], xMbedContext.pk_ctx, xIsPrivate );
xResult = prvRsaKeyAttParse( &( pxTemplate[ ulIndex ] ), xMbedContext.pk_ctx, xIsPrivate );

if( xResult != CKR_OK )
{
Expand Down Expand Up @@ -2585,7 +2585,7 @@ static CK_RV prvCreateSHA256HMAC( CK_ATTRIBUTE * pxTemplate,
{
for( ulIndex = 0; ulIndex < ulCount; ulIndex++ )
{
xResult = prvHMACKeyAttParse( &pxTemplate[ ulIndex ], &pxSecretKeyValue, &ulSecretKeyValueLen );
xResult = prvHMACKeyAttParse( &( pxTemplate[ ulIndex ] ), &pxSecretKeyValue, &ulSecretKeyValueLen );

if( xResult != CKR_OK )
{
Expand Down Expand Up @@ -2710,7 +2710,7 @@ static CK_RV prvCreateAESCMAC( CK_ATTRIBUTE * pxTemplate,
{
for( ulIndex = 0; ulIndex < ulCount; ulIndex++ )
{
xResult = prvCMACKeyAttParse( &pxTemplate[ ulIndex ], &pxSecretKeyValue, &ulSecretKeyValueLen );
xResult = prvCMACKeyAttParse( &( pxTemplate[ ulIndex ] ), &pxSecretKeyValue, &ulSecretKeyValueLen );

if( xResult != CKR_OK )
{
Expand Down Expand Up @@ -5132,7 +5132,7 @@ CK_DECLARE_FUNCTION( CK_RV, C_Verify )( CK_SESSION_HANDLE hSession,
mbedtls_mpi_init( &xR );
mbedtls_mpi_init( &xS );

lMbedTLSResult = mbedtls_mpi_read_binary( &xR, &pSignature[ 0 ], 32 );
lMbedTLSResult = mbedtls_mpi_read_binary( &xR, &( pSignature[ 0 ] ), 32 );

if( lMbedTLSResult != 0 )
{
Expand All @@ -5144,7 +5144,7 @@ CK_DECLARE_FUNCTION( CK_RV, C_Verify )( CK_SESSION_HANDLE hSession,
}
else
{
lMbedTLSResult = mbedtls_mpi_read_binary( &xS, &pSignature[ 32 ], 32 );
lMbedTLSResult = mbedtls_mpi_read_binary( &xS, &( pSignature[ 32 ] ), 32 );

if( lMbedTLSResult != 0 )
{
Expand Down Expand Up @@ -5633,7 +5633,7 @@ CK_DECLARE_FUNCTION( CK_RV, C_GenerateKeyPair )( CK_SESSION_HANDLE hSession,
for( ulIndex = 0; ulIndex < ulPrivateKeyAttributeCount; ++ulIndex )
{
xResult = prvCheckGenerateKeyPairPrivateTemplate( &pxPrivateLabel,
&pPrivateKeyTemplate[ ulIndex ],
&( pPrivateKeyTemplate[ ulIndex ] ),
&xAttributeMap );

if( xResult != CKR_OK )
Expand All @@ -5657,7 +5657,7 @@ CK_DECLARE_FUNCTION( CK_RV, C_GenerateKeyPair )( CK_SESSION_HANDLE hSession,
for( ulIndex = 0; ulIndex < ulPublicKeyAttributeCount; ++ulIndex )
{
xResult = prvCheckGenerateKeyPairPublicTemplate( &pxPublicLabel,
&pPublicKeyTemplate[ ulIndex ],
&( pPublicKeyTemplate[ ulIndex ] ),
&xAttributeMap );

if( xResult != CKR_OK )
Expand Down
63 changes: 32 additions & 31 deletions tools/coverity/misra.config
Original file line number Diff line number Diff line change
@@ -1,58 +1,59 @@
// MISRA C-2012 Rules

{
version : "2.0",
standard : "c2012",
title: "Coverity MISRA Configuration",
deviations : [
// Disable the following rules.
"version" : "2.0",
"standard" : "c2012",
"title": "Coverity MISRA Configuration",
"deviations" : [
{
deviation: "Directive 4.5",
reason: "Allow names that MISRA considers ambiguous (such as enum IOT_MQTT_CONNECT and function IotMqtt_Connect)."
"deviation": "Directive 4.5",
"reason": "Allow names that MISRA considers ambiguous (such as enum IOT_MQTT_CONNECT and function IotMqtt_Connect)."
},
{
deviation: "Directive 4.8",
reason: "Allow inclusion of unused types. Header files for a specific port, which are needed by all files, may define types that are not used by a specific file."
"deviation": "Directive 4.8",
"reason": "Allow inclusion of unused types. Header files for a specific port, which are needed by all files, may define types that are not used by a specific file."
},
{
deviation: "Directive 4.9",
reason: "Allow inclusion of function like macros. Logging is done using function like macros."
"deviation": "Directive 4.9",
"reason": "Allow inclusion of function like macros. Logging is done using function like macros."
},
{
deviation: "Directive 4.12",
reason: "Allow use of malloc. This library uses malloc to create cryptographic objects."
"deviation": "Directive 4.12",
"reason": "Allow use of malloc. This library uses malloc to create cryptographic objects."
},
{
deviation: "Rule 2.3",
reason: "Allow unused types. Library headers may define types intended for the application's use, but not used within the library files."
"deviation": "Rule 2.3",
"reason": "Allow unused types. Library headers may define types intended for the application's use, but not used within the library files."
},
{
deviation: "Rule 2.4",
reason: "Allow unused macros. Library headers may define macros intended for the application's use, but not used by a specific file."
"deviation": "Rule 2.4",
"reason": "Allow unused macros. Library headers may define macros intended for the application's use, but not used by a specific file."
},
{
deviation: "Rule 2.5",
reason: "Allow unused macros. Library headers may define macros intended for the application's use, but not used by a specific file."
"deviation": "Rule 2.5",
"reason": "Allow unused macros. Library headers may define macros intended for the application's use, but not used by a specific file."
},
{
deviation: "Rule 3.1",
reason: "Allow nested comments. Documentation blocks contain comments for example code."
"deviation": "Rule 3.1",
"reason": "Allow nested comments. Documentation blocks contain comments for example code."
},
{
deviation: "Rule 8.7",
reason: "API functions are not used by library. They must be externally visible in order to be used by the application."
"deviation": "Rule 8.7",
"reason": "API functions are not used by library. They must be externally visible in order to be used by the application."
},
{
deviation: "Rule 8.13",
reason: "The PKCS #11 API is defined by the PKCS #11 header files distributed by OASIS. There are some parameters that could be const qualified in this implementation, but since the API cannot be modified, are not const qualified."
"deviation": "Rule 8.13",
"reason": "The PKCS #11 API is defined by the PKCS #11 header files distributed by OASIS. There are some parameters that could be const qualified in this implementation, but since the API cannot be modified, are not const qualified."
},
{
deviation: "Rule 21.1",
reason: "Allow use of all macro names. For compatibility, some macros introduced in C99 are defined for use with C90 compilers."
"deviation": "Rule 20.5",
"reason": "Allow use of undef for a workaround to run in windows."
},
{
deviation: "Rule 21.2",
reason: " Allow use of all macro and identifier names. For compatibility, some macros introduced in C99 are defined for use with C90 compilers."
"deviation": "Rule 21.1",
"reason": "Allow use of all macro names. For compatibility, some macros introduced in C99 are defined for use with C90 compilers."
},
{
"deviation": "Rule 21.2",
"reason": " Allow use of all macro and identifier names. For compatibility, some macros introduced in C99 are defined for use with C90 compilers."
}
]
}

0 comments on commit e0cd4db

Please sign in to comment.