| Message ID | 20190619162921.12509-1-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | crypto: switch to crypto API for ESSIV generation | expand |
On Wed, Jun 19, 2019 at 7:29 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> 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
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
On Thu, 20 Jun 2019 at 13:22, Milan Broz <gmazyland@gmail.com> 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.
On 20/06/2019 13:54, Ard Biesheuvel wrote: > On Thu, 20 Jun 2019 at 13:22, Milan Broz <gmazyland@gmail.com> 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
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 <gmazyland@gmail.com> 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
On Thu, 20 Jun 2019 at 15:14, Milan Broz <gmazyland@gmail.com> 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 <gmazyland@gmail.com> 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?
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
On Fri, 21 Jun 2019 at 09:01, Milan Broz <gmazyland@gmail.com> 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 ...
On Fri, 21 Jun 2019 at 09:06, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Fri, 21 Jun 2019 at 09:01, Milan Broz <gmazyland@gmail.com> 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.