mbox series

[RFC,v3,00/18] fscrypt: key management improvements

Message ID 20190220065249.32099-1-ebiggers@kernel.org (mailing list archive)
Headers show
Series fscrypt: key management improvements | expand

Message

Eric Biggers Feb. 20, 2019, 6:52 a.m. UTC
Hello,

This patchset makes major improvements to how keys are added, removed,
and derived in fscrypt, aka ext4/f2fs/ubifs encryption.  It does this by
adding new ioctls that add and remove encryption keys directly to/from
the filesystem, and by adding a new encryption policy version ("v2")
where the user-provided keys are only used as input to HKDF-SHA512 and
are identified by their cryptographic hash.

All new APIs and all cryptosystem changes are documented in
Documentation/filesystems/fscrypt.rst.  Userspace can use the new key
management ioctls with existing encrypted directories, but migrating to
v2 encryption policies is needed for the full benefits.

These changes solve four interrelated problems:

(1) Providing fscrypt keys via process-subscribed keyrings is abusing
    encryption as an OS-level access control mechanism, causing many
    bugs where processes don't get access to the keys they need -- e.g.,
    when a 'sudo' command or a system service needs to access encrypted
    files.  It's also inconsistent with the filesystem/VFS "view" of
    encrypted files which is global, so sometimes things randomly happen
    to work anyway due to caching.  Regardless, currently almost all
    fscrypt users actually do need global keys, so they're having to use
    workarounds that heavily abuse the session or user keyrings, e.g.
    Android and Chromium OS both use a systemwide "session keyring" and
    the 'fscrypt' tool links all user keyrings into root's user keyring.

(2) Currently there's no way to securely and efficiently remove a
    fscrypt key such that not only is the original key wiped, but also
    all files and directories protected by that key are "locked" and
    their per-file keys wiped.  Many users want this and are using
    'echo 2 > /proc/sys/vm/drop_caches' as a workaround, but this is
    root-only, and also is overkill so can be a performance disaster.

(3) The key derivation function (KDF) that fscrypt uses to derive
    per-file keys is nonstandard, inflexible, and has some weaknesses
    such as being reversible and not evenly distributing the entropy
    from the user-provided keys.

(4) fscrypt doesn't check that the correct key was supplied.  This can
    be a security vulnerability, since it allows malicious local users
    to associate the wrong key with files to which they have read-only
    access, causing other users' processes to read/write the wrong data.

Ultimately, the solutions to these problems all tie into each other.  By
adding a filesystem-level encryption keyring with ioctls to add/remove
keys to/from it, the keys are made usable filesystem-wide (solves
problem #1).  It also becomes easy to track the inodes that were
"unlocked" with each key, so they can be evicted when the key is removed
(solves problem #2).  Moreover, the filesystem-level keyring is a
natural place to store an HMAC transform keyed by each key, thus making
it easy and efficient to switch the KDF to HKDF (solves problem #3).

Finally, to check that the correct key was supplied, I use HKDF to
derive a cryptographically secure key_identifier for each key (solves
problem #4).  This in combination with key quotas and other careful
precautions also makes it safe to allow non-root users to add and remove
keys to/from the filesystem-level keyring.  Thus, all problems are
solved without having to restrict the fscrypt API to root only.

The patchset is organized as follows:

- Patches 1-10 add new ioctls FS_IOC_ADD_ENCRYPTION_KEY,
  FS_IOC_REMOVE_ENCRYPTION_KEY, and FS_IOC_GET_ENCRYPTION_KEY_STATUS.
  Adding a key logically "unlocks" all files on the filesystem that are
  protected by that key; removing a key "locks" them again.

- Patches 11-14 add support for v2 encryption policies.

- Patches 15-17 wire up the new ioctls to ext4, f2fs, and ubifs.

- Patch 18 updates the fscrypt documentation for all the changes.

Changes v2 => v3:
    - Use ->drop_inode() to trigger the inode eviction during/after
      FS_IOC_REMOVE_ENCRYPTION_KEY, as suggested by Dave Chinner.
    - A few small cleanups.

v1 of this patchset was sent in October 2017 with title "fscrypt:
filesystem-level keyring and v2 policy support".  This revived version
follows the same basic design but incorporates numerous improvements,
such as splitting keyinfo.c into multiple files for much better
understandability, and introducing "per-mode" encryption keys to
implement the semantics of the DIRECT_KEY encryption policy flag.

This applies to the fscrypt.git tree.  You can also get it from git at:

	Repository:   https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git
	Branch:       fscrypt-key-mgmt-improvements-v3

I've started writing xfstests for the new APIs.  So far they cover basic
functionality.  They can be found at:

	Repository:   https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
	Branch:       fscrypt-key-mgmt-improvements

The xfstests depend on new xfs_io commands which can be found at:

	Repository:   https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfsprogs-dev.git
	Branch:       fscrypt-key-mgmt-improvements

I've also made proof-of-concept changes to the 'fscrypt' userspace
program (https://github.com/google/fscrypt) to make it support v2
encryption policies.  You can find these changes in git at:

	Repository:   https://github.com/ebiggers/fscrypt.git
	Branch:       fscrypt-key-mgmt-improvements

To make the 'fscrypt' userspace program experimentally use v2 encryption
policies on new encrypted directories, add the following to
/etc/fscrypt.conf within the "options" section:

	"policy_version": "2"

It's also planned for Android and Chromium OS to switch to the new
ioctls and eventually to v2 encryption policies.  Work-in-progress,
proof-of-concept changes by Satya Tangirala for AOSP can be found at
https://android-review.googlesource.com/q/topic:fscrypt-key-mgmt-improvements

Eric Biggers (18):
  fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h>
  fscrypt: use FSCRYPT_ prefix for uapi constants
  fscrypt: use FSCRYPT_* definitions, not FS_*
  fs: add ->s_master_keys to struct super_block
  fscrypt: add ->ci_inode to fscrypt_info
  fscrypt: refactor v1 policy key setup into keysetup_legacy.c
  fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl
  fs/dcache.c: add shrink_dcache_inode()
  fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl
  fscrypt: add FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl
  fscrypt: add an HKDF-SHA512 implementation
  fscrypt: v2 encryption policy support
  fscrypt: allow unprivileged users to add/remove keys for v2 policies
  fscrypt: require that key be added when setting a v2 encryption policy
  ext4: wire up new fscrypt ioctls
  f2fs: wire up new fscrypt ioctls
  ubifs: wire up new fscrypt ioctls
  fscrypt: document the new ioctls and policy version

 Documentation/filesystems/fscrypt.rst | 670 +++++++++++++++----
 MAINTAINERS                           |   1 +
 fs/crypto/Kconfig                     |   2 +
 fs/crypto/Makefile                    |  10 +-
 fs/crypto/crypto.c                    |  14 +-
 fs/crypto/fname.c                     |   5 +-
 fs/crypto/fscrypt_private.h           | 364 ++++++++--
 fs/crypto/hkdf.c                      | 188 ++++++
 fs/crypto/keyinfo.c                   | 592 -----------------
 fs/crypto/keyring.c                   | 911 ++++++++++++++++++++++++++
 fs/crypto/keysetup.c                  | 540 +++++++++++++++
 fs/crypto/keysetup_legacy.c           | 340 ++++++++++
 fs/crypto/policy.c                    | 388 ++++++++---
 fs/dcache.c                           |  32 +
 fs/ext4/ioctl.c                       |  24 +
 fs/ext4/super.c                       |   3 +
 fs/f2fs/file.c                        |  46 ++
 fs/f2fs/super.c                       |   2 +
 fs/super.c                            |   3 +
 fs/ubifs/ioctl.c                      |  24 +-
 fs/ubifs/super.c                      |   3 +
 include/linux/dcache.h                |   1 +
 include/linux/fs.h                    |   1 +
 include/linux/fscrypt.h               |  43 +-
 include/uapi/linux/fs.h               |  54 +-
 include/uapi/linux/fscrypt.h          | 163 +++++
 26 files changed, 3496 insertions(+), 928 deletions(-)
 create mode 100644 fs/crypto/hkdf.c
 delete mode 100644 fs/crypto/keyinfo.c
 create mode 100644 fs/crypto/keyring.c
 create mode 100644 fs/crypto/keysetup.c
 create mode 100644 fs/crypto/keysetup_legacy.c
 create mode 100644 include/uapi/linux/fscrypt.h

Comments

Andreas Dilger Feb. 20, 2019, 7:18 a.m. UTC | #1
On Feb 19, 2019, at 10:52 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> Hello,
> 
> This patchset makes major improvements to how keys are added, removed,
> and derived in fscrypt, aka ext4/f2fs/ubifs encryption.  It does this by
> adding new ioctls that add and remove encryption keys directly to/from
> the filesystem, and by adding a new encryption policy version ("v2")
> where the user-provided keys are only used as input to HKDF-SHA512 and
> are identified by their cryptographic hash.

Just to confirm for the layman reader - the fact that the crypto keys
are registered with the filesystem and not with the user process doesn't
prevent user(s) from having different crypto keys for different subdirs
in a single filesystem?

Secondly, does this progress fscrypt toward allowing multiple master keys
to decrypt a single per-file key?

Cheers, Andreas

> All new APIs and all cryptosystem changes are documented in
> Documentation/filesystems/fscrypt.rst.  Userspace can use the new key
> management ioctls with existing encrypted directories, but migrating to
> v2 encryption policies is needed for the full benefits.
> 
> These changes solve four interrelated problems:
> 
> (1) Providing fscrypt keys via process-subscribed keyrings is abusing
>    encryption as an OS-level access control mechanism, causing many
>    bugs where processes don't get access to the keys they need -- e.g.,
>    when a 'sudo' command or a system service needs to access encrypted
>    files.  It's also inconsistent with the filesystem/VFS "view" of
>    encrypted files which is global, so sometimes things randomly happen
>    to work anyway due to caching.  Regardless, currently almost all
>    fscrypt users actually do need global keys, so they're having to use
>    workarounds that heavily abuse the session or user keyrings, e.g.
>    Android and Chromium OS both use a systemwide "session keyring" and
>    the 'fscrypt' tool links all user keyrings into root's user keyring.
> 
> (2) Currently there's no way to securely and efficiently remove a
>    fscrypt key such that not only is the original key wiped, but also
>    all files and directories protected by that key are "locked" and
>    their per-file keys wiped.  Many users want this and are using
>    'echo 2 > /proc/sys/vm/drop_caches' as a workaround, but this is
>    root-only, and also is overkill so can be a performance disaster.
> 
> (3) The key derivation function (KDF) that fscrypt uses to derive
>    per-file keys is nonstandard, inflexible, and has some weaknesses
>    such as being reversible and not evenly distributing the entropy
>    from the user-provided keys.
> 
> (4) fscrypt doesn't check that the correct key was supplied.  This can
>    be a security vulnerability, since it allows malicious local users
>    to associate the wrong key with files to which they have read-only
>    access, causing other users' processes to read/write the wrong data.
> 
> Ultimately, the solutions to these problems all tie into each other.  By
> adding a filesystem-level encryption keyring with ioctls to add/remove
> keys to/from it, the keys are made usable filesystem-wide (solves
> problem #1).  It also becomes easy to track the inodes that were
> "unlocked" with each key, so they can be evicted when the key is removed
> (solves problem #2).  Moreover, the filesystem-level keyring is a
> natural place to store an HMAC transform keyed by each key, thus making
> it easy and efficient to switch the KDF to HKDF (solves problem #3).
> 
> Finally, to check that the correct key was supplied, I use HKDF to
> derive a cryptographically secure key_identifier for each key (solves
> problem #4).  This in combination with key quotas and other careful
> precautions also makes it safe to allow non-root users to add and remove
> keys to/from the filesystem-level keyring.  Thus, all problems are
> solved without having to restrict the fscrypt API to root only.
> 
> The patchset is organized as follows:
> 
> - Patches 1-10 add new ioctls FS_IOC_ADD_ENCRYPTION_KEY,
>  FS_IOC_REMOVE_ENCRYPTION_KEY, and FS_IOC_GET_ENCRYPTION_KEY_STATUS.
>  Adding a key logically "unlocks" all files on the filesystem that are
>  protected by that key; removing a key "locks" them again.
> 
> - Patches 11-14 add support for v2 encryption policies.
> 
> - Patches 15-17 wire up the new ioctls to ext4, f2fs, and ubifs.
> 
> - Patch 18 updates the fscrypt documentation for all the changes.
> 
> Changes v2 => v3:
>    - Use ->drop_inode() to trigger the inode eviction during/after
>      FS_IOC_REMOVE_ENCRYPTION_KEY, as suggested by Dave Chinner.
>    - A few small cleanups.
> 
> v1 of this patchset was sent in October 2017 with title "fscrypt:
> filesystem-level keyring and v2 policy support".  This revived version
> follows the same basic design but incorporates numerous improvements,
> such as splitting keyinfo.c into multiple files for much better
> understandability, and introducing "per-mode" encryption keys to
> implement the semantics of the DIRECT_KEY encryption policy flag.
> 
> This applies to the fscrypt.git tree.  You can also get it from git at:
> 
> 	Repository:   https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git
> 	Branch:       fscrypt-key-mgmt-improvements-v3
> 
> I've started writing xfstests for the new APIs.  So far they cover basic
> functionality.  They can be found at:
> 
> 	Repository:   https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
> 	Branch:       fscrypt-key-mgmt-improvements
> 
> The xfstests depend on new xfs_io commands which can be found at:
> 
> 	Repository:   https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfsprogs-dev.git
> 	Branch:       fscrypt-key-mgmt-improvements
> 
> I've also made proof-of-concept changes to the 'fscrypt' userspace
> program (https://github.com/google/fscrypt) to make it support v2
> encryption policies.  You can find these changes in git at:
> 
> 	Repository:   https://github.com/ebiggers/fscrypt.git
> 	Branch:       fscrypt-key-mgmt-improvements
> 
> To make the 'fscrypt' userspace program experimentally use v2 encryption
> policies on new encrypted directories, add the following to
> /etc/fscrypt.conf within the "options" section:
> 
> 	"policy_version": "2"
> 
> It's also planned for Android and Chromium OS to switch to the new
> ioctls and eventually to v2 encryption policies.  Work-in-progress,
> proof-of-concept changes by Satya Tangirala for AOSP can be found at
> https://android-review.googlesource.com/q/topic:fscrypt-key-mgmt-improvements
> 
> Eric Biggers (18):
>  fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h>
>  fscrypt: use FSCRYPT_ prefix for uapi constants
>  fscrypt: use FSCRYPT_* definitions, not FS_*
>  fs: add ->s_master_keys to struct super_block
>  fscrypt: add ->ci_inode to fscrypt_info
>  fscrypt: refactor v1 policy key setup into keysetup_legacy.c
>  fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl
>  fs/dcache.c: add shrink_dcache_inode()
>  fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl
>  fscrypt: add FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl
>  fscrypt: add an HKDF-SHA512 implementation
>  fscrypt: v2 encryption policy support
>  fscrypt: allow unprivileged users to add/remove keys for v2 policies
>  fscrypt: require that key be added when setting a v2 encryption policy
>  ext4: wire up new fscrypt ioctls
>  f2fs: wire up new fscrypt ioctls
>  ubifs: wire up new fscrypt ioctls
>  fscrypt: document the new ioctls and policy version
> 
> Documentation/filesystems/fscrypt.rst | 670 +++++++++++++++----
> MAINTAINERS                           |   1 +
> fs/crypto/Kconfig                     |   2 +
> fs/crypto/Makefile                    |  10 +-
> fs/crypto/crypto.c                    |  14 +-
> fs/crypto/fname.c                     |   5 +-
> fs/crypto/fscrypt_private.h           | 364 ++++++++--
> fs/crypto/hkdf.c                      | 188 ++++++
> fs/crypto/keyinfo.c                   | 592 -----------------
> fs/crypto/keyring.c                   | 911 ++++++++++++++++++++++++++
> fs/crypto/keysetup.c                  | 540 +++++++++++++++
> fs/crypto/keysetup_legacy.c           | 340 ++++++++++
> fs/crypto/policy.c                    | 388 ++++++++---
> fs/dcache.c                           |  32 +
> fs/ext4/ioctl.c                       |  24 +
> fs/ext4/super.c                       |   3 +
> fs/f2fs/file.c                        |  46 ++
> fs/f2fs/super.c                       |   2 +
> fs/super.c                            |   3 +
> fs/ubifs/ioctl.c                      |  24 +-
> fs/ubifs/super.c                      |   3 +
> include/linux/dcache.h                |   1 +
> include/linux/fs.h                    |   1 +
> include/linux/fscrypt.h               |  43 +-
> include/uapi/linux/fs.h               |  54 +-
> include/uapi/linux/fscrypt.h          | 163 +++++
> 26 files changed, 3496 insertions(+), 928 deletions(-)
> create mode 100644 fs/crypto/hkdf.c
> delete mode 100644 fs/crypto/keyinfo.c
> create mode 100644 fs/crypto/keyring.c
> create mode 100644 fs/crypto/keysetup.c
> create mode 100644 fs/crypto/keysetup_legacy.c
> create mode 100644 include/uapi/linux/fscrypt.h
> 
> --
> 2.20.1
> 


Cheers, Andreas
Eric Biggers Feb. 20, 2019, 7:54 a.m. UTC | #2
On Tue, Feb 19, 2019 at 11:18:21PM -0800, Andreas Dilger wrote:
> On Feb 19, 2019, at 10:52 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> > 
> > Hello,
> > 
> > This patchset makes major improvements to how keys are added, removed,
> > and derived in fscrypt, aka ext4/f2fs/ubifs encryption.  It does this by
> > adding new ioctls that add and remove encryption keys directly to/from
> > the filesystem, and by adding a new encryption policy version ("v2")
> > where the user-provided keys are only used as input to HKDF-SHA512 and
> > are identified by their cryptographic hash.
> 
> Just to confirm for the layman reader - the fact that the crypto keys
> are registered with the filesystem and not with the user process doesn't
> prevent user(s) from having different crypto keys for different subdirs
> in a single filesystem?

Correct, that hasn't changed.  Different directories can use different keys.

> 
> Secondly, does this progress fscrypt toward allowing multiple master keys
> to decrypt a single per-file key?
> 

It's not a goal of this patchset as no one has really been asking for it yet.
Per-file keys are still derived from the master key.

Note that it's already possible to wrap the master keys in userspace.  Thus you
can already have multiple passwords that unlock the same encrypted directory.
You just can't do this with *individual files*.  Maybe that's good enough.

However, this patchset does fix the denial of service vulnerabilities that
currently exist when users share an encrypted directory.  Also, since it adds a
real KDF, it makes it much easier to derive a wrapping key to wrap the per-file
keys if that were to become a requirement.  (The master key could be used as the
wrapping key directly, but that's inflexible and more error-prone.)

- Eric
David Howells Feb. 20, 2019, 6:07 p.m. UTC | #3
I have a couple of patches that add ACLs to keyrings, that you can find at the
top of the branch here:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-acl

I have other patches that allow tags to be used as subjects in the ACL, with a
container supplying a tag, e.g.:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=container&id=e0fdc5613a32b9475b025f58e7a2267329b3f3a4

Might something similar be of use to you here, perhaps specifying a tag
referring to the blockdev of interest rather than the container?

David
Eric Biggers Feb. 20, 2019, 6:36 p.m. UTC | #4
Hi David,

On Wed, Feb 20, 2019 at 06:07:22PM +0000, David Howells wrote:
> I have a couple of patches that add ACLs to keyrings, that you can find at the
> top of the branch here:
> 
> 	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-acl
> 
> I have other patches that allow tags to be used as subjects in the ACL, with a
> container supplying a tag, e.g.:
> 
> 	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=container&id=e0fdc5613a32b9475b025f58e7a2267329b3f3a4
> 
> Might something similar be of use to you here, perhaps specifying a tag
> referring to the blockdev of interest rather than the container?
> 
> David

I don't think so.  The main point of adding keys directly to the filesystem is
that it's generally inappropriate to apply OS-level access control or
process-based visibility restrictions to fscrypt keys given that:

- The unlocked/locked status of files is already filesystem-level.
- The purpose of encryption is orthogonal to OS-level access control.

The ioctl based interface also makes it *much* easier to implement all the
semantics needed for removing fscrypt keys.

I don't see how key ACLs would be appropriate here, except that key ACLs would
allow userspace to opt-in to fixing the denial of service vulnerability where
keyctl_invalidate() only requires Search permission.  But for fscrypt that's
addressed by my proposal too, and is just one of many problems it addresses.

- Eric