linux-ext4 - Re: [PATCH v9 09/11] fscrypt: add inline encryption support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200326054536.GD858@sol.localdomain>
Date:   Wed, 25 Mar 2020 22:45:36 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Satya Tangirala <satyat@...gle.com>
Cc:     linux-block@...r.kernel.org, linux-scsi@...r.kernel.org,
        linux-fscrypt@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net, linux-ext4@...r.kernel.org,
        Barani Muthukumaran <bmuthuku@....qualcomm.com>,
        Kuohong Wang <kuohong.wang@...iatek.com>,
        Kim Boojin <boojin.kim@...sung.com>
Subject: Re: [PATCH v9 09/11] fscrypt: add inline encryption support

On Wed, Mar 25, 2020 at 08:07:00PM -0700, Satya Tangirala wrote:
> +/* Enable inline encryption for this file if supported. */
> +void fscrypt_select_encryption_impl(struct fscrypt_info *ci)
> +{
> +	const struct inode *inode = ci->ci_inode;
> +	struct super_block *sb = inode->i_sb;
> +
> +	/* The file must need contents encryption, not filenames encryption */
> +	if (!fscrypt_needs_contents_encryption(inode))
> +		return;
> +
> +	/* blk-crypto must implement the needed encryption algorithm */
> +	if (ci->ci_mode->blk_crypto_mode == BLK_ENCRYPTION_MODE_INVALID)
> +		return;
> +
> +	/* The filesystem must be mounted with -o inlinecrypt */
> +	if (!(sb->s_flags & SB_INLINECRYPT))
> +		return;
> +
> +	ci->ci_inlinecrypt = true;
> +}

A bug I came across last week when writing a new test is that '-o inlinecrypt'
can break some fscrypt settings because it enables blk-crypto even when
CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK is unset and the hardware doesn't support
the algorithm.  For example, adding '-o inlinecrypt' can make Adiantum-encrypted
files stop working, due to the hardware only supporting AES-XTS.

That's undesirable.  Adding '-o inlinecrypt' should just make inline encryption
be used where it can, and not break anything.

To fix this, we should make fscrypt_select_encryption_impl() only set
->ci_inlinecrypt if either blk-crypto-fallback is enabled or if all the
filesystem's devices support the algorithm.

In v7+ of this patchset, this is a bit tricky because now
blk_ksm_crypto_key_supported() takes in a 'struct blk_crypto_key', which
fscrypt_select_encryption_impl() doesn't have available yet.  Perhaps make
blk_ksm_crypto_key_supported() a wrapper around a function like
blk_ksm_crypto_setting_supported() that takes a new struct:

	struct blk_crypto_setting {
	        enum blk_crypto_mode_num crypto_mode;
		unsigned int data_unit_size;
		unsigned int dun_bytes;
	};

Then maybe add blk_crypto_setting_supported() which returns true if either
blk_ksm_crypto_key_supported() *or* blk-crypto-fallback is enabled.

- Eric