From: Eric Biggers <ebiggers@kernel.org> To: Daniel Rosenberg <drosen@google.com> Cc: Theodore Ts'o <tytso@mit.edu>, linux-ext4@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <chao@kernel.org>, linux-f2fs-devel@lists.sourceforge.net, linux-fscrypt@vger.kernel.org, Alexander Viro <viro@zeniv.linux.org.uk>, Andreas Dilger <adilger.kernel@dilger.ca>, Jonathan Corbet <corbet@lwn.net>, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Gabriel Krisman Bertazi <krisman@collabora.com>, kernel-team@android.com Subject: Re: [PATCH 2/8] fscrypt: Don't allow v1 policies with casefolding Date: Tue, 3 Dec 2019 15:37:20 -0800 [thread overview] Message-ID: <20191203233720.GC727@sol.localdomain> (raw) In-Reply-To: <20191203051049.44573-3-drosen@google.com> On Mon, Dec 02, 2019 at 09:10:43PM -0800, Daniel Rosenberg wrote: > Casefolding requires a derived key for computing the siphash. > This is available for v2 policies, but not v1, so we disallow it for v1. > > Signed-off-by: Daniel Rosenberg <drosen@google.com> > --- > fs/crypto/policy.c | 26 +++++++++++++++++++++++--- > fs/inode.c | 8 ++++++++ > include/linux/fscrypt.h | 7 +++++++ > 3 files changed, 38 insertions(+), 3 deletions(-) > > diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c > index 96f528071bed..94d96d3212d6 100644 > --- a/fs/crypto/policy.c > +++ b/fs/crypto/policy.c > @@ -67,9 +67,9 @@ static bool supported_iv_ino_lblk_64_policy( > * fscrypt_supported_policy - check whether an encryption policy is supported > * > * Given an encryption policy, check whether all its encryption modes and other > - * settings are supported by this kernel. (But we don't currently don't check > - * for crypto API support here, so attempting to use an algorithm not configured > - * into the crypto API will still fail later.) > + * settings are supported by this kernel on the given inode. (But we don't > + * currently don't check for crypto API support here, so attempting to use an > + * algorithm not configured into the crypto API will still fail later.) > * > * Return: %true if supported, else %false > */ > @@ -97,6 +97,12 @@ bool fscrypt_supported_policy(const union fscrypt_policy *policy_u, > return false; > } > > + if (IS_CASEFOLDED(inode)) { > + fscrypt_warn(inode, > + "v1 policy does not support casefolded directories"); > + return false; > + } > + > return true; > } > case FSCRYPT_POLICY_V2: { > @@ -530,3 +536,17 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child, > return preload ? fscrypt_get_encryption_info(child): 0; > } > EXPORT_SYMBOL(fscrypt_inherit_context); > + > +int fscrypt_set_casefolding_allowed(struct inode *inode) > +{ > + union fscrypt_policy policy; > + int ret = fscrypt_get_policy(inode, &policy); > + > + if (ret < 0) > + return ret; In fs/crypto/ we're trying to use 'err' rather than 'ret' when a variable can only be 0 or a negative errno value. I.e.: union fscrypt_policy policy; int err; err = fscrypt_get_policy(inode, &policy); if (err) return err; > + > + if (policy.version == FSCRYPT_POLICY_V2) > + return 0; > + else > + return -EINVAL; > +} In kernel code normally an early return is used in cases like this. I.e.: if (policy.version != FSCRYPT_POLICY_V2) return -EINVAL; return 0; > @@ -2245,6 +2246,13 @@ int vfs_ioc_setflags_prepare(struct inode *inode, unsigned int oldflags, > !capable(CAP_LINUX_IMMUTABLE)) > return -EPERM; > > + /* > + * When a directory is encrypted, the CASEFOLD flag can only be turned > + * on if the fscrypt policy supports it. > + */ > + if (IS_ENCRYPTED(inode) && (flags & ~oldflags & FS_CASEFOLD_FL)) > + return fscrypt_set_casefolding_allowed(inode); > + > return 0; > } > EXPORT_SYMBOL(vfs_ioc_setflags_prepare); This needs to only return early on error. Otherwise when people add checks for more flags later, those checks will not be executed when the CASEFOLD flag is enabled on an encrypted directory. I.e.: if (IS_ENCRYPTED(inode) && (flags & ~oldflags & FS_CASEFOLD_FL)) { err = fscrypt_set_casefolding_allowed(inode); if (err) return err; } I'm also wondering if this is the right level of abstraction. Maybe the API should be: err = fscrypt_ioc_setflags_prepare(inode, oldflags, flags); if (err) return err; Then the VFS code will be "obvious", and the comment: /* * When a directory is encrypted, the CASEFOLD flag can only be turned * on if the fscrypt policy supports it. */ can be moved to the definition of fscrypt_ioc_setflags_prepare() in fs/crypto/. - Eric
WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org> To: Daniel Rosenberg <drosen@google.com> Cc: Theodore Ts'o <tytso@mit.edu>, Jonathan Corbet <corbet@lwn.net>, kernel-team@android.com, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-fscrypt@vger.kernel.org, Andreas Dilger <adilger.kernel@dilger.ca>, Alexander Viro <viro@zeniv.linux.org.uk>, linux-fsdevel@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>, linux-ext4@vger.kernel.org, Gabriel Krisman Bertazi <krisman@collabora.com> Subject: Re: [f2fs-dev] [PATCH 2/8] fscrypt: Don't allow v1 policies with casefolding Date: Tue, 3 Dec 2019 15:37:20 -0800 [thread overview] Message-ID: <20191203233720.GC727@sol.localdomain> (raw) In-Reply-To: <20191203051049.44573-3-drosen@google.com> On Mon, Dec 02, 2019 at 09:10:43PM -0800, Daniel Rosenberg wrote: > Casefolding requires a derived key for computing the siphash. > This is available for v2 policies, but not v1, so we disallow it for v1. > > Signed-off-by: Daniel Rosenberg <drosen@google.com> > --- > fs/crypto/policy.c | 26 +++++++++++++++++++++++--- > fs/inode.c | 8 ++++++++ > include/linux/fscrypt.h | 7 +++++++ > 3 files changed, 38 insertions(+), 3 deletions(-) > > diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c > index 96f528071bed..94d96d3212d6 100644 > --- a/fs/crypto/policy.c > +++ b/fs/crypto/policy.c > @@ -67,9 +67,9 @@ static bool supported_iv_ino_lblk_64_policy( > * fscrypt_supported_policy - check whether an encryption policy is supported > * > * Given an encryption policy, check whether all its encryption modes and other > - * settings are supported by this kernel. (But we don't currently don't check > - * for crypto API support here, so attempting to use an algorithm not configured > - * into the crypto API will still fail later.) > + * settings are supported by this kernel on the given inode. (But we don't > + * currently don't check for crypto API support here, so attempting to use an > + * algorithm not configured into the crypto API will still fail later.) > * > * Return: %true if supported, else %false > */ > @@ -97,6 +97,12 @@ bool fscrypt_supported_policy(const union fscrypt_policy *policy_u, > return false; > } > > + if (IS_CASEFOLDED(inode)) { > + fscrypt_warn(inode, > + "v1 policy does not support casefolded directories"); > + return false; > + } > + > return true; > } > case FSCRYPT_POLICY_V2: { > @@ -530,3 +536,17 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child, > return preload ? fscrypt_get_encryption_info(child): 0; > } > EXPORT_SYMBOL(fscrypt_inherit_context); > + > +int fscrypt_set_casefolding_allowed(struct inode *inode) > +{ > + union fscrypt_policy policy; > + int ret = fscrypt_get_policy(inode, &policy); > + > + if (ret < 0) > + return ret; In fs/crypto/ we're trying to use 'err' rather than 'ret' when a variable can only be 0 or a negative errno value. I.e.: union fscrypt_policy policy; int err; err = fscrypt_get_policy(inode, &policy); if (err) return err; > + > + if (policy.version == FSCRYPT_POLICY_V2) > + return 0; > + else > + return -EINVAL; > +} In kernel code normally an early return is used in cases like this. I.e.: if (policy.version != FSCRYPT_POLICY_V2) return -EINVAL; return 0; > @@ -2245,6 +2246,13 @@ int vfs_ioc_setflags_prepare(struct inode *inode, unsigned int oldflags, > !capable(CAP_LINUX_IMMUTABLE)) > return -EPERM; > > + /* > + * When a directory is encrypted, the CASEFOLD flag can only be turned > + * on if the fscrypt policy supports it. > + */ > + if (IS_ENCRYPTED(inode) && (flags & ~oldflags & FS_CASEFOLD_FL)) > + return fscrypt_set_casefolding_allowed(inode); > + > return 0; > } > EXPORT_SYMBOL(vfs_ioc_setflags_prepare); This needs to only return early on error. Otherwise when people add checks for more flags later, those checks will not be executed when the CASEFOLD flag is enabled on an encrypted directory. I.e.: if (IS_ENCRYPTED(inode) && (flags & ~oldflags & FS_CASEFOLD_FL)) { err = fscrypt_set_casefolding_allowed(inode); if (err) return err; } I'm also wondering if this is the right level of abstraction. Maybe the API should be: err = fscrypt_ioc_setflags_prepare(inode, oldflags, flags); if (err) return err; Then the VFS code will be "obvious", and the comment: /* * When a directory is encrypted, the CASEFOLD flag can only be turned * on if the fscrypt policy supports it. */ can be moved to the definition of fscrypt_ioc_setflags_prepare() in fs/crypto/. - Eric _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2019-12-03 23:37 UTC|newest] Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-03 5:10 [PATCH 0/8] Support for Casefolding and Encryption Daniel Rosenberg 2019-12-03 5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel 2019-12-03 5:10 ` [PATCH 1/8] fscrypt: Add siphash and hash key for policy v2 Daniel Rosenberg 2019-12-03 5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel 2019-12-03 23:25 ` Eric Biggers 2019-12-03 23:25 ` [f2fs-dev] " Eric Biggers 2019-12-03 5:10 ` [PATCH 2/8] fscrypt: Don't allow v1 policies with casefolding Daniel Rosenberg 2019-12-03 5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel 2019-12-03 23:37 ` Eric Biggers [this message] 2019-12-03 23:37 ` Eric Biggers 2019-12-03 5:10 ` [PATCH 3/8] fscrypt: Change format of no-key token Daniel Rosenberg 2019-12-03 5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel 2019-12-04 0:09 ` Eric Biggers 2019-12-04 0:09 ` [f2fs-dev] " Eric Biggers 2019-12-03 5:10 ` [PATCH 4/8] vfs: Fold casefolding into vfs Daniel Rosenberg 2019-12-03 5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel 2019-12-03 7:41 ` Gao Xiang 2019-12-03 7:41 ` [f2fs-dev] " Gao Xiang 2019-12-03 19:42 ` Gabriel Krisman Bertazi 2019-12-03 19:42 ` [f2fs-dev] " Gabriel Krisman Bertazi 2019-12-03 20:34 ` Eric Biggers 2019-12-03 20:34 ` [f2fs-dev] " Eric Biggers 2019-12-03 21:21 ` Gabriel Krisman Bertazi 2019-12-03 21:21 ` [f2fs-dev] " Gabriel Krisman Bertazi 2019-12-04 0:32 ` Eric Biggers 2019-12-04 0:32 ` [f2fs-dev] " Eric Biggers 2019-12-03 19:31 ` Gabriel Krisman Bertazi 2019-12-03 19:31 ` [f2fs-dev] " Gabriel Krisman Bertazi 2020-01-03 20:26 ` Theodore Y. Ts'o 2020-01-03 20:26 ` [f2fs-dev] " Theodore Y. Ts'o 2019-12-03 5:10 ` [PATCH 5/8] f2fs: Handle casefolding with Encryption Daniel Rosenberg 2019-12-03 5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel 2019-12-05 1:17 ` kbuild test robot 2019-12-05 1:17 ` kbuild test robot 2019-12-05 1:17 ` [f2fs-dev] " kbuild test robot 2019-12-03 5:10 ` [PATCH 6/8] ext4: Use struct super_block's casefold data Daniel Rosenberg 2019-12-03 5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel 2019-12-03 19:44 ` Gabriel Krisman Bertazi 2019-12-03 19:44 ` [f2fs-dev] " Gabriel Krisman Bertazi 2019-12-03 5:10 ` [PATCH 7/8] ext4: Hande casefolding with encryption Daniel Rosenberg 2019-12-03 5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel 2019-12-03 5:10 ` [PATCH 8/8] ext4: Optimize match for casefolded encrypted dirs Daniel Rosenberg 2019-12-03 5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191203233720.GC727@sol.localdomain \ --to=ebiggers@kernel.org \ --cc=adilger.kernel@dilger.ca \ --cc=chao@kernel.org \ --cc=corbet@lwn.net \ --cc=drosen@google.com \ --cc=jaegeuk@kernel.org \ --cc=kernel-team@android.com \ --cc=krisman@collabora.com \ --cc=linux-doc@vger.kernel.org \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=tytso@mit.edu \ --cc=viro@zeniv.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.