[PATCH 1/2] dm-crypt: Properly handle extra key string in initialization

[PATCH 1/2] dm-crypt: Properly handle extra key string in initialization

[Milan Broz]

Some encryption modes use extra keys (e.g. loopAES has IV seed)
which are not used in block cipher initialization but are part
of key string in table constructor.

Patch adds additional field which described lengh of this extra
keys and substracts it before real key encryption setting.

Because extra keys are calculated during IV mode setting,
key initialization is moved after this step.

For now, this change has no effect to supported modes
(thanks to ilog2 rounding) but is required by following patch.

Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
 drivers/md/dm-crypt.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 0fce0bc..878bda7 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -171,7 +171,8 @@ struct crypt_config {
 
 	unsigned long flags;
 	unsigned int key_size;
-	unsigned int key_parts;
+	unsigned int key_parts;     /* independent parts in key buffer */
+	unsigned int key_extra_size;/* additional keys length */
 	u8 key[0];
 };
 
@@ -1274,9 +1275,12 @@ static int crypt_alloc_tfms(struct crypt_config *cc, char *ciphermode)
 
 static int crypt_setkey_allcpus(struct crypt_config *cc)
 {
-	unsigned subkey_size = cc->key_size >> ilog2(cc->tfms_count);
+	unsigned subkey_size;
 	int err = 0, i, r;
 
+	/* Ignore extra keys (which are used for IV etc) */
+	subkey_size = (cc->key_size - cc->key_extra_size) >> ilog2(cc->tfms_count);
+
 	for (i = 0; i < cc->tfms_count; i++) {
 		r = crypto_ablkcipher_setkey(cc->tfms[i],
 					     cc->key + (i * subkey_size),
@@ -1409,6 +1413,7 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 		return -EINVAL;
 	}
 	cc->key_parts = cc->tfms_count;
+	cc->key_extra_size = 0;
 
 	cc->cipher = kstrdup(cipher, GFP_KERNEL);
 	if (!cc->cipher)
@@ -1460,13 +1465,6 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 		goto bad;
 	}
 
-	/* Initialize and set key */
-	ret = crypt_set_key(cc, key);
-	if (ret < 0) {
-		ti->error = "Error decoding and setting key";
-		goto bad;
-	}
-
 	/* Initialize IV */
 	cc->iv_size = crypto_ablkcipher_ivsize(any_tfm(cc));
 	if (cc->iv_size)
@@ -1497,14 +1495,23 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 		 * to length of provided multi-key string.
 		 * If present (version 3), last key is used as IV seed.
 		 */
-		if (cc->key_size % cc->key_parts)
+		if (cc->key_size % cc->key_parts) {
 			cc->key_parts++;
+			cc->key_extra_size = cc->key_size / cc->key_parts;
+		}
 	} else {
 		ret = -EINVAL;
 		ti->error = "Invalid IV mode";
 		goto bad;
 	}
 
+	/* Initialize and set key */
+	ret = crypt_set_key(cc, key);
+	if (ret < 0) {
+		ti->error = "Error decoding and setting key";
+		goto bad;
+	}
+
 	/* Allocate IV */
 	if (cc->iv_gen_ops && cc->iv_gen_ops->ctr) {
 		ret = cc->iv_gen_ops->ctr(cc, ti, ivopts);
-- 
1.8.4.rc3

[PATCH 2/2] dm-crypt: Add TCW IV mode for old CBC TCRYPT containers.

[Milan Broz]

The dmcrypt already can activate TCRYPT (TrueCrypt compatible) containers
in LRW or XTS block encryption mode.

TCRYPT containers prior to version 4.1 used CBC mode with some
additional tweaks.

This patch adds support for these containers.

For now, there is no support for chained ciphers, this TCW mode support
only containers encrypted with one cipher
(Tested with AES, Twofish, Serpentm CAST5 and TripleDES).

While TCRYPT CBC mode is legacy and is known to be vulnerable
to some watermarking attacks (e.g. revealing of hidden disk
existence) it can be still useful to mount old containers
without using 3rd party software or for independent forensic
analysis of such containers.

(Both userspace and kernel code is independent implementation
based on format documentation and completely avoids use of original
source code.)

Encryption uses CBC mode with special IV generated from
additional key, xored with sector number.

There is also second key used for "whitening" of sectors.
Whitening key is xored with sector number and mixed using
CRC32 and resulting value is applied to whole sector.
(Detailed calculation is in Truecrypt documentation for version < 4.1
and will be also described on dmcrypt site.)

IV and whitening key is concatenated with encryption key,
so kernel receives all these keys as K|IV_key|Whitening_key
in one string.
Length of IV key is always the same as IV of selected cipher
and length of whitening key is fixed to TCW_WHITENING_SIZE,
so key string can be split properly.

The experimental support for activation of these containers
is already present in git devel brach of cryptsetup.

Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
 drivers/md/dm-crypt.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 177 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 878bda7..0b3923d 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2,6 +2,7 @@
  * Copyright (C) 2003 Christophe Saout <christophe@saout.de>
  * Copyright (C) 2004 Clemens Fruhwirth <clemens@endorphin.org>
  * Copyright (C) 2006-2009 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2013 Milan Broz <gmazyland@gmail.com>
  *
  * This file is released under the GPL.
  */
@@ -98,6 +99,13 @@ struct iv_lmk_private {
 	u8 *seed;
 };
 
+#define TCW_WHITENING_SIZE 16
+struct iv_tcw_private {
+	struct crypto_shash *crc32_tfm;
+	u8 *iv_seed;
+	u8 *whitening;
+};
+
 /*
  * Crypt: maps a linear range of a block device
  * and encrypts / decrypts at the same time.
@@ -139,6 +147,7 @@ struct crypt_config {
 		struct iv_essiv_private essiv;
 		struct iv_benbi_private benbi;
 		struct iv_lmk_private lmk;
+		struct iv_tcw_private tcw;
 	} iv_gen_private;
 	sector_t iv_offset;
 	unsigned int iv_size;
@@ -231,6 +240,16 @@ static struct crypto_ablkcipher *any_tfm(struct crypt_config *cc)
  *         version 3: the same as version 2 with additional IV seed
  *                   (it uses 65 keys, last key is used as IV seed)
  *
+ * tcw:  Compatible implementation of the block chaining mode used
+ *       by the TrueCrypt device encryption system (prior to version 4.1).
+ *       For more info see http://www.truecrypt.org.
+ *       It operates on full 512 byte sectors and uses CBC
+ *       with an IV derived from initial key and the sector number.
+ *       In addition, whitening value is applied on every sector, whitening
+ *       is calculated from initial key, sector number and mixed using CRC32.
+ *       Note that this encryption scheme is vulnerable to watermarking attacks
+ *       and should be used for old compatible containers access only.
+ *
  * plumb: unimplemented, see:
  * http://article.gmane.org/gmane.linux.kernel.device-mapper.dm-crypt/454
  */
@@ -609,6 +628,150 @@ static int crypt_iv_lmk_post(struct crypt_config *cc, u8 *iv,
 	return r;
 }
 
+static void crypt_iv_tcw_dtr(struct crypt_config *cc)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+
+	kzfree(tcw->iv_seed);
+	tcw->iv_seed = NULL;
+	kzfree(tcw->whitening);
+	tcw->whitening = NULL;
+
+	if (tcw->crc32_tfm && !IS_ERR(tcw->crc32_tfm))
+		crypto_free_shash(tcw->crc32_tfm);
+	tcw->crc32_tfm = NULL;
+}
+
+static int crypt_iv_tcw_ctr(struct crypt_config *cc, struct dm_target *ti,
+			    const char *opts)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+
+	if (cc->key_size <= (cc->iv_size + TCW_WHITENING_SIZE)) {
+		ti->error = "Wrong key size for TCW";
+		return -EINVAL;
+	}
+
+	tcw->crc32_tfm = crypto_alloc_shash("crc32", 0, 0);
+	if (IS_ERR(tcw->crc32_tfm)) {
+		ti->error = "Error initializing CRC32 in TCW";
+		return PTR_ERR(tcw->crc32_tfm);
+	}
+
+	tcw->iv_seed = kzalloc(cc->iv_size, GFP_KERNEL);
+	tcw->whitening = kzalloc(TCW_WHITENING_SIZE, GFP_KERNEL);
+	if (!tcw->iv_seed || !tcw->whitening) {
+		crypt_iv_tcw_dtr(cc);
+		ti->error = "Error kmallocing seed storage in TCW";
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int crypt_iv_tcw_init(struct crypt_config *cc)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+	int key_offset = cc->key_size - cc->iv_size - TCW_WHITENING_SIZE;
+
+	memcpy(tcw->iv_seed, &cc->key[key_offset], cc->iv_size);
+	memcpy(tcw->whitening, &cc->key[key_offset + cc->iv_size],
+	       TCW_WHITENING_SIZE);
+
+	return 0;
+}
+
+static int crypt_iv_tcw_wipe(struct crypt_config *cc)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+
+	memset(tcw->iv_seed, 0, cc->iv_size);
+	memset(tcw->whitening, 0, TCW_WHITENING_SIZE);
+
+	return 0;
+}
+
+static int crypt_iv_tcw_whitening(struct crypt_config *cc,
+				  struct dm_crypt_request *dmreq,
+				  u8 *data)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+	u64 sector = cpu_to_le64((u64)dmreq->iv_sector);
+	u8 buf[TCW_WHITENING_SIZE];
+	struct {
+		struct shash_desc desc;
+		char ctx[crypto_shash_descsize(tcw->crc32_tfm)];
+	} sdesc;
+	int i, r;
+
+	/* xor whitening with sector number */
+	memcpy(buf, tcw->whitening, TCW_WHITENING_SIZE);
+	crypto_xor(buf, (u8*)&sector, 8);
+	crypto_xor(&buf[8], (u8*)&sector, 8);
+
+	/* calculate crc32 for every 32bit part and xor it */
+	sdesc.desc.tfm = tcw->crc32_tfm;
+	sdesc.desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+	for (i = 0; i < 4; i++) {
+		if ((r = crypto_shash_init(&sdesc.desc)))
+			goto out;
+		if ((r = crypto_shash_update(&sdesc.desc, &buf[i * 4], 4)))
+			goto out;
+		if ((r = crypto_shash_final(&sdesc.desc, &buf[i * 4])))
+			goto out;
+	}
+	crypto_xor(&buf[0], &buf[12], 4);
+	crypto_xor(&buf[4], &buf[8], 4);
+
+	/* apply whitening (8 bytes) to whole sector */
+	for (i = 0; i < ((1 << SECTOR_SHIFT) / 8); i++)
+		crypto_xor(data + i * 8, buf, 8);
+out:
+	memset(buf, 0, sizeof(buf));
+	return r;
+}
+
+static int crypt_iv_tcw_gen(struct crypt_config *cc, u8 *iv,
+			    struct dm_crypt_request *dmreq)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+	u64 sector = cpu_to_le64((u64)dmreq->iv_sector);
+	u8 *src;
+	int r = 0;
+
+	/* Remove whitening from ciphertext */
+	if (bio_data_dir(dmreq->ctx->bio_in) != WRITE) {
+		src = kmap_atomic(sg_page(&dmreq->sg_in));
+		r = crypt_iv_tcw_whitening(cc, dmreq, src + dmreq->sg_in.offset);
+		kunmap_atomic(src);
+	}
+
+	/* Calculate IV */
+	memcpy(iv, tcw->iv_seed, cc->iv_size);
+	crypto_xor(iv, (u8*)&sector, 8);
+	if (cc->iv_size > 8)
+		crypto_xor(&iv[8], (u8*)&sector, cc->iv_size - 8);
+
+	return r;
+}
+
+static int crypt_iv_tcw_post(struct crypt_config *cc, u8 *iv,
+			     struct dm_crypt_request *dmreq)
+{
+	u8 *dst;
+	int r;
+
+	if (bio_data_dir(dmreq->ctx->bio_in) != WRITE)
+		return 0;
+
+	/* Apply whitening on ciphertext */
+	dst = kmap_atomic(sg_page(&dmreq->sg_out));
+	r = crypt_iv_tcw_whitening(cc, dmreq, dst + dmreq->sg_out.offset);
+	kunmap_atomic(dst);
+
+	return r;
+}
+
 static struct crypt_iv_operations crypt_iv_plain_ops = {
 	.generator = crypt_iv_plain_gen
 };
@@ -644,6 +807,15 @@ static struct crypt_iv_operations crypt_iv_lmk_ops = {
 	.post	   = crypt_iv_lmk_post
 };
 
+static struct crypt_iv_operations crypt_iv_tcw_ops = {
+	.ctr	   = crypt_iv_tcw_ctr,
+	.dtr	   = crypt_iv_tcw_dtr,
+	.init	   = crypt_iv_tcw_init,
+	.wipe	   = crypt_iv_tcw_wipe,
+	.generator = crypt_iv_tcw_gen,
+	.post	   = crypt_iv_tcw_post
+};
+
 static void crypt_convert_init(struct crypt_config *cc,
 			       struct convert_context *ctx,
 			       struct bio *bio_out, struct bio *bio_in,
@@ -1499,6 +1671,10 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 			cc->key_parts++;
 			cc->key_extra_size = cc->key_size / cc->key_parts;
 		}
+	} else if (strcmp(ivmode, "tcw") == 0) {
+		cc->iv_gen_ops = &crypt_iv_tcw_ops;
+		cc->key_parts += 2; /* IV + whitening */
+		cc->key_extra_size = cc->iv_size + TCW_WHITENING_SIZE;
 	} else {
 		ret = -EINVAL;
 		ti->error = "Invalid IV mode";
@@ -1824,7 +2000,7 @@ static int crypt_iterate_devices(struct dm_target *ti,
 
 static struct target_type crypt_target = {
 	.name   = "crypt",
-	.version = {1, 12, 1},
+	.version = {1, 13, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,
-- 
1.8.4.rc3

Re: [PATCH 1/2] dm-crypt: Properly handle extra key string in initialization

[Mike Snitzer]

On Sun, Oct 20 2013 at  9:16am -0400,
Milan Broz <gmazyland@gmail.com> wrote:

> Some encryption modes use extra keys (e.g. loopAES has IV seed)
> which are not used in block cipher initialization but are part
> of key string in table constructor.
> 
> Patch adds additional field which described lengh of this extra
> keys and substracts it before real key encryption setting.

typo, s/lengh/length/

But that aside, an example of the extra keys use with a theoretical
example via ctr input -- documented in patch header -- would be
helpful.  As it stands the code is doing some unituitive things (see
below).

> Because extra keys are calculated during IV mode setting,
> key initialization is moved after this step.
> 
> For now, this change has no effect to supported modes
> (thanks to ilog2 rounding) but is required by following patch.
> 
> Signed-off-by: Milan Broz <gmazyland@gmail.com>
> ---
>  drivers/md/dm-crypt.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 0fce0bc..878bda7 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -1497,14 +1495,23 @@ static int crypt_ctr_cipher(struct dm_target *ti,
>  		 * to length of provided multi-key string.
>  		 * If present (version 3), last key is used as IV seed.
>  		 */
> -		if (cc->key_size % cc->key_parts)
> +		if (cc->key_size % cc->key_parts) {
>  			cc->key_parts++;
> +			cc->key_extra_size = cc->key_size / cc->key_parts;
> +		}

This is leveraging existing heuristics of bumping key_parts and then
using it to establish 'key_extra_size' -- but the definition of "extra
size" is eluding me.  Say key_size=101, kepyparts=10 -- remainder is 1.
So then key_extra_size = 9.

All a bit opaque to me without a ctr usage example to help document in
the patch header.

Re: [PATCH 2/2] dm-crypt: Add TCW IV mode for old CBC TCRYPT containers.

[Mike Snitzer]

The following patch header is confusing given the mix of legacy and
new concepts.  I understand you're trying to establish context for
what's new but to this reader I'm a bit lost in the jargon.  Specifics
below (and please forgive my naive questions).

On Sun, Oct 20 2013 at  9:16am -0400,
Milan Broz <gmazyland@gmail.com> wrote:

> The dmcrypt already can activate TCRYPT (TrueCrypt compatible) containers
> in LRW or XTS block encryption mode.
> 
> TCRYPT containers prior to version 4.1 used CBC mode with some
> additional tweaks.
> 
> This patch adds support for these containers.
> 
> For now, there is no support for chained ciphers, this TCW mode support
> only containers encrypted with one cipher
> (Tested with AES, Twofish, Serpentm CAST5 and TripleDES).

What does TCW mean?  How does it relate to CBC?
- Is TCW mode: "CBC mode with some additional tweaks"?

> While TCRYPT CBC mode is legacy and is known to be vulnerable
> to some watermarking attacks (e.g. revealing of hidden disk
> existence) it can be still useful to mount old containers
> without using 3rd party software or for independent forensic
> analysis of such containers.

Now you're switching back to referring to "TCRYPT CBC mode".  What
happened to "TCW mode"?

> (Both userspace and kernel code is independent implementation
> based on format documentation and completely avoids use of original
> source code.)
> 
> Encryption uses CBC mode with special IV generated from
> additional key, xored with sector number.
> 
> There is also second key used for "whitening" of sectors.
> Whitening key is xored with sector number and mixed using
> CRC32 and resulting value is applied to whole sector.
> (Detailed calculation is in Truecrypt documentation for version < 4.1
> and will be also described on dmcrypt site.)

Can you add a pointer to the Truecrypt documentation for < 4.1?  Or a
pointer to the dmcrypt site documentation?

> IV and whitening key is concatenated with encryption key,
> so kernel receives all these keys as K|IV_key|Whitening_key
> in one string.
> Length of IV key is always the same as IV of selected cipher
> and length of whitening key is fixed to TCW_WHITENING_SIZE,
> so key string can be split properly.
> 
> The experimental support for activation of these containers
> is already present in git devel brach of cryptsetup.

Again, an example that documents a theoretical ctr line (before and
after patch?) would probably go a long way to help clarify what is new
here.

Re: [PATCH 1/2] dm-crypt: Properly handle extra key string in initialization

[Milan Broz]

On 28.10.2013 16:44, Mike Snitzer wrote:
> On Sun, Oct 20 2013 at  9:16am -0400,
> Milan Broz <gmazyland@gmail.com> wrote:

>> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
>> index 0fce0bc..878bda7 100644
>> --- a/drivers/md/dm-crypt.c
>> +++ b/drivers/md/dm-crypt.c
>> @@ -1497,14 +1495,23 @@ static int crypt_ctr_cipher(struct dm_target *ti,
>>   		 * to length of provided multi-key string.
>>   		 * If present (version 3), last key is used as IV seed.
>>   		 */
>> -		if (cc->key_size % cc->key_parts)
>> +		if (cc->key_size % cc->key_parts) {
>>   			cc->key_parts++;
>> +			cc->key_extra_size = cc->key_size / cc->key_parts;
>> +		}
>
> This is leveraging existing heuristics of bumping key_parts and then
> using it to establish 'key_extra_size' -- but the definition of "extra
> size" is eluding me.  Say key_size=101, kepyparts=10 -- remainder is 1.
> So then key_extra_size = 9.
>
> All a bit opaque to me without a ctr usage example to help document in
> the patch header.

All this is because the table format was not designed to include more
independent keys...
(And changing it now will cause more incompatibility problems it solves.)


Anyway, if this helps:

"TCW" is just a shortcut to identify this mode,
(TrueCrypt with whitening if you want)

It uses is normal CBC mode (note cbc in cipher definition)
but with additional operation (whitening) and with key seeded
Initial vector. These additional operations are implemented
in "dmcrypt tcw iv generator".

It requires 3 independent keys:

K - is the normal encryption key, size depends on algorithm
(this is what you see for "normal" dmcrypt mapping)

Kiv - is the key used to seed Initial vector generator,
size is always the same as IV size of algorithm above
(so it is variable)

Kw - is the key used to seed whitening (and additional operation)
size is always 16 bytes (fixed)

I am partially abusing IV generator in dmcrypt for other
work (whitening) (but the same way as we already have in loopAES format).

In fact, whitening is not related to IV at all but this is the simplest
way hot to implement it.

(And again, it is very similar to loopAES handling which
uses multiple keys already.)

Example:

For activation used by cryptsetup
# cryptsetup tcryptOpen ../tc-cbc/tc-cbc-serpent.img tc

we have this table (split to parts)

# dmsetup table --showkeys tc

0 16383 crypt serpent-cbc-tcw \

34f95b96abff946b64f1339ff8653cc77c38697c93b797a496f3786e86eed778 \
# ^^^ this is serpent cipher encryption key K

1850d5112bbae17d209b8310a8f3a034 \
# ^^^ this is the seed for IV, Kiv
  
f1cd297667bc0cd1438fad28d87ef6a1 \
# ^^^ this is the seed for whitening, Kw

1 7:0 1


IOW, for tcw constructor the format of key is simply

K | Kiv | Kw

and key_parts and key_extra_size is just helper how to parse
it properly.

So, key parts = 3 in this case (we have 3 keys - K, Kiv, Kw).

key_extra_size is length of key string which contains additional
keys sizeof(Kiv + Kw) which are not used for encryption algorithm
but are processed elsewhere (in iv generator ctr).


Example for existing loopAES ("lmk IV generator") key is e.g.

K1...K64 | Kiv

(first 64 keys are for 64 independent tcms, Kiv is additional
Key used to seed IV. Thus key_parts = 65, key_extra_size = sizeof(Kiv)
because all keys including Kiv are always of the same size, code
can simply do cc->key_extra_size = cc->key_size / cc->key_parts)


In short, key_parts should now contain number of independent keys
in dmcrypt key mapping table string.

key_extra_size should contain length of the key which must
be cut off when setting core cipher key because this part is
processed elsewhere.

Milan

Re: [PATCH 2/2] dm-crypt: Add TCW IV mode for old CBC TCRYPT containers.

[Milan Broz]

On 28.10.2013 17:08, Mike Snitzer wrote:
> What does TCW mean?  How does it relate to CBC?
> - Is TCW mode: "CBC mode with some additional tweaks"?

Yes, as said in previous mail, it is just shortcut.

>
>> While TCRYPT CBC mode is legacy and is known to be vulnerable
>> to some watermarking attacks (e.g. revealing of hidden disk
>> existence) it can be still useful to mount old containers
>> without using 3rd party software or for independent forensic
>> analysis of such containers.
>
> Now you're switching back to referring to "TCRYPT CBC mode".  What
> happened to "TCW mode"?

I am talking about implementation in general, not dmcrypt specific one.
It is still the same mode of course.

>> There is also second key used for "whitening" of sectors.
>> Whitening key is xored with sector number and mixed using
>> CRC32 and resulting value is applied to whole sector.
>> (Detailed calculation is in Truecrypt documentation for version < 4.1
>> and will be also described on dmcrypt site.)
>
> Can you add a pointer to the Truecrypt documentation for < 4.1?

I am afraid they removed all old documentation from site.
(but search google e.g. for truecrypt-3.1a-user-guide.pdf or
some similar version which use CBC mode, whitening and IV
generator is described there as well)

>  Or a pointer to the dmcrypt site documentation?

Description is not yet there (once it is in kernel I will add it)
but link is referenced even from kernel Documentation
http://code.google.com/p/cryptsetup/wiki/DMCrypt

(and yes, seems that IV generators need better description there)

Thanks,
Milan

[PATCH 1/2] dm-crypt: Properly handle extra key string in initialization

[Milan Broz]

Some encryption modes use extra keys (e.g. loopAES has IV seed)
which are not used in block cipher initialization but are part
of key string in table constructor.

Patch adds additional field which described lenght of this extra
keys and substracts it before real key encryption setting.

So the key_size always includes the size of provided key
in mapping table in bytes.

The key_parts descibes how many parts (usually keys) contains
the whole key buffer and key_extra_size contains size in bytes
of additional keys part (this number of bytes must be cut off
because is processed by IV generator).

| K1 | K2 | .... | K64 |      Kiv       |
|----------- key_size ----------------- |
|                      |-key_extra_size-|
|     [64 keys]           [1 key]       | => key_parts = 65

Example where key string contains main key K, whitening key
Kw and IV seed Kiv:

|     K       |   Kiv   |       Kw       |
|--------------- key_size ---------------|
|             |-----key_extra_size-------|
|  [1 key]    | [1 key] |     [1 key]    | => key_parts = 3

Because extra keys are calculated during IV mode setting,
key initialization is moved after this step.

For now, this change has no effect to supported modes
(thanks to ilog2 rounding) but is required by following patch.

Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
 drivers/md/dm-crypt.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 0fce0bc..878bda7 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -171,7 +171,8 @@ struct crypt_config {
 
 	unsigned long flags;
 	unsigned int key_size;
-	unsigned int key_parts;
+	unsigned int key_parts;     /* independent parts in key buffer */
+	unsigned int key_extra_size;/* additional keys length */
 	u8 key[0];
 };
 
@@ -1274,9 +1275,12 @@ static int crypt_alloc_tfms(struct crypt_config *cc, char *ciphermode)
 
 static int crypt_setkey_allcpus(struct crypt_config *cc)
 {
-	unsigned subkey_size = cc->key_size >> ilog2(cc->tfms_count);
+	unsigned subkey_size;
 	int err = 0, i, r;
 
+	/* Ignore extra keys (which are used for IV etc) */
+	subkey_size = (cc->key_size - cc->key_extra_size) >> ilog2(cc->tfms_count);
+
 	for (i = 0; i < cc->tfms_count; i++) {
 		r = crypto_ablkcipher_setkey(cc->tfms[i],
 					     cc->key + (i * subkey_size),
@@ -1409,6 +1413,7 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 		return -EINVAL;
 	}
 	cc->key_parts = cc->tfms_count;
+	cc->key_extra_size = 0;
 
 	cc->cipher = kstrdup(cipher, GFP_KERNEL);
 	if (!cc->cipher)
@@ -1460,13 +1465,6 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 		goto bad;
 	}
 
-	/* Initialize and set key */
-	ret = crypt_set_key(cc, key);
-	if (ret < 0) {
-		ti->error = "Error decoding and setting key";
-		goto bad;
-	}
-
 	/* Initialize IV */
 	cc->iv_size = crypto_ablkcipher_ivsize(any_tfm(cc));
 	if (cc->iv_size)
@@ -1497,14 +1495,23 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 		 * to length of provided multi-key string.
 		 * If present (version 3), last key is used as IV seed.
 		 */
-		if (cc->key_size % cc->key_parts)
+		if (cc->key_size % cc->key_parts) {
 			cc->key_parts++;
+			cc->key_extra_size = cc->key_size / cc->key_parts;
+		}
 	} else {
 		ret = -EINVAL;
 		ti->error = "Invalid IV mode";
 		goto bad;
 	}
 
+	/* Initialize and set key */
+	ret = crypt_set_key(cc, key);
+	if (ret < 0) {
+		ti->error = "Error decoding and setting key";
+		goto bad;
+	}
+
 	/* Allocate IV */
 	if (cc->iv_gen_ops && cc->iv_gen_ops->ctr) {
 		ret = cc->iv_gen_ops->ctr(cc, ti, ivopts);
-- 
1.8.4.rc3

[PATCH 2/2] dm-crypt: Add TCW IV mode for old CBC TCRYPT containers.

[Milan Broz]

The dmcrypt already can activate TCRYPT (TrueCrypt compatible)
containers in LRW or XTS block encryption mode.

TCRYPT containers prior to version 4.1 used CBC mode with some
additional tweaks.

This patch adds support for these containers.

The mode is implemented using special IV generator named TCW
(TrueCrypt IV with whitening).

TCW IV supports only containers encrypted with one cipher
(Tested with AES, Twofish, Serpent, CAST5 and TripleDES).

While this mode is legacy and is known to be vulnerable
to some watermarking attacks (e.g. revealing of hidden disk
existence) it can be still useful to mount old containers
without using 3rd party software or for independent forensic
analysis of such containers.

(Both userspace and kernel code is independent implementation
based on format documentation and completely avoids use of original
source code.)

The TCW IV generator uses two additional keys, Kw (whitening
seed, size is always 16 bytes - TCW_WHITENING_SIZE) and
Kiv (IV seed, size is always of the IV size of selected cipher).
These keys are concatenated to main encryption key in mapping table.

While whitening is completely independent from IV, it is
implemented inside IV generator for simplification.

Whitening value is always 16 bytes long and is calculated
per sector from provided Kw as initial seed, xored with
sector number and mixed with CRC32 algorithm.
Resulting value is xored with ciphertext sector content.

IV is calculated from provided Kiv as initial seed and
xored with sector number.

Detailed calculation is in Truecrypt documentation for version < 4.1
and will be also described on dmcrypt site
http://code.google.com/p/cryptsetup/wiki/DMCrypt

The experimental support for activation of these containers
is already present in git devel brach of cryptsetup.

Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
 Documentation/device-mapper/dm-crypt.txt |  11 +-
 drivers/md/dm-crypt.c                    | 178 ++++++++++++++++++++++++++++++-
 2 files changed, 186 insertions(+), 3 deletions(-)

diff --git a/Documentation/device-mapper/dm-crypt.txt b/Documentation/device-mapper/dm-crypt.txt
index 2c656ae..89cbf3e 100644
--- a/Documentation/device-mapper/dm-crypt.txt
+++ b/Documentation/device-mapper/dm-crypt.txt
@@ -4,12 +4,15 @@ dm-crypt
 Device-Mapper's "crypt" target provides transparent encryption of block devices
 using the kernel crypto API.
 
+For more detailed description of parameters see also DMCrypt page
+http://code.google.com/p/cryptsetup/wiki/DMCrypt
+
 Parameters: <cipher> <key> <iv_offset> <device path> \
 	      <offset> [<#opt_params> <opt_params>]
 
 <cipher>
     Encryption cipher and an optional IV generation mode.
-    (In format cipher[:keycount]-chainmode-ivopts:ivmode).
+    (In format cipher[:keycount]-chainmode-ivmode[:ivopts]).
     Examples:
        des
        aes-cbc-essiv:sha256
@@ -19,7 +22,11 @@ Parameters: <cipher> <key> <iv_offset> <device path> \
 
 <key>
     Key used for encryption. It is encoded as a hexadecimal number.
-    You can only use key sizes that are valid for the selected cipher.
+    You can only use key sizes that are valid for the selected cipher
+    in combination with selected iv mode.
+    Note that for some iv modes the key string can contain additional
+    keys (for example IV seed) so the key contains more parts contatenated
+    to one string.
 
 <keycount>
     Multi-key compatibility mode. You can define <keycount> keys and
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 878bda7..0b3923d 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2,6 +2,7 @@
  * Copyright (C) 2003 Christophe Saout <christophe@saout.de>
  * Copyright (C) 2004 Clemens Fruhwirth <clemens@endorphin.org>
  * Copyright (C) 2006-2009 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2013 Milan Broz <gmazyland@gmail.com>
  *
  * This file is released under the GPL.
  */
@@ -98,6 +99,13 @@ struct iv_lmk_private {
 	u8 *seed;
 };
 
+#define TCW_WHITENING_SIZE 16
+struct iv_tcw_private {
+	struct crypto_shash *crc32_tfm;
+	u8 *iv_seed;
+	u8 *whitening;
+};
+
 /*
  * Crypt: maps a linear range of a block device
  * and encrypts / decrypts at the same time.
@@ -139,6 +147,7 @@ struct crypt_config {
 		struct iv_essiv_private essiv;
 		struct iv_benbi_private benbi;
 		struct iv_lmk_private lmk;
+		struct iv_tcw_private tcw;
 	} iv_gen_private;
 	sector_t iv_offset;
 	unsigned int iv_size;
@@ -231,6 +240,16 @@ static struct crypto_ablkcipher *any_tfm(struct crypt_config *cc)
  *         version 3: the same as version 2 with additional IV seed
  *                   (it uses 65 keys, last key is used as IV seed)
  *
+ * tcw:  Compatible implementation of the block chaining mode used
+ *       by the TrueCrypt device encryption system (prior to version 4.1).
+ *       For more info see http://www.truecrypt.org.
+ *       It operates on full 512 byte sectors and uses CBC
+ *       with an IV derived from initial key and the sector number.
+ *       In addition, whitening value is applied on every sector, whitening
+ *       is calculated from initial key, sector number and mixed using CRC32.
+ *       Note that this encryption scheme is vulnerable to watermarking attacks
+ *       and should be used for old compatible containers access only.
+ *
  * plumb: unimplemented, see:
  * http://article.gmane.org/gmane.linux.kernel.device-mapper.dm-crypt/454
  */
@@ -609,6 +628,150 @@ static int crypt_iv_lmk_post(struct crypt_config *cc, u8 *iv,
 	return r;
 }
 
+static void crypt_iv_tcw_dtr(struct crypt_config *cc)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+
+	kzfree(tcw->iv_seed);
+	tcw->iv_seed = NULL;
+	kzfree(tcw->whitening);
+	tcw->whitening = NULL;
+
+	if (tcw->crc32_tfm && !IS_ERR(tcw->crc32_tfm))
+		crypto_free_shash(tcw->crc32_tfm);
+	tcw->crc32_tfm = NULL;
+}
+
+static int crypt_iv_tcw_ctr(struct crypt_config *cc, struct dm_target *ti,
+			    const char *opts)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+
+	if (cc->key_size <= (cc->iv_size + TCW_WHITENING_SIZE)) {
+		ti->error = "Wrong key size for TCW";
+		return -EINVAL;
+	}
+
+	tcw->crc32_tfm = crypto_alloc_shash("crc32", 0, 0);
+	if (IS_ERR(tcw->crc32_tfm)) {
+		ti->error = "Error initializing CRC32 in TCW";
+		return PTR_ERR(tcw->crc32_tfm);
+	}
+
+	tcw->iv_seed = kzalloc(cc->iv_size, GFP_KERNEL);
+	tcw->whitening = kzalloc(TCW_WHITENING_SIZE, GFP_KERNEL);
+	if (!tcw->iv_seed || !tcw->whitening) {
+		crypt_iv_tcw_dtr(cc);
+		ti->error = "Error kmallocing seed storage in TCW";
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int crypt_iv_tcw_init(struct crypt_config *cc)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+	int key_offset = cc->key_size - cc->iv_size - TCW_WHITENING_SIZE;
+
+	memcpy(tcw->iv_seed, &cc->key[key_offset], cc->iv_size);
+	memcpy(tcw->whitening, &cc->key[key_offset + cc->iv_size],
+	       TCW_WHITENING_SIZE);
+
+	return 0;
+}
+
+static int crypt_iv_tcw_wipe(struct crypt_config *cc)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+
+	memset(tcw->iv_seed, 0, cc->iv_size);
+	memset(tcw->whitening, 0, TCW_WHITENING_SIZE);
+
+	return 0;
+}
+
+static int crypt_iv_tcw_whitening(struct crypt_config *cc,
+				  struct dm_crypt_request *dmreq,
+				  u8 *data)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+	u64 sector = cpu_to_le64((u64)dmreq->iv_sector);
+	u8 buf[TCW_WHITENING_SIZE];
+	struct {
+		struct shash_desc desc;
+		char ctx[crypto_shash_descsize(tcw->crc32_tfm)];
+	} sdesc;
+	int i, r;
+
+	/* xor whitening with sector number */
+	memcpy(buf, tcw->whitening, TCW_WHITENING_SIZE);
+	crypto_xor(buf, (u8*)&sector, 8);
+	crypto_xor(&buf[8], (u8*)&sector, 8);
+
+	/* calculate crc32 for every 32bit part and xor it */
+	sdesc.desc.tfm = tcw->crc32_tfm;
+	sdesc.desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+	for (i = 0; i < 4; i++) {
+		if ((r = crypto_shash_init(&sdesc.desc)))
+			goto out;
+		if ((r = crypto_shash_update(&sdesc.desc, &buf[i * 4], 4)))
+			goto out;
+		if ((r = crypto_shash_final(&sdesc.desc, &buf[i * 4])))
+			goto out;
+	}
+	crypto_xor(&buf[0], &buf[12], 4);
+	crypto_xor(&buf[4], &buf[8], 4);
+
+	/* apply whitening (8 bytes) to whole sector */
+	for (i = 0; i < ((1 << SECTOR_SHIFT) / 8); i++)
+		crypto_xor(data + i * 8, buf, 8);
+out:
+	memset(buf, 0, sizeof(buf));
+	return r;
+}
+
+static int crypt_iv_tcw_gen(struct crypt_config *cc, u8 *iv,
+			    struct dm_crypt_request *dmreq)
+{
+	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
+	u64 sector = cpu_to_le64((u64)dmreq->iv_sector);
+	u8 *src;
+	int r = 0;
+
+	/* Remove whitening from ciphertext */
+	if (bio_data_dir(dmreq->ctx->bio_in) != WRITE) {
+		src = kmap_atomic(sg_page(&dmreq->sg_in));
+		r = crypt_iv_tcw_whitening(cc, dmreq, src + dmreq->sg_in.offset);
+		kunmap_atomic(src);
+	}
+
+	/* Calculate IV */
+	memcpy(iv, tcw->iv_seed, cc->iv_size);
+	crypto_xor(iv, (u8*)&sector, 8);
+	if (cc->iv_size > 8)
+		crypto_xor(&iv[8], (u8*)&sector, cc->iv_size - 8);
+
+	return r;
+}
+
+static int crypt_iv_tcw_post(struct crypt_config *cc, u8 *iv,
+			     struct dm_crypt_request *dmreq)
+{
+	u8 *dst;
+	int r;
+
+	if (bio_data_dir(dmreq->ctx->bio_in) != WRITE)
+		return 0;
+
+	/* Apply whitening on ciphertext */
+	dst = kmap_atomic(sg_page(&dmreq->sg_out));
+	r = crypt_iv_tcw_whitening(cc, dmreq, dst + dmreq->sg_out.offset);
+	kunmap_atomic(dst);
+
+	return r;
+}
+
 static struct crypt_iv_operations crypt_iv_plain_ops = {
 	.generator = crypt_iv_plain_gen
 };
@@ -644,6 +807,15 @@ static struct crypt_iv_operations crypt_iv_lmk_ops = {
 	.post	   = crypt_iv_lmk_post
 };
 
+static struct crypt_iv_operations crypt_iv_tcw_ops = {
+	.ctr	   = crypt_iv_tcw_ctr,
+	.dtr	   = crypt_iv_tcw_dtr,
+	.init	   = crypt_iv_tcw_init,
+	.wipe	   = crypt_iv_tcw_wipe,
+	.generator = crypt_iv_tcw_gen,
+	.post	   = crypt_iv_tcw_post
+};
+
 static void crypt_convert_init(struct crypt_config *cc,
 			       struct convert_context *ctx,
 			       struct bio *bio_out, struct bio *bio_in,
@@ -1499,6 +1671,10 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 			cc->key_parts++;
 			cc->key_extra_size = cc->key_size / cc->key_parts;
 		}
+	} else if (strcmp(ivmode, "tcw") == 0) {
+		cc->iv_gen_ops = &crypt_iv_tcw_ops;
+		cc->key_parts += 2; /* IV + whitening */
+		cc->key_extra_size = cc->iv_size + TCW_WHITENING_SIZE;
 	} else {
 		ret = -EINVAL;
 		ti->error = "Invalid IV mode";
@@ -1824,7 +2000,7 @@ static int crypt_iterate_devices(struct dm_target *ti,
 
 static struct target_type crypt_target = {
 	.name   = "crypt",
-	.version = {1, 12, 1},
+	.version = {1, 13, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,
-- 
1.8.4.rc3

Re: [PATCH 1/2] dm-crypt: Properly handle extra key string in initialization

[Mike Snitzer]

On Mon, Oct 28 2013 at  6:21pm -0400,
Milan Broz <gmazyland@gmail.com> wrote:

> Some encryption modes use extra keys (e.g. loopAES has IV seed)
> which are not used in block cipher initialization but are part
> of key string in table constructor.
> 
> Patch adds additional field which described lenght of this extra
> keys and substracts it before real key encryption setting.
> 
> So the key_size always includes the size of provided key
> in mapping table in bytes.
> 
> The key_parts descibes how many parts (usually keys) contains
> the whole key buffer and key_extra_size contains size in bytes
> of additional keys part (this number of bytes must be cut off
> because is processed by IV generator).
> 
> | K1 | K2 | .... | K64 |      Kiv       |
> |----------- key_size ----------------- |
> |                      |-key_extra_size-|
> |     [64 keys]           [1 key]       | => key_parts = 65
> 
> Example where key string contains main key K, whitening key
> Kw and IV seed Kiv:
> 
> |     K       |   Kiv   |       Kw       |
> |--------------- key_size ---------------|
> |             |-----key_extra_size-------|
> |  [1 key]    | [1 key] |     [1 key]    | => key_parts = 3
> 
> Because extra keys are calculated during IV mode setting,
> key initialization is moved after this step.
> 
> For now, this change has no effect to supported modes
> (thanks to ilog2 rounding) but is required by following patch.
> 
> Signed-off-by: Milan Broz <gmazyland@gmail.com>

I pushed this to linux-next (for v3.13), see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=60039e2b0474f4e6b66746b9e179ca384d3e5c96

I tweaked the header a little and some whitespace.

Re: [PATCH 2/2] dm-crypt: Add TCW IV mode for old CBC TCRYPT containers.

[Mike Snitzer]

On Mon, Oct 28 2013 at  6:21pm -0400,
Milan Broz <gmazyland@gmail.com> wrote:

> The dmcrypt already can activate TCRYPT (TrueCrypt compatible)
> containers in LRW or XTS block encryption mode.
> 
> TCRYPT containers prior to version 4.1 used CBC mode with some
> additional tweaks.
> 
> This patch adds support for these containers.
> 
> The mode is implemented using special IV generator named TCW
> (TrueCrypt IV with whitening).
> 
> TCW IV supports only containers encrypted with one cipher
> (Tested with AES, Twofish, Serpent, CAST5 and TripleDES).
> 
> While this mode is legacy and is known to be vulnerable
> to some watermarking attacks (e.g. revealing of hidden disk
> existence) it can be still useful to mount old containers
> without using 3rd party software or for independent forensic
> analysis of such containers.
> 
> (Both userspace and kernel code is independent implementation
> based on format documentation and completely avoids use of original
> source code.)
> 
> The TCW IV generator uses two additional keys, Kw (whitening
> seed, size is always 16 bytes - TCW_WHITENING_SIZE) and
> Kiv (IV seed, size is always of the IV size of selected cipher).
> These keys are concatenated to main encryption key in mapping table.
> 
> While whitening is completely independent from IV, it is
> implemented inside IV generator for simplification.
> 
> Whitening value is always 16 bytes long and is calculated
> per sector from provided Kw as initial seed, xored with
> sector number and mixed with CRC32 algorithm.
> Resulting value is xored with ciphertext sector content.
> 
> IV is calculated from provided Kiv as initial seed and
> xored with sector number.
> 
> Detailed calculation is in Truecrypt documentation for version < 4.1
> and will be also described on dmcrypt site
> http://code.google.com/p/cryptsetup/wiki/DMCrypt
> 
> The experimental support for activation of these containers
> is already present in git devel brach of cryptsetup.
> 
> Signed-off-by: Milan Broz <gmazyland@gmail.com>

I pushed this to linux-next (for v3.13), see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=8a478f032b40a28a66559a91095d0e0733194389

Tweaked the header and text in dm-crypt.txt and maybe a few other
comments.

Re: [PATCH 1/2] dm-crypt: Properly handle extra key string in initialization

[Alasdair G Kergon]

On Mon, Oct 28, 2013 at 11:21:03PM +0100, Milan Broz wrote:
> +			cc->key_extra_size = cc->key_size / cc->key_parts;

I don't understand the logic that leads to this assignment.

Are there some implicit constraints on the input?

Do all invalid inputs produce an appropriate error?

Is there a clearer way to set out the calculation and can it be annotated?

Can we rename key_extra_size to extrakey_size (like subkey_size)?

Is extrakey_size always less than subkey_size?

Alasdair

Re: [PATCH 1/2] dm-crypt: Properly handle extra key string in initialization

[Alasdair G Kergon]

On Mon, Oct 28, 2013 at 11:21:03PM +0100, Milan Broz wrote:
> Example where key string contains main key K, whitening key
> Kw and IV seed Kiv:
> 
> |     K       |   Kiv   |       Kw       |
> |--------------- key_size ---------------|
> |             |-----key_extra_size-------|
> |  [1 key]    | [1 key] |     [1 key]    | => key_parts = 3
 
Ah, part of the confusion is because this patch does not actually support 
this case yet, does it?  That's for the next patch!

Can we update this patch header accordingly?
Maybe even move those ASCII diagrams into the code as comments.

Alasdair

Re: [PATCH 2/2] dm-crypt: Add TCW IV mode for old CBC TCRYPT containers.

[Alasdair G Kergon]

On Mon, Oct 28, 2013 at 11:21:04PM +0100, Milan Broz wrote:
> +++ b/Documentation/device-mapper/dm-crypt.txt
> @@ -4,12 +4,15 @@ dm-crypt

> +	crypto_xor(buf, (u8*)&sector, 8);
                           ^
Please insert a single space in casts like this: (u8 *)

> +	for (i = 0; i < 4; i++) {
> +		if ((r = crypto_shash_init(&sdesc.desc)))
> +			goto out;

Please change these to this form:
		r = crypto_shash_init(&sdesc.desc));
		if (r)
			goto out;

Alasdair

Re: [PATCH 1/2] dm-crypt: Properly handle extra key string in initialization

[Milan Broz]

On 10/30/2013 03:48 AM, Alasdair G Kergon wrote:
> On Mon, Oct 28, 2013 at 11:21:03PM +0100, Milan Broz wrote:
>> +			cc->key_extra_size = cc->key_size / cc->key_parts;
> 
> I don't understand the logic that leads to this assignment.
> 
> Are there some implicit constraints on the input?

This part is only for loopAES (lmk IV generator). In this situation only
one configuration is used in real world which is processed by this code,
65 keys of the same size (64 cipher keys and one IV seed).

The extra part (IV seed key) is thus exactly size of one key.

In dmcrypt lmk is implemented more generic (not limited to 64 cipher keys)
but there is still requirement that all keys are the same length.

(If there is no comment in code mentioning the same size requirement,
it should be here, will add it.)

> 
> Do all invalid inputs produce an appropriate error?

I hope it should.
If the key is correct size, cipher is initialised. If not, it returns error.

If you send garbage and it is of correct length, it is just garbage-in, garbage out.

N.B the problem with dmcrypt table is that some parameters of cipher
are just derived from key string length (e.g. AES-128/192/256).
If user want AES128 but send doubled key, we have no way to detect it...
 
> Is there a clearer way to set out the calculation and can it be annotated?
Dunno. I think comment mentioned above is the best way.

> Can we rename key_extra_size to extrakey_size (like subkey_size)?

I am not sure it is good idea. key_extra_size is related to key_size,
that's why it has this name...

> Is extrakey_size always less than subkey_size?

If you mean subkey in crypt_setkey_allcpus(), no, in principle
they are unrelated.

(subkey size is key size for main encryption, extra key can contain
whatever is needed for initializing IV generator)

For example (very old TrueCrypt container)
3DES cipher, which has K of size 24 bytes (3 * 64bits)

this is table:
0 16383 crypt des3_ede-cbc-tcw 0123f1b2c40ab6240f6defacf46ccb5804d4cf6ed8521b845017b3311a3e120328dc5edc44eb35566cb73f5233738aac 1 7:0 1

Here: key_size is 48, key_parts = 3, extra_key_size = 24.

So cipher subkey, IOW the 3DES key, is 48 - 24 = 24.

And in CW "IV generator" is extra key decoded:
Kw is always 16 bytes, thus Kiv = 24 - 16 = 8 bytes.
(Which is correct, because 3DES has block size 64bits = 8 bytes
and this match IV)

Thanks,
Milan
p.s.
The other changes in following ... no problem, just code style nitpicking ;-)
I will change it and resend once will have time to do it.

[PATCH 3/4] dm-crypt: Fix code formatting to make agk happy

[Milan Broz]

Fix code style format.

Add note that loopAES key must be the same size.
(probably should be folded into previous patch)

Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
 drivers/md/dm-crypt.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index cfdaa12..2d6ee75f 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -706,18 +706,21 @@ static int crypt_iv_tcw_whitening(struct crypt_config *cc,
 
 	/* xor whitening with sector number */
 	memcpy(buf, tcw->whitening, TCW_WHITENING_SIZE);
-	crypto_xor(buf, (u8*)&sector, 8);
-	crypto_xor(&buf[8], (u8*)&sector, 8);
+	crypto_xor(buf, (u8 *)&sector, 8);
+	crypto_xor(&buf[8], (u8 *)&sector, 8);
 
 	/* calculate crc32 for every 32bit part and xor it */
 	sdesc.desc.tfm = tcw->crc32_tfm;
 	sdesc.desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 	for (i = 0; i < 4; i++) {
-		if ((r = crypto_shash_init(&sdesc.desc)))
+		r = crypto_shash_init(&sdesc.desc);
+		if (r)
 			goto out;
-		if ((r = crypto_shash_update(&sdesc.desc, &buf[i * 4], 4)))
+		r = crypto_shash_update(&sdesc.desc, &buf[i * 4], 4);
+		if (r)
 			goto out;
-		if ((r = crypto_shash_final(&sdesc.desc, &buf[i * 4])))
+		r = crypto_shash_final(&sdesc.desc, &buf[i * 4]);
+		if (r)
 			goto out;
 	}
 	crypto_xor(&buf[0], &buf[12], 4);
@@ -748,9 +751,9 @@ static int crypt_iv_tcw_gen(struct crypt_config *cc, u8 *iv,
 
 	/* Calculate IV */
 	memcpy(iv, tcw->iv_seed, cc->iv_size);
-	crypto_xor(iv, (u8*)&sector, 8);
+	crypto_xor(iv, (u8 *)&sector, 8);
 	if (cc->iv_size > 8)
-		crypto_xor(&iv[8], (u8*)&sector, cc->iv_size - 8);
+		crypto_xor(&iv[8], (u8 *)&sector, cc->iv_size - 8);
 
 	return r;
 }
@@ -1663,9 +1666,11 @@ static int crypt_ctr_cipher(struct dm_target *ti,
 		cc->iv_gen_ops = &crypt_iv_null_ops;
 	else if (strcmp(ivmode, "lmk") == 0) {
 		cc->iv_gen_ops = &crypt_iv_lmk_ops;
-		/* Version 2 and 3 is recognised according
+		/*
+		 * Version 2 and 3 is recognised according
 		 * to length of provided multi-key string.
 		 * If present (version 3), last key is used as IV seed.
+		 * All keys (including IV seed) are always the same size.
 		 */
 		if (cc->key_size % cc->key_parts) {
 			cc->key_parts++;
-- 
1.8.4.2

[PATCH 4/4] dm-crypt: Fix sparse (different base types) warnings

[Milan Broz]

drivers/md/dm-crypt.c:575:16: warning: incorrect type in assignment (different base types)
drivers/md/dm-crypt.c:575:16:    expected unsigned int [unsigned] <noident>
drivers/md/dm-crypt.c:575:16:    got restricted __le32 [usertype] <noident>
...

Explicitly define endianess to silence these annoying warnings.

N.B. there is still warning about variable length array,
but this code is common use of hash structure on stack on
many parts of the kernel.

Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
 drivers/md/dm-crypt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 2d6ee75f..b71583e 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -550,7 +550,7 @@ static int crypt_iv_lmk_one(struct crypt_config *cc, u8 *iv,
 		char ctx[crypto_shash_descsize(lmk->hash_tfm)];
 	} sdesc;
 	struct md5_state md5state;
-	u32 buf[4];
+	__le32 buf[4];
 	int i, r;
 
 	sdesc.desc.tfm = lmk->hash_tfm;
@@ -696,7 +696,7 @@ static int crypt_iv_tcw_whitening(struct crypt_config *cc,
 				  u8 *data)
 {
 	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
-	u64 sector = cpu_to_le64((u64)dmreq->iv_sector);
+	__le64 sector = cpu_to_le64((u64)dmreq->iv_sector);
 	u8 buf[TCW_WHITENING_SIZE];
 	struct {
 		struct shash_desc desc;
@@ -738,7 +738,7 @@ static int crypt_iv_tcw_gen(struct crypt_config *cc, u8 *iv,
 			    struct dm_crypt_request *dmreq)
 {
 	struct iv_tcw_private *tcw = &cc->iv_gen_private.tcw;
-	u64 sector = cpu_to_le64((u64)dmreq->iv_sector);
+	__le64 sector = cpu_to_le64((u64)dmreq->iv_sector);
 	u8 *src;
 	int r = 0;
 
-- 
1.8.4.2

Re: [PATCH 4/4] dm-crypt: Fix sparse (different base types) warnings

[Alasdair G Kergon]

On Sat, Nov 02, 2013 at 10:24:04PM +0100, Milan Broz wrote:
> N.B. there is still warning about variable length array,
> but this code is common use of hash structure on stack on
> many parts of the kernel.
 
Indeed, I never discovered a satisfactory way to avoid those ones.

Alasdair