2017-10-23 21:40:33

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support

From: Eric Biggers <[email protected]>

Hello,

This patchset solves multiple interrelated problems with how filesystem
encryption keys are managed (for ext4, f2fs, and ubifs), including:

(1) There is a visibility mismatch between the filesystem/VFS "view" of
encrypted files (which is global) and the process-subscribed
keyrings (which are not global). Relying on process-subscribed
keyrings to provide the encryption keys on-demand makes it quite
difficult to support even simple things like running 'sudo', if
encrypted files need to be accessed.

(2) There is no API to securely remove an encryption key, which should
wipe all secret keys from memory and revert the encrypted files to
their ciphertext "view". Many users want this, even to the extent
that they're already working around it using the very bad hack of
'echo 2 > /proc/sys/vm/drop_caches', or alternatively hacking in an
ioctl to drop caches for a specific filesystem.

(3) The key derivation function (KDF) used to derive the per-file
encryption keys is nonstandard and has a number of problems, such as
being trivially reversible. We've wanted to replace it for some
time now.

(4) There is no verification that the correct master key was supplied.
This is actually a security vulnerability, as it allows malicious
local users to associate the wrong key with files to which they have
*read-only* access.

This patchset is based loosely on my earlier patchset "fscrypt: key
verification and KDF improvement". However, while the earlier patchset
solved problems (3) and (4) above, it ignored (1) and (2).
Consequently, it ended up with a solution which probably would have had
to be reworked when we also solved (1) and (2). For example, the
'key_hash' field was hacked on to the existing on-disk format just to
solve (4), but really we need it a different way to solve (1) and (2) as
well, at least for non-root users. There was also a filesystem-level
key cache hacked on for caching the HMAC transforms for HKDF, but really
it should be a real keyring which you can add and remove keys from, as
we need that anyway for (1) and (2).

By considering all the problems together we end up with a solution which
should be simpler in the end, notwithstanding the length of this
patchset.

This patchset is organized as follows:

- Patches 1-6 introduce a filesystem-level crypto keyring and a new
ioctl, FS_IOC_ADD_ENCRYPTION_KEY, which adds a master encryption key
to it. This solves problem (1) above, though initially only for use
cases where a privileged process sets up the keys. Patch 20 will make
it unprivileged in some cases.

- Patches 7-10 add a new ioctl, FS_IOC_REMOVE_ENCRYPTION_KEY, which
removes a master encryption key from the filesystem-level crypto
keyring. It also evicts the inodes which had been "unlocked" using
the key. This solves problem (2) above, though initially only for use
cases where a privileged process sets up the keys. Patch 20 will make
it unprivileged in some cases.

- Patch 11 adds an ioctl FS_IOC_GET_ENCRYPTION_KEY_STATUS which
retrieves the status of a key in the filesystem-level crypto keyring.

- Patches 12-14 wire up the above ioctls to ext4, f2fs, and ubifs.

- Patches 15-25 introduce a new encryption policy version ("v2") where
master_key_descriptor is replaced with master_key_identifier, which is
a cryptographic hash of the master key. This allows opening the
FS_IOC_ADD_ENCRYPTION_KEY and FS_IOC_REMOVE_ENCRYPTION_KEY ioctls up
to non-root users. In turn, this avoids any need to rely on the
process-subscribed keyrings and encounter their visibility problems,
and it allows non-root users to securely remove their encryption keys.
I also take the opportunity to replace the AES-ECB-based KDF with
HKDF-SHA512, which is also used to compute the master_key_identifier
so that we pass the master key into only a single cryptographic
primitive.

Note that patches 1-14 can be reviewed (and potentially even merged) on
their own, without patches 15-25. At just that point, the ioctls to
manage filesystem-level keys will be usable for existing encrypted
files, for privileged users only. However, to understand some of the
decisions made when designing the ioctls, it will be helpful to see how
the later patches extend the ioctls to also be usable for v2 encryption
policies and by unprivileged users.

Please review all API and on-disk format changes carefully, as we will
be locked into them once merged.

You can also get this patchset from git at:

Repository: https://github.com/ebiggers/linux.git
Branch: fscrypt-v2-policy-and-api_v1

It has received light testing. I've also made proof-of-concept changes
to the 'fscrypt' userspace program to make it support v2 encryption
policies and the filesystem-level keyring. You can find those userspace
changes in git at:

Repository: https://github.com/ebiggers/fscrypt.git
Branch: v2-policy-support

To make the 'fscrypt' userspace program use v2 policies for new
encrypted directories, add

"policy_version": "2"

to /etc/fscrypt.conf within the "options" section. (Again: for now
please consider the userspace changes proof-of-concept quality only!
So far I've been focusing on the kernel changes.)

It's intended that the other major users of filesystem-level encryption,
including the Android and Chromium OS key management systems, will
switch to the new API and encryption policy version as well.

Eric Biggers (25):
fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h>
fscrypt: use FSCRYPT_ prefix for uapi constants
fscrypt: use FSCRYPT_* definitions, not FS_*
fscrypt: refactor finding and deriving key
fs: add ->s_master_keys to struct super_block
fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl
fs/inode.c: export inode_lru_list_del()
fs/inode.c: rename and export dispose_list()
fs/dcache.c: add shrink_dcache_inode()
fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl
fscrypt: add FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl
ext4 crypto: wire up new ioctls for managing encryption keys
f2fs crypto: wire up new ioctls for managing encryption keys
ubifs crypto: wire up new ioctls for managing encryption keys
fscrypt: add UAPI definitions to get/set v2 encryption policies
fscrypt: implement basic handling of v2 encryption policies
fscrypt: add an HKDF-SHA512 implementation
fscrypt: allow adding and removing keys for v2 encryption policies
fscrypt: use HKDF-SHA512 to derive the per-file keys for v2 policies
fscrypt: allow unprivileged users to add/remove keys for v2 policies
fscrypt: require that key be added when setting a v2 encryption policy
ext4 crypto: wire up FS_IOC_GET_ENCRYPTION_POLICY_EX
f2fs crypto: wire up FS_IOC_GET_ENCRYPTION_POLICY_EX
ubifs crypto: wire up FS_IOC_GET_ENCRYPTION_POLICY_EX
fscrypt: document the new ioctls and policy version

Documentation/filesystems/fscrypt.rst | 575 ++++++++++--
fs/crypto/Kconfig | 2 +
fs/crypto/crypto.c | 19 +-
fs/crypto/fname.c | 4 +-
fs/crypto/fscrypt_private.h | 196 +++-
fs/crypto/keyinfo.c | 1619 ++++++++++++++++++++++++++++++---
fs/crypto/policy.c | 373 +++++---
fs/dcache.c | 33 +
fs/ext4/ioctl.c | 22 +
fs/f2fs/file.c | 21 +-
fs/inode.c | 24 +-
fs/super.c | 3 +
fs/ubifs/ioctl.c | 24 +-
include/linux/dcache.h | 1 +
include/linux/fs.h | 6 +
include/linux/fscrypt.h | 12 +-
include/linux/fscrypt_notsupp.h | 23 +
include/linux/fscrypt_supp.h | 4 +
include/uapi/linux/fs.h | 50 +-
include/uapi/linux/fscrypt.h | 159 ++++
20 files changed, 2724 insertions(+), 446 deletions(-)
create mode 100644 include/uapi/linux/fscrypt.h

--
2.15.0.rc0.271.g36b669edcc-goog


2017-10-23 21:40:34

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 01/25] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h>

From: Eric Biggers <[email protected]>

There are going to be more filesystem encryption definitions added, and
we don't want to use a disproportionate amount of space in <linux/fs.h>
for filesystem encryption stuff. So move the fscrypt definitions to a
new header <linux/fscrypt.h>.

For compatibility with existing userspace programs which may be
including <linux/fs.h>, <linux/fs.h> still includes the new header.
(It's debatable whether we really need this, though; the filesystem
encryption API is new enough that most if not all programs that are
using it have to declare it themselves anyway.)

Signed-off-by: Eric Biggers <[email protected]>
---
include/linux/fscrypt.h | 2 +-
include/uapi/linux/fs.h | 50 +++--------------------------------------
include/uapi/linux/fscrypt.h | 53 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+), 48 deletions(-)
create mode 100644 include/uapi/linux/fscrypt.h

diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 53437bfdfcbc..f7aa7d62e235 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -19,7 +19,7 @@
#include <linux/bio.h>
#include <linux/dcache.h>
#include <crypto/skcipher.h>
-#include <uapi/linux/fs.h>
+#include <uapi/linux/fscrypt.h>

#define FS_CRYPTO_BLOCK_SIZE 16

diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 56235dddea7d..6ecd3ee9960c 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -12,6 +12,9 @@
#include <linux/limits.h>
#include <linux/ioctl.h>
#include <linux/types.h>
+#ifndef __KERNEL__
+#include <linux/fscrypt.h>
+#endif

/*
* It's silly to have NR_OPEN bigger than NR_FILE, but you can change
@@ -253,53 +256,6 @@ struct fsxattr {
#define FS_IOC_FSGETXATTR _IOR ('X', 31, struct fsxattr)
#define FS_IOC_FSSETXATTR _IOW ('X', 32, struct fsxattr)

-/*
- * File system encryption support
- */
-/* Policy provided via an ioctl on the topmost directory */
-#define FS_KEY_DESCRIPTOR_SIZE 8
-
-#define FS_POLICY_FLAGS_PAD_4 0x00
-#define FS_POLICY_FLAGS_PAD_8 0x01
-#define FS_POLICY_FLAGS_PAD_16 0x02
-#define FS_POLICY_FLAGS_PAD_32 0x03
-#define FS_POLICY_FLAGS_PAD_MASK 0x03
-#define FS_POLICY_FLAGS_VALID 0x03
-
-/* Encryption algorithms */
-#define FS_ENCRYPTION_MODE_INVALID 0
-#define FS_ENCRYPTION_MODE_AES_256_XTS 1
-#define FS_ENCRYPTION_MODE_AES_256_GCM 2
-#define FS_ENCRYPTION_MODE_AES_256_CBC 3
-#define FS_ENCRYPTION_MODE_AES_256_CTS 4
-#define FS_ENCRYPTION_MODE_AES_128_CBC 5
-#define FS_ENCRYPTION_MODE_AES_128_CTS 6
-
-struct fscrypt_policy {
- __u8 version;
- __u8 contents_encryption_mode;
- __u8 filenames_encryption_mode;
- __u8 flags;
- __u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
-};
-
-#define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
-#define FS_IOC_GET_ENCRYPTION_PWSALT _IOW('f', 20, __u8[16])
-#define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy)
-
-/* Parameters for passing an encryption key into the kernel keyring */
-#define FS_KEY_DESC_PREFIX "fscrypt:"
-#define FS_KEY_DESC_PREFIX_SIZE 8
-
-/* Structure that userspace passes to the kernel keyring */
-#define FS_MAX_KEY_SIZE 64
-
-struct fscrypt_key {
- __u32 mode;
- __u8 raw[FS_MAX_KEY_SIZE];
- __u32 size;
-};
-
/*
* Inode flags (FS_IOC_GETFLAGS / FS_IOC_SETFLAGS)
*
diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
new file mode 100644
index 000000000000..c09209fc42ea
--- /dev/null
+++ b/include/uapi/linux/fscrypt.h
@@ -0,0 +1,53 @@
+#ifndef _UAPI_LINUX_FSCRYPT_H
+#define _UAPI_LINUX_FSCRYPT_H
+
+#include <linux/types.h>
+
+/*
+ * File system encryption support
+ */
+/* Policy provided via an ioctl on the topmost directory */
+#define FS_KEY_DESCRIPTOR_SIZE 8
+
+#define FS_POLICY_FLAGS_PAD_4 0x00
+#define FS_POLICY_FLAGS_PAD_8 0x01
+#define FS_POLICY_FLAGS_PAD_16 0x02
+#define FS_POLICY_FLAGS_PAD_32 0x03
+#define FS_POLICY_FLAGS_PAD_MASK 0x03
+#define FS_POLICY_FLAGS_VALID 0x03
+
+/* Encryption algorithms */
+#define FS_ENCRYPTION_MODE_INVALID 0
+#define FS_ENCRYPTION_MODE_AES_256_XTS 1
+#define FS_ENCRYPTION_MODE_AES_256_GCM 2
+#define FS_ENCRYPTION_MODE_AES_256_CBC 3
+#define FS_ENCRYPTION_MODE_AES_256_CTS 4
+#define FS_ENCRYPTION_MODE_AES_128_CBC 5
+#define FS_ENCRYPTION_MODE_AES_128_CTS 6
+
+struct fscrypt_policy {
+ __u8 version;
+ __u8 contents_encryption_mode;
+ __u8 filenames_encryption_mode;
+ __u8 flags;
+ __u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
+};
+
+#define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
+#define FS_IOC_GET_ENCRYPTION_PWSALT _IOW('f', 20, __u8[16])
+#define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy)
+
+/* Parameters for passing an encryption key into the kernel keyring */
+#define FS_KEY_DESC_PREFIX "fscrypt:"
+#define FS_KEY_DESC_PREFIX_SIZE 8
+
+/* Structure that userspace passes to the kernel keyring */
+#define FS_MAX_KEY_SIZE 64
+
+struct fscrypt_key {
+ __u32 mode;
+ __u8 raw[FS_MAX_KEY_SIZE];
+ __u32 size;
+};
+
+#endif /* _UAPI_LINUX_FSCRYPT_H */
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:35

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 02/25] fscrypt: use FSCRYPT_ prefix for uapi constants

From: Eric Biggers <[email protected]>

Prefix all filesystem encryption UAPI constants except the ioctl numbers
with "FSCRYPT_" rather than with "FS_". This namespaces the constants
more appropriately and makes it clear that they are related specifically
to the filesystem encryption feature, and to the 'fscrypt_*' structures.
With some of the old names like "FS_POLICY_FLAGS_VALID", it was not
immediately clear that the constant had anything to do with encryption.

This is also useful because we'll be adding more encryption-related
constants, e.g. for the policy version, and we'd otherwise have to
choose whether to use unclear names like FS_POLICY_VERSION_* or
inconsistent names like FS_ENCRYPTION_POLICY_VERSION_*.

For source compatibility with older userspace programs, keep the old
names defined as aliases to the new ones. (It's debatable whether we
really need this, though; the filesystem encryption API is new enough
that most if not all programs that are using it have to declare it
themselves anyway.)

Signed-off-by: Eric Biggers <[email protected]>
---
Documentation/filesystems/fscrypt.rst | 14 ++++-----
include/uapi/linux/fscrypt.h | 59 ++++++++++++++++++++++++-----------
2 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 776ddc655f79..f12956f3f1f8 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -251,14 +251,14 @@ empty directory or verifies that a directory or regular file already
has the specified encryption policy. It takes in a pointer to a
:c:type:`struct fscrypt_policy`, defined as follows::

- #define FS_KEY_DESCRIPTOR_SIZE 8
+ #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8

struct fscrypt_policy {
__u8 version;
__u8 contents_encryption_mode;
__u8 filenames_encryption_mode;
__u8 flags;
- __u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
+ __u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
};

This structure must be initialized as follows:
@@ -274,7 +274,7 @@ This structure must be initialized as follows:

- ``flags`` must be set to a value from ``<linux/fs.h>`` which
identifies the amount of NUL-padding to use when encrypting
- filenames. If unsure, use FS_POLICY_FLAGS_PAD_32 (0x3).
+ filenames. If unsure, use FSCRYPT_POLICY_FLAGS_PAD_32 (0x3).

- ``master_key_descriptor`` specifies how to find the master key in
the keyring; see `Adding keys`_. It is up to userspace to choose a
@@ -374,11 +374,11 @@ followed by the 16-character lower case hex representation of the
``master_key_descriptor`` that was set in the encryption policy. The
key payload must conform to the following structure::

- #define FS_MAX_KEY_SIZE 64
+ #define FSCRYPT_MAX_KEY_SIZE 64

struct fscrypt_key {
u32 mode;
- u8 raw[FS_MAX_KEY_SIZE];
+ u8 raw[FSCRYPT_MAX_KEY_SIZE];
u32 size;
};

@@ -533,7 +533,7 @@ much confusion if an encryption policy were to be added to or removed
from anything other than an empty directory.) The struct is defined
as follows::

- #define FS_KEY_DESCRIPTOR_SIZE 8
+ #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
#define FS_KEY_DERIVATION_NONCE_SIZE 16

struct fscrypt_context {
@@ -541,7 +541,7 @@ as follows::
u8 contents_encryption_mode;
u8 filenames_encryption_mode;
u8 flags;
- u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
+ u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
};

diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index c09209fc42ea..26c381a40279 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -7,30 +7,31 @@
* File system encryption support
*/
/* Policy provided via an ioctl on the topmost directory */
-#define FS_KEY_DESCRIPTOR_SIZE 8
+#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8

-#define FS_POLICY_FLAGS_PAD_4 0x00
-#define FS_POLICY_FLAGS_PAD_8 0x01
-#define FS_POLICY_FLAGS_PAD_16 0x02
-#define FS_POLICY_FLAGS_PAD_32 0x03
-#define FS_POLICY_FLAGS_PAD_MASK 0x03
-#define FS_POLICY_FLAGS_VALID 0x03
+/* Encryption policy flags */
+#define FSCRYPT_POLICY_FLAGS_PAD_4 0x00
+#define FSCRYPT_POLICY_FLAGS_PAD_8 0x01
+#define FSCRYPT_POLICY_FLAGS_PAD_16 0x02
+#define FSCRYPT_POLICY_FLAGS_PAD_32 0x03
+#define FSCRYPT_POLICY_FLAGS_PAD_MASK 0x03
+#define FSCRYPT_POLICY_FLAGS_VALID 0x03

/* Encryption algorithms */
-#define FS_ENCRYPTION_MODE_INVALID 0
-#define FS_ENCRYPTION_MODE_AES_256_XTS 1
-#define FS_ENCRYPTION_MODE_AES_256_GCM 2
-#define FS_ENCRYPTION_MODE_AES_256_CBC 3
-#define FS_ENCRYPTION_MODE_AES_256_CTS 4
-#define FS_ENCRYPTION_MODE_AES_128_CBC 5
-#define FS_ENCRYPTION_MODE_AES_128_CTS 6
+#define FSCRYPT_MODE_INVALID 0
+#define FSCRYPT_MODE_AES_256_XTS 1
+#define FSCRYPT_MODE_AES_256_GCM 2
+#define FSCRYPT_MODE_AES_256_CBC 3
+#define FSCRYPT_MODE_AES_256_CTS 4
+#define FSCRYPT_MODE_AES_128_CBC 5
+#define FSCRYPT_MODE_AES_128_CTS 6

struct fscrypt_policy {
__u8 version;
__u8 contents_encryption_mode;
__u8 filenames_encryption_mode;
__u8 flags;
- __u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
+ __u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
};

#define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
@@ -38,16 +39,36 @@ struct fscrypt_policy {
#define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy)

/* Parameters for passing an encryption key into the kernel keyring */
-#define FS_KEY_DESC_PREFIX "fscrypt:"
-#define FS_KEY_DESC_PREFIX_SIZE 8
+#define FSCRYPT_KEY_DESC_PREFIX "fscrypt:"
+#define FSCRYPT_KEY_DESC_PREFIX_SIZE 8

/* Structure that userspace passes to the kernel keyring */
-#define FS_MAX_KEY_SIZE 64
+#define FSCRYPT_MAX_KEY_SIZE 64

struct fscrypt_key {
__u32 mode;
- __u8 raw[FS_MAX_KEY_SIZE];
+ __u8 raw[FSCRYPT_MAX_KEY_SIZE];
__u32 size;
};
+/**********************************************************************/
+
+/* old names; don't add anything new here! */
+#define FS_POLICY_FLAGS_PAD_4 FSCRYPT_POLICY_FLAGS_PAD_4
+#define FS_POLICY_FLAGS_PAD_8 FSCRYPT_POLICY_FLAGS_PAD_8
+#define FS_POLICY_FLAGS_PAD_16 FSCRYPT_POLICY_FLAGS_PAD_16
+#define FS_POLICY_FLAGS_PAD_32 FSCRYPT_POLICY_FLAGS_PAD_32
+#define FS_POLICY_FLAGS_PAD_MASK FSCRYPT_POLICY_FLAGS_PAD_MASK
+#define FS_POLICY_FLAGS_VALID FSCRYPT_POLICY_FLAGS_VALID
+#define FS_KEY_DESCRIPTOR_SIZE FSCRYPT_KEY_DESCRIPTOR_SIZE
+#define FS_ENCRYPTION_MODE_INVALID FSCRYPT_MODE_INVALID
+#define FS_ENCRYPTION_MODE_AES_256_XTS FSCRYPT_MODE_AES_256_XTS
+#define FS_ENCRYPTION_MODE_AES_256_GCM FSCRYPT_MODE_AES_256_GCM
+#define FS_ENCRYPTION_MODE_AES_256_CBC FSCRYPT_MODE_AES_256_CBC
+#define FS_ENCRYPTION_MODE_AES_256_CTS FSCRYPT_MODE_AES_256_CTS
+#define FS_ENCRYPTION_MODE_AES_128_CBC FSCRYPT_MODE_AES_128_CBC
+#define FS_ENCRYPTION_MODE_AES_128_CTS FSCRYPT_MODE_AES_128_CTS
+#define FS_KEY_DESC_PREFIX FSCRYPT_KEY_DESC_PREFIX
+#define FS_KEY_DESC_PREFIX_SIZE FSCRYPT_KEY_DESC_PREFIX_SIZE
+#define FS_MAX_KEY_SIZE FSCRYPT_MAX_KEY_SIZE

#endif /* _UAPI_LINUX_FSCRYPT_H */
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:37

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 04/25] fscrypt: refactor finding and deriving key

From: Eric Biggers <[email protected]>

In preparation for introducing a new way to find the master keys and
derive the per-file keys, clean up the current method. This includes:

- Introduce a helper function find_and_derive_key() so that we don't
have to add more code directly to fscrypt_get_encryption_info().

- Don't pass the 'struct fscrypt_key' directly into derive_key_aes().
This is in preparation for the case where we find the master key in a
filesystem-level keyring, where (for good reasons) the key payload
will *not* be formatted as the UAPI 'struct fscrypt_key'.

- Separate finding the key from key derivation. In particular, it
*only* makes sense to fall back to the alternate key description
prefix if searching for the "fscrypt:" prefix returns -ENOKEY. It
doesn't make sense to do so when derive_key_aes() fails, for example.

- Improve the error messages for when the fscrypt_key is invalid.

- Rename 'raw_key' to 'derived_key' for clarity.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/crypto/keyinfo.c | 205 ++++++++++++++++++++++++++++------------------------
1 file changed, 109 insertions(+), 96 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index ac41f646e7b7..d3a97c2cd4dd 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -28,113 +28,138 @@ static void derive_crypt_complete(struct crypto_async_request *req, int rc)
complete(&ecr->completion);
}

-/**
- * derive_key_aes() - Derive a key using AES-128-ECB
- * @deriving_key: Encryption key used for derivation.
- * @source_key: Source key to which to apply derivation.
- * @derived_raw_key: Derived raw key.
+/*
+ * Key derivation function. This generates the derived key by encrypting the
+ * master key with AES-128-ECB using the nonce as the AES key.
*
- * Return: Zero on success; non-zero otherwise.
+ * The master key must be at least as long as the derived key. If the master
+ * key is longer, then only the first 'derived_keysize' bytes are used.
*/
-static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
- const struct fscrypt_key *source_key,
- u8 derived_raw_key[FSCRYPT_MAX_KEY_SIZE])
+static int derive_key_aes(const u8 *master_key,
+ const struct fscrypt_context *ctx,
+ u8 *derived_key, unsigned int derived_keysize)
{
- int res = 0;
+ int err;
struct skcipher_request *req = NULL;
DECLARE_FS_COMPLETION_RESULT(ecr);
struct scatterlist src_sg, dst_sg;
- struct crypto_skcipher *tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
+ struct crypto_skcipher *tfm;
+
+ tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);

- if (IS_ERR(tfm)) {
- res = PTR_ERR(tfm);
- tfm = NULL;
- goto out;
- }
crypto_skcipher_set_flags(tfm, CRYPTO_TFM_REQ_WEAK_KEY);
req = skcipher_request_alloc(tfm, GFP_NOFS);
if (!req) {
- res = -ENOMEM;
+ err = -ENOMEM;
goto out;
}
skcipher_request_set_callback(req,
CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
derive_crypt_complete, &ecr);
- res = crypto_skcipher_setkey(tfm, deriving_key,
- FS_AES_128_ECB_KEY_SIZE);
- if (res < 0)
+
+ BUILD_BUG_ON(sizeof(ctx->nonce) != FS_AES_128_ECB_KEY_SIZE);
+ err = crypto_skcipher_setkey(tfm, ctx->nonce, sizeof(ctx->nonce));
+ if (err)
goto out;

- sg_init_one(&src_sg, source_key->raw, source_key->size);
- sg_init_one(&dst_sg, derived_raw_key, source_key->size);
- skcipher_request_set_crypt(req, &src_sg, &dst_sg, source_key->size,
+ sg_init_one(&src_sg, master_key, derived_keysize);
+ sg_init_one(&dst_sg, derived_key, derived_keysize);
+ skcipher_request_set_crypt(req, &src_sg, &dst_sg, derived_keysize,
NULL);
- res = crypto_skcipher_encrypt(req);
- if (res == -EINPROGRESS || res == -EBUSY) {
+ err = crypto_skcipher_encrypt(req);
+ if (err == -EINPROGRESS || err == -EBUSY) {
wait_for_completion(&ecr.completion);
- res = ecr.res;
+ err = ecr.res;
}
out:
skcipher_request_free(req);
crypto_free_skcipher(tfm);
- return res;
+ return err;
}

-static int validate_user_key(struct fscrypt_info *crypt_info,
- struct fscrypt_context *ctx, u8 *raw_key,
- const char *prefix, int min_keysize)
+/*
+ * Search the current task's subscribed keyrings for a "logon" key with
+ * description prefix:descriptor, and if found acquire a read lock on it and
+ * return a pointer to its validated payload in *payload_ret.
+ */
+static struct key *
+find_and_lock_process_key(const char *prefix,
+ const u8 descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE],
+ unsigned int min_keysize,
+ const struct fscrypt_key **payload_ret)
{
char *description;
- struct key *keyring_key;
- struct fscrypt_key *master_key;
+ struct key *key;
const struct user_key_payload *ukp;
- int res;
+ const struct fscrypt_key *payload;

description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
- FSCRYPT_KEY_DESCRIPTOR_SIZE,
- ctx->master_key_descriptor);
+ FSCRYPT_KEY_DESCRIPTOR_SIZE, descriptor);
if (!description)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);

- keyring_key = request_key(&key_type_logon, description, NULL);
+ key = request_key(&key_type_logon, description, NULL);
kfree(description);
- if (IS_ERR(keyring_key))
- return PTR_ERR(keyring_key);
- down_read(&keyring_key->sem);
-
- if (keyring_key->type != &key_type_logon) {
- printk_once(KERN_WARNING
- "%s: key type must be logon\n", __func__);
- res = -ENOKEY;
- goto out;
- }
- ukp = user_key_payload_locked(keyring_key);
- if (!ukp) {
- /* key was revoked before we acquired its semaphore */
- res = -EKEYREVOKED;
- goto out;
+ if (IS_ERR(key))
+ return key;
+
+ down_read(&key->sem);
+ ukp = user_key_payload_locked(key);
+
+ if (!ukp) /* was the key revoked before we acquired its semaphore? */
+ goto invalid;
+
+ payload = (const struct fscrypt_key *)ukp->data;
+
+ if (ukp->datalen != sizeof(struct fscrypt_key) ||
+ payload->size < 1 || payload->size > FSCRYPT_MAX_KEY_SIZE) {
+ pr_warn_ratelimited("fscrypt: key with description '%s' has invalid payload\n",
+ key->description);
+ goto invalid;
}
- if (ukp->datalen != sizeof(struct fscrypt_key)) {
- res = -EINVAL;
- goto out;
+
+ if (payload->size < min_keysize) {
+ pr_warn_ratelimited("fscrypt: key with description '%s' is too short "
+ "(got %u bytes, need %u+ bytes)\n",
+ key->description,
+ payload->size, min_keysize);
+ goto invalid;
}
- master_key = (struct fscrypt_key *)ukp->data;
- BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
-
- if (master_key->size < min_keysize ||
- master_key->size > FSCRYPT_MAX_KEY_SIZE
- || master_key->size % AES_BLOCK_SIZE != 0) {
- printk_once(KERN_WARNING
- "%s: key size incorrect: %d\n",
- __func__, master_key->size);
- res = -ENOKEY;
- goto out;
+
+ *payload_ret = payload;
+ return key;
+
+invalid:
+ up_read(&key->sem);
+ key_put(key);
+ return ERR_PTR(-ENOKEY);
+}
+
+/* Find the master key, then derive the inode's actual encryption key */
+static int find_and_derive_key(const struct inode *inode,
+ const struct fscrypt_context *ctx,
+ u8 *derived_key, unsigned int derived_keysize)
+{
+ struct key *key;
+ const struct fscrypt_key *payload;
+ int err;
+
+ key = find_and_lock_process_key(FSCRYPT_KEY_DESC_PREFIX,
+ ctx->master_key_descriptor,
+ derived_keysize, &payload);
+ if (key == ERR_PTR(-ENOKEY) && inode->i_sb->s_cop->key_prefix) {
+ key = find_and_lock_process_key(inode->i_sb->s_cop->key_prefix,
+ ctx->master_key_descriptor,
+ derived_keysize, &payload);
}
- res = derive_key_aes(ctx->nonce, master_key, raw_key);
-out:
- up_read(&keyring_key->sem);
- key_put(keyring_key);
- return res;
+ if (IS_ERR(key))
+ return PTR_ERR(key);
+ err = derive_key_aes(payload->raw, ctx, derived_key, derived_keysize);
+ up_read(&key->sem);
+ key_put(key);
+ return err;
}

static const struct {
@@ -256,8 +281,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
struct fscrypt_context ctx;
struct crypto_skcipher *ctfm;
const char *cipher_str;
- int keysize;
- u8 *raw_key = NULL;
+ unsigned int derived_keysize;
+ u8 *derived_key = NULL;
int res;

if (inode->i_crypt_info)
@@ -301,7 +326,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
sizeof(crypt_info->ci_master_key));

- res = determine_cipher_type(crypt_info, inode, &cipher_str, &keysize);
+ res = determine_cipher_type(crypt_info, inode,
+ &cipher_str, &derived_keysize);
if (res)
goto out;

@@ -310,24 +336,14 @@ int fscrypt_get_encryption_info(struct inode *inode)
* crypto API as part of key derivation.
*/
res = -ENOMEM;
- raw_key = kmalloc(FSCRYPT_MAX_KEY_SIZE, GFP_NOFS);
- if (!raw_key)
+ derived_key = kmalloc(FS_MAX_KEY_SIZE, GFP_NOFS);
+ if (!derived_key)
goto out;

- res = validate_user_key(crypt_info, &ctx, raw_key,
- FSCRYPT_KEY_DESC_PREFIX, keysize);
- if (res && inode->i_sb->s_cop->key_prefix) {
- int res2 = validate_user_key(crypt_info, &ctx, raw_key,
- inode->i_sb->s_cop->key_prefix,
- keysize);
- if (res2) {
- if (res2 == -ENOKEY)
- res = -ENOKEY;
- goto out;
- }
- } else if (res) {
+ res = find_and_derive_key(inode, &ctx, derived_key, derived_keysize);
+ if (res)
goto out;
- }
+
ctfm = crypto_alloc_skcipher(cipher_str, 0, 0);
if (!ctfm || IS_ERR(ctfm)) {
res = ctfm ? PTR_ERR(ctfm) : -ENOMEM;
@@ -338,17 +354,14 @@ int fscrypt_get_encryption_info(struct inode *inode)
crypt_info->ci_ctfm = ctfm;
crypto_skcipher_clear_flags(ctfm, ~0);
crypto_skcipher_set_flags(ctfm, CRYPTO_TFM_REQ_WEAK_KEY);
- /*
- * if the provided key is longer than keysize, we use the first
- * keysize bytes of the derived key only
- */
- res = crypto_skcipher_setkey(ctfm, raw_key, keysize);
+ res = crypto_skcipher_setkey(ctfm, derived_key, derived_keysize);
if (res)
goto out;

if (S_ISREG(inode->i_mode) &&
crypt_info->ci_data_mode == FSCRYPT_MODE_AES_128_CBC) {
- res = init_essiv_generator(crypt_info, raw_key, keysize);
+ res = init_essiv_generator(crypt_info, derived_key,
+ derived_keysize);
if (res) {
pr_debug("%s: error %d (inode %lu) allocating essiv tfm\n",
__func__, res, inode->i_ino);
@@ -361,7 +374,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
if (res == -ENOKEY)
res = 0;
put_crypt_info(crypt_info);
- kzfree(raw_key);
+ kzfree(derived_key);
return res;
}
EXPORT_SYMBOL(fscrypt_get_encryption_info);
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:42:27

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 13/25] f2fs crypto: wire up new ioctls for managing encryption keys

From: Eric Biggers <[email protected]>

Signed-off-by: Eric Biggers <[email protected]>
---
fs/f2fs/file.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 517e112c8a9a..0296a9594fe7 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2651,6 +2651,12 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
return f2fs_ioc_get_encryption_policy(filp, arg);
case F2FS_IOC_GET_ENCRYPTION_PWSALT:
return f2fs_ioc_get_encryption_pwsalt(filp, arg);
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ return fscrypt_ioctl_add_key(filp, (void __user *)arg);
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ return fscrypt_ioctl_remove_key(filp, (const void __user *)arg);
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
+ return fscrypt_ioctl_get_key_status(filp, (void __user *)arg);
case F2FS_IOC_GARBAGE_COLLECT:
return f2fs_ioc_gc(filp, arg);
case F2FS_IOC_GARBAGE_COLLECT_RANGE:
@@ -2731,6 +2737,9 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case F2FS_IOC_SET_ENCRYPTION_POLICY:
case F2FS_IOC_GET_ENCRYPTION_PWSALT:
case F2FS_IOC_GET_ENCRYPTION_POLICY:
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
case F2FS_IOC_GARBAGE_COLLECT:
case F2FS_IOC_GARBAGE_COLLECT_RANGE:
case F2FS_IOC_WRITE_CHECKPOINT:
--
2.15.0.rc0.271.g36b669edcc-goog

2017-10-23 21:40:42

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 09/25] fs/dcache.c: add shrink_dcache_inode()

From: Eric Biggers <[email protected]>

When a filesystem encryption key is removed, we need all files which had
been "unlocked" (had ->i_crypt_info set up) with it to appear "locked"
again. This is most easily done by evicting the inodes. This can
currently be done using 'echo 2 > /proc/sys/vm/drop_caches'; however,
that is overkill and not usable by non-root users.

To evict just the needed inodes we also need the ability to evict those
inodes' dentries, since an inode is pinned by its dentries. Therefore,
add a function shrink_dcache_inode() which iterates through an inode's
dentries and evicts any unused ones as well as any unused descendants
(since there may be negative dentries pinning the inode's dentries).

Signed-off-by: Eric Biggers <[email protected]>
---
fs/dcache.c | 33 +++++++++++++++++++++++++++++++++
include/linux/dcache.h | 1 +
2 files changed, 34 insertions(+)

diff --git a/fs/dcache.c b/fs/dcache.c
index f90141387f01..455540e889f8 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1456,6 +1456,39 @@ void shrink_dcache_parent(struct dentry *parent)
}
EXPORT_SYMBOL(shrink_dcache_parent);

+/**
+ * shrink_dcache_inode - prune dcache for inode
+ * @inode: inode to prune
+ *
+ * Evict all unused aliases of the specified inode from the dcache. This is
+ * intended to be used when trying to evict a specific inode, since inodes are
+ * pinned by their dentries. We also have to descend to ->d_subdirs for each
+ * alias, since aliases may be pinned by negative child dentries.
+ */
+void shrink_dcache_inode(struct inode *inode)
+{
+ for (;;) {
+ struct select_data data;
+ struct dentry *dentry;
+
+ INIT_LIST_HEAD(&data.dispose);
+ data.start = NULL;
+ data.found = 0;
+
+ spin_lock(&inode->i_lock);
+ hlist_for_each_entry(dentry, &inode->i_dentry, d_u.d_alias)
+ d_walk(dentry, &data, select_collect, NULL);
+ spin_unlock(&inode->i_lock);
+
+ if (!data.found)
+ break;
+
+ shrink_dentry_list(&data.dispose);
+ cond_resched();
+ }
+}
+EXPORT_SYMBOL(shrink_dcache_inode);
+
static enum d_walk_ret umount_check(void *_data, struct dentry *dentry)
{
/* it has busy descendents; complain about those instead */
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index ed1a7cf6923a..fb08199d67d5 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -245,6 +245,7 @@ extern struct dentry * d_obtain_alias(struct inode *);
extern struct dentry * d_obtain_root(struct inode *);
extern void shrink_dcache_sb(struct super_block *);
extern void shrink_dcache_parent(struct dentry *);
+extern void shrink_dcache_inode(struct inode *);
extern void shrink_dcache_for_umount(struct super_block *);
extern void d_invalidate(struct dentry *);

--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:38

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 05/25] fs: add ->s_master_keys to struct super_block

From: Eric Biggers <[email protected]>

Add an ->s_master_keys keyring to 'struct super_block' for holding
encryption keys which have been added to the filesystem. This keyring
will be populated using a new fscrypt ioctl.

This is needed for several reasons, including:

- To solve the visibility problems of having filesystem encryption keys
stored in process-subscribed keyrings, while the VFS state of the
filesystem is actually global.

- To implement a proper API for removing keys, which among other things
will require maintaining the list of inodes that are using each master
key so that we can evict the inodes when the key is removed.

- To allow caching a crypto transform for each master key so that we
don't have to repeatedly allocate one over and over.

See later patches for full details, including why it wouldn't be enough
to add the concept of a "global keyring" to the keyrings API instead.

->s_master_keys will only be allocated when someone tries to add a key
for the first time. Otherwise it will stay NULL.

Note that this could go in the filesystem-specific superblocks instead.
However, we already have three filesystems using fs/crypto/, so it's
useful to have it in the VFS.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/super.c | 3 +++
include/linux/fs.h | 4 ++++
2 files changed, 7 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index 166c4ee0d0ed..161a9d05aa9f 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -168,6 +168,9 @@ static void destroy_super(struct super_block *s)
WARN_ON(!list_empty(&s->s_mounts));
put_user_ns(s->s_user_ns);
kfree(s->s_subtype);
+#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
+ key_put(s->s_master_keys);
+#endif
call_rcu(&s->rcu, destroy_super_rcu);
}

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3efd5ded21c9..8cfb0877d32c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1440,6 +1440,10 @@ struct super_block {

spinlock_t s_inode_wblist_lock;
struct list_head s_inodes_wb; /* writeback inodes */
+
+#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
+ struct key *s_master_keys; /* master crypto keys in use */
+#endif
} __randomize_layout;

/* Helper functions so that in most cases filesystems will
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:40

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 07/25] fs/inode.c: export inode_lru_list_del()

From: Eric Biggers <[email protected]>

When a filesystem encryption key is removed, we need all files which had
been "unlocked" (had ->i_crypt_info set up) with it to appear "locked"
again. This is most easily done by evicting the inodes. This can
currently be done using 'echo 2 > /proc/sys/vm/drop_caches'; however,
that is overkill and not usable by non-root users. In preparation for
allowing fs/crypto/ to evict just the needed inodes, export
inode_lru_list_del() to modules.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/inode.c | 5 ++---
include/linux/fs.h | 1 +
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index d1e35b53bb23..30ce98956801 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -420,13 +420,12 @@ void inode_add_lru(struct inode *inode)
inode_lru_list_add(inode);
}

-
-static void inode_lru_list_del(struct inode *inode)
+void inode_lru_list_del(struct inode *inode)
{
-
if (list_lru_del(&inode->i_sb->s_inode_lru, &inode->i_lru))
this_cpu_dec(nr_unused);
}
+EXPORT_SYMBOL_GPL(inode_lru_list_del);

/**
* inode_sb_list_add - add inode to the superblock list of inodes
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8cfb0877d32c..2833ace2f01d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2879,6 +2879,7 @@ static inline void lockdep_annotate_inode_mutex_key(struct inode *inode) { };
extern void unlock_new_inode(struct inode *);
extern unsigned int get_next_ino(void);
extern void evict_inodes(struct super_block *sb);
+extern void inode_lru_list_del(struct inode *inode);

extern void __iget(struct inode * inode);
extern void iget_failed(struct inode *);
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:39

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 06/25] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl

From: Eric Biggers <[email protected]>

Add a new filesystem encryption ioctl, FS_IOC_ADD_ENCRYPTION_KEY. This
ioctl adds a master encryption key to the filesystem encryption keyring
->s_master_keys.

When a process tries to access an encrypted file that has not yet been
"unlocked" (set up with an ->i_crypt_info containing a crypto transform
keyed by the file's derived key), fscrypt_get_encryption_info() will
search for the master key in ->s_master_keys before falling back to the
process-subscribed keyrings.

For now this ioctl is root-only, which is necessary in part because the
keys are identified by master_key_descriptor, which is not
cryptographically tied to the actual key payload. However, a later
patch will introduce a new encryption policy version where the key is
identified by a cryptographic hash. This will, in combination with
other protections, make it possible for non-root users to use this ioctl
in some situations.

Why we need this
~~~~~~~~~~~~~~~~

The main problem is that the "locked/unlocked" (ciphertext/plaintext)
status of encrypted files is global, but currently the master keys are
not. We only look for master keys in the process-subscribed keyrings;
that is, the current thread keyring, process keyring, and session
keyring, where the session keyring usually contains the user keyring.
This means we have to put the master keys in the keyrings for individual
users or for individual sessions. This causes much confusion as soon as
a process with a different UID, such as a 'sudo' command, tries to
access encrypted files. In such a situation, whether each individual
inode appears "locked" or "unlocked" will depend on whether it was
previously accessed and happens to still be in the inode cache, which is
more or less nondeterministic.

It may seem that we should indeed provide each process its own "view" of
the filesystem depending on whether it "has the key" or not. However
that would be extremely difficult to do without a separate mount, due to
the way the VFS caches work. Furthermore, it is actually missing the
point of encryption because it would *not* be encryption that would
provide the different "views", but rather kernel *code*. Thus, it would
simply be an access control mechanism largely redundant with the many
existing access control mechanisms such as UNIX file permissions and
LSMs. The reality is that the confidentially of encrypted files *after
the kernel already has the encryption key in memory* is only protected
by the correctness of the kernel, not by the mathematical properties of
encryption.

And at the end of the day, almost all users of filesystem encryption we
are aware of do really need the global view, because they need encrypted
files to be accessible to processes running under different UIDs. This
can be as simple as needing to be able to run 'sudo', or it can be
something more complex like Android's key management system where
applications running under different UIDs as well as system processes
need access to the same encrypted files.

As a result, some very ugly hacks have been added to try to emulate
globally visible keys. The Android and Chromium OS key management
systems simply create a "session" keyring in PID 1 and put all the keys
in it, which abuses the "session" keyring to have nothing to do with a
"session", but rather be a global keyring. This is fragile, as it means
that the "session" keyring must never be changed. There have also been
bugs involving processes that were forked before the "session" keyring
was created, causing them to miss out on the keys.

Meanwhile, filesystem encryption tools written for general-purpose Linux
distributions have no such ability to abuse the "session" keyring. They
instead must implement "interesting" workarounds such as linking all the
user keyrings into root's user keyring, as is done by the fscrypt
userspace tool (see the design document at https://goo.gl/55cCrI). This
raises security concerns, to say the least.

By having an API to add a key to the *filesystem* we'll be able to
eliminate all the above hacks and better express the intended semantics:
the "locked/unlocked" status of an encrypted directory is global. And
orthogonally to encryption, existing mechanisms such as file permissions
and LSMs can and should continue to be used for the purpose of *access
control*.

Why use a custom key type
~~~~~~~~~~~~~~~~~~~~~~~~~

The keys the new ioctl adds to ->s_master_keys are still "keys" in the
sense of the keyrings service, but they have a custom key type rather
than the "logon" key type we currently require when userspace provides a
key via a process-subscribed keyring.

Judging just from this patch alone, the "logon" key type would be
sufficient. However, later patches will be solving problems such as the
nonstandard KDF and the lack of a key removal API. The solutions for
these problems will require tracking information on a per-master-key
basis. Therefore, we'll need a custom structure associated with each
master key anyway. A custom key type lets us do that easily.

Why not use add_key()
~~~~~~~~~~~~~~~~~~~~~

Instead of adding a new ioctl() to add a key, we could have userspace
use the add_key() system call. In combination with an ioctl which
retrieves the key ID of ->s_master_keys, add_key() could be used to add
a key to ->s_master_keys. Alternatively, we could add the concept of a
"global keyring" or "namespace keyring" to the keyrings service, where
that keyring would be searched in addition to the process-subscribed
keyrings. Then, add_key() could add a key to that.

This actually makes sense given only the present patch. However,
unfortunately it falls apart once we consider the follow-on changes.

First, we also need to add the ability to remove an encryption key, and
it will need to have more specialized semantics than keyctl_unlink() or
keyctl_revoke() can provide. For example, we must not only wipe the
master key *secret* from memory, but we must also try to evict all the
inodes which had been "unlocked" using the key. And it's possible that
even though the master key secret was wiped, some inodes could not be
evicted, since they may be busy. In that case, we still want to wipe
the master key *secret* so that no more encrypted files can be
"unlocked". But we also want to allow userspace to retry the request
later, so that evicting the remaining inodes can be re-attempted.
Alternatively, we want the same list of inodes to be picked up again if
the secret happens to be added again. Trying to shoehorn these specific
semantics into the keyrings API would be very difficult.

Later we also want to open up the add/remove key operations to non-root
users by taking advantage of a new encryption policy version which
includes a cryptographic hash of the master key. This is needed because
otherwise we wouldn't be able to fully replace the process-subscribed
keyrings and avoid all its problems mentioned earlier. But to actually
make non-root use secure, we'll need to do some extra accounting where
we keep track of all users who have added a given key, then only really
remove a key after all users have removed it. Non-root users also
cannot simply be given write permission to a global keyring. So again,
it seems that trying to shoehorn the needed semantics into the keyrings
API would just create problems.

Nevertheless, we do still use the keyrings service internally so that we
reuse some code and get some "free" functionality such as having the
keys show up in /proc/keys for debugging purposes.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/crypto/crypto.c | 12 +-
fs/crypto/fscrypt_private.h | 3 +
fs/crypto/keyinfo.c | 351 +++++++++++++++++++++++++++++++++++++++-
include/linux/fscrypt_notsupp.h | 5 +
include/linux/fscrypt_supp.h | 1 +
include/uapi/linux/fscrypt.h | 41 +++--
6 files changed, 397 insertions(+), 16 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 608f6bbe0f31..489c504ac20d 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -24,6 +24,7 @@
#include <linux/module.h>
#include <linux/scatterlist.h>
#include <linux/ratelimit.h>
+#include <linux/key-type.h>
#include <linux/dcache.h>
#include <linux/namei.h>
#include <crypto/aes.h>
@@ -449,6 +450,8 @@ int fscrypt_initialize(unsigned int cop_flags)
*/
static int __init fscrypt_init(void)
{
+ int err = -ENOMEM;
+
fscrypt_read_workqueue = alloc_workqueue("fscrypt_read_queue",
WQ_HIGHPRI, 0);
if (!fscrypt_read_workqueue)
@@ -462,14 +465,20 @@ static int __init fscrypt_init(void)
if (!fscrypt_info_cachep)
goto fail_free_ctx;

+ err = register_key_type(&key_type_fscrypt_mk);
+ if (err)
+ goto fail_free_info;
+
return 0;

+fail_free_info:
+ kmem_cache_destroy(fscrypt_info_cachep);
fail_free_ctx:
kmem_cache_destroy(fscrypt_ctx_cachep);
fail_free_queue:
destroy_workqueue(fscrypt_read_workqueue);
fail:
- return -ENOMEM;
+ return err;
}
module_init(fscrypt_init)

@@ -484,6 +493,7 @@ static void __exit fscrypt_exit(void)
destroy_workqueue(fscrypt_read_workqueue);
kmem_cache_destroy(fscrypt_ctx_cachep);
kmem_cache_destroy(fscrypt_info_cachep);
+ unregister_key_type(&key_type_fscrypt_mk);

fscrypt_essiv_cleanup();
}
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 5cb80a2d39ea..b2fad12eeedb 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -27,6 +27,8 @@

#define FS_KEY_DERIVATION_NONCE_SIZE 16

+#define FSCRYPT_MIN_KEY_SIZE 16
+
/**
* Encryption context for inode
*
@@ -93,6 +95,7 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
gfp_t gfp_flags);

/* keyinfo.c */
+extern struct key_type key_type_fscrypt_mk;
extern void __exit fscrypt_essiv_cleanup(void);

#endif /* _FSCRYPT_PRIVATE_H */
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index d3a97c2cd4dd..3f1cb8bbc1e5 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -9,14 +9,307 @@
*/

#include <keys/user-type.h>
-#include <linux/scatterlist.h>
+#include <linux/key-type.h>
#include <linux/ratelimit.h>
+#include <linux/scatterlist.h>
+#include <linux/seq_file.h>
#include <crypto/aes.h>
#include <crypto/sha.h>
#include "fscrypt_private.h"

static struct crypto_shash *essiv_hash_tfm;

+/*
+ * fscrypt_master_key_secret - secret key material of an in-use master key
+ */
+struct fscrypt_master_key_secret {
+
+ /* Size of the raw key in bytes */
+ u32 size;
+
+ /* The raw key */
+ u8 raw[FSCRYPT_MAX_KEY_SIZE];
+};
+
+/*
+ * fscrypt_master_key - an in-use master key
+ *
+ * This represents a master encryption key which has been added to the
+ * filesystem and can be used to "unlock" the encrypted files which were
+ * encrypted with it.
+ */
+struct fscrypt_master_key {
+
+ /* The secret key material */
+ struct fscrypt_master_key_secret mk_secret;
+
+ /* Arbitrary key descriptor which was assigned by userspace */
+ struct fscrypt_key_specifier mk_spec;
+};
+
+static inline int master_key_spec_len(const struct fscrypt_key_specifier *spec)
+{
+ switch (spec->type) {
+ case FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR:
+ return FSCRYPT_KEY_DESCRIPTOR_SIZE;
+ }
+ return 0;
+}
+
+static inline bool valid_key_spec(const struct fscrypt_key_specifier *spec)
+{
+ if (spec->reserved)
+ return false;
+ return master_key_spec_len(spec) != 0;
+}
+
+static void wipe_master_key_secret(struct fscrypt_master_key_secret *secret)
+{
+ memzero_explicit(secret, sizeof(*secret));
+}
+
+static void move_master_key_secret(struct fscrypt_master_key_secret *dst,
+ struct fscrypt_master_key_secret *src)
+{
+ memcpy(dst, src, sizeof(*dst));
+ memzero_explicit(src, sizeof(*src));
+}
+
+static void free_master_key(struct fscrypt_master_key *mk)
+{
+ wipe_master_key_secret(&mk->mk_secret);
+ kzfree(mk);
+}
+
+static int fscrypt_key_instantiate(struct key *key,
+ struct key_preparsed_payload *prep)
+{
+ key->payload.data[0] = (struct fscrypt_master_key *)prep->data;
+ return 0;
+}
+
+static void fscrypt_key_destroy(struct key *key)
+{
+ free_master_key(key->payload.data[0]);
+}
+
+static void fscrypt_key_describe(const struct key *key, struct seq_file *m)
+{
+ seq_puts(m, key->description);
+}
+
+/*
+ * Type of key in ->s_master_keys. Each key of this type represents a master
+ * key which has been added to the filesystem. Its payload is a
+ * 'struct fscrypt_master_key'.
+ */
+struct key_type key_type_fscrypt_mk = {
+ .name = "._fscrypt",
+ .instantiate = fscrypt_key_instantiate,
+ .destroy = fscrypt_key_destroy,
+ .describe = fscrypt_key_describe,
+};
+
+/*
+ * Search ->s_master_keys. Note that we mark the keyring reference as
+ * "possessed" so that we can use the KEY_POS_SEARCH permission.
+ */
+static struct key *search_fscrypt_keyring(struct key *keyring,
+ struct key_type *type,
+ const char *description)
+{
+ key_ref_t keyref;
+
+ keyref = keyring_search(make_key_ref(keyring, 1), type, description);
+ if (IS_ERR(keyref)) {
+ if (PTR_ERR(keyref) == -EAGAIN)
+ keyref = ERR_PTR(-ENOKEY);
+ return ERR_CAST(keyref);
+ }
+ return key_ref_to_ptr(keyref);
+}
+
+#define FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE \
+ (sizeof("fscrypt-") - 1 + sizeof(((struct super_block *)0)->s_id) + 1)
+
+#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_DESCRIPTOR_SIZE + 1)
+
+static void format_fs_keyring_description(
+ char description[FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE],
+ const struct super_block *sb)
+{
+ sprintf(description, "fscrypt-%s", sb->s_id);
+}
+
+static void format_mk_description(
+ char description[FSCRYPT_MK_DESCRIPTION_SIZE],
+ const struct fscrypt_key_specifier *mk_spec)
+{
+ sprintf(description, "%*phN",
+ master_key_spec_len(mk_spec), mk_spec->max_specifier);
+}
+
+/*
+ * Find the specified master key in ->s_master_keys.
+ * Returns ERR_PTR(-ENOKEY) if not found.
+ */
+static struct key *find_master_key(struct super_block *sb,
+ const struct fscrypt_key_specifier *mk_spec)
+{
+ struct key *keyring;
+ char description[FSCRYPT_MK_DESCRIPTION_SIZE];
+
+ /* pairs with smp_store_release() in add_to_filesystem_keyring() */
+ keyring = smp_load_acquire(&sb->s_master_keys);
+ if (keyring == NULL)
+ return ERR_PTR(-ENOKEY);
+
+ format_mk_description(description, mk_spec);
+ return search_fscrypt_keyring(keyring, &key_type_fscrypt_mk,
+ description);
+}
+
+static struct key *
+allocate_master_key(struct fscrypt_master_key_secret *secret,
+ const struct fscrypt_key_specifier *mk_spec)
+{
+ struct fscrypt_master_key *mk;
+ struct key *key;
+ char description[FSCRYPT_MK_DESCRIPTION_SIZE];
+ int err;
+
+ mk = kzalloc(sizeof(*mk), GFP_NOFS);
+ if (!mk)
+ return ERR_PTR(-ENOMEM);
+
+ mk->mk_spec = *mk_spec;
+
+ move_master_key_secret(&mk->mk_secret, secret);
+
+ format_mk_description(description, mk_spec);
+ key = key_alloc(&key_type_fscrypt_mk, description,
+ GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
+ KEY_POS_SEARCH | KEY_USR_SEARCH |
+ KEY_USR_READ | KEY_USR_VIEW, 0, NULL);
+ if (IS_ERR(key))
+ goto out_free_mk;
+
+ err = key_instantiate_and_link(key, mk, sizeof(*mk), NULL, NULL);
+ if (err) {
+ key_put(key);
+ key = ERR_PTR(err);
+ goto out_free_mk;
+ }
+ return key;
+
+out_free_mk:
+ free_master_key(mk);
+ return key;
+}
+
+/*
+ * Add the given key to ->s_master_keys, creating ->s_master_keys if it doesn't
+ * already exist. Synchronized by fscrypt_add_key_mutex.
+ */
+static int add_to_filesystem_keyring(struct super_block *sb, struct key *key)
+{
+ struct key *keyring = sb->s_master_keys;
+
+ if (keyring == NULL) {
+ char description[FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE];
+
+ format_fs_keyring_description(description, sb);
+ keyring = keyring_alloc(description, GLOBAL_ROOT_UID,
+ GLOBAL_ROOT_GID, current_cred(),
+ KEY_POS_SEARCH | KEY_USR_SEARCH |
+ KEY_USR_READ | KEY_USR_VIEW,
+ KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
+ if (IS_ERR(keyring))
+ return PTR_ERR(keyring);
+
+ /* Pairs with smp_load_acquire() in find_master_key() */
+ smp_store_release(&sb->s_master_keys, keyring);
+ }
+
+ return key_link(keyring, key);
+}
+
+static int add_master_key(struct super_block *sb,
+ struct fscrypt_master_key_secret *secret,
+ const struct fscrypt_key_specifier *mk_spec)
+{
+ struct key *key;
+ int err;
+ static DEFINE_MUTEX(fscrypt_add_key_mutex);
+
+ mutex_lock(&fscrypt_add_key_mutex); /* serialize find + link */
+ key = find_master_key(sb, mk_spec);
+ if (IS_ERR(key)) {
+ if (key != ERR_PTR(-ENOKEY)) {
+ err = PTR_ERR(key);
+ goto out_unlock;
+ }
+ /* Didn't find the key in ->s_master_keys; add it. */
+
+ key = allocate_master_key(secret, mk_spec);
+ if (IS_ERR(key)) {
+ err = PTR_ERR(key);
+ goto out_unlock;
+ }
+ err = add_to_filesystem_keyring(sb, key);
+ if (err)
+ goto out_put_key;
+ }
+ err = 0;
+out_put_key:
+ key_put(key);
+out_unlock:
+ mutex_unlock(&fscrypt_add_key_mutex);
+ return err;
+}
+
+/*
+ * Add a master encryption key to the filesystem, causing all files which were
+ * encrypted with it to appear "unlocked" (decrypted) when accessed.
+ */
+int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
+{
+ struct super_block *sb = file_inode(filp)->i_sb;
+ struct fscrypt_add_key_args __user *uarg = _uarg;
+ struct fscrypt_add_key_args arg;
+ struct fscrypt_master_key_secret secret;
+ int err;
+
+ if (copy_from_user(&arg, uarg, sizeof(arg)))
+ return -EFAULT;
+
+ if (arg.raw_size < FSCRYPT_MIN_KEY_SIZE ||
+ arg.raw_size > FSCRYPT_MAX_KEY_SIZE)
+ return -EINVAL;
+
+ if (arg.reserved1 ||
+ memchr_inv(arg.reserved2, 0, sizeof(arg.reserved2)))
+ return -EINVAL;
+
+ if (!valid_key_spec(&arg.key_spec))
+ return -EINVAL;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ memset(&secret, 0, sizeof(secret));
+ secret.size = arg.raw_size;
+ err = -EFAULT;
+ if (copy_from_user(secret.raw, uarg->raw, secret.size))
+ goto out_wipe_secret;
+
+ err = add_master_key(sb, &secret, &arg.key_spec);
+out_wipe_secret:
+ wipe_master_key_secret(&secret);
+ return err;
+}
+EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);
+
static void derive_crypt_complete(struct crypto_async_request *req, int rc)
{
struct fscrypt_completion_result *ecr = req->data;
@@ -137,10 +430,10 @@ find_and_lock_process_key(const char *prefix,
return ERR_PTR(-ENOKEY);
}

-/* Find the master key, then derive the inode's actual encryption key */
-static int find_and_derive_key(const struct inode *inode,
- const struct fscrypt_context *ctx,
- u8 *derived_key, unsigned int derived_keysize)
+static int find_and_derive_key_legacy(const struct inode *inode,
+ const struct fscrypt_context *ctx,
+ u8 *derived_key,
+ unsigned int derived_keysize)
{
struct key *key;
const struct fscrypt_key *payload;
@@ -162,6 +455,54 @@ static int find_and_derive_key(const struct inode *inode,
return err;
}

+/* Find the master key, then derive the inode's actual encryption key */
+static int find_and_derive_key(const struct inode *inode,
+ const struct fscrypt_context *ctx,
+ u8 *derived_key, unsigned int derived_keysize)
+{
+ struct key *key;
+ struct fscrypt_master_key *mk;
+ struct fscrypt_key_specifier mk_spec;
+ int err;
+
+ mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR;
+ memcpy(mk_spec.descriptor, ctx->master_key_descriptor,
+ FSCRYPT_KEY_DESCRIPTOR_SIZE);
+
+ key = find_master_key(inode->i_sb, &mk_spec);
+ if (IS_ERR(key)) {
+ if (key != ERR_PTR(-ENOKEY))
+ return PTR_ERR(key);
+ /*
+ * As a legacy fallback, we search the current task's subscribed
+ * keyrings in addition to ->s_master_keys.
+ */
+ return find_and_derive_key_legacy(inode, ctx, derived_key,
+ derived_keysize);
+ }
+ mk = key->payload.data[0];
+
+ /*
+ * Require that the master key be at least as long as the derived key.
+ * Otherwise, the derived key cannot possibly contain as much entropy as
+ * that required by the encryption mode it will be used for.
+ */
+ if (mk->mk_secret.size < derived_keysize) {
+ pr_warn_ratelimited("fscrypt: key with description '%s' is too short "
+ "(got %u bytes, need %u+ bytes)\n",
+ key->description,
+ mk->mk_secret.size, derived_keysize);
+ err = -ENOKEY;
+ goto out_put_key;
+ }
+
+ err = derive_key_aes(mk->mk_secret.raw, ctx,
+ derived_key, derived_keysize);
+out_put_key:
+ key_put(key);
+ return err;
+}
+
static const struct {
const char *cipher_str;
int keysize;
diff --git a/include/linux/fscrypt_notsupp.h b/include/linux/fscrypt_notsupp.h
index c4c6bf2c390e..7ca8a44fc984 100644
--- a/include/linux/fscrypt_notsupp.h
+++ b/include/linux/fscrypt_notsupp.h
@@ -84,6 +84,11 @@ static inline int fscrypt_inherit_context(struct inode *parent,
}

/* keyinfo.c */
+static inline int fscrypt_ioctl_add_key(struct file *filp, void __user *arg)
+{
+ return -EOPNOTSUPP;
+}
+
static inline int fscrypt_get_encryption_info(struct inode *inode)
{
return -EOPNOTSUPP;
diff --git a/include/linux/fscrypt_supp.h b/include/linux/fscrypt_supp.h
index 2db5e9706f60..313943214d57 100644
--- a/include/linux/fscrypt_supp.h
+++ b/include/linux/fscrypt_supp.h
@@ -42,6 +42,7 @@ extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
extern int fscrypt_inherit_context(struct inode *, struct inode *,
void *, bool);
/* keyinfo.c */
+extern int fscrypt_ioctl_add_key(struct file *filp, void __user *arg);
extern int fscrypt_get_encryption_info(struct inode *);
extern void fscrypt_put_encryption_info(struct inode *, struct fscrypt_info *);

diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index 26c381a40279..aebe5d84d091 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -34,22 +34,43 @@ struct fscrypt_policy {
__u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
};

-#define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
-#define FS_IOC_GET_ENCRYPTION_PWSALT _IOW('f', 20, __u8[16])
-#define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy)
-
-/* Parameters for passing an encryption key into the kernel keyring */
+/*
+ * Process-subscribed "logon" key description prefix and payload format.
+ * Deprecated; prefer FS_IOC_ADD_ENCRYPTION_KEY instead.
+ */
#define FSCRYPT_KEY_DESC_PREFIX "fscrypt:"
-#define FSCRYPT_KEY_DESC_PREFIX_SIZE 8
-
-/* Structure that userspace passes to the kernel keyring */
-#define FSCRYPT_MAX_KEY_SIZE 64
-
+#define FSCRYPT_KEY_DESC_PREFIX_SIZE 8
+#define FSCRYPT_MAX_KEY_SIZE 64
struct fscrypt_key {
__u32 mode;
__u8 raw[FSCRYPT_MAX_KEY_SIZE];
__u32 size;
};
+
+struct fscrypt_key_specifier {
+ __u32 type;
+#define FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR 1
+ __u32 reserved;
+ union {
+ __u8 max_specifier[32];
+ __u8 descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+ };
+};
+
+/* Struct passed to FS_IOC_ADD_ENCRYPTION_KEY */
+struct fscrypt_add_key_args {
+ __u32 raw_size;
+ __u32 reserved1;
+ __u64 reserved2[2];
+ struct fscrypt_key_specifier key_spec;
+ __u8 raw[];
+};
+
+#define FS_IOC_SET_ENCRYPTION_POLICY _IOR( 'f', 19, struct fscrypt_policy)
+#define FS_IOC_GET_ENCRYPTION_PWSALT _IOW( 'f', 20, __u8[16])
+#define FS_IOC_GET_ENCRYPTION_POLICY _IOW( 'f', 21, struct fscrypt_policy)
+#define FS_IOC_ADD_ENCRYPTION_KEY _IOWR('f', 22, struct fscrypt_add_key_args)
+
/**********************************************************************/

/* old names; don't add anything new here! */
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:55

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 22/25] ext4 crypto: wire up FS_IOC_GET_ENCRYPTION_POLICY_EX

From: Eric Biggers <[email protected]>

FS_IOC_GET_ENCRYPTION_POLICY_EX allows filesystem encryption users to
retrieve the encryption policy for files and directories that use a v2
encryption policy. Unlike the original FS_IOC_GET_ENCRYPTION_POLICY,
FS_IOC_GET_ENCRYPTION_POLICY_EX is also extensible to new versions of
the policy struct that may be added in the future.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/ext4/ioctl.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index b8a6765a556f..954042f311dc 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -978,6 +978,9 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
case EXT4_IOC_GET_ENCRYPTION_POLICY:
return fscrypt_ioctl_get_policy(filp, (void __user *)arg);

+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+ return fscrypt_ioctl_get_policy_ex(filp, (void __user *)arg);
+
case FS_IOC_ADD_ENCRYPTION_KEY:
if (!ext4_has_feature_encrypt(sb))
return -EOPNOTSUPP;
@@ -1117,6 +1120,7 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case EXT4_IOC_SET_ENCRYPTION_POLICY:
case EXT4_IOC_GET_ENCRYPTION_PWSALT:
case EXT4_IOC_GET_ENCRYPTION_POLICY:
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
case FS_IOC_ADD_ENCRYPTION_KEY:
case FS_IOC_REMOVE_ENCRYPTION_KEY:
case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:42:26

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 12/25] ext4 crypto: wire up new ioctls for managing encryption keys

From: Eric Biggers <[email protected]>

Signed-off-by: Eric Biggers <[email protected]>
---
fs/ext4/ioctl.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index afb66d4ab5cf..b8a6765a556f 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -978,6 +978,21 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
case EXT4_IOC_GET_ENCRYPTION_POLICY:
return fscrypt_ioctl_get_policy(filp, (void __user *)arg);

+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ if (!ext4_has_feature_encrypt(sb))
+ return -EOPNOTSUPP;
+ return fscrypt_ioctl_add_key(filp, (void __user *)arg);
+
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ if (!ext4_has_feature_encrypt(sb))
+ return -EOPNOTSUPP;
+ return fscrypt_ioctl_remove_key(filp, (const void __user *)arg);
+
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
+ if (!ext4_has_feature_encrypt(sb))
+ return -EOPNOTSUPP;
+ return fscrypt_ioctl_get_key_status(filp, (void __user *)arg);
+
case EXT4_IOC_FSGETXATTR:
{
struct fsxattr fa;
@@ -1102,6 +1117,9 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case EXT4_IOC_SET_ENCRYPTION_POLICY:
case EXT4_IOC_GET_ENCRYPTION_PWSALT:
case EXT4_IOC_GET_ENCRYPTION_POLICY:
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
case EXT4_IOC_SHUTDOWN:
case FS_IOC_GETFSMAP:
break;
--
2.15.0.rc0.271.g36b669edcc-goog

2017-10-23 21:40:41

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 08/25] fs/inode.c: rename and export dispose_list()

From: Eric Biggers <[email protected]>

When a filesystem encryption key is removed, we need all files which had
been "unlocked" (had ->i_crypt_info set up) with it to appear "locked"
again. This is most easily done by evicting the inodes. This can
currently be done using 'echo 2 > /proc/sys/vm/drop_caches'; however,
that is overkill and not usable by non-root users. In preparation for
allowing fs/crypto/ to evict just the needed inodes, export
dispose_list() to modules. For clarity also rename it to
evict_inode_list().

Signed-off-by: Eric Biggers <[email protected]>
---
fs/inode.c | 19 ++++++++++---------
include/linux/fs.h | 1 +
2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 30ce98956801..fe47930835c0 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -570,13 +570,13 @@ static void evict(struct inode *inode)
}

/*
- * dispose_list - dispose of the contents of a local list
- * @head: the head of the list to free
+ * evict_inode_list - evict each inode in a local list of inodes
+ * @head: the head of the list
*
- * Dispose-list gets a local list with local inodes in it, so it doesn't
+ * This gets a local list with local inodes in it, so it doesn't
* need to worry about list corruption and SMP locks.
*/
-static void dispose_list(struct list_head *head)
+void evict_inode_list(struct list_head *head)
{
while (!list_empty(head)) {
struct inode *inode;
@@ -588,6 +588,7 @@ static void dispose_list(struct list_head *head)
cond_resched();
}
}
+EXPORT_SYMBOL_GPL(evict_inode_list);

/**
* evict_inodes - evict all evictable inodes for a superblock
@@ -628,13 +629,13 @@ void evict_inodes(struct super_block *sb)
if (need_resched()) {
spin_unlock(&sb->s_inode_list_lock);
cond_resched();
- dispose_list(&dispose);
+ evict_inode_list(&dispose);
goto again;
}
}
spin_unlock(&sb->s_inode_list_lock);

- dispose_list(&dispose);
+ evict_inode_list(&dispose);
}
EXPORT_SYMBOL_GPL(evict_inodes);

@@ -679,7 +680,7 @@ int invalidate_inodes(struct super_block *sb, bool kill_dirty)
}
spin_unlock(&sb->s_inode_list_lock);

- dispose_list(&dispose);
+ evict_inode_list(&dispose);

return busy;
}
@@ -763,7 +764,7 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
* Walk the superblock inode LRU for freeable inodes and attempt to free them.
* This is called from the superblock shrinker function with a number of inodes
* to trim from the LRU. Inodes to be freed are moved to a temporary list and
- * then are freed outside inode_lock by dispose_list().
+ * then are freed outside inode_lock by evict_inode_list().
*/
long prune_icache_sb(struct super_block *sb, struct shrink_control *sc)
{
@@ -772,7 +773,7 @@ long prune_icache_sb(struct super_block *sb, struct shrink_control *sc)

freed = list_lru_shrink_walk(&sb->s_inode_lru, sc,
inode_lru_isolate, &freeable);
- dispose_list(&freeable);
+ evict_inode_list(&freeable);
return freed;
}

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2833ace2f01d..e0a8dae5f9dc 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2880,6 +2880,7 @@ extern void unlock_new_inode(struct inode *);
extern unsigned int get_next_ino(void);
extern void evict_inodes(struct super_block *sb);
extern void inode_lru_list_del(struct inode *inode);
+extern void evict_inode_list(struct list_head *head);

extern void __iget(struct inode * inode);
extern void iget_failed(struct inode *);
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:54

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 21/25] fscrypt: require that key be added when setting a v2 encryption policy

From: Eric Biggers <[email protected]>

By looking up the master keys in a filesystem-level keyring rather than
in the calling processes' key hierarchy, it becomes possible for a user
to set an encryption policy which refers to some key they don't actually
know, then encrypt their files using that key. Cryptographically this
shouldn't actually be a major problem; for one, every file will still be
encrypted with a unique derived key, rather than with the master key
directly. But to be on the safe side, enforce that a v2 encryption
policy can only be set if the user has previously added the key, or has
capable(CAP_FOWNER).

We tolerate that this problem will continue to exist for v1 encryption
policies, however; there is no way around that.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/crypto/fscrypt_private.h | 2 ++
fs/crypto/keyinfo.c | 42 ++++++++++++++++++++++++++++++++++++++++++
fs/crypto/policy.c | 6 ++++++
3 files changed, 50 insertions(+)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index d0a63086fa95..7a0d5b6c2504 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -234,6 +234,8 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
gfp_t gfp_flags);

/* keyinfo.c */
+extern int fscrypt_verify_key_added(struct super_block *sb,
+ const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]);
extern struct key_type key_type_fscrypt_mk;
extern struct key_type key_type_fscrypt_mk_user;
extern void __exit fscrypt_essiv_cleanup(void);
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 1fe44983239a..fd59f37dad10 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -851,6 +851,48 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
}
EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);

+/*
+ * Verify that the current user has added a master key that has the given
+ * identifier (returns -ENOKEY if not). This is needed to prevent a user from
+ * encrypting their files using some other user's key which they don't actually
+ * know. Cryptographically speaking, it's debatable how much of a problem this
+ * actually would be, but it's best to just forbid it.
+ *
+ * The system administrator (CAP_FOWNER) can override this, which should be
+ * enough for any use cases where encryption policies are being set using keys
+ * that were chosen ahead of time but aren't available at the moment.
+ */
+int fscrypt_verify_key_added(struct super_block *sb,
+ const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+ struct fscrypt_key_specifier mk_spec;
+ struct key *key, *mk_user;
+ struct fscrypt_master_key *mk;
+ int err;
+
+ mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
+ memcpy(mk_spec.identifier, identifier, FSCRYPT_KEY_IDENTIFIER_SIZE);
+
+ key = find_master_key(sb, &mk_spec);
+ if (IS_ERR(key)) {
+ err = PTR_ERR(key);
+ goto out;
+ }
+ mk = key->payload.data[0];
+ mk_user = find_master_key_user(mk);
+ if (IS_ERR(mk_user)) {
+ err = PTR_ERR(mk_user);
+ } else {
+ key_put(mk_user);
+ err = 0;
+ }
+ key_put(key);
+out:
+ if (err == -ENOKEY && capable(CAP_FOWNER))
+ err = 0;
+ return err;
+}
+
static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
{
struct fscrypt_info *ci;
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 27a391038f73..cfb404def9ed 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -170,6 +170,7 @@ static int set_encryption_policy(struct inode *inode,
const union fscrypt_policy *policy)
{
union fscrypt_context ctx;
+ int err;

if (!fscrypt_supported_policy(policy))
return -EINVAL;
@@ -190,6 +191,11 @@ static int set_encryption_policy(struct inode *inode,
*/
pr_warn_once("%s (pid %d) is setting less secure v1 encryption policy; recommend upgrading to v2.\n",
current->comm, current->pid);
+ } else {
+ err = fscrypt_verify_key_added(inode->i_sb,
+ policy->v2.master_key_identifier);
+ if (err)
+ return err;
}

return inode->i_sb->s_cop->set_context(inode, &ctx,
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:43

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 10/25] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl

From: Eric Biggers <[email protected]>

Problem
~~~~~~~

Many filesystem encryption users want the ability to remove encryption
keys, causing the corresponding encrypted directories to appear "locked"
(presented in ciphertext form) again. Moreover, users want removing an
encryption key to *really* remove it, in the sense that the removed keys
cannot be recovered even if kernel memory is compromised, e.g. by the
exploit of a kernel security vulnerability or by a physical attack.
This is desirable after a user logs out of the system, for example. In
many cases users even already assume this to be the case and are
surprised to hear when it's not.

It is *not* sufficient to simply unlink the master key from the keyring
(or to revoke or invalidate it), since files are encrypted with per-file
keys instead of with the master keys directly. Therefore, to really
remove a key we must also remove the per-file keys, e.g. by evicting the
corresponding inodes from the inode cache. This also would have the
benefit of making encrypted files appear "locked" again.

Currently the workaround is to run:

sync
echo 2 > /proc/sys/vm/drop_caches

This is a very bad solution because it evicts all not-in-use inodes in
the system rather than just the inodes associated with the key being
removed. Moreover, it requires root privileges, so non-root users
cannot lock their encrypted directories. Finally, the drop_caches
sysctl was originally meant for debugging purposes only.

Nevertheless, the largest users of filesystem encryption (Android and
Chromium OS) actually want this capability badly enough that they are
actually using the drop_caches workaround. Similarly, the drop_caches
workaround is also used in the PAM module provided by the fscrypt
userspace tool (https://github.com/google/fscrypt). Needless to say,
this is causing significant performance problems due to inodes for
unencrypted system files being evicted. So a real solution is needed.

Solution
~~~~~~~~

To properly solve this problem, we need an API which removes and wipes
the given master key, *and* removes and and wipes the corresponding
per-file keys. This requires tracking which inodes have been "unlocked"
using each master key. Originally that was not possible because the
kernel didn't actually have a centralized notion of what a master key
even was. But now that we have the filesystem-level keyring
->s_master_keys it is finally possible.

Add this API as a new ioctl, FS_IOC_REMOVE_ENCRYPTION_KEY. It is the
counterpart of FS_IOC_ADD_ENCRYPTION_KEY.

FS_IOC_REMOVE_ENCRYPTION_KEY first wipes the master key's secret from
memory. Then, it syncs the filesystem and tries to evict the list of
inodes that had been "unlocked" with the key. Evicting the inodes has
several effects, including:

- The actual keys used to encrypt the data (in ->i_crypt_info->ci_ctfm)
are wiped from memory. Thus, they can no longer be recovered, even if
kernel memory is later compromised.

- The encrypted files and directories once again appear "locked", i.e.
in ciphertext or in "encrypted" form. This is highly desirable from a
user interface perspective. It can also be desirable from a security
perspective (although sometimes for the wrong reasons!).

- The pagecache pages are freed, which allows the plaintext file
contents to be overwritten in memory later as the system continues
running. Currently we do not actually wipe the pages on free, nor
does the kernel more generally wipe memory on free either. Thus, for
now we tolerate that an attacker who later gains access to kernel
memory may be able to see portions of file contents and file names in
plaintext in unallocated memory. Security-conscious users who do not
mind a performance hit may ameliorate this by enabling page poisoning.

Of course, some inodes may still be in use when a master key is removed,
and we cannot simply revoke random file descriptors, mmap's, etc. The
approach we take is to skip in-use inodes, and notify userspace by
returning -EBUSY if any inodes could not be evicted. Still, even in
this case the master key secret is removed, so no more files can be
unlocked with it. Moreover, most of the inodes should still be evicted
as well. Userspace can then retry the ioctl later to evict the
remaining inodes. Alternatively, if userspace adds the key again, then
the refreshed secret will be associated with the existing list of inodes
so that they are correctly tracked for future key removals.

For now, FS_IOC_REMOVE_ENCRYPTION_KEY has to be restricted to privileged
users only, just like FS_IOC_ADD_ENCRYPTION_KEY. This is sufficient for
use cases where all encryption keys are managed by a privileged process,
e.g. as is the case on Android and Chromium OS.

But in the more general case, non-root users need to be able to both
unlock *and* lock their own encrypted directories. As it turns out, we
will indeed be able to support this through these ioctls, but non-root
use will need to be tied to the use of a new encryption policy version
which identifies the master key using a cryptographic hash. (See later
patches.)

Signed-off-by: Eric Biggers <[email protected]>
---
fs/crypto/fscrypt_private.h | 18 ++-
fs/crypto/keyinfo.c | 347 ++++++++++++++++++++++++++++++++++++++--
fs/crypto/policy.c | 5 +-
include/linux/fscrypt_notsupp.h | 6 +
include/linux/fscrypt_supp.h | 1 +
include/uapi/linux/fscrypt.h | 7 +
6 files changed, 367 insertions(+), 17 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index b2fad12eeedb..2fdc4e5c0771 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -61,7 +61,23 @@ struct fscrypt_info {
u8 ci_flags;
struct crypto_skcipher *ci_ctfm;
struct crypto_cipher *ci_essiv_tfm;
- u8 ci_master_key[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+ u8 ci_master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+
+ /*
+ * The master key with which this inode was unlocked (decrypted). This
+ * will be NULL if the master key was found in a process-subscribed
+ * keyring rather than in the filesystem-level keyring.
+ */
+ struct key *ci_master_key;
+
+ /* Link in list of inodes that were unlocked with the master key */
+ struct list_head ci_master_key_link;
+
+ /*
+ * Back-pointer to the inode, needed during key removal. Only set when
+ * ->ci_master_key is set.
+ */
+ struct inode *ci_inode;
};

typedef enum {
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 3f1cb8bbc1e5..dc2697cf9114 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -40,11 +40,35 @@ struct fscrypt_master_key_secret {
*/
struct fscrypt_master_key {

- /* The secret key material */
+ /*
+ * The secret key material. After FS_IOC_REMOVE_ENCRYPTION_KEY is
+ * executed, this is wiped and no new inodes can be unlocked with this
+ * key; however, there may still be inodes in ->mk_decrypted_inodes
+ * which could not be evicted. As long as some inodes still remain,
+ * FS_IOC_REMOVE_ENCRYPTION_KEY can be retried, or
+ * FS_IOC_ADD_ENCRYPTION_KEY can add the secret again.
+ *
+ * Locking: protected by key->sem.
+ */
struct fscrypt_master_key_secret mk_secret;

/* Arbitrary key descriptor which was assigned by userspace */
struct fscrypt_key_specifier mk_spec;
+
+ /*
+ * Length of ->mk_decrypted_inodes, plus one if mk_secret is present.
+ * Once this goes to 0, the master key is removed from ->s_master_keys.
+ * This struct will continue to live as long as the 'struct key' whose
+ * payload it is, but we won't let this reference count rise again.
+ */
+ refcount_t mk_refcount;
+
+ /*
+ * List of inodes that were unlocked using this key. This allows the
+ * inodes to be evicted efficiently if the key is removed.
+ */
+ struct list_head mk_decrypted_inodes;
+ spinlock_t mk_decrypted_inodes_lock;
};

static inline int master_key_spec_len(const struct fscrypt_key_specifier *spec)
@@ -63,6 +87,12 @@ static inline bool valid_key_spec(const struct fscrypt_key_specifier *spec)
return master_key_spec_len(spec) != 0;
}

+static bool
+is_master_key_secret_present(const struct fscrypt_master_key_secret *secret)
+{
+ return secret->size != 0;
+}
+
static void wipe_master_key_secret(struct fscrypt_master_key_secret *secret)
{
memzero_explicit(secret, sizeof(*secret));
@@ -96,6 +126,13 @@ static void fscrypt_key_destroy(struct key *key)
static void fscrypt_key_describe(const struct key *key, struct seq_file *m)
{
seq_puts(m, key->description);
+
+ if (key_is_positive(key)) {
+ struct fscrypt_master_key *mk = key->payload.data[0];
+
+ if (!is_master_key_secret_present(&mk->mk_secret))
+ seq_puts(m, ": secret removed");
+ }
}

/*
@@ -122,7 +159,8 @@ static struct key *search_fscrypt_keyring(struct key *keyring,

keyref = keyring_search(make_key_ref(keyring, 1), type, description);
if (IS_ERR(keyref)) {
- if (PTR_ERR(keyref) == -EAGAIN)
+ if (PTR_ERR(keyref) == -EAGAIN ||
+ PTR_ERR(keyref) == -EKEYREVOKED)
keyref = ERR_PTR(-ENOKEY);
return ERR_CAST(keyref);
}
@@ -186,6 +224,10 @@ allocate_master_key(struct fscrypt_master_key_secret *secret,

move_master_key_secret(&mk->mk_secret, secret);

+ refcount_set(&mk->mk_refcount, 1); /* secret is present */
+ INIT_LIST_HEAD(&mk->mk_decrypted_inodes);
+ spin_lock_init(&mk->mk_decrypted_inodes_lock);
+
format_mk_description(description, mk_spec);
key = key_alloc(&key_type_fscrypt_mk, description,
GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
@@ -243,6 +285,7 @@ static int add_master_key(struct super_block *sb,
static DEFINE_MUTEX(fscrypt_add_key_mutex);

mutex_lock(&fscrypt_add_key_mutex); /* serialize find + link */
+retry:
key = find_master_key(sb, mk_spec);
if (IS_ERR(key)) {
if (key != ERR_PTR(-ENOKEY)) {
@@ -259,6 +302,33 @@ static int add_master_key(struct super_block *sb,
err = add_to_filesystem_keyring(sb, key);
if (err)
goto out_put_key;
+ } else {
+ struct fscrypt_master_key *mk = key->payload.data[0];
+ bool rekey;
+
+ /* Found the key in ->s_master_keys */
+
+ down_write(&key->sem);
+
+ /*
+ * Take a reference if we'll be re-adding ->mk_secret. If we
+ * couldn't take a reference, then the key is being removed from
+ * ->s_master_keys and can no longer be used. So invalidate the
+ * key (someone else is doing that too, but they might be
+ * slower) and retry searching ->s_master_keys.
+ */
+ rekey = !is_master_key_secret_present(&mk->mk_secret);
+ if (rekey && !refcount_inc_not_zero(&mk->mk_refcount)) {
+ up_write(&key->sem);
+ key_invalidate(key);
+ key_put(key);
+ goto retry;
+ }
+
+ /* Re-add the secret key material if needed */
+ if (rekey)
+ move_master_key_secret(&mk->mk_secret, secret);
+ up_write(&key->sem);
}
err = 0;
out_put_key:
@@ -270,7 +340,8 @@ static int add_master_key(struct super_block *sb,

/*
* Add a master encryption key to the filesystem, causing all files which were
- * encrypted with it to appear "unlocked" (decrypted) when accessed.
+ * encrypted with it to appear "unlocked" (decrypted) when accessed. The key
+ * can be removed later by FS_IOC_REMOVE_ENCRYPTION_KEY.
*/
int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
{
@@ -310,6 +381,191 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
}
EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);

+static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
+{
+ struct fscrypt_info *ci;
+ struct inode *inode;
+ struct inode *toput_inode = NULL;
+
+ spin_lock(&mk->mk_decrypted_inodes_lock);
+
+ list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
+ inode = ci->ci_inode;
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
+ spin_unlock(&inode->i_lock);
+ continue;
+ }
+ __iget(inode);
+ spin_unlock(&inode->i_lock);
+ spin_unlock(&mk->mk_decrypted_inodes_lock);
+
+ shrink_dcache_inode(inode);
+ iput(toput_inode);
+ toput_inode = inode;
+
+ spin_lock(&mk->mk_decrypted_inodes_lock);
+ }
+
+ spin_unlock(&mk->mk_decrypted_inodes_lock);
+ iput(toput_inode);
+}
+
+static int evict_decrypted_inodes(struct fscrypt_master_key *mk)
+{
+ struct fscrypt_info *ci;
+ struct inode *inode;
+ LIST_HEAD(dispose);
+ unsigned long num_busy = 0;
+ unsigned long busy_ino;
+
+ spin_lock(&mk->mk_decrypted_inodes_lock);
+
+ list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
+ inode = ci->ci_inode;
+ spin_lock(&inode->i_lock);
+
+ if (inode->i_state & (I_FREEING | I_WILL_FREE))
+ goto next;
+
+ if (atomic_read(&inode->i_count) ||
+ (inode->i_state & ~I_REFERENCED)) {
+ num_busy++;
+ busy_ino = inode->i_ino;
+ goto next;
+ }
+
+ inode->i_state |= I_FREEING;
+ inode_lru_list_del(inode);
+ list_add(&inode->i_lru, &dispose);
+next:
+ spin_unlock(&inode->i_lock);
+ }
+
+ spin_unlock(&mk->mk_decrypted_inodes_lock);
+
+ evict_inode_list(&dispose);
+
+ if (unlikely(num_busy)) {
+ pr_warn_ratelimited("fscrypt: %lu inodes still busy after removing key with description %*phN (%sino: %lu)\n",
+ num_busy, master_key_spec_len(&mk->mk_spec),
+ mk->mk_spec.max_specifier,
+ (num_busy > 1 ? "example " : ""), busy_ino);
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+static int try_to_lock_encrypted_files(struct super_block *sb,
+ struct fscrypt_master_key *mk)
+{
+ int err1;
+ int err2;
+
+ /*
+ * An inode can't be evicted while it still has dirty pages, or while
+ * the inode itself is still dirty. Thus, we first have to clean all
+ * the inodes in ->mk_decrypted_inodes.
+ *
+ * Just do it the easy way: call sync_filesystem(). It's overkill, but
+ * it works, and it's more important to minimize the amount of caches we
+ * drop than the amount of data we sync. Also, unprivileged users can
+ * already call sync_filesystem() via sys_syncfs() or sys_sync().
+ */
+ down_read(&sb->s_umount);
+ err1 = sync_filesystem(sb);
+ up_read(&sb->s_umount);
+
+ /*
+ * Inodes are pinned by their dentries, so we have to evict the dentries
+ * first. We could potentially just call shrink_dcache_sb() here, but
+ * that would be overkill, and an unprivileged user shouldn't be able to
+ * evict all dentries for the entire filesystem. Instead, go through
+ * the inodes' alias lists and try to evict each dentry.
+ */
+ evict_dentries_for_decrypted_inodes(mk);
+
+ /*
+ * Finally, iterate through ->mk_decrypted_inodes and evict as many
+ * inodes as we can. Similarly, we could potentially just call
+ * invalidate_inodes() here, but that would be overkill, and an
+ * unprivileged user shouldn't be able to evict all inodes for the
+ * entire filesystem.
+ *
+ * Note that ideally, we wouldn't really evict the inodes, but rather
+ * just free their ->i_crypt_info and pagecache. But eviction is *much*
+ * easier to correctly implement without causing use-after-free bugs.
+ */
+ err2 = evict_decrypted_inodes(mk);
+
+ return err1 ?: err2;
+}
+
+/*
+ * Try to remove an fscrypt master encryption key.
+ *
+ * First we wipe the actual master key secret from memory, so that no more
+ * inodes can be unlocked with it. Then, we try to evict all cached inodes that
+ * had been unlocked using the key. Since this can fail for in-use inodes, this
+ * is expected to be used in cooperation with userspace ensuring that none of
+ * the files are still open.
+ *
+ * If, nevertheless, some inodes could not be evicted, we return -EBUSY
+ * (although we still evicted as many inodes as possible) and keep the 'struct
+ * key' and the 'struct fscrypt_master_key' around to keep track of the list of
+ * remaining inodes. Userspace can then retry the ioctl later to retry evicting
+ * the remaining inodes, or alternatively can add the secret key again.
+ *
+ * Note that even though we wipe the encryption *keys* from memory, decrypted
+ * data can likely still be found in memory, e.g. in pagecache pages that have
+ * been freed. Wiping such data is currently out of scope, short of users who
+ * may choose to enable page and slab poisoning systemwide.
+ */
+int fscrypt_ioctl_remove_key(struct file *filp, const void __user *uarg)
+{
+ struct super_block *sb = file_inode(filp)->i_sb;
+ struct fscrypt_remove_key_args arg;
+ struct key *key;
+ struct fscrypt_master_key *mk;
+ int err;
+ bool dead;
+
+ if (copy_from_user(&arg, uarg, sizeof(arg)))
+ return -EFAULT;
+
+ if (memchr_inv(arg.reserved, 0, sizeof(arg.reserved)))
+ return -EINVAL;
+
+ if (!valid_key_spec(&arg.key_spec))
+ return -EINVAL;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ key = find_master_key(sb, &arg.key_spec);
+ if (IS_ERR(key))
+ return PTR_ERR(key);
+ mk = key->payload.data[0];
+
+ down_write(&key->sem);
+ dead = false;
+ if (is_master_key_secret_present(&mk->mk_secret)) {
+ wipe_master_key_secret(&mk->mk_secret);
+ dead = refcount_dec_and_test(&mk->mk_refcount);
+ }
+ up_write(&key->sem);
+ if (dead) {
+ key_invalidate(key);
+ err = 0;
+ } else {
+ err = try_to_lock_encrypted_files(sb, mk);
+ }
+ key_put(key);
+ return err;
+}
+EXPORT_SYMBOL_GPL(fscrypt_ioctl_remove_key);
+
static void derive_crypt_complete(struct crypto_async_request *req, int rc)
{
struct fscrypt_completion_result *ecr = req->data;
@@ -455,10 +711,20 @@ static int find_and_derive_key_legacy(const struct inode *inode,
return err;
}

-/* Find the master key, then derive the inode's actual encryption key */
+/*
+ * Find the master key, then derive the inode's actual encryption key.
+ *
+ * If the master key is found in the filesystem-level keyring, then the
+ * corresponding 'struct key' is returned read-locked in *master_key_ret. This
+ * is needed because we need to hold the semaphore until we link the new
+ * fscrypt_info into ->mk_decrypted_inodes, but in the case where multiple tasks
+ * are racing to set up an inode's ->i_crypt_info, only the winner should link
+ * its fscrypt_info into ->mk_decrypted_inodes.
+ */
static int find_and_derive_key(const struct inode *inode,
const struct fscrypt_context *ctx,
- u8 *derived_key, unsigned int derived_keysize)
+ u8 *derived_key, unsigned int derived_keysize,
+ struct key **master_key_ret)
{
struct key *key;
struct fscrypt_master_key *mk;
@@ -481,6 +747,13 @@ static int find_and_derive_key(const struct inode *inode,
derived_keysize);
}
mk = key->payload.data[0];
+ down_read(&key->sem);
+
+ /* Has the secret been removed using FS_IOC_REMOVE_ENCRYPTION_KEY? */
+ if (!is_master_key_secret_present(&mk->mk_secret)) {
+ err = -ENOKEY;
+ goto out_release_key;
+ }

/*
* Require that the master key be at least as long as the derived key.
@@ -493,12 +766,19 @@ static int find_and_derive_key(const struct inode *inode,
key->description,
mk->mk_secret.size, derived_keysize);
err = -ENOKEY;
- goto out_put_key;
+ goto out_release_key;
}

err = derive_key_aes(mk->mk_secret.raw, ctx,
derived_key, derived_keysize);
-out_put_key:
+ if (err)
+ goto out_release_key;
+
+ *master_key_ret = key;
+ return 0;
+
+out_release_key:
+ up_read(&key->sem);
key_put(key);
return err;
}
@@ -542,11 +822,32 @@ static int determine_cipher_type(struct fscrypt_info *ci, struct inode *inode,

static void put_crypt_info(struct fscrypt_info *ci)
{
+ struct key *key;
+
if (!ci)
return;

crypto_free_skcipher(ci->ci_ctfm);
crypto_free_cipher(ci->ci_essiv_tfm);
+ key = ci->ci_master_key;
+ if (key) {
+ struct fscrypt_master_key *mk = key->payload.data[0];
+
+ /*
+ * Remove this inode from the list of inodes that were unlocked
+ * with the master key.
+ *
+ * In addition, if we're removing the last inode from a key that
+ * already had its secret removed, invalidate the key so that it
+ * gets removed from ->s_master_keys.
+ */
+ spin_lock(&mk->mk_decrypted_inodes_lock);
+ list_del(&ci->ci_master_key_link);
+ spin_unlock(&mk->mk_decrypted_inodes_lock);
+ if (refcount_dec_and_test(&mk->mk_refcount))
+ key_invalidate(key);
+ key_put(key);
+ }
kmem_cache_free(fscrypt_info_cachep, ci);
}

@@ -624,6 +925,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
const char *cipher_str;
unsigned int derived_keysize;
u8 *derived_key = NULL;
+ struct key *master_key = NULL;
int res;

if (inode->i_crypt_info)
@@ -655,17 +957,15 @@ int fscrypt_get_encryption_info(struct inode *inode)
if (ctx.flags & ~FSCRYPT_POLICY_FLAGS_VALID)
return -EINVAL;

- crypt_info = kmem_cache_alloc(fscrypt_info_cachep, GFP_NOFS);
+ crypt_info = kmem_cache_zalloc(fscrypt_info_cachep, GFP_NOFS);
if (!crypt_info)
return -ENOMEM;

crypt_info->ci_flags = ctx.flags;
crypt_info->ci_data_mode = ctx.contents_encryption_mode;
crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
- crypt_info->ci_ctfm = NULL;
- crypt_info->ci_essiv_tfm = NULL;
- memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
- sizeof(crypt_info->ci_master_key));
+ memcpy(crypt_info->ci_master_key_descriptor, ctx.master_key_descriptor,
+ FSCRYPT_KEY_DESCRIPTOR_SIZE);

res = determine_cipher_type(crypt_info, inode,
&cipher_str, &derived_keysize);
@@ -681,7 +981,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
if (!derived_key)
goto out;

- res = find_and_derive_key(inode, &ctx, derived_key, derived_keysize);
+ res = find_and_derive_key(inode, &ctx, derived_key, derived_keysize,
+ &master_key);
if (res)
goto out;

@@ -709,9 +1010,27 @@ int fscrypt_get_encryption_info(struct inode *inode)
goto out;
}
}
- if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) == NULL)
+
+ if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) == NULL) {
+ if (master_key) {
+ struct fscrypt_master_key *mk =
+ master_key->payload.data[0];
+
+ crypt_info->ci_inode = inode;
+ refcount_inc(&mk->mk_refcount);
+ crypt_info->ci_master_key = key_get(master_key);
+ spin_lock(&mk->mk_decrypted_inodes_lock);
+ list_add(&crypt_info->ci_master_key_link,
+ &mk->mk_decrypted_inodes);
+ spin_unlock(&mk->mk_decrypted_inodes_lock);
+ }
crypt_info = NULL;
+ }
out:
+ if (master_key) {
+ up_read(&master_key->sem);
+ key_put(master_key);
+ }
if (res == -ENOKEY)
res = 0;
put_crypt_info(crypt_info);
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 19332a6fd52d..a856c8941be6 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -198,7 +198,8 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
child_ci = child->i_crypt_info;

if (parent_ci && child_ci) {
- return memcmp(parent_ci->ci_master_key, child_ci->ci_master_key,
+ return memcmp(parent_ci->ci_master_key_descriptor,
+ child_ci->ci_master_key_descriptor,
FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
(parent_ci->ci_data_mode == child_ci->ci_data_mode) &&
(parent_ci->ci_filename_mode ==
@@ -253,7 +254,7 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
ctx.contents_encryption_mode = ci->ci_data_mode;
ctx.filenames_encryption_mode = ci->ci_filename_mode;
ctx.flags = ci->ci_flags;
- memcpy(ctx.master_key_descriptor, ci->ci_master_key,
+ memcpy(ctx.master_key_descriptor, ci->ci_master_key_descriptor,
FSCRYPT_KEY_DESCRIPTOR_SIZE);
get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
BUILD_BUG_ON(sizeof(ctx) != FSCRYPT_SET_CONTEXT_MAX_SIZE);
diff --git a/include/linux/fscrypt_notsupp.h b/include/linux/fscrypt_notsupp.h
index 7ca8a44fc984..92616bfdc294 100644
--- a/include/linux/fscrypt_notsupp.h
+++ b/include/linux/fscrypt_notsupp.h
@@ -89,6 +89,12 @@ static inline int fscrypt_ioctl_add_key(struct file *filp, void __user *arg)
return -EOPNOTSUPP;
}

+static inline int fscrypt_ioctl_remove_key(struct file *filp,
+ const void __user *arg)
+{
+ return -EOPNOTSUPP;
+}
+
static inline int fscrypt_get_encryption_info(struct inode *inode)
{
return -EOPNOTSUPP;
diff --git a/include/linux/fscrypt_supp.h b/include/linux/fscrypt_supp.h
index 313943214d57..620ca4f1bafe 100644
--- a/include/linux/fscrypt_supp.h
+++ b/include/linux/fscrypt_supp.h
@@ -43,6 +43,7 @@ extern int fscrypt_inherit_context(struct inode *, struct inode *,
void *, bool);
/* keyinfo.c */
extern int fscrypt_ioctl_add_key(struct file *filp, void __user *arg);
+extern int fscrypt_ioctl_remove_key(struct file *filp, const void __user *arg);
extern int fscrypt_get_encryption_info(struct inode *);
extern void fscrypt_put_encryption_info(struct inode *, struct fscrypt_info *);

diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index aebe5d84d091..5d02f138668c 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -66,10 +66,17 @@ struct fscrypt_add_key_args {
__u8 raw[];
};

+/* Struct passed to FS_IOC_REMOVE_ENCRYPTION_KEY */
+struct fscrypt_remove_key_args {
+ __u64 reserved[3];
+ struct fscrypt_key_specifier key_spec;
+};
+
#define FS_IOC_SET_ENCRYPTION_POLICY _IOR( 'f', 19, struct fscrypt_policy)
#define FS_IOC_GET_ENCRYPTION_PWSALT _IOW( 'f', 20, __u8[16])
#define FS_IOC_GET_ENCRYPTION_POLICY _IOW( 'f', 21, struct fscrypt_policy)
#define FS_IOC_ADD_ENCRYPTION_KEY _IOWR('f', 22, struct fscrypt_add_key_args)
+#define FS_IOC_REMOVE_ENCRYPTION_KEY _IOR( 'f', 23, struct fscrypt_remove_key_args)

/**********************************************************************/

--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:56

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 23/25] f2fs crypto: wire up FS_IOC_GET_ENCRYPTION_POLICY_EX

From: Eric Biggers <[email protected]>

FS_IOC_GET_ENCRYPTION_POLICY_EX allows filesystem encryption users to
retrieve the encryption policy for files and directories that use a v2
encryption policy. Unlike the original FS_IOC_GET_ENCRYPTION_POLICY,
FS_IOC_GET_ENCRYPTION_POLICY_EX is also extensible to new versions of
the policy struct that may be added in the future.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/f2fs/file.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 0296a9594fe7..68b6ba732f25 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1887,11 +1887,6 @@ static int f2fs_ioc_set_encryption_policy(struct file *filp, unsigned long arg)
return fscrypt_ioctl_set_policy(filp, (const void __user *)arg);
}

-static int f2fs_ioc_get_encryption_policy(struct file *filp, unsigned long arg)
-{
- return fscrypt_ioctl_get_policy(filp, (void __user *)arg);
-}
-
static int f2fs_ioc_get_encryption_pwsalt(struct file *filp, unsigned long arg)
{
struct inode *inode = file_inode(filp);
@@ -2647,10 +2642,12 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
return f2fs_ioc_fitrim(filp, arg);
case F2FS_IOC_SET_ENCRYPTION_POLICY:
return f2fs_ioc_set_encryption_policy(filp, arg);
- case F2FS_IOC_GET_ENCRYPTION_POLICY:
- return f2fs_ioc_get_encryption_policy(filp, arg);
case F2FS_IOC_GET_ENCRYPTION_PWSALT:
return f2fs_ioc_get_encryption_pwsalt(filp, arg);
+ case F2FS_IOC_GET_ENCRYPTION_POLICY:
+ return fscrypt_ioctl_get_policy(filp, (void __user *)arg);
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+ return fscrypt_ioctl_get_policy_ex(filp, (void __user *)arg);
case FS_IOC_ADD_ENCRYPTION_KEY:
return fscrypt_ioctl_add_key(filp, (void __user *)arg);
case FS_IOC_REMOVE_ENCRYPTION_KEY:
@@ -2737,6 +2734,7 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case F2FS_IOC_SET_ENCRYPTION_POLICY:
case F2FS_IOC_GET_ENCRYPTION_PWSALT:
case F2FS_IOC_GET_ENCRYPTION_POLICY:
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
case FS_IOC_ADD_ENCRYPTION_KEY:
case FS_IOC_REMOVE_ENCRYPTION_KEY:
case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
--
2.15.0.rc0.271.g36b669edcc-goog

2017-10-23 21:42:42

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 24/25] ubifs crypto: wire up FS_IOC_GET_ENCRYPTION_POLICY_EX

From: Eric Biggers <[email protected]>

FS_IOC_GET_ENCRYPTION_POLICY_EX allows filesystem encryption users to
retrieve the encryption policy for files and directories that use a v2
encryption policy. Unlike the original FS_IOC_GET_ENCRYPTION_POLICY,
FS_IOC_GET_ENCRYPTION_POLICY_EX is also extensible to new versions of
the policy struct that may be added in the future.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/ubifs/ioctl.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c
index 09ede2d1898f..2064c01ac864 100644
--- a/fs/ubifs/ioctl.c
+++ b/fs/ubifs/ioctl.c
@@ -197,13 +197,12 @@ long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
return -EOPNOTSUPP;
#endif
}
- case FS_IOC_GET_ENCRYPTION_POLICY: {
-#ifdef CONFIG_UBIFS_FS_ENCRYPTION
+
+ case FS_IOC_GET_ENCRYPTION_POLICY:
return fscrypt_ioctl_get_policy(file, (void __user *)arg);
-#else
- return -EOPNOTSUPP;
-#endif
- }
+
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+ return fscrypt_ioctl_get_policy_ex(file, (void __user *)arg);

case FS_IOC_ADD_ENCRYPTION_KEY:
return fscrypt_ioctl_add_key(file, (void __user *)arg);
@@ -231,6 +230,7 @@ long ubifs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
break;
case FS_IOC_SET_ENCRYPTION_POLICY:
case FS_IOC_GET_ENCRYPTION_POLICY:
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
case FS_IOC_ADD_ENCRYPTION_KEY:
case FS_IOC_REMOVE_ENCRYPTION_KEY:
case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
--
2.15.0.rc0.271.g36b669edcc-goog

2017-10-23 21:40:48

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 15/25] fscrypt: add UAPI definitions to get/set v2 encryption policies

From: Eric Biggers <[email protected]>

Currently, fscrypt policies and xattrs identify the master key by
master_key_descriptor, which is an arbitrary 8-byte value used to form
the description of the keyring key. However, there is no verification
that the key descriptor in any way corresponds with the key payload.
Since ->i_crypt_info by necessity gets set up on a "first come, first
serve" basis, this flaw allows a process with only read-only access to
an encrypted file or directory to provide the wrong key, causing the
file contents or directory listing to be corrupted. This is a bug with
security implications which must be fixed.

To fix this bug without simply locking down adding keys to root, we must
replace master_key_descriptor with a cryptographic hash of the key. We
name the replacement master_key_identifier and make it 16 bytes long,
which should provide enough collision resistance, and more importantly
preimage resistance, without bloating the size of the encryption xattr
too much.

This will be both an on-disk format and API change, since we'll need to
define new versions of both the fscrypt_context and fscrypt_policy.
This patch begins this process by defining the UAPI changes to manage v2
policies. (Note: we jump to version 2 even though the previous policy
version number was 0 because the fscrypt_context was actually already
using version 1, not version 0. It would be really confusing to have
them always be 1 off from each other.)

The existing FS_IOC_SET_ENCRYPTION_POLICY will be used to set a v2
policy, as the kernel will be able to examine the 'version' field.

However, a new ioctl FS_IOC_GET_ENCRYPTION_POLICY_EX is needed to get a
v2 policy, since the returned struct needs to be larger. This ioctl
includes a size field as input, so that it can be used for both v1 and
v2 policies as well as any new policy versions that may get added in the
future.

Signed-off-by: Eric Biggers <[email protected]>
---
include/uapi/linux/fscrypt.h | 37 +++++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index 9da153df238a..065060b20a34 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -7,7 +7,6 @@
* File system encryption support
*/
/* Policy provided via an ioctl on the topmost directory */
-#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8

/* Encryption policy flags */
#define FSCRYPT_POLICY_FLAGS_PAD_4 0x00
@@ -26,13 +25,22 @@
#define FSCRYPT_MODE_AES_128_CBC 5
#define FSCRYPT_MODE_AES_128_CTS 6

-struct fscrypt_policy {
+/*
+ * Legacy policy version; no key verification (potentially insecure).
+ * For new encrypted directories, use fscrypt_policy_v2 instead.
+ *
+ * Careful: the .version field for this is actually 0, not 1.
+ */
+#define FSCRYPT_POLICY_VERSION_LEGACY 0
+#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
+struct fscrypt_policy_v1 {
__u8 version;
__u8 contents_encryption_mode;
__u8 filenames_encryption_mode;
__u8 flags;
__u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
};
+#define fscrypt_policy fscrypt_policy_v1

/*
* Process-subscribed "logon" key description prefix and payload format.
@@ -47,6 +55,30 @@ struct fscrypt_key {
__u32 size;
};

+/*
+ * New policy version with HKDF and key verification (recommended).
+ */
+#define FSCRYPT_POLICY_VERSION_2 2
+#define FSCRYPT_KEY_IDENTIFIER_SIZE 16
+struct fscrypt_policy_v2 {
+ __u8 version;
+ __u8 contents_encryption_mode;
+ __u8 filenames_encryption_mode;
+ __u8 flags;
+ __u8 reserved[4];
+ __u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+};
+
+/* Struct passed to FS_IOC_GET_ENCRYPTION_POLICY_EX */
+struct fscrypt_get_policy_ex_args {
+ __u64 size; /* input/output */
+ union {
+ __u8 version;
+ struct fscrypt_policy_v1 v1;
+ struct fscrypt_policy_v2 v2;
+ } policy; /* output */
+};
+
struct fscrypt_key_specifier {
__u32 type;
#define FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR 1
@@ -91,6 +123,7 @@ struct fscrypt_get_key_status_args {
#define FS_IOC_SET_ENCRYPTION_POLICY _IOR( 'f', 19, struct fscrypt_policy)
#define FS_IOC_GET_ENCRYPTION_PWSALT _IOW( 'f', 20, __u8[16])
#define FS_IOC_GET_ENCRYPTION_POLICY _IOW( 'f', 21, struct fscrypt_policy)
+#define FS_IOC_GET_ENCRYPTION_POLICY_EX _IOWR('f', 21, __u8[9]) /* size + version */
#define FS_IOC_ADD_ENCRYPTION_KEY _IOWR('f', 22, struct fscrypt_add_key_args)
#define FS_IOC_REMOVE_ENCRYPTION_KEY _IOR( 'f', 23, struct fscrypt_remove_key_args)
#define FS_IOC_GET_ENCRYPTION_KEY_STATUS _IOWR('f',24, struct fscrypt_get_key_status_args)
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:51

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 18/25] fscrypt: allow adding and removing keys for v2 encryption policies

From: Eric Biggers <[email protected]>

Extend the FS_IOC_ADD_ENCRYPTION_KEY and FS_IOC_REMOVE_ENCRYPTION_KEY
ioctls to support adding and removing keys for use by v2 encryption
policies.

Keys for v2 encryption policies are identified by a 16-byte
"identifier", which is a cryptographic hash of the key, instead of by an
8-byte "descriptor". For FS_IOC_ADD_ENCRYPTION_KEY, the kernel
calculates the key identifier and copies it to userspace. Userspace is
not allowed to choose the key identifier, since the kernel would have to
recalculate it anyway to verify that it is correct.

For FS_IOC_REMOVE_ENCRYPTION_KEY, userspace provides the key identifier
rather than the key descriptor to identify the key to be removed. For
both ioctls, a type field indicates whether the old or new way of
specifying keys is being used. A common structure is used, 'struct
fscrypt_key_specifier', and it has some extra space just in case we have
to introduce a new way to identify keys in the future.

Note that keys for v1 and v2 encryption policies are both stored in
->s_master_keys, but their descriptions will be of different lengths.
Therefore, they cannot be mixed up when we search for a key.

For now the ioctls still always require capable(CAP_SYS_ADMIN). We'll
be able to relax that soon, but only for v2 policies.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/crypto/keyinfo.c | 75 ++++++++++++++++++++++++++++++++++++++++----
include/uapi/linux/fscrypt.h | 2 ++
2 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index e9de625ddfe4..c9e7b2b262d2 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -32,6 +32,19 @@
#define HKDF_HMAC_ALG "hmac(sha512)"
#define HKDF_HASHLEN SHA512_DIGEST_SIZE

+/*
+ * The list of contexts in which we use HKDF to derive additional keys from a
+ * master key. The values in this list are used as the first byte of the
+ * application-specific info string to guarantee that info strings are never
+ * repeated between contexts.
+ *
+ * Keys derived with different info strings are cryptographically isolated from
+ * each other --- knowledge of one derived key doesn't reveal any others.
+ * (This property is particularly important for the derived key used as the
+ * "key identifier", as that is stored in the clear.)
+ */
+#define HKDF_CONTEXT_KEY_IDENTIFIER 1
+
/*
* HKDF consists of two steps:
*
@@ -228,7 +241,12 @@ struct fscrypt_master_key {
*/
struct fscrypt_master_key_secret mk_secret;

- /* Arbitrary key descriptor which was assigned by userspace */
+ /*
+ * For v1 policy keys: an arbitrary key descriptor which was assigned by
+ * userspace (->descriptor).
+ *
+ * For v2 policy keys: a cryptographic hash of this key (->identifier).
+ */
struct fscrypt_key_specifier mk_spec;

/*
@@ -252,6 +270,8 @@ static inline int master_key_spec_len(const struct fscrypt_key_specifier *spec)
switch (spec->type) {
case FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR:
return FSCRYPT_KEY_DESCRIPTOR_SIZE;
+ case FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER:
+ return FSCRYPT_KEY_IDENTIFIER_SIZE;
}
return 0;
}
@@ -346,7 +366,7 @@ static struct key *search_fscrypt_keyring(struct key *keyring,
#define FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE \
(sizeof("fscrypt-") - 1 + sizeof(((struct super_block *)0)->s_id) + 1)

-#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_DESCRIPTOR_SIZE + 1)
+#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_IDENTIFIER_SIZE + 1)

static void format_fs_keyring_description(
char description[FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE],
@@ -550,6 +570,39 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
if (copy_from_user(secret.raw, uarg->raw, secret.size))
goto out_wipe_secret;

+ if (arg.key_spec.type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
+ struct crypto_shash *hmac_tfm;
+
+ hmac_tfm = allocate_hmac_tfm(secret.raw, secret.size);
+ if (IS_ERR(hmac_tfm)) {
+ err = PTR_ERR(hmac_tfm);
+ goto out_wipe_secret;
+ }
+
+ /*
+ * Hash the master key to get the key identifier, then return it
+ * to userspace. Specifically, we derive the key identifier
+ * from the master key with HKDF-SHA512, using a unique
+ * application-specific info string. This is preferable to a
+ * simple truncated SHA512 because it means we only ever use the
+ * master key for a single cryptographic primitive (HKDF-SHA512)
+ * rather than two (HKDF-SHA512 and SHA512). It *maybe* would
+ * be okay, but cryptographically it would be bad practice.
+ */
+ err = hkdf_expand(hmac_tfm, HKDF_CONTEXT_KEY_IDENTIFIER,
+ NULL, 0, arg.key_spec.identifier,
+ FSCRYPT_KEY_IDENTIFIER_SIZE);
+ crypto_free_shash(hmac_tfm);
+ if (err)
+ goto out_wipe_secret;
+
+ err = -EFAULT;
+ if (copy_to_user(uarg->key_spec.identifier,
+ arg.key_spec.identifier,
+ FSCRYPT_KEY_IDENTIFIER_SIZE))
+ goto out_wipe_secret;
+ }
+
err = add_master_key(sb, &secret, &arg.key_spec);
out_wipe_secret:
wipe_master_key_secret(&secret);
@@ -872,6 +925,10 @@ static int derive_key_aes(const u8 *master_key,
* Search the current task's subscribed keyrings for a "logon" key with
* description prefix:descriptor, and if found acquire a read lock on it and
* return a pointer to its validated payload in *payload_ret.
+ *
+ * This is only used for v1 encryption policies, where keys are identified by
+ * master_key_descriptor. With newer policy versions, only the filesystem-level
+ * keyring (->s_master_keys) is supported.
*/
static struct key *
find_and_lock_process_key(const char *prefix,
@@ -977,17 +1034,23 @@ static int find_and_derive_key(const struct inode *inode,
memcpy(mk_spec.descriptor, ctx->v1.master_key_descriptor,
FSCRYPT_KEY_DESCRIPTOR_SIZE);
break;
+ case FSCRYPT_CONTEXT_V2:
+ mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
+ memcpy(mk_spec.identifier, ctx->v2.master_key_identifier,
+ FSCRYPT_KEY_IDENTIFIER_SIZE);
+ break;
default:
- return -EOPNOTSUPP;
+ return -EOPNOTSUPP; /* should have been checked earlier too */
}

key = find_master_key(inode->i_sb, &mk_spec);
if (IS_ERR(key)) {
- if (key != ERR_PTR(-ENOKEY))
+ if (key != ERR_PTR(-ENOKEY) ||
+ ctx->v1.version != FSCRYPT_CONTEXT_V1)
return PTR_ERR(key);
/*
- * As a legacy fallback, we search the current task's subscribed
- * keyrings in addition to ->s_master_keys.
+ * As a legacy fallback for v1 policies, we search the current
+ * task's subscribed keyrings in addition to ->s_master_keys.
*/
return find_and_derive_key_legacy(inode, &ctx->v1, derived_key,
derived_keysize);
diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index 065060b20a34..1918bdc0c6d7 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -82,10 +82,12 @@ struct fscrypt_get_policy_ex_args {
struct fscrypt_key_specifier {
__u32 type;
#define FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR 1
+#define FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER 2
__u32 reserved;
union {
__u8 max_specifier[32];
__u8 descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+ __u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
};
};

--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:58

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 25/25] fscrypt: document the new ioctls and policy version

From: Eric Biggers <[email protected]>

Update the fscrypt documentation file to catch up to all the latest
changes, including the new ioctls to manage master encryption keys in
the filesystem-level keyring, the support for v2 encryption policies,
and the new key derivation function.

Signed-off-by: Eric Biggers <[email protected]>
---
Documentation/filesystems/fscrypt.rst | 565 ++++++++++++++++++++++++++++------
1 file changed, 472 insertions(+), 93 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index f12956f3f1f8..f2d0ff2269f3 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -72,6 +72,9 @@ Online attacks
fscrypt (and storage encryption in general) can only provide limited
protection, if any at all, against online attacks. In detail:

+Side-channel attacks
+~~~~~~~~~~~~~~~~~~~~
+
fscrypt is only resistant to side-channel attacks, such as timing or
electromagnetic attacks, to the extent that the underlying Linux
Cryptographic API algorithms are. If a vulnerable algorithm is used,
@@ -80,29 +83,86 @@ attacker to mount a side channel attack against the online system.
Side channel attacks may also be mounted against applications
consuming decrypted data.

-After an encryption key has been provided, fscrypt is not designed to
-hide the plaintext file contents or filenames from other users on the
-same system, regardless of the visibility of the keyring key.
-Instead, existing access control mechanisms such as file mode bits,
-POSIX ACLs, LSMs, or mount namespaces should be used for this purpose.
-Also note that as long as the encryption keys are *anywhere* in
-memory, an online attacker can necessarily compromise them by mounting
-a physical attack or by exploiting any kernel security vulnerability
-which provides an arbitrary memory read primitive.
-
-While it is ostensibly possible to "evict" keys from the system,
-recently accessed encrypted files will remain accessible at least
-until the filesystem is unmounted or the VFS caches are dropped, e.g.
-using ``echo 2 > /proc/sys/vm/drop_caches``. Even after that, if the
-RAM is compromised before being powered off, it will likely still be
-possible to recover portions of the plaintext file contents, if not
-some of the encryption keys as well. (Since Linux v4.12, all
-in-kernel keys related to fscrypt are sanitized before being freed.
-However, userspace would need to do its part as well.)
-
-Currently, fscrypt does not prevent a user from maliciously providing
-an incorrect key for another user's existing encrypted files. A
-protection against this is planned.
+Unauthorized file access
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+After an encryption key has been added, fscrypt does not hide the
+plaintext file contents or filenames from other users on the same
+system. Instead, existing access control mechanisms such as file mode
+bits, POSIX ACLs, LSMs, or namespaces should be used for this purpose.
+
+(For the reasoning behind this, understand that while the key is
+added, the confidentiality of the data, from the perspective of the
+system itself, is *not* protected by the mathematical properties of
+encryption but rather only by the correctness of the kernel.
+Therefore, any encryption-specific access control checks would merely
+be enforced by kernel *code* and therefore would be largely redundant
+with the wide variety of access control mechanisms already available.)
+
+Kernel compromise
+~~~~~~~~~~~~~~~~~
+
+An attacker who compromises the system enough to read from arbitrary
+memory, e.g. by mounting a physical attack or by exploiting a kernel
+security vulnerability, can compromise all encryption keys that are
+currently in use.
+
+However, fscrypt does allow an encryption key to be removed from the
+kernel, which may protect it from later compromise.
+
+In more detail, the FS_IOC_REMOVE_ENCRYPTION_KEY ioctl will wipe a
+master encryption key from kernel memory. Moreover, it will try to
+evict all cached inodes which had been "unlocked" using the key,
+thereby wiping their derived encryption keys and making them once
+again appear "locked", i.e. in ciphertext or encrypted form.
+
+However, FS_IOC_REMOVE_ENCRYPTION_KEY has some limitations:
+
+- Derived keys for in-use files will *not* be removed or wiped.
+ Therefore, for maximum effect, userspace should close the relevant
+ encrypted files and directories before removing a master key, as
+ well as kill any processes whose working directory is in an affected
+ encrypted directory.
+
+- The kernel cannot magically wipe copies of the master key(s) that
+ userspace might have as well. Therefore, userspace must wipe all
+ copies of the master key(s) it makes as well. Naturally, the same
+ also applies to all higher levels in the key hierarchy, e.g. to all
+ key(s) that are used to wrap or derive the fscrypt master keys.
+ Userspace should also follow other security precautions such as
+ mlock()ing memory containing keys to prevent it from being swapped
+ out.
+
+- In general, decrypted contents and filenames in the kernel VFS
+ caches are freed but not wiped. Therefore, portions thereof may be
+ recoverable from freed memory, even after the corresponding key(s)
+ were wiped. To partially solve this, you may enable page poisoning
+ by enabling CONFIG_PAGE_POISONING in your kernel config and adding
+ page_poison=1 to your kernel command line. However, that has a
+ performance cost.
+
+- Secret keys might still exist in CPU registers, in crypto
+ accelerator hardware (if used by the crypto API to provide any of
+ the algorithms), or in other places not explicitly considered here.
+
+Limitations of v1 policies
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The original encryption policy version (which we call "v1") had some
+weaknesses with respect to online attacks:
+
+- There was no verification that the provided master key was correct.
+ Therefore, malicious users could associate the wrong key with
+ encrypted files, even files to which they had only read-only access.
+
+- A compromise of any file's derived encryption key also compromised
+ the master key it was derived from.
+
+- Non-root users could not securely remove encryption keys.
+
+All the above problems are fixed with v2 encryption policies
+(:c:type:`fscrypt_policy_v2`). For this reason, it's recommended to
+use v2 encryption policies for all new encrypted directories.

Key hierarchy
=============
@@ -167,19 +227,27 @@ master keys or to support rotating master keys. Instead, the master
keys may be wrapped in userspace, e.g. as done by the `fscrypt
<https://github.com/google/fscrypt>`_ tool.

-The current KDF encrypts the master key using the 16-byte nonce as an
-AES-128-ECB key. The output is used as the derived key. If the
-output is longer than needed, then it is truncated to the needed
-length. Truncation is the norm for directories and symlinks, since
-those use the CTS-CBC encryption mode which requires a key half as
-long as that required by the XTS encryption mode.
-
-Note: this KDF meets the primary security requirement, which is to
-produce unique derived keys that preserve the entropy of the master
-key, assuming that the master key is already a good pseudorandom key.
-However, it is nonstandard and has some problems such as being
-reversible, so it is generally considered to be a mistake! It may be
-replaced with HKDF or another more standard KDF in the future.
+A different KDF is used depending on the encryption policy version:
+
+For v1 encryption policies, the KDF is somewhat ad-hoc: we encrypt the
+master key with AES-128-ECB using the file's 16-byte nonce as the AES
+key, and the resulting ciphertext is used as the derived key. If the
+master key is longer than the derived key, then only the needed prefix
+of the ciphertext is used. Truncation is the norm for directories and
+symlinks, since those use the CTS-CBC encryption mode which requires a
+key half as long as that required by the XTS encryption mode.
+
+For v2 encryption policies, the KDF is HKDF-SHA512. HKDF is preferred
+to the AES-based KDF because HKDF is standardized and has a number of
+desirable properties such as being nonreversible and evenly
+distributing the entropy from the master key. To derive a file's
+encryption key using HKDF, the master key is used as the "input key
+material", a fixed value is used as the "salt", and the file's 16-byte
+nonce prefixed with a context byte is used as the
+"application-specific information string". (A fixed salt is used
+because there is no random salt available on a per-master-key basis,
+and the master keys should already be good pseudorandom keys that are
+long enough to make dictionary attacks infeasible.)

Encryption modes and usage
==========================
@@ -249,21 +317,38 @@ Setting an encryption policy
The FS_IOC_SET_ENCRYPTION_POLICY ioctl sets an encryption policy on an
empty directory or verifies that a directory or regular file already
has the specified encryption policy. It takes in a pointer to a
-:c:type:`struct fscrypt_policy`, defined as follows::
-
- #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
+:c:type:`struct fscrypt_policy_v1` or a :c:type:`struct
+fscrypt_policy_v2`, defined as follows::

- struct fscrypt_policy {
+ #define FSCRYPT_POLICY_VERSION_LEGACY 0
+ #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
+ struct fscrypt_policy_v1 {
__u8 version;
__u8 contents_encryption_mode;
__u8 filenames_encryption_mode;
__u8 flags;
__u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
};
+ #define fscrypt_policy fscrypt_policy_v1
+
+ #define FSCRYPT_POLICY_VERSION_2 2
+ #define FSCRYPT_KEY_IDENTIFIER_SIZE 16
+ struct fscrypt_policy_v2 {
+ __u8 version;
+ __u8 contents_encryption_mode;
+ __u8 filenames_encryption_mode;
+ __u8 flags;
+ __u8 reserved[4];
+ __u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+ };

This structure must be initialized as follows:

-- ``version`` must be 0.
+- ``version`` must be FSCRYPT_POLICY_VERSION_LEGACY (0) if the struct
+ is :c:type:`fscrypt_policy_v1` or FSCRYPT_POLICY_VERSION_2 (2) if
+ the struct is :c:type:`fscrypt_policy_v2`. (Note: we refer to the
+ legacy policy version as "v1", though its version number was really
+ 0.) For new encrypted directories, use v2 policies.

- ``contents_encryption_mode`` and ``filenames_encryption_mode`` must
be set to constants from ``<linux/fs.h>`` which identify the
@@ -276,15 +361,25 @@ This structure must be initialized as follows:
identifies the amount of NUL-padding to use when encrypting
filenames. If unsure, use FSCRYPT_POLICY_FLAGS_PAD_32 (0x3).

-- ``master_key_descriptor`` specifies how to find the master key in
- the keyring; see `Adding keys`_. It is up to userspace to choose a
- unique ``master_key_descriptor`` for each master key. The e4crypt
- and fscrypt tools use the first 8 bytes of
+- For v2 encryption policies, ``reserved`` must be zeroed.
+
+- For v1 encryption policies, ``master_key_descriptor`` specifies how
+ to find the master key in a keyring; see `Adding keys`_. It is up
+ to userspace to choose a unique ``master_key_descriptor`` for each
+ master key. The e4crypt and fscrypt tools use the first 8 bytes of
``SHA-512(SHA-512(master_key))``, but this particular scheme is not
required. Also, the master key need not be in the keyring yet when
FS_IOC_SET_ENCRYPTION_POLICY is executed. However, it must be added
before any files can be created in the encrypted directory.

+ For v2 encryption policies, ``master_key_descriptor`` has been
+ replaced with ``master_key_identifier``, which is longer and cannot
+ be arbitrarily chosen. Instead, the key must first be added using
+ FS_IOC_ADD_ENCRYPTION_KEY, as described in `Adding keys`_. Then,
+ the ``key_spec.identifier`` the kernel returned in the
+ :c:type:`struct fscrypt_add_key_args` must be used as the
+ ``master_key_identifier`` in the ``struct fscrypt_policy_v2``.
+
If the file is not yet encrypted, then FS_IOC_SET_ENCRYPTION_POLICY
verifies that the file is an empty directory. If so, the specified
encryption policy is assigned to the directory, turning it into an
@@ -300,6 +395,15 @@ policy exactly matches the actual one. If they match, then the ioctl
returns 0. Otherwise, it fails with EEXIST. This works on both
regular files and directories, including nonempty directories.

+When a v2 encryption policy is assigned to a directory, it is also
+required that either the specified key has been added by the current
+user, or the caller has CAP_FOWNER in the initial user namespace.
+(This is needed to prevent a user from encrypting their data with
+another user's key.) The key must remain added while
+FS_IOC_SET_ENCRYPTION_POLICY is executing. However, if the new
+encrypted directory does not need to be accessed immediately, then the
+key can be removed right away afterwards.
+
Note that the ext4 filesystem does not allow the root directory to be
encrypted, even if it is empty. Users who want to encrypt an entire
filesystem with one key should consider using dm-crypt instead.
@@ -312,7 +416,9 @@ FS_IOC_SET_ENCRYPTION_POLICY can fail with the following errors:
- ``EEXIST``: the file is already encrypted with an encryption policy
different from the one specified
- ``EINVAL``: an invalid encryption policy was specified (invalid
- version, mode(s), or flags)
+ version, mode(s), or flags; or reserved bits were set)
+- ``ENOKEY``: a v2 encryption policy was specified, but the key with
+ the specified ``master_key_identifier`` has not been added
- ``ENOTDIR``: the file is unencrypted and is a regular file, not a
directory
- ``ENOTEMPTY``: the file is unencrypted and is a nonempty directory
@@ -331,25 +437,82 @@ FS_IOC_SET_ENCRYPTION_POLICY can fail with the following errors:
Getting an encryption policy
----------------------------

-The FS_IOC_GET_ENCRYPTION_POLICY ioctl retrieves the :c:type:`struct
-fscrypt_policy`, if any, for a directory or regular file. See above
-for the struct definition. No additional permissions are required
-beyond the ability to open the file.
+Two ioctls are available to get a file's encryption policy:
+
+- FS_IOC_GET_ENCRYPTION_POLICY_EX
+- FS_IOC_GET_ENCRYPTION_POLICY
+
+The extended (_EX) version of the ioctl is more general and is
+recommended to use when possible. However, on older kernels only the
+original ioctl is available. Applications should try the extended
+version, and if it fails with ENOTTY fall back to the original
+version.
+
+Preferred method
+~~~~~~~~~~~~~~~~
+
+The FS_IOC_GET_ENCRYPTION_POLICY_EX ioctl retrieves the encryption
+policy, if any, for a directory or regular file. No additional
+permissions are required beyond the ability to open the file. It
+takes in a pointer to a buffer formatted as a :c:type:`struct
+fscrypt_get_policy_ex_args`, defined as follows::
+
+ struct fscrypt_get_policy_ex_args {
+ __u64 size;
+ union {
+ __u8 version;
+ struct fscrypt_policy_v1 v1;
+ struct fscrypt_policy_v2 v2;
+ } policy;
+ };
+
+The caller must initialize ``size`` to the size of the buffer in
+bytes, including both the ``size`` field and the space available for
+the policy struct. It is recommended to use ``sizeof(struct
+fscrypt_get_policy_ex_args)``.
+
+On successful return, ``size`` is set to the actual number of bytes
+returned, including both the ``size`` field and the actual size of the
+returned policy struct. In addition, the ``version`` field should be
+used to determine the actual policy version returned. Note that the
+version code for the "v1" policy is actually 0
+(FSCRYPT_POLICY_VERSION_LEGACY).

-FS_IOC_GET_ENCRYPTION_POLICY can fail with the following errors:
+FS_IOC_GET_ENCRYPTION_POLICY_EX can fail with the following errors:

- ``EINVAL``: the file is encrypted, but it uses an unrecognized
- encryption context format
+ encryption policy version; or, an invalid ``size`` was provided
- ``ENODATA``: the file is not encrypted
-- ``ENOTTY``: this type of filesystem does not implement encryption
+- ``ENOTTY``: this type of filesystem does not implement encryption,
+ or this kernel is too old to support FS_IOC_GET_ENCRYPTION_POLICY_EX
+ (try FS_IOC_GET_ENCRYPTION_POLICY instead)
- ``EOPNOTSUPP``: the kernel was not configured with encryption
support for this filesystem
+- ``EOVERFLOW``: the file is encrypted and uses a recognized
+ encryption policy version, but the policy struct does not fit into
+ the provided buffer

Note: if you only need to know whether a file is encrypted or not, on
most filesystems it is also possible to use the FS_IOC_GETFLAGS ioctl
and check for FS_ENCRYPT_FL, or to use the statx() system call and
check for STATX_ATTR_ENCRYPTED in stx_attributes.

+Legacy method
+~~~~~~~~~~~~~
+
+The FS_IOC_GET_ENCRYPTION_POLICY ioctl can also retrieve the
+encryption policy, if any, for a directory or regular file. However,
+unlike the extended version (FS_IOC_GET_ENCRYPTION_POLICY_EX),
+FS_IOC_GET_ENCRYPTION_POLICY only supports the original policy
+version. It takes in a pointer directly to a :c:type:`struct
+fscrypt_policy_v1` rather than a :c:type:`struct
+fscrypt_get_policy_ex_args`.
+
+The error codes for FS_IOC_GET_ENCRYPTION_POLICY are the same as those
+for FS_IOC_GET_ENCRYPTION_POLICY_EX, except that
+FS_IOC_GET_ENCRYPTION_POLICY also returns ``EINVAL`` if the file is
+encrypted using a newer encryption policy version.
+
Getting the per-filesystem salt
-------------------------------

@@ -365,8 +528,97 @@ generate and manage any needed salt(s) in userspace.
Adding keys
-----------

-To provide a master key, userspace must add it to an appropriate
-keyring using the add_key() system call (see:
+Preferred method
+~~~~~~~~~~~~~~~~
+
+The FS_IOC_ADD_ENCRYPTION_KEY ioctl adds a master encryption key to
+the filesystem, making all files on the filesystem which were
+encrypted using that key appear "unlocked", i.e. in plaintext form.
+It takes in a pointer to a :c:type:`struct fscrypt_add_key_args`,
+defined as follows::
+
+ struct fscrypt_add_key_args {
+ __u32 raw_size;
+ __u32 reserved1;
+ __u64 reserved2[2];
+ struct fscrypt_key_specifier key_spec;
+ __u8 raw[];
+ };
+
+ struct fscrypt_key_specifier {
+ __u32 type;
+ #define FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR 1
+ #define FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER 2
+ __u32 reserved;
+ union {
+ __u8 max_specifier[32];
+ __u8 descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+ __u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+ };
+ };
+
+:c:type:`struct fscrypt_add_key_args` must be initialized as follows:
+
+- ``raw_size`` must be the size of the ``raw`` key provided, in bytes.
+
+- If the key is being added for use by v1 encryption policies, then
+ ``key_spec.type`` must contain FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR, and
+ ``key_spec.descriptor`` must contain the descriptor of the key being
+ added, corresponding to the value in the ``master_key_descriptor``
+ field of :c:type:`struct fscrypt_policy_v1`. To add this type of
+ key, the calling process must have the CAP_SYS_ADMIN capability in
+ the initial user namespace.
+
+ Alternatively, if the key is being added for use by v2 encryption
+ policies, then ``key_spec.type`` must contain
+ FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER, and ``key_spec.identifier`` is an
+ *output* field which the kernel fills in with a cryptographic hash
+ of the key. To add this type of key, the calling process does not
+ need any privileges. However, the number of keys that can be added
+ is limited by the user's quota for the keyrings service (see
+ ``Documentation/security/keys/core.rst``).
+
+- ``raw`` is a variable-length field which must contain the actual
+ key, ``raw_size`` bytes long.
+
+- All reserved fields must be zeroed.
+
+FS_IOC_ADD_ENCRYPTION_KEY can fail with the following errors:
+
+- ``EACCES``: FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR was specified, but the
+ caller does not have the CAP_SYS_ADMIN capability in the initial
+ user namespace
+- ``EDQUOT``: the key quota for this user would be exceeded by adding
+ the key
+- ``EINVAL``: invalid key size or key specifier type, or reserved bits
+ were set
+- ``ENOTTY``: this type of filesystem does not implement encryption
+- ``EOPNOTSUPP``: the kernel was not configured with encryption
+ support for this filesystem, or the filesystem superblock has not
+ had encryption enabled on it
+
+Legacy method
+~~~~~~~~~~~~~
+
+For v1 encryption policies, a master encryption key can also be
+provided by adding it to a process-subscribed keyring, e.g. to a
+session keyring, or to a user keyring if the user keyring is linked
+into the session keyring.
+
+This method is deprecated (and not supported for v2 encryption
+policies) for several reasons. First, it cannot be used in
+combination with FS_IOC_REMOVE_ENCRYPTION_KEY (see `Removing keys`_),
+so for removing a key a workaround such as keyctl_unlink() in
+combination with ``sync; echo 2 > /proc/sys/vm/drop_caches`` would
+have to be used. Second, it doesn't match the fact that the
+locked/unlocked status of encrypted files (i.e. whether they appear to
+be in plaintext form or in ciphertext form) is global. This mismatch
+has caused much confusion as well as real problems when processes
+running under different UIDs, such as a ``sudo`` command, need to
+access encrypted files.
+
+Nevertheless, to add a key to one of the process-subscribed keyrings,
+the add_key() system call can be used (see:
``Documentation/security/keys/core.rst``). The key type must be
"logon"; keys of this type are kept in kernel memory and cannot be
read back by userspace. The key description must be "fscrypt:"
@@ -391,26 +643,143 @@ with a filesystem-specific prefix such as "ext4:". However, the
filesystem-specific prefixes are deprecated and should not be used in
new programs.

-There are several different types of keyrings in which encryption keys
-may be placed, such as a session keyring, a user session keyring, or a
-user keyring. Each key must be placed in a keyring that is "attached"
-to all processes that might need to access files encrypted with it, in
-the sense that request_key() will find the key. Generally, if only
-processes belonging to a specific user need to access a given
-encrypted directory and no session keyring has been installed, then
-that directory's key should be placed in that user's user session
-keyring or user keyring. Otherwise, a session keyring should be
-installed if needed, and the key should be linked into that session
-keyring, or in a keyring linked into that session keyring.
-
-Note: introducing the complex visibility semantics of keyrings here
-was arguably a mistake --- especially given that by design, after any
-process successfully opens an encrypted file (thereby setting up the
-per-file key), possessing the keyring key is not actually required for
-any process to read/write the file until its in-memory inode is
-evicted. In the future there probably should be a way to provide keys
-directly to the filesystem instead, which would make the intended
-semantics clearer.
+Removing keys
+-------------
+
+The FS_IOC_REMOVE_ENCRYPTION_KEY ioctl can be used to remove a master
+encryption key from the kernel, wiping the corresponding secrets from
+memory and causing any files which were "unlocked" with the key to
+appear "locked" again. It takes in a pointer to a :c:type:`struct
+fscrypt_remove_key_args`, defined as follows::
+
+ struct fscrypt_remove_key_args {
+ __u32 flags;
+ #define FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS 0x00000001
+ __u32 reserved1;
+ __u64 reserved2[2];
+ struct fscrypt_key_specifier key_spec;
+ };
+
+This structure must be initialized as follows:
+
+- ``flags`` can contain the following flags:
+
+ - ``FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS`` specifies that the key
+ should be removed even if it has also been added by other users.
+ Specifying this flag requires the CAP_SYS_ADMIN capability in
+ the initial user namespace.
+
+- The key to remove is specified by ``key_spec``:
+
+ - To remove a key used by v1 encryption policies, set
+ ``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR and fill
+ in ``key_spec.descriptor``. To remove this type of key, the
+ calling process must have the CAP_SYS_ADMIN capability in the
+ initial user namespace.
+
+ - To remove a key used by v2 encryption policies, set
+ ``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER and fill
+ in ``key_spec.identifier``. To remove this type of key, no
+ privileges are needed. However, users can only remove keys that
+ they added themselves, subject to privileged override with
+ FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS.
+
+- All reserved fields must be zeroed.
+
+FS_IOC_REMOVE_ENCRYPTION_KEY can fail with the following errors:
+
+- ``EACCES``: The FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR key specifier type
+ and/or the FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS flag was specified, but
+ the caller does not have the CAP_SYS_ADMIN capability in the initial
+ user namespace
+- ``EBUSY``: the master key secret was wiped from memory, but some
+ files which were unlocked with it are still in use. Such files
+ could not be locked, nor could their per-file keys be wiped from
+ memory. The ioctl may be retried later to re-attempt locking the
+ remaining files.
+- ``EINVAL``: invalid flags or key specifier type, or reserved bits
+ were set
+- ``ENOKEY``: the key is not present or has already been removed
+- ``ENOTTY``: this type of filesystem does not implement encryption
+- ``EOPNOTSUPP``: the kernel was not configured with encryption
+ support for this filesystem, or the filesystem superblock has not
+ had encryption enabled on it
+- ``EUSERS``: the key cannot be removed because other users have added
+ it too
+
+Before using this ioctl, please read the `Kernel compromise`_ section
+for a discussion of the security goals and limitations of this ioctl.
+
+Getting key status
+------------------
+
+The FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl retrieves the status of a
+master encryption key. It takes in a pointer to a :c:type:`struct
+fscrypt_get_key_status_args`, defined as follows::
+
+ struct fscrypt_get_key_status_args {
+ /* input */
+ __u64 reserved1[3];
+ struct fscrypt_key_specifier key_spec;
+
+ /* output */
+ __u32 status;
+ #define FSCRYPT_KEY_STATUS_ABSENT 1
+ #define FSCRYPT_KEY_STATUS_PRESENT 2
+ #define FSCRYPT_KEY_STATUS_INCOMPLETELY_REMOVED 3
+ __u32 status_flags;
+ #define FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF 0x00000001
+ __u32 user_count;
+ __u32 reserved2;
+ __u64 reserved3[6];
+ };
+
+The caller must zero ``reserved1``, then fill in ``key_spec``:
+
+ - To get the status of a key for v1 encryption policies, set
+ ``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR and fill
+ in ``key_spec.descriptor``.
+
+ - To get the status of a key for v2 encryption policies, set
+ ``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER and fill
+ in ``key_spec.identifier``.
+
+On success, 0 is returned and the kernel fills in the output fields:
+
+- ``status`` indicates whether the key is absent, present, or
+ incompletely removed. Incompletely removed means that the master
+ secret has been removed, but some files are still in use; i.e.,
+ FS_IOC_REMOVE_ENCRYPTION_KEY returned EBUSY.
+
+- ``status_flags`` can contain the following flags:
+
+ - ``FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF`` indicates that the key
+ has added by the current user. This is only set for keys
+ identified by ``identifier`` rather than by ``descriptor``.
+
+- ``user_count`` specifies the number of users who have added the key.
+ This is only set for keys identified by ``identifier`` rather than
+ by ``descriptor``.
+
+FS_IOC_GET_ENCRYPTION_KEY_STATUS can fail with the following errors:
+
+- ``EINVAL``: invalid key specifier type, or reserved bits were set
+- ``ENOTTY``: this type of filesystem does not implement encryption
+- ``EOPNOTSUPP``: the kernel was not configured with encryption
+ support for this filesystem, or the filesystem superblock has not
+ had encryption enabled on it
+
+Among other use cases, FS_IOC_GET_ENCRYPTION_KEY_STATUS might be
+useful for determining whether the key for a given encrypted directory
+needs to be added before prompting the user for the passphrase needed
+to derive the key.
+
+FS_IOC_GET_ENCRYPTION_KEY_STATUS can only get the status of keys in
+the filesystem-level keyring, i.e. the keyring managed by
+FS_IOC_ADD_ENCRYPTION_KEY and FS_IOC_REMOVE_ENCRYPTION_KEY. It cannot
+get the status of a key that has only been added for use by v1
+encryption policies using the legacy mechanism involving
+process-subscribed keyrings.

Access semantics
================
@@ -459,7 +828,7 @@ Without the key

Some filesystem operations may be performed on encrypted regular
files, directories, and symlinks even before their encryption key has
-been provided:
+been added, or after their encryption key has been removed:

- File metadata may be read, e.g. using stat().

@@ -524,20 +893,20 @@ Encryption context
------------------

An encryption policy is represented on-disk by a :c:type:`struct
-fscrypt_context`. It is up to individual filesystems to decide where
-to store it, but normally it would be stored in a hidden extended
-attribute. It should *not* be exposed by the xattr-related system
-calls such as getxattr() and setxattr() because of the special
-semantics of the encryption xattr. (In particular, there would be
-much confusion if an encryption policy were to be added to or removed
-from anything other than an empty directory.) The struct is defined
-as follows::
+fscrypt_context_v1` or a :c:type:`struct fscrypt_context_v2`. It is
+up to individual filesystems to decide where to store it, but normally
+it would be stored in a hidden extended attribute. It should *not* be
+exposed by the xattr-related system calls such as getxattr() and
+setxattr() because of the special semantics of the encryption xattr.
+(In particular, there would be much confusion if an encryption policy
+were to be added to or removed from anything other than an empty
+directory.) These structs are defined as follows::

- #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
#define FS_KEY_DERIVATION_NONCE_SIZE 16

- struct fscrypt_context {
- u8 format;
+ #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
+ struct fscrypt_context_v1 {
+ u8 version;
u8 contents_encryption_mode;
u8 filenames_encryption_mode;
u8 flags;
@@ -545,12 +914,22 @@ as follows::
u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
};

-Note that :c:type:`struct fscrypt_context` contains the same
-information as :c:type:`struct fscrypt_policy` (see `Setting an
-encryption policy`_), except that :c:type:`struct fscrypt_context`
-also contains a nonce. The nonce is randomly generated by the kernel
-and is used to derive the inode's encryption key as described in
-`Per-file keys`_.
+ #define FSCRYPT_KEY_IDENTIFIER_SIZE 16
+ struct fscrypt_context_v2 {
+ u8 version;
+ u8 contents_encryption_mode;
+ u8 filenames_encryption_mode;
+ u8 flags;
+ u8 reserved[4];
+ u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+ u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
+ };
+
+Note that the context structs contain the same information as the
+corresponding policy structs (see `Setting an encryption policy`_),
+except that the context structs also contain a nonce. The nonce is
+randomly generated by the kernel and is used to derive the inode's
+encryption key as described in `Per-file keys`_.

Data path changes
-----------------
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:52

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 19/25] fscrypt: use HKDF-SHA512 to derive the per-file keys for v2 policies

From: Eric Biggers <[email protected]>

The AES-ECB-based method we're using to derive the per-inode encryption
keys is nonstandard and has a number of problems, such as being
trivially reversible. Fix these problems for v2 encryption policies by
deriving the keys using HKDF-SHA512 instead. The inode's nonce prefixed
with a context byte is used as the application-specific info string.

Supposedly, one of the reasons that HKDF wasn't used originally was
because of performance concerns. However, we actually can derive on the
order of 1 million keys per second, so it's likely not a bottleneck in
practice. Moreover, although HKDF-SHA512 can require a bit more actual
crypto work per key derivation than the old KDF, the real world
performance is actually just as good or even better than the old KDF.
This is because the old KDF has to allocate and key a new "ecb(aes)"
transform for every key derivation (since it's keyed with the nonce
rather than the master key), whereas with HKDF we simply use a cached,
pre-keyed "hmac(sha512)" transform. And the old KDF often spends more
time allocating its crypto transform than doing actual crypto work.

Another benefit to switching to HKDF is that we no longer need to hold
the raw master key in memory, but rather only an HMAC transform keyed by
a pseudorandom key extracted from the master key. Of course, for the
software HMAC implementation there is no security benefit, since
compromising the state of the HMAC transform is equivalent to
compromising the raw master key. However, there could be a security
benefit if used with an HMAC implementation that holds the secret in
crypto accelerator hardware rather than in main memory.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/crypto/fscrypt_private.h | 4 ++--
fs/crypto/keyinfo.c | 58 ++++++++++++++++++++++++++++++++++-----------
2 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index dec85c4b14a8..6d420c9a85bd 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -49,7 +49,7 @@ struct fscrypt_context_v1 {

/*
* A unique value used in combination with the master key to derive the
- * file's actual encryption key
+ * file's actual encryption key, using the AES-ECB-based KDF.
*/
u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
};
@@ -78,7 +78,7 @@ struct fscrypt_context_v2 {

/*
* A unique value used in combination with the master key to derive the
- * file's actual encryption key
+ * file's actual encryption key, using HKDF-SHA512.
*/
u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
};
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index c9e7b2b262d2..ec181c4eca56 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -44,6 +44,7 @@
* "key identifier", as that is stored in the clear.)
*/
#define HKDF_CONTEXT_KEY_IDENTIFIER 1
+#define HKDF_CONTEXT_PER_FILE_KEY 2

/*
* HKDF consists of two steps:
@@ -213,10 +214,18 @@ static struct crypto_shash *allocate_hmac_tfm(const u8 *master_key, u32 size)
*/
struct fscrypt_master_key_secret {

- /* Size of the raw key in bytes */
+ /*
+ * For v2 policy keys: an HMAC transform keyed by the pseudorandom key
+ * generated by computing HKDF-Extract with the raw master key as the
+ * input key material. This is used to efficiently derive the per-inode
+ * encryption keys using HKDF-Expand later.
+ */
+ struct crypto_shash *hmac_tfm;
+
+ /* Size of the raw key in bytes. Set even if ->raw isn't set. */
u32 size;

- /* The raw key */
+ /* For v1 policy keys: the raw key. */
u8 raw[FSCRYPT_MAX_KEY_SIZE];
};

@@ -291,6 +300,7 @@ is_master_key_secret_present(const struct fscrypt_master_key_secret *secret)

static void wipe_master_key_secret(struct fscrypt_master_key_secret *secret)
{
+ crypto_free_shash(secret->hmac_tfm);
memzero_explicit(secret, sizeof(*secret));
}

@@ -571,14 +581,17 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
goto out_wipe_secret;

if (arg.key_spec.type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
- struct crypto_shash *hmac_tfm;

- hmac_tfm = allocate_hmac_tfm(secret.raw, secret.size);
- if (IS_ERR(hmac_tfm)) {
- err = PTR_ERR(hmac_tfm);
+ secret.hmac_tfm = allocate_hmac_tfm(secret.raw, secret.size);
+ if (IS_ERR(secret.hmac_tfm)) {
+ err = PTR_ERR(secret.hmac_tfm);
+ secret.hmac_tfm = NULL;
goto out_wipe_secret;
}

+ /* The raw key is no longer needed */
+ memzero_explicit(secret.raw, sizeof(secret.raw));
+
/*
* Hash the master key to get the key identifier, then return it
* to userspace. Specifically, we derive the key identifier
@@ -589,10 +602,9 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
* rather than two (HKDF-SHA512 and SHA512). It *maybe* would
* be okay, but cryptographically it would be bad practice.
*/
- err = hkdf_expand(hmac_tfm, HKDF_CONTEXT_KEY_IDENTIFIER,
+ err = hkdf_expand(secret.hmac_tfm, HKDF_CONTEXT_KEY_IDENTIFIER,
NULL, 0, arg.key_spec.identifier,
FSCRYPT_KEY_IDENTIFIER_SIZE);
- crypto_free_shash(hmac_tfm);
if (err)
goto out_wipe_secret;

@@ -871,8 +883,13 @@ static void derive_crypt_complete(struct crypto_async_request *req, int rc)
}

/*
- * Key derivation function. This generates the derived key by encrypting the
- * master key with AES-128-ECB using the nonce as the AES key.
+ * Legacy key derivation function. This generates the derived key by encrypting
+ * the master key with AES-128-ECB using the nonce as the AES key. This
+ * provides a unique derived key with sufficient entropy for each inode.
+ * However, it's nonstandard, non-extensible, doesn't evenly distribute the
+ * entropy from the master key, and is trivially reversible: an attacker who
+ * compromises a derived key can "decrypt" it to get back to the master key,
+ * then derive any other key. For all new code, use HKDF instead.
*
* The master key must be at least as long as the derived key. If the master
* key is longer, then only the first 'derived_keysize' bytes are used.
@@ -1078,8 +1095,21 @@ static int find_and_derive_key(const struct inode *inode,
goto out_release_key;
}

- err = derive_key_aes(mk->mk_secret.raw, &ctx->v1,
- derived_key, derived_keysize);
+ /*
+ * Derive the inode's encryption key, given the master key and the nonce
+ * from the inode's fscrypt_context. v1 policies used an AES-ECB-based
+ * KDF (Key Derivation Function). Newer policies use HKDF-SHA512, which
+ * fixes a number of problems with the AES-ECB-based KDF.
+ */
+ if (ctx->v1.version == FSCRYPT_CONTEXT_V1) {
+ err = derive_key_aes(mk->mk_secret.raw, &ctx->v1,
+ derived_key, derived_keysize);
+ } else {
+ err = hkdf_expand(mk->mk_secret.hmac_tfm,
+ HKDF_CONTEXT_PER_FILE_KEY,
+ ctx->v2.nonce, sizeof(ctx->v2.nonce),
+ derived_key, derived_keysize);
+ }
if (err)
goto out_release_key;

@@ -1275,8 +1305,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
goto out;

/*
- * This cannot be a stack buffer because it is passed to the scatterlist
- * crypto API as part of key derivation.
+ * This cannot be a stack buffer because it may be passed to the
+ * scatterlist crypto API during key derivation.
*/
res = -ENOMEM;
derived_key = kmalloc(FS_MAX_KEY_SIZE, GFP_NOFS);
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:47

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 14/25] ubifs crypto: wire up new ioctls for managing encryption keys

From: Eric Biggers <[email protected]>

Signed-off-by: Eric Biggers <[email protected]>
---
fs/ubifs/ioctl.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c
index 0164bcc827f8..09ede2d1898f 100644
--- a/fs/ubifs/ioctl.c
+++ b/fs/ubifs/ioctl.c
@@ -205,6 +205,15 @@ long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
#endif
}

+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ return fscrypt_ioctl_add_key(file, (void __user *)arg);
+
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ return fscrypt_ioctl_remove_key(file, (const void __user *)arg);
+
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
+ return fscrypt_ioctl_get_key_status(file, (void __user *)arg);
+
default:
return -ENOTTY;
}
@@ -222,6 +231,9 @@ long ubifs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
break;
case FS_IOC_SET_ENCRYPTION_POLICY:
case FS_IOC_GET_ENCRYPTION_POLICY:
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
break;
default:
return -ENOIOCTLCMD;
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:50

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 17/25] fscrypt: add an HKDF-SHA512 implementation

From: Eric Biggers <[email protected]>

For v2 encryption policies, we need to hash the master key to compute a
master_key_identifier. Naively, we could just use a truncated SHA-512
or another common cryptographic hash function. However, that would
cause the same key material to be used in two different ways: both as
input to the hash function, and as input to the AES-ECB-based KDF when
deriving the per-inode encryption keys. There *probably* isn't any
practical attack on that, but still it would be better "crypto hygiene"
to use the key material for one purpose only, e.g. just for a KDF.

It also happens that the AES-based KDF is on our list of things to fix.
While it does generate unique derived keys with sufficient entropy, it
is nonstandard and has some problems that don't exist in standard KDFs
such as HKDF. For example, the AES-based KDF is reversible: given a
derived key and nonce, an attacker can easily compute the master key.
This was maybe okay for the original threat model of ext4 encryption
where the master key and derived keys were considered equally hard to
compromise. But now we would like to be more robust against threats
such as a derived key being compromised through a timing attack, or a
derived key for an in-use file being compromised after the master key
has already been wiped from memory via FS_IOC_REMOVE_ENCRYPTION_KEY.

HKDF also has other advantages over the AES-ECB-based KDF such as evenly
distributing the entropy from the input key material and being more
extensible to deriving other key material that may be needed.

Since we're introducing a new encryption policy version that already
includes an on-disk format change, and we also now have a good place to
cache an HMAC transform for each master key (struct fscrypt_master_key)
so that HKDF can be implemented efficiently, we finally have a chance to
switch to HKDF to derive the per-file keys.

In addition, we'll use HKDF to derive the master_key_identifier,
avoiding the need for a separate cryptographic hash primitive. This is
secure because the output from HKDF is cryptographically isolated, i.e.
sending some output in the clear doesn't reveal any other output, in a
computational sense. (This is assuming that application-specific info
strings aren't repeated between different uses of HKDF, but we'll use
context bytes to ensure that.)

Thus, this patch adds an implementation of HKDF to keyinfo.c, using an
HMAC transform allocated from the crypto API. Later patches will make
use of it.

Note that using HKDF-SHA512 as the key derivation function will
introduce a dependency on the security and implementation of SHA-512,
whereas before we were using only AES for both key derivation and
encryption. However, by using HMAC rather than the hash function
directly, HKDF is designed to remain secure even if various classes of
attacks, e.g. collision attacks, are found against the underlying
unkeyed hash function. Even HMAC-MD5 is still considered secure in
practice, despite MD5 itself having been heavily compromised. And
meanwhile, the AES-based KDF used the public nonce as the cipher *key*,
which is an unusual case which probably hasn't undergone much
cryptanalysis. HKDF-SHA512 seems like a safer bet.

We *could* actually avoid introducing a hash primitive by instantiating
HKDF-Expand with CMAC-AES256 as the pseudorandom function rather than
HMAC-SHA512. That would work; however, the HKDF specification doesn't
explicitly allow a non-HMAC pseudorandom function, so it would be less
standard. It would also require skipping HKDF-Extract and making the
API accept only 32-byte master keys, since otherwise HKDF-Extract using
CMAC-AES would produce a pseudorandom key only 16 bytes long which would
only be enough for AES-128, not AES-256.

References:

- RFC 5869. "HMAC-based Extract-and-Expand Key Derivation Function
(HKDF)". https://tools.ietf.org/html/rfc5869

- Krawczyk (2010). "Cryptographic Extraction and Key Derivation: The
HKDF Scheme". https://eprint.iacr.org/2010/264.pdf

Signed-off-by: Eric Biggers <[email protected]>
---
fs/crypto/Kconfig | 2 +
fs/crypto/keyinfo.c | 180 +++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 181 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index 02b7d91c9231..bbd4e38b293c 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -8,6 +8,8 @@ config FS_ENCRYPTION
select CRYPTO_CTS
select CRYPTO_CTR
select CRYPTO_SHA256
+ select CRYPTO_SHA512
+ select CRYPTO_HMAC
select KEYS
help
Enable encryption of files and directories. This
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 937a678ebba1..e9de625ddfe4 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -6,6 +6,11 @@
* This contains encryption key functions.
*
* Written by Michael Halcrow, Ildar Muslukhov, and Uday Savagaonkar, 2015.
+ *
+ * HKDF support and key add/remove API added by Eric Biggers, 2017.
+ *
+ * The implementation and usage of HKDF should conform to RFC-5869 ("HMAC-based
+ * Extract-and-Expand Key Derivation Function").
*/

#include <keys/user-type.h>
@@ -14,10 +19,181 @@
#include <linux/scatterlist.h>
#include <linux/seq_file.h>
#include <crypto/aes.h>
+#include <crypto/hash.h>
#include <crypto/sha.h>
#include "fscrypt_private.h"

-static struct crypto_shash *essiv_hash_tfm;
+/*
+ * Any unkeyed cryptographic hash algorithm can be used with HKDF, but we use
+ * SHA-512 because it is reasonably secure and efficient; and since it produces
+ * a 64-byte digest, deriving an AES-256-XTS key preserves all 64 bytes of
+ * entropy from the master key and requires only one iteration of HKDF-Expand.
+ */
+#define HKDF_HMAC_ALG "hmac(sha512)"
+#define HKDF_HASHLEN SHA512_DIGEST_SIZE
+
+/*
+ * HKDF consists of two steps:
+ *
+ * 1. HKDF-Extract: extract a fixed-length pseudorandom key from the
+ * input keying material and optional salt.
+ * 2. HKDF-Expand: expand the pseudorandom key into output keying material of
+ * any length, parameterized by an application-specific info string.
+ *
+ * HKDF-Extract can be skipped if the input is already a good pseudorandom key
+ * that is at least as long as the hash. While the fscrypt master keys should
+ * already be good pseudorandom keys, when using encryption algorithms that use
+ * short keys (e.g. AES-128-CBC) we'd like to permit the master key to be
+ * shorter than HKDF_HASHLEN bytes. Thus, we still must do HKDF-Extract.
+ *
+ * Ideally, HKDF-Extract would be passed a random salt for each distinct input
+ * key. Details about the advantages of a random salt can be found in the HKDF
+ * paper (Krawczyk, 2010; "Cryptographic Extraction and Key Derivation: The HKDF
+ * Scheme"). However, we do not have the ability to store a salt on a
+ * per-master-key basis. Thus, we have to use a fixed salt. This is sufficient
+ * as long as the master keys are already pseudorandom and are long enough to
+ * make dictionary attacks infeasible. This should be the case if userspace
+ * used a cryptographically secure random number generator, e.g. /dev/urandom,
+ * to generate the master keys and it was initialized with sufficient entropy.
+ *
+ * For the fixed salt we use "fscrypt_hkdf_salt" rather than default of all 0's
+ * defined by RFC-5869. This is only to be slightly more robust against
+ * userspace (unwisely) reusing the master keys for different purposes.
+ * Logically, it's more likely that the keys would be passed to unsalted
+ * HKDF-SHA512 than specifically to "fscrypt_hkdf_salt"-salted HKDF-SHA512.
+ * Of course, a random salt would be better for this purpose.
+ */
+
+#define HKDF_SALT "fscrypt_hkdf_salt"
+#define HKDF_SALT_LEN (sizeof(HKDF_SALT) - 1)
+
+/*
+ * HKDF-Extract (RFC-5869 section 2.2). This extracts a pseudorandom key 'prk'
+ * from the input key material 'ikm' and a salt. See explanation above for why
+ * we use a fixed salt.
+ */
+static int hkdf_extract(struct crypto_shash *hmac_tfm,
+ const u8 *ikm, unsigned int ikmlen,
+ u8 prk[HKDF_HASHLEN])
+{
+ SHASH_DESC_ON_STACK(desc, hmac_tfm);
+ int err;
+
+ desc->tfm = hmac_tfm;
+ desc->flags = 0;
+
+ err = crypto_shash_setkey(hmac_tfm, HKDF_SALT, HKDF_SALT_LEN);
+ if (err)
+ goto out;
+
+ err = crypto_shash_digest(desc, ikm, ikmlen, prk);
+out:
+ shash_desc_zero(desc);
+ return err;
+}
+
+/*
+ * HKDF-Expand (RFC-5869 section 2.3). This expands the pseudorandom key, which
+ * has already been keyed into 'hmac_tfm', into 'okmlen' bytes of output keying
+ * material, parameterized by the application-specific information string of
+ * 'info' prefixed with the 'context' byte. ('context' isn't part of the HKDF
+ * specification; it's just a prefix we add to our application-specific info
+ * strings to guarantee that we don't accidentally repeat an info string when
+ * using HKDF for different purposes.)
+ */
+static int hkdf_expand(struct crypto_shash *hmac_tfm, u8 context,
+ const u8 *info, unsigned int infolen,
+ u8 *okm, unsigned int okmlen)
+{
+ SHASH_DESC_ON_STACK(desc, hmac_tfm);
+ int err;
+ const u8 *prev = NULL;
+ unsigned int i;
+ u8 counter = 1;
+ u8 tmp[HKDF_HASHLEN];
+
+ desc->tfm = hmac_tfm;
+ desc->flags = 0;
+
+ if (unlikely(okmlen > 255 * HKDF_HASHLEN))
+ return -EINVAL;
+
+ for (i = 0; i < okmlen; i += HKDF_HASHLEN) {
+
+ err = crypto_shash_init(desc);
+ if (err)
+ goto out;
+
+ if (prev) {
+ err = crypto_shash_update(desc, prev, HKDF_HASHLEN);
+ if (err)
+ goto out;
+ }
+
+ err = crypto_shash_update(desc, &context, 1);
+ if (err)
+ goto out;
+
+ err = crypto_shash_update(desc, info, infolen);
+ if (err)
+ goto out;
+
+ if (okmlen - i < HKDF_HASHLEN) {
+ err = crypto_shash_finup(desc, &counter, 1, tmp);
+ if (err)
+ goto out;
+ memcpy(&okm[i], tmp, okmlen - i);
+ memzero_explicit(tmp, sizeof(tmp));
+ } else {
+ err = crypto_shash_finup(desc, &counter, 1, &okm[i]);
+ if (err)
+ goto out;
+ }
+ counter++;
+ prev = &okm[i];
+ }
+ err = 0;
+out:
+ shash_desc_zero(desc);
+ return err;
+}
+
+/*
+ * Precompute HKDF-Extract using the master key as the input key material, then
+ * return an HMAC transform that is keyed using the resulting pseudorandom key.
+ * This can be used to derive further key material using HKDF-Expand.
+ */
+static struct crypto_shash *allocate_hmac_tfm(const u8 *master_key, u32 size)
+{
+ struct crypto_shash *hmac_tfm;
+ u8 prk[HKDF_HASHLEN];
+ int err;
+
+ hmac_tfm = crypto_alloc_shash(HKDF_HMAC_ALG, 0, 0);
+ if (IS_ERR(hmac_tfm)) {
+ pr_warn("fscrypt: error allocating " HKDF_HMAC_ALG ": %ld\n",
+ PTR_ERR(hmac_tfm));
+ goto out;
+ }
+
+ BUG_ON(crypto_shash_digestsize(hmac_tfm) != sizeof(prk));
+
+ err = hkdf_extract(hmac_tfm, master_key, size, prk);
+ if (err)
+ goto fail;
+
+ err = crypto_shash_setkey(hmac_tfm, prk, sizeof(prk));
+ if (err)
+ goto fail;
+out:
+ memzero_explicit(prk, sizeof(prk));
+ return hmac_tfm;
+
+fail:
+ crypto_free_shash(hmac_tfm);
+ hmac_tfm = ERR_PTR(err);
+ goto out;
+}

/*
* fscrypt_master_key_secret - secret key material of an in-use master key
@@ -914,6 +1090,8 @@ static void put_crypt_info(struct fscrypt_info *ci)
kmem_cache_free(fscrypt_info_cachep, ci);
}

+static struct crypto_shash *essiv_hash_tfm;
+
static int derive_essiv_salt(const u8 *key, int keysize, u8 *salt)
{
struct crypto_shash *tfm = READ_ONCE(essiv_hash_tfm);
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:49

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 16/25] fscrypt: implement basic handling of v2 encryption policies

From: Eric Biggers <[email protected]>

Update the fscrypt internals to handle v2 encryption policies. This
includes supporting getting and setting them, translating them to/from
the on-disk fscrypt_context. It also includes storing either a v1 or v2
policy struct in the fscrypt_info for use by fscrypt_inherit_context()
and fscrypt_has_permitted_context(). (Previously we were storing the
individual fields, but it is a bit easier to store a policy struct.)

An fscrypt_policy_v1 (previously 'fscrypt_policy') maps to/from an
fscrypt_context_v1 (previously 'fscrypt_context'), while an
fscrypt_policy_v2 maps to/from an fscrypt_context_v2.

Key management for v2 policies will be implemented by later patches.
For now, attempting to set up an inode's key just fails with EOPNOTSUPP.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/crypto/fname.c | 4 +-
fs/crypto/fscrypt_private.h | 172 ++++++++++++++++---
fs/crypto/keyinfo.c | 70 ++++----
fs/crypto/policy.c | 368 ++++++++++++++++++++++++++++------------
include/linux/fscrypt.h | 2 +-
include/linux/fscrypt_notsupp.h | 6 +
include/linux/fscrypt_supp.h | 1 +
7 files changed, 452 insertions(+), 171 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index c91bcef65b9f..78dc0e3f6328 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -46,7 +46,7 @@ static int fname_encrypt(struct inode *inode,
int res = 0;
char iv[FS_CRYPTO_BLOCK_SIZE];
struct scatterlist sg;
- int padding = 4 << (ci->ci_flags & FSCRYPT_POLICY_FLAGS_PAD_MASK);
+ int padding = fscrypt_policy_fname_padding(&ci->ci_policy);
unsigned int lim;
unsigned int cryptlen;

@@ -217,7 +217,7 @@ u32 fscrypt_fname_encrypted_size(const struct inode *inode, u32 ilen)
struct fscrypt_info *ci = inode->i_crypt_info;

if (ci)
- padding = 4 << (ci->ci_flags & FSCRYPT_POLICY_FLAGS_PAD_MASK);
+ padding = fscrypt_policy_fname_padding(&ci->ci_policy);
ilen = max(ilen, (u32)FS_CRYPTO_BLOCK_SIZE);
return round_up(ilen, padding);
}
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 2fdc4e5c0771..dec85c4b14a8 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -29,39 +29,159 @@

#define FSCRYPT_MIN_KEY_SIZE 16

-/**
- * Encryption context for inode
- *
- * Protector format:
- * 1 byte: Protector format (1 = this version)
- * 1 byte: File contents encryption mode
- * 1 byte: File names encryption mode
- * 1 byte: Flags
- * 8 bytes: Master Key descriptor
- * 16 bytes: Encryption Key derivation nonce
- */
-struct fscrypt_context {
- u8 format;
+struct fscrypt_context_v1 {
+
+ u8 version; /* FSCRYPT_CONTEXT_V1 */
+
+ /* Same meaning as in v2 context --- see below */
u8 contents_encryption_mode;
u8 filenames_encryption_mode;
u8 flags;
+
+ /*
+ * Descriptor for this file's master key in a process-subscribed keyring
+ * --- typically a session keyring, or a user keyring linked into a
+ * session or user session keyring. This is an arbitrary value, chosen
+ * by userspace when it set the encryption policy. It is *not*
+ * necessarily tied to the actual key payload.
+ */
u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+
+ /*
+ * A unique value used in combination with the master key to derive the
+ * file's actual encryption key
+ */
u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
-} __packed;
+};
+
+struct fscrypt_context_v2 {
+
+ u8 version; /* FSCRYPT_CONTEXT_V2 */
+
+ /* Encryption mode for the contents of regular files */
+ u8 contents_encryption_mode;

-#define FS_ENCRYPTION_CONTEXT_FORMAT_V1 1
+ /* Encryption mode for filenames in directories and symlink targets */
+ u8 filenames_encryption_mode;
+
+ /* Options that affect how encryption is done (e.g. padding amount) */
+ u8 flags;
+
+ /* Reserved, must be 0 */
+ u8 reserved[4];
+
+ /*
+ * A cryptographic hash of the master key with which this file is
+ * encrypted
+ */
+ u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+
+ /*
+ * A unique value used in combination with the master key to derive the
+ * file's actual encryption key
+ */
+ u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
+};
+
+/**
+ * fscrypt_context - the encryption context for an inode
+ *
+ * Filesystems usually store this in an extended attribute. It identifies the
+ * encryption algorithm and key with which the file is encrypted.
+ *
+ * Since this is stored on-disk, be careful not to reorder fields or add any
+ * implicit padding bytes!
+ */
+union fscrypt_context {
+ struct fscrypt_context_v1 v1;
+ struct fscrypt_context_v2 v2;
+};
+
+#define FSCRYPT_CONTEXT_V1 1
+#define FSCRYPT_CONTEXT_V2 2
+
+static inline int fscrypt_context_size(const union fscrypt_context *ctx)
+{
+ switch (ctx->v1.version) {
+ case FSCRYPT_CONTEXT_V1:
+ return sizeof(ctx->v1);
+ case FSCRYPT_CONTEXT_V2:
+ return sizeof(ctx->v2);
+ }
+ return 0;
+}
+
+static inline bool
+fscrypt_valid_context_format(const union fscrypt_context *ctx, int size)
+{
+ return size >= 1 && size == fscrypt_context_size(ctx);
+}
+
+#undef fscrypt_policy
+union fscrypt_policy {
+ struct fscrypt_policy_v1 v1;
+ struct fscrypt_policy_v2 v2;
+};
+
+static inline int fscrypt_policy_size(const union fscrypt_policy *policy)
+{
+ switch (policy->v1.version) {
+ case FSCRYPT_POLICY_VERSION_LEGACY:
+ return sizeof(policy->v1);
+ case FSCRYPT_POLICY_VERSION_2:
+ return sizeof(policy->v2);
+ }
+ return 0;
+}
+
+static inline u8
+fscrypt_policy_contents_mode(const union fscrypt_policy *policy)
+{
+ switch (policy->v1.version) {
+ case FSCRYPT_POLICY_VERSION_LEGACY:
+ return policy->v1.contents_encryption_mode;
+ case FSCRYPT_POLICY_VERSION_2:
+ return policy->v2.contents_encryption_mode;
+ }
+ BUG();
+}
+
+static inline u8
+fscrypt_policy_fnames_mode(const union fscrypt_policy *policy)
+{
+ switch (policy->v1.version) {
+ case FSCRYPT_POLICY_VERSION_LEGACY:
+ return policy->v1.filenames_encryption_mode;
+ case FSCRYPT_POLICY_VERSION_2:
+ return policy->v2.filenames_encryption_mode;
+ }
+ BUG();
+}
+
+static inline int
+fscrypt_policy_fname_padding(const union fscrypt_policy *policy)
+{
+ switch (policy->v1.version) {
+ case FSCRYPT_POLICY_VERSION_LEGACY:
+ return 4 << (policy->v1.flags & FSCRYPT_POLICY_FLAGS_PAD_MASK);
+ case FSCRYPT_POLICY_VERSION_2:
+ return 4 << (policy->v2.flags & FSCRYPT_POLICY_FLAGS_PAD_MASK);
+ }
+ BUG();
+}

/*
- * A pointer to this structure is stored in the file system's in-core
- * representation of an inode.
+ * fscrypt_info - the "encryption key" for an inode
+ *
+ * When an encrypted file's key is made available, an instance of this struct is
+ * allocated and stored in ->i_crypt_info. Once created, it remains until the
+ * inode is evicted.
*/
struct fscrypt_info {
- u8 ci_data_mode;
- u8 ci_filename_mode;
- u8 ci_flags;
+
+ /* The actual crypto transforms needed for encryption and decryption */
struct crypto_skcipher *ci_ctfm;
struct crypto_cipher *ci_essiv_tfm;
- u8 ci_master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];

/*
* The master key with which this inode was unlocked (decrypted). This
@@ -78,6 +198,9 @@ struct fscrypt_info {
* ->ci_master_key is set.
*/
struct inode *ci_inode;
+
+ /* The encryption policy used by this file */
+ union fscrypt_policy ci_policy;
};

typedef enum {
@@ -114,4 +237,11 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
extern struct key_type key_type_fscrypt_mk;
extern void __exit fscrypt_essiv_cleanup(void);

+/* policy.c */
+extern bool fscrypt_policies_equal(const union fscrypt_policy *policy1,
+ const union fscrypt_policy *policy2);
+extern bool fscrypt_supported_policy(const union fscrypt_policy *policy_u);
+extern void fscrypt_context_to_policy(const union fscrypt_context *ctx_u,
+ union fscrypt_policy *policy_u);
+
#endif /* _FSCRYPT_PRIVATE_H */
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 4052030a4c96..937a678ebba1 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -649,7 +649,7 @@ static void derive_crypt_complete(struct crypto_async_request *req, int rc)
* key is longer, then only the first 'derived_keysize' bytes are used.
*/
static int derive_key_aes(const u8 *master_key,
- const struct fscrypt_context *ctx,
+ const struct fscrypt_context_v1 *ctx,
u8 *derived_key, unsigned int derived_keysize)
{
int err;
@@ -751,7 +751,7 @@ find_and_lock_process_key(const char *prefix,
}

static int find_and_derive_key_legacy(const struct inode *inode,
- const struct fscrypt_context *ctx,
+ const struct fscrypt_context_v1 *ctx,
u8 *derived_key,
unsigned int derived_keysize)
{
@@ -786,7 +786,7 @@ static int find_and_derive_key_legacy(const struct inode *inode,
* its fscrypt_info into ->mk_decrypted_inodes.
*/
static int find_and_derive_key(const struct inode *inode,
- const struct fscrypt_context *ctx,
+ const union fscrypt_context *ctx,
u8 *derived_key, unsigned int derived_keysize,
struct key **master_key_ret)
{
@@ -795,9 +795,15 @@ static int find_and_derive_key(const struct inode *inode,
struct fscrypt_key_specifier mk_spec;
int err;

- mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR;
- memcpy(mk_spec.descriptor, ctx->master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
+ switch (ctx->v1.version) {
+ case FSCRYPT_CONTEXT_V1:
+ mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR;
+ memcpy(mk_spec.descriptor, ctx->v1.master_key_descriptor,
+ FSCRYPT_KEY_DESCRIPTOR_SIZE);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }

key = find_master_key(inode->i_sb, &mk_spec);
if (IS_ERR(key)) {
@@ -807,7 +813,7 @@ static int find_and_derive_key(const struct inode *inode,
* As a legacy fallback, we search the current task's subscribed
* keyrings in addition to ->s_master_keys.
*/
- return find_and_derive_key_legacy(inode, ctx, derived_key,
+ return find_and_derive_key_legacy(inode, &ctx->v1, derived_key,
derived_keysize);
}
mk = key->payload.data[0];
@@ -833,7 +839,7 @@ static int find_and_derive_key(const struct inode *inode,
goto out_release_key;
}

- err = derive_key_aes(mk->mk_secret.raw, ctx,
+ err = derive_key_aes(mk->mk_secret.raw, &ctx->v1,
derived_key, derived_keysize);
if (err)
goto out_release_key;
@@ -862,17 +868,10 @@ static int determine_cipher_type(struct fscrypt_info *ci, struct inode *inode,
{
u32 mode;

- if (!fscrypt_valid_enc_modes(ci->ci_data_mode, ci->ci_filename_mode)) {
- pr_warn_ratelimited("fscrypt: inode %lu uses unsupported encryption modes (contents mode %d, filenames mode %d)\n",
- inode->i_ino,
- ci->ci_data_mode, ci->ci_filename_mode);
- return -EINVAL;
- }
-
if (S_ISREG(inode->i_mode)) {
- mode = ci->ci_data_mode;
+ mode = fscrypt_policy_contents_mode(&ci->ci_policy);
} else if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode)) {
- mode = ci->ci_filename_mode;
+ mode = fscrypt_policy_fnames_mode(&ci->ci_policy);
} else {
WARN_ONCE(1, "fscrypt: filesystem tried to load encryption info for inode %lu, which is not encryptable (file type %d)\n",
inode->i_ino, (inode->i_mode & S_IFMT));
@@ -984,7 +983,7 @@ void __exit fscrypt_essiv_cleanup(void)
int fscrypt_get_encryption_info(struct inode *inode)
{
struct fscrypt_info *crypt_info;
- struct fscrypt_context ctx;
+ union fscrypt_context ctx;
struct crypto_skcipher *ctfm;
const char *cipher_str;
unsigned int derived_keysize;
@@ -1006,33 +1005,31 @@ int fscrypt_get_encryption_info(struct inode *inode)
return res;
/* Fake up a context for an unencrypted directory */
memset(&ctx, 0, sizeof(ctx));
- ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
- ctx.contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
- ctx.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
- memset(ctx.master_key_descriptor, 0x42,
+ ctx.v1.version = FSCRYPT_CONTEXT_V1;
+ ctx.v1.contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
+ ctx.v1.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
+ memset(ctx.v1.master_key_descriptor, 0x42,
FSCRYPT_KEY_DESCRIPTOR_SIZE);
- } else if (res != sizeof(ctx)) {
- return -EINVAL;
+ res = sizeof(ctx.v1);
}

- if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
- return -EINVAL;
-
- if (ctx.flags & ~FSCRYPT_POLICY_FLAGS_VALID)
+ if (!fscrypt_valid_context_format(&ctx, res))
return -EINVAL;

crypt_info = kmem_cache_zalloc(fscrypt_info_cachep, GFP_NOFS);
if (!crypt_info)
return -ENOMEM;

- crypt_info->ci_flags = ctx.flags;
- crypt_info->ci_data_mode = ctx.contents_encryption_mode;
- crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
- memcpy(crypt_info->ci_master_key_descriptor, ctx.master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
+ fscrypt_context_to_policy(&ctx, &crypt_info->ci_policy);
+ if (!fscrypt_supported_policy(&crypt_info->ci_policy)) {
+ res = -EINVAL;
+ pr_warn_ratelimited("fscrypt: inode %lu uses unsupported encryption policy\n",
+ inode->i_ino);
+ goto out;
+ }

- res = determine_cipher_type(crypt_info, inode,
- &cipher_str, &derived_keysize);
+ res = determine_cipher_type(crypt_info, inode, &cipher_str,
+ &derived_keysize);
if (res)
goto out;

@@ -1065,7 +1062,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
goto out;

if (S_ISREG(inode->i_mode) &&
- crypt_info->ci_data_mode == FSCRYPT_MODE_AES_128_CBC) {
+ (fscrypt_policy_contents_mode(&crypt_info->ci_policy) ==
+ FSCRYPT_MODE_AES_128_CBC)) {
res = init_essiv_generator(crypt_info, derived_key,
derived_keysize);
if (res) {
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index a856c8941be6..27a391038f73 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -6,6 +6,7 @@
*
* Written by Michael Halcrow, 2015.
* Modified by Jaegeuk Kim, 2015.
+ * Modified by Eric Biggers, 2017 for v2 policy support.
*/

#include <linux/random.h>
@@ -13,84 +14,227 @@
#include <linux/mount.h>
#include "fscrypt_private.h"

-/*
- * check whether an encryption policy is consistent with an encryption context
- */
-static bool is_encryption_context_consistent_with_policy(
- const struct fscrypt_context *ctx,
- const struct fscrypt_policy *policy)
+bool fscrypt_policies_equal(const union fscrypt_policy *policy1,
+ const union fscrypt_policy *policy2)
+{
+ if (policy1->v1.version != policy2->v1.version)
+ return false;
+
+ return !memcmp(policy1, policy2, fscrypt_policy_size(policy1));
+}
+
+bool fscrypt_supported_policy(const union fscrypt_policy *policy_u)
+{
+ switch (policy_u->v1.version) {
+ case FSCRYPT_POLICY_VERSION_LEGACY: {
+ const struct fscrypt_policy_v1 *policy = &policy_u->v1;
+
+ if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
+ policy->filenames_encryption_mode))
+ return false;
+
+ if (policy->flags & ~FSCRYPT_POLICY_FLAGS_VALID)
+ return false;
+
+ return true;
+ }
+ case FSCRYPT_POLICY_VERSION_2: {
+ const struct fscrypt_policy_v2 *policy = &policy_u->v2;
+
+ if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
+ policy->filenames_encryption_mode))
+ return false;
+
+ if (policy->flags & ~FSCRYPT_POLICY_FLAGS_VALID)
+ return false;
+
+ if (memchr_inv(policy->reserved, 0, sizeof(policy->reserved)))
+ return false;
+
+ return true;
+ }
+ }
+ return false;
+}
+
+static void fscrypt_policy_to_context(const union fscrypt_policy *policy_u,
+ union fscrypt_context *ctx_u)
{
- return memcmp(ctx->master_key_descriptor, policy->master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
- (ctx->flags == policy->flags) &&
- (ctx->contents_encryption_mode ==
- policy->contents_encryption_mode) &&
- (ctx->filenames_encryption_mode ==
- policy->filenames_encryption_mode);
+ memset(ctx_u, 0, sizeof(*ctx_u));
+
+ switch (policy_u->v1.version) {
+ case FSCRYPT_POLICY_VERSION_LEGACY: {
+ const struct fscrypt_policy_v1 *policy = &policy_u->v1;
+ struct fscrypt_context_v1 *ctx = &ctx_u->v1;
+
+ ctx->version = FSCRYPT_CONTEXT_V1;
+ ctx->contents_encryption_mode =
+ policy->contents_encryption_mode;
+ ctx->filenames_encryption_mode =
+ policy->filenames_encryption_mode;
+ ctx->flags = policy->flags;
+ memcpy(ctx->master_key_descriptor,
+ policy->master_key_descriptor,
+ sizeof(ctx->master_key_descriptor));
+ get_random_bytes(ctx->nonce, sizeof(ctx->nonce));
+ break;
+ }
+ case FSCRYPT_POLICY_VERSION_2: {
+ const struct fscrypt_policy_v2 *policy = &policy_u->v2;
+ struct fscrypt_context_v2 *ctx = &ctx_u->v2;
+
+ ctx->version = FSCRYPT_CONTEXT_V2;
+ ctx->contents_encryption_mode =
+ policy->contents_encryption_mode;
+ ctx->filenames_encryption_mode =
+ policy->filenames_encryption_mode;
+ ctx->flags = policy->flags;
+ memcpy(ctx->reserved, policy->reserved, sizeof(ctx->reserved));
+ memcpy(ctx->master_key_identifier,
+ policy->master_key_identifier,
+ sizeof(ctx->master_key_identifier));
+ get_random_bytes(ctx->nonce, sizeof(ctx->nonce));
+ break;
+ }
+ default:
+ BUG();
+ }
}

-static int create_encryption_context_from_policy(struct inode *inode,
- const struct fscrypt_policy *policy)
+void fscrypt_context_to_policy(const union fscrypt_context *ctx_u,
+ union fscrypt_policy *policy_u)
{
- struct fscrypt_context ctx;
+ memset(policy_u, 0, sizeof(*policy_u));
+
+ switch (ctx_u->v1.version) {
+ case FSCRYPT_CONTEXT_V1: {
+ const struct fscrypt_context_v1 *ctx = &ctx_u->v1;
+ struct fscrypt_policy_v1 *policy = &policy_u->v1;
+
+ policy->version = FSCRYPT_POLICY_VERSION_LEGACY;
+ policy->contents_encryption_mode =
+ ctx->contents_encryption_mode;
+ policy->filenames_encryption_mode =
+ ctx->filenames_encryption_mode;
+ policy->flags = ctx->flags;
+ memcpy(policy->master_key_descriptor,
+ ctx->master_key_descriptor,
+ sizeof(policy->master_key_descriptor));
+ return;
+ }
+ case FSCRYPT_CONTEXT_V2: {
+ const struct fscrypt_context_v2 *ctx = &ctx_u->v2;
+ struct fscrypt_policy_v2 *policy = &policy_u->v2;
+
+ policy->version = FSCRYPT_POLICY_VERSION_2;
+ policy->contents_encryption_mode =
+ ctx->contents_encryption_mode;
+ policy->filenames_encryption_mode =
+ ctx->filenames_encryption_mode;
+ policy->flags = ctx->flags;
+ memcpy(policy->reserved, ctx->reserved,
+ sizeof(policy->reserved));
+ memcpy(policy->master_key_identifier,
+ ctx->master_key_identifier,
+ sizeof(policy->master_key_identifier));
+ return;
+ }
+ default:
+ BUG();
+ }
+}

- ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
- memcpy(ctx.master_key_descriptor, policy->master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
+static int fscrypt_get_policy(struct inode *inode, union fscrypt_policy *policy)
+{
+ union fscrypt_context ctx;
+ int ret;
+
+ if (inode->i_crypt_info) {
+ *policy = inode->i_crypt_info->ci_policy;
+ return 0;
+ }
+
+ if (!IS_ENCRYPTED(inode))
+ return -ENODATA;

- if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
- policy->filenames_encryption_mode))
+ ret = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
+ if (ret < 0)
+ return (ret == -ERANGE) ? -EINVAL : ret;
+ if (!fscrypt_valid_context_format(&ctx, ret))
return -EINVAL;
+ fscrypt_context_to_policy(&ctx, policy);
+ return 0;
+}
+
+static int set_encryption_policy(struct inode *inode,
+ const union fscrypt_policy *policy)
+{
+ union fscrypt_context ctx;

- if (policy->flags & ~FSCRYPT_POLICY_FLAGS_VALID)
+ if (!fscrypt_supported_policy(policy))
return -EINVAL;

- ctx.contents_encryption_mode = policy->contents_encryption_mode;
- ctx.filenames_encryption_mode = policy->filenames_encryption_mode;
- ctx.flags = policy->flags;
- BUILD_BUG_ON(sizeof(ctx.nonce) != FS_KEY_DERIVATION_NONCE_SIZE);
- get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
+ fscrypt_policy_to_context(policy, &ctx);
+
+ if (policy->v1.version == FSCRYPT_POLICY_VERSION_LEGACY) {
+ /*
+ * The original encryption policy version provided no way of
+ * verifying that the correct master key was supplied, which was
+ * insecure in scenarios where multiple users have access to the
+ * same encrypted files (even just read-only access). The new
+ * encryption policy version fixes this and also implies use of
+ * an improved key derivation function and allows non-root users
+ * to securely remove keys. So as long as compatibility with
+ * old kernels isn't required, it is recommended to use the new
+ * policy version for all new encrypted directories.
+ */
+ pr_warn_once("%s (pid %d) is setting less secure v1 encryption policy; recommend upgrading to v2.\n",
+ current->comm, current->pid);
+ }

- return inode->i_sb->s_cop->set_context(inode, &ctx, sizeof(ctx), NULL);
+ return inode->i_sb->s_cop->set_context(inode, &ctx,
+ fscrypt_context_size(&ctx),
+ NULL);
}

int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
{
- struct fscrypt_policy policy;
+ union fscrypt_policy policy;
+ union fscrypt_policy existing_policy;
struct inode *inode = file_inode(filp);
+ int size;
int ret;
- struct fscrypt_context ctx;

- if (copy_from_user(&policy, arg, sizeof(policy)))
+ if (copy_from_user(&policy, arg, sizeof(u8)))
+ return -EFAULT;
+
+ size = fscrypt_policy_size(&policy);
+ if (size == 0)
+ return -EINVAL;
+
+ if (copy_from_user((u8 *)&policy + 1, arg + 1, size - 1))
return -EFAULT;

if (!inode_owner_or_capable(inode))
return -EACCES;

- if (policy.version != 0)
- return -EINVAL;
-
ret = mnt_want_write_file(filp);
if (ret)
return ret;

inode_lock(inode);

- ret = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
+ ret = fscrypt_get_policy(inode, &existing_policy);
if (ret == -ENODATA) {
if (!S_ISDIR(inode->i_mode))
ret = -ENOTDIR;
else if (!inode->i_sb->s_cop->empty_dir(inode))
ret = -ENOTEMPTY;
else
- ret = create_encryption_context_from_policy(inode,
- &policy);
- } else if (ret == sizeof(ctx) &&
- is_encryption_context_consistent_with_policy(&ctx,
- &policy)) {
- /* The file already uses the same encryption policy. */
- ret = 0;
- } else if (ret >= 0 || ret == -ERANGE) {
+ ret = set_encryption_policy(inode, &policy);
+ } else if (ret == -EINVAL ||
+ (ret == 0 && !fscrypt_policies_equal(&policy,
+ &existing_policy))) {
/* The file already uses a different encryption policy. */
ret = -EEXIST;
}
@@ -102,36 +246,61 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
}
EXPORT_SYMBOL(fscrypt_ioctl_set_policy);

+/* Original ioctl version; can only get the original policy version */
int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg)
{
- struct inode *inode = file_inode(filp);
- struct fscrypt_context ctx;
- struct fscrypt_policy policy;
- int res;
+ union fscrypt_policy policy;
+ int err;

- if (!IS_ENCRYPTED(inode))
- return -ENODATA;
+ err = fscrypt_get_policy(file_inode(filp), &policy);
+ if (err)
+ return err;

- res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
- if (res < 0 && res != -ERANGE)
- return res;
- if (res != sizeof(ctx))
+ if (policy.v1.version != FSCRYPT_POLICY_VERSION_LEGACY)
return -EINVAL;
- if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
+
+ if (copy_to_user(arg, &policy, sizeof(policy.v1)))
+ return -EFAULT;
+ return 0;
+}
+EXPORT_SYMBOL(fscrypt_ioctl_get_policy);
+
+/* Extended ioctl version; can get policies of any version */
+int fscrypt_ioctl_get_policy_ex(struct file *filp, void __user *_arg)
+{
+ struct fscrypt_get_policy_ex_args __user *arg = _arg;
+ __u64 size;
+ __u64 actual_size;
+ size_t policy_size;
+ union fscrypt_policy policy;
+ int err;
+
+ if (get_user(size, &arg->size))
+ return -EFAULT;
+
+ if (size <= offsetof(struct fscrypt_get_policy_ex_args, policy) ||
+ size >= 65536)
return -EINVAL;

- policy.version = 0;
- policy.contents_encryption_mode = ctx.contents_encryption_mode;
- policy.filenames_encryption_mode = ctx.filenames_encryption_mode;
- policy.flags = ctx.flags;
- memcpy(policy.master_key_descriptor, ctx.master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
+ err = fscrypt_get_policy(file_inode(filp), &policy);
+ if (err)
+ return err;
+
+ policy_size = fscrypt_policy_size(&policy);
+ actual_size = offsetof(struct fscrypt_get_policy_ex_args, policy) +
+ policy_size;

- if (copy_to_user(arg, &policy, sizeof(policy)))
+ if (size < actual_size)
+ return -EOVERFLOW;
+
+ if (put_user(actual_size, &arg->size))
+ return -EFAULT;
+
+ if (copy_to_user(&arg->policy, &policy, policy_size))
return -EFAULT;
return 0;
}
-EXPORT_SYMBOL(fscrypt_ioctl_get_policy);
+EXPORT_SYMBOL(fscrypt_ioctl_get_policy_ex);

/**
* fscrypt_has_permitted_context() - is a file's encryption policy permitted
@@ -155,10 +324,8 @@ EXPORT_SYMBOL(fscrypt_ioctl_get_policy);
*/
int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
{
- const struct fscrypt_operations *cops = parent->i_sb->s_cop;
- const struct fscrypt_info *parent_ci, *child_ci;
- struct fscrypt_context parent_ctx, child_ctx;
- int res;
+ union fscrypt_policy parent_policy, child_policy;
+ int err;

/* No restrictions on file types which are never encrypted */
if (!S_ISREG(child->i_mode) && !S_ISDIR(child->i_mode) &&
@@ -188,41 +355,22 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
* In any case, if an unexpected error occurs, fall back to "forbidden".
*/

- res = fscrypt_get_encryption_info(parent);
- if (res)
+ err = fscrypt_get_encryption_info(parent);
+ if (err)
return 0;
- res = fscrypt_get_encryption_info(child);
- if (res)
+ err = fscrypt_get_encryption_info(child);
+ if (err)
return 0;
- parent_ci = parent->i_crypt_info;
- child_ci = child->i_crypt_info;
-
- if (parent_ci && child_ci) {
- return memcmp(parent_ci->ci_master_key_descriptor,
- child_ci->ci_master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
- (parent_ci->ci_data_mode == child_ci->ci_data_mode) &&
- (parent_ci->ci_filename_mode ==
- child_ci->ci_filename_mode) &&
- (parent_ci->ci_flags == child_ci->ci_flags);
- }

- res = cops->get_context(parent, &parent_ctx, sizeof(parent_ctx));
- if (res != sizeof(parent_ctx))
+ err = fscrypt_get_policy(parent, &parent_policy);
+ if (err)
return 0;

- res = cops->get_context(child, &child_ctx, sizeof(child_ctx));
- if (res != sizeof(child_ctx))
+ err = fscrypt_get_policy(child, &child_policy);
+ if (err)
return 0;

- return memcmp(parent_ctx.master_key_descriptor,
- child_ctx.master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
- (parent_ctx.contents_encryption_mode ==
- child_ctx.contents_encryption_mode) &&
- (parent_ctx.filenames_encryption_mode ==
- child_ctx.filenames_encryption_mode) &&
- (parent_ctx.flags == child_ctx.flags);
+ return fscrypt_policies_equal(&parent_policy, &child_policy);
}
EXPORT_SYMBOL(fscrypt_has_permitted_context);

@@ -238,30 +386,28 @@ EXPORT_SYMBOL(fscrypt_has_permitted_context);
int fscrypt_inherit_context(struct inode *parent, struct inode *child,
void *fs_data, bool preload)
{
- struct fscrypt_context ctx;
- struct fscrypt_info *ci;
- int res;
+ int err;
+ const struct fscrypt_info *ci;
+ union fscrypt_context ctx;

- res = fscrypt_get_encryption_info(parent);
- if (res < 0)
- return res;
+ err = fscrypt_get_encryption_info(parent);
+ if (err)
+ return err;

ci = parent->i_crypt_info;
if (ci == NULL)
return -ENOKEY;

- ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
- ctx.contents_encryption_mode = ci->ci_data_mode;
- ctx.filenames_encryption_mode = ci->ci_filename_mode;
- ctx.flags = ci->ci_flags;
- memcpy(ctx.master_key_descriptor, ci->ci_master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
- get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
BUILD_BUG_ON(sizeof(ctx) != FSCRYPT_SET_CONTEXT_MAX_SIZE);
- res = parent->i_sb->s_cop->set_context(child, &ctx,
- sizeof(ctx), fs_data);
- if (res)
- return res;
+
+ fscrypt_policy_to_context(&ci->ci_policy, &ctx);
+
+ err = parent->i_sb->s_cop->set_context(child, &ctx,
+ fscrypt_context_size(&ctx),
+ fs_data);
+ if (err)
+ return err;
+
return preload ? fscrypt_get_encryption_info(child): 0;
}
EXPORT_SYMBOL(fscrypt_inherit_context);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 671ce57e4673..aa8c6e8bfed8 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -86,7 +86,7 @@ struct fscrypt_operations {
};

/* Maximum value for the third parameter of fscrypt_operations.set_context(). */
-#define FSCRYPT_SET_CONTEXT_MAX_SIZE 28
+#define FSCRYPT_SET_CONTEXT_MAX_SIZE 40

static inline bool fscrypt_dummy_context_enabled(struct inode *inode)
{
diff --git a/include/linux/fscrypt_notsupp.h b/include/linux/fscrypt_notsupp.h
index bd60f951b06a..41bd5b70e343 100644
--- a/include/linux/fscrypt_notsupp.h
+++ b/include/linux/fscrypt_notsupp.h
@@ -70,6 +70,12 @@ static inline int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg)
return -EOPNOTSUPP;
}

+static inline int fscrypt_ioctl_get_policy_ex(struct file *filp,
+ void __user *arg)
+{
+ return -EOPNOTSUPP;
+}
+
static inline int fscrypt_has_permitted_context(struct inode *parent,
struct inode *child)
{
diff --git a/include/linux/fscrypt_supp.h b/include/linux/fscrypt_supp.h
index ace278056dbe..a11b0b2d14b0 100644
--- a/include/linux/fscrypt_supp.h
+++ b/include/linux/fscrypt_supp.h
@@ -38,6 +38,7 @@ static inline void fscrypt_set_encrypted_dentry(struct dentry *dentry)
/* policy.c */
extern int fscrypt_ioctl_set_policy(struct file *, const void __user *);
extern int fscrypt_ioctl_get_policy(struct file *, void __user *);
+extern int fscrypt_ioctl_get_policy_ex(struct file *, void __user *);
extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
extern int fscrypt_inherit_context(struct inode *, struct inode *,
void *, bool);
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-23 21:40:53

by Eric Biggers

[permalink] [raw]
Subject: [RFC PATCH 20/25] fscrypt: allow unprivileged users to add/remove keys for v2 policies

From: Eric Biggers <[email protected]>

Having the FS_IOC_ADD_ENCRYPTION_KEY and FS_IOC_REMOVE_ENCRYPTION_KEY
ioctls be root-only is sufficient for some users of filesystem
encryption, e.g. Android and Chromium OS where all the encryption keys
are managed by a privileged process. However, it is not sufficient for
general use where non-root users are setting up encrypted directories.
If these ioctls were root-only, such users would have to continue to use
process-subscribed keyrings and would continue to run into all the
problems noted earlier, including visibility problems when processes
running under different UIDs need to be able to access the files, and
the inability to remove the key, "locking" the directory.

Fortunately, we can indeed make the ioctls unprivileged, but only for v2
encryption policies and only after a few additional changes which this
patch implements.

First, to allow any user to add a key with filesystem-level visibility,
the keys must be identified using a cryptographic hash so that users
cannot add the wrong key for other users' files. We use the
key_identifier for this, which is why v2 encryption policies are a
requirement.

Second, we charge each key a user adds to their quota for the keyrings
service. Thus, a user can't cause a denial of service by adding a very
large number of keys.

Third, we have to be careful about when a key is allowed to be removed,
given that multiple users may add the same key (although that should
*not* normally be the case; it's astronomically unlikely for keys to
collide by chance, so it should only happen as a result of explicit
sharing or compromise). One might consider only allowing the first user
who added a key to remove it, or allowing any user who knows a key to
remove it. But neither of those are good enough because we don't want a
user on the system who knows another user's key to be able to cause a
denial of service where the former user removes the latter user's key at
an inopportune time. After all, it *should* be the case that if you
have an encrypted directory and you give everyone in the world the key,
including malicious users on the same system, it should still be no less
secure than *not* using encryption.

The solution is to keep track of which users have added a key and only
really remove the key once all users have removed it.

However, it is tolerated that a user will be unable to remove a key,
i.e. unable to "lock" their encrypted files, if another user has added
the same key. But in a sense, this is actually a good thing because it
will avoid providing a false notion of security where a key appears to
have been "removed" when actually it's still in memory, available to any
attacker who compromises the operating system kernel.

Signed-off-by: Eric Biggers <[email protected]>
---
fs/crypto/crypto.c | 7 +
fs/crypto/fscrypt_private.h | 1 +
fs/crypto/keyinfo.c | 341 ++++++++++++++++++++++++++++++++++++++++---
include/uapi/linux/fscrypt.h | 11 +-
4 files changed, 338 insertions(+), 22 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 489c504ac20d..e57a13889689 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -469,8 +469,14 @@ static int __init fscrypt_init(void)
if (err)
goto fail_free_info;

+ err = register_key_type(&key_type_fscrypt_mk_user);
+ if (err)
+ goto fail_unregister_fscrypt_mk_type;
+
return 0;

+fail_unregister_fscrypt_mk_type:
+ unregister_key_type(&key_type_fscrypt_mk);
fail_free_info:
kmem_cache_destroy(fscrypt_info_cachep);
fail_free_ctx:
@@ -494,6 +500,7 @@ static void __exit fscrypt_exit(void)
kmem_cache_destroy(fscrypt_ctx_cachep);
kmem_cache_destroy(fscrypt_info_cachep);
unregister_key_type(&key_type_fscrypt_mk);
+ unregister_key_type(&key_type_fscrypt_mk_user);

fscrypt_essiv_cleanup();
}
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 6d420c9a85bd..d0a63086fa95 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -235,6 +235,7 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,

/* keyinfo.c */
extern struct key_type key_type_fscrypt_mk;
+extern struct key_type key_type_fscrypt_mk_user;
extern void __exit fscrypt_essiv_cleanup(void);

/* policy.c */
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index ec181c4eca56..1fe44983239a 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -246,9 +246,16 @@ struct fscrypt_master_key {
* FS_IOC_REMOVE_ENCRYPTION_KEY can be retried, or
* FS_IOC_ADD_ENCRYPTION_KEY can add the secret again.
*
- * Locking: protected by key->sem.
+ * Locking: protected by key->sem (outer) and mk_secret_sem (inner).
+ * The reason for two locks is that key->sem also protects modifying
+ * mk_users, which ranks it above the semaphore for the keyring key
+ * type, which is in turn above page faults (via keyring_read). But
+ * sometimes filesystems call fscrypt_get_encryption_info() from within
+ * a transaction, which ranks it below page faults. So we need a
+ * separate lock which protects *just* mk_secret, not also mk_users.
*/
struct fscrypt_master_key_secret mk_secret;
+ struct rw_semaphore mk_secret_sem;

/*
* For v1 policy keys: an arbitrary key descriptor which was assigned by
@@ -258,6 +265,19 @@ struct fscrypt_master_key {
*/
struct fscrypt_key_specifier mk_spec;

+ /*
+ * Keyring which contains a key of type 'key_type_fscrypt_mk_user' for
+ * each user who has added this key. Normally there would be just one
+ * user who adds a particular key, but it's possible that multiple users
+ * would add the same key, and we don't want to allow one user to remove
+ * it before the others want it removed too.
+ *
+ * This is NULL for v1 policy keys.
+ *
+ * Locking: protected by key->sem.
+ */
+ struct key *mk_users;
+
/*
* Length of ->mk_decrypted_inodes, plus one if mk_secret is present.
* Once this goes to 0, the master key is removed from ->s_master_keys.
@@ -314,6 +334,7 @@ static void move_master_key_secret(struct fscrypt_master_key_secret *dst,
static void free_master_key(struct fscrypt_master_key *mk)
{
wipe_master_key_secret(&mk->mk_secret);
+ key_put(mk->mk_users);
kzfree(mk);
}

@@ -353,9 +374,42 @@ struct key_type key_type_fscrypt_mk = {
.describe = fscrypt_key_describe,
};

+static int fscrypt_mk_user_key_instantiate(struct key *key,
+ struct key_preparsed_payload *prep)
+{
+ /*
+ * We just charge FSCRYPT_MAX_KEY_SIZE bytes to the user's key quota for
+ * each key, regardless of the exact key size. The amount of memory
+ * actually used is greater than the size of the raw key anyway.
+ */
+ return key_payload_reserve(key, FSCRYPT_MAX_KEY_SIZE);
+}
+
+static void fscrypt_mk_user_key_describe(const struct key *key,
+ struct seq_file *m)
+{
+ seq_puts(m, key->description);
+}
+
+/*
+ * Type of key in ->mk_users. Each key of this type represents a particular
+ * user who has added a particular master key.
+ *
+ * Note that the name of this key type really should be something like
+ * ".fscrypt-user" instead of simply ".fscrypt". But the shorter name is chosen
+ * mainly for simplicity of presentation in /proc/keys when read by a non-root
+ * user. And it is expected to be rare that a key is actually added by multiple
+ * users, since users should keep their encryption keys confidential.
+ */
+struct key_type key_type_fscrypt_mk_user = {
+ .name = ".fscrypt",
+ .instantiate = fscrypt_mk_user_key_instantiate,
+ .describe = fscrypt_mk_user_key_describe,
+};
+
/*
- * Search ->s_master_keys. Note that we mark the keyring reference as
- * "possessed" so that we can use the KEY_POS_SEARCH permission.
+ * Search either ->s_master_keys or ->mk_users. Note that we mark the keyring
+ * reference as "possessed" so that we can use the KEY_POS_SEARCH permission.
*/
static struct key *search_fscrypt_keyring(struct key *keyring,
struct key_type *type,
@@ -378,6 +432,13 @@ static struct key *search_fscrypt_keyring(struct key *keyring,

#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_IDENTIFIER_SIZE + 1)

+#define FSCRYPT_MK_USERS_DESCRIPTION_SIZE \
+ (sizeof("fscrypt-") - 1 + 2 * FSCRYPT_KEY_IDENTIFIER_SIZE + \
+ sizeof("-users") - 1 + 1)
+
+#define FSCRYPT_MK_USER_DESCRIPTION_SIZE \
+ (2 * FSCRYPT_KEY_IDENTIFIER_SIZE + sizeof(".uid.") - 1 + 10 + 1)
+
static void format_fs_keyring_description(
char description[FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE],
const struct super_block *sb)
@@ -393,6 +454,23 @@ static void format_mk_description(
master_key_spec_len(mk_spec), mk_spec->max_specifier);
}

+static void format_mk_users_keyring_description(
+ char description[FSCRYPT_MK_USERS_DESCRIPTION_SIZE],
+ const u8 mk_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+ sprintf(description, "fscrypt-%*phN-users",
+ FSCRYPT_KEY_IDENTIFIER_SIZE, mk_identifier);
+}
+
+static void format_mk_user_description(
+ char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE],
+ const u8 mk_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+
+ sprintf(description, "%*phN.uid.%u", FSCRYPT_KEY_IDENTIFIER_SIZE,
+ mk_identifier, __kuid_val(current_fsuid()));
+}
+
/*
* Find the specified master key in ->s_master_keys.
* Returns ERR_PTR(-ENOKEY) if not found.
@@ -413,6 +491,80 @@ static struct key *find_master_key(struct super_block *sb,
description);
}

+/*
+ * Find the current user's key in the master key's ->mk_users.
+ * Returns ERR_PTR(-ENOKEY) if not found.
+ */
+static struct key *find_master_key_user(struct fscrypt_master_key *mk)
+{
+ char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE];
+
+ format_mk_user_description(description, mk->mk_spec.identifier);
+ return search_fscrypt_keyring(mk->mk_users, &key_type_fscrypt_mk_user,
+ description);
+}
+
+static int allocate_master_key_users_keyring(struct fscrypt_master_key *mk)
+{
+ char description[FSCRYPT_MK_USERS_DESCRIPTION_SIZE];
+ struct key *mk_users;
+
+ format_mk_users_keyring_description(description,
+ mk->mk_spec.identifier);
+
+ mk_users = keyring_alloc(description, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+ current_cred(), KEY_POS_SEARCH |
+ KEY_USR_SEARCH | KEY_USR_READ | KEY_USR_VIEW,
+ KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
+ if (IS_ERR(mk_users))
+ return PTR_ERR(mk_users);
+
+ mk->mk_users = mk_users;
+ return 0;
+}
+
+/*
+ * Give the current user a key in ->mk_users. This charges the user's quota and
+ * marks the master key as added by the current user, so that it cannot be
+ * removed by another user with the key.
+ */
+static int add_master_key_user(struct fscrypt_master_key *mk)
+{
+ char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE];
+ struct key *mk_user;
+ int err;
+
+ format_mk_user_description(description, mk->mk_spec.identifier);
+ mk_user = key_alloc(&key_type_fscrypt_mk_user, description,
+ current_fsuid(), current_gid(), current_cred(),
+ KEY_POS_SEARCH | KEY_USR_VIEW, 0, NULL);
+ if (IS_ERR(mk_user))
+ return PTR_ERR(mk_user);
+
+ err = key_instantiate_and_link(mk_user, NULL, 0, mk->mk_users, NULL);
+ key_put(mk_user);
+ return err;
+}
+
+/*
+ * Remove the current user's key from ->mk_users, if present.
+ */
+static int remove_master_key_user(struct fscrypt_master_key *mk)
+{
+ struct key *mk_user;
+ int err;
+
+ mk_user = find_master_key_user(mk);
+ if (IS_ERR(mk_user)) {
+ if (mk_user != ERR_PTR(-ENOKEY))
+ return PTR_ERR(mk_user);
+ return 0;
+ }
+ err = key_unlink(mk->mk_users, mk_user);
+ key_put(mk_user);
+ return err;
+}
+
static struct key *
allocate_master_key(struct fscrypt_master_key_secret *secret,
const struct fscrypt_key_specifier *mk_spec)
@@ -429,16 +581,36 @@ allocate_master_key(struct fscrypt_master_key_secret *secret,
mk->mk_spec = *mk_spec;

move_master_key_secret(&mk->mk_secret, secret);
+ init_rwsem(&mk->mk_secret_sem);

refcount_set(&mk->mk_refcount, 1); /* secret is present */
INIT_LIST_HEAD(&mk->mk_decrypted_inodes);
spin_lock_init(&mk->mk_decrypted_inodes_lock);

+ if (mk_spec->type != FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR) {
+ err = allocate_master_key_users_keyring(mk);
+ if (err) {
+ key = ERR_PTR(err);
+ goto out_free_mk;
+ }
+
+ err = add_master_key_user(mk);
+ if (err) {
+ key = ERR_PTR(err);
+ goto out_free_mk;
+ }
+ }
+
+ /*
+ * Note that we don't charge this key to anyone's quota since it's owned
+ * by root, and the keys in ->mk_users are charged instead.
+ */
format_mk_description(description, mk_spec);
key = key_alloc(&key_type_fscrypt_mk, description,
GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
KEY_POS_SEARCH | KEY_USR_SEARCH |
- KEY_USR_READ | KEY_USR_VIEW, 0, NULL);
+ KEY_USR_READ | KEY_USR_VIEW,
+ KEY_ALLOC_NOT_IN_QUOTA, NULL);
if (IS_ERR(key))
goto out_free_mk;

@@ -510,12 +682,31 @@ static int add_master_key(struct super_block *sb,
goto out_put_key;
} else {
struct fscrypt_master_key *mk = key->payload.data[0];
+ struct key *mk_user;
bool rekey;

/* Found the key in ->s_master_keys */

down_write(&key->sem);

+ /*
+ * If the current user is already in ->mk_users, then there's
+ * nothing to do.
+ */
+ if (mk->mk_users) {
+ mk_user = find_master_key_user(mk);
+ if (mk_user != ERR_PTR(-ENOKEY)) {
+ up_write(&key->sem);
+ if (IS_ERR(mk_user)) {
+ err = PTR_ERR(mk_user);
+ } else {
+ key_put(mk_user);
+ err = 0;
+ }
+ goto out_put_key;
+ }
+ }
+
/*
* Take a reference if we'll be re-adding ->mk_secret. If we
* couldn't take a reference, then the key is being removed from
@@ -531,9 +722,24 @@ static int add_master_key(struct super_block *sb,
goto retry;
}

+ /* Add the current user to ->mk_users */
+ if (mk->mk_users) {
+ err = add_master_key_user(mk);
+ if (err) {
+ up_write(&key->sem);
+ if (rekey &&
+ refcount_dec_and_test(&mk->mk_refcount))
+ key_invalidate(key);
+ goto out_put_key;
+ }
+ }
+
/* Re-add the secret key material if needed */
- if (rekey)
+ if (rekey) {
+ down_write(&mk->mk_secret_sem);
move_master_key_secret(&mk->mk_secret, secret);
+ up_write(&mk->mk_secret_sem);
+ }
up_write(&key->sem);
}
err = 0;
@@ -548,6 +754,23 @@ static int add_master_key(struct super_block *sb,
* Add a master encryption key to the filesystem, causing all files which were
* encrypted with it to appear "unlocked" (decrypted) when accessed. The key
* can be removed later by FS_IOC_REMOVE_ENCRYPTION_KEY.
+ *
+ * When adding a key for use by v1 encryption policies, this ioctl is
+ * privileged, and userspace must provide the 'key_descriptor'.
+ *
+ * When adding a key for use by v2+ encryption policies, this ioctl is
+ * unprivileged. This is needed, in general, to allow non-root users to use
+ * encryption without encountering the visibility problems of process-subscribed
+ * keyrings and the inability to properly remove keys. This works by having
+ * each key identified by its cryptographically secure hash --- the
+ * 'key_identifier'. The cryptographic hash ensures that a malicious user
+ * cannot add the wrong key for a given identifier. Furthermore, each added key
+ * is charged to the appropriate user's quota for the keyrings service, which
+ * prevents a malicious user from adding too many keys. Finally, we forbid a
+ * user from removing a key while other users have added it too, which prevents
+ * a user who knows another user's key from causing a denial-of-service by
+ * removing it at an inopportune time. (We tolerate that a user who knows a key
+ * can prevent other users from removing it.)
*/
int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
{
@@ -571,9 +794,6 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
if (!valid_key_spec(&arg.key_spec))
return -EINVAL;

- if (!capable(CAP_SYS_ADMIN))
- return -EACCES;
-
memset(&secret, 0, sizeof(secret));
secret.size = arg.raw_size;
err = -EFAULT;
@@ -613,6 +833,15 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
arg.key_spec.identifier,
FSCRYPT_KEY_IDENTIFIER_SIZE))
goto out_wipe_secret;
+ } else {
+ /*
+ * Only root can add keys that are identified by an arbitrary
+ * descriptor rather than by a cryptographic hash --- since
+ * otherwise a malicious user could add the wrong key.
+ */
+ err = -EACCES;
+ if (!capable(CAP_SYS_ADMIN))
+ goto out_wipe_secret;
}

err = add_master_key(sb, &secret, &arg.key_spec);
@@ -744,7 +973,9 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
}

/*
- * Try to remove an fscrypt master encryption key.
+ * Try to remove an fscrypt master encryption key. If other users have also
+ * added the key, we'll remove the current user's usage of the key, then return
+ * -EUSERS. Otherwise we'll continue on and try to actually remove the key.
*
* First we wipe the actual master key secret from memory, so that no more
* inodes can be unlocked with it. Then, we try to evict all cached inodes that
@@ -775,13 +1006,33 @@ int fscrypt_ioctl_remove_key(struct file *filp, const void __user *uarg)
if (copy_from_user(&arg, uarg, sizeof(arg)))
return -EFAULT;

- if (memchr_inv(arg.reserved, 0, sizeof(arg.reserved)))
+ if (arg.flags & ~FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS)
+ return -EINVAL;
+
+ if (arg.reserved1)
+ return -EINVAL;
+
+ if (memchr_inv(arg.reserved2, 0, sizeof(arg.reserved2)))
return -EINVAL;

if (!valid_key_spec(&arg.key_spec))
return -EINVAL;

- if (!capable(CAP_SYS_ADMIN))
+ /*
+ * Only root can request that the key be removed no matter how many
+ * user(s) have added it.
+ */
+ if ((arg.flags & FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS) &&
+ !capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ /*
+ * Only root can remove keys that are identified by an arbitrary
+ * descriptor rather than by a cryptographic hash --- since only root
+ * can add such keys.
+ */
+ if (arg.key_spec.type != FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER &&
+ !capable(CAP_SYS_ADMIN))
return -EACCES;

key = find_master_key(sb, &arg.key_spec);
@@ -790,10 +1041,34 @@ int fscrypt_ioctl_remove_key(struct file *filp, const void __user *uarg)
mk = key->payload.data[0];

down_write(&key->sem);
+
+ if (mk->mk_users && mk->mk_users->keys.nr_leaves_on_tree != 0) {
+ if (arg.flags & FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS)
+ err = keyring_clear(mk->mk_users);
+ else
+ err = remove_master_key_user(mk);
+ if (err) {
+ up_write(&key->sem);
+ goto out_put_key;
+ }
+ if (mk->mk_users->keys.nr_leaves_on_tree != 0) {
+ /*
+ * Other users have still added the key too. We removed
+ * the current user's usage of the key if there was one,
+ * but we still can't remove the key itself.
+ */
+ err = -EUSERS;
+ up_write(&key->sem);
+ goto out_put_key;
+ }
+ }
+
dead = false;
if (is_master_key_secret_present(&mk->mk_secret)) {
+ down_write(&mk->mk_secret_sem);
wipe_master_key_secret(&mk->mk_secret);
dead = refcount_dec_and_test(&mk->mk_refcount);
+ up_write(&mk->mk_secret_sem);
}
up_write(&key->sem);
if (dead) {
@@ -802,6 +1077,7 @@ int fscrypt_ioctl_remove_key(struct file *filp, const void __user *uarg)
} else {
err = try_to_lock_encrypted_files(sb, mk);
}
+out_put_key:
key_put(key);
return err;
}
@@ -818,6 +1094,15 @@ EXPORT_SYMBOL_GPL(fscrypt_ioctl_remove_key);
* regular file in it (which can confuse the "incompletely removed" state with
* absent or present).
*
+ * In addition, for v2 policy keys we allow applications to determine, via
+ * ->status_flags and ->user_count, whether the key has been added by the
+ * current user, by other users, or by both. Most applications should not need
+ * this, since ordinarily only one user should know a given key. However, if a
+ * secret key is shared by multiple users, applications may wish to add an
+ * already-present key to prevent other users from removing it. This ioctl can
+ * be used to check whether that really is the case before the work is done to
+ * add the key --- which might e.g. require prompting the user for a passphrase.
+ *
* Note: this ioctl only works with keys added to the filesystem-level keyring.
* It does *not* work with keys added via the old mechanism which involved
* process-subscribed keyrings.
@@ -839,6 +1124,8 @@ int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
if (!valid_key_spec(&arg.key_spec))
return -EINVAL;

+ arg.status_flags = 0;
+ arg.user_count = 0;
arg.reserved2 = 0;
memset(arg.reserved3, 0, sizeof(arg.reserved3));

@@ -860,6 +1147,19 @@ int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
}

arg.status = FSCRYPT_KEY_STATUS_PRESENT;
+ if (mk->mk_users) {
+ struct key *mk_user;
+
+ arg.user_count = mk->mk_users->keys.nr_leaves_on_tree;
+ mk_user = find_master_key_user(mk);
+ if (!IS_ERR(mk_user)) {
+ arg.status_flags |= FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF;
+ key_put(mk_user);
+ } else if (mk_user != ERR_PTR(-ENOKEY)) {
+ err = PTR_ERR(mk_user);
+ goto out_release_key;
+ }
+ }
err = 0;
out_release_key:
up_read(&key->sem);
@@ -1029,11 +1329,12 @@ static int find_and_derive_key_legacy(const struct inode *inode,
* Find the master key, then derive the inode's actual encryption key.
*
* If the master key is found in the filesystem-level keyring, then the
- * corresponding 'struct key' is returned read-locked in *master_key_ret. This
- * is needed because we need to hold the semaphore until we link the new
- * fscrypt_info into ->mk_decrypted_inodes, but in the case where multiple tasks
- * are racing to set up an inode's ->i_crypt_info, only the winner should link
- * its fscrypt_info into ->mk_decrypted_inodes.
+ * corresponding 'struct key' is returned in *master_key_ret with
+ * ->mk_secret_sem read-locked. This is needed because we need to hold
+ * ->mk_secret_sem until we link the new fscrypt_info into
+ * ->mk_decrypted_inodes, but in the case where multiple tasks are racing to set
+ * up an inode's ->i_crypt_info, only the winner should link its fscrypt_info
+ * into ->mk_decrypted_inodes.
*/
static int find_and_derive_key(const struct inode *inode,
const union fscrypt_context *ctx,
@@ -1073,7 +1374,7 @@ static int find_and_derive_key(const struct inode *inode,
derived_keysize);
}
mk = key->payload.data[0];
- down_read(&key->sem);
+ down_read(&mk->mk_secret_sem);

/* Has the secret been removed using FS_IOC_REMOVE_ENCRYPTION_KEY? */
if (!is_master_key_secret_present(&mk->mk_secret)) {
@@ -1117,7 +1418,7 @@ static int find_and_derive_key(const struct inode *inode,
return 0;

out_release_key:
- up_read(&key->sem);
+ up_read(&mk->mk_secret_sem);
key_put(key);
return err;
}
@@ -1361,7 +1662,9 @@ int fscrypt_get_encryption_info(struct inode *inode)
}
out:
if (master_key) {
- up_read(&master_key->sem);
+ struct fscrypt_master_key *mk = master_key->payload.data[0];
+
+ up_read(&mk->mk_secret_sem);
key_put(master_key);
}
if (res == -ENOKEY)
diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index 1918bdc0c6d7..901e87d8fb74 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -102,7 +102,10 @@ struct fscrypt_add_key_args {

/* Struct passed to FS_IOC_REMOVE_ENCRYPTION_KEY */
struct fscrypt_remove_key_args {
- __u64 reserved[3];
+ __u32 flags;
+#define FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS 0x00000001
+ __u32 reserved1;
+ __u64 reserved2[2];
struct fscrypt_key_specifier key_spec;
};

@@ -117,9 +120,11 @@ struct fscrypt_get_key_status_args {
#define FSCRYPT_KEY_STATUS_ABSENT 1
#define FSCRYPT_KEY_STATUS_PRESENT 2
#define FSCRYPT_KEY_STATUS_INCOMPLETELY_REMOVED 3
+ __u32 status_flags;
+#define FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF 0x00000001
+ __u32 user_count;
__u32 reserved2;
-
- __u64 reserved3[7];
+ __u64 reserved3[6];
};

#define FS_IOC_SET_ENCRYPTION_POLICY _IOR( 'f', 19, struct fscrypt_policy)
--
2.15.0.rc0.271.g36b669edcc-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-27 18:01:41

by Michael Halcrow

[permalink] [raw]
Subject: Re: [RFC PATCH 01/25] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h>

On Mon, Oct 23, 2017 at 02:40:34PM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> There are going to be more filesystem encryption definitions added, and
> we don't want to use a disproportionate amount of space in <linux/fs.h>
> for filesystem encryption stuff. So move the fscrypt definitions to a
> new header <linux/fscrypt.h>.
>
> For compatibility with existing userspace programs which may be
> including <linux/fs.h>, <linux/fs.h> still includes the new header.
> (It's debatable whether we really need this, though; the filesystem
> encryption API is new enough that most if not all programs that are
> using it have to declare it themselves anyway.)
>
> Signed-off-by: Eric Biggers <[email protected]>

Reviewed-by: Michael Halcrow <[email protected]>

> ---
> include/linux/fscrypt.h | 2 +-
> include/uapi/linux/fs.h | 50 +++--------------------------------------
> include/uapi/linux/fscrypt.h | 53 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 57 insertions(+), 48 deletions(-)
> create mode 100644 include/uapi/linux/fscrypt.h
>
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index 53437bfdfcbc..f7aa7d62e235 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -19,7 +19,7 @@
> #include <linux/bio.h>
> #include <linux/dcache.h>
> #include <crypto/skcipher.h>
> -#include <uapi/linux/fs.h>
> +#include <uapi/linux/fscrypt.h>
>
> #define FS_CRYPTO_BLOCK_SIZE 16
>
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 56235dddea7d..6ecd3ee9960c 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -12,6 +12,9 @@
> #include <linux/limits.h>
> #include <linux/ioctl.h>
> #include <linux/types.h>
> +#ifndef __KERNEL__
> +#include <linux/fscrypt.h>
> +#endif
>
> /*
> * It's silly to have NR_OPEN bigger than NR_FILE, but you can change
> @@ -253,53 +256,6 @@ struct fsxattr {
> #define FS_IOC_FSGETXATTR _IOR ('X', 31, struct fsxattr)
> #define FS_IOC_FSSETXATTR _IOW ('X', 32, struct fsxattr)
>
> -/*
> - * File system encryption support
> - */
> -/* Policy provided via an ioctl on the topmost directory */
> -#define FS_KEY_DESCRIPTOR_SIZE 8
> -
> -#define FS_POLICY_FLAGS_PAD_4 0x00
> -#define FS_POLICY_FLAGS_PAD_8 0x01
> -#define FS_POLICY_FLAGS_PAD_16 0x02
> -#define FS_POLICY_FLAGS_PAD_32 0x03
> -#define FS_POLICY_FLAGS_PAD_MASK 0x03
> -#define FS_POLICY_FLAGS_VALID 0x03
> -
> -/* Encryption algorithms */
> -#define FS_ENCRYPTION_MODE_INVALID 0
> -#define FS_ENCRYPTION_MODE_AES_256_XTS 1
> -#define FS_ENCRYPTION_MODE_AES_256_GCM 2
> -#define FS_ENCRYPTION_MODE_AES_256_CBC 3
> -#define FS_ENCRYPTION_MODE_AES_256_CTS 4
> -#define FS_ENCRYPTION_MODE_AES_128_CBC 5
> -#define FS_ENCRYPTION_MODE_AES_128_CTS 6
> -
> -struct fscrypt_policy {
> - __u8 version;
> - __u8 contents_encryption_mode;
> - __u8 filenames_encryption_mode;
> - __u8 flags;
> - __u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
> -};
> -
> -#define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
> -#define FS_IOC_GET_ENCRYPTION_PWSALT _IOW('f', 20, __u8[16])
> -#define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy)
> -
> -/* Parameters for passing an encryption key into the kernel keyring */
> -#define FS_KEY_DESC_PREFIX "fscrypt:"
> -#define FS_KEY_DESC_PREFIX_SIZE 8
> -
> -/* Structure that userspace passes to the kernel keyring */
> -#define FS_MAX_KEY_SIZE 64
> -
> -struct fscrypt_key {
> - __u32 mode;
> - __u8 raw[FS_MAX_KEY_SIZE];
> - __u32 size;
> -};
> -
> /*
> * Inode flags (FS_IOC_GETFLAGS / FS_IOC_SETFLAGS)
> *
> diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
> new file mode 100644
> index 000000000000..c09209fc42ea
> --- /dev/null
> +++ b/include/uapi/linux/fscrypt.h
> @@ -0,0 +1,53 @@
> +#ifndef _UAPI_LINUX_FSCRYPT_H
> +#define _UAPI_LINUX_FSCRYPT_H
> +
> +#include <linux/types.h>
> +
> +/*
> + * File system encryption support
> + */
> +/* Policy provided via an ioctl on the topmost directory */
> +#define FS_KEY_DESCRIPTOR_SIZE 8
> +
> +#define FS_POLICY_FLAGS_PAD_4 0x00
> +#define FS_POLICY_FLAGS_PAD_8 0x01
> +#define FS_POLICY_FLAGS_PAD_16 0x02
> +#define FS_POLICY_FLAGS_PAD_32 0x03
> +#define FS_POLICY_FLAGS_PAD_MASK 0x03
> +#define FS_POLICY_FLAGS_VALID 0x03
> +
> +/* Encryption algorithms */
> +#define FS_ENCRYPTION_MODE_INVALID 0
> +#define FS_ENCRYPTION_MODE_AES_256_XTS 1
> +#define FS_ENCRYPTION_MODE_AES_256_GCM 2
> +#define FS_ENCRYPTION_MODE_AES_256_CBC 3
> +#define FS_ENCRYPTION_MODE_AES_256_CTS 4
> +#define FS_ENCRYPTION_MODE_AES_128_CBC 5
> +#define FS_ENCRYPTION_MODE_AES_128_CTS 6
> +
> +struct fscrypt_policy {
> + __u8 version;
> + __u8 contents_encryption_mode;
> + __u8 filenames_encryption_mode;
> + __u8 flags;
> + __u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
> +};
> +
> +#define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
> +#define FS_IOC_GET_ENCRYPTION_PWSALT _IOW('f', 20, __u8[16])
> +#define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy)
> +
> +/* Parameters for passing an encryption key into the kernel keyring */
> +#define FS_KEY_DESC_PREFIX "fscrypt:"
> +#define FS_KEY_DESC_PREFIX_SIZE 8
> +
> +/* Structure that userspace passes to the kernel keyring */
> +#define FS_MAX_KEY_SIZE 64
> +
> +struct fscrypt_key {
> + __u32 mode;
> + __u8 raw[FS_MAX_KEY_SIZE];
> + __u32 size;
> +};
> +
> +#endif /* _UAPI_LINUX_FSCRYPT_H */
> --
> 2.15.0.rc0.271.g36b669edcc-goog
>

Subject: Re: [RFC PATCH 02/25] fscrypt: use FSCRYPT_ prefix for uapi constants

On Mon, Oct 23, 2017 at 02:40:35PM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Prefix all filesystem encryption UAPI constants except the ioctl numbers
> with "FSCRYPT_" rather than with "FS_". This namespaces the constants
> more appropriately and makes it clear that they are related specifically
> to the filesystem encryption feature, and to the 'fscrypt_*' structures.
> With some of the old names like "FS_POLICY_FLAGS_VALID", it was not
> immediately clear that the constant had anything to do with encryption.
>
> This is also useful because we'll be adding more encryption-related
> constants, e.g. for the policy version, and we'd otherwise have to
> choose whether to use unclear names like FS_POLICY_VERSION_* or
> inconsistent names like FS_ENCRYPTION_POLICY_VERSION_*.
>
> For source compatibility with older userspace programs, keep the old
> names defined as aliases to the new ones. (It's debatable whether we
> really need this, though; the filesystem encryption API is new enough
> that most if not all programs that are using it have to declare it
> themselves anyway.)
>
> Signed-off-by: Eric Biggers <[email protected]>

Reviewed-by: Michael Halcrow <[email protected]>

> ---
> Documentation/filesystems/fscrypt.rst | 14 ++++-----
> include/uapi/linux/fscrypt.h | 59 ++++++++++++++++++++++++-----------
> 2 files changed, 47 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
> index 776ddc655f79..f12956f3f1f8 100644
> --- a/Documentation/filesystems/fscrypt.rst
> +++ b/Documentation/filesystems/fscrypt.rst
> @@ -251,14 +251,14 @@ empty directory or verifies that a directory or regular file already
> has the specified encryption policy. It takes in a pointer to a
> :c:type:`struct fscrypt_policy`, defined as follows::
>
> - #define FS_KEY_DESCRIPTOR_SIZE 8
> + #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
>
> struct fscrypt_policy {
> __u8 version;
> __u8 contents_encryption_mode;
> __u8 filenames_encryption_mode;
> __u8 flags;
> - __u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
> + __u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
> };
>
> This structure must be initialized as follows:
> @@ -274,7 +274,7 @@ This structure must be initialized as follows:
>
> - ``flags`` must be set to a value from ``<linux/fs.h>`` which
> identifies the amount of NUL-padding to use when encrypting
> - filenames. If unsure, use FS_POLICY_FLAGS_PAD_32 (0x3).
> + filenames. If unsure, use FSCRYPT_POLICY_FLAGS_PAD_32 (0x3).
>
> - ``master_key_descriptor`` specifies how to find the master key in
> the keyring; see `Adding keys`_. It is up to userspace to choose a
> @@ -374,11 +374,11 @@ followed by the 16-character lower case hex representation of the
> ``master_key_descriptor`` that was set in the encryption policy. The
> key payload must conform to the following structure::
>
> - #define FS_MAX_KEY_SIZE 64
> + #define FSCRYPT_MAX_KEY_SIZE 64
>
> struct fscrypt_key {
> u32 mode;
> - u8 raw[FS_MAX_KEY_SIZE];
> + u8 raw[FSCRYPT_MAX_KEY_SIZE];
> u32 size;
> };
>
> @@ -533,7 +533,7 @@ much confusion if an encryption policy were to be added to or removed
> from anything other than an empty directory.) The struct is defined
> as follows::
>
> - #define FS_KEY_DESCRIPTOR_SIZE 8
> + #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
> #define FS_KEY_DERIVATION_NONCE_SIZE 16
>
> struct fscrypt_context {
> @@ -541,7 +541,7 @@ as follows::
> u8 contents_encryption_mode;
> u8 filenames_encryption_mode;
> u8 flags;
> - u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
> + u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
> u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
> };
>
> diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
> index c09209fc42ea..26c381a40279 100644
> --- a/include/uapi/linux/fscrypt.h
> +++ b/include/uapi/linux/fscrypt.h
> @@ -7,30 +7,31 @@
> * File system encryption support
> */
> /* Policy provided via an ioctl on the topmost directory */
> -#define FS_KEY_DESCRIPTOR_SIZE 8
> +#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
>
> -#define FS_POLICY_FLAGS_PAD_4 0x00
> -#define FS_POLICY_FLAGS_PAD_8 0x01
> -#define FS_POLICY_FLAGS_PAD_16 0x02
> -#define FS_POLICY_FLAGS_PAD_32 0x03
> -#define FS_POLICY_FLAGS_PAD_MASK 0x03
> -#define FS_POLICY_FLAGS_VALID 0x03
> +/* Encryption policy flags */
> +#define FSCRYPT_POLICY_FLAGS_PAD_4 0x00
> +#define FSCRYPT_POLICY_FLAGS_PAD_8 0x01
> +#define FSCRYPT_POLICY_FLAGS_PAD_16 0x02
> +#define FSCRYPT_POLICY_FLAGS_PAD_32 0x03
> +#define FSCRYPT_POLICY_FLAGS_PAD_MASK 0x03
> +#define FSCRYPT_POLICY_FLAGS_VALID 0x03
>
> /* Encryption algorithms */
> -#define FS_ENCRYPTION_MODE_INVALID 0
> -#define FS_ENCRYPTION_MODE_AES_256_XTS 1
> -#define FS_ENCRYPTION_MODE_AES_256_GCM 2
> -#define FS_ENCRYPTION_MODE_AES_256_CBC 3
> -#define FS_ENCRYPTION_MODE_AES_256_CTS 4
> -#define FS_ENCRYPTION_MODE_AES_128_CBC 5
> -#define FS_ENCRYPTION_MODE_AES_128_CTS 6
> +#define FSCRYPT_MODE_INVALID 0
> +#define FSCRYPT_MODE_AES_256_XTS 1
> +#define FSCRYPT_MODE_AES_256_GCM 2
> +#define FSCRYPT_MODE_AES_256_CBC 3
> +#define FSCRYPT_MODE_AES_256_CTS 4
> +#define FSCRYPT_MODE_AES_128_CBC 5
> +#define FSCRYPT_MODE_AES_128_CTS 6
>
> struct fscrypt_policy {
> __u8 version;
> __u8 contents_encryption_mode;
> __u8 filenames_encryption_mode;
> __u8 flags;
> - __u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
> + __u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
> };
>
> #define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
> @@ -38,16 +39,36 @@ struct fscrypt_policy {
> #define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy)
>
> /* Parameters for passing an encryption key into the kernel keyring */
> -#define FS_KEY_DESC_PREFIX "fscrypt:"
> -#define FS_KEY_DESC_PREFIX_SIZE 8
> +#define FSCRYPT_KEY_DESC_PREFIX "fscrypt:"
> +#define FSCRYPT_KEY_DESC_PREFIX_SIZE 8
>
> /* Structure that userspace passes to the kernel keyring */
> -#define FS_MAX_KEY_SIZE 64
> +#define FSCRYPT_MAX_KEY_SIZE 64
>
> struct fscrypt_key {
> __u32 mode;
> - __u8 raw[FS_MAX_KEY_SIZE];
> + __u8 raw[FSCRYPT_MAX_KEY_SIZE];
> __u32 size;
> };
> +/**********************************************************************/
> +
> +/* old names; don't add anything new here! */
> +#define FS_POLICY_FLAGS_PAD_4 FSCRYPT_POLICY_FLAGS_PAD_4
> +#define FS_POLICY_FLAGS_PAD_8 FSCRYPT_POLICY_FLAGS_PAD_8
> +#define FS_POLICY_FLAGS_PAD_16 FSCRYPT_POLICY_FLAGS_PAD_16
> +#define FS_POLICY_FLAGS_PAD_32 FSCRYPT_POLICY_FLAGS_PAD_32
> +#define FS_POLICY_FLAGS_PAD_MASK FSCRYPT_POLICY_FLAGS_PAD_MASK
> +#define FS_POLICY_FLAGS_VALID FSCRYPT_POLICY_FLAGS_VALID
> +#define FS_KEY_DESCRIPTOR_SIZE FSCRYPT_KEY_DESCRIPTOR_SIZE
> +#define FS_ENCRYPTION_MODE_INVALID FSCRYPT_MODE_INVALID
> +#define FS_ENCRYPTION_MODE_AES_256_XTS FSCRYPT_MODE_AES_256_XTS
> +#define FS_ENCRYPTION_MODE_AES_256_GCM FSCRYPT_MODE_AES_256_GCM
> +#define FS_ENCRYPTION_MODE_AES_256_CBC FSCRYPT_MODE_AES_256_CBC
> +#define FS_ENCRYPTION_MODE_AES_256_CTS FSCRYPT_MODE_AES_256_CTS
> +#define FS_ENCRYPTION_MODE_AES_128_CBC FSCRYPT_MODE_AES_128_CBC
> +#define FS_ENCRYPTION_MODE_AES_128_CTS FSCRYPT_MODE_AES_128_CTS
> +#define FS_KEY_DESC_PREFIX FSCRYPT_KEY_DESC_PREFIX
> +#define FS_KEY_DESC_PREFIX_SIZE FSCRYPT_KEY_DESC_PREFIX_SIZE
> +#define FS_MAX_KEY_SIZE FSCRYPT_MAX_KEY_SIZE
>
> #endif /* _UAPI_LINUX_FSCRYPT_H */
> --
> 2.15.0.rc0.271.g36b669edcc-goog
>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-27 18:23:03

by Michael Halcrow

[permalink] [raw]
Subject: Re: [RFC PATCH 04/25] fscrypt: refactor finding and deriving key

On Mon, Oct 23, 2017 at 02:40:37PM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> In preparation for introducing a new way to find the master keys and
> derive the per-file keys, clean up the current method. This includes:
>
> - Introduce a helper function find_and_derive_key() so that we don't
> have to add more code directly to fscrypt_get_encryption_info().
>
> - Don't pass the 'struct fscrypt_key' directly into derive_key_aes().
> This is in preparation for the case where we find the master key in a
> filesystem-level keyring, where (for good reasons) the key payload
> will *not* be formatted as the UAPI 'struct fscrypt_key'.
>
> - Separate finding the key from key derivation. In particular, it
> *only* makes sense to fall back to the alternate key description
> prefix if searching for the "fscrypt:" prefix returns -ENOKEY. It
> doesn't make sense to do so when derive_key_aes() fails, for example.
>
> - Improve the error messages for when the fscrypt_key is invalid.
>
> - Rename 'raw_key' to 'derived_key' for clarity.

With all the crypto code I've delt with where a 'key' is actually just
a handle or a reference, I've developed a personal habit is to call
the buffer that contains the actual key bytes 'raw' to emphasize that
there's tangible secret material present.

That said, it's probably fine to call it 'derived_key' in this
instance.

>
> Signed-off-by: Eric Biggers <[email protected]>

Reviewed-by: Michael Halcrow <[email protected]>

> ---
> fs/crypto/keyinfo.c | 205 ++++++++++++++++++++++++++++------------------------
> 1 file changed, 109 insertions(+), 96 deletions(-)
>
> diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
> index ac41f646e7b7..d3a97c2cd4dd 100644
> --- a/fs/crypto/keyinfo.c
> +++ b/fs/crypto/keyinfo.c
> @@ -28,113 +28,138 @@ static void derive_crypt_complete(struct crypto_async_request *req, int rc)
> complete(&ecr->completion);
> }
>
> -/**
> - * derive_key_aes() - Derive a key using AES-128-ECB
> - * @deriving_key: Encryption key used for derivation.
> - * @source_key: Source key to which to apply derivation.
> - * @derived_raw_key: Derived raw key.
> +/*
> + * Key derivation function. This generates the derived key by encrypting the
> + * master key with AES-128-ECB using the nonce as the AES key.
> *
> - * Return: Zero on success; non-zero otherwise.
> + * The master key must be at least as long as the derived key. If the master
> + * key is longer, then only the first 'derived_keysize' bytes are used.
> */
> -static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
> - const struct fscrypt_key *source_key,
> - u8 derived_raw_key[FSCRYPT_MAX_KEY_SIZE])
> +static int derive_key_aes(const u8 *master_key,
> + const struct fscrypt_context *ctx,
> + u8 *derived_key, unsigned int derived_keysize)
> {
> - int res = 0;
> + int err;
> struct skcipher_request *req = NULL;
> DECLARE_FS_COMPLETION_RESULT(ecr);
> struct scatterlist src_sg, dst_sg;
> - struct crypto_skcipher *tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
> + struct crypto_skcipher *tfm;
> +
> + tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
> + if (IS_ERR(tfm))
> + return PTR_ERR(tfm);
>
> - if (IS_ERR(tfm)) {
> - res = PTR_ERR(tfm);
> - tfm = NULL;
> - goto out;
> - }
> crypto_skcipher_set_flags(tfm, CRYPTO_TFM_REQ_WEAK_KEY);
> req = skcipher_request_alloc(tfm, GFP_NOFS);
> if (!req) {
> - res = -ENOMEM;
> + err = -ENOMEM;
> goto out;
> }
> skcipher_request_set_callback(req,
> CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
> derive_crypt_complete, &ecr);
> - res = crypto_skcipher_setkey(tfm, deriving_key,
> - FS_AES_128_ECB_KEY_SIZE);
> - if (res < 0)
> +
> + BUILD_BUG_ON(sizeof(ctx->nonce) != FS_AES_128_ECB_KEY_SIZE);
> + err = crypto_skcipher_setkey(tfm, ctx->nonce, sizeof(ctx->nonce));
> + if (err)
> goto out;
>
> - sg_init_one(&src_sg, source_key->raw, source_key->size);
> - sg_init_one(&dst_sg, derived_raw_key, source_key->size);
> - skcipher_request_set_crypt(req, &src_sg, &dst_sg, source_key->size,
> + sg_init_one(&src_sg, master_key, derived_keysize);
> + sg_init_one(&dst_sg, derived_key, derived_keysize);
> + skcipher_request_set_crypt(req, &src_sg, &dst_sg, derived_keysize,
> NULL);
> - res = crypto_skcipher_encrypt(req);
> - if (res == -EINPROGRESS || res == -EBUSY) {
> + err = crypto_skcipher_encrypt(req);
> + if (err == -EINPROGRESS || err == -EBUSY) {
> wait_for_completion(&ecr.completion);
> - res = ecr.res;
> + err = ecr.res;
> }
> out:
> skcipher_request_free(req);
> crypto_free_skcipher(tfm);
> - return res;
> + return err;
> }
>
> -static int validate_user_key(struct fscrypt_info *crypt_info,
> - struct fscrypt_context *ctx, u8 *raw_key,
> - const char *prefix, int min_keysize)
> +/*
> + * Search the current task's subscribed keyrings for a "logon" key with
> + * description prefix:descriptor, and if found acquire a read lock on it and
> + * return a pointer to its validated payload in *payload_ret.
> + */
> +static struct key *
> +find_and_lock_process_key(const char *prefix,
> + const u8 descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE],
> + unsigned int min_keysize,
> + const struct fscrypt_key **payload_ret)

Since we've already crossed the threshold of returning at least one
object via a ptr-to-ptr param, maybe doing that for struct key too and
just making the return value of the function be an int err would be
cleaner than PTR_ERR/ERR_PTR?

> {
> char *description;
> - struct key *keyring_key;
> - struct fscrypt_key *master_key;
> + struct key *key;
> const struct user_key_payload *ukp;
> - int res;
> + const struct fscrypt_key *payload;
>
> description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
> - FSCRYPT_KEY_DESCRIPTOR_SIZE,
> - ctx->master_key_descriptor);
> + FSCRYPT_KEY_DESCRIPTOR_SIZE, descriptor);
> if (!description)
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
>
> - keyring_key = request_key(&key_type_logon, description, NULL);
> + key = request_key(&key_type_logon, description, NULL);
> kfree(description);
> - if (IS_ERR(keyring_key))
> - return PTR_ERR(keyring_key);
> - down_read(&keyring_key->sem);
> -
> - if (keyring_key->type != &key_type_logon) {
> - printk_once(KERN_WARNING
> - "%s: key type must be logon\n", __func__);
> - res = -ENOKEY;
> - goto out;
> - }
> - ukp = user_key_payload_locked(keyring_key);
> - if (!ukp) {
> - /* key was revoked before we acquired its semaphore */
> - res = -EKEYREVOKED;
> - goto out;
> + if (IS_ERR(key))
> + return key;
> +
> + down_read(&key->sem);
> + ukp = user_key_payload_locked(key);
> +
> + if (!ukp) /* was the key revoked before we acquired its semaphore? */
> + goto invalid;
> +
> + payload = (const struct fscrypt_key *)ukp->data;
> +
> + if (ukp->datalen != sizeof(struct fscrypt_key) ||
> + payload->size < 1 || payload->size > FSCRYPT_MAX_KEY_SIZE) {
> + pr_warn_ratelimited("fscrypt: key with description '%s' has invalid payload\n",
> + key->description);
> + goto invalid;
> }
> - if (ukp->datalen != sizeof(struct fscrypt_key)) {
> - res = -EINVAL;
> - goto out;
> +
> + if (payload->size < min_keysize) {
> + pr_warn_ratelimited("fscrypt: key with description '%s' is too short "
> + "(got %u bytes, need %u+ bytes)\n",
> + key->description,
> + payload->size, min_keysize);
> + goto invalid;

A common (yet high-impact) mistake is to pass in only 256 bits of
entropic key material in the 512-bit buffer for AES-256-XTS, leaving
the second half all 0's. I've actually seen that done in pre-release
code. It would be an easy check just to see if userspace did that,
but then we're on the slippery slope of how much key strength
validation we should do on the key, if we're going to do any at all.

> }
> - master_key = (struct fscrypt_key *)ukp->data;
> - BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
> -
> - if (master_key->size < min_keysize ||
> - master_key->size > FSCRYPT_MAX_KEY_SIZE
> - || master_key->size % AES_BLOCK_SIZE != 0) {
> - printk_once(KERN_WARNING
> - "%s: key size incorrect: %d\n",
> - __func__, master_key->size);
> - res = -ENOKEY;
> - goto out;
> +
> + *payload_ret = payload;
> + return key;
> +
> +invalid:
> + up_read(&key->sem);
> + key_put(key);
> + return ERR_PTR(-ENOKEY);
> +}
> +
> +/* Find the master key, then derive the inode's actual encryption key */
> +static int find_and_derive_key(const struct inode *inode,
> + const struct fscrypt_context *ctx,
> + u8 *derived_key, unsigned int derived_keysize)
> +{
> + struct key *key;
> + const struct fscrypt_key *payload;
> + int err;
> +
> + key = find_and_lock_process_key(FSCRYPT_KEY_DESC_PREFIX,
> + ctx->master_key_descriptor,
> + derived_keysize, &payload);
> + if (key == ERR_PTR(-ENOKEY) && inode->i_sb->s_cop->key_prefix) {
> + key = find_and_lock_process_key(inode->i_sb->s_cop->key_prefix,
> + ctx->master_key_descriptor,
> + derived_keysize, &payload);
> }
> - res = derive_key_aes(ctx->nonce, master_key, raw_key);
> -out:
> - up_read(&keyring_key->sem);
> - key_put(keyring_key);
> - return res;
> + if (IS_ERR(key))
> + return PTR_ERR(key);
> + err = derive_key_aes(payload->raw, ctx, derived_key, derived_keysize);
> + up_read(&key->sem);
> + key_put(key);
> + return err;
> }
>
> static const struct {
> @@ -256,8 +281,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
> struct fscrypt_context ctx;
> struct crypto_skcipher *ctfm;
> const char *cipher_str;
> - int keysize;
> - u8 *raw_key = NULL;
> + unsigned int derived_keysize;
> + u8 *derived_key = NULL;
> int res;
>
> if (inode->i_crypt_info)
> @@ -301,7 +326,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
> memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
> sizeof(crypt_info->ci_master_key));
>
> - res = determine_cipher_type(crypt_info, inode, &cipher_str, &keysize);
> + res = determine_cipher_type(crypt_info, inode,
> + &cipher_str, &derived_keysize);
> if (res)
> goto out;
>
> @@ -310,24 +336,14 @@ int fscrypt_get_encryption_info(struct inode *inode)
> * crypto API as part of key derivation.
> */
> res = -ENOMEM;
> - raw_key = kmalloc(FSCRYPT_MAX_KEY_SIZE, GFP_NOFS);
> - if (!raw_key)
> + derived_key = kmalloc(FS_MAX_KEY_SIZE, GFP_NOFS);
> + if (!derived_key)
> goto out;
>
> - res = validate_user_key(crypt_info, &ctx, raw_key,
> - FSCRYPT_KEY_DESC_PREFIX, keysize);
> - if (res && inode->i_sb->s_cop->key_prefix) {
> - int res2 = validate_user_key(crypt_info, &ctx, raw_key,
> - inode->i_sb->s_cop->key_prefix,
> - keysize);
> - if (res2) {
> - if (res2 == -ENOKEY)
> - res = -ENOKEY;
> - goto out;
> - }
> - } else if (res) {
> + res = find_and_derive_key(inode, &ctx, derived_key, derived_keysize);
> + if (res)
> goto out;
> - }
> +
> ctfm = crypto_alloc_skcipher(cipher_str, 0, 0);
> if (!ctfm || IS_ERR(ctfm)) {
> res = ctfm ? PTR_ERR(ctfm) : -ENOMEM;
> @@ -338,17 +354,14 @@ int fscrypt_get_encryption_info(struct inode *inode)
> crypt_info->ci_ctfm = ctfm;
> crypto_skcipher_clear_flags(ctfm, ~0);
> crypto_skcipher_set_flags(ctfm, CRYPTO_TFM_REQ_WEAK_KEY);
> - /*
> - * if the provided key is longer than keysize, we use the first
> - * keysize bytes of the derived key only
> - */
> - res = crypto_skcipher_setkey(ctfm, raw_key, keysize);
> + res = crypto_skcipher_setkey(ctfm, derived_key, derived_keysize);
> if (res)
> goto out;
>
> if (S_ISREG(inode->i_mode) &&
> crypt_info->ci_data_mode == FSCRYPT_MODE_AES_128_CBC) {
> - res = init_essiv_generator(crypt_info, raw_key, keysize);
> + res = init_essiv_generator(crypt_info, derived_key,
> + derived_keysize);
> if (res) {
> pr_debug("%s: error %d (inode %lu) allocating essiv tfm\n",
> __func__, res, inode->i_ino);
> @@ -361,7 +374,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
> if (res == -ENOKEY)
> res = 0;
> put_crypt_info(crypt_info);
> - kzfree(raw_key);
> + kzfree(derived_key);
> return res;
> }
> EXPORT_SYMBOL(fscrypt_get_encryption_info);
> --
> 2.15.0.rc0.271.g36b669edcc-goog
>

2017-10-27 18:26:33

by Michael Halcrow

[permalink] [raw]
Subject: Re: [RFC PATCH 05/25] fs: add ->s_master_keys to struct super_block

On Mon, Oct 23, 2017 at 02:40:38PM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Add an ->s_master_keys keyring to 'struct super_block' for holding
> encryption keys which have been added to the filesystem. This keyring
> will be populated using a new fscrypt ioctl.
>
> This is needed for several reasons, including:
>
> - To solve the visibility problems of having filesystem encryption keys
> stored in process-subscribed keyrings, while the VFS state of the
> filesystem is actually global.
>
> - To implement a proper API for removing keys, which among other things
> will require maintaining the list of inodes that are using each master
> key so that we can evict the inodes when the key is removed.
>
> - To allow caching a crypto transform for each master key so that we
> don't have to repeatedly allocate one over and over.
>
> See later patches for full details, including why it wouldn't be enough
> to add the concept of a "global keyring" to the keyrings API instead.
>
> ->s_master_keys will only be allocated when someone tries to add a key
> for the first time. Otherwise it will stay NULL.
>
> Note that this could go in the filesystem-specific superblocks instead.
> However, we already have three filesystems using fs/crypto/, so it's
> useful to have it in the VFS.
>
> Signed-off-by: Eric Biggers <[email protected]>

Reviewed-by: Michael Halcrow <[email protected]>

> ---
> fs/super.c | 3 +++
> include/linux/fs.h | 4 ++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/fs/super.c b/fs/super.c
> index 166c4ee0d0ed..161a9d05aa9f 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -168,6 +168,9 @@ static void destroy_super(struct super_block *s)
> WARN_ON(!list_empty(&s->s_mounts));
> put_user_ns(s->s_user_ns);
> kfree(s->s_subtype);
> +#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
> + key_put(s->s_master_keys);
> +#endif
> call_rcu(&s->rcu, destroy_super_rcu);
> }
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 3efd5ded21c9..8cfb0877d32c 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1440,6 +1440,10 @@ struct super_block {
>
> spinlock_t s_inode_wblist_lock;
> struct list_head s_inodes_wb; /* writeback inodes */
> +
> +#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
> + struct key *s_master_keys; /* master crypto keys in use */
> +#endif
> } __randomize_layout;
>
> /* Helper functions so that in most cases filesystems will
> --
> 2.15.0.rc0.271.g36b669edcc-goog
>

2017-10-27 20:14:49

by Michael Halcrow

[permalink] [raw]
Subject: Re: [RFC PATCH 06/25] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl

On Mon, Oct 23, 2017 at 02:40:39PM -0700, Eric Biggers wrote:
> By having an API to add a key to the *filesystem* we'll be able to
> eliminate all the above hacks and better express the intended semantics:
> the "locked/unlocked" status of an encrypted directory is global. And
> orthogonally to encryption, existing mechanisms such as file permissions
> and LSMs can and should continue to be used for the purpose of *access
> control*.

At some point I'd like to try to tackle the problem of making the
encryption policy somehow *reflect* the access control policy.

For now this change cleans up a real mess and makes things much more
manageable and predictable.

> Signed-off-by: Eric Biggers <[email protected]>

Reviewed-by: Michael Halcrow <[email protected]>

> ---
> fs/crypto/crypto.c | 12 +-
> fs/crypto/fscrypt_private.h | 3 +
> fs/crypto/keyinfo.c | 351 +++++++++++++++++++++++++++++++++++++++-
> include/linux/fscrypt_notsupp.h | 5 +
> include/linux/fscrypt_supp.h | 1 +
> include/uapi/linux/fscrypt.h | 41 +++--
> 6 files changed, 397 insertions(+), 16 deletions(-)
>
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index 608f6bbe0f31..489c504ac20d 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -24,6 +24,7 @@
> #include <linux/module.h>
> #include <linux/scatterlist.h>
> #include <linux/ratelimit.h>
> +#include <linux/key-type.h>
> #include <linux/dcache.h>
> #include <linux/namei.h>
> #include <crypto/aes.h>
> @@ -449,6 +450,8 @@ int fscrypt_initialize(unsigned int cop_flags)
> */
> static int __init fscrypt_init(void)
> {
> + int err = -ENOMEM;
> +
> fscrypt_read_workqueue = alloc_workqueue("fscrypt_read_queue",
> WQ_HIGHPRI, 0);
> if (!fscrypt_read_workqueue)
> @@ -462,14 +465,20 @@ static int __init fscrypt_init(void)
> if (!fscrypt_info_cachep)
> goto fail_free_ctx;
>
> + err = register_key_type(&key_type_fscrypt_mk);
> + if (err)
> + goto fail_free_info;
> +
> return 0;
>
> +fail_free_info:
> + kmem_cache_destroy(fscrypt_info_cachep);
> fail_free_ctx:
> kmem_cache_destroy(fscrypt_ctx_cachep);
> fail_free_queue:
> destroy_workqueue(fscrypt_read_workqueue);
> fail:
> - return -ENOMEM;
> + return err;
> }
> module_init(fscrypt_init)
>
> @@ -484,6 +493,7 @@ static void __exit fscrypt_exit(void)
> destroy_workqueue(fscrypt_read_workqueue);
> kmem_cache_destroy(fscrypt_ctx_cachep);
> kmem_cache_destroy(fscrypt_info_cachep);
> + unregister_key_type(&key_type_fscrypt_mk);
>
> fscrypt_essiv_cleanup();
> }
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 5cb80a2d39ea..b2fad12eeedb 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -27,6 +27,8 @@
>
> #define FS_KEY_DERIVATION_NONCE_SIZE 16
>
> +#define FSCRYPT_MIN_KEY_SIZE 16
> +
> /**
> * Encryption context for inode
> *
> @@ -93,6 +95,7 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
> gfp_t gfp_flags);
>
> /* keyinfo.c */
> +extern struct key_type key_type_fscrypt_mk;
> extern void __exit fscrypt_essiv_cleanup(void);
>
> #endif /* _FSCRYPT_PRIVATE_H */
> diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
> index d3a97c2cd4dd..3f1cb8bbc1e5 100644
> --- a/fs/crypto/keyinfo.c
> +++ b/fs/crypto/keyinfo.c
> @@ -9,14 +9,307 @@
> */
>
> #include <keys/user-type.h>
> -#include <linux/scatterlist.h>
> +#include <linux/key-type.h>
> #include <linux/ratelimit.h>
> +#include <linux/scatterlist.h>
> +#include <linux/seq_file.h>
> #include <crypto/aes.h>
> #include <crypto/sha.h>
> #include "fscrypt_private.h"
>
> static struct crypto_shash *essiv_hash_tfm;
>
> +/*
> + * fscrypt_master_key_secret - secret key material of an in-use master key
> + */
> +struct fscrypt_master_key_secret {
> +
> + /* Size of the raw key in bytes */
> + u32 size;
> +
> + /* The raw key */
> + u8 raw[FSCRYPT_MAX_KEY_SIZE];
> +};

With structs fscrypt introduces, I suggest __randomize_layout wherever
feasible.

> +#define FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE \
> + (sizeof("fscrypt-") - 1 + sizeof(((struct super_block
> *)0)->s_id) + 1)

What function do the " - 1" and " + 1" parts serve here? Readability?

> + key = find_master_key(inode->i_sb, &mk_spec);
> + if (IS_ERR(key)) {
> + if (key != ERR_PTR(-ENOKEY))
> + return PTR_ERR(key);
> + /*
> + * As a legacy fallback, we search the current task's subscribed
> + * keyrings in addition to ->s_master_keys.

Please add an explicit comment that it's important for security that
the ordering of these two searches be preserved.

> + */
> + return find_and_derive_key_legacy(inode, ctx, derived_key,
> + derived_keysize);
> + }
> + mk = key->payload.data[0];
> +
> + /*
> + * Require that the master key be at least as long as the derived key.
> + * Otherwise, the derived key cannot possibly contain as much entropy as
> + * that required by the encryption mode it will be used for.
> + */
> + if (mk->mk_secret.size < derived_keysize) {

As I've mentioned in a previous patch in this set, if we're going to
get opinionated about source entropy, there's more we could
measure/estimate than just the length.

2017-10-27 20:28:28

by Michael Halcrow

[permalink] [raw]
Subject: Re: [RFC PATCH 07/25] fs/inode.c: export inode_lru_list_del()

On Mon, Oct 23, 2017 at 02:40:40PM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> When a filesystem encryption key is removed, we need all files which had
> been "unlocked" (had ->i_crypt_info set up) with it to appear "locked"
> again. This is most easily done by evicting the inodes. This can
> currently be done using 'echo 2 > /proc/sys/vm/drop_caches'; however,
> that is overkill and not usable by non-root users. In preparation for
> allowing fs/crypto/ to evict just the needed inodes, export
> inode_lru_list_del() to modules.
>
> Signed-off-by: Eric Biggers <[email protected]>

Reviewed-by: Michael Halcrow <[email protected]>

> ---
> fs/inode.c | 5 ++---
> include/linux/fs.h | 1 +
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index d1e35b53bb23..30ce98956801 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -420,13 +420,12 @@ void inode_add_lru(struct inode *inode)
> inode_lru_list_add(inode);
> }
>
> -
> -static void inode_lru_list_del(struct inode *inode)
> +void inode_lru_list_del(struct inode *inode)
> {
> -
> if (list_lru_del(&inode->i_sb->s_inode_lru, &inode->i_lru))
> this_cpu_dec(nr_unused);
> }
> +EXPORT_SYMBOL_GPL(inode_lru_list_del);
>
> /**
> * inode_sb_list_add - add inode to the superblock list of inodes
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8cfb0877d32c..2833ace2f01d 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2879,6 +2879,7 @@ static inline void lockdep_annotate_inode_mutex_key(struct inode *inode) { };
> extern void unlock_new_inode(struct inode *);
> extern unsigned int get_next_ino(void);
> extern void evict_inodes(struct super_block *sb);
> +extern void inode_lru_list_del(struct inode *inode);
>
> extern void __iget(struct inode * inode);
> extern void iget_failed(struct inode *);
> --
> 2.15.0.rc0.271.g36b669edcc-goog
>