Re: [PATCH 03/36] fscrypt: export fscrypt_fname_encrypt and fscrypt_fname_encrypted_size

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Dec 09, 2021 at 10:36:14AM -0500, Jeff Layton wrote:
> For ceph, we want to use our own scheme for handling filenames that are
> are longer than NAME_MAX after encryption and base64 encoding. This

base64 => Base64.  (base64 and base64url are types of Base64.)

> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 8fa23d525b5c..3be04b5aa570 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -130,6 +130,7 @@ int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL(fscrypt_fname_encrypt);

The documentation for the @inode parameter could use a mention that the inode's
key must have already been set up.  External callers could get that wrong.

Also, I'd prefer EXPORT_SYMBOL_GPL for anything that isn't generic functionality
like Base64 encoding/decoding.

> +/**
> + * fscrypt_fname_encrypted_size() - calculate length of encrypted filename
> + * @inode: 		parent inode of dentry name being encrypted

Likewise, this should mention that the inode's key must have already been set
up.

> + * Filenames must be padded out to at least the end of an fscrypt block before
> + * encrypting them.

That's not really correct.  The padding amount depends on the padding flags, as
well as whether the filename gets truncated at max_len or not.  Also there's not
really any such thing as an "fscrypt block".  (FS_CRYPTO_BLOCK_SIZE, which is 16
bytes, is misnamed.  It really should be two separate things like
FSCRYPT_MIN_FNAME_CTEXT_SIZE and FSCRYPT_CONTENTS_CTEXT_ALIGNMENT.)

How about just writing something like:

    Filenames that are shorter than the maximum length may have their lengths
    increased slightly by encryption, due to padding that is applied.

> + *
> + * Return: false if the orig_len is shorter than max_len. Otherwise, true and
> + * 	   fill out encrypted_len_ret with the length (up to max_len).

false if orig_len is *greater* than max_len.

> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 5b0a9e6478b5..51e42767dbd6 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -297,14 +297,11 @@ void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num,
>  			 const struct fscrypt_info *ci);
>  
>  /* fname.c */
> -int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
> -			  u8 *out, unsigned int olen);
> -bool fscrypt_fname_encrypted_size(const union fscrypt_policy *policy,
> -				  u32 orig_len, u32 max_len,
> -				  u32 *encrypted_len_ret);
> +bool __fscrypt_fname_encrypted_size(const union fscrypt_policy *policy,
> +				    u32 orig_len, u32 max_len,
> +                                    u32 *encrypted_len_ret);

This is indented with spaces, not tabs.

- Eric