kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Clean up PKCS5_PBKDF2_HMAC.

Browse Source 
This was a mess. HMAC_CTX_copy_ex would avoid having to cleanup and init
the HMAC_CTX repeatedly, but even that is unnecessary. hctx_tpl was just
to reuse the key. Instead, HMAC_CTX already can be reset with the same
key. (Alas, with a slightly odd API, but so it goes.) Do that, and use
goto err to cleanup the error-handling.

Thanks to upstream's b98530d6e09f4cb34c791b8840e936c1fc1467cf for
drawing attention to this. (Though we've diverged significantly from
upstream with all the heap-allocated bits, so I didn't use the change
itself.)

While I'm here, tidy up some variable names and cite the newer RFC.

Change-Id: Ic1259f46b7c5a14dc341b8cee385be5508ac4daf
Reviewed-on: https://boringssl-review.googlesource.com/14605
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
David Benjamin 2017-04-03 18:12:33 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent 8b487b73aa
commit 42329a828b
1 changed files with 54 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

115
crypto/evp/pbkdf.c
View File

@ -65,83 +65,76 @@
int PKCS5_PBKDF2_HMAC(const char *password, size_t password_len,
const uint8_t *salt, size_t salt_len, unsigned iterations,
const EVP_MD *digest, size_t key_len, uint8_t *out_key) {
uint8_t digest_tmp[EVP_MAX_MD_SIZE], *p, itmp[4];
size_t cplen, mdlen, tkeylen, k;
unsigned j;
/* See RFC 8018, section 5.2. */
int ret = 0;
size_t md_len = EVP_MD_size(digest);
uint32_t i = 1;
HMAC_CTX hctx_tpl, hctx;
HMAC_CTX hctx;
HMAC_CTX_init(&hctx);
mdlen = EVP_MD_size(digest);
HMAC_CTX_init(&hctx_tpl);
p = out_key;
tkeylen = key_len;
if (!HMAC_Init_ex(&hctx_tpl, password, password_len, digest, NULL)) {
HMAC_CTX_cleanup(&hctx_tpl);
return 0;
if (!HMAC_Init_ex(&hctx, password, password_len, digest, NULL)) {
goto err;
}
while (tkeylen) {
if (tkeylen > mdlen) {
cplen = mdlen;
} else {
cplen = tkeylen;
while (key_len > 0) {
size_t todo = md_len;
if (todo > key_len) {
todo = key_len;
}
/* We are unlikely to ever use more than 256 blocks (5120 bits!)
* but just in case... */
itmp[0] = (uint8_t)((i >> 24) & 0xff);
itmp[1] = (uint8_t)((i >> 16) & 0xff);
itmp[2] = (uint8_t)((i >> 8) & 0xff);
itmp[3] = (uint8_t)(i & 0xff);
if (!HMAC_CTX_copy(&hctx, &hctx_tpl)) {
HMAC_CTX_cleanup(&hctx_tpl);
return 0;
}
if (!HMAC_Update(&hctx, salt, salt_len) ||
!HMAC_Update(&hctx, itmp, 4) ||
uint8_t i_buf[4];
i_buf[0] = (uint8_t)((i >> 24) & 0xff);
i_buf[1] = (uint8_t)((i >> 16) & 0xff);
i_buf[2] = (uint8_t)((i >> 8) & 0xff);
i_buf[3] = (uint8_t)(i & 0xff);
/* Compute U_1. */
uint8_t digest_tmp[EVP_MAX_MD_SIZE];
if (!HMAC_Init_ex(&hctx, NULL, 0, NULL, NULL) ||
!HMAC_Update(&hctx, salt, salt_len) ||
!HMAC_Update(&hctx, i_buf, 4) ||
!HMAC_Final(&hctx, digest_tmp, NULL)) {
HMAC_CTX_cleanup(&hctx_tpl);
HMAC_CTX_cleanup(&hctx);
return 0;
goto err;
}
HMAC_CTX_cleanup(&hctx);
OPENSSL_memcpy(p, digest_tmp, cplen);
for (j = 1; j < iterations; j++) {
if (!HMAC_CTX_copy(&hctx, &hctx_tpl)) {
HMAC_CTX_cleanup(&hctx_tpl);
return 0;
}
if (!HMAC_Update(&hctx, digest_tmp, mdlen) ||
OPENSSL_memcpy(out_key, digest_tmp, todo);
for (unsigned j = 1; j < iterations; j++) {
/* Compute the remaining U_* values and XOR. */
if (!HMAC_Init_ex(&hctx, NULL, 0, NULL, NULL) ||
!HMAC_Update(&hctx, digest_tmp, md_len) ||
!HMAC_Final(&hctx, digest_tmp, NULL)) {
HMAC_CTX_cleanup(&hctx_tpl);
HMAC_CTX_cleanup(&hctx);
return 0;
goto err;
}
HMAC_CTX_cleanup(&hctx);
for (k = 0; k < cplen; k++) {
p[k] ^= digest_tmp[k];
for (size_t k = 0; k < todo; k++) {
out_key[k] ^= digest_tmp[k];
}
}
tkeylen -= cplen;
key_len -= todo;
out_key += todo;
i++;
p += cplen;
}
HMAC_CTX_cleanup(&hctx_tpl);
// RFC 2898 describes iterations (c) as being a "positive integer", so a
// value of 0 is an error.
//
// Unfortunatley not all consumers of PKCS5_PBKDF2_HMAC() check their return
// value, expecting it to succeed and unconditonally using |out_key|.
// As a precaution for such callsites in external code, the old behavior
// of iterations < 1 being treated as iterations == 1 is preserved, but
// additionally an error result is returned.
//
// TODO(eroman): Figure out how to remove this compatibility hack, or change
// the default to something more sensible like 2048.
/* RFC 8018 describes iterations (c) as being a "positive integer", so a
* value of 0 is an error.
*
* Unfortunately not all consumers of PKCS5_PBKDF2_HMAC() check their return
* value, expecting it to succeed and unconditionally using |out_key|. As a
* precaution for such callsites in external code, the old behavior of
* iterations < 1 being treated as iterations == 1 is preserved, but
* additionally an error result is returned.
*
* TODO(eroman): Figure out how to remove this compatibility hack, or change
* the default to something more sensible like 2048. */
if (iterations == 0) {
return 0;
goto err;
}
return 1;
ret = 1;
err:
HMAC_CTX_cleanup(&hctx);
return ret;
}
int PKCS5_PBKDF2_HMAC_SHA1(const char *password, size_t password_len,