OpenSSL PBKDF2-HMAC-SHAx proof of concept I have regrettably been away from C programming for a very long time, so I'd like to get a quick code review of a first proof of concept, before I get too far into adding capabilities, using the getopt library for arguments, alternate output and input character sets, a PolarSSL implementation, and so on and so forth.

Target platforms: gcc 4.5+ for MinGW for Windows, gcc 4.7+ for Linux.

My ranking of concerns:


Security holes
Incorrect use of OpenSSL
Memory leaks
Performance
Suboptimal or incorrect compilation commands
Inefficient code
deviations from ANSI C/portability
Unclean code
Suggestions for improvement
other


As a purely secondary interest, if anyone has advice on the getopt library, C code for Base32/Base64/etc output formats, or PolarSSL use, I'd appreciate it.

Linux compilation:

gcc -O3 -lssl pbkdf2.c -o pbkdf2


Windows compilation:

gcc -O3 -I OpenSSLLibraryPath\include -L OpenSSLBinariesPath -leay32 pbkdf2.c -o pbkdf2


The code:

#include &lt;string.h&gt;
#include &lt;stdio.h&gt;
#include &lt;stdint.h&gt;
#include &lt;openssl/evp.h&gt;
#include &lt;openssl/sha.h&gt;

// Originally from http://stackoverflow.com/questions/10067729/fast-sha-2-authentication-with-apache-is-it-even-possible


void PBKDF2_HMAC_SHA_256(const char* pass, const unsigned char* salt, int32_t iterations, uint32_t outputbytes, char* HexResult)
{
    unsigned int i;
    unsigned char digest[outputbytes];
    PKCS5_PBKDF2_HMAC(pass, strlen(pass), salt, strlen(salt), iterations, EVP_sha256(), outputbytes, digest);
    for (i = 0; i &lt; sizeof(digest); i++)
        sprintf(HexResult + (i * 2), "%02x", 255 &amp; digest[i]);
}

void PBKDF2_HMAC_SHA_512(const char* pass, const unsigned char* salt, int32_t iterations, uint32_t outputbytes, char* HexResult)
{
    unsigned int i;
    unsigned char digest[outputbytes];
    PKCS5_PBKDF2_HMAC(pass, strlen(pass), salt, strlen(salt), iterations, EVP_sha512(), outputbytes, digest);
    for (i = 0; i &lt; sizeof(digest); i++)
        sprintf(HexResult + (i * 2), "%02x", 255 &amp; digest[i]);
}


int main(int argc, char *argv[])
{
  printf("Usage: %s SHA-512|SHA-256 password salt iterations outputlengthinbytes\nSHA-512 is recommended (for password hashing, outputlengthinbytes &lt;= 64)\nSHA-256 is not as good, (for password hashing, outputlengthinbytes &lt;= 32)\n",argv[0]);
  if(argc != 6){printf("Incorrect number of parameters.\n");return 2;};
  char pass[strlen(argv[2])];
  unsigned char salt[strlen(argv[3])];
  int32_t iterations;
  uint32_t outputbytes;

  strcpy(pass,argv[2]);
  strcpy(salt,argv[3]);
  iterations = atoi(argv[4]);
  outputbytes = atoi(argv[5]);
  printf("\n%s\n",argv[1]);

  char HexResult[2*outputbytes-1];

//    printf("Computing PBKDF2(HMAC-SHA512, '%s', '%s', %d, %d) ...\n", pass, salt, iterations, outputbytes);
  memset(HexResult, 0, outputbytes);
  if (strcmp(argv[1],"SHA-512")==0)
    {
    PBKDF2_HMAC_SHA_512(pass, salt, iterations, outputbytes, HexResult);
    }
  else if (strcmp(argv[1],"SHA-256")==0)
    {
    PBKDF2_HMAC_SHA_256(pass, salt, iterations, outputbytes, HexResult);
    }
  else
    {
    printf("Invalid value for hash - choose one of:\nSHA-512\nSHA-256\n");
    return 1;
    }

  printf("Result: in Base16 (hex)\n%s\n", HexResult);
  return 0;
}
Things you did well on:


Looks like you weren't too rusty based on this code.  Looks very nice organizationally.
You return different values for different error conditions.


Things you could improve on:

Standards


Initialize i in your for loops.(C99)

for (unsigned int i = 0; i &lt; sizeof(digest); i++)



Syntax


Don't over-compact your code.


if(argc != 6){printf("Incorrect number of parameters.\n");return 2;};



Let it breathe a bit.

if(argc != 6)
{
    printf("Incorrect number of parameters.\n");
    return 2;
}

Use puts() instead of printf() when you aren't formatting a string.

puts("Incorrect number of parameters.");

You don't follow proper C naming conventions.


 char HexResult[2*outputbytes-1];



Either use camelCase, or snake_case with variable names.
Your indentation is a bit odd.


if (strcmp(argv[1],"SHA-512")==0)
  {
  PBKDF2_HMAC_SHA_512(pass, salt, iterations, outputbytes, HexResult);
  }



But that could be a copy-paste issue.


Commenting:


The only comments I see are a link to a Stack Overflow post, and a commented-out bit of old code.  Use more comments to explain what your code does, and how it accomplishes that/why it works.

Look at it as if a new developer was going to look at your code.  You should try to explain it so that he could understand it's basic function and how it works right away.  You might thank yourself when you return back to this project in the future, forgetting completely how it works. ;)