Fix complexity and hygiene improvements

source/include/sigv4_internal.h

@@ -28,8 +28,6 @@
 #ifndef SIGV4_INTERNAL_H_
 #define SIGV4_INTERNAL_H_
 
-#include <stdbool.h>
-
 /* SIGV4_DO_NOT_USE_CUSTOM_CONFIG allows building of the SigV4 library without a
  * config file. If a config file is provided, the SIGV4_DO_NOT_USE_CUSTOM_CONFIG
  * macro must not be defined.

source/sigv4.c

@@ -2272,6 +2272,11 @@ static int32_t hmacAddKey( HmacContext_t * pHmacContext,
     }
     else
     {
+        /* To reduce the key length to less than the hash block size, this branch performs
+         * hash operations. We want to perform hash operations only when we have received the
+         * entire key. */
+        assert( isKeyPrefix == false );
+
         returnStatus = pCryptoInterface->hashInit( pCryptoInterface->pHashContext );
 
         /* Has part of the key that is cached in the HMAC context. */
@@ -2813,6 +2818,7 @@ static SigV4Status_t generateSigningKey( const SigV4Parameters_t * pSigV4Params,
                                  SIGV4_HMAC_SIGNING_KEY_PREFIX,
                                  SIGV4_HMAC_SIGNING_KEY_PREFIX_LEN,
                                  true /* Is key prefix. */ );
+        /* The above call should always succeed as it only populates the HMAC key cache. */
         assert( hmacStatus == 0 );
     }
 
@@ -2988,20 +2994,9 @@ SigV4Status_t SigV4_GenerateHTTPAuthorization( const SigV4Parameters_t * pParams
 
     if( returnStatus == SigV4Success )
     {
-        authPrefixLen = *authBufLen;
-
-        /* Default arguments. */
-        if( ( pParams->pAlgorithm == NULL ) || ( pParams->algorithmLen == 0U ) )
-        {
-            /* The default algorithm is AWS4-HMAC-SHA256. */
-            pAlgorithm = SIGV4_AWS4_HMAC_SHA256;
-            algorithmLen = SIGV4_AWS4_HMAC_SHA256_LENGTH;
-        }
-        else
-        {
-            pAlgorithm = pParams->pAlgorithm;
-            algorithmLen = pParams->algorithmLen;
-        }
+        /* If the SigV4 algorithm is not specified, use "AWS4-HMAC-256" as the default algorithm. */
+        pAlgorithm = ( pParams->pAlgorithm == NULL ) ? SIGV4_AWS4_HMAC_SHA256 : pParams->pAlgorithm;
+        algorithmLen = ( pParams->pAlgorithm == NULL ) ? SIGV4_AWS4_HMAC_SHA256_LENGTH : pParams->algorithmLen;
     }
 
     if( returnStatus == SigV4Success )
@@ -3025,6 +3020,7 @@ SigV4Status_t SigV4_GenerateHTTPAuthorization( const SigV4Parameters_t * pParams
     /* Write the prefix of the Authorizaton header value. */
     if( returnStatus == SigV4Success )
     {
+        authPrefixLen = *authBufLen;
         returnStatus = generateAuthorizationValuePrefix( pParams,
                                                          pAlgorithm, algorithmLen,
                                                          pSignedHeaders, signedHeadersLen,

source/sigv4_quicksort.c

@@ -147,8 +147,7 @@ static void quickSortHelper( void * pArray,
         /* Calculate length of the left partition containing items smaller
          * than the pivot element.
          * The length is zero if either:
-         * 1. The pivoted item is the smallest in the
-         * the array before partitioning.
+         * 1. The pivoted item is the smallest in the the array before partitioning.
          *              OR
          * 2. The left partition is only of single length which can be treated as
          * sorted, and thus, of zero length for avoided adding to the stack. */
@@ -157,7 +156,7 @@ static void quickSortHelper( void * pArray,
         /* Calculate length of the right partition containing items greater than
          * or equal to the pivot item.
          * The calculated length is zero if either:
-         * 1. The pivoted item is the smallest in the the array before partitioning.
+         * 1. The pivoted item is the greatest in the the array before partitioning.
          *              OR
          * 2. The right partition contains only a single length which can be treated as
          * sorted, and thereby, of zero length to avoid adding to the stack. */
@@ -202,31 +201,32 @@ static size_t partition( void * pArray,
                          size_t itemSize,
                          ComparisonFunc_t comparator )
 {
-    void * pivot;
+    uint8_t * pivot;
+    uint8_t * pArrayLocal = ( uint8_t * ) pArray;
     size_t i = low - 1U, j = low;
 
     assert( pArray != NULL );
     assert( comparator != NULL );
 
     /* Choose pivot as the highest indexed item in the current partition. */
-    pivot = pArray + ( high * itemSize );
+    pivot = pArrayLocal + ( high * itemSize );
 
     /* Iterate over all elements of the current array to partition it
      * in comparison to the chosen pivot with smaller items on the left
      * and larger or equal to items on the right. */
     for( ; j < high; j++ )
     {
         /* Use comparator function to check current element is smaller than the pivot */
-        if( comparator( pArray + ( j * itemSize ), pivot ) < 0 )
+        if( comparator( pArrayLocal + ( j * itemSize ), pivot ) < 0 )
         {
             ++i;
-            swap( pArray + ( i * itemSize ), pArray + ( j * itemSize ), itemSize );
+            swap( pArrayLocal + ( i * itemSize ), pArrayLocal + ( j * itemSize ), itemSize );
         }
     }
 
     /* Place the pivot between the smaller and larger item chunks of
      * the array. This represents the 2 partitions of the array. */
-    swap( pArray + ( ( i + 1U ) * itemSize ), pivot, itemSize );
+    swap( pArrayLocal + ( ( i + 1U ) * itemSize ), pivot, itemSize );
 
     /* Return the pivot item's index. */
     return i + 1U;

test/unit-test/sigv4_utest.c

@@ -245,37 +245,9 @@ static int32_t valid_sha256_final( void * pHashContext,
     return -1;
 }
 
-/*==================== Echo Implementation of Crypto Interface ===================== */
-
-static uint8_t hashEchoBuffer[ SIGV4_HASH_MAX_BLOCK_LENGTH ];
-static size_t hashInputLen;
-
-/* These hash functions simply take the input and write it back to the output.
- * The purpose of which is make it possible to write tests without having to
- * know the computed hash of the string to sign. */
-static int32_t echo_hash_init( void * pHashContext )
-{
-    return 0;
-}
-
-static int32_t echo_hash_update( void * pHashContext,
-                                 const uint8_t * pInput,
-                                 size_t inputLen )
-{
-    hashInputLen = inputLen;
-    ( void ) memcpy( hashEchoBuffer, pInput, inputLen );
-}
-
-static int32_t echo_hash_final( void * pHashContext,
-                                uint8_t * pOutput,
-                                size_t outputLen )
-{
-    ( void ) memcpy( pOutput, hashEchoBuffer, hashInputLen );
-}
-
 /*==================== Failable Implementation of Crypto Interface ===================== */
 
-#define HAPPY_PATH_HASH_ITERATIONS    12U
+#define HASH_ERROR_BRANCH_COVERAGE_ITERATIONS    12U
 
 static size_t hashInitCalledCount = 0U, hashInitCallToFail = SIZE_MAX;
 static size_t updateHashCalledCount = 0U, updateHashCallToFail = SIZE_MAX;
@@ -1186,24 +1158,28 @@ void test_SigV4_GenerateHTTPAuthorization_Hash_Errors()
 
     params.pCredentials->pSecretAccessKey = SECRET_KEY_LONGER_THAN_HASH_BLOCK;
     params.pCredentials->secretAccessKeyLen = strlen( SECRET_KEY_LONGER_THAN_HASH_BLOCK );
+    char failureMessage[ 250 ];
 
-    for( i = 0U; i < HAPPY_PATH_HASH_ITERATIONS; i++ )
+    for( i = 0U; i < HASH_ERROR_BRANCH_COVERAGE_ITERATIONS; i++ )
     {
         resetFailableHashParams();
         hashInitCallToFail = i;
         params.pCryptoInterface->hashInit = hash_init_failable;
         returnStatus = SigV4_GenerateHTTPAuthorization( &params, authBuf, &authBufLen, &signature, &signatureLen );
-        TEST_ASSERT_EQUAL( SigV4HashError, returnStatus );
+        snprintf( failureMessage, sizeof( failureMessage ), "Expected SigV4HashError from hashInit failure at call count %ld", i );
+        TEST_ASSERT_EQUAL_MESSAGE( SigV4HashError, returnStatus, failureMessage );
 
         resetFailableHashParams();
         updateHashCallToFail = i;
         returnStatus = SigV4_GenerateHTTPAuthorization( &params, authBuf, &authBufLen, &signature, &signatureLen );
-        TEST_ASSERT_EQUAL( SigV4HashError, returnStatus );
+        snprintf( failureMessage, sizeof( failureMessage ), "Expected SigV4HashError from hashUpdate failure at call count %ld", i );
+        TEST_ASSERT_EQUAL_MESSAGE( SigV4HashError, returnStatus, failureMessage );
 
         resetFailableHashParams();
         finalHashCallToFail = i;
         returnStatus = SigV4_GenerateHTTPAuthorization( &params, authBuf, &authBufLen, &signature, &signatureLen );
-        TEST_ASSERT_EQUAL( SigV4HashError, returnStatus );
+        snprintf( failureMessage, sizeof( failureMessage ), "Expected SigV4HashError from hashFinal failure at call count %ld", i );
+        TEST_ASSERT_EQUAL_MESSAGE( SigV4HashError, returnStatus, failureMessage );
     }
 }