Re: [PATCH v9 3/7] md: dm-crypt: switch to ESSIV crypto API template
On Mon, 12 Aug 2019 at 16:51, Milan Broz wrote:
>
> On 12/08/2019 09:50, Ard Biesheuvel wrote:
> > On Mon, 12 Aug 2019 at 10:44, Milan Broz wrote:
> >>
> >> On 12/08/2019 08:54, Ard Biesheuvel wrote:
> >>> On Mon, 12 Aug 2019 at 09:33, Milan Broz wrote:
> Try for example
> # cryptsetup luksFormat /dev/sdc -c aes-cbc-essiv:sha256 --integrity
> hmac-sha256 -q -i1
>
> It should produce Crypto API string
> authenc(hmac(sha256),essiv(cbc(aes),sha256))
> while it produces
> essiv(authenc(hmac(sha256),cbc(aes)),sha256)
> (and fails).
>
> >>>
> >>> No. I don't know why it fails, but the latter is actually the correct
> >>> string. The essiv template is instantiated either as a skcipher or as
> >>> an aead, and it encapsulates the entire transformation. (This is
> >>> necessary considering that the IV is passed via the AAD and so the
> >>> ESSIV handling needs to touch that as well)
> >>
> >> Hm. Constructing these strings seems to be more confusing than dmcrypt
> >> mode combinations :-)
> >>
> >> But you are right, I actually tried the former string
> >> (authenc(hmac(sha256),essiv(cbc(aes),sha256)))
> >> and it worked, but I guess the authenticated IV (AAD) was actually the
> >> input to IV (plain sector number)
> >> not the output of ESSIV? Do I understand it correctly now?
> >>
> >
> > Indeed. The former string instantiates the skcipher version of the
> > ESSIV template, and so the AAD handling is omitted, and we end up
> > using the plain IV in the authentication rather than the encrypted IV.
> >
> > So when using the latter string, does it produce any error messages
> > when it fails?
>
> The error is
> table: 253:1: crypt: Error decoding and setting key
>
> and it is failing in crypt_setkey() int this crypto_aead_setkey();
>
> And it is because it now wrongly calculates MAC key length.
> (We have two keys here - one for length-preserving CBC-ESSIV encryption
> and one for HMAC.)
>
> This super-ugly hotfix helps here... I guess it can be done better :-)
>
Weird. It did work fine before, but now that I have dropped the 'md:
dm-crypt: infer ESSIV block cipher from cipher string directly' patch,
we are probably taking a different code path and hitting this error.
I'll try to fix this cleanly. Thanks for doing the diagnosis.
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index e9a0093c88ee..7b06d975a2e1 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -2342,6 +2342,9 @@ static int crypt_ctr_auth_cipher(struct crypt_config
> *cc, char *cipher_api)
> char *start, *end, *mac_alg = NULL;
> struct crypto_ahash *mac;
>
> + if (strstarts(cipher_api, "essiv(authenc("))
> + cipher_api += strlen("essiv(");
> +
> if (!strstarts(cipher_api, "authenc("))
> return 0;
>
> Milan
Re: [PATCH v9 3/7] md: dm-crypt: switch to ESSIV crypto API template
On 12/08/2019 09:50, Ard Biesheuvel wrote:
> On Mon, 12 Aug 2019 at 10:44, Milan Broz wrote:
>>
>> On 12/08/2019 08:54, Ard Biesheuvel wrote:
>>> On Mon, 12 Aug 2019 at 09:33, Milan Broz wrote:
Try for example
# cryptsetup luksFormat /dev/sdc -c aes-cbc-essiv:sha256 --integrity
hmac-sha256 -q -i1
It should produce Crypto API string
authenc(hmac(sha256),essiv(cbc(aes),sha256))
while it produces
essiv(authenc(hmac(sha256),cbc(aes)),sha256)
(and fails).
>>>
>>> No. I don't know why it fails, but the latter is actually the correct
>>> string. The essiv template is instantiated either as a skcipher or as
>>> an aead, and it encapsulates the entire transformation. (This is
>>> necessary considering that the IV is passed via the AAD and so the
>>> ESSIV handling needs to touch that as well)
>>
>> Hm. Constructing these strings seems to be more confusing than dmcrypt mode
>> combinations :-)
>>
>> But you are right, I actually tried the former string
>> (authenc(hmac(sha256),essiv(cbc(aes),sha256)))
>> and it worked, but I guess the authenticated IV (AAD) was actually the input
>> to IV (plain sector number)
>> not the output of ESSIV? Do I understand it correctly now?
>>
>
> Indeed. The former string instantiates the skcipher version of the
> ESSIV template, and so the AAD handling is omitted, and we end up
> using the plain IV in the authentication rather than the encrypted IV.
>
> So when using the latter string, does it produce any error messages
> when it fails?
The error is
table: 253:1: crypt: Error decoding and setting key
and it is failing in crypt_setkey() int this crypto_aead_setkey();
And it is because it now wrongly calculates MAC key length.
(We have two keys here - one for length-preserving CBC-ESSIV encryption
and one for HMAC.)
This super-ugly hotfix helps here... I guess it can be done better :-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index e9a0093c88ee..7b06d975a2e1 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2342,6 +2342,9 @@ static int crypt_ctr_auth_cipher(struct crypt_config *cc,
char *cipher_api)
char *start, *end, *mac_alg = NULL;
struct crypto_ahash *mac;
+ if (strstarts(cipher_api, "essiv(authenc("))
+ cipher_api += strlen("essiv(");
+
if (!strstarts(cipher_api, "authenc("))
return 0;
Milan
Re: [PATCH v9 3/7] md: dm-crypt: switch to ESSIV crypto API template
On 12/08/2019 08:54, Ard Biesheuvel wrote: > On Mon, 12 Aug 2019 at 09:33, Milan Broz wrote: >> Try for example >> # cryptsetup luksFormat /dev/sdc -c aes-cbc-essiv:sha256 --integrity >> hmac-sha256 -q -i1 >> >> It should produce Crypto API string >> authenc(hmac(sha256),essiv(cbc(aes),sha256)) >> while it produces >> essiv(authenc(hmac(sha256),cbc(aes)),sha256) >> (and fails). >> > > No. I don't know why it fails, but the latter is actually the correct > string. The essiv template is instantiated either as a skcipher or as > an aead, and it encapsulates the entire transformation. (This is > necessary considering that the IV is passed via the AAD and so the > ESSIV handling needs to touch that as well) Hm. Constructing these strings seems to be more confusing than dmcrypt mode combinations :-) But you are right, I actually tried the former string (authenc(hmac(sha256),essiv(cbc(aes),sha256))) and it worked, but I guess the authenticated IV (AAD) was actually the input to IV (plain sector number) not the output of ESSIV? Do I understand it correctly now? Milan
Re: [PATCH v9 3/7] md: dm-crypt: switch to ESSIV crypto API template
On Mon, 12 Aug 2019 at 09:33, Milan Broz wrote:
>
> Hi,
>
> On 10/08/2019 11:40, Ard Biesheuvel wrote:
> > Replace the explicit ESSIV handling in the dm-crypt driver with calls
> > into the crypto API, which now possesses the capability to perform
> > this processing within the crypto subsystem.
> >
> > Signed-off-by: Ard Biesheuvel
> > ---
> > drivers/md/Kconfig| 1 +
> > drivers/md/dm-crypt.c | 194
> > 2 files changed, 33 insertions(+), 162 deletions(-)
> >
> > diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> > index 3834332f4963..b727e8f15264 100644
> > --- a/drivers/md/Kconfig
> > +++ b/drivers/md/Kconfig
> ...
> > @@ -2493,6 +2339,20 @@ static int crypt_ctr_cipher_new(struct dm_target
> > *ti, char *cipher_in, char *key
> > if (*ivmode && !strcmp(*ivmode, "lmk"))
> > cc->tfms_count = 64;
> >
> > + if (*ivmode && !strcmp(*ivmode, "essiv")) {
> > + if (!*ivopts) {
> > + ti->error = "Digest algorithm missing for ESSIV mode";
> > + return -EINVAL;
> > + }
> > + ret = snprintf(buf, CRYPTO_MAX_ALG_NAME, "essiv(%s,%s)",
> > +cipher_api, *ivopts);
>
> This is wrong. It works only in length-preserving modes, not in AEAD modes.
>
> Try for example
> # cryptsetup luksFormat /dev/sdc -c aes-cbc-essiv:sha256 --integrity
> hmac-sha256 -q -i1
>
> It should produce Crypto API string
> authenc(hmac(sha256),essiv(cbc(aes),sha256))
> while it produces
> essiv(authenc(hmac(sha256),cbc(aes)),sha256)
> (and fails).
>
No. I don't know why it fails, but the latter is actually the correct
string. The essiv template is instantiated either as a skcipher or as
an aead, and it encapsulates the entire transformation. (This is
necessary considering that the IV is passed via the AAD and so the
ESSIV handling needs to touch that as well)
This code worked fine in my testing: I could instantiate
essiv(authenc(hmac(sha256),cbc(aes)),sha256)
essiv(authenc(hmac(sha1),cbc(aes)),sha256)
where the former worked as expected (including fuzz testing of the
arm64 implementation), and the second got instantiated as well, but
with a complaint about a missing test case.
So I'm not sure why this is failing, I will try to check once I have
access to my ordinary development environment.
> You can run "luks2-integrity-test" from cryptsetup test suite to detect it.
>
> Just the test does not fail, it prints N/A for ESSIV use cases - we need to
> deal with older kernels...
> I can probable change it to fail unconditionally though.
>
> ...
> > @@ -2579,9 +2439,19 @@ static int crypt_ctr_cipher_old(struct dm_target
> > *ti, char *cipher_in, char *key
> > if (!cipher_api)
> > goto bad_mem;
> >
> > - ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
> > -"%s(%s)", chainmode, cipher);
> > - if (ret < 0) {
> > + if (*ivmode && !strcmp(*ivmode, "essiv")) {
> > + if (!*ivopts) {
> > + ti->error = "Digest algorithm missing for ESSIV mode";
> > + kfree(cipher_api);
> > + return -EINVAL;
> > + }
> > + ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
> > +"essiv(%s(%s),%s)", chainmode, cipher,
> > *ivopts);
>
> I guess here it is ok, because old forma cannot use AEAD.
>
> Thanks,
> Milan
Re: [PATCH v9 3/7] md: dm-crypt: switch to ESSIV crypto API template
Hi,
On 10/08/2019 11:40, Ard Biesheuvel wrote:
> Replace the explicit ESSIV handling in the dm-crypt driver with calls
> into the crypto API, which now possesses the capability to perform
> this processing within the crypto subsystem.
>
> Signed-off-by: Ard Biesheuvel
> ---
> drivers/md/Kconfig| 1 +
> drivers/md/dm-crypt.c | 194
> 2 files changed, 33 insertions(+), 162 deletions(-)
>
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 3834332f4963..b727e8f15264 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
...
> @@ -2493,6 +2339,20 @@ static int crypt_ctr_cipher_new(struct dm_target *ti,
> char *cipher_in, char *key
> if (*ivmode && !strcmp(*ivmode, "lmk"))
> cc->tfms_count = 64;
>
> + if (*ivmode && !strcmp(*ivmode, "essiv")) {
> + if (!*ivopts) {
> + ti->error = "Digest algorithm missing for ESSIV mode";
> + return -EINVAL;
> + }
> + ret = snprintf(buf, CRYPTO_MAX_ALG_NAME, "essiv(%s,%s)",
> +cipher_api, *ivopts);
This is wrong. It works only in length-preserving modes, not in AEAD modes.
Try for example
# cryptsetup luksFormat /dev/sdc -c aes-cbc-essiv:sha256 --integrity
hmac-sha256 -q -i1
It should produce Crypto API string
authenc(hmac(sha256),essiv(cbc(aes),sha256))
while it produces
essiv(authenc(hmac(sha256),cbc(aes)),sha256)
(and fails).
You can run "luks2-integrity-test" from cryptsetup test suite to detect it.
Just the test does not fail, it prints N/A for ESSIV use cases - we need to
deal with older kernels...
I can probable change it to fail unconditionally though.
...
> @@ -2579,9 +2439,19 @@ static int crypt_ctr_cipher_old(struct dm_target *ti,
> char *cipher_in, char *key
> if (!cipher_api)
> goto bad_mem;
>
> - ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
> -"%s(%s)", chainmode, cipher);
> - if (ret < 0) {
> + if (*ivmode && !strcmp(*ivmode, "essiv")) {
> + if (!*ivopts) {
> + ti->error = "Digest algorithm missing for ESSIV mode";
> + kfree(cipher_api);
> + return -EINVAL;
> + }
> + ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
> +"essiv(%s(%s),%s)", chainmode, cipher, *ivopts);
I guess here it is ok, because old forma cannot use AEAD.
Thanks,
Milan
