Fixed PKCS5_PBKDF2_HMAC() to adhere to the documentation.
authorPéter Budai <buc@peterbudai.eu>
Tue, 11 Oct 2016 17:26:23 +0000 (19:26 +0200)
committerRich Salz <rsalz@openssl.org>
Fri, 17 Mar 2017 12:47:11 +0000 (08:47 -0400)
The documentation of this function states that the password parameter
can be NULL. However, the implementation returns an error in this case
due to the inner workings of the HMAC_Init_ex() function.
With this change, NULL password will be treated as an empty string and
PKCS5_PBKDF2_HMAC() no longer fails on this input.

I have also added two new test cases that tests the handling of the
special values NULL and -1 of the password and passlen parameters,
respectively.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1692)

crypto/evp/p5_crpt2.c
test/evp_test.c
test/evptests.txt

index 024996fc4957e6ab364290372f197dbc44247af3..c7b08e164fc13e9ae836c5568ae50d3eb6532856 100644 (file)
@@ -33,6 +33,7 @@ int PKCS5_PBKDF2_HMAC(const char *pass, int passlen,
                       const unsigned char *salt, int saltlen, int iter,
                       const EVP_MD *digest, int keylen, unsigned char *out)
 {
+    const char *empty = "";
     unsigned char digtmp[EVP_MAX_MD_SIZE], *p, itmp[4];
     int cplen, j, k, tkeylen, mdlen;
     unsigned long i = 1;
@@ -47,10 +48,12 @@ int PKCS5_PBKDF2_HMAC(const char *pass, int passlen,
         return 0;
     p = out;
     tkeylen = keylen;
-    if (!pass)
+    if (pass == NULL) {
+        pass = empty;
         passlen = 0;
-    else if (passlen == -1)
+    } else if (passlen == -1) {
         passlen = strlen(pass);
+    }
     if (!HMAC_Init_ex(hctx_tpl, pass, passlen, digest, NULL)) {
         HMAC_CTX_free(hctx_tpl);
         return 0;
index d924e3f6fcb801521fadd20ddf4271e5fa200a79..f9dafec23a0f70bd6437941df86eba46d636f60a 100644 (file)
@@ -127,6 +127,8 @@ static int test_bin(const char *value, unsigned char **buf, size_t *buflen)
     long len;
 
     *buflen = 0;
+
+    /* Check for empty value */
     if (!*value) {
         /*
          * Don't return NULL for zero length buffer.
@@ -141,6 +143,14 @@ static int test_bin(const char *value, unsigned char **buf, size_t *buflen)
         *buflen = 0;
         return 1;
     }
+
+    /* Check for NULL literal */
+    if (strcmp(value, "NULL") == 0) {
+        *buf = NULL;
+        *buflen = 0;
+        return 1;
+    }
+
     /* Check for string literal */
     if (value[0] == '"') {
         size_t vlen;
@@ -155,6 +165,7 @@ static int test_bin(const char *value, unsigned char **buf, size_t *buflen)
         return 1;
     }
 
+    /* Otherwise assume as hex literal and convert it to binary buffer */
     *buf = OPENSSL_hexstr2buf(value, &len);
     if (!*buf) {
         fprintf(stderr, "Value=%s\n", value);
@@ -640,7 +651,7 @@ int main(int argc, char **argv)
 
     memset(&t, 0, sizeof(t));
     t.start_line = -1;
-    in = BIO_new_file(argv[1], "r");
+    in = BIO_new_file(argv[1], "rb");
     if (in == NULL) {
         fprintf(stderr, "Can't open %s for reading\n", argv[1]);
         return 1;
index e9e796448a8f7480aa3fe78a992c5055a10a8ca6..a305b02e98e3b3e172a29a14cc52d3c30c232712 100644 (file)
@@ -3482,6 +3482,49 @@ iter = 4096
 MD = sha512
 Key = 9d9e9c4cd21fe4be24d5b8244c759665
 
+# PBKDF2 tests for empty and NULL inputs
+PBE = pbkdf2
+Password = ""
+Salt = "salt"
+iter = 1
+MD = sha1
+Key = a33dddc30478185515311f8752895d36ea4363a2
+
+PBE = pbkdf2
+Password = ""
+Salt = "salt"
+iter = 1
+MD = sha256
+Key = f135c27993baf98773c5cdb40a5706ce6a345cde
+
+PBE = pbkdf2
+Password = ""
+Salt = "salt"
+iter = 1
+MD = sha512
+Key = 00ef42cdbfc98d29db20976608e455567fdddf14
+
+PBE = pbkdf2
+Password = NULL
+Salt = "salt"
+iter = 1
+MD = sha1
+Key = a33dddc30478185515311f8752895d36ea4363a2
+
+PBE = pbkdf2
+Password = NULL
+Salt = "salt"
+iter = 1
+MD = sha256
+Key = f135c27993baf98773c5cdb40a5706ce6a345cde
+
+PBE = pbkdf2
+Password = NULL
+Salt = "salt"
+iter = 1
+MD = sha512
+Key = 00ef42cdbfc98d29db20976608e455567fdddf14
+
 # Base64 tests
 
 Encoding = canonical