[PATCH v1] luks1: fix pbkdf iteration estimation with sha1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



A previous commit attempted to fix the pbkdf iteration
estimation:

  commit 4609fd87d7f3cbccf1ed6db257f01b18a1edcb55
  Author: Milan Broz <gmazyland@xxxxxxxxx>
  Date:   Thu Oct 29 11:52:18 2015 +0100

    Fix PBKDF2 iteration benchmark for longer key sizes.

Before this commit, it would estimate using a derived
key size of 1. After this commit, it will estimate using
a derived key size matching the key bytes recorded in
the LUKS header.

Unfortuntely this is still incorrect as there are two
different places where luks estimates pbkdf2 iterations.
For the key slots, the derived key size needs to match
master key bytes, but for the master key digest, it must
match LUKS_DIGESTSIZE.

The relationship between the derived key size and the
pbkdf2 hash size is directly related to the time spent
in computation. For example, with aes, key size 256 and
sha1, we have

  key size = 32
  hash digest = 20

Since the sha1 hash has as a 20 byte digest size, when
doing estimation with a derived key size of 32, each
iteration invokes 2 sha1 operations. If we passed a derived
key size of 20, then each iteration invokes 1 sha1 operation.

Thus, by passing 32, instead of 20, when estimating the
master key digest iterations, we're getting a value of
iterations that is 50% too small. This only affects the
sha1 hash, and only when using key sizes greater than
160 bits, since all other hashes have a large enough
digest size that they'll only ever require 1 operation
per iteration.

A second, minor mistake is made when scaling the value
of iterations per second, to match the requested iteration
time. There are 1000 milliseconds in 1 second, but the
code is scaling by 1024, making the number of iterations
ever so slightly smaller than it should be. This isn't
a significant issue, but for clarity it is good to use
the correct scaling factor for ms -> secs.

As a final cleanup, the callers to estimate iterations
are also switched so that the key/salt parameters also
match the value that will be used in the real pbkdf
call later, instead of a fixed 3 character string.

Comparative data generated with

  $ for h in sha1 sha256 sha512
    do
      echo 123456 | \
        cryptsetup luksFormat --cipher aes \
	  --key-size 256  --hash $h -q /dev/loop1
    done

Before this commit:

    master key iters: 375750
    key slot 1 iters: 3020648

    master key iters: 396250
    key slot 1 iters: 3141103

    master key iters: 312750
    key slot 1 iters: 2473429

After this commit:

    master key iters: 776500
    key slot 1 iters: 3052622

    master key iters: 398250
    key slot 1 iters: 3206654

    master key iters: 314500
    key slot 1 iters: 2514570

Notice only the master key iters changed. Although
it looks wierd that sha1 requires less iterations
than sha256 for key slot 1, this is expected because
each iteration is doing 2 * sha1 ops, but only
1 * sha256 ops causing sha1 to be more expensive
on aggregate.

Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
---
 lib/luks1/keymanage.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/lib/luks1/keymanage.c b/lib/luks1/keymanage.c
index 4a9cbd6..f1682c1 100644
--- a/lib/luks1/keymanage.c
+++ b/lib/luks1/keymanage.c
@@ -707,8 +707,11 @@ int LUKS_generate_phdr(struct luks_phdr *header,
 		return r;
 	}
 
-	r = crypt_benchmark_kdf(ctx, "pbkdf2", header->hashSpec,
-				"foo", 3, "bar", 3, PBKDF2_per_sec);
+	r = crypt_pbkdf_check("pbkdf2", header->hashSpec,
+			      vk->key, vk->keylength,
+			      header->mkDigestSalt, LUKS_SALTSIZE,
+			      LUKS_DIGESTSIZE,
+			      PBKDF2_per_sec);
 	if (r < 0) {
 		log_err(ctx, _("Not compatible PBKDF2 options (using hash algorithm %s).\n"),
 			header->hashSpec);
@@ -717,7 +720,7 @@ int LUKS_generate_phdr(struct luks_phdr *header,
 
 	/* Compute master key digest */
 	iteration_time_ms /= 8;
-	header->mkDigestIterations = at_least((uint32_t)(*PBKDF2_per_sec/1024) * iteration_time_ms,
+	header->mkDigestIterations = at_least((uint32_t)(*PBKDF2_per_sec/1000) * iteration_time_ms,
 					      LUKS_MKD_ITERATIONS_MIN);
 
 	r = crypt_pbkdf("pbkdf2", header->hashSpec, vk->key,vk->keylength,
@@ -803,8 +806,11 @@ int LUKS_set_key(unsigned int keyIndex,
 
 	log_dbg("Calculating data for key slot %d", keyIndex);
 
-	r = crypt_benchmark_kdf(ctx, "pbkdf2", hdr->hashSpec,
-				"foo", 3, "bar", 3, PBKDF2_per_sec);
+	r = crypt_pbkdf_check("pbkdf2", hdr->hashSpec,
+			      password, passwordLen,
+			      hdr->keyblock[keyIndex].passwordSalt, LUKS_SALTSIZE,
+			      hdr->keyBytes,
+			      PBKDF2_per_sec);
 	if (r < 0) {
 		log_err(ctx, _("Not compatible PBKDF2 options (using hash algorithm %s).\n"),
 			hdr->hashSpec);
@@ -816,7 +822,7 @@ int LUKS_set_key(unsigned int keyIndex,
 	 * Final iteration count is at least LUKS_SLOT_ITERATIONS_MIN
 	 */
 	PBKDF2_temp = *PBKDF2_per_sec * (uint64_t)iteration_time_ms;
-	PBKDF2_temp /= 1024;
+	PBKDF2_temp /= 1000;
 	if (PBKDF2_temp > UINT32_MAX)
 		PBKDF2_temp = UINT32_MAX;
 	hdr->keyblock[keyIndex].passwordIterations = at_least((uint32_t)PBKDF2_temp,
-- 
2.7.4

_______________________________________________
dm-crypt mailing list
dm-crypt@xxxxxxxx
http://www.saout.de/mailman/listinfo/dm-crypt



[Index of Archives]     [Device Mapper Devel]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]     [Fedora Docs]

  Powered by Linux