mbox series

[v2,00/20] btrfs: add fscrypt integration

Message ID cover.1662420176.git.sweettea-kernel@dorminy.me (mailing list archive)
Headers show
Series btrfs: add fscrypt integration | expand

Message

Sweet Tea Dorminy Sept. 6, 2022, 12:35 a.m. UTC
This is a changeset adding encryption to btrfs.

Last October, Omar Sandoval sent out a design document for having fscrypt
integration with btrfs [1]. In summary, it proposes btrfs storing its
own encryption IVs on a per-file-extent basis. fscrypt usually encrypts
files using an IV derived from per-inode information; this would prevent
snapshotting or reflinking or data relocation for btrfs. We have
refined this into a fscrypt extent context object, opaque to the
filesystem, which fscrypt uses to generate an IV associated with each
block in an extent. Thus, all the inodes sharing a particular
key and file extent may decrypt the extent.

This series implements this integration for the simple
case, non-compressed data extents. Followup changes will allow
encryption of compressed extents, inline extents, and verity items, and
will add tests around subvolume encryption. This series should provide
encryption for the simplest cases, but this series should not be used in
production, as there are likely bugs.
 
Preliminary btrfs-progs changes are available at [2]; fstests changes
are available at [3].

[1]
https://lore.kernel.org/linux-btrfs/YXGyq+buM79A1S0L@relinquished.localdomain/

Changelog:

v2:
 - Amended the fscrypt side to generically add extent contexts,
   hopefully as per Eric Biggers' past comments. IVs are now entirely
   abstracted within an extent context, and there is no longer a new
   encryption policy, as DIRECT_KEY sufficiently encapsulates the
   needs of extent-based encryption. Documented its usage in btrfs
   briefly in the documentation. 
 - Adjusted the btrfs side to deal in opaque extent contexts. Improved
   optimization to skip storing inode contexts if they are the same as
   the inode's root item's inode context.
 - Combined 'add fscrypt operation table to superblock' into 'start
   using fscrypt hooks'.
 - https://lore.kernel.org/linux-btrfs/cover.1662420176.git.sweettea-kernel@dorminy.me
 - progs: https://lore.kernel.org/linux-btrfs/cover.1662417859.git.sweettea-kernel@dorminy.me
 - tests: https://lore.kernel.org/linux-btrfs/cover.1662417905.git.sweettea-kernel@dorminy.me

v1:
 - Recombined the fscrypt changes back into this patchset.
 - Fixed several races and incorrectly ordered operations.
 - Improved IV retrieval to correctly distinguish between
   filename/symlink encryption and encryption of block 0 of a file.
 - https://lore.kernel.org/linux-btrfs/cover.1660744500.git.sweettea-kernel@dorminy.me
 - progs: https://lore.kernel.org/linux-btrfs/cover.1660729916.git.sweettea-kernel@dorminy.me
 - tests: https://lore.kernel.org/linux-btrfs/cover.1660729861.git.sweettea-kernel@dorminy.me

RFC v2: 
 - Fixed all warnings and known incorrectnesses.
 - Split fscrypt changes into their own patchset:
    https://lore.kernel.org/linux-fscrypt/cover.1658623235.git.sweettea-kernel@dorminy.me
 - Combined and reordered changes so that enabling fscrypt is the last change.
 - Removed unnecessary factoring.
 - Split a cleanup change off.
 - https://lore.kernel.org/linux-btrfs/cover.1658623319.git.sweettea-kernel@dorminy.me 

RFC v1:
 - https://lore.kernel.org/linux-btrfs/cover.1657707686.git.sweettea-kernel@dorminy.me

Omar Sandoval (14):
  fscrypt: expose fscrypt_nokey_name
  fscrypt: add flag allowing partially-encrypted directories
  fscrypt: add fscrypt_have_same_policy() to check inode compatibility
  btrfs: store directory's encryption state
  btrfs: factor a fscrypt_name matching method
  btrfs: disable various operations on encrypted inodes
  btrfs: start using fscrypt hooks.
  btrfs: add fscrypt_context items.
  btrfs: translate btrfs encryption flags and encrypted inode flag.
  btrfs: Add new FEATURE_INCOMPAT_FSCRYPT feature flag.
  btrfs: reuse encrypted filename hash when possible.
  btrfs: adapt directory read and lookup to potentially encrypted
    filenames
  btrfs: encrypt normal file extent data if appropriate
  btrfs: implement fscrypt ioctls

Sweet Tea Dorminy (6):
  fscrypt: allow fscrypt_generate_iv() to distinguish filenames
  fscrypt: add extent-based encryption
  fscrypt: document btrfs' fscrypt quirks.
  btrfs: use fscrypt_names instead of name/len everywhere.
  btrfs: setup fscrypt_names from dentrys using helper
  btrfs: store a fscrypt extent context per normal file extent

 Documentation/filesystems/fscrypt.rst |  62 ++-
 fs/btrfs/Makefile                     |   1 +
 fs/btrfs/btrfs_inode.h                |   3 +
 fs/btrfs/ctree.h                      | 119 ++++--
 fs/btrfs/delayed-inode.c              |  48 ++-
 fs/btrfs/delayed-inode.h              |   9 +-
 fs/btrfs/dir-item.c                   | 119 +++---
 fs/btrfs/extent_io.c                  |  93 ++++-
 fs/btrfs/extent_io.h                  |   2 +
 fs/btrfs/extent_map.h                 |   4 +
 fs/btrfs/file-item.c                  |  22 +-
 fs/btrfs/file.c                       |  11 +-
 fs/btrfs/fscrypt.c                    | 244 +++++++++++
 fs/btrfs/fscrypt.h                    |  49 +++
 fs/btrfs/inode-item.c                 |  84 ++--
 fs/btrfs/inode-item.h                 |  14 +-
 fs/btrfs/inode.c                      | 581 +++++++++++++++++++-------
 fs/btrfs/ioctl.c                      |  80 +++-
 fs/btrfs/ordered-data.c               |   9 +-
 fs/btrfs/ordered-data.h               |   4 +-
 fs/btrfs/print-tree.c                 |   4 +-
 fs/btrfs/props.c                      |  11 +-
 fs/btrfs/reflink.c                    |   8 +
 fs/btrfs/root-tree.c                  |  20 +-
 fs/btrfs/send.c                       | 141 ++++---
 fs/btrfs/super.c                      |   8 +-
 fs/btrfs/transaction.c                |  43 +-
 fs/btrfs/tree-checker.c               |  56 ++-
 fs/btrfs/tree-log.c                   | 237 ++++++-----
 fs/btrfs/tree-log.h                   |   4 +-
 fs/btrfs/xattr.c                      |  21 +-
 fs/crypto/crypto.c                    |  24 +-
 fs/crypto/fname.c                     |  60 +--
 fs/crypto/fscrypt_private.h           |  26 +-
 fs/crypto/inline_crypt.c              |  29 +-
 fs/crypto/policy.c                    | 103 +++++
 include/linux/fscrypt.h               |  81 ++++
 include/uapi/linux/btrfs.h            |   1 +
 include/uapi/linux/btrfs_tree.h       |  26 ++
 39 files changed, 1881 insertions(+), 580 deletions(-)
 create mode 100644 fs/btrfs/fscrypt.c
 create mode 100644 fs/btrfs/fscrypt.h

Comments

Eric Biggers Sept. 6, 2022, 10:35 p.m. UTC | #1
On Mon, Sep 05, 2022 at 08:35:15PM -0400, Sweet Tea Dorminy wrote:
> This is a changeset adding encryption to btrfs.

What git tree and commit does this apply to?

- Eric
Sweet Tea Dorminy Sept. 6, 2022, 11:01 p.m. UTC | #2
On 9/6/22 18:35, Eric Biggers wrote:
> On Mon, Sep 05, 2022 at 08:35:15PM -0400, Sweet Tea Dorminy wrote:
>> This is a changeset adding encryption to btrfs.
> 
> What git tree and commit does this apply to?

https://github.com/kdave/btrfs-devel/; branch misc-next. Should apply 
cleanly to its current tip, 76ccdc004e12312ea056811d530043ff11d050c6 .
Eric Biggers Sept. 6, 2022, 11:10 p.m. UTC | #3
On Tue, Sep 06, 2022 at 07:01:15PM -0400, Sweet Tea Dorminy wrote:
> 
> 
> On 9/6/22 18:35, Eric Biggers wrote:
> > On Mon, Sep 05, 2022 at 08:35:15PM -0400, Sweet Tea Dorminy wrote:
> > > This is a changeset adding encryption to btrfs.
> > 
> > What git tree and commit does this apply to?
> 
> https://github.com/kdave/btrfs-devel/; branch misc-next. Should apply
> cleanly to its current tip, 76ccdc004e12312ea056811d530043ff11d050c6 .

Patch 8 wasn't received by linux-fscrypt for some reason, any idea why?

$ b4 am cover.1662420176.git.sweettea-kernel@dorminy.me
Looking up https://lore.kernel.org/linux-fscrypt/cover.1662420176.git.sweettea-kernel%40dorminy.me
Grabbing thread from lore.kernel.org/linux-fscrypt/cover.1662420176.git.sweettea-kernel%40dorminy.me/t.mbox.gz
Analyzing 22 messages in the thread
Checking attestation on all messages, may take a moment...
---
  [PATCH v2 1/20] fscrypt: expose fscrypt_nokey_name
  [PATCH v2 2/20] fscrypt: add flag allowing partially-encrypted directories
  [PATCH v2 3/20] fscrypt: add fscrypt_have_same_policy() to check inode compatibility
  [PATCH v2 4/20] fscrypt: allow fscrypt_generate_iv() to distinguish filenames
  [PATCH v2 5/20] fscrypt: add extent-based encryption
  [PATCH v2 6/20] fscrypt: document btrfs' fscrypt quirks.
  [PATCH v2 7/20] btrfs: store directory's encryption state
  ERROR: missing [8/20]!
  [PATCH v2 9/20] btrfs: setup fscrypt_names from dentrys using helper
  [PATCH v2 10/20] btrfs: factor a fscrypt_name matching method
  [PATCH v2 11/20] btrfs: disable various operations on encrypted inodes
  [PATCH v2 12/20] btrfs: start using fscrypt hooks.
  [PATCH v2 13/20] btrfs: add fscrypt_context items.
  [PATCH v2 14/20] btrfs: translate btrfs encryption flags and encrypted inode flag.
  [PATCH v2 15/20] btrfs: store a fscrypt extent context per normal file extent
  [PATCH v2 16/20] btrfs: Add new FEATURE_INCOMPAT_FSCRYPT feature flag.
  [PATCH v2 17/20] btrfs: reuse encrypted filename hash when possible.
  [PATCH v2 18/20] btrfs: adapt directory read and lookup to potentially encrypted filenames
  [PATCH v2 19/20] btrfs: encrypt normal file extent data if appropriate
  [PATCH v2 20/20] btrfs: implement fscrypt ioctls
Sweet Tea Dorminy Sept. 7, 2022, 12:01 a.m. UTC | #4
On 2022-09-06 19:10, Eric Biggers wrote:
> On Tue, Sep 06, 2022 at 07:01:15PM -0400, Sweet Tea Dorminy wrote:
>> 
>> 
>> On 9/6/22 18:35, Eric Biggers wrote:
>> > On Mon, Sep 05, 2022 at 08:35:15PM -0400, Sweet Tea Dorminy wrote:
>> > > This is a changeset adding encryption to btrfs.
>> >
>> > What git tree and commit does this apply to?
>> 
>> https://github.com/kdave/btrfs-devel/; branch misc-next. Should apply
>> cleanly to its current tip, 76ccdc004e12312ea056811d530043ff11d050c6 .
> 
> Patch 8 wasn't received by linux-fscrypt for some reason, any idea why?
> 
> $ b4 am cover.1662420176.git.sweettea-kernel@dorminy.me
> Looking up
> https://lore.kernel.org/linux-fscrypt/cover.1662420176.git.sweettea-kernel%40dorminy.me
> Grabbing thread from
> lore.kernel.org/linux-fscrypt/cover.1662420176.git.sweettea-kernel%40dorminy.me/t.mbox.gz
> Analyzing 22 messages in the thread
> Checking attestation on all messages, may take a moment...
> ---
>   [PATCH v2 1/20] fscrypt: expose fscrypt_nokey_name
>   [PATCH v2 2/20] fscrypt: add flag allowing partially-encrypted 
> directories
>   [PATCH v2 3/20] fscrypt: add fscrypt_have_same_policy() to check
> inode compatibility
>   [PATCH v2 4/20] fscrypt: allow fscrypt_generate_iv() to distinguish 
> filenames
>   [PATCH v2 5/20] fscrypt: add extent-based encryption
>   [PATCH v2 6/20] fscrypt: document btrfs' fscrypt quirks.
>   [PATCH v2 7/20] btrfs: store directory's encryption state
>   ERROR: missing [8/20]!
>   [PATCH v2 9/20] btrfs: setup fscrypt_names from dentrys using helper
>   [PATCH v2 10/20] btrfs: factor a fscrypt_name matching method
>   [PATCH v2 11/20] btrfs: disable various operations on encrypted 
> inodes
>   [PATCH v2 12/20] btrfs: start using fscrypt hooks.
>   [PATCH v2 13/20] btrfs: add fscrypt_context items.
>   [PATCH v2 14/20] btrfs: translate btrfs encryption flags and
> encrypted inode flag.
>   [PATCH v2 15/20] btrfs: store a fscrypt extent context per normal 
> file extent
>   [PATCH v2 16/20] btrfs: Add new FEATURE_INCOMPAT_FSCRYPT feature 
> flag.
>   [PATCH v2 17/20] btrfs: reuse encrypted filename hash when possible.
>   [PATCH v2 18/20] btrfs: adapt directory read and lookup to
> potentially encrypted filenames
>   [PATCH v2 19/20] btrfs: encrypt normal file extent data if 
> appropriate
>   [PATCH v2 20/20] btrfs: implement fscrypt ioctls

Patch 8 is large and dull, and bounced from linux-fscrypt@ for size, but 
b4 gets it for me. b4 -p linux-fscrypt recreates the missing [8/20]; is 
it possible that option is set somewhere in your configuration?

$ b4 am -o- cover.1662420176.git.sweettea-kernel@dorminy.me | git am
Looking up 
https://lore.kernel.org/r/cover.1662420176.git.sweettea-kernel%40dorminy.me
Grabbing thread from 
lore.kernel.org/all/cover.1662420176.git.sweettea-kernel%40dorminy.me/t.mbox.gz
Analyzing 23 messages in the thread
Checking attestation on all messages, may take a moment...
---
   ✓ [PATCH v2 1/20] fscrypt: expose fscrypt_nokey_name
   ✓ [PATCH v2 2/20] fscrypt: add flag allowing partially-encrypted 
directories
   ✓ [PATCH v2 3/20] fscrypt: add fscrypt_have_same_policy() to check 
inode compatibility
   ✓ [PATCH v2 4/20] fscrypt: allow fscrypt_generate_iv() to distinguish 
filenames
   ✓ [PATCH v2 5/20] fscrypt: add extent-based encryption
   ✓ [PATCH v2 6/20] fscrypt: document btrfs' fscrypt quirks.
   ✓ [PATCH v2 7/20] btrfs: store directory's encryption state
   ✓ [PATCH v2 8/20] btrfs: use fscrypt_names instead of name/len 
everywhere.
   ✓ [PATCH v2 9/20] btrfs: setup fscrypt_names from dentrys using helper
   ✓ [PATCH v2 10/20] btrfs: factor a fscrypt_name matching method
   ✓ [PATCH v2 11/20] btrfs: disable various operations on encrypted 
inodes
     + Reported-by: kernel test robot <lkp@intel.com> (✓ DKIM/intel.com)
   ✓ [PATCH v2 12/20] btrfs: start using fscrypt hooks.
   ✓ [PATCH v2 13/20] btrfs: add fscrypt_context items.
   ✓ [PATCH v2 14/20] btrfs: translate btrfs encryption flags and 
encrypted inode flag.
   ✓ [PATCH v2 15/20] btrfs: store a fscrypt extent context per normal 
file extent
   ✓ [PATCH v2 16/20] btrfs: Add new FEATURE_INCOMPAT_FSCRYPT feature 
flag.
   ✓ [PATCH v2 17/20] btrfs: reuse encrypted filename hash when possible.
   ✓ [PATCH v2 18/20] btrfs: adapt directory read and lookup to 
potentially encrypted filenames
   ✓ [PATCH v2 19/20] btrfs: encrypt normal file extent data if 
appropriate
   ✓ [PATCH v2 20/20] btrfs: implement fscrypt ioctls
   ---
   ✓ Signed: DKIM/dorminy.me
---
Total patches: 20
---
  Link: 
https://lore.kernel.org/r/cover.1662420176.git.sweettea-kernel@dorminy.me
...
David Sterba Sept. 7, 2022, 7:38 p.m. UTC | #5
On Mon, Sep 05, 2022 at 08:35:15PM -0400, Sweet Tea Dorminy wrote:
> This is a changeset adding encryption to btrfs.
> 
> Last October, Omar Sandoval sent out a design document for having fscrypt
> integration with btrfs [1]. In summary, it proposes btrfs storing its
> own encryption IVs on a per-file-extent basis. fscrypt usually encrypts
> files using an IV derived from per-inode information; this would prevent
> snapshotting or reflinking or data relocation for btrfs. We have
> refined this into a fscrypt extent context object, opaque to the
> filesystem, which fscrypt uses to generate an IV associated with each
> block in an extent. Thus, all the inodes sharing a particular
> key and file extent may decrypt the extent.
> 
> This series implements this integration for the simple
> case, non-compressed data extents. Followup changes will allow
> encryption of compressed extents, inline extents, and verity items, and
> will add tests around subvolume encryption. This series should provide
> encryption for the simplest cases, but this series should not be used in
> production, as there are likely bugs.
>  
> Preliminary btrfs-progs changes are available at [2]; fstests changes
> are available at [3].

I did a quick pass to check if there's anything that could be merged to
6.1 as preparatory, the fname is a candidate but I've also seen random
coding style issues that I'd like to get fixed first.

One thing I've noticed is that the incompat bit is only defined but not
used anywhere. Any functional change should be guarded behind it, and
once the implementation is complete the enabling part is in a separate
patch.

Regarding the build config options, I assume that the fscrypt support is
optional, so it should build with and without the option. I'm not sure
I've see enough ifdefs for that. For such features there should be a
line in btrfs_print_mod_info, like we have eg. for verity.

I'll post other comments under the patches.