Re: [f2fs-dev] [PATCH 6/9] dm crypt: support diskcipher

Re: [f2fs-dev] [PATCH 6/9] dm crypt: support diskcipher

[boojin.kim]

On Wed, Aug 21, 2019 at 17:19:41PM +0900, Herbert Xu wrote:

> On Wed, Aug 21, 2019 at 04:57:41PM +0900, boojin.kim wrote:
> >
> > Can you tell me which patch you mentioned? Is this?
> > https://patches.linaro.org/project/linux-crypto/list/?series=22762
> >
>
> Yes this is the one.
>
> Cheers,

I looked at ESSIV patch-set.
Can you please confirm that you mean:
'Register FMP as the template of skcipher instead of diskcipher,
and simplify the interface for FMP user'

If yes, I think the following API needs to be added to skcipher:  
- _set(): BIO submitter (dm-crypt, f2fs, ext4) sets cipher to BIO.
- _mergeable(): Block layer checks if two BIOs have the same cipher.
- _get(): Storage driver gets cipher from BIO.
- _set_crypt(): Storage driver gets crypto information from cipher and 
writes it on the descriptor of Storage controller.
Is it acceptable to skcipher ?




_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 6/9] dm crypt: support diskcipher

[Herbert Xu]

On Fri, Aug 23, 2019 at 01:20:37PM +0900, boojin.kim wrote:
>
> If yes, I think the following API needs to be added to skcipher:  
> - _set(): BIO submitter (dm-crypt, f2fs, ext4) sets cipher to BIO.
> - _mergeable(): Block layer checks if two BIOs have the same cipher.
> - _get(): Storage driver gets cipher from BIO.
> - _set_crypt(): Storage driver gets crypto information from cipher and 
> writes it on the descriptor of Storage controller.
> Is it acceptable to skcipher ?

No.  If you're after total offload then the crypto API is not for
you.  What we can support is the offloading of encryption/decryption
over many sectors.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 6/9] dm crypt: support diskcipher

[boojin.kim]

On Fri, Aug 23, 2019 at 01:28:37PM +0900, Herbert Xu wrote:
>
> No.  If you're after total offload then the crypto API is not for
> you.  What we can support is the offloading of encryption/decryption
> over many sectors.
>
> Cheers,

FMP doesn't use encrypt/decrypt of crypto API because it doesn't
expose cipher-text to DRAM.
But, Crypto API has many useful features such as cipher management,
cipher allocation with cipher name, key management and test manager.
All these features are useful for FMP.
FMP has been cerified with FIPS as below by using test vectors and
test manager of Crypto API.
https://csrc.nist.gov/projects/cryptographic-module-validation-program/Certi
ficate/3255
https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-pr
ogram/documents/security-policies/140sp3255.pdf

Can't I use crypto APIs to take advantage of this?
I want to find a good way that FMP can use crypto API.

Thanks
Boojin Kim.



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 6/9] dm crypt: support diskcipher

[Herbert Xu]

On Wed, Aug 21, 2019 at 04:57:41PM +0900, boojin.kim wrote:
>
> Can you tell me which patch you mentioned? Is this?
> https://patches.linaro.org/project/linux-crypto/list/?series=22762

Yes this is the one.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 6/9] dm crypt: support diskcipher

[boojin.kim]

On Wed, Aug 21, 2019 at 09:35:36AM +0200, Herbert Xu Herbert wrote:

> I agree.  Please take a look at the recent ESSIV patches on
> linux-crypto and build multi-block operations on top of them
> which can then be implemented by the hardware.
>
> Cheers,

Can you tell me which patch you mentioned? Is this?
https://patches.linaro.org/project/linux-crypto/list/?series=22762



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 6/9] dm crypt: support diskcipher

[boojin.kim]

On Wed, Aug 21, 2019 at 09:13:36AM +0200, Milan Broz wrote: 
>
> NACK.
> 
> The whole principle of dm-crypt target is that it NEVER EVER submits
> plaintext data down the stack in bio.
> 
> If you want to do some lower/higher layer encryption, use key management
> on a different layer.
> So here, just setup encryption for fs, do not stack it with dm-crypt.
> 
> Also, dm-crypt is software-independent solution
> (software-based full disk encryption), it must not depend on
> any underlying hardware.
> Hardware can be of course used used for acceleration, but then
> just implement proper crypto API module that accelerates particular
cipher.

I'm sorry for breaking the basic rules of dm-crypt. 
But, if I want to use the H/W crypto accelerator running in storage
controller,
I have to drop plaintext to bio.
I think the "proper crypto API module" that you mentioned is diskcipher
because diskcipher isn't only for FMP.
Diskcipher is a crypto API that supports encryption on storage controllers.

Thanks
Boojin Kim



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 6/9] dm crypt: support diskcipher

[boojin.kim]

This patch supports dm-crypt to use diskcipher in a specific ivmode
(disk or fmp).
Dm-crypt allocates diskcipher and sets the key on it.
Then, dm-crypt sets diskcipher into BIO and submits the BIO without
any additional data encryption.

Cc: Alasdair Kergon <agk@redhat.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: dm-devel@redhat.com
Signed-off-by: Boojin Kim <boojin.kim@samsung.com>
---
 drivers/md/dm-crypt.c | 112
++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 103 insertions(+), 9 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 9f8b654..271cfcc 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -37,6 +37,7 @@
 #include <keys/user-type.h>
 
 #include <linux/device-mapper.h>
+#include <crypto/diskcipher.h>
 
 #define DM_MSG_PREFIX "crypt"
 
@@ -130,6 +131,8 @@ enum flags { DM_CRYPT_SUSPENDED, DM_CRYPT_KEY_VALID,
 enum cipher_flags {
 	CRYPT_MODE_INTEGRITY_AEAD,	/* Use authenticated mode for cihper
*/
 	CRYPT_IV_LARGE_SECTORS,		/* Calculate IV from sector_size,
not 512B sectors */
+	CRYPT_MODE_DISKCIPHER,
+	CRYPT_MODE_SKCIPHER,
 };
 
 /*
@@ -170,6 +173,7 @@ struct crypt_config {
 	union {
 		struct crypto_skcipher **tfms;
 		struct crypto_aead **tfms_aead;
+		struct crypto_diskcipher **tfms_diskc;
 	} cipher_tfm;
 	unsigned tfms_count;
 	unsigned long cipher_flags;
@@ -955,6 +959,17 @@ static bool crypt_integrity_hmac(struct crypt_config
*cc)
 	return crypt_integrity_aead(cc) && cc->key_mac_size;
 }
 
+static bool crypt_mode_diskcipher(struct crypt_config *cc)
+{
+	return test_bit(CRYPT_MODE_DISKCIPHER, &cc->cipher_flags);
+}
+
+static bool crypt_mode_skcipher(struct crypt_config *cc)
+{
+	return test_bit(CRYPT_MODE_SKCIPHER, &cc->cipher_flags);
+}
+
+
 /* Get sg containing data */
 static struct scatterlist *crypt_get_sg_data(struct crypt_config *cc,
 					     struct scatterlist *sg)
@@ -1573,13 +1588,13 @@ static void crypt_endio(struct bio *clone)
 	/*
 	 * free the processed pages
 	 */
-	if (rw == WRITE)
+	if ((rw == WRITE) && !crypt_mode_diskcipher(cc))
 		crypt_free_buffer_pages(cc, clone);
 
 	error = clone->bi_status;
 	bio_put(clone);
 
-	if (rw == READ && !error) {
+	if (rw == READ && !error && !crypt_mode_diskcipher(cc)) {
 		kcryptd_queue_crypt(io);
 		return;
 	}
@@ -1618,6 +1633,11 @@ static int kcryptd_io_read(struct dm_crypt_io *io,
gfp_t gfp)
 	crypt_inc_pending(io);
 
 	clone_init(io, clone);
+
+	if (crypt_mode_diskcipher(cc))
+		crypto_diskcipher_set(clone,
+			cc->cipher_tfm.tfms_diskc[0], NULL, 0);
+
 	clone->bi_iter.bi_sector = cc->start + io->sector;
 
 	if (dm_crypt_integrity_io_alloc(io, clone)) {
@@ -1907,10 +1927,29 @@ static void crypt_free_tfms_skcipher(struct
crypt_config *cc)
 	cc->cipher_tfm.tfms = NULL;
 }
 
+static void crypt_free_tfms_diskcipher(struct crypt_config *cc)
+{
+	if (!crypt_mode_diskcipher(cc))
+		return;
+
+	if (cc->cipher_tfm.tfms_diskc[0] &&
+		!IS_ERR(cc->cipher_tfm.tfms_diskc[0])) {
+		crypto_diskcipher_clearkey(cc->cipher_tfm.tfms_diskc[0]);
+		crypto_free_diskcipher(cc->cipher_tfm.tfms_diskc[0]);
+		cc->cipher_tfm.tfms_diskc[0] = NULL;
+	}
+
+	kfree(cc->cipher_tfm.tfms_diskc);
+	cc->cipher_tfm.tfms_diskc = NULL;
+}
+
+
 static void crypt_free_tfms(struct crypt_config *cc)
 {
 	if (crypt_integrity_aead(cc))
 		crypt_free_tfms_aead(cc);
+	else if (crypt_mode_diskcipher(cc))
+		crypt_free_tfms_diskcipher(cc);
 	else
 		crypt_free_tfms_skcipher(cc);
 }
@@ -1934,6 +1973,7 @@ static int crypt_alloc_tfms_skcipher(struct
crypt_config *cc, char *ciphermode)
 			return err;
 		}
 	}
+	set_bit(CRYPT_MODE_SKCIPHER, &cc->cipher_flags);
 
 	/*
 	 * dm-crypt performance can vary greatly depending on which crypto
@@ -1965,10 +2005,34 @@ static int crypt_alloc_tfms_aead(struct crypt_config
*cc, char *ciphermode)
 	return 0;
 }
 
+static int crypt_alloc_tfms_diskcipher(struct crypt_config *cc,
+				char *ciphermode)
+{
+	int err;
+
+	cc->cipher_tfm.tfms = kmalloc(sizeof(struct crypto_aead *),
GFP_KERNEL);
+	if (!cc->cipher_tfm.tfms)
+		return -ENOMEM;
+
+	cc->cipher_tfm.tfms_diskc[0] =
+	    crypto_alloc_diskcipher(ciphermode, 0, 0, 1);
+	if (IS_ERR(cc->cipher_tfm.tfms_diskc[0])) {
+		err = PTR_ERR(cc->cipher_tfm.tfms_diskc[0]);
+		crypt_free_tfms(cc);
+		pr_err("%s: no diskcipher with %s\n", __func__, ciphermode);
+		return err;
+	}
+	pr_info("%s is done with %s\n", __func__, ciphermode);
+
+	return 0;
+}
+
 static int crypt_alloc_tfms(struct crypt_config *cc, char *ciphermode)
 {
 	if (crypt_integrity_aead(cc))
 		return crypt_alloc_tfms_aead(cc, ciphermode);
+	else if (crypt_mode_diskcipher(cc))
+		return crypt_alloc_tfms_diskcipher(cc, ciphermode);
 	else
 		return crypt_alloc_tfms_skcipher(cc, ciphermode);
 }
@@ -2030,6 +2094,11 @@ static int crypt_setkey(struct crypt_config *cc)
 			r = crypto_aead_setkey(cc->cipher_tfm.tfms_aead[i],
 					       cc->key + (i * subkey_size),
 					       subkey_size);
+		else if (crypt_mode_diskcipher(cc))
+			r = crypto_diskcipher_setkey(
+
cc->cipher_tfm.tfms_diskc[i],
+						cc->key + (i * subkey_size),
+						subkey_size, 1);
 		else
 			r = crypto_skcipher_setkey(cc->cipher_tfm.tfms[i],
 						   cc->key + (i *
subkey_size),
@@ -2510,7 +2579,7 @@ static int crypt_ctr_cipher_new(struct dm_target *ti,
char *cipher_in, char *key
 			return -ENOMEM;
 		}
 		cc->iv_size = crypto_aead_ivsize(any_tfm_aead(cc));
-	} else
+	} else if (crypt_mode_skcipher(cc))
 		cc->iv_size = crypto_skcipher_ivsize(any_tfm(cc));
 
 	ret = crypt_ctr_blkdev_cipher(cc);
@@ -2560,6 +2629,9 @@ static int crypt_ctr_cipher_old(struct dm_target *ti,
char *cipher_in, char *key
 	chainmode = strsep(&tmp, "-");
 	*ivmode = strsep(&tmp, ":");
 	*ivopts = tmp;
+	if (*ivmode)
+		if (!strcmp(*ivmode, "disk") || !strcmp(*ivmode, "fmp"))
+			set_bit(CRYPT_MODE_DISKCIPHER, &cc->cipher_flags);
 
 	/*
 	 * For compatibility with the original dm-crypt mapping format, if
@@ -2621,9 +2693,11 @@ static int crypt_ctr_cipher(struct dm_target *ti,
char *cipher_in, char *key)
 		return ret;
 
 	/* Initialize IV */
-	ret = crypt_ctr_ivmode(ti, ivmode);
-	if (ret < 0)
-		return ret;
+	if (!crypt_mode_diskcipher(cc)) {
+		ret = crypt_ctr_ivmode(ti, ivmode);
+		if (ret < 0)
+			return ret;
+	}
 
 	/* Initialize and set key */
 	ret = crypt_set_key(cc, key);
@@ -2654,6 +2728,11 @@ static int crypt_ctr_cipher(struct dm_target *ti,
char *cipher_in, char *key)
 	if (cc->key_string)
 		memset(cc->key, 0, cc->key_size * sizeof(u8));
 
+	pr_info("%s with ivmode:%s, ivopts:%s, aead:%d, diskcipher:%d(%p),
skcipher:%d\n",
+			__func__, ivmode, ivopts, crypt_integrity_aead(cc),
+			crypt_mode_diskcipher(cc),
cc->cipher_tfm.tfms_diskc[0],
+			crypt_mode_skcipher(cc));
+
 	return ret;
 }
 
@@ -2788,11 +2867,15 @@ static int crypt_ctr(struct dm_target *ti, unsigned
int argc, char **argv)
 	ret = crypt_ctr_cipher(ti, argv[0], argv[1]);
 	if (ret < 0)
 		goto bad;
-
 	if (crypt_integrity_aead(cc)) {
 		cc->dmreq_start = sizeof(struct aead_request);
 		cc->dmreq_start += crypto_aead_reqsize(any_tfm_aead(cc));
 		align_mask = crypto_aead_alignmask(any_tfm_aead(cc));
+	} else if (crypt_mode_diskcipher(cc)) {
+		cc->per_bio_data_size = ti->per_io_data_size =
+			ALIGN(sizeof(struct dm_crypt_io),
+			ARCH_KMALLOC_MINALIGN);
+		goto get_bio;
 	} else {
 		cc->dmreq_start = sizeof(struct skcipher_request);
 		cc->dmreq_start += crypto_skcipher_reqsize(any_tfm(cc));
@@ -2836,6 +2919,7 @@ static int crypt_ctr(struct dm_target *ti, unsigned
int argc, char **argv)
 		goto bad;
 	}
 
+get_bio:
 	ret = bioset_init(&cc->bs, MIN_IOS, 0, BIOSET_NEED_BVECS);
 	if (ret) {
 		ti->error = "Cannot allocate crypt bioset";
@@ -2893,6 +2977,12 @@ static int crypt_ctr(struct dm_target *ti, unsigned
int argc, char **argv)
 		goto bad;
 	}
 
+	if (crypt_mode_diskcipher(cc)) {
+		cc->crypt_queue = NULL;
+		cc->write_thread = NULL;
+		goto out;
+	}
+
 	if (test_bit(DM_CRYPT_SAME_CPU, &cc->flags))
 		cc->crypt_queue = alloc_workqueue("kcryptd/%s",
 						  WQ_HIGHPRI |
WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM,
@@ -2918,6 +3008,7 @@ static int crypt_ctr(struct dm_target *ti, unsigned
int argc, char **argv)
 	}
 	wake_up_process(cc->write_thread);
 
+out:
 	ti->num_flush_bios = 1;
 
 	return 0;
@@ -2981,10 +3072,10 @@ static int crypt_map(struct dm_target *ti, struct
bio *bio)
 
 	if (crypt_integrity_aead(cc))
 		io->ctx.r.req_aead = (struct aead_request *)(io + 1);
-	else
+	else if (crypt_mode_skcipher(cc))
 		io->ctx.r.req = (struct skcipher_request *)(io + 1);
 
-	if (bio_data_dir(io->base_bio) == READ) {
+	if ((bio_data_dir(io->base_bio) == READ) ||
crypt_mode_diskcipher(cc)) {
 		if (kcryptd_io_read(io, GFP_NOWAIT))
 			kcryptd_queue_read(io);
 	} else
@@ -3143,6 +3234,9 @@ static void crypt_io_hints(struct dm_target *ti,
struct queue_limits *limits)
 	limits->physical_block_size =
 		max_t(unsigned, limits->physical_block_size,
cc->sector_size);
 	limits->io_min = max_t(unsigned, limits->io_min, cc->sector_size);
+
+	if (crypt_mode_diskcipher(cc))
+		limits->logical_block_size = PAGE_SIZE;
 }
 
 static struct target_type crypt_target = {
-- 
2.7.4



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel