[PATCH] dm-crypt: Fix parsing of extended IV arguments.

[PATCH] dm-crypt: Fix parsing of extended IV arguments.

[Milan Broz]

The dm-crypt cipher specification in a mapping table is defined as
  cipher[:keycount]-chainmode-ivmode[:ivopts] or with the new crypt API format
  capi:cipher_api_spec-ivmode[:ivopts].

For ESSIV, the parameter includes hash specification, for example aes-cbc-essiv:sha256.

The implementation expected that additional IV option never includes another dash '-' character

Unfortunately, with SHA3, we have now names like sha3-256, so the mapping table parser fails:

dmsetup create test --table "0 8 crypt aes-cbc-essiv:sha3-256 9c1185a5c5e9fc54612808977ee8f5b9e 0 /dev/sdb 0"
  or (new format)
dmsetup create test --table "0 8 crypt capi:cbc(aes)-essiv:sha3-256 9c1185a5c5e9fc54612808977ee8f5b9e 0 /dev/sdb 0"

  device-mapper: crypt: Ignoring unexpected additional cipher options
  device-mapper: table: 253:0: crypt: Error creating IV
  device-mapper: ioctl: error adding target to table

This patch fixes the dm-crypt constructor to ignore additional dash in IV options and also removes
bogus warning (that is ignored anyway).

[This patch should go into stable tree as well.]

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

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 0ff22159a0ca..71bfb85f9652 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2414,9 +2414,21 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key
 	 * capi:cipher_api_spec-iv:ivopts
 	 */
 	tmp = &cipher_in[strlen("capi:")];
-	cipher_api = strsep(&tmp, "-");
-	*ivmode = strsep(&tmp, ":");
-	*ivopts = tmp;
+
+	/* Separate IV options if present, it can contain another '-' in hash name */
+	*ivopts = strrchr(tmp, ':');
+	if (*ivopts) {
+		**ivopts = '\0';
+		(*ivopts)++;
+	}
+	/* Parse IV mode */
+	*ivmode = strrchr(tmp, '-');
+	if (*ivmode) {
+		**ivmode = '\0';
+		(*ivmode)++;
+	}
+	/* The rest is crypto API spec */
+	cipher_api = tmp;
 
 	if (*ivmode && !strcmp(*ivmode, "lmk"))
 		cc->tfms_count = 64;
@@ -2486,11 +2498,8 @@ static int crypt_ctr_cipher_old(struct dm_target *ti, char *cipher_in, char *key
 		goto bad_mem;
 
 	chainmode = strsep(&tmp, "-");
-	*ivopts = strsep(&tmp, "-");
-	*ivmode = strsep(&*ivopts, ":");
-
-	if (tmp)
-		DMWARN("Ignoring unexpected additional cipher options");
+	*ivmode   = strsep(&tmp, ":");
+	*ivopts = tmp;
 
 	/*
 	 * For compatibility with the original dm-crypt mapping format, if
-- 
2.20.1

Re: dm-crypt: Fix parsing of extended IV arguments.

[Mike Snitzer]

On Wed, Jan 09 2019 at  5:57am -0500,
Milan Broz <gmazyland@gmail.com> wrote:

> The dm-crypt cipher specification in a mapping table is defined as
>   cipher[:keycount]-chainmode-ivmode[:ivopts] or with the new crypt API format
>   capi:cipher_api_spec-ivmode[:ivopts].
> 
> For ESSIV, the parameter includes hash specification, for example aes-cbc-essiv:sha256.
> 
> The implementation expected that additional IV option never includes another dash '-' character
> 
> Unfortunately, with SHA3, we have now names like sha3-256, so the mapping table parser fails:
> 
> dmsetup create test --table "0 8 crypt aes-cbc-essiv:sha3-256 9c1185a5c5e9fc54612808977ee8f5b9e 0 /dev/sdb 0"
>   or (new format)
> dmsetup create test --table "0 8 crypt capi:cbc(aes)-essiv:sha3-256 9c1185a5c5e9fc54612808977ee8f5b9e 0 /dev/sdb 0"
> 
>   device-mapper: crypt: Ignoring unexpected additional cipher options
>   device-mapper: table: 253:0: crypt: Error creating IV
>   device-mapper: ioctl: error adding target to table
> 
> This patch fixes the dm-crypt constructor to ignore additional dash in IV options and also removes
> bogus warning (that is ignored anyway).
> 
> [This patch should go into stable tree as well.]

Rather than this it'd be useful to just be more explicit, e.g.:

Fixes: XXXXXXXX ("commit subject")
Cc: stable@vger.kernel.org # > 4.x?

Once I know which commit exposed us to this problem I can take care of
getting this fix staged for 5.0-rcX inclussion.

Thanks,
Mike

Re: dm-crypt: Fix parsing of extended IV arguments.

[Milan Broz]

On 10/01/2019 20:18, Mike Snitzer wrote:
> On Wed, Jan 09 2019 at  5:57am -0500,
> Milan Broz <gmazyland@gmail.com> wrote:

>> This patch fixes the dm-crypt constructor to ignore additional dash in IV options and also removes
>> bogus warning (that is ignored anyway).
>>
>> [This patch should go into stable tree as well.]
> 
> Rather than this it'd be useful to just be more explicit, e.g.:

There is no single reference commit.

The new capi: was introduced in
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=33d2f09fcb357fd1861c4959d1d3505492bf91f8

but I think the second part applies even to older kernel, but not sure it makes sense to fix it there (I guess not)...

So perhaps the best is to apply it to stable kernel >= 4.12

Milan

Re: dm-crypt: Fix parsing of extended IV arguments.

[Mike Snitzer]

On Thu, Jan 10 2019 at  2:50pm -0500,
Milan Broz <gmazyland@gmail.com> wrote:

> On 10/01/2019 20:18, Mike Snitzer wrote:
> > On Wed, Jan 09 2019 at  5:57am -0500,
> > Milan Broz <gmazyland@gmail.com> wrote:
> 
> >> This patch fixes the dm-crypt constructor to ignore additional dash in IV options and also removes
> >> bogus warning (that is ignored anyway).
> >>
> >> [This patch should go into stable tree as well.]
> > 
> > Rather than this it'd be useful to just be more explicit, e.g.:
> 
> There is no single reference commit.
> 
> The new capi: was introduced in
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=33d2f09fcb357fd1861c4959d1d3505492bf91f8
> 
> but I think the second part applies even to older kernel, but not sure it makes sense to fix it there (I guess not)...
> 
> So perhaps the best is to apply it to stable kernel >= 4.12

OK, now staged, I tweaked your header though because (like your email
reply) it was lacking proper wrapping.

See: https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.0&id=1856b9f7bcc8e9bdcccc360aabb56fbd4dd6c565

Thanks,
Mike