HAL2 Hash issue when handling incomplete bytes

[GClarksonBAI]

I believe I’ve encountered an Issue with the current version of the STM32 HAL2 and wanted to raise an issue, but I’m not sure where best to raise the issue.

The one of the new features in the new HAL2 hash is that HAL_HASH_Update can handle input sizes that don’t perfectly line up with the block size, using HASH_SaveRemainingBytes to save any excess bytes at the end of a call and then calling HASH_AppendLastIncompleteWord at the next call to write those excess bytes to peripheral along with using some bytes from the new buffer to bulk it up to 4.
 

This means that I can write something like: (where the buffer is 163 bytes long)

HAL_HASH_Update(&hhash, &u8InputBuffer[0], 10,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[10], 10,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[20], 1,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[21], 62,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[83], 7,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[90], 73,0xFFFF);

This correctly computes the correct HASH. If however I do:

HAL_HASH_Update(&hhash, &u8InputBuffer[0], 10,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[10], 10,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[20], 1,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[21], 1,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[22], 1,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[23], 1,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[24], 1,0xFFFF);
HAL_HASH_Update(&hhash, &u8InputBuffer[25], 138,0xFFFF);

This fails to calculate the correct Hash. I’ve investigated and the issue looks to be in HASH_AppendLastIncompleteWord. The problem is that HASH_AppendLastIncompleteWord doesn’t check the number of bytes that have come in, it just assumes that there will be at least 4 bytes in the buffer.

Please let me know where I can raise this so that it can be fixed.
 

 

[Saket_Om]

Hello ​@GClarksonBAI 

A call to HAL_HASH_Finish() is missing in your code. 

This is noted in the driver.

“Consecutive calls to HAL_HASH_Update() can be used to feed several input buffers back-to-back to the
  *         peripheral that will yield a single HASH signature once all buffers have been entered.
  *         Wrap-up of input buffers feeding and retrieval of digest is done by a call to HAL_HASH_Finish().”

Please refer to the example code below: 

{
    hhash.instance = HAL_HASH;
    HAL_HASH_DeInit(&hhash);
    HAL_HASH_Init(&hhash, HAL_HASH);

    hash_cfg.data_swapping  = HAL_HASH_DATA_SWAP_BYTE;
    hash_cfg.algorithm = HAL_HASH_ALGO_SHA1;
    if (HAL_HASH_SetConfig(&hhash, &hash_cfg) != HAL_OK)
    {
      return ++errorcounter;
    }
    /* feed Input  */
    if(HAL_HASH_Update(&hhash, Msg65_Input, 10,  1) != HAL_OK)
    {
      return ++errorcounter;
    }
    /* feed Input  */
    if(HAL_HASH_Update(&hhash, Msg65_Input+10, 19, 1000) != HAL_OK)
    {
      return ++errorcounter;
    }
    /* feed Input  */
    if(HAL_HASH_Update(&hhash, Msg65_Input+29, 3, 1000) != HAL_OK)
    {
      return ++errorcounter;
    }
    /* feed Input & Compute SHA1 */
    if(HAL_HASH_Update(&hhash, Msg65_Input+32, 33, 1000) != HAL_OK)
    {
      return ++errorcounter;
    }

    if(HAL_HASH_Finish(&hhash, Sha1output, 20, &sha1output_size, 1000) != HAL_OK)
    {
      return ++errorcounter;
    }

 

[GClarksonBAI]

Hi,

I was only including a snippet of code to help explain my point. My full code is using HAL_HASH_Finish as well as HAL_HASH_SetConfig which is how I am getting a hash out. Taking the example code you sent I can replicate the issue with:

/* will produce a correct hash */
{
   hhash.instance = HAL_HASH;
   HAL_HASH_DeInit(&hhash);
   HAL_HASH_Init(&hhash, HAL_HASH);
   hash_cfg.data_swapping = HAL_HASH_DATA_SWAP_BYTE;
   hash_cfg.algorithm = HAL_HASH_ALGO_SHA1;

   if (HAL_HASH_SetConfig(&hhash, &hash_cfg) != HAL_OK)
   {
   return ++errorcounter;
   }

   /* feed Input first half*/
   if(HAL_HASH_Update(&hhash, &Msg65_Input[0], 30, 1) != HAL_OK)
   {
      return ++errorcounter;
   }

   /* feed Input second*/
   if(HAL_HASH_Update(&hhash, &Msg65_Input[30], 35, 1) != HAL_OK)
   {
      return ++errorcounter;
   }

   if(HAL_HASH_Finish(&hhash, Sha1output, 20, &sha1output_size, 1000) != HAL_OK)
   {
      return ++errorcounter;
   }
}

/* will produce an incorrect hash */
{
   hhash.instance = HAL_HASH;
   HAL_HASH_DeInit(&hhash);
   HAL_HASH_Init(&hhash, HAL_HASH);
   hash_cfg.data_swapping = HAL_HASH_DATA_SWAP_BYTE;
   hash_cfg.algorithm = HAL_HASH_ALGO_SHA1;

   if (HAL_HASH_SetConfig(&hhash, &hash_cfg) != HAL_OK)
   {
   return ++errorcounter;
   }

   /* feed Input */
   for(uint8_t index = 0; index < sizeof(Msg65_Input); index++)
   {
      if(HAL_HASH_Update(&hhash, Msg65_Input[index], 1, 1) != HAL_OK)
      {
         return ++errorcounter;
      }
   }

   if(HAL_HASH_Finish(&hhash, Sha1output, 20, &sha1output_size, 1000) != HAL_OK)
   {
      return ++errorcounter;
   }
}

As I mentioned in the previous message the issue is in HASH_AppendLastIncompleteWord

static void HASH_AppendLastIncompleteWord(hal_hash_handle_t *hhash)
{
  /* Process additional bytes */
  uint32_t tmp = 0U;
  for (uint32_t i = 0U; i < hhash->remain_bytes_number; i++)
  {
    tmp |= ((uint32_t)hhash->remain_bytes[i] << ((uint32_t)(i * 8U)));
  }

  /* Process the first bytes of the buffer to complete the 4 bytes */
  for (uint32_t i = 0U; (i < (4U - (uint32_t)hhash->remain_bytes_number)) && (i < hhash->input_size_byte); i++)
  {
    tmp |= (*(uint32_t *)(uint32_t)hhash->p_input_buff) << ((i + (uint32_t)hhash->remain_bytes_number) * 8U);
    hhash->p_input_buff += 1U;
  }
  HASH_GET_INSTANCE(hhash)->DIN = tmp;

  /* Adjust buffer size after processing first bytes */
  hhash->input_size_byte -= (hhash->input_size_byte > (4U - (uint32_t)hhash->remain_bytes_number)) ?
                            (4U - (uint32_t)hhash->remain_bytes_number) : hhash->input_size_byte;
  hhash->remain_bytes_number = 0U;
}

The problem is that there is no check that hhash->p_input_buff contains enough bytes to complete the 4 bytes required.

[Saket_Om]

Hello ​@GClarksonBAI 

There is a mistake in your for loop. 

it should be like below: 

/* feed Input */
   for(uint8_t index = 0; index < sizeof(Msg65_Input)-20; index++)
   {
      if(HAL_HASH_Update(&hhash, Msg65_Input[index], 1, 1) != HAL_OK)
      {
         return ++errorcounter;
      }
   }

 

[GClarksonBAI]

No, it was correct.

You’re now just not sending the last 20 bytes of the message.

If you’re looking at HAL_HASH_Finish and thinking that is sending 20 bytes then you’ve misread HAL_HASH_Finish