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

[Eric Biggers <ebiggers@kernel.org>]

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.

> , 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@linux.alibaba.com>
> ---
>  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?)

- Eric