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 <[email protected]>
---
drivers/md/Kconfig | 1 +
drivers/md/dm-crypt.c | 200 ++++----------------
2 files changed, 36 insertions(+), 165 deletions(-)
diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 45254b3ef715..30ca87cf25db 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -271,6 +271,7 @@ config DM_CRYPT
depends on BLK_DEV_DM
select CRYPTO
select CRYPTO_CBC
+ select CRYPTO_ESSIV
---help---
This device-mapper target allows you to create a device that
transparently encrypts the data on it. You'll need to activate
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index f001f1104cb5..e26a09c2d1a6 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -98,11 +98,6 @@ struct crypt_iv_operations {
struct dm_crypt_request *dmreq);
};
-struct iv_essiv_private {
- struct crypto_shash *hash_tfm;
- u8 *salt;
-};
-
struct iv_benbi_private {
int shift;
};
@@ -155,7 +150,6 @@ struct crypt_config {
const struct crypt_iv_operations *iv_gen_ops;
union {
- struct iv_essiv_private essiv;
struct iv_benbi_private benbi;
struct iv_lmk_private lmk;
struct iv_tcw_private tcw;
@@ -165,8 +159,6 @@ struct crypt_config {
unsigned short int sector_size;
unsigned char sector_shift;
- /* ESSIV: struct crypto_cipher *essiv_tfm */
- void *iv_private;
union {
struct crypto_skcipher **tfms;
struct crypto_aead **tfms_aead;
@@ -323,157 +315,15 @@ static int crypt_iv_plain64be_gen(struct crypt_config *cc, u8 *iv,
return 0;
}
-/* Initialise ESSIV - compute salt but no local memory allocations */
-static int crypt_iv_essiv_init(struct crypt_config *cc)
-{
- struct iv_essiv_private *essiv = &cc->iv_gen_private.essiv;
- SHASH_DESC_ON_STACK(desc, essiv->hash_tfm);
- struct crypto_cipher *essiv_tfm;
- int err;
-
- desc->tfm = essiv->hash_tfm;
-
- err = crypto_shash_digest(desc, cc->key, cc->key_size, essiv->salt);
- shash_desc_zero(desc);
- if (err)
- return err;
-
- essiv_tfm = cc->iv_private;
-
- err = crypto_cipher_setkey(essiv_tfm, essiv->salt,
- crypto_shash_digestsize(essiv->hash_tfm));
- if (err)
- return err;
-
- return 0;
-}
-
-/* Wipe salt and reset key derived from volume key */
-static int crypt_iv_essiv_wipe(struct crypt_config *cc)
-{
- struct iv_essiv_private *essiv = &cc->iv_gen_private.essiv;
- unsigned salt_size = crypto_shash_digestsize(essiv->hash_tfm);
- struct crypto_cipher *essiv_tfm;
- int r, err = 0;
-
- memset(essiv->salt, 0, salt_size);
-
- essiv_tfm = cc->iv_private;
- r = crypto_cipher_setkey(essiv_tfm, essiv->salt, salt_size);
- if (r)
- err = r;
-
- return err;
-}
-
-/* Allocate the cipher for ESSIV */
-static struct crypto_cipher *alloc_essiv_cipher(struct crypt_config *cc,
- struct dm_target *ti,
- const u8 *salt,
- unsigned int saltsize)
-{
- struct crypto_cipher *essiv_tfm;
- int err;
-
- /* Setup the essiv_tfm with the given salt */
- essiv_tfm = crypto_alloc_cipher(cc->cipher, 0, 0);
- if (IS_ERR(essiv_tfm)) {
- ti->error = "Error allocating crypto tfm for ESSIV";
- return essiv_tfm;
- }
-
- if (crypto_cipher_blocksize(essiv_tfm) != cc->iv_size) {
- ti->error = "Block size of ESSIV cipher does "
- "not match IV size of block cipher";
- crypto_free_cipher(essiv_tfm);
- return ERR_PTR(-EINVAL);
- }
-
- err = crypto_cipher_setkey(essiv_tfm, salt, saltsize);
- if (err) {
- ti->error = "Failed to set key for ESSIV cipher";
- crypto_free_cipher(essiv_tfm);
- return ERR_PTR(err);
- }
-
- return essiv_tfm;
-}
-
-static void crypt_iv_essiv_dtr(struct crypt_config *cc)
-{
- struct crypto_cipher *essiv_tfm;
- struct iv_essiv_private *essiv = &cc->iv_gen_private.essiv;
-
- crypto_free_shash(essiv->hash_tfm);
- essiv->hash_tfm = NULL;
-
- kzfree(essiv->salt);
- essiv->salt = NULL;
-
- essiv_tfm = cc->iv_private;
-
- if (essiv_tfm)
- crypto_free_cipher(essiv_tfm);
-
- cc->iv_private = NULL;
-}
-
-static int crypt_iv_essiv_ctr(struct crypt_config *cc, struct dm_target *ti,
- const char *opts)
-{
- struct crypto_cipher *essiv_tfm = NULL;
- struct crypto_shash *hash_tfm = NULL;
- u8 *salt = NULL;
- int err;
-
- if (!opts) {
- ti->error = "Digest algorithm missing for ESSIV mode";
- return -EINVAL;
- }
-
- /* Allocate hash algorithm */
- hash_tfm = crypto_alloc_shash(opts, 0, 0);
- if (IS_ERR(hash_tfm)) {
- ti->error = "Error initializing ESSIV hash";
- err = PTR_ERR(hash_tfm);
- goto bad;
- }
-
- salt = kzalloc(crypto_shash_digestsize(hash_tfm), GFP_KERNEL);
- if (!salt) {
- ti->error = "Error kmallocing salt storage in ESSIV";
- err = -ENOMEM;
- goto bad;
- }
-
- cc->iv_gen_private.essiv.salt = salt;
- cc->iv_gen_private.essiv.hash_tfm = hash_tfm;
-
- essiv_tfm = alloc_essiv_cipher(cc, ti, salt,
- crypto_shash_digestsize(hash_tfm));
- if (IS_ERR(essiv_tfm)) {
- crypt_iv_essiv_dtr(cc);
- return PTR_ERR(essiv_tfm);
- }
- cc->iv_private = essiv_tfm;
-
- return 0;
-
-bad:
- if (hash_tfm && !IS_ERR(hash_tfm))
- crypto_free_shash(hash_tfm);
- kfree(salt);
- return err;
-}
-
static int crypt_iv_essiv_gen(struct crypt_config *cc, u8 *iv,
struct dm_crypt_request *dmreq)
{
- struct crypto_cipher *essiv_tfm = cc->iv_private;
-
+ /*
+ * ESSIV encryption of the IV is now handled by the crypto API,
+ * so just pass the plain sector number here.
+ */
memset(iv, 0, cc->iv_size);
*(__le64 *)iv = cpu_to_le64(dmreq->iv_sector);
- crypto_cipher_encrypt_one(essiv_tfm, iv, iv);
return 0;
}
@@ -854,10 +704,6 @@ static const struct crypt_iv_operations crypt_iv_plain64be_ops = {
};
static const struct crypt_iv_operations crypt_iv_essiv_ops = {
- .ctr = crypt_iv_essiv_ctr,
- .dtr = crypt_iv_essiv_dtr,
- .init = crypt_iv_essiv_init,
- .wipe = crypt_iv_essiv_wipe,
.generator = crypt_iv_essiv_gen
};
@@ -2397,7 +2243,7 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key
char **ivmode, char **ivopts)
{
struct crypt_config *cc = ti->private;
- char *tmp, *cipher_api;
+ char *tmp, *cipher_api, buf[CRYPTO_MAX_ALG_NAME];
int ret = -EINVAL;
cc->tfms_count = 1;
@@ -2435,9 +2281,19 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key
}
ret = crypt_ctr_blkdev_cipher(cc, cipher_api);
- if (ret < 0) {
- ti->error = "Cannot allocate cipher string";
- return -ENOMEM;
+ if (ret < 0)
+ goto bad_mem;
+
+ 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,%s)",
+ cipher_api, cc->cipher, *ivopts);
+ if (ret < 0 || ret >= CRYPTO_MAX_ALG_NAME)
+ goto bad_mem;
+ cipher_api = buf;
}
cc->key_parts = cc->tfms_count;
@@ -2456,6 +2312,9 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key
cc->iv_size = crypto_skcipher_ivsize(any_tfm(cc));
return 0;
+bad_mem:
+ ti->error = "Cannot allocate cipher string";
+ return -ENOMEM;
}
static int crypt_ctr_cipher_old(struct dm_target *ti, char *cipher_in, char *key,
@@ -2515,9 +2374,20 @@ 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,%s)", chainmode, cipher,
+ cipher, *ivopts);
+ } else {
+ ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
+ "%s(%s)", chainmode, cipher);
+ }
+ if (ret < 0 || ret >= CRYPTO_MAX_ALG_NAME) {
kfree(cipher_api);
goto bad_mem;
}
--
2.20.1
On 28/06/2019 17:21, 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 <[email protected]>
> drivers/md/dm-crypt.c | 200 ++++----------------
...
> -/* Wipe salt and reset key derived from volume key */
> -static int crypt_iv_essiv_wipe(struct crypt_config *cc)
Do I understand it correctly, that this is now called inside the whole cipher
set key in wipe command (in crypt_wipe_key())?
(Wipe message is meant to suspend the device and wipe all key material
from memory without actually destroying the device.)
> -{
> - struct iv_essiv_private *essiv = &cc->iv_gen_private.essiv;
> - unsigned salt_size = crypto_shash_digestsize(essiv->hash_tfm);
> - struct crypto_cipher *essiv_tfm;
> - int r, err = 0;
> -
> - memset(essiv->salt, 0, salt_size);
> -
> - essiv_tfm = cc->iv_private;
> - r = crypto_cipher_setkey(essiv_tfm, essiv->salt, salt_size);
> - if (r)
> - err = r;
> -
> - return err;
> -}
...
> @@ -2435,9 +2281,19 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key
> }
>
> ret = crypt_ctr_blkdev_cipher(cc, cipher_api);
> - if (ret < 0) {
> - ti->error = "Cannot allocate cipher string";
> - return -ENOMEM;
> + if (ret < 0)
> + goto bad_mem;
> +
> + 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,%s)",
> + cipher_api, cc->cipher, *ivopts);
> + if (ret < 0 || ret >= CRYPTO_MAX_ALG_NAME)
> + goto bad_mem;
Hm, nitpicking, but goto from only one place while we have another -ENOMEM above...
Just place this here without goto?
> + ti->error = "Cannot allocate cipher string";
> + return -ENOMEM;
Otherwise
Reviewed-by: Milan Broz <[email protected]>
Thanks,
Milan
On Mon, 1 Jul 2019 at 10:59, Milan Broz <[email protected]> wrote:
>
> On 28/06/2019 17:21, 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 <[email protected]>
>
> > drivers/md/dm-crypt.c | 200 ++++----------------
>
> ...
>
> > -/* Wipe salt and reset key derived from volume key */
> > -static int crypt_iv_essiv_wipe(struct crypt_config *cc)
>
> Do I understand it correctly, that this is now called inside the whole cipher
> set key in wipe command (in crypt_wipe_key())?
>
> (Wipe message is meant to suspend the device and wipe all key material
> from memory without actually destroying the device.)
>
Yes, setting the random key in wipe() triggers the SHA256 operation as
normal, which is slightly wasteful but not a big deal imo.
> > -{
> > - struct iv_essiv_private *essiv = &cc->iv_gen_private.essiv;
> > - unsigned salt_size = crypto_shash_digestsize(essiv->hash_tfm);
> > - struct crypto_cipher *essiv_tfm;
> > - int r, err = 0;
> > -
> > - memset(essiv->salt, 0, salt_size);
> > -
> > - essiv_tfm = cc->iv_private;
> > - r = crypto_cipher_setkey(essiv_tfm, essiv->salt, salt_size);
> > - if (r)
> > - err = r;
> > -
> > - return err;
> > -}
>
> ...
>
> > @@ -2435,9 +2281,19 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key
> > }
> >
> > ret = crypt_ctr_blkdev_cipher(cc, cipher_api);
> > - if (ret < 0) {
> > - ti->error = "Cannot allocate cipher string";
> > - return -ENOMEM;
> > + if (ret < 0)
> > + goto bad_mem;
> > +
> > + 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,%s)",
> > + cipher_api, cc->cipher, *ivopts);
> > + if (ret < 0 || ret >= CRYPTO_MAX_ALG_NAME)
> > + goto bad_mem;
>
> Hm, nitpicking, but goto from only one place while we have another -ENOMEM above...
>
> Just place this here without goto?
>
OK
> > + ti->error = "Cannot allocate cipher string";
> > + return -ENOMEM;
>
> Otherwise
>
> Reviewed-by: Milan Broz <[email protected]>
>
> Thanks,
> Milan
On Mon, 1 Jul 2019 at 10:59, Milan Broz <[email protected]> wrote:
>
> On 28/06/2019 17:21, 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 <[email protected]>
>
> > drivers/md/dm-crypt.c | 200 ++++----------------
>
> ...
>
> > -/* Wipe salt and reset key derived from volume key */
> > -static int crypt_iv_essiv_wipe(struct crypt_config *cc)
>
> Do I understand it correctly, that this is now called inside the whole cipher
> set key in wipe command (in crypt_wipe_key())?
>
> (Wipe message is meant to suspend the device and wipe all key material
> from memory without actually destroying the device.)
>
> > -{
> > - struct iv_essiv_private *essiv = &cc->iv_gen_private.essiv;
> > - unsigned salt_size = crypto_shash_digestsize(essiv->hash_tfm);
> > - struct crypto_cipher *essiv_tfm;
> > - int r, err = 0;
> > -
> > - memset(essiv->salt, 0, salt_size);
> > -
> > - essiv_tfm = cc->iv_private;
> > - r = crypto_cipher_setkey(essiv_tfm, essiv->salt, salt_size);
> > - if (r)
> > - err = r;
> > -
> > - return err;
> > -}
>
> ...
>
> > @@ -2435,9 +2281,19 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key
> > }
> >
> > ret = crypt_ctr_blkdev_cipher(cc, cipher_api);
> > - if (ret < 0) {
> > - ti->error = "Cannot allocate cipher string";
> > - return -ENOMEM;
> > + if (ret < 0)
> > + goto bad_mem;
> > +
> > + 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,%s)",
> > + cipher_api, cc->cipher, *ivopts);
> > + if (ret < 0 || ret >= CRYPTO_MAX_ALG_NAME)
> > + goto bad_mem;
>
> Hm, nitpicking, but goto from only one place while we have another -ENOMEM above...
>
> Just place this here without goto?
>
Actually, the bad_mem label is used 10 lines up as well.
So I'll keep this goto in the next revision.
> > + ti->error = "Cannot allocate cipher string";
> > + return -ENOMEM;
>
> Otherwise
>
> Reviewed-by: Milan Broz <[email protected]>
>
> Thanks,
> Milan