Re: [PATCH 00/29] UBIFS File Encryption v1

[Theodore Ts'o]

On Sun, Nov 27, 2016 at 05:27:58PM -0800, Eric Biggers wrote:
> 
> Shouldn't the branch be rebased to remove the CONFIG_VMAP_STACK fixes which 
> are
> already in Linus' tree?
> 
>   fscrypto: don't use on-stack buffer for key derivation
>   fscrypto: don't use on-stack buffer for filename encryption
> 
> Otherwise we'll end up with duplicate commits.

Given that the ubifs folks are depending on the existing branch,
having duplicate commits is considered an acceptable tradeoff to not
rebasing a published commit that other trees are depending on.

I tell people that the ext4.git dev branch is a rewinding branch, so
people shouldn't be building other trees on top of it unless they are
willing to deal with the fact that it can be rebased.  However, i
didn't give that warning for the fscrypt branch, so I'd much rather
not rewind/rebase it.

- Ted

Re: [PATCH 00/29] UBIFS File Encryption v1

[Theodore Ts'o]

On Sun, Nov 27, 2016 at 05:27:58PM -0800, Eric Biggers wrote:
> 
> Shouldn't the branch be rebased to remove the CONFIG_VMAP_STACK fixes which 
> are
> already in Linus' tree?
> 
>   fscrypto: don't use on-stack buffer for key derivation
>   fscrypto: don't use on-stack buffer for filename encryption
> 
> Otherwise we'll end up with duplicate commits.

Given that the ubifs folks are depending on the existing branch,
having duplicate commits is considered an acceptable tradeoff to not
rebasing a published commit that other trees are depending on.

I tell people that the ext4.git dev branch is a rewinding branch, so
people shouldn't be building other trees on top of it unless they are
willing to deal with the fact that it can be rebased.  However, i
didn't give that warning for the fscrypt branch, so I'd much rather
not rewind/rebase it.

- Ted

Re: [PATCH 00/29] UBIFS File Encryption v1

[Eric Biggers]

On Sun, Nov 27, 2016 at 11:21:56PM +0100, Richard Weinberger wrote:
> Ted,
> 
> On 27.11.2016 18:52, Theodore Ts'o wrote:
> > On Fri, Nov 25, 2016 at 09:18:12AM +0100, Richard Weinberger wrote:
> >>
> >> Do you want us to address Eric's review comments on top of the fscrypt
> >> branch or shall we rebase?
> >> I'd suggest the former.
> > 
> > Yes, let's address them on top of the existing fscrypt branch.  I
> > don't consider any of his comments super-serious --- they were mostly
> > documentation or comments level changes unless I missed something.
> 
> Okay. Then I'll queue UBIFS encryption for the v4.10 merge window.
> Just to be sure, I base my UBIFS next tree on your fscrypt tree such that
> it will build fine and Linus won't see same commits with a different sha1?
> Usually I'm a lucky maintainer and not have to deal with dependencies
> between pull requests. :-)
> 
> Thanks,
> //richard

Shouldn't the branch be rebased to remove the CONFIG_VMAP_STACK fixes which are
already in Linus' tree?

fscrypto: don't use on-stack buffer for key derivation
fscrypto: don't use on-stack buffer for filename encryption

Otherwise we'll end up with duplicate commits.

Eric

Re: [PATCH 00/29] UBIFS File Encryption v1

[Eric Biggers]

On Sun, Nov 27, 2016 at 11:21:56PM +0100, Richard Weinberger wrote:
> Ted,
> 
> On 27.11.2016 18:52, Theodore Ts'o wrote:
> > On Fri, Nov 25, 2016 at 09:18:12AM +0100, Richard Weinberger wrote:
> >>
> >> Do you want us to address Eric's review comments on top of the fscrypt
> >> branch or shall we rebase?
> >> I'd suggest the former.
> > 
> > Yes, let's address them on top of the existing fscrypt branch.  I
> > don't consider any of his comments super-serious --- they were mostly
> > documentation or comments level changes unless I missed something.
> 
> Okay. Then I'll queue UBIFS encryption for the v4.10 merge window.
> Just to be sure, I base my UBIFS next tree on your fscrypt tree such that
> it will build fine and Linus won't see same commits with a different sha1?
> Usually I'm a lucky maintainer and not have to deal with dependencies
> between pull requests. :-)
> 
> Thanks,
> //richard

Shouldn't the branch be rebased to remove the CONFIG_VMAP_STACK fixes which are
already in Linus' tree?

fscrypto: don't use on-stack buffer for key derivation
fscrypto: don't use on-stack buffer for filename encryption

Otherwise we'll end up with duplicate commits.

Eric

Re: [PATCH 00/29] UBIFS File Encryption v1

[Theodore Ts'o]

On Sun, Nov 27, 2016 at 11:21:56PM +0100, Richard Weinberger wrote:
> 
> Okay. Then I'll queue UBIFS encryption for the v4.10 merge window.
> Just to be sure, I base my UBIFS next tree on your fscrypt tree such that
> it will build fine and Linus won't see same commits with a different sha1?

Yep, if you pull down the dev branch from ext4.git, and run the
command:

git log --merges ext4/origin^..ext4/dev

you should see:

commit 6da22013bb7907b33c87968c25034b409a6161a2
Merge: a2f6d9c4c081 a6e089128617
Author: Theodore Ts'o 
Date:   Sun Nov 13 22:02:22 2016 -0500

Merge branch 'fscrypt' into origin

commit a2f6d9c4c081ec2a02529b8af2c04f3e557a3a3e
Merge: bc33b0ca11e3 9484ab1bf446
Author: Theodore Ts'o 
Date:   Sun Nov 13 22:02:15 2016 -0500

Merge branch 'dax-4.10-iomap-pmd' into origin

Cheers,

- Ted

Re: [PATCH 00/29] UBIFS File Encryption v1

[Theodore Ts'o]

On Sun, Nov 27, 2016 at 11:21:56PM +0100, Richard Weinberger wrote:
> 
> Okay. Then I'll queue UBIFS encryption for the v4.10 merge window.
> Just to be sure, I base my UBIFS next tree on your fscrypt tree such that
> it will build fine and Linus won't see same commits with a different sha1?

Yep, if you pull down the dev branch from ext4.git, and run the
command:

git log --merges ext4/origin^..ext4/dev

you should see:

commit 6da22013bb7907b33c87968c25034b409a6161a2
Merge: a2f6d9c4c081 a6e089128617
Author: Theodore Ts'o 
Date:   Sun Nov 13 22:02:22 2016 -0500

Merge branch 'fscrypt' into origin

commit a2f6d9c4c081ec2a02529b8af2c04f3e557a3a3e
Merge: bc33b0ca11e3 9484ab1bf446
Author: Theodore Ts'o 
Date:   Sun Nov 13 22:02:15 2016 -0500

Merge branch 'dax-4.10-iomap-pmd' into origin

Cheers,

- Ted

Re: [PATCH 00/29] UBIFS File Encryption v1

[Richard Weinberger]

Ted,

On 27.11.2016 18:52, Theodore Ts'o wrote:
> On Fri, Nov 25, 2016 at 09:18:12AM +0100, Richard Weinberger wrote:
>>
>> Do you want us to address Eric's review comments on top of the fscrypt
>> branch or shall we rebase?
>> I'd suggest the former.
> 
> Yes, let's address them on top of the existing fscrypt branch.  I
> don't consider any of his comments super-serious --- they were mostly
> documentation or comments level changes unless I missed something.

Okay. Then I'll queue UBIFS encryption for the v4.10 merge window.
Just to be sure, I base my UBIFS next tree on your fscrypt tree such that
it will build fine and Linus won't see same commits with a different sha1?
Usually I'm a lucky maintainer and not have to deal with dependencies
between pull requests. :-)

Thanks,
//richard

Re: [PATCH 00/29] UBIFS File Encryption v1

[Richard Weinberger]

Ted,

On 27.11.2016 18:52, Theodore Ts'o wrote:
> On Fri, Nov 25, 2016 at 09:18:12AM +0100, Richard Weinberger wrote:
>>
>> Do you want us to address Eric's review comments on top of the fscrypt
>> branch or shall we rebase?
>> I'd suggest the former.
> 
> Yes, let's address them on top of the existing fscrypt branch.  I
> don't consider any of his comments super-serious --- they were mostly
> documentation or comments level changes unless I missed something.

Okay. Then I'll queue UBIFS encryption for the v4.10 merge window.
Just to be sure, I base my UBIFS next tree on your fscrypt tree such that
it will build fine and Linus won't see same commits with a different sha1?
Usually I'm a lucky maintainer and not have to deal with dependencies
between pull requests. :-)

Thanks,
//richard

Re: [PATCH 00/29] UBIFS File Encryption v1

[Theodore Ts'o]

On Fri, Nov 25, 2016 at 09:18:12AM +0100, Richard Weinberger wrote:
> 
> Do you want us to address Eric's review comments on top of the fscrypt
> branch or shall we rebase?
> I'd suggest the former.

Yes, let's address them on top of the existing fscrypt branch.  I
don't consider any of his comments super-serious --- they were mostly
documentation or comments level changes unless I missed something.

   - Ted

Re: [PATCH 00/29] UBIFS File Encryption v1

[Theodore Ts'o]

On Fri, Nov 25, 2016 at 09:18:12AM +0100, Richard Weinberger wrote:
> 
> Do you want us to address Eric's review comments on top of the fscrypt
> branch or shall we rebase?
> I'd suggest the former.

Yes, let's address them on top of the existing fscrypt branch.  I
don't consider any of his comments super-serious --- they were mostly
documentation or comments level changes unless I missed something.

   - Ted

Re: [PATCH 00/29] UBIFS File Encryption v1

[Richard Weinberger]

Ted,

On 14.11.2016 04:05, Theodore Ts'o wrote:
> Richard,
> 
> Your fscrypt patches look good.  I've created an fscrypt branch on the
> ext4.git tree which contains your changes as well as Eric Bigger's
> recent fspatch cleanups changes.  If you want to base your ubifs
> changes on that branch, that would be great.  The ext4 dev branch will
> be including that fscrypt branch, so it will be feeding into
> linux-next that way.  If you also base your patches on that, it will
> avoid duplicate patches in linux-next and in Linus's tree when he
> pulls them.

Do you want us to address Eric's review comments on top of the fscrypt
branch or shall we rebase?
I'd suggest the former.

Thanks,
//richard

Re: [PATCH 00/29] UBIFS File Encryption v1

[Richard Weinberger]

Ted,

On 14.11.2016 04:05, Theodore Ts'o wrote:
> Richard,
> 
> Your fscrypt patches look good.  I've created an fscrypt branch on the
> ext4.git tree which contains your changes as well as Eric Bigger's
> recent fspatch cleanups changes.  If you want to base your ubifs
> changes on that branch, that would be great.  The ext4 dev branch will
> be including that fscrypt branch, so it will be feeding into
> linux-next that way.  If you also base your patches on that, it will
> avoid duplicate patches in linux-next and in Linus's tree when he
> pulls them.

Do you want us to address Eric's review comments on top of the fscrypt
branch or shall we rebase?
I'd suggest the former.

Thanks,
//richard

Re: [PATCH 00/29] UBIFS File Encryption v1

[Richard Weinberger]

Ted,

On 14.11.2016 04:05, Theodore Ts'o wrote:
> Richard,
> 
> Your fscrypt patches look good.  I've created an fscrypt branch on the
> ext4.git tree which contains your changes as well as Eric Bigger's
> recent fspatch cleanups changes.  If you want to base your ubifs
> changes on that branch, that would be great.  The ext4 dev branch will
> be including that fscrypt branch, so it will be feeding into
> linux-next that way.  If you also base your patches on that, it will
> avoid duplicate patches in linux-next and in Linus's tree when he
> pulls them.

Will do!

> One quick question.  When ubifs uses subpage blocks, can you assume that
> they are always powers of two?  Or more to the point, can you assume
> that it will always be a multiple of the cipher's blocksize?

Yes, UBIFS takes care about that. Subpage blocks are always a multiple of
FS_CRYPTO_BLOCK_SIZE.

Thanks,
//richard

Re: [PATCH 00/29] UBIFS File Encryption v1

[Richard Weinberger]

Ted,

On 14.11.2016 04:05, Theodore Ts'o wrote:
> Richard,
> 
> Your fscrypt patches look good.  I've created an fscrypt branch on the
> ext4.git tree which contains your changes as well as Eric Bigger's
> recent fspatch cleanups changes.  If you want to base your ubifs
> changes on that branch, that would be great.  The ext4 dev branch will
> be including that fscrypt branch, so it will be feeding into
> linux-next that way.  If you also base your patches on that, it will
> avoid duplicate patches in linux-next and in Linus's tree when he
> pulls them.

Will do!

> One quick question.  When ubifs uses subpage blocks, can you assume that
> they are always powers of two?  Or more to the point, can you assume
> that it will always be a multiple of the cipher's blocksize?

Yes, UBIFS takes care about that. Subpage blocks are always a multiple of
FS_CRYPTO_BLOCK_SIZE.

Thanks,
//richard

Re: [PATCH 00/29] UBIFS File Encryption v1

[Theodore Ts'o]

Richard,

Your fscrypt patches look good.  I've created an fscrypt branch on the
ext4.git tree which contains your changes as well as Eric Bigger's
recent fspatch cleanups changes.  If you want to base your ubifs
changes on that branch, that would be great.  The ext4 dev branch will
be including that fscrypt branch, so it will be feeding into
linux-next that way.  If you also base your patches on that, it will
avoid duplicate patches in linux-next and in Linus's tree when he
pulls them.

One quick question.  When ubifs uses subpage blocks, can you assume that
they are always powers of two?  Or more to the point, can you assume
that it will always be a multiple of the cipher's blocksize?

- Ted

Re: [PATCH 00/29] UBIFS File Encryption v1

[Theodore Ts'o]

Richard,

Your fscrypt patches look good.  I've created an fscrypt branch on the
ext4.git tree which contains your changes as well as Eric Bigger's
recent fspatch cleanups changes.  If you want to base your ubifs
changes on that branch, that would be great.  The ext4 dev branch will
be including that fscrypt branch, so it will be feeding into
linux-next that way.  If you also base your patches on that, it will
avoid duplicate patches in linux-next and in Linus's tree when he
pulls them.

One quick question.  When ubifs uses subpage blocks, can you assume that
they are always powers of two?  Or more to the point, can you assume
that it will always be a multiple of the cipher's blocksize?

- Ted

[PATCH 00/29] UBIFS File Encryption v1

[Richard Weinberger]

This patch series implements file level encryption for UBIFS.
It makes use of the generic fscrypto framework as used by ext4 and f2fs.
Among file contents also file names are encrypted,
for more details on fscrypto please see [0] and [1].

To support encrypted files in UBIFS multiple preparations were needed.
The first five patches touch fscrypto code and add support for
kmalloc()'ed pages.
UBIFS has a different IO model than ext4 and f2fs because it uses MTD
instead of the block layer. But the changes are small and non-invasive.
In UBIFS itself the biggest change was supporting hash lookups.
Now UBIFS is able to provide a 64bit cookie which can be used later
to locate a file. This change will also allow us implementing proper
NFS and telldir() support, but that will be a different patch series.
Because of these changes the UBIFS write version is now 5.

As userspace component I'm currently using e4crypt from e2fsprogs with
EXT2FS_KEY_DESC_PREFIX set to "fscrypt:" instead of "ext4:".
A common tool will hopefully emerge soon[2]. I don't want an UBIFS
specific tool in mtd-utils.

The series is based on 4.9-rc3.
It can be obtained from:
git://git.infradead.org/users/rw/linux.git ubifs_crypt_v1

[0] https://lwn.net/Articles/639427/
[1] 
https://docs.google.com/document/d/1ft26lUQyuSpiu6VleP70_npaWdRfXFoNnB8JYnykNTg/edit
[2] http://www.spinics.net/lists/linux-fsdevel/msg103107.html

Changes since v0, https://lwn.net/Articles/704261/
 - Rebased to v4.9-rc4
 - Made fscrypto functions generic instead of adding new versions (hch)
 - Addressed various comments (Eric and Ted)

David Gstir (5):
  fscrypt: Add in-place encryption mode
  fscrypt: Allow fscrypt_decrypt_page() to function with non-writeback
pages
  fscrypt: Enable partial page encryption
  fscrypt: Constify struct inode pointer
  fscrypt: Let fs select encryption index/tweak

Richard Weinberger (24):
  ubifs: Export ubifs_check_dir_empty()
  ubifs: Export xattr get and set functions
  ubifs: Define UBIFS crypto context xattr
  ubifs: Add skeleton for fscrypto
  ubifs: Massage ubifs_listxattr() for encryption context
  ubifs: Implement directory open operation
  ubifs: Implement file open operation
  ubifs: Enforce crypto policy in ->link and ->rename
  ubifs: Preload crypto context in ->lookup()
  ubifs: Massage assert in ubifs_xattr_set() wrt. fscrypto
  ubifs: Enforce crypto policy in mmap
  ubifs: Introduce new data node field, compr_size
  ubifs: Constify struct inode pointer in ubifs_crypt_is_encrypted()
  ubifs: Implement encrypt/decrypt for all IO
  ubifs: Relax checks in ubifs_validate_entry()
  ubifs: Make r5 hash binary string aware
  ubifs: Implement encrypted filenames
  ubifs: Add support for encrypted symlinks
  ubifs: Rename tnc_read_node_nm
  ubifs: Add full hash lookup support
  ubifs: Use a random number for cookies
  ubifs: Implement UBIFS_FLG_DOUBLE_HASH
  ubifs: Implement UBIFS_FLG_ENCRYPTION
  ubifs: Raise write version to 5

 fs/crypto/crypto.c   |  83 
 fs/crypto/fname.c|   4 +-
 fs/ext4/inode.c  |   7 +-
 fs/ext4/page-io.c|   3 +-
 fs/f2fs/data.c   |   5 +-
 fs/ubifs/Kconfig |  11 ++
 fs/ubifs/Makefile|   1 +
 fs/ubifs/crypto.c|  97 ++
 fs/ubifs/debug.c |  14 +-
 fs/ubifs/dir.c   | 478 ---
 fs/ubifs/file.c  | 108 ++-
 fs/ubifs/ioctl.c |  40 
 fs/ubifs/journal.c   | 224 --
 fs/ubifs/key.h   |  21 ++-
 fs/ubifs/replay.c|  10 +-
 fs/ubifs/sb.c|  59 ++
 fs/ubifs/super.c |  17 +-
 fs/ubifs/tnc.c   | 168 +
 fs/ubifs/ubifs-media.h   |  29 ++-
 fs/ubifs/ubifs.h | 104 +--
 fs/ubifs/xattr.c | 116 +++-
 include/linux/fscrypto.h |  38 ++--
 22 files changed, 1299 insertions(+), 338 deletions(-)
 create mode 100644 fs/ubifs/crypto.c

-- 
2.7.3

[PATCH 00/29] UBIFS File Encryption v1

[Richard Weinberger]

This patch series implements file level encryption for UBIFS.
It makes use of the generic fscrypto framework as used by ext4 and f2fs.
Among file contents also file names are encrypted,
for more details on fscrypto please see [0] and [1].

To support encrypted files in UBIFS multiple preparations were needed.
The first five patches touch fscrypto code and add support for
kmalloc()'ed pages.
UBIFS has a different IO model than ext4 and f2fs because it uses MTD
instead of the block layer. But the changes are small and non-invasive.
In UBIFS itself the biggest change was supporting hash lookups.
Now UBIFS is able to provide a 64bit cookie which can be used later
to locate a file. This change will also allow us implementing proper
NFS and telldir() support, but that will be a different patch series.
Because of these changes the UBIFS write version is now 5.

As userspace component I'm currently using e4crypt from e2fsprogs with
EXT2FS_KEY_DESC_PREFIX set to "fscrypt:" instead of "ext4:".
A common tool will hopefully emerge soon[2]. I don't want an UBIFS
specific tool in mtd-utils.

The series is based on 4.9-rc3.
It can be obtained from:
git://git.infradead.org/users/rw/linux.git ubifs_crypt_v1

[0] https://lwn.net/Articles/639427/
[1] 
https://docs.google.com/document/d/1ft26lUQyuSpiu6VleP70_npaWdRfXFoNnB8JYnykNTg/edit
[2] http://www.spinics.net/lists/linux-fsdevel/msg103107.html

Changes since v0, https://lwn.net/Articles/704261/
 - Rebased to v4.9-rc4
 - Made fscrypto functions generic instead of adding new versions (hch)
 - Addressed various comments (Eric and Ted)

David Gstir (5):
  fscrypt: Add in-place encryption mode
  fscrypt: Allow fscrypt_decrypt_page() to function with non-writeback
pages
  fscrypt: Enable partial page encryption
  fscrypt: Constify struct inode pointer
  fscrypt: Let fs select encryption index/tweak

Richard Weinberger (24):
  ubifs: Export ubifs_check_dir_empty()
  ubifs: Export xattr get and set functions
  ubifs: Define UBIFS crypto context xattr
  ubifs: Add skeleton for fscrypto
  ubifs: Massage ubifs_listxattr() for encryption context
  ubifs: Implement directory open operation
  ubifs: Implement file open operation
  ubifs: Enforce crypto policy in ->link and ->rename
  ubifs: Preload crypto context in ->lookup()
  ubifs: Massage assert in ubifs_xattr_set() wrt. fscrypto
  ubifs: Enforce crypto policy in mmap
  ubifs: Introduce new data node field, compr_size
  ubifs: Constify struct inode pointer in ubifs_crypt_is_encrypted()
  ubifs: Implement encrypt/decrypt for all IO
  ubifs: Relax checks in ubifs_validate_entry()
  ubifs: Make r5 hash binary string aware
  ubifs: Implement encrypted filenames
  ubifs: Add support for encrypted symlinks
  ubifs: Rename tnc_read_node_nm
  ubifs: Add full hash lookup support
  ubifs: Use a random number for cookies
  ubifs: Implement UBIFS_FLG_DOUBLE_HASH
  ubifs: Implement UBIFS_FLG_ENCRYPTION
  ubifs: Raise write version to 5

 fs/crypto/crypto.c   |  83 
 fs/crypto/fname.c|   4 +-
 fs/ext4/inode.c  |   7 +-
 fs/ext4/page-io.c|   3 +-
 fs/f2fs/data.c   |   5 +-
 fs/ubifs/Kconfig |  11 ++
 fs/ubifs/Makefile|   1 +
 fs/ubifs/crypto.c|  97 ++
 fs/ubifs/debug.c |  14 +-
 fs/ubifs/dir.c   | 478 ---
 fs/ubifs/file.c  | 108 ++-
 fs/ubifs/ioctl.c |  40 
 fs/ubifs/journal.c   | 224 --
 fs/ubifs/key.h   |  21 ++-
 fs/ubifs/replay.c|  10 +-
 fs/ubifs/sb.c|  59 ++
 fs/ubifs/super.c |  17 +-
 fs/ubifs/tnc.c   | 168 +
 fs/ubifs/ubifs-media.h   |  29 ++-
 fs/ubifs/ubifs.h | 104 +--
 fs/ubifs/xattr.c | 116 +++-
 include/linux/fscrypto.h |  38 ++--
 22 files changed, 1299 insertions(+), 338 deletions(-)
 create mode 100644 fs/ubifs/crypto.c

-- 
2.7.3