crypto: use uint64_t for pbkdf iteration count parameters

The qcrypto_pbkdf_count_iters method uses a 64 bit int
but then checks its value against INT32_MAX before
returning it. This bounds check is premature, because
the calling code may well scale the iteration count
by some value. It is thus better to return a 64-bit
integer and let the caller do range checking.

For consistency the qcrypto_pbkdf method is also changed
to accept a 64bit int, though this is somewhat academic
since nettle is limited to taking an 'int' while gcrypt
is limited to taking a 'long int'.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrange 2016-09-12 12:50:12 +01:00
parent 0f2fa73ba0
commit 59b060be18
6 changed files with 57 additions and 40 deletions

View File

@ -917,6 +917,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
const char *hash_alg;
char *cipher_mode_spec = NULL;
QCryptoCipherAlgorithm ivcipheralg = 0;
uint64_t iters;
memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
if (!luks_opts.has_cipher_alg) {
@ -1064,12 +1065,11 @@ qcrypto_block_luks_create(QCryptoBlock *block,
/* Determine how many iterations we need to hash the master
* key, in order to have 1 second of compute time used
*/
luks->header.master_key_iterations =
qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
masterkey, luks->header.key_bytes,
luks->header.master_key_salt,
QCRYPTO_BLOCK_LUKS_SALT_LEN,
&local_err);
iters = qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
masterkey, luks->header.key_bytes,
luks->header.master_key_salt,
QCRYPTO_BLOCK_LUKS_SALT_LEN,
&local_err);
if (local_err) {
error_propagate(errp, local_err);
goto error;
@ -1079,11 +1079,15 @@ qcrypto_block_luks_create(QCryptoBlock *block,
* explanation why they chose /= 8... Probably so that
* if all 8 keyslots are active we only spend 1 second
* in total time to check all keys */
luks->header.master_key_iterations /= 8;
luks->header.master_key_iterations = MAX(
luks->header.master_key_iterations,
QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS);
iters /= 8;
if (iters > UINT32_MAX) {
error_setg_errno(errp, ERANGE,
"PBKDF iterations %llu larger than %u",
(unsigned long long)iters, UINT32_MAX);
goto error;
}
iters = MAX(iters, QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS);
luks->header.master_key_iterations = iters;
/* Hash the master key, saving the result in the LUKS
* header. This hash is used when opening the encrypted
@ -1131,22 +1135,28 @@ qcrypto_block_luks_create(QCryptoBlock *block,
/* Again we determine how many iterations are required to
* hash the user password while consuming 1 second of compute
* time */
luks->header.key_slots[0].iterations =
qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
(uint8_t *)password, strlen(password),
luks->header.key_slots[0].salt,
QCRYPTO_BLOCK_LUKS_SALT_LEN,
&local_err);
iters = qcrypto_pbkdf2_count_iters(luks_opts.hash_alg,
(uint8_t *)password, strlen(password),
luks->header.key_slots[0].salt,
QCRYPTO_BLOCK_LUKS_SALT_LEN,
&local_err);
if (local_err) {
error_propagate(errp, local_err);
goto error;
}
/* Why /= 2 ? That matches cryptsetup, but there's no
* explanation why they chose /= 2... */
luks->header.key_slots[0].iterations /= 2;
luks->header.key_slots[0].iterations = MAX(
luks->header.key_slots[0].iterations,
QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS);
iters /= 2;
if (iters > UINT32_MAX) {
error_setg_errno(errp, ERANGE,
"PBKDF iterations %llu larger than %u",
(unsigned long long)iters, UINT32_MAX);
goto error;
}
luks->header.key_slots[0].iterations =
MAX(iters, QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS);
/* Generate a key that we'll use to encrypt the master

View File

@ -38,7 +38,7 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash)
int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
const uint8_t *key, size_t nkey,
const uint8_t *salt, size_t nsalt,
unsigned int iterations,
uint64_t iterations,
uint8_t *out, size_t nout,
Error **errp)
{
@ -49,6 +49,13 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
};
int ret;
if (iterations > ULONG_MAX) {
error_setg_errno(errp, ERANGE,
"PBKDF iterations %llu must be less than %lu",
(long long unsigned)iterations, ULONG_MAX);
return -1;
}
if (hash >= G_N_ELEMENTS(hash_map) ||
hash_map[hash] == GCRY_MD_NONE) {
error_setg(errp, "Unexpected hash algorithm %d", hash);

View File

@ -38,10 +38,16 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash)
int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
const uint8_t *key, size_t nkey,
const uint8_t *salt, size_t nsalt,
unsigned int iterations,
uint64_t iterations,
uint8_t *out, size_t nout,
Error **errp)
{
if (iterations > UINT_MAX) {
error_setg_errno(errp, ERANGE,
"PBKDF iterations %llu must be less than %u",
(long long unsigned)iterations, UINT_MAX);
return -1;
}
switch (hash) {
case QCRYPTO_HASH_ALG_SHA1:
pbkdf2_hmac_sha1(nkey, key,

View File

@ -32,7 +32,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash G_GNUC_UNUSED,
size_t nkey G_GNUC_UNUSED,
const uint8_t *salt G_GNUC_UNUSED,
size_t nsalt G_GNUC_UNUSED,
unsigned int iterations G_GNUC_UNUSED,
uint64_t iterations G_GNUC_UNUSED,
uint8_t *out G_GNUC_UNUSED,
size_t nout G_GNUC_UNUSED,
Error **errp)

View File

@ -62,13 +62,13 @@ static int qcrypto_pbkdf2_get_thread_cpu(unsigned long long *val_ms,
#endif
}
int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
const uint8_t *key, size_t nkey,
const uint8_t *salt, size_t nsalt,
Error **errp)
uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
const uint8_t *key, size_t nkey,
const uint8_t *salt, size_t nsalt,
Error **errp)
{
uint8_t out[32];
long long int iterations = (1 << 15);
uint64_t iterations = (1 << 15);
unsigned long long delta_ms, start_ms, end_ms;
while (1) {
@ -100,11 +100,5 @@ int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
iterations = iterations * 1000 / delta_ms;
if (iterations > INT32_MAX) {
error_setg(errp, "Iterations %lld too large for a 32-bit int",
iterations);
return -1;
}
return iterations;
}

View File

@ -122,7 +122,7 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash);
int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
const uint8_t *key, size_t nkey,
const uint8_t *salt, size_t nsalt,
unsigned int iterations,
uint64_t iterations,
uint8_t *out, size_t nout,
Error **errp);
@ -144,9 +144,9 @@ int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
*
* Returns: number of iterations in 1 second, -1 on error
*/
int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
const uint8_t *key, size_t nkey,
const uint8_t *salt, size_t nsalt,
Error **errp);
uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
const uint8_t *key, size_t nkey,
const uint8_t *salt, size_t nsalt,
Error **errp);
#endif /* QCRYPTO_PBKDF_H */