On Mon, May 17, 2021 at 03:27:48PM -0700, Omar Sandoval wrote:

On Mon, May 17, 2021 at 02:32:47PM -0700, Linus Torvalds wrote:

quoted

On Mon, May 17, 2021 at 11:35 AM Omar Sandoval [off-list ref] wrote:

quoted

Patches 1-3 add the VFS support, UAPI, and documentation. Patches 4-7
are Btrfs prep patches. Patch 8 adds Btrfs encoded read support and
patch 9 adds Btrfs encoded write support.

I don't love the RWF_ENCODED flag, but if that's the way people think
this should be done, as a model this looks reasonable to me.

I'm not sure what the deal with the encryption metadata is. I realize
there is currently only one encryption type ("none") in this series,
but it's not clear how any other encryption type would actually ever
be described. It's not like you can pass in the key (well, I guess
passing in the key would be fine, but passing it back out certainly
would not be).  A key ID from a keyring?

So there's presumably some future plan for it, but it would be good to
verify that that plan makes sense..

What I'm imagining for fscrypt is:

1. Add ENCODED_IOV_ENCRYPTION_* types for fscrypt. Consumers at least
   need to be able to distinguish between encryption policy versions,
   DIRECT_KEY policies, and IV_INO_LBLK_{64,32} policies, and maybe
   other details.
2. Use RWF_ENCODED only for the data itself.
3. Add new fscrypt ioctls to get and set the encryption key.

The interesting part is (3). If I'm reading the fscrypt documentation
correctly, in the default mode, each file is encrypted with a per-file
key that is a function of the master key for the directory tree and a
per-file nonce.

Userspace manages the master key, we have a FS_IOC_GET_ENCRYPTION_NONCE
ioctl, and the key derivation function is documented. So, userspace
already has all of the pieces it needs to get the encryption key, and
all of the information it needs to decrypt the data it gets from
RWF_ENCODED if it so desires.

On the set/write side, the user can set the same master key and policy
with FS_IOC_SET_ENCRYPTION_POLICY, and we'd need something like an
FS_IOC_SET_ENCRYPTION_NONCE ioctl (possibly with a requirement that it
be set when the file is empty). I think that's it.

The details will vary for the other fscrypt policies, but that's the
gist of it. I added the fscrypt maintainers to correct me if I missed
something.

Well, assuming we're talking about regular files only (so file contents
encryption, not filenames encryption), with fscrypt the information needed to
understand a file's encrypted data is the following:

1. The encryption key

2. The filesystem's block size

3. The encryption context:

    struct fscrypt_context_v2 {                                                      
         u8 version; /* FSCRYPT_CONTEXT_V2 */                                     
         u8 contents_encryption_mode;                                             
         u8 filenames_encryption_mode;                                            
         u8 flags;                                                                
         u8 __reserved[4];                                                        
         u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];                   
         u8 nonce[FSCRYPT_FILE_NONCE_SIZE];                                       
    };                                                                               

   (Or alternatively struct fscrypt_policy_v2 + the nonce field separately;
    that results in the same fields as struct fscrypt_context_v2.)

This is definitely more complex than the compression cases like "the data is a
zlib stream".  So the question is, how much of this metadata (if any) should
actually be passed around during RWF_ENCODED pread/pwrite operations, and how
much should be out-of-band.

I feel like this should be mostly out-of-band (e.g. via the existing ioctls
FS_IOC_{GET,SET}_ENCRYPTION_POLICY), especially given that compression and
encryption could be combined which would make describing the on-disk data even
more difficult.

But I'm not sure what you intended.

- Eric