HMAC-SHA1 implementation

I am trying out a small piece of code that would generate HMAC-SHA1. I have been asked to code the HMAC implementation myself using the OpenSSL libs for SHA1 calculation.

After 'wiki'ing for the algorithm, here is what I have below. I have used input with RFC 2104 specified test values:


key = 0x0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b

key_len = 16
bytes

data = "Hi There"

data_len = 8 bytes digest =
0x9294727a3638bb1c13f48ef8158bfc9d

But the output I get is:

675b0b3a1b4ddf4e124872da6c2f632bfed957e9*

Is this really not the way to implement HMAC? How else can this be done?

```
#include
#include
#include
#include
#include
#include
#include
#include
#include / for memset() /
#include

#define IPAD 0x36
#define OPAD 0x5C

#define SHA1_DIGESTLENGTH 20
#define SHA1_BLOCK_LENGTH 64
#define COUNTER_LENGTH 8

typedef unsigned char uint8_t;
typedef unsigned short int uint16_t;
typedef unsigned int uint32_t;

/**
* Key
*/
#define SECRET { 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b , 0x0b , 0x0b , 0x0b , 0x0b , 0x0b , 0x0b , 0x0b , 0x0b}
#define COUNTER { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }
/*
* hmac sha1.
*/
int main(int argc, char *argv[]){
unsigned char ipad[SHA1_BLOCK_LENGTH];
unsigned char opad[SHA1_BLOCK_LENGTH];
unsigned char algKey[SHA1_BLOCK_LENGTH];
unsigned char msgBuffer[SHA1_BLOCK_LENGTH + COUNTER_LENGTH];
unsigned char valBuffer[SHA1_BLOCK_LENGTH + 20];
unsigned int i;
uint8_t ctr[] = "Hi There";
uint8_t hash[20];
uint8_t key[]= SECRET;
int len = 16;
memset(hash, 0, sizeof(hash));
memset(algKey, 0, sizeof(algKey));
strncpy((char )algKey, (char )key, len);

memset(ipad, IPAD, sizeof(ipad));
memset(opad, OPAD, sizeof(opad));

for (i = 0; i < SHA1_BLOCK_LENGTH; i++){
ipad[i] ^= algKey[i];
}

for (i = 0; i < SHA1_BLOCK_LENGTH; i++){
opad[i] ^= algKey[i];
}

-
You can make these #defines:

#define SHA1_DIGESTLENGTH 20
#define SHA1_BLOCK_LENGTH 64
#define COUNTER_LENGTH 8

into a more concise enum:

typedef enum { COUNTER=8, SHA1_DIGEST=20, SHA1_BLOCK=64 } Length;

This could also be done with SECRET and COUNTER, but each hex value would need a name.

If you use the enum recommendation, you may need to rename the second COUNTER to avoid possible name-clashing. Perhaps rename it based on what the counter is for.

-
You don't need your own typedefs for these:

typedef unsigned           char uint8_t;
typedef unsigned short     int uint16_t;
typedef unsigned           int uint32_t;

Prefer to use the typedefs already defined in `.

-
For unformatted output with a newline, use puts() instead of printf():

puts("Some text...");

-
I'd recommend splitting this program into separate functions. This will make it easier to tell how the it works and make it easier to maintain. A function should have one purpose, andmain() should have minimal implementation (such as acquiring input and calling other functions).

-
There's no need to call exit(1) within main(). Just return 1.

The return 0` is also not needed at the end. Reaching this point implies successful termination, so the compiler will do this return for you.

StackExchange Code Review Q#26673, answer score: 6