Re: [PATCH 03/18] f2fs crypto: declare some definitions for f2fs encryption feature

Dave Chinner
2015-05-14T00:37:39Z

On Tue, May 12, 2015 at 11:48:02PM -0700, Jaegeuk Kim wrote:
> On Wed, May 13, 2015 at 12:02:08PM +1000, Dave Chinner wrote:
> > On Fri, May 08, 2015 at 09:20:38PM -0700, Jaegeuk Kim wrote:
> > > This definitions will be used by inode and superblock for encyption.
> >
> > How much of this crypto stuff is common with or only slightly
> > modified from the ext4 code? Is the behaviour and features the
> > same? Is the user API and management tools the same?
> >
> > IMO, if there is any amount of overlap, then we should be
> > implementing this stuff as generic code, not propagating the same
> > code through multiple filesystems via copy-n-paste-n-modify. This
> > will simply end up with diverging code, different bugs and feature
> > sets, and none of the implementations will get the review and
> > maintenance they really require...
> >
> > And, FWIW, this is the reason why I originally asked for the ext4
> > encryption code to be pulled up to the VFS: precisely so we didn't
> > end up with a rapid proliferation of individual in-filesystem
> > encryption implementations that are all slightly different...
>
> Totally agreed!
>
> AFAIK, Ted wants to push the codes as a crypto library into fs/ finally, so
> I believe most part of crypto codes are common.

Can I suggest fs/crypto/ if there are going to be multiple files?

> But, in order to realize that quickly, Ted implemented the feature to finalize
> on-disk and in-memory design in EXT4 as a first step.
> Then, I've been catching up and validating its design by implementing it in
> F2FS, which also intends to figure out what crypto codes can be exactly common.

Excellent. That will make it easier and less error prone for other
filesystems to implement it, too!

> As Ted mentioned before, since next android version tries to use per-file
> encryption, F2FS also needs to support it as quick as possible likewise EXT4.

Fair enough.

> Meanwhile, surely I've been working on writing patches to push them into fs/;
> currenlty, I did for cryto.c and will do for crypto_key.c and crypto_fname.c.
> But, it needs to think about crypto_policy.c differently, since it may depend
> on how each filesystem stores the policy information respectively; we cannot
> push all the filesystems should use xattrs, right?

All filesystems likely to implement per-file crypto support xattrs,
and this is exactly what xattrs are designed for. e.g. we already
require xattrs for generic security labels, ACLs, etc. Hence
per-file crypto information should also use a common, shared xattr
format. That way it only needs to be implemented once in the generic
code and there's very little (hopefully nothing!) each filesystem
has to customise to store the crypto information for each file.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com