[v3,0/6] crypto: switch to crypto API for ESSIV generation

Message

Ard Biesheuvel June 19, 2019, 4:29 p.m. UTC

This series creates an ESSIV template that produces a skcipher or AEAD
transform based on a tuple of the form '<skcipher>,<cipher>,<shash>'
(or '<aead>,<cipher>,<shash>' for the AEAD case). It exposes the
encapsulated sync or async skcipher/aead by passing through all operations,
while using the cipher/shash pair to transform the input IV into an ESSIV
output IV.

This matches what both users of ESSIV in the kernel do, and so it is proposed
as a replacement for those, in patches #2 and #4.

This code has been tested using the fscrypt test suggested by Eric
(generic/549), as well as the mode-test script suggested by Milan for
the dm-crypt case. I also tested the aead case in a virtual machine,
but it definitely needs some wider testing from the dm-crypt experts.

Changes since v2:
- fixed a couple of bugs that snuck in after I'd done the bulk of my
  testing
- some cosmetic tweaks to the ESSIV template skcipher setkey function
  to align it with the aead one
- add a test case for essiv(cbc(aes),aes,sha256)
- add an accelerated implementation for arm64 that combines the IV
  derivation and the actual en/decryption in a single asm routine

Scroll down for tcrypt speed test result comparing the essiv template
with the asm implementation. Bare cbc(aes) tests included for reference
as well. Taken on a 2GHz Cortex-A57 (AMD Seattle)

Code can be found here
https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=essiv-v3

Cc: Herbert Xu <[e-mail]>
Cc: Eric Biggers <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: Gilad Ben-Yossef <[e-mail]>
Cc: Milan Broz <[e-mail]>

Ard Biesheuvel (6):
  crypto: essiv - create wrapper template for ESSIV generation
  fs: crypto: invoke crypto API for ESSIV handling
  md: dm-crypt: infer ESSIV block cipher from cipher string directly
  md: dm-crypt: switch to ESSIV crypto API template
  crypto: essiv - add test vector for essiv(cbc(aes),aes,sha256)
  crypto: arm64/aes - implement accelerated ESSIV/CBC mode

 arch/arm64/crypto/aes-glue.c  | 129 ++++
 arch/arm64/crypto/aes-modes.S |  99 +++
 crypto/Kconfig                |   4 +
 crypto/Makefile               |   1 +
 crypto/essiv.c                | 630 ++++++++++++++++++++
 crypto/tcrypt.c               |   9 +
 crypto/testmgr.c              |   6 +
 crypto/testmgr.h              | 208 +++++++
 drivers/md/Kconfig            |   1 +
 drivers/md/dm-crypt.c         | 237 ++------
 fs/crypto/Kconfig             |   1 +
 fs/crypto/crypto.c            |   5 -
 fs/crypto/fscrypt_private.h   |   9 -
 fs/crypto/keyinfo.c           |  88 +--
 14 files changed, 1132 insertions(+), 295 deletions(-)
 create mode 100644 crypto/essiv.c

Comments

Gilad Ben-Yossef June 20, 2019, 7:07 a.m. UTC | #1

On Wed, Jun 19, 2019 at 7:29 PM Ard Biesheuvel
<[e-mail]> wrote:
>
> This series creates an ESSIV template that produces a skcipher or AEAD
> transform based on a tuple of the form '<skcipher>,<cipher>,<shash>'
> (or '<aead>,<cipher>,<shash>' for the AEAD case). It exposes the
> encapsulated sync or async skcipher/aead by passing through all operations,
> while using the cipher/shash pair to transform the input IV into an ESSIV
> output IV.
>
> This matches what both users of ESSIV in the kernel do, and so it is proposed
> as a replacement for those, in patches #2 and #4.
>
> This code has been tested using the fscrypt test suggested by Eric
> (generic/549), as well as the mode-test script suggested by Milan for
> the dm-crypt case. I also tested the aead case in a virtual machine,
> but it definitely needs some wider testing from the dm-crypt experts.
>
> Changes since v2:
> - fixed a couple of bugs that snuck in after I'd done the bulk of my
>   testing
> - some cosmetic tweaks to the ESSIV template skcipher setkey function
>   to align it with the aead one
> - add a test case for essiv(cbc(aes),aes,sha256)
> - add an accelerated implementation for arm64 that combines the IV
>   derivation and the actual en/decryption in a single asm routine
>
> Scroll down for tcrypt speed test result comparing the essiv template
> with the asm implementation. Bare cbc(aes) tests included for reference
> as well. Taken on a 2GHz Cortex-A57 (AMD Seattle)
>
> Code can be found here
> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=essiv-v3


Thank you Ard for this work. It is very useful. I am testing this now
with the essiv implementation inside CryptoCell.

One possible future optimization this opens the door for is having the
template auto-increment the sector number.

This will allow the device manager or fscrypt code to ask for crypto
services on buffer spanning over a single sector size
and have the crypto code automatically increment the sector number
when processing the buffer.

This may potentially shave a few cycles because it can potentially
turn multiple calls into the crypto API in one, giving
the crypto code a larger buffer to work on.

This is actually supported by CryptoCell hardware and to the best of
my knowledge also by a similar HW from Qualcomm
via out-of-tree patches found in the Android tree.

If this makes sense to you perhaps it is a good idea to have the
template format be:

<skcipher>,<cipher>,<shash>, <sector size>

Where for now we will only support a sector size of '0' (i.e. do not
auto-increment) and later extend or am I over engineering? :-)

Thanks,
Gilad

Milan Broz June 20, 2019, 11:22 a.m. UTC | #2

On 19/06/2019 18:29, Ard Biesheuvel wrote:
> This series creates an ESSIV template that produces a skcipher or AEAD
> transform based on a tuple of the form '<skcipher>,<cipher>,<shash>'
> (or '<aead>,<cipher>,<shash>' for the AEAD case). It exposes the
> encapsulated sync or async skcipher/aead by passing through all operations,
> while using the cipher/shash pair to transform the input IV into an ESSIV
> output IV.
> 
> This matches what both users of ESSIV in the kernel do, and so it is proposed
> as a replacement for those, in patches #2 and #4.
> 
> This code has been tested using the fscrypt test suggested by Eric
> (generic/549), as well as the mode-test script suggested by Milan for
> the dm-crypt case. I also tested the aead case in a virtual machine,
> but it definitely needs some wider testing from the dm-crypt experts.
> 
> Changes since v2:
> - fixed a couple of bugs that snuck in after I'd done the bulk of my
>   testing
> - some cosmetic tweaks to the ESSIV template skcipher setkey function
>   to align it with the aead one
> - add a test case for essiv(cbc(aes),aes,sha256)
> - add an accelerated implementation for arm64 that combines the IV
>   derivation and the actual en/decryption in a single asm routine

I run tests for the whole patchset, including some older scripts and seems
it works for dm-crypt now.

For the new CRYPTO_ESSIV option - dm-crypt must unconditionally
select it (we rely on all IV generators availability in userspace),
but that's already done in patch 4.

Thanks,
Milan

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Ard Biesheuvel June 20, 2019, 11:54 a.m. UTC | #3

On Thu, 20 Jun 2019 at 13:22, Milan Broz <[e-mail]> wrote:
>
> On 19/06/2019 18:29, Ard Biesheuvel wrote:
> > This series creates an ESSIV template that produces a skcipher or AEAD
> > transform based on a tuple of the form '<skcipher>,<cipher>,<shash>'
> > (or '<aead>,<cipher>,<shash>' for the AEAD case). It exposes the
> > encapsulated sync or async skcipher/aead by passing through all operations,
> > while using the cipher/shash pair to transform the input IV into an ESSIV
> > output IV.
> >
> > This matches what both users of ESSIV in the kernel do, and so it is proposed
> > as a replacement for those, in patches #2 and #4.
> >
> > This code has been tested using the fscrypt test suggested by Eric
> > (generic/549), as well as the mode-test script suggested by Milan for
> > the dm-crypt case. I also tested the aead case in a virtual machine,
> > but it definitely needs some wider testing from the dm-crypt experts.
> >
> > Changes since v2:
> > - fixed a couple of bugs that snuck in after I'd done the bulk of my
> >   testing
> > - some cosmetic tweaks to the ESSIV template skcipher setkey function
> >   to align it with the aead one
> > - add a test case for essiv(cbc(aes),aes,sha256)
> > - add an accelerated implementation for arm64 that combines the IV
> >   derivation and the actual en/decryption in a single asm routine
>
> I run tests for the whole patchset, including some older scripts and seems
> it works for dm-crypt now.
>

Thanks Milan, that is really helpful.

Does this include configurations that combine authenc with essiv?

> For the new CRYPTO_ESSIV option - dm-crypt must unconditionally
> select it (we rely on all IV generators availability in userspace),
> but that's already done in patch 4.
>

Indeed.

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Milan Broz June 20, 2019, 12:09 p.m. UTC | #4

On 20/06/2019 13:54, Ard Biesheuvel wrote:
> On Thu, 20 Jun 2019 at 13:22, Milan Broz <[e-mail]> wrote:
>>
>> On 19/06/2019 18:29, Ard Biesheuvel wrote:
>>> This series creates an ESSIV template that produces a skcipher or AEAD
>>> transform based on a tuple of the form '<skcipher>,<cipher>,<shash>'
>>> (or '<aead>,<cipher>,<shash>' for the AEAD case). It exposes the
>>> encapsulated sync or async skcipher/aead by passing through all operations,
>>> while using the cipher/shash pair to transform the input IV into an ESSIV
>>> output IV.
>>>
>>> This matches what both users of ESSIV in the kernel do, and so it is proposed
>>> as a replacement for those, in patches #2 and #4.
>>>
>>> This code has been tested using the fscrypt test suggested by Eric
>>> (generic/549), as well as the mode-test script suggested by Milan for
>>> the dm-crypt case. I also tested the aead case in a virtual machine,
>>> but it definitely needs some wider testing from the dm-crypt experts.
>>>
>>> Changes since v2:
>>> - fixed a couple of bugs that snuck in after I'd done the bulk of my
>>>   testing
>>> - some cosmetic tweaks to the ESSIV template skcipher setkey function
>>>   to align it with the aead one
>>> - add a test case for essiv(cbc(aes),aes,sha256)
>>> - add an accelerated implementation for arm64 that combines the IV
>>>   derivation and the actual en/decryption in a single asm routine
>>
>> I run tests for the whole patchset, including some older scripts and seems
>> it works for dm-crypt now.
>>
> 
> Thanks Milan, that is really helpful.
> 
> Does this include configurations that combine authenc with essiv?

Hm, seems that we are missing these in luks2-integrity-test. I'll add them there.

I also used this older test
https://gitlab.com/omos/dm-crypt-test-scripts/blob/master/root/test_dmintegrity.sh

(just aes-gcm-random need to be commented out, we never supported this format, it was
written for some devel version)

But seems ESSIV is there tested only without AEAD composition...

So yes, this AEAD part need more testing.

Milan

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Milan Broz June 20, 2019, 1:14 p.m. UTC | #5

On 20/06/2019 14:09, Milan Broz wrote:
> On 20/06/2019 13:54, Ard Biesheuvel wrote:
>> On Thu, 20 Jun 2019 at 13:22, Milan Broz <[e-mail]> wrote:
>>>
>>> On 19/06/2019 18:29, Ard Biesheuvel wrote:
>>>> This series creates an ESSIV template that produces a skcipher or AEAD
>>>> transform based on a tuple of the form '<skcipher>,<cipher>,<shash>'
>>>> (or '<aead>,<cipher>,<shash>' for the AEAD case). It exposes the
>>>> encapsulated sync or async skcipher/aead by passing through all operations,
>>>> while using the cipher/shash pair to transform the input IV into an ESSIV
>>>> output IV.
>>>>
>>>> This matches what both users of ESSIV in the kernel do, and so it is proposed
>>>> as a replacement for those, in patches #2 and #4.
>>>>
>>>> This code has been tested using the fscrypt test suggested by Eric
>>>> (generic/549), as well as the mode-test script suggested by Milan for
>>>> the dm-crypt case. I also tested the aead case in a virtual machine,
>>>> but it definitely needs some wider testing from the dm-crypt experts.
>>>>
>>>> Changes since v2:
>>>> - fixed a couple of bugs that snuck in after I'd done the bulk of my
>>>>   testing
>>>> - some cosmetic tweaks to the ESSIV template skcipher setkey function
>>>>   to align it with the aead one
>>>> - add a test case for essiv(cbc(aes),aes,sha256)
>>>> - add an accelerated implementation for arm64 that combines the IV
>>>>   derivation and the actual en/decryption in a single asm routine
>>>
>>> I run tests for the whole patchset, including some older scripts and seems
>>> it works for dm-crypt now.
>>>
>>
>> Thanks Milan, that is really helpful.
>>
>> Does this include configurations that combine authenc with essiv?
> 
> Hm, seems that we are missing these in luks2-integrity-test. I'll add them there.
> 
> I also used this older test
> https://gitlab.com/omos/dm-crypt-test-scripts/blob/master/root/test_dmintegrity.sh
> 
> (just aes-gcm-random need to be commented out, we never supported this format, it was
> written for some devel version)
> 
> But seems ESSIV is there tested only without AEAD composition...
> 
> So yes, this AEAD part need more testing.

And unfortunately it does not work - it returns EIO on sectors where it should not be data corruption.

I added few lines with length-preserving mode with ESSIV + AEAD, please could you run luks2-integrity-test
in cryptsetup upstream?

This patch adds the tests:
https://gitlab.com/cryptsetup/cryptsetup/commit/4c74ff5e5ae328cb61b44bf99f98d08ffee3366a

It is ok on mainline kernel, fails with the patchset:

# ./luks2-integrity-test 
[aes-cbc-essiv:sha256:hmac-sha256:128:512][FORMAT][ACTIVATE]sha256sum: /dev/mapper/dmi_test: Input/output error
[FAIL]
 Expecting ee501705a084cd0ab6f4a28014bcf62b8bfa3434de00b82743c50b3abf06232c got .

FAILED backtrace:
77 ./luks2-integrity-test
112 intformat ./luks2-integrity-test
127 main ./luks2-integrity-test

Milan

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Ard Biesheuvel June 20, 2019, 1:52 p.m. UTC | #6

On Thu, 20 Jun 2019 at 15:14, Milan Broz <[e-mail]> wrote:
>
> On 20/06/2019 14:09, Milan Broz wrote:
> > On 20/06/2019 13:54, Ard Biesheuvel wrote:
> >> On Thu, 20 Jun 2019 at 13:22, Milan Broz <[e-mail]> wrote:
> >>>
> >>> On 19/06/2019 18:29, Ard Biesheuvel wrote:
> >>>> This series creates an ESSIV template that produces a skcipher or AEAD
> >>>> transform based on a tuple of the form '<skcipher>,<cipher>,<shash>'
> >>>> (or '<aead>,<cipher>,<shash>' for the AEAD case). It exposes the
> >>>> encapsulated sync or async skcipher/aead by passing through all operations,
> >>>> while using the cipher/shash pair to transform the input IV into an ESSIV
> >>>> output IV.
> >>>>
> >>>> This matches what both users of ESSIV in the kernel do, and so it is proposed
> >>>> as a replacement for those, in patches #2 and #4.
> >>>>
> >>>> This code has been tested using the fscrypt test suggested by Eric
> >>>> (generic/549), as well as the mode-test script suggested by Milan for
> >>>> the dm-crypt case. I also tested the aead case in a virtual machine,
> >>>> but it definitely needs some wider testing from the dm-crypt experts.
> >>>>
> >>>> Changes since v2:
> >>>> - fixed a couple of bugs that snuck in after I'd done the bulk of my
> >>>>   testing
> >>>> - some cosmetic tweaks to the ESSIV template skcipher setkey function
> >>>>   to align it with the aead one
> >>>> - add a test case for essiv(cbc(aes),aes,sha256)
> >>>> - add an accelerated implementation for arm64 that combines the IV
> >>>>   derivation and the actual en/decryption in a single asm routine
> >>>
> >>> I run tests for the whole patchset, including some older scripts and seems
> >>> it works for dm-crypt now.
> >>>
> >>
> >> Thanks Milan, that is really helpful.
> >>
> >> Does this include configurations that combine authenc with essiv?
> >
> > Hm, seems that we are missing these in luks2-integrity-test. I'll add them there.
> >
> > I also used this older test
> > https://gitlab.com/omos/dm-crypt-test-scripts/blob/master/root/test_dmintegrity.sh
> >
> > (just aes-gcm-random need to be commented out, we never supported this format, it was
> > written for some devel version)
> >
> > But seems ESSIV is there tested only without AEAD composition...
> >
> > So yes, this AEAD part need more testing.
>
> And unfortunately it does not work - it returns EIO on sectors where it should not be data corruption.
>
> I added few lines with length-preserving mode with ESSIV + AEAD, please could you run luks2-integrity-test
> in cryptsetup upstream?
>
> This patch adds the tests:
> https://gitlab.com/cryptsetup/cryptsetup/commit/4c74ff5e5ae328cb61b44bf99f98d08ffee3366a
>
> It is ok on mainline kernel, fails with the patchset:
>
> # ./luks2-integrity-test
> [aes-cbc-essiv:sha256:hmac-sha256:128:512][FORMAT][ACTIVATE]sha256sum: /dev/mapper/dmi_test: Input/output error
> [FAIL]
>  Expecting ee501705a084cd0ab6f4a28014bcf62b8bfa3434de00b82743c50b3abf06232c got .
>
> FAILED backtrace:
> 77 ./luks2-integrity-test
> 112 intformat ./luks2-integrity-test
> 127 main ./luks2-integrity-test
>

OK, I will investigate.

I did my testing in a VM using a volume that was created using a
distro kernel, and mounted and used it using a kernel with these
changes applied.

Likewise, if I take a working key.img and mode-test.img, i can mount
it and use it on the system running these patches.

I noticed that this test uses algif_skcipher not algif_aead when it
formats the volume, and so I wonder if the way userland creates the
image is affected by this?

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Milan Broz June 21, 2019, 7:01 a.m. UTC | #7

On 20/06/2019 15:52, Ard Biesheuvel wrote:
>>>> Does this include configurations that combine authenc with essiv?
>>>
>>> Hm, seems that we are missing these in luks2-integrity-test. I'll add them there.
>>>
>>> I also used this older test
>>> https://gitlab.com/omos/dm-crypt-test-scripts/blob/master/root/test_dmintegrity.sh
>>>
>>> (just aes-gcm-random need to be commented out, we never supported this format, it was
>>> written for some devel version)
>>>
>>> But seems ESSIV is there tested only without AEAD composition...
>>>
>>> So yes, this AEAD part need more testing.
>>
>> And unfortunately it does not work - it returns EIO on sectors where it should not be data corruption.
>>
>> I added few lines with length-preserving mode with ESSIV + AEAD, please could you run luks2-integrity-test
>> in cryptsetup upstream?
>>
>> This patch adds the tests:
>> https://gitlab.com/cryptsetup/cryptsetup/commit/4c74ff5e5ae328cb61b44bf99f98d08ffee3366a
>>
>> It is ok on mainline kernel, fails with the patchset:
>>
>> # ./luks2-integrity-test
>> [aes-cbc-essiv:sha256:hmac-sha256:128:512][FORMAT][ACTIVATE]sha256sum: /dev/mapper/dmi_test: Input/output error
>> [FAIL]
>>  Expecting ee501705a084cd0ab6f4a28014bcf62b8bfa3434de00b82743c50b3abf06232c got .
>>
>> FAILED backtrace:
>> 77 ./luks2-integrity-test
>> 112 intformat ./luks2-integrity-test
>> 127 main ./luks2-integrity-test
>>
> 
> OK, I will investigate.
> 
> I did my testing in a VM using a volume that was created using a
> distro kernel, and mounted and used it using a kernel with these
> changes applied.
> 
> Likewise, if I take a working key.img and mode-test.img, i can mount
> it and use it on the system running these patches.
> 
> I noticed that this test uses algif_skcipher not algif_aead when it
> formats the volume, and so I wonder if the way userland creates the
> image is affected by this?

Not sure if I understand the question, but I do not think userspace even touch data area here
(except direct-io wiping after the format, but it does not read it back).

It only encrypts keyslots - and here we cannot use AEAD (in fact it is already
authenticated by a LUKS digest).

So if the data area uses AEAD (or composition of length-preserving mode and
some authentication tag like HMAC), we fallback to non-AEAD for keyslot encryption.

In short, to test it, you need to activate device (that works ok with your patches)
and *access* the data, testing LUKS format and just keyslot access will never use AEAD.

So init the data by direct-io writes, and try to read them back (with dd).

For testing data on dm-integrity (or dm-crypt with AEAD encryption stacked oved dm-integrity)
I used small utility, maybe it could be useful https://github.com/mbroz/dm_int_tools 

Milan

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Ard Biesheuvel June 21, 2019, 7:06 a.m. UTC | #8

On Fri, 21 Jun 2019 at 09:01, Milan Broz <[e-mail]> wrote:
>
> On 20/06/2019 15:52, Ard Biesheuvel wrote:
> >>>> Does this include configurations that combine authenc with essiv?
> >>>
> >>> Hm, seems that we are missing these in luks2-integrity-test. I'll add them there.
> >>>
> >>> I also used this older test
> >>> https://gitlab.com/omos/dm-crypt-test-scripts/blob/master/root/test_dmintegrity.sh
> >>>
> >>> (just aes-gcm-random need to be commented out, we never supported this format, it was
> >>> written for some devel version)
> >>>
> >>> But seems ESSIV is there tested only without AEAD composition...
> >>>
> >>> So yes, this AEAD part need more testing.
> >>
> >> And unfortunately it does not work - it returns EIO on sectors where it should not be data corruption.
> >>
> >> I added few lines with length-preserving mode with ESSIV + AEAD, please could you run luks2-integrity-test
> >> in cryptsetup upstream?
> >>
> >> This patch adds the tests:
> >> https://gitlab.com/cryptsetup/cryptsetup/commit/4c74ff5e5ae328cb61b44bf99f98d08ffee3366a
> >>
> >> It is ok on mainline kernel, fails with the patchset:
> >>
> >> # ./luks2-integrity-test
> >> [aes-cbc-essiv:sha256:hmac-sha256:128:512][FORMAT][ACTIVATE]sha256sum: /dev/mapper/dmi_test: Input/output error
> >> [FAIL]
> >>  Expecting ee501705a084cd0ab6f4a28014bcf62b8bfa3434de00b82743c50b3abf06232c got .
> >>
> >> FAILED backtrace:
> >> 77 ./luks2-integrity-test
> >> 112 intformat ./luks2-integrity-test
> >> 127 main ./luks2-integrity-test
> >>
> >
> > OK, I will investigate.
> >
> > I did my testing in a VM using a volume that was created using a
> > distro kernel, and mounted and used it using a kernel with these
> > changes applied.
> >
> > Likewise, if I take a working key.img and mode-test.img, i can mount
> > it and use it on the system running these patches.
> >
> > I noticed that this test uses algif_skcipher not algif_aead when it
> > formats the volume, and so I wonder if the way userland creates the
> > image is affected by this?
>
> Not sure if I understand the question, but I do not think userspace even touch data area here
> (except direct-io wiping after the format, but it does not read it back).
>
> It only encrypts keyslots - and here we cannot use AEAD (in fact it is already
> authenticated by a LUKS digest).
>
> So if the data area uses AEAD (or composition of length-preserving mode and
> some authentication tag like HMAC), we fallback to non-AEAD for keyslot encryption.
>
> In short, to test it, you need to activate device (that works ok with your patches)
> and *access* the data, testing LUKS format and just keyslot access will never use AEAD.
>
> So init the data by direct-io writes, and try to read them back (with dd).
>
> For testing data on dm-integrity (or dm-crypt with AEAD encryption stacked oved dm-integrity)
> I used small utility, maybe it could be useful https://github.com/mbroz/dm_int_tools
>

Thanks.

It appears that my code generates the wrong authentication tags on
encryption, but on decryption it works fine.
I'll keep digging ...

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Ard Biesheuvel June 21, 2019, 7:37 a.m. UTC | #9

On Fri, 21 Jun 2019 at 09:06, Ard Biesheuvel <[e-mail]> wrote:
>
> On Fri, 21 Jun 2019 at 09:01, Milan Broz <[e-mail]> wrote:
> >
> > On 20/06/2019 15:52, Ard Biesheuvel wrote:
> > >>>> Does this include configurations that combine authenc with essiv?
> > >>>
> > >>> Hm, seems that we are missing these in luks2-integrity-test. I'll add them there.
> > >>>
> > >>> I also used this older test
> > >>> https://gitlab.com/omos/dm-crypt-test-scripts/blob/master/root/test_dmintegrity.sh
> > >>>
> > >>> (just aes-gcm-random need to be commented out, we never supported this format, it was
> > >>> written for some devel version)
> > >>>
> > >>> But seems ESSIV is there tested only without AEAD composition...
> > >>>
> > >>> So yes, this AEAD part need more testing.
> > >>
> > >> And unfortunately it does not work - it returns EIO on sectors where it should not be data corruption.
> > >>
> > >> I added few lines with length-preserving mode with ESSIV + AEAD, please could you run luks2-integrity-test
> > >> in cryptsetup upstream?
> > >>
> > >> This patch adds the tests:
> > >> https://gitlab.com/cryptsetup/cryptsetup/commit/4c74ff5e5ae328cb61b44bf99f98d08ffee3366a
> > >>
> > >> It is ok on mainline kernel, fails with the patchset:
> > >>
> > >> # ./luks2-integrity-test
> > >> [aes-cbc-essiv:sha256:hmac-sha256:128:512][FORMAT][ACTIVATE]sha256sum: /dev/mapper/dmi_test: Input/output error
> > >> [FAIL]
> > >>  Expecting ee501705a084cd0ab6f4a28014bcf62b8bfa3434de00b82743c50b3abf06232c got .
> > >>
> > >> FAILED backtrace:
> > >> 77 ./luks2-integrity-test
> > >> 112 intformat ./luks2-integrity-test
> > >> 127 main ./luks2-integrity-test
> > >>
> > >
> > > OK, I will investigate.
> > >
> > > I did my testing in a VM using a volume that was created using a
> > > distro kernel, and mounted and used it using a kernel with these
> > > changes applied.
> > >
> > > Likewise, if I take a working key.img and mode-test.img, i can mount
> > > it and use it on the system running these patches.
> > >
> > > I noticed that this test uses algif_skcipher not algif_aead when it
> > > formats the volume, and so I wonder if the way userland creates the
> > > image is affected by this?
> >
> > Not sure if I understand the question, but I do not think userspace even touch data area here
> > (except direct-io wiping after the format, but it does not read it back).
> >
> > It only encrypts keyslots - and here we cannot use AEAD (in fact it is already
> > authenticated by a LUKS digest).
> >
> > So if the data area uses AEAD (or composition of length-preserving mode and
> > some authentication tag like HMAC), we fallback to non-AEAD for keyslot encryption.
> >
> > In short, to test it, you need to activate device (that works ok with your patches)
> > and *access* the data, testing LUKS format and just keyslot access will never use AEAD.
> >
> > So init the data by direct-io writes, and try to read them back (with dd).
> >
> > For testing data on dm-integrity (or dm-crypt with AEAD encryption stacked oved dm-integrity)
> > I used small utility, maybe it could be useful https://github.com/mbroz/dm_int_tools
> >
>
> Thanks.
>
> It appears that my code generates the wrong authentication tags on
> encryption, but on decryption it works fine.
> I'll keep digging ...

OK, mystery solved.

The skcipher inside authenc() was corrupting the IV before the hmac
got a chance to read it.

I'll send out an updated version of the series.

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel