[v8,0/7] crypto: switch to crypto API for ESSIV generation

Message

Ard Biesheuvel July 4, 2019, 6:30 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.

Note to the maintainer: patches #2 .. #4 are against other subsystems, and
so it might make sense to take only the crypto patches (#1, #5, and optionally
#6 and #7) now, and allow the other subsystem maintainers to take the other
patches at their leisure during the next cycle.

This code has been tested using the fscrypt test suggested by Eric
(generic/549 *), as well as the mode-test scripts 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.

* due to sloppiness on my part, the fscrypt change was actually broken
  since I switched back to using full size IVs in the ESSIV template.
  This time, I fixed this, and reran the test in question and it passed.

The consensus appears to be that it would be useful if the crypto API
encapsulates the handling of multiple subsequent blocks that are
encrypted using a 64-bit LE counter as IV, and makes it the duty of
the algo to increment the counter between blocks. However, this is
equally suitable for non-ESSIV transforms (or even more so), and so
this is left as a future enhancement to  be applied on top.

Changes since v7:
- rebase onto cryptodev/master
- drop change to ivsize in #2
- add Milan's R-b's

Changes since v6:
- make CRYPTO_ESSIV user selectable so we can opt out of selecting it even
  if FS_ENCRYPTION (which cannot be built as a module) is enabled
- move a comment along with the code it referred to (#3), not that this change
  and removing some redundant braces makes the diff look totally different
- add Milan's R-b to #3 and #4

Changes since v5:
- drop redundant #includes and drop some unneeded braces (#2)
- add test case for essiv(authenc(hmac(sha256),cbc(aes)),aes,sha256)
- make ESSIV driver deal with assoc data that is described by more than two
  scatterlist entries - this only happens when the extended tests are being
  performed, so don't optimize for it
- clarify that both fscrypt and dm-crypt only use ESSIV in special cases (#7)

Changes since v4:
- make the ESSIV template IV size equal the IV size of the encapsulated
  cipher - defining it as 8 bytes was needlessly restrictive, and also
  complicated the code for no reason
- add a missing kfree() spotted by Smatch
- add additional algo length name checks when constructing the essiv()
  cipher name
- reinstate the 'essiv' IV generation implementation in dm-crypt, but
  make its generation function identical to plain64le (and drop the other
  methods)
- fix a bug in the arm64 CE/NEON code
- simplify the arm64 code by reusing more of the existing CBC implementation
  (patch #6 is new to this series and was added for this reason)

Changes since v3:
- address various review comments from Eric on patch #1
- use Kconfig's 'imply' instead of 'select' to permit CRYPTO_ESSIV to be
  enabled as a module or disabled entirely even if fscrypt is compiled in (#2)
- fix an issue in the AEAD encrypt path caused by the IV being clobbered by
  the inner skcipher before the hmac was being calculated

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-v8

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 (7):
  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-cts-cbc - factor out CBC en/decryption of a walk
  crypto: arm64/aes - implement accelerated ESSIV/CBC mode

 arch/arm64/crypto/aes-glue.c  | 205 +++++--
 arch/arm64/crypto/aes-modes.S |  29 +-
 crypto/Kconfig                |  28 +
 crypto/Makefile               |   1 +
 crypto/essiv.c                | 640 ++++++++++++++++++++
 crypto/tcrypt.c               |   9 +
 crypto/testmgr.c              |  14 +
 crypto/testmgr.h              | 497 +++++++++++++++
 drivers/md/Kconfig            |   1 +
 drivers/md/dm-crypt.c         | 235 ++-----
 fs/crypto/Kconfig             |   1 +
 fs/crypto/crypto.c            |   5 -
 fs/crypto/fscrypt_private.h   |   9 -
 fs/crypto/keyinfo.c           |  93 +--
 14 files changed, 1435 insertions(+), 332 deletions(-)
 create mode 100644 crypto/essiv.c

Comments

Ard Biesheuvel July 5, 2019, 6:35 a.m. UTC | #1

On Thu, 4 Jul 2019 at 20:30, 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.
>
> Note to the maintainer: patches #2 .. #4 are against other subsystems, and
> so it might make sense to take only the crypto patches (#1, #5, and optionally
> #6 and #7) now, and allow the other subsystem maintainers to take the other
> patches at their leisure during the next cycle.
>

Please disregard #6 and #7 for now - the IV handling is busted after
the rebase to cryptodev/master. #1 and #5 should be fine though

Since I am obviously unsuccessful in cranking out these patches
without bugs in time before my leave, I am going to stop trying, and I
will get back to this work when I return from my leave, around the end
of September.


> This code has been tested using the fscrypt test suggested by Eric
> (generic/549 *), as well as the mode-test scripts 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.
>
> * due to sloppiness on my part, the fscrypt change was actually broken
>   since I switched back to using full size IVs in the ESSIV template.
>   This time, I fixed this, and reran the test in question and it passed.
>
> The consensus appears to be that it would be useful if the crypto API
> encapsulates the handling of multiple subsequent blocks that are
> encrypted using a 64-bit LE counter as IV, and makes it the duty of
> the algo to increment the counter between blocks. However, this is
> equally suitable for non-ESSIV transforms (or even more so), and so
> this is left as a future enhancement to  be applied on top.
>
> Changes since v7:
> - rebase onto cryptodev/master
> - drop change to ivsize in #2
> - add Milan's R-b's
>
> Changes since v6:
> - make CRYPTO_ESSIV user selectable so we can opt out of selecting it even
>   if FS_ENCRYPTION (which cannot be built as a module) is enabled
> - move a comment along with the code it referred to (#3), not that this change
>   and removing some redundant braces makes the diff look totally different
> - add Milan's R-b to #3 and #4
>
> Changes since v5:
> - drop redundant #includes and drop some unneeded braces (#2)
> - add test case for essiv(authenc(hmac(sha256),cbc(aes)),aes,sha256)
> - make ESSIV driver deal with assoc data that is described by more than two
>   scatterlist entries - this only happens when the extended tests are being
>   performed, so don't optimize for it
> - clarify that both fscrypt and dm-crypt only use ESSIV in special cases (#7)
>
> Changes since v4:
> - make the ESSIV template IV size equal the IV size of the encapsulated
>   cipher - defining it as 8 bytes was needlessly restrictive, and also
>   complicated the code for no reason
> - add a missing kfree() spotted by Smatch
> - add additional algo length name checks when constructing the essiv()
>   cipher name
> - reinstate the 'essiv' IV generation implementation in dm-crypt, but
>   make its generation function identical to plain64le (and drop the other
>   methods)
> - fix a bug in the arm64 CE/NEON code
> - simplify the arm64 code by reusing more of the existing CBC implementation
>   (patch #6 is new to this series and was added for this reason)
>
> Changes since v3:
> - address various review comments from Eric on patch #1
> - use Kconfig's 'imply' instead of 'select' to permit CRYPTO_ESSIV to be
>   enabled as a module or disabled entirely even if fscrypt is compiled in (#2)
> - fix an issue in the AEAD encrypt path caused by the IV being clobbered by
>   the inner skcipher before the hmac was being calculated
>
> 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-v8
>
> 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 (7):
>   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-cts-cbc - factor out CBC en/decryption of a walk
>   crypto: arm64/aes - implement accelerated ESSIV/CBC mode
>
>  arch/arm64/crypto/aes-glue.c  | 205 +++++--
>  arch/arm64/crypto/aes-modes.S |  29 +-
>  crypto/Kconfig                |  28 +
>  crypto/Makefile               |   1 +
>  crypto/essiv.c                | 640 ++++++++++++++++++++
>  crypto/tcrypt.c               |   9 +
>  crypto/testmgr.c              |  14 +
>  crypto/testmgr.h              | 497 +++++++++++++++
>  drivers/md/Kconfig            |   1 +
>  drivers/md/dm-crypt.c         | 235 ++-----
>  fs/crypto/Kconfig             |   1 +
>  fs/crypto/crypto.c            |   5 -
>  fs/crypto/fscrypt_private.h   |   9 -
>  fs/crypto/keyinfo.c           |  93 +--
>  14 files changed, 1435 insertions(+), 332 deletions(-)
>  create mode 100644 crypto/essiv.c
>
> --
> 2.17.1
>