mbox series

[RFC,00/17] fscrypt: add per-extent encryption keys

Message ID cover.1672547582.git.sweettea-kernel@dorminy.me (mailing list archive)
Headers show
Series fscrypt: add per-extent encryption keys | expand

Message

Sweet Tea Dorminy Jan. 1, 2023, 5:06 a.m. UTC
Last month, after a discussion of using fscrypt in btrfs, several
potential areas for expansion of fscrypt functionality were identified:
specifically, per-extent keys, authenticated encryption, and 'rekeying'
a directory tree [1]. These additions will permit btrfs to have better
cryptographic characteristics than previous attempts at expanding btrfs
to use fscrypt.

This attempts to implement the first of these, per-extent keys (in
analogy to the current per-inode keys) in fscrypt. For a filesystem
using per-extent keys, the idea is that each regular file inode is
linked to its parent directory's fscrypt_info, while each extent in
the filesystem -- opaque to fscrypt -- stores a fscrypt_info providing
the key for the data in that extent. For non-regular files, the inode
has its own fscrypt_info as in current ("inode-based") fscrypt.  

IV generation methods using logical block numbers use the logical block
number within the extent, and for IV generation methods using inode
numbers, such filesystems may optionally implement a method providing an
equivalent on a per-extent basis. 

Known limitations: change 12 ("fscrypt: notify per-extent infos if
master key vanishes") does not sufficiently argue that there cannot be a
race between freeing a master key and using it for some pending extent IO.
Change 16 ("fscrypt: disable inline encryption for extent-based
encryption") merely disables inline encryption, when it should implement
generating appropriate inline encryption info for extent infos.

This has not been thoroughly tested against a btrfs implementation of
the interfaces -- I've thrown out everything here and tried something
new several times, and while I think this interface is a decent one, I
would like to get input on it in parallel with finishing the btrfs side
of this part, and the other elements of the design mentioned in [1]

[1] https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing

*** BLURB HERE ***

Sweet Tea Dorminy (17):
  fscrypt: factor accessing inode->i_crypt_info
  fscrypt: separate getting info for a specific block
  fscrypt: adjust effective lblks based on extents
  fscrypt: factor out fscrypt_set_inode_info()
  fscrypt: use parent dir's info for extent-based encryption.
  fscrypt: add a super_block pointer to fscrypt_info
  fscrypt: update comments about inodes to include extents
  fscrypt: rename mk->mk_decrypted_inodes*
  fscrypt: make fscrypt_setup_encryption_info generic for extents
  fscrypt: let fscrypt_infos be owned by an extent
  fscrypt: update all the *per_file_* function names
  fscrypt: notify per-extent infos if master key vanishes
  fscrypt: use an optional ino equivalent for per-extent infos
  fscrypt: add creation/usage/freeing of per-extent infos
  fscrypt: allow load/save of extent contexts
  fscrypt: disable inline encryption for extent-based encryption
  fscrypt: update documentation to mention per-extent keys.

 Documentation/filesystems/fscrypt.rst |  38 +++-
 fs/crypto/crypto.c                    |  17 +-
 fs/crypto/fname.c                     |   9 +-
 fs/crypto/fscrypt_private.h           | 174 +++++++++++++----
 fs/crypto/hooks.c                     |   2 +-
 fs/crypto/inline_crypt.c              |  42 ++--
 fs/crypto/keyring.c                   |  67 ++++---
 fs/crypto/keysetup.c                  | 263 ++++++++++++++++++++------
 fs/crypto/keysetup_v1.c               |  24 +--
 fs/crypto/policy.c                    |  28 ++-
 include/linux/fscrypt.h               |  76 ++++++++
 11 files changed, 580 insertions(+), 160 deletions(-)


base-commit: b7af0635c87ff78d6bd523298ab7471f9ffd3ce5

Comments

Neal Gompa Feb. 22, 2023, 11:52 a.m. UTC | #1
On Sun, Jan 1, 2023 at 12:08 AM Sweet Tea Dorminy
<sweettea-kernel@dorminy.me> wrote:
>
> Last month, after a discussion of using fscrypt in btrfs, several
> potential areas for expansion of fscrypt functionality were identified:
> specifically, per-extent keys, authenticated encryption, and 'rekeying'
> a directory tree [1]. These additions will permit btrfs to have better
> cryptographic characteristics than previous attempts at expanding btrfs
> to use fscrypt.
>
> This attempts to implement the first of these, per-extent keys (in
> analogy to the current per-inode keys) in fscrypt. For a filesystem
> using per-extent keys, the idea is that each regular file inode is
> linked to its parent directory's fscrypt_info, while each extent in
> the filesystem -- opaque to fscrypt -- stores a fscrypt_info providing
> the key for the data in that extent. For non-regular files, the inode
> has its own fscrypt_info as in current ("inode-based") fscrypt.
>
> IV generation methods using logical block numbers use the logical block
> number within the extent, and for IV generation methods using inode
> numbers, such filesystems may optionally implement a method providing an
> equivalent on a per-extent basis.
>
> Known limitations: change 12 ("fscrypt: notify per-extent infos if
> master key vanishes") does not sufficiently argue that there cannot be a
> race between freeing a master key and using it for some pending extent IO.
> Change 16 ("fscrypt: disable inline encryption for extent-based
> encryption") merely disables inline encryption, when it should implement
> generating appropriate inline encryption info for extent infos.
>
> This has not been thoroughly tested against a btrfs implementation of
> the interfaces -- I've thrown out everything here and tried something
> new several times, and while I think this interface is a decent one, I
> would like to get input on it in parallel with finishing the btrfs side
> of this part, and the other elements of the design mentioned in [1]
>
> [1] https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
>
> *** BLURB HERE ***
>
> Sweet Tea Dorminy (17):
>   fscrypt: factor accessing inode->i_crypt_info
>   fscrypt: separate getting info for a specific block
>   fscrypt: adjust effective lblks based on extents
>   fscrypt: factor out fscrypt_set_inode_info()
>   fscrypt: use parent dir's info for extent-based encryption.
>   fscrypt: add a super_block pointer to fscrypt_info
>   fscrypt: update comments about inodes to include extents
>   fscrypt: rename mk->mk_decrypted_inodes*
>   fscrypt: make fscrypt_setup_encryption_info generic for extents
>   fscrypt: let fscrypt_infos be owned by an extent
>   fscrypt: update all the *per_file_* function names
>   fscrypt: notify per-extent infos if master key vanishes
>   fscrypt: use an optional ino equivalent for per-extent infos
>   fscrypt: add creation/usage/freeing of per-extent infos
>   fscrypt: allow load/save of extent contexts
>   fscrypt: disable inline encryption for extent-based encryption
>   fscrypt: update documentation to mention per-extent keys.
>
>  Documentation/filesystems/fscrypt.rst |  38 +++-
>  fs/crypto/crypto.c                    |  17 +-
>  fs/crypto/fname.c                     |   9 +-
>  fs/crypto/fscrypt_private.h           | 174 +++++++++++++----
>  fs/crypto/hooks.c                     |   2 +-
>  fs/crypto/inline_crypt.c              |  42 ++--
>  fs/crypto/keyring.c                   |  67 ++++---
>  fs/crypto/keysetup.c                  | 263 ++++++++++++++++++++------
>  fs/crypto/keysetup_v1.c               |  24 +--
>  fs/crypto/policy.c                    |  28 ++-
>  include/linux/fscrypt.h               |  76 ++++++++
>  11 files changed, 580 insertions(+), 160 deletions(-)
>
>
> base-commit: b7af0635c87ff78d6bd523298ab7471f9ffd3ce5
> --
> 2.38.1
>

I'm surprised that this submission generated no discussion across a
timeframe of over a month. Is this normal for RFC patch sets?
Sweet Tea Dorminy Feb. 22, 2023, 2:13 p.m. UTC | #2
On 2/22/23 06:52, Neal Gompa wrote:
> On Sun, Jan 1, 2023 at 12:08 AM Sweet Tea Dorminy
> <sweettea-kernel@dorminy.me> wrote:
>>
>> Last month, after a discussion of using fscrypt in btrfs, several
>> potential areas for expansion of fscrypt functionality were identified:
>> specifically, per-extent keys, authenticated encryption, and 'rekeying'
>> a directory tree [1]. These additions will permit btrfs to have better
>> cryptographic characteristics than previous attempts at expanding btrfs
>> to use fscrypt.
>>
>> This attempts to implement the first of these, per-extent keys (in
>> analogy to the current per-inode keys) in fscrypt. For a filesystem
>> using per-extent keys, the idea is that each regular file inode is
>> linked to its parent directory's fscrypt_info, while each extent in
>> the filesystem -- opaque to fscrypt -- stores a fscrypt_info providing
>> the key for the data in that extent. For non-regular files, the inode
>> has its own fscrypt_info as in current ("inode-based") fscrypt.
>>
>> IV generation methods using logical block numbers use the logical block
>> number within the extent, and for IV generation methods using inode
>> numbers, such filesystems may optionally implement a method providing an
>> equivalent on a per-extent basis.
>>
>> Known limitations: change 12 ("fscrypt: notify per-extent infos if
>> master key vanishes") does not sufficiently argue that there cannot be a
>> race between freeing a master key and using it for some pending extent IO.
>> Change 16 ("fscrypt: disable inline encryption for extent-based
>> encryption") merely disables inline encryption, when it should implement
>> generating appropriate inline encryption info for extent infos.
>>
>> This has not been thoroughly tested against a btrfs implementation of
>> the interfaces -- I've thrown out everything here and tried something
>> new several times, and while I think this interface is a decent one, I
>> would like to get input on it in parallel with finishing the btrfs side
>> of this part, and the other elements of the design mentioned in [1]
>>
>> [1] https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
>>
>> *** BLURB HERE ***
>>
>> Sweet Tea Dorminy (17):
>>    fscrypt: factor accessing inode->i_crypt_info
>>    fscrypt: separate getting info for a specific block
>>    fscrypt: adjust effective lblks based on extents
>>    fscrypt: factor out fscrypt_set_inode_info()
>>    fscrypt: use parent dir's info for extent-based encryption.
>>    fscrypt: add a super_block pointer to fscrypt_info
>>    fscrypt: update comments about inodes to include extents
>>    fscrypt: rename mk->mk_decrypted_inodes*
>>    fscrypt: make fscrypt_setup_encryption_info generic for extents
>>    fscrypt: let fscrypt_infos be owned by an extent
>>    fscrypt: update all the *per_file_* function names
>>    fscrypt: notify per-extent infos if master key vanishes
>>    fscrypt: use an optional ino equivalent for per-extent infos
>>    fscrypt: add creation/usage/freeing of per-extent infos
>>    fscrypt: allow load/save of extent contexts
>>    fscrypt: disable inline encryption for extent-based encryption
>>    fscrypt: update documentation to mention per-extent keys.
>>
>>   Documentation/filesystems/fscrypt.rst |  38 +++-
>>   fs/crypto/crypto.c                    |  17 +-
>>   fs/crypto/fname.c                     |   9 +-
>>   fs/crypto/fscrypt_private.h           | 174 +++++++++++++----
>>   fs/crypto/hooks.c                     |   2 +-
>>   fs/crypto/inline_crypt.c              |  42 ++--
>>   fs/crypto/keyring.c                   |  67 ++++---
>>   fs/crypto/keysetup.c                  | 263 ++++++++++++++++++++------
>>   fs/crypto/keysetup_v1.c               |  24 +--
>>   fs/crypto/policy.c                    |  28 ++-
>>   include/linux/fscrypt.h               |  76 ++++++++
>>   11 files changed, 580 insertions(+), 160 deletions(-)
>>
>>
>> base-commit: b7af0635c87ff78d6bd523298ab7471f9ffd3ce5
>> --
>> 2.38.1
>>
> 
> I'm surprised that this submission generated no discussion across a
> timeframe of over a month. Is this normal for RFC patch sets?

Eric pointed out some issues with patches 1 and 15 on 1/2. I've been on 
parental leave and have been busier with new little one than expected, 
and haven't sent out a new version yet. But I'm back to work in a week 
and this is my primary priority.
Eric Biggers Feb. 22, 2023, 8:53 p.m. UTC | #3
On Wed, Feb 22, 2023 at 09:13:47AM -0500, Sweet Tea Dorminy wrote:
> > 
> > I'm surprised that this submission generated no discussion across a
> > timeframe of over a month. Is this normal for RFC patch sets?
> 
> Eric pointed out some issues with patches 1 and 15 on 1/2. I've been on
> parental leave and have been busier with new little one than expected, and
> haven't sent out a new version yet. But I'm back to work in a week and this
> is my primary priority.

IMO, most of the patchset will change as a result of addressing my feedback.  So
I haven't had much motivation to review the current version in detail.  I'm
looking forward to the next version; I'm glad you're still working on it!

- Eric