Consolidated file encryption interface/semantics?

Consolidated file encryption interface/semantics?

[Richard Weinberger]

Hi!

I consider adding file encryption to UBIFS.
While looking into ext4 and f2fs I realized that both
use the same data structures/concepts.

f2fs copy&pasted a lot from ext4.
Before I do the next copy&paste, I'd to ask whether it would make sense
to more parts of the ioctl() interface out to VFS?

Let's checkout the user visible interface:

ext4 offers:
EXT4_IOC_SET_ENCRYPTION_POLICY
EXT4_IOC_GET_ENCRYPTION_PWSALT
EXT4_IOC_GET_ENCRYPTION_POLICY

with:
#define EXT4_KEY_DESCRIPTOR_SIZE 8

/* Policy provided via an ioctl on the topmost directory */
struct ext4_encryption_policy {
        char version;
        char contents_encryption_mode;
        char filenames_encryption_mode;
        char flags;
        char master_key_descriptor[EXT4_KEY_DESCRIPTOR_SIZE];
} __attribute__((__packed__));


f2fs:
F2FS_IOC_SET_ENCRYPTION_POLICY
F2FS_IOC_GET_ENCRYPTION_POLICY
F2FS_IOC_GET_ENCRYPTION_PWSALT

#define F2FS_KEY_DESCRIPTOR_SIZE        8

/* Policy provided via an ioctl on the topmost directory */
struct f2fs_encryption_policy {
        char version;
        char contents_encryption_mode;
        char filenames_encryption_mode;
        char flags;
        char master_key_descriptor[F2FS_KEY_DESCRIPTOR_SIZE];
} __attribute__((__packed__));

So, the data structures are identical and AFAIK also the supported cipher modes are.
But as both use their own ioctls having a single tool to control file encryption
can be error prone in future.
Interestingly the current ioctls for ext4 and f2fs resolve to the same integers,
is this on purpose? :)

Wouldn't it be worthwhile having exactly the same ioctls such that util-linux could offer
a decent file encryption tool which can be used by all file systems with file encryption
support?

Another thing are semantics, ext4 implemented a policy which controls
under which conditions encrypted files are allowed to be unlinked, moved, etc...
f2fs adopted these from ext4. But can't we do that in VFS or at least
agree one a policy and document it? :-)

Thanks,
//richard

Re: Consolidated file encryption interface/semantics?

[Dave Chinner]

On Mon, Jan 11, 2016 at 11:56:25PM +0100, Richard Weinberger wrote:
> Hi!
> 
> I consider adding file encryption to UBIFS.
> While looking into ext4 and f2fs I realized that both
> use the same data structures/concepts.

When the ext4 code was first posted, I noted that 90% of the
implementation was not ext4 specific at all. I asked that the
encryption part of it be lifted to fs/ and the API made generic so
that it can be shared without needing to copy and re-review
encryption code (which is notoriously hard to get right) when others
implement the same fucntionality. This was considered SEP.

> f2fs copy&pasted a lot from ext4.

Yes, they did, even though I asked them not to, but to instead pull
all the code that is non-ext4 specific up to the VFS first and then
use that. Again, this was considered SEP.

> Before I do the next copy&paste, I'd to ask whether it would make sense
> to more parts of the ioctl() interface out to VFS?

Yes, unfortunately you're the first "somebody else" who is going to
have to deal with this problem. It's not just the API - most of the
encryption implementation is also copy and pasted, too.

This simply hasn't got to the top of my list of stuff to do for XFS
yet, so I haven't started down this path yet. However, if you do
start on the hard work  to pull all this stuff up into the VFS and
generic helpers, I'll do what I can to get the necessary parts of
XFS working with it as well....

[snip]

> So, the data structures are identical and AFAIK also the supported cipher modes are.
> But as both use their own ioctls having a single tool to control file encryption
> can be error prone in future.
> Interestingly the current ioctls for ext4 and f2fs resolve to the same integers,
> is this on purpose? :)

Yes, so that when someone factors it all into a generic
implementation the new "FS_IOC*" ioctl API will be binary compatible
with the existing ext4 and f2fs ioctl implementations and the
existing userspace tools will just continue to work.

> Wouldn't it be worthwhile having exactly the same ioctls such that util-linux could offer
> a decent file encryption tool which can be used by all file systems with file encryption
> support?

Yes, it should have been done that way in the first place.

> Another thing are semantics, ext4 implemented a policy which controls
> under which conditions encrypted files are allowed to be unlinked, moved, etc...
> f2fs adopted these from ext4. But can't we do that in VFS or at least
> agree one a policy and document it? :-)

Yes, we should. Having a single set of policies implemented and
enforced at the VFS is how this should be done. Users will
absolutely hate us if per-file encryption devolves into APIs and
policies that differ between filesystems.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: Consolidated file encryption interface/semantics?

[Theodore Ts'o]

On Mon, Jan 11, 2016 at 11:56:25PM +0100, Richard Weinberger wrote:
> 
> So, the data structures are identical and AFAIK also the supported cipher modes are.
> But as both use their own ioctls having a single tool to control file encryption
> can be error prone in future.
> Interestingly the current ioctls for ext4 and f2fs resolve to the same integers,
> is this on purpose? :)

As far as tools to control file encryption, there is the beginnings of
such a tool in the e2fsprogs directory, called e4crypt[1], which I've
been using to do some of my testing.  There is also a dummy encryption
key mount option for stress testing using xfstests.

[1] http://git.kernel.org/cgit/fs/ext2/e2fsprogs.git/tree/misc/e4crypt.c

There is also code in the Android Open Source Project which uses these
ioctls although the way the user key management is handled is a bit
different.  (In particular, they don't use the per-file system salt,
but instead use a different mechanism to derive the key from stored
secret information --- which could potentially use ARM Trustzone for
those devices that have it --- and the user's pin / password.)

Since the primary / initial user for this code has been in the Android
Userspace, the e4crypt tool hasn't gotten as much attention as I would
like.  One of the thigns on my "to-implement" list is storing the
per-file system password salts which it knows about in a dot file so
that if you are using multiple USB thumb drives, when you enter the
password once, the password can be transformed into different keys for
the different USB thumb drives, even if some of them aren't currently
available to the system because they currently aren't plugged in and
available to the system.

> Another thing are semantics, ext4 implemented a policy which controls
> under which conditions encrypted files are allowed to be unlinked, moved, etc...
> f2fs adopted these from ext4. But can't we do that in VFS or at least
> agree one a policy and document it? :-)

It's been on my todo list to uplift the common code into a VFS
library, and so it wasn't my intent to make it be someone else's
problems.  I just haven't had time to get to that point yet, and I
also wanted to make sure we had adequate implementation and practice
experience before we uplifted the code, since it was faster to make
changes when all of the code is in a single layer.  Hopefully I will
have time to start work on this in the next month or two.

The final thing I'll note is that for *most* use cases, it makes for
more sense to do encryption at the block layer.  It's only if you need
per-user, or per-work profile keying, then maybe it makes sense to
have file system level encryption.  So you might want to consider
whether for your intended use case, whether or not it makes sense for
UBIFS to have encryption, or whether you would be better off doing it
below the file system, at the block or MTD layer.

Cheers,

					- Ted

P.S.  Also on the todo list is to update the following document from
"as designed" to "as built".  In particular there were some
late-breaking changes in how file naem and symlink encryption were
handled that didn't get reflected in the design doc:

https://docs.google.com/document/d/1ft26lUQyuSpiu6VleP70_npaWdRfXFoNnB8JYnykNTg/edit#

Re: [f2fs-dev] Consolidated file encryption interface/semantics?

[Jaegeuk Kim]

Hello,

Actually, I tried to prepare this quite long time ago [1], which was stuck
that moment unfortunately, since I needed to wait for how AOSP finally treats
with this feature. At some moment later, I couldn't even follow up every ext4
changes into this patch set, since the feature was not quickly settled down in
AOSP rather than what I expected.

But, now it seems that there's no strong reason to postpone this work any more.
If Ted doen't mind, I'd like to investigate all the diffs between ext4 and f2fs
first. And, in the mean time, let me get any comments about missing or ugly
design points in this patch set.
(Please, take a look at its basic approach only.)

[1] http://git.kernel.org/cgit/linux/kernel/git/jaegeuk/f2fs.git/log/?h=crypto

This patch set adds the following files.
 - include/linux/fscrypt.h
 - fs/crypto/crypto.c
 - fs/crypto/policy.c
 - fs/crypto/keyinfo.c
 - fs/crypto/fname.c

And, it adds the below function pointers for each filesystems.

struct fscrypt_operations {
	int (*get_context)(struct inode *, void *, size_t, void *);
	int (*set_context)(struct inode *, const void *, size_t, int, void *);
	bool (*is_encrypted)(struct inode *);
	bool (*empty_dir)(struct inode *);
	unsigned (*max_namelen)(struct inode *);
};

Thanks,

On Mon, Jan 11, 2016 at 11:56:25PM +0100, Richard Weinberger wrote:
> Hi!
> 
> I consider adding file encryption to UBIFS.
> While looking into ext4 and f2fs I realized that both
> use the same data structures/concepts.
> 
> f2fs copy&pasted a lot from ext4.
> Before I do the next copy&paste, I'd to ask whether it would make sense
> to more parts of the ioctl() interface out to VFS?
> 
> Let's checkout the user visible interface:
> 
> ext4 offers:
> EXT4_IOC_SET_ENCRYPTION_POLICY
> EXT4_IOC_GET_ENCRYPTION_PWSALT
> EXT4_IOC_GET_ENCRYPTION_POLICY
> 
> with:
> #define EXT4_KEY_DESCRIPTOR_SIZE 8
> 
> /* Policy provided via an ioctl on the topmost directory */
> struct ext4_encryption_policy {
>         char version;
>         char contents_encryption_mode;
>         char filenames_encryption_mode;
>         char flags;
>         char master_key_descriptor[EXT4_KEY_DESCRIPTOR_SIZE];
> } __attribute__((__packed__));
> 
> 
> f2fs:
> F2FS_IOC_SET_ENCRYPTION_POLICY
> F2FS_IOC_GET_ENCRYPTION_POLICY
> F2FS_IOC_GET_ENCRYPTION_PWSALT
> 
> #define F2FS_KEY_DESCRIPTOR_SIZE        8
> 
> /* Policy provided via an ioctl on the topmost directory */
> struct f2fs_encryption_policy {
>         char version;
>         char contents_encryption_mode;
>         char filenames_encryption_mode;
>         char flags;
>         char master_key_descriptor[F2FS_KEY_DESCRIPTOR_SIZE];
> } __attribute__((__packed__));
> 
> So, the data structures are identical and AFAIK also the supported cipher modes are.
> But as both use their own ioctls having a single tool to control file encryption
> can be error prone in future.
> Interestingly the current ioctls for ext4 and f2fs resolve to the same integers,
> is this on purpose? :)
> 
> Wouldn't it be worthwhile having exactly the same ioctls such that util-linux could offer
> a decent file encryption tool which can be used by all file systems with file encryption
> support?
> 
> Another thing are semantics, ext4 implemented a policy which controls
> under which conditions encrypted files are allowed to be unlinked, moved, etc...
> f2fs adopted these from ext4. But can't we do that in VFS or at least
> agree one a policy and document it? :-)
> 
> Thanks,
> //richard
> 
> ------------------------------------------------------------------------------
> Site24x7 APM Insight: Get Deep Visibility into Application Performance
> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
> Monitor end-to-end web transactions and take corrective actions now
> Troubleshoot faster and improve end-user experience. Signup Now!
> http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] Consolidated file encryption interface/semantics?

[Theodore Ts'o]

On Mon, Jan 11, 2016 at 11:47:56PM -0800, Jaegeuk Kim wrote:
> 
> Actually, I tried to prepare this quite long time ago [1], which was stuck
> that moment unfortunately, since I needed to wait for how AOSP finally treats
> with this feature. At some moment later, I couldn't even follow up every ext4
> changes into this patch set, since the feature was not quickly settled down in
> AOSP rather than what I expected.

Yeah, unfortunately the feature didn't end up landing in M because
there was an ARM/Android specific bug that couldn't be found using
xfstests running on x86 (this is why I detoured to port xfstests and
xfsprogs so they could build against bionic C library; I'm still
hoping to find someone who is willing to help create an adb-xfstests
system ala kvm-xfstests and gce-xfstests which fastboots the kernel to
be tested and then launches the tests and collects the results using
adb, by the way).

In any case, I did finally find the bug, but not in time for the M
release.  So if you take the latest ext4 crypto bug fixes from
upstream and apply them to the AOSP versions of the Nexus 9, Nexus 5X,
and Nexus 6P kernels, and then replace AOSP version of e2fsprogs with
the upstream e2fsprogs (use the master or next branch, and run the
script ./util/gen-android-files to create the Android.mk files), and
finally then change the fstab file in the 
aosp/devices/<manufacturer>/<model>/fstab.<model> file and change the
mount options so instead of "forceencrypt=/dev/block/platform/..." it
uses the mount option "fileencryption" for the /data partition, this
should allow you to build an AOSP system image that should work well
enough to demo / test the kernel features.

Some of the future work that I have planned or in progress:

1) Patches that allow userspace to move / backup a set of encrypted
files from one file system to another without having access to the
key.  Dmitri asked for this as a feature request for desktop/laptop
uses of ext4 encryption, and happily it turns out there is a reason
why this would be useful for Android as well.  These patches have been
floated on the ext4 list, but I don't yet have support for moving /
backing up symlinks, so these patches won't be landing during the
current merge window.

2) Work to allow IP blocks that have line speed encryption engines,
such Qualcomm (as evidenced by the patch they tried to send for
ecryptfs at https://lkml.org/lkml/2015/11/9/660), but one which is
done in an upstream acceptable way, which means plumbing the crypto
support through the block layer in a way which is SOC vendor agnostic.
(Probably using a similar mechanism to how we tie DIF/DIX information
to a block I/O request.)  I'm hoping Qualcomm and possibly other SOC
vendors will be willing to work with me so this we can have something
which is upstream acceptable (since I don't plan to accept a patch
which adds some random, hacky, function pointers to code that only
lives in a BSP kernel).  This may be a useful thing to discuss at the
in Raleigh at the LSF/MM workshop in April, assuming I can get
representatives from the relevant SOC vendors to show up.

> But, now it seems that there's no strong reason to postpone this work any more.
> If Ted doen't mind, I'd like to investigate all the diffs between ext4 and f2fs
> first.

Sure, if you could help with this I'd greatly appreciate it.

There have been a number of improvements and optimizations since the
last time you sync'ed the f2fs code with ext4, but I'm sure you'll see
that as you look at the various commits.

Regards,

					- Ted