Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1505839 > unrolled thread

[PATCH 00/26] UBIFS File Encryption

Started byRichard Weinberger <richard@nod.at>
First post2016-10-21 14:50 +0200
Last post2016-10-21 15:10 +0200
Articles 20 on this page of 49 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 00/26] UBIFS File Encryption Richard Weinberger <richard@nod.at> - 2016-10-21 14:50 +0200
    [PATCH 26/26] ubifs: Raise write version to 5 Richard Weinberger <richard@nod.at> - 2016-10-21 14:50 +0200
      Re: [PATCH 26/26] ubifs: Raise write version to 5 Michael Halcrow <mhalcrow@google.com> - 2016-10-21 19:40 +0200
        Re: [PATCH 26/26] ubifs: Raise write version to 5 Theodore Ts'o <tytso@mit.edu> - 2016-10-21 19:50 +0200
          Re: [PATCH 26/26] ubifs: Raise write version to 5 Eric Biggers <ebiggers@google.com> - 2016-10-21 20:20 +0200
            Re: [PATCH 26/26] ubifs: Raise write version to 5 Theodore Ts'o <tytso@mit.edu> - 2016-10-22 00:40 +0200
          Re: [PATCH 26/26] ubifs: Raise write version to 5 Richard Weinberger <richard@nod.at> - 2016-10-24 09:10 +0200
    [PATCH 05/26] ubifs: Define UBIFS crypto context xattr Richard Weinberger <richard@nod.at> - 2016-10-21 14:50 +0200
    [PATCH 10/26] ubifs: Enforce crypto policy in ->link and ->rename Richard Weinberger <richard@nod.at> - 2016-10-21 14:50 +0200
    [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
      Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO Michael Halcrow <mhalcrow@google.com> - 2016-10-21 19:20 +0200
        Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO Richard Weinberger <richard@nod.at> - 2016-10-21 19:30 +0200
          Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO Michael Halcrow <mhalcrow@google.com> - 2016-10-21 20:00 +0200
            Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO Richard Weinberger <richard@nod.at> - 2016-10-21 20:30 +0200
      Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO Eric Biggers <ebiggers@google.com> - 2016-10-21 20:30 +0200
        Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO Richard Weinberger <richard@nod.at> - 2016-10-24 09:10 +0200
    [PATCH 07/26] ubifs: Massage ubifs_listxattr() for encryption context Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 13/26] ubifs: Enforce crypto policy in mmap Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 11/26] ubifs: Preload crypto context in ->lookup() Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 21/26] ubifs: Rename tnc_read_node_nm Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 12/26] ubifs: Massage assert in ubifs_xattr_set() wrt. fscrypto Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 02/26] fscrypto: Constify struct inode pointer Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
      Re: [PATCH 02/26] fscrypto: Constify struct inode pointer Theodore Ts'o <tytso@mit.edu> - 2016-10-21 17:00 +0200
        Re: [PATCH 02/26] fscrypto: Constify struct inode pointer Richard Weinberger <richard@nod.at> - 2016-10-21 17:20 +0200
    [PATCH 22/26] ubifs: Add full hash lookup support Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 24/26] ubifs: Implement UBIFS_FLG_DOUBLE_HASH Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 25/26] ubifs: Implement UBIFS_FLG_ENCRYPTION Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
      Re: [PATCH 25/26] ubifs: Implement UBIFS_FLG_ENCRYPTION Eric Biggers <ebiggers@google.com> - 2016-10-21 20:40 +0200
        Re: [PATCH 25/26] ubifs: Implement UBIFS_FLG_ENCRYPTION Richard Weinberger <richard@nod.at> - 2016-10-24 09:00 +0200
          Re: [PATCH 25/26] ubifs: Implement UBIFS_FLG_ENCRYPTION Theodore Ts'o <tytso@mit.edu> - 2016-10-24 15:50 +0200
    [PATCH 17/26] ubifs: Make r5 hash binary string aware Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 20/26] ubifs: Add support for encrypted symlinks Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
      Re: [PATCH 20/26] ubifs: Add support for encrypted symlinks Eric Biggers <ebiggers@google.com> - 2016-10-21 20:50 +0200
        Re: [PATCH 20/26] ubifs: Add support for encrypted symlinks Richard Weinberger <richard@nod.at> - 2016-10-24 09:00 +0200
    [PATCH 23/26] ubifs: Use a random number for cookies Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 08/26] ubifs: Implement directory open operation Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 14/26] ubifs: Introduce new data node field, compr_size Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 16/26] ubifs: Relax checks in ubifs_validate_entry() Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 18/26] ubifs: Constify struct inode pointer in ubifs_crypt_is_encrypted() Richard Weinberger <richard@nod.at> - 2016-10-21 15:00 +0200
    [PATCH 06/26] ubifs: Add skeleton for fscrypto Richard Weinberger <richard@nod.at> - 2016-10-21 15:10 +0200
    Re: [PATCH 01/26] fscrypto: Add buffer operations Christoph Hellwig <hch@infradead.org> - 2016-10-21 15:10 +0200
      Re: [PATCH 01/26] fscrypto: Add buffer operations Richard Weinberger <richard@nod.at> - 2016-10-21 15:20 +0200
        Re: [PATCH 01/26] fscrypto: Add buffer operations Christoph Hellwig <hch@infradead.org> - 2016-10-21 15:30 +0200
          Re: [PATCH 01/26] fscrypto: Add buffer operations Theodore Ts'o <tytso@mit.edu> - 2016-10-21 17:20 +0200
          Re: [PATCH 01/26] fscrypto: Add buffer operations Richard Weinberger <richard@nod.at> - 2016-10-24 09:10 +0200
    [PATCH 09/26] ubifs: Implement file open operation Richard Weinberger <richard@nod.at> - 2016-10-21 15:10 +0200
    [PATCH 03/26] ubifs: Export ubifs_check_dir_empty() Richard Weinberger <richard@nod.at> - 2016-10-21 15:10 +0200
    [PATCH 04/26] ubifs: Export xattr get and set functions Richard Weinberger <richard@nod.at> - 2016-10-21 15:10 +0200
    [PATCH 01/26] fscrypto: Add buffer operations Richard Weinberger <richard@nod.at> - 2016-10-21 15:10 +0200

Page 1 of 3  [1] 2 3  Next page →


#1505839 — [PATCH 00/26] UBIFS File Encryption

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 14:50 +0200
Subject[PATCH 00/26] UBIFS File Encryption
Message-ID<suHqN-5zP-5@gated-at.bofh.it>
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.
Patches 1/26 and 2/26 touch fscrypto code and add support to work on
buffers instead of 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 in 4.8-rc7, but will be rebased soon to 4.9-rcX.
I'm still sending the series now to get feedback as soon as possible.
It can be obtained from:
git://git.infradead.org/users/rw/linux.git ubifs_crypt_v0

[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

Richard Weinberger (26):
  fscrypto: Add buffer operations
  fscrypto: Constify struct inode pointer
  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: Implement encrypt/decrypt for all IO
  ubifs: Relax checks in ubifs_validate_entry()
  ubifs: Make r5 hash binary string aware
  ubifs: Constify struct inode pointer in ubifs_crypt_is_encrypted()
  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       |  65 +++++--
 fs/ubifs/Kconfig         |  11 ++
 fs/ubifs/Makefile        |   1 +
 fs/ubifs/crypto.c        |  45 +++++
 fs/ubifs/debug.c         |  14 +-
 fs/ubifs/dir.c           | 431 +++++++++++++++++++++++++++++++++++++++--------
 fs/ubifs/file.c          | 130 +++++++++++++-
 fs/ubifs/ioctl.c         |  40 +++++
 fs/ubifs/journal.c       | 202 ++++++++++++++--------
 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         |  82 ++++++++-
 fs/ubifs/xattr.c         | 112 +++++++-----
 include/linux/fscrypto.h |  30 +++-
 18 files changed, 1202 insertions(+), 265 deletions(-)
 create mode 100644 fs/ubifs/crypto.c

-- 
2.7.3

[toc] | [next] | [standalone]


#1505840 — [PATCH 26/26] ubifs: Raise write version to 5

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 14:50 +0200
Subject[PATCH 26/26] ubifs: Raise write version to 5
Message-ID<suHqN-5zP-15@gated-at.bofh.it>
In reply to#1505839
Starting with version 5 the following properties change:
 - UBIFS_FLG_DOUBLE_HASH is mandatory
 - UBIFS_FLG_ENCRYPTION is optional but depdens on UBIFS_FLG_DOUBLE_HASH
 - Filesystems with unknown super block flags will be rejected, this
   allows us in future to add new features without raising the UBIFS
   write version.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/sb.c          | 17 +++++++++++++++++
 fs/ubifs/ubifs-media.h |  4 +++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index 54cef70ea16f..7f1ead29e727 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -466,6 +466,16 @@ static int validate_sb(struct ubifs_info *c, struct ubifs_sb_node *sup)
 		goto failed;
 	}
 
+	if (!c->double_hash && c->fmt_version >= 5) {
+		err = 16;
+		goto failed;
+	}
+
+	if (c->encrypted && c->fmt_version < 5) {
+		err = 17;
+		goto failed;
+	}
+
 	return 0;
 
 failed:
@@ -624,6 +634,13 @@ int ubifs_read_superblock(struct ubifs_info *c)
 	c->double_hash = !!(sup_flags & UBIFS_FLG_DOUBLE_HASH);
 	c->encrypted = !!(sup_flags & UBIFS_FLG_ENCRYPTION);
 
+	if ((sup_flags & ~UBIFS_FLG_MASK) != 0) {
+		ubifs_err(c, "Unknown feature flags found: %#x",
+			  sup_flags & ~UBIFS_FLG_MASK);
+		err = -EINVAL;
+		goto out;
+	}
+
 #ifndef CONFIG_UBIFS_FS_ENCRYPTION
 	if (c->encrypted) {
 		ubifs_err(c, "file system contains encrypted files but UBIFS"
diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
index bdc7935a5e41..e8c23c9d4f4a 100644
--- a/fs/ubifs/ubifs-media.h
+++ b/fs/ubifs/ubifs-media.h
@@ -46,7 +46,7 @@
  * UBIFS went into mainline kernel with format version 4. The older formats
  * were development formats.
  */
-#define UBIFS_FORMAT_VERSION 4
+#define UBIFS_FORMAT_VERSION 5
 
 /*
  * Read-only compatibility version. If the UBIFS format is changed, older UBIFS
@@ -429,6 +429,8 @@ enum {
 	UBIFS_FLG_ENCRYPTION = 0x10,
 };
 
+#define UBIFS_FLG_MASK (UBIFS_FLG_BIGLPT|UBIFS_FLG_SPACE_FIXUP|UBIFS_FLG_DOUBLE_HASH|UBIFS_FLG_ENCRYPTION)
+
 /**
  * struct ubifs_ch - common header node.
  * @magic: UBIFS node magic number (%UBIFS_NODE_MAGIC)
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1506171 — Re: [PATCH 26/26] ubifs: Raise write version to 5

FromMichael Halcrow <mhalcrow@google.com>
Date2016-10-21 19:40 +0200
SubjectRe: [PATCH 26/26] ubifs: Raise write version to 5
Message-ID<suLXs-kW-15@gated-at.bofh.it>
In reply to#1505840
On Fri, Oct 21, 2016 at 02:48:41PM +0200, Richard Weinberger wrote:
> Starting with version 5 the following properties change:
>  - UBIFS_FLG_DOUBLE_HASH is mandatory
>  - UBIFS_FLG_ENCRYPTION is optional but depdens on UBIFS_FLG_DOUBLE_HASH
>  - Filesystems with unknown super block flags will be rejected, this
>    allows us in future to add new features without raising the UBIFS
>    write version.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  fs/ubifs/sb.c          | 17 +++++++++++++++++
>  fs/ubifs/ubifs-media.h |  4 +++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
> index 54cef70ea16f..7f1ead29e727 100644
> --- a/fs/ubifs/sb.c
> +++ b/fs/ubifs/sb.c
> @@ -466,6 +466,16 @@ static int validate_sb(struct ubifs_info *c, struct ubifs_sb_node *sup)
>  		goto failed;
>  	}
>  
> +	if (!c->double_hash && c->fmt_version >= 5) {
> +		err = 16;
> +		goto failed;
> +	}
> +
> +	if (c->encrypted && c->fmt_version < 5) {
> +		err = 17;
> +		goto failed;
> +	}
> +
>  	return 0;
>  
>  failed:
> @@ -624,6 +634,13 @@ int ubifs_read_superblock(struct ubifs_info *c)
>  	c->double_hash = !!(sup_flags & UBIFS_FLG_DOUBLE_HASH);
>  	c->encrypted = !!(sup_flags & UBIFS_FLG_ENCRYPTION);
>  
> +	if ((sup_flags & ~UBIFS_FLG_MASK) != 0) {
> +		ubifs_err(c, "Unknown feature flags found: %#x",
> +			  sup_flags & ~UBIFS_FLG_MASK);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
>  #ifndef CONFIG_UBIFS_FS_ENCRYPTION
>  	if (c->encrypted) {
>  		ubifs_err(c, "file system contains encrypted files but UBIFS"
> diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
> index bdc7935a5e41..e8c23c9d4f4a 100644
> --- a/fs/ubifs/ubifs-media.h
> +++ b/fs/ubifs/ubifs-media.h
> @@ -46,7 +46,7 @@
>   * UBIFS went into mainline kernel with format version 4. The older formats
>   * were development formats.
>   */
> -#define UBIFS_FORMAT_VERSION 4
> +#define UBIFS_FORMAT_VERSION 5

Alex Cope is working on a fix for file name encryption in ext4 so that
common plaintext prefixes don't result in common ciphertext prefixes.
Older kernels will not be able to read the new file names.

>  
>  /*
>   * Read-only compatibility version. If the UBIFS format is changed, older UBIFS
> @@ -429,6 +429,8 @@ enum {
>  	UBIFS_FLG_ENCRYPTION = 0x10,
>  };
>  
> +#define UBIFS_FLG_MASK (UBIFS_FLG_BIGLPT|UBIFS_FLG_SPACE_FIXUP|UBIFS_FLG_DOUBLE_HASH|UBIFS_FLG_ENCRYPTION)
> +
>  /**
>   * struct ubifs_ch - common header node.
>   * @magic: UBIFS node magic number (%UBIFS_NODE_MAGIC)
> -- 
> 2.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]


#1506188 — Re: [PATCH 26/26] ubifs: Raise write version to 5

FromTheodore Ts'o <tytso@mit.edu>
Date2016-10-21 19:50 +0200
SubjectRe: [PATCH 26/26] ubifs: Raise write version to 5
Message-ID<suM78-oJ-43@gated-at.bofh.it>
In reply to#1506171
On Fri, Oct 21, 2016 at 10:31:54AM -0700, Michael Halcrow wrote:
> > diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
> > index bdc7935a5e41..e8c23c9d4f4a 100644
> > --- a/fs/ubifs/ubifs-media.h
> > +++ b/fs/ubifs/ubifs-media.h
> > @@ -46,7 +46,7 @@
> >   * UBIFS went into mainline kernel with format version 4. The older formats
> >   * were development formats.
> >   */
> > -#define UBIFS_FORMAT_VERSION 4
> > +#define UBIFS_FORMAT_VERSION 5
> 
> Alex Cope is working on a fix for file name encryption in ext4 so that
> common plaintext prefixes don't result in common ciphertext prefixes.
> Older kernels will not be able to read the new file names.

To be clear, this will be done in the context of a new encryption
mode.  In terms of how Ubifs will handle things, that's going to
depend on whether ubifs uses a single major version number or whether
they have a feature bitmask like other filesystems, including ext4.

This is better because if the user doesn't use a particular feature,
especially a feature like encryption which is optional, we don't want
the file system to reject the mount unnecessarily.  In the case of
encryption, where there may be new encryption algorithms used, or
maybe even hardware-specific encryption modes if you are using in-line
encryption where the encryption is done in hardware, the question is
whether you want to refuse the mount if you know it won't work, or
just throw an error when there is an attempt to access a directory or
file which is encrypted using an encryption algorithm which a
particular kernel version doesn't support.

We did leave some room in the ext4 superblock for an encryption format
version number, as ewll as an array of which encryption algorithsm are
in use in the superblock, but we haven't really decided which strategy
we want to use.

My current thinking is that there ought to be a warning at mount time
if there are some directories which the kernel being used won't be
able to access, but we should just fail the mount entirely, since
there might be a lot of unencrypted files that the user might want to access.

Cheers,

							- Ted

[toc] | [prev] | [next] | [standalone]


#1506205 — Re: [PATCH 26/26] ubifs: Raise write version to 5

FromEric Biggers <ebiggers@google.com>
Date2016-10-21 20:20 +0200
SubjectRe: [PATCH 26/26] ubifs: Raise write version to 5
Message-ID<suMA9-Sc-7@gated-at.bofh.it>
In reply to#1506188
On Fri, Oct 21, 2016 at 01:47:59PM -0400, Theodore Ts'o wrote:
> On Fri, Oct 21, 2016 at 10:31:54AM -0700, Michael Halcrow wrote:
> > > diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
> > > index bdc7935a5e41..e8c23c9d4f4a 100644
> > > --- a/fs/ubifs/ubifs-media.h
> > > +++ b/fs/ubifs/ubifs-media.h
> > > @@ -46,7 +46,7 @@
> > >   * UBIFS went into mainline kernel with format version 4. The older formats
> > >   * were development formats.
> > >   */
> > > -#define UBIFS_FORMAT_VERSION 4
> > > +#define UBIFS_FORMAT_VERSION 5
> > 
> > Alex Cope is working on a fix for file name encryption in ext4 so that
> > common plaintext prefixes don't result in common ciphertext prefixes.
> > Older kernels will not be able to read the new file names.
> 
> To be clear, this will be done in the context of a new encryption
> mode.  In terms of how Ubifs will handle things, that's going to
> depend on whether ubifs uses a single major version number or whether
> they have a feature bitmask like other filesystems, including ext4.
> 

I don't think it's reasonable to require require changes to filesystems whenever
someone introduces a new encryption mode --- contents, filenames, or both.
Filesystems need to be able to handle unsupported encryption modes in some way
that makes sense.  Currently, when it sees an unsupported encryption mode
fscrypto will behave as if the encryption key is not available and will also
print a one-time warning to the kernel log.  This happens when a file is
accessed, not when the filesystem is mounted.  As far as I can tell, ext4, f2fs,
and ubifs would all behave this way because this code is shared.  I think this
is probably the most realistic behavior.

Eric

[toc] | [prev] | [next] | [standalone]


#1506336 — Re: [PATCH 26/26] ubifs: Raise write version to 5

FromTheodore Ts'o <tytso@mit.edu>
Date2016-10-22 00:40 +0200
SubjectRe: [PATCH 26/26] ubifs: Raise write version to 5
Message-ID<suQDL-3qP-1@gated-at.bofh.it>
In reply to#1506205
On Fri, Oct 21, 2016 at 11:19:31AM -0700, Eric Biggers wrote:
> 
> I don't think it's reasonable to require require changes to filesystems whenever
> someone introduces a new encryption mode --- contents, filenames, or both.
> Filesystems need to be able to handle unsupported encryption modes in some way
> that makes sense.  Currently, when it sees an unsupported encryption mode
> fscrypto will behave as if the encryption key is not available and will also
> print a one-time warning to the kernel log.  This happens when a file is
> accessed, not when the filesystem is mounted.  As far as I can tell, ext4, f2fs,
> and ubifs would all behave this way because this code is shared.  I think this
> is probably the most realistic behavior.

I tend to agree, but file systems may choose some alternate approach
if they want to "fail fast" (e.g., at mount time).  I wouldn't want to
do that for ext4, but if ubifs (or some other file system) wants do
something more draconian, they can be afraid to do that.  Failing
that, some kind of one-time warning makes sense.

What I would like to do though is to is to have a callback so that
code in fs/crypto can call a file system specific notification
routine.  e.g., for ext4, we would probably want to be able to call
ext4_warning() and ext4_error() from fs/crypto, and other file systems
might want to have a different set of notification routines.

This way we can print a message like

kernel: EXT4-fs warning (device sdb1): fscrypto_xxx: foo bar baz

and if we later on have a way of sending file system specific warnings
or errors through some kind of IPC mechanism, such as netlink or some
future kdbus scheme, we can send the warning and error messages out
the same way we send other filesystem specific error messages.

	      	      	     	       	      - Ted

P.S.  BTW, we actually _do_ have something hacked together inside the
Google production kernel which pipes ext4_error() messages to a
netlink socket, so that monitoring systems don't have scrape dmesg or
/var/log/messages.

If anyone inside or outside google is interested in that
functionality, I can make the code available.  There's nothing
sensitive or Google specific in it; it's just that unfortunately,
getting that code cleaned up and upstreamed has just never made it
"above the fold" on the priority list, the engineer who originally
implemented it is no longer on the team --- and I never had the time
to cleanup work to get the code to upstream quality myself.

[toc] | [prev] | [next] | [standalone]


#1506949 — Re: [PATCH 26/26] ubifs: Raise write version to 5

FromRichard Weinberger <richard@nod.at>
Date2016-10-24 09:10 +0200
SubjectRe: [PATCH 26/26] ubifs: Raise write version to 5
Message-ID<svHyp-4js-7@gated-at.bofh.it>
In reply to#1506188
Ted,

On 21.10.2016 19:47, Theodore Ts'o wrote:
> On Fri, Oct 21, 2016 at 10:31:54AM -0700, Michael Halcrow wrote:
>>> diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
>>> index bdc7935a5e41..e8c23c9d4f4a 100644
>>> --- a/fs/ubifs/ubifs-media.h
>>> +++ b/fs/ubifs/ubifs-media.h
>>> @@ -46,7 +46,7 @@
>>>   * UBIFS went into mainline kernel with format version 4. The older formats
>>>   * were development formats.
>>>   */
>>> -#define UBIFS_FORMAT_VERSION 4
>>> +#define UBIFS_FORMAT_VERSION 5
>>
>> Alex Cope is working on a fix for file name encryption in ext4 so that
>> common plaintext prefixes don't result in common ciphertext prefixes.
>> Older kernels will not be able to read the new file names.
> 
> To be clear, this will be done in the context of a new encryption
> mode.  In terms of how Ubifs will handle things, that's going to
> depend on whether ubifs uses a single major version number or whether
> they have a feature bitmask like other filesystems, including ext4.

With write version 5, UBIFS has a real feature bitmask.
UBIFS has a feature bitmask since ever but never enforced it.
i.e. you could set bits which are unknown to UBIFS and it sill mounted.
Now UBIFS will refuse to mount when features are set which are not known/enabled
by this implementation.

Maybe I'll add another bitmap just for crypto do be able to support different cipher
modes.

Thanks,
//richard

[toc] | [prev] | [next] | [standalone]


#1505841 — [PATCH 05/26] ubifs: Define UBIFS crypto context xattr

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 14:50 +0200
Subject[PATCH 05/26] ubifs: Define UBIFS crypto context xattr
Message-ID<suHqN-5zP-23@gated-at.bofh.it>
In reply to#1505839
Like ext4 UBIFS will store the crypto context in a xattr
attribute.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/ubifs-media.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
index e24380cf46ed..d47e9569b3de 100644
--- a/fs/ubifs/ubifs-media.h
+++ b/fs/ubifs/ubifs-media.h
@@ -301,6 +301,13 @@ enum {
 #define UBIFS_MAX_NODE_SZ UBIFS_MAX_INO_NODE_SZ
 
 /*
+ * xattr name of UBIFS encryption context, we don't use a prefix
+ * nor a long name to not waste space on the flash.
+ */
+#define UBIFS_XATTR_NAME_ENCRYPTION_CONTEXT "c"
+
+
+/*
  * On-flash inode flags.
  *
  * UBIFS_COMPR_FL: use compression for this inode
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1505855 — [PATCH 10/26] ubifs: Enforce crypto policy in ->link and ->rename

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 14:50 +0200
Subject[PATCH 10/26] ubifs: Enforce crypto policy in ->link and ->rename
Message-ID<suHqO-5zP-41@gated-at.bofh.it>
In reply to#1505839
When a file is moved or linked into another directory
its current crypto policy has to be compatible with the
target policy.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/dir.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index e80de3852c3e..a77edc5ca5c9 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -544,6 +544,10 @@ static int ubifs_link(struct dentry *old_dentry, struct inode *dir,
 	ubifs_assert(inode_is_locked(dir));
 	ubifs_assert(inode_is_locked(inode));
 
+	if (ubifs_crypt_is_encrypted(dir) &&
+	    !fscrypt_has_permitted_context(dir, inode))
+		return -EPERM;
+
 	err = dbg_check_synced_i_size(c, inode);
 	if (err)
 		return err;
@@ -1027,6 +1031,11 @@ static int ubifs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	if (unlink)
 		ubifs_assert(inode_is_locked(new_inode));
 
+	if (old_dir != new_dir) {
+		if (ubifs_crypt_is_encrypted(new_dir) &&
+		    !fscrypt_has_permitted_context(new_dir, old_inode))
+			return -EPERM;
+	}
 
 	if (unlink && is_dir) {
 		err = ubifs_check_dir_empty(new_inode);
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1505862 — [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 15:00 +0200
Subject[PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO
Message-ID<suHAu-5DG-5@gated-at.bofh.it>
In reply to#1505839
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/file.c    |  36 ++++++++++++++++++
 fs/ubifs/journal.c | 105 +++++++++++++++++++++++++++++++++++++++++++----------
 fs/ubifs/super.c   |   6 ++-
 fs/ubifs/ubifs.h   |   6 +++
 4 files changed, 131 insertions(+), 22 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index bd0049788e24..60e089d50c82 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -78,6 +78,24 @@ static int read_block(struct inode *inode, void *addr, unsigned int block,
 		goto dump;
 
 	dlen = le32_to_cpu(dn->ch.len) - UBIFS_DATA_NODE_SZ;
+
+	if (ubifs_crypt_is_encrypted(inode)) {
+		int clen = le16_to_cpu(dn->compr_size);
+
+		if (clen <= 0 || clen > UBIFS_BLOCK_SIZE || clen > dlen)
+			goto dump;
+
+		ubifs_assert(dlen <= UBIFS_BLOCK_SIZE);
+		err = fscrypt_decrypt_buffer(inode, &dn->data, &dn->data, dlen, block, GFP_NOFS);
+		if (err) {
+			ubifs_err(c, "fscrypt_decrypt_buffer failed: %i", err);
+			return err;
+		}
+
+		ubifs_assert(clen <= dlen);
+		dlen = clen;
+	}
+
 	out_len = UBIFS_BLOCK_SIZE;
 	err = ubifs_decompress(c, &dn->data, dlen, addr, &out_len,
 			       le16_to_cpu(dn->compr_type));
@@ -650,6 +668,24 @@ static int populate_page(struct ubifs_info *c, struct page *page,
 
 			dlen = le32_to_cpu(dn->ch.len) - UBIFS_DATA_NODE_SZ;
 			out_len = UBIFS_BLOCK_SIZE;
+
+			if (ubifs_crypt_is_encrypted(inode)) {
+				int clen = le16_to_cpu(dn->compr_size);
+
+				if (clen <= 0 || clen > UBIFS_BLOCK_SIZE || clen > dlen)
+					goto out_err;
+
+				ubifs_assert(dlen <= UBIFS_BLOCK_SIZE);
+				err = fscrypt_decrypt_buffer(inode, &dn->data, &dn->data, dlen, page_block, GFP_NOFS);
+				if (err) {
+					ubifs_err(c, "fscrypt_decrypt_buffer failed: %i", err);
+					goto out_err;
+				}
+
+				ubifs_assert(clen <= dlen);
+				dlen = clen;
+			}
+
 			err = ubifs_decompress(c, &dn->data, dlen, addr, &out_len,
 					       le16_to_cpu(dn->compr_type));
 			if (err || len != out_len)
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index a643de4e3d91..da694e520ec8 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -691,11 +691,15 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
 	int err, lnum, offs, compr_type, out_len;
 	int dlen = COMPRESSED_DATA_NODE_BUF_SZ, allocated = 1;
 	struct ubifs_inode *ui = ubifs_inode(inode);
+	bool encrypted = ubifs_crypt_is_encrypted(inode);
 
 	dbg_jnlk(key, "ino %lu, blk %u, len %d, key ",
 		(unsigned long)key_inum(c, key), key_block(c, key), len);
 	ubifs_assert(len <= UBIFS_BLOCK_SIZE);
 
+	if (encrypted)
+		dlen += UBIFS_CIPHER_BLOCK_SIZE;
+
 	data = kmalloc(dlen, GFP_NOFS | __GFP_NOWARN);
 	if (!data) {
 		/*
@@ -724,6 +728,26 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
 	ubifs_compress(c, buf, len, &data->data, &out_len, &compr_type);
 	ubifs_assert(out_len <= UBIFS_BLOCK_SIZE);
 
+	if (encrypted) {
+		void *p = &data->data;
+		int plen = round_up(out_len, UBIFS_CIPHER_BLOCK_SIZE);
+
+		data->compr_size = cpu_to_le16(out_len);
+
+		if (plen != out_len) {
+			memset(p + out_len, 0, plen - out_len);
+			out_len = plen;
+		}
+
+		err = fscrypt_encrypt_buffer(inode, &data->data, &data->data, out_len, key_block(c, key), GFP_NOFS);
+		if (err) {
+			ubifs_err(c, "fscrypt_encrypt_buffer failed: %i", err);
+			goto out_free;
+		}
+	} else {
+		data->compr_size = 0;
+	}
+
 	dlen = UBIFS_DATA_NODE_SZ + out_len;
 	data->compr_type = cpu_to_le16(compr_type);
 
@@ -1083,31 +1107,79 @@ out_free:
 }
 
 /**
- * recomp_data_node - re-compress a truncated data node.
+ * truncate_data_node - re-compress/encrypt a truncated data node.
+ * @c: UBIFS file-system description object
+ * @inode: inode which referes to the data node
+ * @block: data block number
  * @dn: data node to re-compress
  * @new_len: new length
  *
  * This function is used when an inode is truncated and the last data node of
- * the inode has to be re-compressed and re-written.
+ * the inode has to be re-compressed/encrypted and re-written.
  */
-static int recomp_data_node(const struct ubifs_info *c,
-			    struct ubifs_data_node *dn, int *new_len)
+static int truncate_data_node(const struct ubifs_info *c, struct inode *inode,
+			      unsigned int block, struct ubifs_data_node *dn,
+			      int *new_len)
 {
 	void *buf;
-	int err, len, compr_type, out_len;
+	int err, dlen, compr_type, out_len, old_dlen;
 
 	out_len = le32_to_cpu(dn->size);
 	buf = kmalloc(out_len * WORST_COMPR_FACTOR, GFP_NOFS);
 	if (!buf)
 		return -ENOMEM;
 
-	len = le32_to_cpu(dn->ch.len) - UBIFS_DATA_NODE_SZ;
+	dlen = old_dlen = le32_to_cpu(dn->ch.len) - UBIFS_DATA_NODE_SZ;
 	compr_type = le16_to_cpu(dn->compr_type);
-	err = ubifs_decompress(c, &dn->data, len, buf, &out_len, compr_type);
-	if (err)
-		goto out;
 
-	ubifs_compress(c, buf, *new_len, &dn->data, &out_len, &compr_type);
+	if (ubifs_crypt_is_encrypted(inode)) {
+		int clen = le16_to_cpu(dn->compr_size);
+
+		if (clen <= 0 || clen > UBIFS_BLOCK_SIZE || clen > dlen) {
+			ubifs_err(c, "bad compr_size: %i", clen);
+			err = -EINVAL;
+			goto out;
+		}
+
+		err = fscrypt_decrypt_buffer(inode, &dn->data, &dn->data, dlen, block, GFP_NOFS);
+		if (err) {
+			ubifs_err(c, "fscrypt_decrypt_buffer failed: %i", err);
+			goto out;
+		}
+
+		ubifs_assert(clen <= dlen);
+		dlen = clen;
+	}
+
+	if (compr_type != UBIFS_COMPR_NONE) {
+		err = ubifs_decompress(c, &dn->data, dlen, buf, &out_len, compr_type);
+		if (err)
+			goto out;
+
+		ubifs_compress(c, buf, *new_len, &dn->data, &out_len, &compr_type);
+	}
+
+	if (ubifs_crypt_is_encrypted(inode)) {
+		void *p = &dn->data;
+		int plen = round_up(out_len, UBIFS_CIPHER_BLOCK_SIZE);
+
+		ubifs_assert(old_dlen >= plen);
+		dn->compr_size = cpu_to_le16(out_len);
+
+		if (plen != out_len) {
+			memset(p + out_len, 0, plen - out_len);
+			out_len = plen;
+		}
+
+		err = fscrypt_encrypt_buffer(inode, &dn->data, &dn->data, out_len, block, GFP_NOFS);
+		if (err) {
+			ubifs_msg(c, "fscrypt_encrypt_buffer failed: %i", err);
+			goto out;
+		}
+	} else {
+		dn->compr_size = 0;
+	}
+
 	ubifs_assert(out_len <= UBIFS_BLOCK_SIZE);
 	dn->compr_type = cpu_to_le16(compr_type);
 	dn->size = cpu_to_le32(*new_len);
@@ -1179,16 +1251,9 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
 			if (le32_to_cpu(dn->size) <= dlen)
 				dlen = 0; /* Nothing to do */
 			else {
-				int compr_type = le16_to_cpu(dn->compr_type);
-
-				if (compr_type != UBIFS_COMPR_NONE) {
-					err = recomp_data_node(c, dn, &dlen);
-					if (err)
-						goto out_free;
-				} else {
-					dn->size = cpu_to_le32(dlen);
-					dlen += UBIFS_DATA_NODE_SZ;
-				}
+				err = truncate_data_node(c, inode, blk, dn, &dlen);
+				if (err)
+					goto out_free;
 			}
 		}
 	}
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index a31222947265..ae25c908fbe5 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1210,7 +1210,8 @@ static int mount_ubifs(struct ubifs_info *c)
 		bu_init(c);
 
 	if (!c->ro_mount) {
-		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ,
+		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
+					       UBIFS_CIPHER_BLOCK_SIZE,
 					       GFP_KERNEL);
 		if (!c->write_reserve_buf)
 			goto out_free;
@@ -1623,7 +1624,8 @@ static int ubifs_remount_rw(struct ubifs_info *c)
 		goto out;
 	}
 
-	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ, GFP_KERNEL);
+	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
+				       UBIFS_CIPHER_BLOCK_SIZE, GFP_KERNEL);
 	if (!c->write_reserve_buf) {
 		err = -ENOMEM;
 		goto out;
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 63dd027727ca..6c56a08afcd7 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -139,6 +139,12 @@
  */
 #define WORST_COMPR_FACTOR 2
 
+#ifdef CONFIG_UBIFS_FS_ENCRYPTION
+#define UBIFS_CIPHER_BLOCK_SIZE FS_CRYPTO_BLOCK_SIZE
+#else
+#define UBIFS_CIPHER_BLOCK_SIZE 0
+#endif
+
 /*
  * How much memory is needed for a buffer where we compress a data node.
  */
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1506159 — Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO

FromMichael Halcrow <mhalcrow@google.com>
Date2016-10-21 19:20 +0200
SubjectRe: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO
Message-ID<suLE6-dO-5@gated-at.bofh.it>
In reply to#1505862
On Fri, Oct 21, 2016 at 02:48:30PM +0200, Richard Weinberger wrote:
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  fs/ubifs/file.c    |  36 ++++++++++++++++++
>  fs/ubifs/journal.c | 105 +++++++++++++++++++++++++++++++++++++++++++----------
>  fs/ubifs/super.c   |   6 ++-
>  fs/ubifs/ubifs.h   |   6 +++
>  4 files changed, 131 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index bd0049788e24..60e089d50c82 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -78,6 +78,24 @@ static int read_block(struct inode *inode, void *addr, unsigned int block,
>  		goto dump;
>  
>  	dlen = le32_to_cpu(dn->ch.len) - UBIFS_DATA_NODE_SZ;
> +
> +	if (ubifs_crypt_is_encrypted(inode)) {
> +		int clen = le16_to_cpu(dn->compr_size);
> +
> +		if (clen <= 0 || clen > UBIFS_BLOCK_SIZE || clen > dlen)
> +			goto dump;
> +
> +		ubifs_assert(dlen <= UBIFS_BLOCK_SIZE);
> +		err = fscrypt_decrypt_buffer(inode, &dn->data, &dn->data, dlen, block, GFP_NOFS);
> +		if (err) {
> +			ubifs_err(c, "fscrypt_decrypt_buffer failed: %i", err);
> +			return err;
> +		}
> +
> +		ubifs_assert(clen <= dlen);
> +		dlen = clen;
> +	}
> +
>  	out_len = UBIFS_BLOCK_SIZE;
>  	err = ubifs_decompress(c, &dn->data, dlen, addr, &out_len,
>  			       le16_to_cpu(dn->compr_type));
> @@ -650,6 +668,24 @@ static int populate_page(struct ubifs_info *c, struct page *page,
>  
>  			dlen = le32_to_cpu(dn->ch.len) - UBIFS_DATA_NODE_SZ;
>  			out_len = UBIFS_BLOCK_SIZE;
> +
> +			if (ubifs_crypt_is_encrypted(inode)) {
> +				int clen = le16_to_cpu(dn->compr_size);
> +
> +				if (clen <= 0 || clen > UBIFS_BLOCK_SIZE || clen > dlen)
> +					goto out_err;
> +
> +				ubifs_assert(dlen <= UBIFS_BLOCK_SIZE);
> +				err = fscrypt_decrypt_buffer(inode, &dn->data, &dn->data, dlen, page_block, GFP_NOFS);
> +				if (err) {
> +					ubifs_err(c, "fscrypt_decrypt_buffer failed: %i", err);
> +					goto out_err;
> +				}
> +
> +				ubifs_assert(clen <= dlen);
> +				dlen = clen;
> +			}
> +
>  			err = ubifs_decompress(c, &dn->data, dlen, addr, &out_len,
>  					       le16_to_cpu(dn->compr_type));
>  			if (err || len != out_len)
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index a643de4e3d91..da694e520ec8 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -691,11 +691,15 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
>  	int err, lnum, offs, compr_type, out_len;
>  	int dlen = COMPRESSED_DATA_NODE_BUF_SZ, allocated = 1;
>  	struct ubifs_inode *ui = ubifs_inode(inode);
> +	bool encrypted = ubifs_crypt_is_encrypted(inode);
>  
>  	dbg_jnlk(key, "ino %lu, blk %u, len %d, key ",
>  		(unsigned long)key_inum(c, key), key_block(c, key), len);
>  	ubifs_assert(len <= UBIFS_BLOCK_SIZE);
>  
> +	if (encrypted)
> +		dlen += UBIFS_CIPHER_BLOCK_SIZE;
> +
>  	data = kmalloc(dlen, GFP_NOFS | __GFP_NOWARN);
>  	if (!data) {
>  		/*
> @@ -724,6 +728,26 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
>  	ubifs_compress(c, buf, len, &data->data, &out_len, &compr_type);

Compress-before-encrypt is a hazard.

http://www.iacr.org/cryptodb/archive/2002/FSE/3091/3091.pdf

>  	ubifs_assert(out_len <= UBIFS_BLOCK_SIZE);
>  
> +	if (encrypted) {
> +		void *p = &data->data;
> +		int plen = round_up(out_len, UBIFS_CIPHER_BLOCK_SIZE);
> +
> +		data->compr_size = cpu_to_le16(out_len);
> +
> +		if (plen != out_len) {
> +			memset(p + out_len, 0, plen - out_len);
> +			out_len = plen;
> +		}
> +
> +		err = fscrypt_encrypt_buffer(inode, &data->data, &data->data, out_len, key_block(c, key), GFP_NOFS);
> +		if (err) {
> +			ubifs_err(c, "fscrypt_encrypt_buffer failed: %i", err);
> +			goto out_free;
> +		}
> +	} else {
> +		data->compr_size = 0;
> +	}
> +
>  	dlen = UBIFS_DATA_NODE_SZ + out_len;
>  	data->compr_type = cpu_to_le16(compr_type);
>  
> @@ -1083,31 +1107,79 @@ out_free:
>  }
>  
>  /**
> - * recomp_data_node - re-compress a truncated data node.
> + * truncate_data_node - re-compress/encrypt a truncated data node.
> + * @c: UBIFS file-system description object
> + * @inode: inode which referes to the data node
> + * @block: data block number
>   * @dn: data node to re-compress
>   * @new_len: new length
>   *
>   * This function is used when an inode is truncated and the last data node of
> - * the inode has to be re-compressed and re-written.
> + * the inode has to be re-compressed/encrypted and re-written.
>   */
> -static int recomp_data_node(const struct ubifs_info *c,
> -			    struct ubifs_data_node *dn, int *new_len)
> +static int truncate_data_node(const struct ubifs_info *c, struct inode *inode,
> +			      unsigned int block, struct ubifs_data_node *dn,
> +			      int *new_len)
>  {
>  	void *buf;
> -	int err, len, compr_type, out_len;
> +	int err, dlen, compr_type, out_len, old_dlen;
>  
>  	out_len = le32_to_cpu(dn->size);
>  	buf = kmalloc(out_len * WORST_COMPR_FACTOR, GFP_NOFS);
>  	if (!buf)
>  		return -ENOMEM;
>  
> -	len = le32_to_cpu(dn->ch.len) - UBIFS_DATA_NODE_SZ;
> +	dlen = old_dlen = le32_to_cpu(dn->ch.len) - UBIFS_DATA_NODE_SZ;
>  	compr_type = le16_to_cpu(dn->compr_type);
> -	err = ubifs_decompress(c, &dn->data, len, buf, &out_len, compr_type);
> -	if (err)
> -		goto out;
>  
> -	ubifs_compress(c, buf, *new_len, &dn->data, &out_len, &compr_type);
> +	if (ubifs_crypt_is_encrypted(inode)) {
> +		int clen = le16_to_cpu(dn->compr_size);
> +
> +		if (clen <= 0 || clen > UBIFS_BLOCK_SIZE || clen > dlen) {
> +			ubifs_err(c, "bad compr_size: %i", clen);
> +			err = -EINVAL;
> +			goto out;
> +		}
> +
> +		err = fscrypt_decrypt_buffer(inode, &dn->data, &dn->data, dlen, block, GFP_NOFS);
> +		if (err) {
> +			ubifs_err(c, "fscrypt_decrypt_buffer failed: %i", err);
> +			goto out;
> +		}
> +
> +		ubifs_assert(clen <= dlen);
> +		dlen = clen;
> +	}
> +
> +	if (compr_type != UBIFS_COMPR_NONE) {
> +		err = ubifs_decompress(c, &dn->data, dlen, buf, &out_len, compr_type);
> +		if (err)
> +			goto out;
> +
> +		ubifs_compress(c, buf, *new_len, &dn->data, &out_len, &compr_type);
> +	}
> +
> +	if (ubifs_crypt_is_encrypted(inode)) {
> +		void *p = &dn->data;
> +		int plen = round_up(out_len, UBIFS_CIPHER_BLOCK_SIZE);
> +
> +		ubifs_assert(old_dlen >= plen);
> +		dn->compr_size = cpu_to_le16(out_len);
> +
> +		if (plen != out_len) {
> +			memset(p + out_len, 0, plen - out_len);
> +			out_len = plen;
> +		}
> +
> +		err = fscrypt_encrypt_buffer(inode, &dn->data, &dn->data, out_len, block, GFP_NOFS);
> +		if (err) {
> +			ubifs_msg(c, "fscrypt_encrypt_buffer failed: %i", err);
> +			goto out;
> +		}
> +	} else {
> +		dn->compr_size = 0;
> +	}
> +
>  	ubifs_assert(out_len <= UBIFS_BLOCK_SIZE);
>  	dn->compr_type = cpu_to_le16(compr_type);
>  	dn->size = cpu_to_le32(*new_len);
> @@ -1179,16 +1251,9 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
>  			if (le32_to_cpu(dn->size) <= dlen)
>  				dlen = 0; /* Nothing to do */
>  			else {
> -				int compr_type = le16_to_cpu(dn->compr_type);
> -
> -				if (compr_type != UBIFS_COMPR_NONE) {
> -					err = recomp_data_node(c, dn, &dlen);
> -					if (err)
> -						goto out_free;
> -				} else {
> -					dn->size = cpu_to_le32(dlen);
> -					dlen += UBIFS_DATA_NODE_SZ;
> -				}
> +				err = truncate_data_node(c, inode, blk, dn, &dlen);
> +				if (err)
> +					goto out_free;
>  			}
>  		}
>  	}
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index a31222947265..ae25c908fbe5 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1210,7 +1210,8 @@ static int mount_ubifs(struct ubifs_info *c)
>  		bu_init(c);
>  
>  	if (!c->ro_mount) {
> -		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ,
> +		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
> +					       UBIFS_CIPHER_BLOCK_SIZE,
>  					       GFP_KERNEL);
>  		if (!c->write_reserve_buf)
>  			goto out_free;
> @@ -1623,7 +1624,8 @@ static int ubifs_remount_rw(struct ubifs_info *c)
>  		goto out;
>  	}
>  
> -	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ, GFP_KERNEL);
> +	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
> +				       UBIFS_CIPHER_BLOCK_SIZE, GFP_KERNEL);
>  	if (!c->write_reserve_buf) {
>  		err = -ENOMEM;
>  		goto out;
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 63dd027727ca..6c56a08afcd7 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -139,6 +139,12 @@
>   */
>  #define WORST_COMPR_FACTOR 2
>  
> +#ifdef CONFIG_UBIFS_FS_ENCRYPTION
> +#define UBIFS_CIPHER_BLOCK_SIZE FS_CRYPTO_BLOCK_SIZE
> +#else
> +#define UBIFS_CIPHER_BLOCK_SIZE 0
> +#endif
> +
>  /*
>   * How much memory is needed for a buffer where we compress a data node.
>   */
> -- 
> 2.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]


#1506168 — Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 19:30 +0200
SubjectRe: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO
Message-ID<suLNM-hj-31@gated-at.bofh.it>
In reply to#1506159
On 21.10.2016 19:14, Michael Halcrow wrote:
> Compress-before-encrypt is a hazard.
> 
> http://www.iacr.org/cryptodb/archive/2002/FSE/3091/3091.pdf

I'm fully aware of that. But as usual it depends on the use case.
Compression is optional in UBIFS, paranoid users can disable it
when encryption is enabled.

Thanks,
//richard

[toc] | [prev] | [next] | [standalone]


#1506196 — Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO

FromMichael Halcrow <mhalcrow@google.com>
Date2016-10-21 20:00 +0200
SubjectRe: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO
Message-ID<suMgT-so-31@gated-at.bofh.it>
In reply to#1506168
On Fri, Oct 21, 2016 at 07:21:04PM +0200, Richard Weinberger wrote:
> On 21.10.2016 19:14, Michael Halcrow wrote:
> > Compress-before-encrypt is a hazard.
> > 
> > http://www.iacr.org/cryptodb/archive/2002/FSE/3091/3091.pdf
> 
> I'm fully aware of that. But as usual it depends on the use case.
> Compression is optional in UBIFS, paranoid users can disable it
> when encryption is enabled.

It's not the paranoid users I'm concerned about.  It's those building
systems with complexity and nuance on top of UBIFS who aren't paranoid
enough.

I suggest disabling compression by default when encryption is enabled,
unless the user explicitly enables both.

> 
> Thanks,
> //richard

[toc] | [prev] | [next] | [standalone]


#1506212 — Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 20:30 +0200
SubjectRe: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO
Message-ID<suMJP-VN-25@gated-at.bofh.it>
In reply to#1506196
On 21.10.2016 19:52, Michael Halcrow wrote:
> On Fri, Oct 21, 2016 at 07:21:04PM +0200, Richard Weinberger wrote:
>> On 21.10.2016 19:14, Michael Halcrow wrote:
>>> Compress-before-encrypt is a hazard.
>>>
>>> http://www.iacr.org/cryptodb/archive/2002/FSE/3091/3091.pdf
>>
>> I'm fully aware of that. But as usual it depends on the use case.
>> Compression is optional in UBIFS, paranoid users can disable it
>> when encryption is enabled.
> 
> It's not the paranoid users I'm concerned about.  It's those building
> systems with complexity and nuance on top of UBIFS who aren't paranoid
> enough.
> 
> I suggest disabling compression by default when encryption is enabled,
> unless the user explicitly enables both.

That's definitely an option, yes.

Thanks,
//richard

[toc] | [prev] | [next] | [standalone]


#1506211 — Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO

FromEric Biggers <ebiggers@google.com>
Date2016-10-21 20:30 +0200
SubjectRe: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO
Message-ID<suMJP-VN-21@gated-at.bofh.it>
In reply to#1505862
On Fri, Oct 21, 2016 at 02:48:30PM +0200, Richard Weinberger wrote:
> +
> +	if (ubifs_crypt_is_encrypted(inode)) {
> +		int clen = le16_to_cpu(dn->compr_size);
> +
> +		if (clen <= 0 || clen > UBIFS_BLOCK_SIZE || clen > dlen)
> +			goto dump;
> +
> +		ubifs_assert(dlen <= UBIFS_BLOCK_SIZE);
> +		err = fscrypt_decrypt_buffer(inode, &dn->data, &dn->data, dlen, block, GFP_NOFS);
> +		if (err) {
> +			ubifs_err(c, "fscrypt_decrypt_buffer failed: %i", err);
> +			return err;
> +		}
> +
> +		ubifs_assert(clen <= dlen);
> +		dlen = clen;
> +	}
> +

There are several code blocks like this, calling either fscrypt_decrypt_buffer()
or fscrypt_encrypt_buffer(), which seem to be mostly duplicated.  Is it possible
to refactor them into helper functions?  There are also some lines well over 80
characters.

Eric

[toc] | [prev] | [next] | [standalone]


#1506950 — Re: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO

FromRichard Weinberger <richard@nod.at>
Date2016-10-24 09:10 +0200
SubjectRe: [PATCH 15/26] ubifs: Implement encrypt/decrypt for all IO
Message-ID<svHyp-4js-9@gated-at.bofh.it>
In reply to#1506211
Eric,

On 21.10.2016 20:25, Eric Biggers wrote:
> On Fri, Oct 21, 2016 at 02:48:30PM +0200, Richard Weinberger wrote:
>> +
>> +	if (ubifs_crypt_is_encrypted(inode)) {
>> +		int clen = le16_to_cpu(dn->compr_size);
>> +
>> +		if (clen <= 0 || clen > UBIFS_BLOCK_SIZE || clen > dlen)
>> +			goto dump;
>> +
>> +		ubifs_assert(dlen <= UBIFS_BLOCK_SIZE);
>> +		err = fscrypt_decrypt_buffer(inode, &dn->data, &dn->data, dlen, block, GFP_NOFS);
>> +		if (err) {
>> +			ubifs_err(c, "fscrypt_decrypt_buffer failed: %i", err);
>> +			return err;
>> +		}
>> +
>> +		ubifs_assert(clen <= dlen);
>> +		dlen = clen;
>> +	}
>> +
> 
> There are several code blocks like this, calling either fscrypt_decrypt_buffer()
> or fscrypt_encrypt_buffer(), which seem to be mostly duplicated.  Is it possible
> to refactor them into helper functions?  There are also some lines well over 80
> characters.

Yes, that's already on my TODO.

Thanks,
//richard

[toc] | [prev] | [next] | [standalone]


#1505864 — [PATCH 07/26] ubifs: Massage ubifs_listxattr() for encryption context

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 15:00 +0200
Subject[PATCH 07/26] ubifs: Massage ubifs_listxattr() for encryption context
Message-ID<suHAu-5DG-11@gated-at.bofh.it>
In reply to#1505839
We have to make sure that we don't expose our internal
crypto context to userspace.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/xattr.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
index b390902c48aa..54ec2b9fd506 100644
--- a/fs/ubifs/xattr.c
+++ b/fs/ubifs/xattr.c
@@ -394,6 +394,20 @@ out_unlock:
 	return err;
 }
 
+static bool xattr_visible(const char *name)
+{
+	/* File encryption related xattrs are for internal use only */
+	if (strcmp(name, UBIFS_XATTR_NAME_ENCRYPTION_CONTEXT) == 0)
+		return false;
+
+	/* Show trusted namespace only for "power" users */
+	if (strncmp(name, XATTR_TRUSTED_PREFIX,
+		    XATTR_TRUSTED_PREFIX_LEN) == 0 && !capable(CAP_SYS_ADMIN))
+		return false;
+
+	return true;
+}
+
 ssize_t ubifs_listxattr(struct dentry *dentry, char *buffer, size_t size)
 {
 	union ubifs_key key;
@@ -429,10 +443,7 @@ ssize_t ubifs_listxattr(struct dentry *dentry, char *buffer, size_t size)
 		nm.name = xent->name;
 		nm.len = le16_to_cpu(xent->nlen);
 
-		/* Show trusted namespace only for "power" users */
-		if (strncmp(xent->name, XATTR_TRUSTED_PREFIX,
-			    XATTR_TRUSTED_PREFIX_LEN) ||
-		    capable(CAP_SYS_ADMIN)) {
+		if (xattr_visible(xent->name)) {
 			memcpy(buffer + written, nm.name, nm.len + 1);
 			written += nm.len + 1;
 		}
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1505865 — [PATCH 13/26] ubifs: Enforce crypto policy in mmap

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 15:00 +0200
Subject[PATCH 13/26] ubifs: Enforce crypto policy in mmap
Message-ID<suHAu-5DG-19@gated-at.bofh.it>
In reply to#1505839
We need this extra check in mmap because a process could
gain an already opened fd.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/file.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index fc79278969e5..bd0049788e24 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1594,6 +1594,15 @@ static const struct vm_operations_struct ubifs_file_vm_ops = {
 static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	int err;
+	struct inode *inode = file->f_mapping->host;
+
+	if (ubifs_crypt_is_encrypted(inode)) {
+		err = fscrypt_get_encryption_info(inode);
+		if (err)
+			return -EACCES;
+		if (!fscrypt_has_encryption_key(inode))
+			return -ENOKEY;
+	}
 
 	err = generic_file_mmap(file, vma);
 	if (err)
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1505867 — [PATCH 11/26] ubifs: Preload crypto context in ->lookup()

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 15:00 +0200
Subject[PATCH 11/26] ubifs: Preload crypto context in ->lookup()
Message-ID<suHAu-5DG-21@gated-at.bofh.it>
In reply to#1505839
...and mark the dentry as encrypted.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/dir.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index a77edc5ca5c9..7ee32768ba19 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -218,6 +218,21 @@ static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry,
 
 	dbg_gen("'%pd' in dir ino %lu", dentry, dir->i_ino);
 
+	if (ubifs_crypt_is_encrypted(dir)) {
+		err = fscrypt_get_encryption_info(dir);
+
+		/*
+		 * DCACHE_ENCRYPTED_WITH_KEY is set if the dentry is
+		 * created while the directory was encrypted and we
+		 * have access to the key.
+		 */
+		if (fscrypt_has_encryption_key(dir))
+			fscrypt_set_encrypted_dentry(dentry);
+		fscrypt_set_d_op(dentry);
+		if (err && err != -ENOKEY)
+			return ERR_PTR(err);
+	}
+
 	if (dentry->d_name.len > UBIFS_MAX_NLEN)
 		return ERR_PTR(-ENAMETOOLONG);
 
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


#1505868 — [PATCH 21/26] ubifs: Rename tnc_read_node_nm

FromRichard Weinberger <richard@nod.at>
Date2016-10-21 15:00 +0200
Subject[PATCH 21/26] ubifs: Rename tnc_read_node_nm
Message-ID<suHAu-5DG-15@gated-at.bofh.it>
In reply to#1505839
tnc_read_hashed_node() is a better name since we read a node
by a given hash, not a name.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/ubifs/tnc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
index 0d0030461cb4..0d751297873e 100644
--- a/fs/ubifs/tnc.c
+++ b/fs/ubifs/tnc.c
@@ -378,7 +378,7 @@ static void lnc_free(struct ubifs_zbranch *zbr)
 }
 
 /**
- * tnc_read_node_nm - read a "hashed" leaf node.
+ * tnc_read_hashed_node - read a "hashed" leaf node.
  * @c: UBIFS file-system description object
  * @zbr: key and position of the node
  * @node: node is returned here
@@ -388,8 +388,8 @@ static void lnc_free(struct ubifs_zbranch *zbr)
  * added to LNC. Returns zero in case of success or a negative negative error
  * code in case of failure.
  */
-static int tnc_read_node_nm(struct ubifs_info *c, struct ubifs_zbranch *zbr,
-			    void *node)
+static int tnc_read_hashed_node(struct ubifs_info *c, struct ubifs_zbranch *zbr,
+				void *node)
 {
 	int err;
 
@@ -1454,7 +1454,7 @@ again:
 		 * In this case the leaf node cache gets used, so we pass the
 		 * address of the zbranch and keep the mutex locked
 		 */
-		err = tnc_read_node_nm(c, zt, node);
+		err = tnc_read_hashed_node(c, zt, node);
 		goto out;
 	}
 	if (safely) {
@@ -1817,7 +1817,7 @@ static int do_lookup_nm(struct ubifs_info *c, const union ubifs_key *key,
 		goto out_unlock;
 	}
 
-	err = tnc_read_node_nm(c, &znode->zbranch[n], node);
+	err = tnc_read_hashed_node(c, &znode->zbranch[n], node);
 
 out_unlock:
 	mutex_unlock(&c->tnc_mutex);
@@ -2815,7 +2815,7 @@ struct ubifs_dent_node *ubifs_tnc_next_ent(struct ubifs_info *c,
 		goto out_free;
 	}
 
-	err = tnc_read_node_nm(c, zbr, dent);
+	err = tnc_read_hashed_node(c, zbr, dent);
 	if (unlikely(err))
 		goto out_free;
 
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]


Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel


csiph-web