| Message ID | 1382998864-10380-1-git-send-email-gmazyland@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
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. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
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
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
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 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
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. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
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);
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(-)