From fa013b65241dfed9b7d9e10e0adfedc9869c797e Mon Sep 17 00:00:00 2001 From: =?utf8?q?P=C3=A9ter=20Budai?= Date: Tue, 11 Oct 2016 19:26:23 +0200 Subject: [PATCH] Fixed PKCS5_PBKDF2_HMAC() to adhere to the documentation. 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 Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/1692) --- crypto/evp/p5_crpt2.c | 7 +++++-- test/evp_test.c | 13 ++++++++++++- test/evptests.txt | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/crypto/evp/p5_crpt2.c b/crypto/evp/p5_crpt2.c index 024996fc49..c7b08e164f 100644 --- a/crypto/evp/p5_crpt2.c +++ b/crypto/evp/p5_crpt2.c @@ -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; diff --git a/test/evp_test.c b/test/evp_test.c index d924e3f6fc..f9dafec23a 100644 --- a/test/evp_test.c +++ b/test/evp_test.c @@ -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; diff --git a/test/evptests.txt b/test/evptests.txt index e9e796448a..a305b02e98 100644 --- a/test/evptests.txt +++ b/test/evptests.txt @@ -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 -- 2.34.1