From 4609fd87d7f3cbccf1ed6db257f01b18a1edcb55 Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Thu, 29 Oct 2015 11:52:18 +0100 Subject: [PATCH] Fix PBKDF2 iteration benchmark for longer key sizes. The previous PBKDF2 benchmark code did not take into account output key length. For SHA1 (with 160-bits output) and 256-bit keys (and longer) it means that the final value was higher than it should be. For other hash algorithms (like SHA256 or SHA512) it caused that iteration count was smaller (in comparison to SHA1) than expected for the requested time period. This patch fixes the code to use key size for the formatted device (or default LUKS key size if running in informational benchmark mode). Thanks to A.Visconti, S.Bossi, A.Calo and H.Ragab (http://www.club.di.unimi.it/) for point this out. (Based on "What users should know about Full Disk Encryption based on LUKS" paper to be presented on CANS2015). --- lib/crypto_backend/crypto_backend.h | 6 ++-- lib/crypto_backend/pbkdf_check.c | 44 ++++++++++++++++++++--------- lib/luks1/keymanage.c | 2 +- lib/utils_benchmark.c | 15 +++++++--- src/cryptsetup.c | 4 +-- 5 files changed, 47 insertions(+), 24 deletions(-) diff --git a/lib/crypto_backend/crypto_backend.h b/lib/crypto_backend/crypto_backend.h index 0aab38c9..b682665f 100644 --- a/lib/crypto_backend/crypto_backend.h +++ b/lib/crypto_backend/crypto_backend.h @@ -58,9 +58,9 @@ int crypt_backend_rng(char *buffer, size_t length, int quality, int fips); /* PBKDF*/ int crypt_pbkdf_check(const char *kdf, const char *hash, - const char *password, size_t password_size, - const char *salt, size_t salt_size, - uint64_t *iter_secs); + const char *password, size_t password_length, + const char *salt, size_t salt_length, + size_t key_length, uint64_t *iter_secs); int crypt_pbkdf(const char *kdf, const char *hash, const char *password, size_t password_length, const char *salt, size_t salt_length, diff --git a/lib/crypto_backend/pbkdf_check.c b/lib/crypto_backend/pbkdf_check.c index c6236cc6..f85bbfbe 100644 --- a/lib/crypto_backend/pbkdf_check.c +++ b/lib/crypto_backend/pbkdf_check.c @@ -18,6 +18,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include #include #include #include @@ -52,31 +53,39 @@ static long time_ms(struct rusage *start, struct rusage *end) /* This code benchmarks PBKDF and returns iterations/second using specified hash */ int crypt_pbkdf_check(const char *kdf, const char *hash, - const char *password, size_t password_size, - const char *salt, size_t salt_size, - uint64_t *iter_secs) + const char *password, size_t password_length, + const char *salt, size_t salt_length, + size_t key_length, uint64_t *iter_secs) { struct rusage rstart, rend; int r = 0, step = 0; long ms = 0; - char buf; + char *key = NULL; unsigned int iterations; - if (!kdf || !hash) + if (!kdf || !hash || key_length <= 0) return -EINVAL; + key = malloc(key_length); + if (!key) + return -ENOMEM; + iterations = 1 << 15; while (ms < 500) { - if (getrusage(RUSAGE_SELF, &rstart) < 0) - return -EINVAL; + if (getrusage(RUSAGE_SELF, &rstart) < 0) { + r = -EINVAL; + goto out; + } - r = crypt_pbkdf(kdf, hash, password, password_size, salt, - salt_size, &buf, 1, iterations); + r = crypt_pbkdf(kdf, hash, password, password_length, salt, + salt_length, key, key_length, iterations); if (r < 0) - return r; + goto out; - if (getrusage(RUSAGE_SELF, &rend) < 0) - return -EINVAL; + if (getrusage(RUSAGE_SELF, &rend) < 0) { + r = -EINVAL; + goto out; + } ms = time_ms(&rstart, &rend); if (ms > 500) @@ -91,11 +100,18 @@ int crypt_pbkdf_check(const char *kdf, const char *hash, else iterations <<= 1; - if (++step > 10 || !iterations) - return -EINVAL; + if (++step > 10 || !iterations) { + r = -EINVAL; + goto out; + } } if (iter_secs) *iter_secs = (iterations * 1000) / ms; +out: + if (key) { + crypt_backend_memzero(key, key_length); + free(key); + } return r; } diff --git a/lib/luks1/keymanage.c b/lib/luks1/keymanage.c index d1d16cff..44673c39 100644 --- a/lib/luks1/keymanage.c +++ b/lib/luks1/keymanage.c @@ -804,7 +804,7 @@ int LUKS_set_key(unsigned int keyIndex, * Avoid floating point operation * Final iteration count is at least LUKS_SLOT_ITERATIONS_MIN */ - PBKDF2_temp = (*PBKDF2_per_sec / 2) * (uint64_t)iteration_time_ms; + PBKDF2_temp = *PBKDF2_per_sec * (uint64_t)iteration_time_ms; PBKDF2_temp /= 1024; if (PBKDF2_temp > UINT32_MAX) PBKDF2_temp = UINT32_MAX; diff --git a/lib/utils_benchmark.c b/lib/utils_benchmark.c index 1e4469b3..86a81f95 100644 --- a/lib/utils_benchmark.c +++ b/lib/utils_benchmark.c @@ -240,7 +240,7 @@ int crypt_benchmark_kdf(struct crypt_device *cd, size_t salt_size, uint64_t *iterations_sec) { - int r; + int r, key_length = 0; if (!iterations_sec) return -EINVAL; @@ -249,14 +249,21 @@ int crypt_benchmark_kdf(struct crypt_device *cd, if (r < 0) return r; + // FIXME: this should be in KDF check API parameters later + if (cd) + key_length = crypt_get_volume_key_size(cd); + + if (key_length == 0) + key_length = DEFAULT_LUKS1_KEYBITS / 8; + if (!strncmp(kdf, "pbkdf2", 6)) r = crypt_pbkdf_check(kdf, hash, password, password_size, - salt, salt_size, iterations_sec); + salt, salt_size, key_length, iterations_sec); else r = -EINVAL; if (!r) - log_dbg("KDF %s, hash %s: %" PRIu64 " iterations per second.", - kdf, hash, *iterations_sec); + log_dbg("KDF %s, hash %s: %" PRIu64 " iterations per second (%d-bits key).", + kdf, hash, *iterations_sec, key_length * 8); return r; } diff --git a/src/cryptsetup.c b/src/cryptsetup.c index d523dc9a..b59b68b9 100644 --- a/src/cryptsetup.c +++ b/src/cryptsetup.c @@ -491,8 +491,8 @@ static int action_benchmark_kdf(const char *hash) if (r < 0) log_std("PBKDF2-%-9s N/A\n", hash); else - log_std("PBKDF2-%-9s %7" PRIu64 " iterations per second\n", - hash, kdf_iters); + log_std("PBKDF2-%-9s %7" PRIu64 " iterations per second for %d-bit key\n", + hash, kdf_iters, DEFAULT_LUKS1_KEYBITS); return r; }