Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 10 Nov 2022 17:47:10 +0100
From:   Niels de Vos <ndevos@redhat.com>
To:     Theodore Ts'o <tytso@mit.edu>
Cc:     linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, Xiubo Li <xiubli@redhat.com>,
        Marcel Lauhoff <marcel.lauhoff@suse.com>
Subject: Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt
Message-ID: <Y20rDl45vSmdEo3N@ndevos-x1>
References: <20221110141225.2308856-1-ndevos@redhat.com>
 <Y20a/akbY8Wcy3qg@mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y20a/akbY8Wcy3qg@mit.edu>
Precedence: bulk

On Thu, Nov 10, 2022 at 10:38:37AM -0500, Theodore Ts'o wrote:
> On Thu, Nov 10, 2022 at 03:12:21PM +0100, Niels de Vos wrote:
> > While more filesystems are getting support for fscrypt, it is useful to
> > be able to disable fscrypt for a selection of filesystems, while
> > enabling it for others.
> 
> Could you say why you find it useful?  Is it because you are concerned
> about the increased binary size of a particular file system if fscrypt
> is enabled?  That hasn't been my experience, the hooks to call into
> fscrypt are small and don't add too much to any one particular file
> system; the bulk of the code is in fs/crypto.

No, this isn't really a concern. I don't think the small size difference
is worth the effort.

> Is it because people are pushing buggy code that doesn't compile if
> you enable, say, CONFIG_FS_XXX and CONFIG_FSCRYPT at the same time?

It is a little of this. Not necessarily for the compiling part, more of
a functional thing. And that is what comes next...

> Is it because a particular distribution doesn't want to support
> fscrypt with a particular file system?  If so, there have been plenty
> of file system features for say, ext4, xfs, and btrfs, which aren't
> supported by a distro, but there isn't a CONFIG_FS_XXX_YYY to disable
> that feature, nor have any distros requested such a thing --- which is
> good because it would be an explosion of new CONFIG parameters.

This is mostly why I sent this RFC. We are interested in enabling
fscrypt for CephFS (soonish) as a network filesystem, but not for local
filesystems (we recommend dm-crypt for those). The idea is that
functionality that isn't available, can also not (easily) cause
breakage.

And, there actually are options like CONFIG_EXT4_FS_POSIX_ACL and
CONFIG_EXT4_FS_SECURITY. Because these exist already, I did not expect
too much concerns with proposing a CONFIG_EXT4_FS_ENCRYPTION...

> Or is it something else?
> 
> Note that nearly all of the file systems will only enable fscrypt if
> some file system feature flag enabls it.  So I'm not sure what's the
> motivation behind adding this configuration option.  If memory serves,
> early in the fscrypt development we did have per-file system CONFIG's
> for fscrypt, but we consciously removed it, just as we no longer have
> per-file system CONFIG's to enable or disable Posix ACL's or extended
> attributes, in the name of simplifying the kernel config.

Thanks for adding some history about this. I understand that extra
options are needed while creating/tuning the filesystems. Preventing
users from setting the right options in a filesystem is not easy, even
if tools from a distribution do not offer setting the options. Disks can
be portable, or network-attached, and have options enabled that an other
distributions kernel does not (want to) support.

Note that even with the additional options, enabling only
CONFIG_FS_ENCRYPTION causes all the filesystems that support fscrypt to
have it enabled. For users there is no change, except that they now have
an option to disable fscrypt support per filesystem.

I hope this explains the intention a bit better. And as there are
existing options for many filesystems to select support for xattrs,
security or ACLs, I hope new options for (de)selecting filesystem
encryption support is not too controversial.

Thanks!
Niels