linux-ext4 - Re: [PATCH V2 3/7] fscrypt: remove filesystem specific build config option

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]

Date:   Mon, 10 Dec 2018 10:04:22 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Chandan Rajendra <chandan@...ux.vnet.ibm.com>
Cc:     linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        linux-doc@...r.kernel.org, linux-mips@...ux-mips.org,
        linux-s390@...r.kernel.org, linux-mtd@...ts.infradead.org,
        linux-fsdevel@...r.kernel.org, tytso@....edu,
        adilger.kernel@...ger.ca, jaegeuk@...nel.org, yuchao0@...wei.com,
        corbet@....net, ralf@...ux-mips.org, paul.burton@...s.com,
        jhogan@...nel.org, green.hu@...il.com, deanbo422@...il.com,
        schwidefsky@...ibm.com, heiko.carstens@...ibm.com, richard@....at,
        dedekind1@...il.com, adrian.hunter@...el.com,
        viro@...iv.linux.org.uk
Subject: Re: [PATCH V2 3/7] fscrypt: remove filesystem specific build config
 option

On Sat, Dec 08, 2018 at 12:37:20PM +0530, Chandan Rajendra wrote:
> On Wednesday, December 5, 2018 5:13:21 AM IST Eric Biggers wrote:
> > Hi Chandan,
> > 
> > On Tue, Dec 04, 2018 at 03:26:46PM +0530, Chandan Rajendra wrote:
> > > In order to have a common code base for fscrypt "post read" processing
> > > for all filesystems which support encryption, this commit removes
> > > filesystem specific build config option (e.g. CONFIG_EXT4_FS_ENCRYPTION)
> > > and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose
> > > value affects all the filesystems making use of fscrypt.
> > > 
> > > Signed-off-by: Chandan Rajendra <chandan@...ux.vnet.ibm.com>
> > [...]
> > > -config F2FS_FS_ENCRYPTION
> > > -	bool "F2FS Encryption"
> > > -	depends on F2FS_FS
> > > -	depends on F2FS_FS_XATTR
> > > -	select FS_ENCRYPTION
> > > -	help
> > > -	  Enable encryption of f2fs files and directories.  This
> > > -	  feature is similar to ecryptfs, but it is more memory
> > > -	  efficient since it avoids caching the encrypted and
> > > -	  decrypted pages in the page cache.
> > > -
> > [...]
> > > -config UBIFS_FS_ENCRYPTION
> > > -	bool "UBIFS Encryption"
> > > -	depends on UBIFS_FS && UBIFS_FS_XATTR && BLOCK
> > > -	select FS_ENCRYPTION
> > > -	default n
> > > -	help
> > > -	  Enable encryption of UBIFS files and directories. This
> > > -	  feature is similar to ecryptfs, but it is more memory
> > > -	  efficient since it avoids caching the encrypted and
> > > -	  decrypted pages in the page cache.
> > 
> > Will it cause problems that now f2fs encryption can be "enabled" without
> > F2FS_FS_XATTR, and ubifs encryption without UBIFS_FS_XATTR && BLOCK?
> > 
> > Otherwise I think this patch looks fine.  I'm a bit concerned about the bloat
> > from making FS_ENCRYPTION non-modular, but given that it will make sharing I/O
> > code much easier, it's probably worthwhile.
> > 
> > It would help to strip down the dependencies of FS_ENCRYPTION to just the stuff
> > needed for just AES-256-XTS and AES-256-CTS.  I already sent out a patch a
> > couple months ago (https://patchwork.kernel.org/patch/10589319/) to remove
> > CONFIG_CTR which isn't used at all; I'll remind Ted to apply that.  But we could
> > also drop CONFIG_SHA256, which is only needed for AES-128-CBC contents
> > encryption.  If we do that, it should be a separate patch, though.
> 
> Hi Eric,
> 
> fscrypt_valid_enc_modes() allows FS_ENCRYPTION_MODE_AES_128_CBC to be used for
> encryption of file's contents. This is consistent with what you had mentioned
> above.
> 
> static inline bool fscrypt_valid_enc_modes(u32 contents_mode,
>                                            u32 filenames_mode)
> {
>         if (contents_mode == FS_ENCRYPTION_MODE_AES_128_CBC &&
>             filenames_mode == FS_ENCRYPTION_MODE_AES_128_CTS)
>                 return true;
> 
>         if (contents_mode == FS_ENCRYPTION_MODE_AES_256_XTS &&
>             filenames_mode == FS_ENCRYPTION_MODE_AES_256_CTS)
>                 return true;
> 
>         return false;
> }
> 
> Hence FS_ENCRYPTION does need to have AES-128-CBC and by extension SHA256 code
> compiled in right? 
> 

No those algorithms don't have to be compiled in if userspace doesn't use the
(FS_ENCRYPTION_MODE_AES_128_CBC, FS_ENCRYPTION_MODE_AES_128_CTS) pair, because
the algorithms are allocated dynamically on-demand through the crypto API.

- Eric