Re: [PATCH 2/2] fscrypt: Add SM4 XTS/CTS symmetric algorithm support

[Tianjia Zhang <tianjia.zhang@xxxxxxxxxxxxxxxxx>]

Hi Eric,

On 11/17/22 1:26 AM, Eric Biggers wrote:
> On Wed, Nov 16, 2022 at 04:24:16PM +0800, Tianjia Zhang wrote:
> > SM4 is a symmetric algorithm widely used in China
>
> So?
>
> What is the use case for adding this to fscrypt specifically?
>
> Just because an algorithm is widely used doesn't necessarily mean it is useful
> or appropriate to support with fscrypt.

We want to provide our users with the ability to encrypt disks and files
using SM4-XTS, the ability to sign SM2/3, and the ability to use
SM4-GCM/CCM with TLS (of course this belongs to other parts), quite a
few users need these features.

> > , this patch enables
> > to use SM4-XTS mode to encrypt file content, and use SM4-CBC-CTS to
> > encrypt filename.
> >
> > Signed-off-by: Tianjia Zhang <tianjia.zhang@xxxxxxxxxxxxxxxxx>
> > ---
> >   Documentation/filesystems/fscrypt.rst |  1 +
> >   fs/crypto/fscrypt_private.h           |  2 +-
> >   fs/crypto/keysetup.c                  | 15 +++++++++++++++
> >   fs/crypto/policy.c                    |  4 ++++
> >   include/uapi/linux/fscrypt.h          |  4 +++-
> >   5 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
> > index 5ba5817c17c2..af27e7b2c74f 100644
> > --- a/Documentation/filesystems/fscrypt.rst
> > +++ b/Documentation/filesystems/fscrypt.rst
> > @@ -336,6 +336,7 @@ Currently, the following pairs of encryption modes are supported:
> >   
> >   - AES-256-XTS for contents and AES-256-CTS-CBC for filenames
> >   - AES-128-CBC for contents and AES-128-CTS-CBC for filenames
> > +- SM4-XTS for contents and SM4-CTS-CBC for filenames
> >   - Adiantum for both contents and filenames
> >   - AES-256-XTS for contents and AES-256-HCTR2 for filenames (v2 policies only)
> >   
> > diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> > index d5f68a0c5d15..e79a701de028 100644
> > --- a/fs/crypto/fscrypt_private.h
> > +++ b/fs/crypto/fscrypt_private.h
> > @@ -31,7 +31,7 @@
> >   #define FSCRYPT_CONTEXT_V2	2
> >   
> >   /* Keep this in sync with include/uapi/linux/fscrypt.h */
> > -#define FSCRYPT_MODE_MAX	FSCRYPT_MODE_AES_256_HCTR2
> > +#define FSCRYPT_MODE_MAX	FSCRYPT_MODE_SM4_CTS
> >   
> >   struct fscrypt_context_v1 {
> >   	u8 version; /* FSCRYPT_CONTEXT_V1 */
> > diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> > index f7407071a952..c0a3f882f5a4 100644
> > --- a/fs/crypto/keysetup.c
> > +++ b/fs/crypto/keysetup.c
> > @@ -59,6 +59,21 @@ struct fscrypt_mode fscrypt_modes[] = {
> >   		.security_strength = 32,
> >   		.ivsize = 32,
> >   	},
> > +	[FSCRYPT_MODE_SM4_XTS] = {
> > +		.friendly_name = "SM4-XTS",
> > +		.cipher_str = "xts(sm4)",
> > +		.keysize = 32,
> > +		.security_strength = 16,
> > +		.ivsize = 16,
> > +		.blk_crypto_mode = BLK_ENCRYPTION_MODE_SM4_XTS,
> > +	},
> > +	[FSCRYPT_MODE_SM4_CTS] = {
> > +		.friendly_name = "SM4-CTS",
> > +		.cipher_str = "cts(cbc(sm4))",
> > +		.keysize = 16,
> > +		.security_strength = 16,
> > +		.ivsize = 16,
> > +	},
> >   };
> >   
> >   static DEFINE_MUTEX(fscrypt_mode_key_setup_mutex);
> > diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
> > index 46757c3052ef..4881fd3af6ee 100644
> > --- a/fs/crypto/policy.c
> > +++ b/fs/crypto/policy.c
> > @@ -75,6 +75,10 @@ static bool fscrypt_valid_enc_modes_v1(u32 contents_mode, u32 filenames_mode)
> >   	    filenames_mode == FSCRYPT_MODE_ADIANTUM)
> >   		return true;
> >   
> > +	if (contents_mode == FSCRYPT_MODE_SM4_XTS &&
> > +	    filenames_mode == FSCRYPT_MODE_SM4_CTS)
> > +		return true;
> > +
> >   	return false;
> >   }
> >   
> > diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
> > index a756b29afcc2..34d791bd162c 100644
> > --- a/include/uapi/linux/fscrypt.h
> > +++ b/include/uapi/linux/fscrypt.h
> > @@ -28,7 +28,9 @@
> >   #define FSCRYPT_MODE_AES_128_CTS		6
> >   #define FSCRYPT_MODE_ADIANTUM			9
> >   #define FSCRYPT_MODE_AES_256_HCTR2		10
> > -/* If adding a mode number > 10, update FSCRYPT_MODE_MAX in fscrypt_private.h */
> > +#define FSCRYPT_MODE_SM4_XTS			11
> > +#define FSCRYPT_MODE_SM4_CTS			12
> > +/* If adding a mode number > 12, update FSCRYPT_MODE_MAX in fscrypt_private.h */
>
> This might be a good time to reclaim some of the unused mode numbers.  Maybe 7-8
> which were very briefly used for Speck128/256.  (Irony not lost?)

This looks awesome, I'll reclaim the gaps in the next version if
possible.

Cheers,
Tianjia