Mbed TLS is now part of TrustedFirmware.org.

hmac sha256 incorrect signature

May 16, 2017 15:20

Good morning,

I believe I've came across a possible bug in the library whereby with a specific payload the generated hash I believe is being null terminated by the hash output, and therefore producing an incorrect signature. Please see example below to replicate issue.

Note: This only happens with this specific input which generates the specific hash below. Other input variations work as expected, since their hash would be different and doesn't have the two zeros '00', which I believe to be the culprit.

Processor ARM11 - 32bit


[sample code]

#include <string.h>
#include "mbedtls/md.h"
#include "mbedtls/sha256.h"

char *_generate_signature(char *api_secret, char *payload)
  int i,j = 0;
  unsigned char hmac_hash[64] = {'\0'};
  unsigned char signature[64] = {'\0'};
  mbedtls_md_context_t ctx;
  mbedtls_md_type_t md_type = MBEDTLS_MD_SHA256;

  char payload_buffer[512] = {'\0'};
  const size_t payload_len = strlen(payload);
  size_t key_len = strlen(api_secret);            
  strcpy(payload_buffer, payload);        

  mbedtls_md_setup(&ctx, mbedtls_md_info_from_type(md_type) , 1); //use hmac
  mbedtls_md_hmac_starts(&ctx, api_secret, key_len);
  mbedtls_md_hmac_update(&ctx, (const unsigned char *) payload_buffer, payload_len);    
  mbedtls_md_hmac_finish(&ctx, hmac_hash);

  //convert bytes to hex string
  for(i=0; i<strlen(hmac_hash); i++)
    sprintf(signature+(i+j), "%02x", hmac_hash[i]);
  return signature;
} //fn _generate_signature

[/sample code]






Current Output


Expected Correct Output 51740cabe01de8335383f65aa2324d5c71b800f47edc9257f1fde425c3452292

Thanks for you assistance.

May 17, 2017 07:46
Ron Eldor

Hi add,
strlen returns the length of a string. meaning it returns how many characters(bytes) there are until \0 character ( which is value 0 ).
When you are converting the bytes to hex, you are using strlen which returns you a different length than it really is, because after the 5c71b8 byte sequence, there is a byte of value 0.
Instead of the strlen, you should be doing sizeof(hmac_hash) (which is 64 ).
Better yet, instead of copying every byte seperately in a loop, it is better you do memcpy( signature, hmac_hash, sizeof(hmac_hash) ).
The hash signatures are not strings, but byte buffers, and should be treated accordingly.
mbed TLS Team member

May 17, 2017 21:03

Hi Ron,

You're absolutely right. Thanks very much. The only reason I'm looping through the byte array is because I needed the signature as a hex string, which happens to overflow now as I've replaced the strlen with sizeof, since it doesn't have a null terminator. Note: The original problem was fixed with your suggestion.

Thanks for your help.