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.
The new USE_FS_ENCRYPTION define gets picked up in
include/linux/fscrypt.h. This allows filesystems to choose to use the
empty function definitions, or the functional ones when fscrypt is to be
used with the filesystem.
Using USE_FS_ENCRYPTION is a relatively clean approach, and requires
minimal changes to the filesystems supporting fscrypt. This RFC is
mostly for checking the acceptance of this solution, or if an other
direction is preferred.
---
Niels de Vos (4):
fscrypt: introduce USE_FS_ENCRYPTION
fs: make fscrypt support an ext4 config option
fs: make fscrypt support a f2fs config option
fs: make fscrypt support a UBIFS config option
Documentation/filesystems/fscrypt.rst | 2 +-
fs/crypto/Kconfig | 3 +++
fs/crypto/fscrypt_private.h | 2 ++
fs/ext4/Kconfig | 13 ++++++++++++-
fs/ext4/Makefile | 2 +-
fs/ext4/ext4.h | 12 ++++++++----
fs/ext4/inode.c | 6 +++---
fs/ext4/namei.c | 6 +++---
fs/ext4/super.c | 6 +++---
fs/ext4/sysfs.c | 8 ++++----
fs/f2fs/Kconfig | 15 +++++++++++++--
fs/f2fs/data.c | 2 +-
fs/f2fs/dir.c | 6 +++---
fs/f2fs/f2fs.h | 8 ++++++--
fs/f2fs/super.c | 6 +++---
fs/f2fs/sysfs.c | 8 ++++----
fs/ubifs/Kconfig | 14 ++++++++++++--
fs/ubifs/Makefile | 2 +-
fs/ubifs/sb.c | 4 ++--
fs/ubifs/ubifs.h | 7 +++++--
include/linux/fscrypt.h | 6 +++---
21 files changed, 93 insertions(+), 45 deletions(-)
--
2.37.3
Add CONFIG_F2FS_FS_ENCRYPTION as a config option, which depends on the
global CONFIG_FS_ENCRYPTION setting. This makes it possible to opt-out
of fscrypt for f2fs, while enabling it for others.
Signed-off-by: Niels de Vos <[email protected]>
---
fs/crypto/Kconfig | 1 +
fs/f2fs/Kconfig | 15 +++++++++++++--
fs/f2fs/data.c | 2 +-
fs/f2fs/dir.c | 6 +++---
fs/f2fs/f2fs.h | 6 +++---
fs/f2fs/super.c | 6 +++---
fs/f2fs/sysfs.c | 8 ++++----
7 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index 7e1267deee51..a809847e820d 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -7,6 +7,7 @@ config FS_ENCRYPTION
select CRYPTO_LIB_SHA256
select KEYS
imply EXT4_FS_ENCRYPTION
+ imply F2FS_FS_ENCRYPTION
help
Enable encryption of files and directories. This
feature is similar to ecryptfs, but it is more memory
diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index 03ef087537c7..801ade82d5c6 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -5,8 +5,6 @@ config F2FS_FS
select NLS
select CRYPTO
select CRYPTO_CRC32
- select F2FS_FS_XATTR if FS_ENCRYPTION
- select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
select FS_IOMAP
select LZ4_COMPRESS if F2FS_FS_LZ4
select LZ4_DECOMPRESS if F2FS_FS_LZ4
@@ -76,6 +74,19 @@ config F2FS_FS_SECURITY
If you are not using a security module, say N.
+config F2FS_FS_ENCRYPTION
+ bool "F2FS with support for filesystem encryption"
+ depends on F2FS_FS
+ depends on FS_ENCRYPTION
+ select F2FS_FS_XATTR
+ select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
+ help
+ Enable encryption of 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.
+
+ If unsure, say N.
+
config F2FS_CHECK_FS
bool "F2FS consistency checking feature"
depends on F2FS_FS
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a71e818cd67b..446d2eba964e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -94,7 +94,7 @@ static enum count_type __read_io_type(struct page *page)
/* postprocessing steps for read bios */
enum bio_post_read_step {
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
STEP_DECRYPT = 1 << 0,
#else
STEP_DECRYPT = 0, /* compile out the decryption-related code */
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 21960a899b6a..206580b312fb 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -114,7 +114,7 @@ static int __f2fs_setup_filename(const struct inode *dir,
fname->usr_fname = crypt_name->usr_fname;
fname->disk_name = crypt_name->disk_name;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
fname->crypto_buf = crypt_name->crypto_buf;
#endif
if (crypt_name->is_nokey_name) {
@@ -171,7 +171,7 @@ int f2fs_prepare_lookup(struct inode *dir, struct dentry *dentry,
void f2fs_free_filename(struct f2fs_filename *fname)
{
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
kfree(fname->crypto_buf.name);
fname->crypto_buf.name = NULL;
#endif
@@ -276,7 +276,7 @@ static inline int f2fs_match_name(const struct inode *dir,
#endif
f.usr_fname = fname->usr_fname;
f.disk_name = fname->disk_name;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
f.crypto_buf = fname->crypto_buf;
#endif
return fscrypt_match_name(&f, de_name, de_name_len);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 194844029633..fd0da8ce6108 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -26,7 +26,7 @@
#include <linux/part_stat.h>
#include <crypto/hash.h>
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
#define USE_FS_ENCRYPTION
#endif
#include <linux/fscrypt.h>
@@ -507,7 +507,7 @@ struct f2fs_filename {
/* The dirhash of this filename */
f2fs_hash_t hash;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
/*
* For lookups in encrypted directories: either the buffer backing
* disk_name, or a buffer that holds the decoded no-key name.
@@ -4194,7 +4194,7 @@ static inline bool f2fs_encrypted_file(struct inode *inode)
static inline void f2fs_set_encrypted_inode(struct inode *inode)
{
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
file_set_encrypt(inode);
f2fs_set_inode_flags(inode);
#endif
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 3834ead04620..224f80bb7eed 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -503,7 +503,7 @@ static int f2fs_set_test_dummy_encryption(struct super_block *sb,
&F2FS_OPTION(sbi).dummy_enc_policy;
int err;
- if (!IS_ENABLED(CONFIG_FS_ENCRYPTION)) {
+ if (!IS_ENABLED(CONFIG_F2FS_FS_ENCRYPTION)) {
f2fs_warn(sbi, "test_dummy_encryption option not supported");
return -EINVAL;
}
@@ -2997,7 +2997,7 @@ static const struct super_operations f2fs_sops = {
.remount_fs = f2fs_remount,
};
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
static int f2fs_get_context(struct inode *inode, void *ctx, size_t len)
{
return f2fs_getxattr(inode, F2FS_XATTR_INDEX_ENCRYPTION,
@@ -4157,7 +4157,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
#endif
sb->s_op = &f2fs_sops;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
sb->s_cop = &f2fs_cryptops;
#endif
#ifdef CONFIG_FS_VERITY
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index df27afd71ef4..65e135a84d57 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -848,13 +848,13 @@ F2FS_GENERAL_RO_ATTR(moved_blocks_foreground);
F2FS_GENERAL_RO_ATTR(avg_vblocks);
#endif
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
F2FS_FEATURE_RO_ATTR(encryption);
F2FS_FEATURE_RO_ATTR(test_dummy_encryption_v2);
#if IS_ENABLED(CONFIG_UNICODE)
F2FS_FEATURE_RO_ATTR(encrypted_casefold);
#endif
-#endif /* CONFIG_FS_ENCRYPTION */
+#endif /* CONFIG_F2FS_FS_ENCRYPTION */
#ifdef CONFIG_BLK_DEV_ZONED
F2FS_FEATURE_RO_ATTR(block_zoned);
F2FS_RO_ATTR(F2FS_SBI, f2fs_sb_info, unusable_blocks_per_sec,
@@ -1000,13 +1000,13 @@ static struct attribute *f2fs_attrs[] = {
ATTRIBUTE_GROUPS(f2fs);
static struct attribute *f2fs_feat_attrs[] = {
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
ATTR_LIST(encryption),
ATTR_LIST(test_dummy_encryption_v2),
#if IS_ENABLED(CONFIG_UNICODE)
ATTR_LIST(encrypted_casefold),
#endif
-#endif /* CONFIG_FS_ENCRYPTION */
+#endif /* CONFIG_F2FS_FS_ENCRYPTION */
#ifdef CONFIG_BLK_DEV_ZONED
ATTR_LIST(block_zoned),
#endif
--
2.37.3
Add CONFIG_UBIFS_FS_ENCRYPTION as a config option, which depends on the
global CONFIG_FS_ENCRYPTION setting. This makes it possible to opt-out
of fscrypt for UBIFS, while enabling it for others.
Signed-off-by: Niels de Vos <[email protected]>
---
fs/crypto/Kconfig | 1 +
fs/ubifs/Kconfig | 14 ++++++++++++--
fs/ubifs/Makefile | 2 +-
fs/ubifs/sb.c | 4 ++--
fs/ubifs/ubifs.h | 6 +++---
5 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index a809847e820d..2aef21786449 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -8,6 +8,7 @@ config FS_ENCRYPTION
select KEYS
imply EXT4_FS_ENCRYPTION
imply F2FS_FS_ENCRYPTION
+ imply UBIFS_FS_ENCRYPTION
help
Enable encryption of files and directories. This
feature is similar to ecryptfs, but it is more memory
diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig
index 45d3d207fb99..886056777d68 100644
--- a/fs/ubifs/Kconfig
+++ b/fs/ubifs/Kconfig
@@ -11,8 +11,6 @@ config UBIFS_FS
select CRYPTO_DEFLATE if UBIFS_FS_ZLIB
select CRYPTO_ZSTD if UBIFS_FS_ZSTD
select CRYPTO_HASH_INFO
- select UBIFS_FS_XATTR if FS_ENCRYPTION
- select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
depends on MTD_UBI
help
UBIFS is a file system for flash devices which works on top of UBI.
@@ -98,4 +96,16 @@ config UBIFS_FS_AUTHENTICATION
sha256, these are not selected automatically since there are many
different options.
+config UBIFS_FS_ENCRYPTION
+ bool "UBIFS with support for filesystem encryption"
+ depends on FS_ENCRYPTION
+ select UBIFS_FS_XATTR
+ select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
+ help
+ Enable encryption of 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.
+
+ If unsure, say N.
+
endif # UBIFS_FS
diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile
index 314c80b24a76..df49a573f8bd 100644
--- a/fs/ubifs/Makefile
+++ b/fs/ubifs/Makefile
@@ -6,6 +6,6 @@ ubifs-y += tnc.o master.o scan.o replay.o log.o commit.o gc.o orphan.o
ubifs-y += budget.o find.o tnc_commit.o compress.o lpt.o lprops.o
ubifs-y += recovery.o ioctl.o lpt_commit.o tnc_misc.o debug.o
ubifs-y += misc.o sysfs.o
-ubifs-$(CONFIG_FS_ENCRYPTION) += crypto.o
+ubifs-$(CONFIG_UBIFS_FS_ENCRYPTION) += crypto.o
ubifs-$(CONFIG_UBIFS_FS_XATTR) += xattr.o
ubifs-$(CONFIG_UBIFS_FS_AUTHENTICATION) += auth.o
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index e7693b94e5b5..1eb2a9be1177 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -746,7 +746,7 @@ int ubifs_read_superblock(struct ubifs_info *c)
goto out;
}
- if (!IS_ENABLED(CONFIG_FS_ENCRYPTION) && c->encrypted) {
+ if (!IS_ENABLED(CONFIG_UBIFS_FS_ENCRYPTION) && c->encrypted) {
ubifs_err(c, "file system contains encrypted files but UBIFS"
" was built without crypto support.");
err = -EINVAL;
@@ -932,7 +932,7 @@ int ubifs_enable_encryption(struct ubifs_info *c)
int err;
struct ubifs_sb_node *sup = c->sup_node;
- if (!IS_ENABLED(CONFIG_FS_ENCRYPTION))
+ if (!IS_ENABLED(CONFIG_UBIFS_FS_ENCRYPTION))
return -EOPNOTSUPP;
if (c->encrypted)
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 3ef0e9ef5015..e20f3284f504 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -33,7 +33,7 @@
#include <crypto/hash.h>
#include <crypto/algapi.h>
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_UBIFS_FS_ENCRYPTION
#define USE_FS_ENCRYPTION
#endif
#include <linux/fscrypt.h>
@@ -134,7 +134,7 @@
*/
#define WORST_COMPR_FACTOR 2
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_UBIFS_FS_ENCRYPTION
#define UBIFS_CIPHER_BLOCK_SIZE FSCRYPT_CONTENTS_ALIGNMENT
#else
#define UBIFS_CIPHER_BLOCK_SIZE 0
@@ -2114,7 +2114,7 @@ void ubifs_sysfs_unregister(struct ubifs_info *c);
#include "misc.h"
#include "key.h"
-#ifndef CONFIG_FS_ENCRYPTION
+#ifndef CONFIG_UBIFS_FS_ENCRYPTION
static inline int ubifs_encrypt(const struct inode *inode,
struct ubifs_data_node *dn,
unsigned int in_len, unsigned int *out_len,
--
2.37.3
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.
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?
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.
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.
Cheers,
- Ted
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
On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> 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...
Actually, I was thinking of getting rid of them, as we've already
gotten rid of EXT4_FS_POSIX_ACL....
> 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.
Sure, but as I said, there are **tons** of file system features that
have not and/or still are not supported for distros, but for which we
don't have kernel config knobs. This includes ext4's bigalloc and
inline data, btrfs's dedup and reflink support, xfs online fsck, etc.,
etc., etc. Heck, ext4 is only supported up to a certain size by Red
Hat, and we don't have a Kernel config so that the kernel will
absolutely refuse to mount an ext4 file system larger than The
Officially Supported RHEL Capacity Limit for Ext4. So what makes
fscrypt different from all of these other unsupported file system
features?
There are plenty of times when I've had to explain to customers why,
sure they could build their own kernels for RHEL 4 (back in the day
when I worked for Big Blue and had to talk to lots of enterprise
customers), but if they did, Red Hat support would refuse to give them
the time of day if they called asking for help. We didn't set up use
digitally signed kernels with trusted boot so that a IBM server would
refuse to boot anything other than An Officially Signed RHEL
Kernel...
What makes fscrypt different that we think we need to enforce this
using technical means, other than a simple, "this feature is not
supported"?
- Ted
On Thu, Nov 10, 2022 at 01:15:38PM -0500, Theodore Ts'o wrote:
> On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> > 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...
>
> Actually, I was thinking of getting rid of them, as we've already
> gotten rid of EXT4_FS_POSIX_ACL....
Sorry, I meant to say that we had gotten rid of EXT4_FS_XATTR.
- Ted
On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> 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...
ext4 is a little weird there as most file systems don't do that.
So I think these should go away for ext4 as well.
> 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.
But why would you do that anyay?
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.
>
> The new USE_FS_ENCRYPTION define gets picked up in
> include/linux/fscrypt.h. This allows filesystems to choose to use the
> empty function definitions, or the functional ones when fscrypt is to be
> used with the filesystem.
>
> Using USE_FS_ENCRYPTION is a relatively clean approach, and requires
> minimal changes to the filesystems supporting fscrypt. This RFC is
> mostly for checking the acceptance of this solution, or if an other
> direction is preferred.
>
> ---
>
> Niels de Vos (4):
> fscrypt: introduce USE_FS_ENCRYPTION
> fs: make fscrypt support an ext4 config option
> fs: make fscrypt support a f2fs config option
> fs: make fscrypt support a UBIFS config option
So as others have pointed out, it doesn't seem worth the complexity to do this.
For a bit of historical context, before Linux v5.1, we did have per-filesystem
options for this: CONFIG_EXT4_ENCRYPTION, CONFIG_F2FS_FS_ENCRYPTION, and
CONFIG_UBIFS_FS_ENCRYPTION. If you enabled one of these, it selected
CONFIG_FS_ENCRYPTION to get the code in fs/crypto/. CONFIG_FS_ENCRYPTION was a
tristate, so the code in fs/crypto/ could be built as a loadable module if it
was only needed by filesystems that were loadable modules themselves.
Having fs/crypto/ possibly be a loadable module was problematic, though, because
it made it impossible to call into fs/crypto/ from built-in code such as
fs/buffer.c, fs/ioctl.c, fs/libfs.c, fs/super.c, fs/iomap/direct-io.c, etc. So
that's why we made CONFIG_FS_ENCRYPTION into a bool. At the same time, we
decided to simplify the kconfig options by removing the per-filesystem options
so that it worked like CONFIG_QUOTA, CONFIG_FS_DAX, CONFIG_FS_POSIX_ACL, etc.
I suppose we *could* have *just* changed CONFIG_FS_ENCRYPTION to a bool to solve
the first problem, and kept the per-filesystem options. I think that wouldn't
have made a lot of sense, though, for the reasons that Ted has already covered.
A further point, beyond what Ted has already covered, is that
non-filesystem-specific code can't honor filesystem-specific options. So e.g.
if you had a filesystem with encryption disabled by kconfig, that then called
into fs/iomap/direct-io.c to process an I/O request, it could potentially still
call into fs/crypto/ to enable encryption on that I/O request, since
fs/iomap/direct-io.c would think that encryption support is enabled.
Granted, that *should* never actually happen, because this would only make a
difference on encrypted files, and the filesystem shouldn't have allowed an
encrypted file to be opened if it doesn't have encryption support enabled. But
it does seem a bit odd, given that it would go against the goal of compiling out
all encryption code for a filesystem.
- Eric
On Thu, Nov 10, 2022 at 01:15:38PM -0500, Theodore Ts'o wrote:
> On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> > 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...
>
> Actually, I was thinking of getting rid of them, as we've already
> gotten rid of EXT4_FS_POSIX_ACL....
>
> > 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.
>
> Sure, but as I said, there are **tons** of file system features that
> have not and/or still are not supported for distros, but for which we
> don't have kernel config knobs. This includes ext4's bigalloc and
> inline data, btrfs's dedup and reflink support, xfs online fsck, etc.,
> etc., etc. Heck, ext4 is only supported up to a certain size by Red
> Hat, and we don't have a Kernel config so that the kernel will
> absolutely refuse to mount an ext4 file system larger than The
> Officially Supported RHEL Capacity Limit for Ext4. So what makes
> fscrypt different from all of these other unsupported file system
> features?
>
> There are plenty of times when I've had to explain to customers why,
> sure they could build their own kernels for RHEL 4 (back in the day
> when I worked for Big Blue and had to talk to lots of enterprise
> customers), but if they did, Red Hat support would refuse to give them
> the time of day if they called asking for help. We didn't set up use
> digitally signed kernels with trusted boot so that a IBM server would
> refuse to boot anything other than An Officially Signed RHEL
> Kernel...
>
> What makes fscrypt different that we think we need to enforce this
> using technical means, other than a simple, "this feature is not
> supported"?
Thanks again for the added details. What you are explaining makes sense,
and I am not sure if there is an other good reason why splitting out
fscrypt support per filesystem would be required. I'm checking with the
folks that suggested doing this, and see where we go from there.
Niels
On Sun, Nov 13, 2022 at 10:02:37PM -0800, Christoph Hellwig wrote:
> On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> > 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...
>
> ext4 is a little weird there as most file systems don't do that.
> So I think these should go away for ext4 as well.
Yeah, I understand that there is a preference for reducing the number of
Kconfig options for filesystems. That indeed would make it a little
easier for users, so I am supportive of that as well.
> > 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.
>
> But why would you do that anyay?
An other mail in this thread contains a description about that. It is
more about being able to provide a kernel build that is fully tested,
and enabling more options (or being unable to disable features)
increases the testing efforts that are needed.
However, as Ted pointed out, there are other features that can not be
disabled or limited per filesystem, so there will always be a gap in
what can practically be tested.
Thanks,
Niels
On Tue, Nov 15, 2022 at 06:10:59PM -0800, Eric Biggers 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.
> >
> > The new USE_FS_ENCRYPTION define gets picked up in
> > include/linux/fscrypt.h. This allows filesystems to choose to use the
> > empty function definitions, or the functional ones when fscrypt is to be
> > used with the filesystem.
> >
> > Using USE_FS_ENCRYPTION is a relatively clean approach, and requires
> > minimal changes to the filesystems supporting fscrypt. This RFC is
> > mostly for checking the acceptance of this solution, or if an other
> > direction is preferred.
> >
> > ---
> >
> > Niels de Vos (4):
> > fscrypt: introduce USE_FS_ENCRYPTION
> > fs: make fscrypt support an ext4 config option
> > fs: make fscrypt support a f2fs config option
> > fs: make fscrypt support a UBIFS config option
>
> So as others have pointed out, it doesn't seem worth the complexity to do this.
>
> For a bit of historical context, before Linux v5.1, we did have per-filesystem
> options for this: CONFIG_EXT4_ENCRYPTION, CONFIG_F2FS_FS_ENCRYPTION, and
> CONFIG_UBIFS_FS_ENCRYPTION. If you enabled one of these, it selected
> CONFIG_FS_ENCRYPTION to get the code in fs/crypto/. CONFIG_FS_ENCRYPTION was a
> tristate, so the code in fs/crypto/ could be built as a loadable module if it
> was only needed by filesystems that were loadable modules themselves.
>
> Having fs/crypto/ possibly be a loadable module was problematic, though, because
> it made it impossible to call into fs/crypto/ from built-in code such as
> fs/buffer.c, fs/ioctl.c, fs/libfs.c, fs/super.c, fs/iomap/direct-io.c, etc. So
> that's why we made CONFIG_FS_ENCRYPTION into a bool. At the same time, we
> decided to simplify the kconfig options by removing the per-filesystem options
> so that it worked like CONFIG_QUOTA, CONFIG_FS_DAX, CONFIG_FS_POSIX_ACL, etc.
>
> I suppose we *could* have *just* changed CONFIG_FS_ENCRYPTION to a bool to solve
> the first problem, and kept the per-filesystem options. I think that wouldn't
> have made a lot of sense, though, for the reasons that Ted has already covered.
Yes, it seems that there is a move to reduce the Kconfig options and
(re)adding per-filesystem encryption support would be counterproductive.
> A further point, beyond what Ted has already covered, is that
> non-filesystem-specific code can't honor filesystem-specific options. So e.g.
> if you had a filesystem with encryption disabled by kconfig, that then called
> into fs/iomap/direct-io.c to process an I/O request, it could potentially still
> call into fs/crypto/ to enable encryption on that I/O request, since
> fs/iomap/direct-io.c would think that encryption support is enabled.
>
> Granted, that *should* never actually happen, because this would only make a
> difference on encrypted files, and the filesystem shouldn't have allowed an
> encrypted file to be opened if it doesn't have encryption support enabled. But
> it does seem a bit odd, given that it would go against the goal of compiling out
> all encryption code for a filesystem.
Ah, yes, indeed! The boundaries between the options would be less clear,
and potential changes to shared functions under fs/ could have incorrect
assumptions about CONFIG_FS_ENCRYPTION. Even if this is not the case
now, optimizations/enhancements in the future might be more complicated
because of this.
Thanks for the additional details! Have a good weekend,
Niels