Re: fs/crypto: root read-access without key

[Eric Biggers <ebiggers3@gmail.com>]

Hi Anand,

On Wed, Feb 15, 2017 at 04:04:37PM +0800, Anand Jain wrote:
> 
>  dcache: yeah its complicated, however I still don't know why
>  file-name was the data-to-protected in the first place. Next,
> 
>  page: a read request by the no-key-user can be checked [1] and
>  returned either ENOKEY or cipher-text using dio.
> 
>  [1] struct inode has crypto_info so it can check the key status
>  and, further more its not at the permission check so no performance
>  impact.
> 

This is incorrect because for a file there is one only inode system-wide, not
one inode per user (or per process).  So everyone will either see the key in the
inode or not.

> > We could try and fix this by adding our own file system unique
> > premissions check, but that slows down every single file open with a
> > keyring search.
> 
>  not unless file-name encryption is part of the data to be
>  encrypted as above.

No, if a permission check were to be added, then it would need to be done for
both regular files (contents encryption) and for directories and symlinks
(filenames encryption).

> 
> >  It also isn't a complete solution because if you pass
> > the fd around via a unix domain socket, or some such you can still end
> > up with weirdness where whether or not the process which is currently
> > trying to operate on the fd has access to the key (via the rings
> > unique, wild, and wonderful, and nonstandard/non-POSIX keyring access
> > scheme).
> 
>  Again this is only applicable in the context of file-name
>  encryption.

No, it's applicable for both contents and filenames encryption, because the file
descriptor could refer to either a regular file (contents encryption) or
directory (filenames encryption).

> 
> > It also isn't complete, since someone could infer whether or not a
> > file exists, unless we also completely spike out the dcache, which
> > would be an even worse performance disaster.
> 
>  part of the file-name encryption only problem.

Well, you could say that, but of course without filename encryption then anyone
can see which filenames exist without having to resort to any clever attacks...

> 
> > So the current model is that if you want to protect file, the Unix
> > permissions do have to be set correctly, and root can read everything.
> > The presense or absense of keys is *not* currently intended to be an
> > access control mechanism.
> 
>  yeah encryption is not about the access control itself, I echo
>  that myself often.
> 
>  Factually the problem here is with the file-name encryption (FNE),
>  And, I still don't know the relation between FNE and the Evil Maid
>  attack which, I have been told as the reason for the mandatory
>  FNE for all solutions.

There are actually several separate protections against such attacks.  First,
the encryption of both contents and filenames makes it more difficult (though
not necessarily impossible) to identify target files.  Second, the filesystem
enforces that every file in a given directory tree uses the same encryption
policy, which makes it more difficult for encryption to be maliciously weakened
or removed (or even impossible, if a trusted process verifies the top-level
encryption policies).

Not encrypting filenames would not be the end of the world, but it's a security
enhancement which is nice to have.  And I think you are blaming filenames
encryption specifically for things which are actually more general concerns.

> 
>  Last but not least, the whole purpose of having the encryption
>  metadata to be at the attribute is to facilitate the compatibility
>  with the existing tools rsync -X ; cp --preserve. IMO we have
>  entirely defeated that purpose here.
> 

Well, the reason it is being stored in an extended attribute (at least for ext4,
f2fs, and ubifs; other filesystems could do it differently) is that this was the
only logical place to put it, and easy to implement.  If somehow only 4-8 bytes
of metadata were needed then it could have been added to the inodes directly,
but it is 28 bytes and will probably get larger in the future.

As for the mostly separate question of the API, this was already covered to a
large extent, but the semantics of the encryption metadata are not like a normal
xattr.  If it *was* exposed it through the xattr system calls, without special
semantics, then there would be some very strange things users could do and
resulting problems that would need to be solved, for example:

* create a file, write some data to it, then set an encryption xattr.  This
  would suddenly change the interpretation of the data; now what was previously
  the plaintext is now the ciphertext.  What happens if pages of the file have
  already been cached or even mmap'ed?

* remove the encryption xattr.  Similar problem; the interpretation of the data
  suddenly changes: now what was previously the ciphertext is now the plaintext.
  What happens if pages are already cached or mmap'ed?

* apply an encryption xattr to a directory after it already has files in it.
  Are the names of those files supposed to be unencrypted or are they supposed
  to be encrypted?

* allow userspace to choose the per-file nonce.  How do you ensure that
  userspace doesn't repeat nonces?  Note that this would happen trivially if a
  file is copied.

So I don't think it is realistic to support backup/restore of "locked" encrypted
files without a special API.  (And I think other OS's tend to do things the same
way, e.g. on Windows note the special functions for backup/restore of EFS
files.)  Of course, if you have a proposed solution for how this would in fact
be possible, then I'm sure people would be willing to listen.

Eric