* [RFC PATCH 01/25] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h>
2017-10-23 21:40 [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support Eric Biggers
@ 2017-10-23 21:40 ` Eric Biggers
2017-10-27 18:01 ` Michael Halcrow
2017-10-23 21:40 ` [RFC PATCH 02/25] fscrypt: use FSCRYPT_ prefix for uapi constants Eric Biggers
` (23 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Eric Biggers @ 2017-10-23 21:40 UTC (permalink / raw)
To: linux-fscrypt
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-mtd, linux-api,
keyrings, Theodore Y . Ts'o, Jaegeuk Kim, Gwendal Grignou,
Ryo Hashimoto, Sarthak Kukreti, Nick Desaulniers, Michael Halcrow,
Eric Biggers
From: Eric Biggers <ebiggers@google.com>
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 <ebiggers@google.com>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC PATCH 01/25] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h>
2017-10-23 21:40 ` [RFC PATCH 01/25] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h> Eric Biggers
@ 2017-10-27 18:01 ` Michael Halcrow
0 siblings, 0 replies; 33+ messages in thread
From: Michael Halcrow @ 2017-10-27 18:01 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
linux-mtd, linux-api, keyrings, Theodore Y . Ts'o,
Jaegeuk Kim, Gwendal Grignou, Ryo Hashimoto, Sarthak Kukreti,
Nick Desaulniers, Eric Biggers
On Mon, Oct 23, 2017 at 02:40:34PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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 <ebiggers@google.com>
Reviewed-by: Michael Halcrow <mhalcrow@google.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 02/25] fscrypt: use FSCRYPT_ prefix for uapi constants
2017-10-23 21:40 [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support Eric Biggers
2017-10-23 21:40 ` [RFC PATCH 01/25] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h> Eric Biggers
@ 2017-10-23 21:40 ` Eric Biggers
2017-10-27 18:02 ` Michael Halcrow
2017-10-23 21:40 ` [RFC PATCH 03/25] fscrypt: use FSCRYPT_* definitions, not FS_* Eric Biggers
` (22 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Eric Biggers @ 2017-10-23 21:40 UTC (permalink / raw)
To: linux-fscrypt
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-mtd, linux-api,
keyrings, Theodore Y . Ts'o, Jaegeuk Kim, Gwendal Grignou,
Ryo Hashimoto, Sarthak Kukreti, Nick Desaulniers, Michael Halcrow,
Eric Biggers
From: Eric Biggers <ebiggers@google.com>
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 <ebiggers@google.com>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC PATCH 02/25] fscrypt: use FSCRYPT_ prefix for uapi constants
2017-10-23 21:40 ` [RFC PATCH 02/25] fscrypt: use FSCRYPT_ prefix for uapi constants Eric Biggers
@ 2017-10-27 18:02 ` Michael Halcrow
0 siblings, 0 replies; 33+ messages in thread
From: Michael Halcrow @ 2017-10-27 18:02 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
linux-mtd, linux-api, keyrings, Theodore Y . Ts'o,
Jaegeuk Kim, Gwendal Grignou, Ryo Hashimoto, Sarthak Kukreti,
Nick Desaulniers, Eric Biggers
On Mon, Oct 23, 2017 at 02:40:35PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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 <ebiggers@google.com>
Reviewed-by: Michael Halcrow <mhalcrow@google.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 03/25] fscrypt: use FSCRYPT_* definitions, not FS_*
2017-10-23 21:40 [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support Eric Biggers
2017-10-23 21:40 ` [RFC PATCH 01/25] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h> Eric Biggers
2017-10-23 21:40 ` [RFC PATCH 02/25] fscrypt: use FSCRYPT_ prefix for uapi constants Eric Biggers
@ 2017-10-23 21:40 ` Eric Biggers
2017-10-27 18:06 ` Michael Halcrow
2017-10-23 21:40 ` [RFC PATCH 04/25] fscrypt: refactor finding and deriving key Eric Biggers
` (21 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Eric Biggers @ 2017-10-23 21:40 UTC (permalink / raw)
To: linux-fscrypt
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-mtd, linux-api,
keyrings, Theodore Y . Ts'o, Jaegeuk Kim, Gwendal Grignou,
Ryo Hashimoto, Sarthak Kukreti, Nick Desaulniers, Michael Halcrow,
Eric Biggers
From: Eric Biggers <ebiggers@google.com>
Update the filesystem encryption kernel code to use the new names for
the UAPI constants rather than the old names.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/crypto/fname.c | 4 ++--
fs/crypto/fscrypt_private.h | 4 ++--
fs/crypto/keyinfo.c | 36 +++++++++++++++++-------------------
fs/crypto/policy.c | 14 +++++++-------
include/linux/fscrypt.h | 8 ++++----
5 files changed, 32 insertions(+), 34 deletions(-)
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 2878289b3ed2..c91bcef65b9f 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 & FS_POLICY_FLAGS_PAD_MASK);
+ int padding = 4 << (ci->ci_flags & FSCRYPT_POLICY_FLAGS_PAD_MASK);
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 & FS_POLICY_FLAGS_PAD_MASK);
+ padding = 4 << (ci->ci_flags & FSCRYPT_POLICY_FLAGS_PAD_MASK);
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 a180981ee6d7..5cb80a2d39ea 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -43,7 +43,7 @@ struct fscrypt_context {
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];
} __packed;
@@ -59,7 +59,7 @@ struct fscrypt_info {
u8 ci_flags;
struct crypto_skcipher *ci_ctfm;
struct crypto_cipher *ci_essiv_tfm;
- u8 ci_master_key[FS_KEY_DESCRIPTOR_SIZE];
+ u8 ci_master_key[FSCRYPT_KEY_DESCRIPTOR_SIZE];
};
typedef enum {
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 7aed93bcfb82..ac41f646e7b7 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -38,7 +38,7 @@ static void derive_crypt_complete(struct crypto_async_request *req, int rc)
*/
static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
const struct fscrypt_key *source_key,
- u8 derived_raw_key[FS_MAX_KEY_SIZE])
+ u8 derived_raw_key[FSCRYPT_MAX_KEY_SIZE])
{
int res = 0;
struct skcipher_request *req = NULL;
@@ -91,7 +91,7 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
int res;
description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
- FS_KEY_DESCRIPTOR_SIZE,
+ FSCRYPT_KEY_DESCRIPTOR_SIZE,
ctx->master_key_descriptor);
if (!description)
return -ENOMEM;
@@ -121,7 +121,8 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
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 > FS_MAX_KEY_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",
@@ -140,14 +141,10 @@ static const struct {
const char *cipher_str;
int keysize;
} available_modes[] = {
- [FS_ENCRYPTION_MODE_AES_256_XTS] = { "xts(aes)",
- FS_AES_256_XTS_KEY_SIZE },
- [FS_ENCRYPTION_MODE_AES_256_CTS] = { "cts(cbc(aes))",
- FS_AES_256_CTS_KEY_SIZE },
- [FS_ENCRYPTION_MODE_AES_128_CBC] = { "cbc(aes)",
- FS_AES_128_CBC_KEY_SIZE },
- [FS_ENCRYPTION_MODE_AES_128_CTS] = { "cts(cbc(aes))",
- FS_AES_128_CTS_KEY_SIZE },
+ [FSCRYPT_MODE_AES_256_XTS] = { "xts(aes)", FS_AES_256_XTS_KEY_SIZE },
+ [FSCRYPT_MODE_AES_256_CTS] = { "cts(cbc(aes))", FS_AES_256_CTS_KEY_SIZE },
+ [FSCRYPT_MODE_AES_128_CBC] = { "cbc(aes)", FS_AES_128_CBC_KEY_SIZE },
+ [FSCRYPT_MODE_AES_128_CTS] = { "cts(cbc(aes))", FS_AES_128_CTS_KEY_SIZE },
};
static int determine_cipher_type(struct fscrypt_info *ci, struct inode *inode,
@@ -278,9 +275,10 @@ int fscrypt_get_encryption_info(struct inode *inode)
/* Fake up a context for an unencrypted directory */
memset(&ctx, 0, sizeof(ctx));
ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
- ctx.contents_encryption_mode = FS_ENCRYPTION_MODE_AES_256_XTS;
- ctx.filenames_encryption_mode = FS_ENCRYPTION_MODE_AES_256_CTS;
- memset(ctx.master_key_descriptor, 0x42, FS_KEY_DESCRIPTOR_SIZE);
+ ctx.contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
+ ctx.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
+ memset(ctx.master_key_descriptor, 0x42,
+ FSCRYPT_KEY_DESCRIPTOR_SIZE);
} else if (res != sizeof(ctx)) {
return -EINVAL;
}
@@ -288,7 +286,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
return -EINVAL;
- if (ctx.flags & ~FS_POLICY_FLAGS_VALID)
+ if (ctx.flags & ~FSCRYPT_POLICY_FLAGS_VALID)
return -EINVAL;
crypt_info = kmem_cache_alloc(fscrypt_info_cachep, GFP_NOFS);
@@ -312,12 +310,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
* crypto API as part of key derivation.
*/
res = -ENOMEM;
- raw_key = kmalloc(FS_MAX_KEY_SIZE, GFP_NOFS);
+ raw_key = kmalloc(FSCRYPT_MAX_KEY_SIZE, GFP_NOFS);
if (!raw_key)
goto out;
- res = validate_user_key(crypt_info, &ctx, raw_key, FS_KEY_DESC_PREFIX,
- keysize);
+ 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,
@@ -349,7 +347,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
goto out;
if (S_ISREG(inode->i_mode) &&
- crypt_info->ci_data_mode == FS_ENCRYPTION_MODE_AES_128_CBC) {
+ crypt_info->ci_data_mode == FSCRYPT_MODE_AES_128_CBC) {
res = init_essiv_generator(crypt_info, raw_key, keysize);
if (res) {
pr_debug("%s: error %d (inode %lu) allocating essiv tfm\n",
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 6a63b8a0d46c..19332a6fd52d 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -21,7 +21,7 @@ static bool is_encryption_context_consistent_with_policy(
const struct fscrypt_policy *policy)
{
return memcmp(ctx->master_key_descriptor, policy->master_key_descriptor,
- FS_KEY_DESCRIPTOR_SIZE) == 0 &&
+ FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
(ctx->flags == policy->flags) &&
(ctx->contents_encryption_mode ==
policy->contents_encryption_mode) &&
@@ -36,13 +36,13 @@ static int create_encryption_context_from_policy(struct inode *inode,
ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
memcpy(ctx.master_key_descriptor, policy->master_key_descriptor,
- FS_KEY_DESCRIPTOR_SIZE);
+ FSCRYPT_KEY_DESCRIPTOR_SIZE);
if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
policy->filenames_encryption_mode))
return -EINVAL;
- if (policy->flags & ~FS_POLICY_FLAGS_VALID)
+ if (policy->flags & ~FSCRYPT_POLICY_FLAGS_VALID)
return -EINVAL;
ctx.contents_encryption_mode = policy->contents_encryption_mode;
@@ -125,7 +125,7 @@ int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg)
policy.filenames_encryption_mode = ctx.filenames_encryption_mode;
policy.flags = ctx.flags;
memcpy(policy.master_key_descriptor, ctx.master_key_descriptor,
- FS_KEY_DESCRIPTOR_SIZE);
+ FSCRYPT_KEY_DESCRIPTOR_SIZE);
if (copy_to_user(arg, &policy, sizeof(policy)))
return -EFAULT;
@@ -199,7 +199,7 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
if (parent_ci && child_ci) {
return memcmp(parent_ci->ci_master_key, child_ci->ci_master_key,
- FS_KEY_DESCRIPTOR_SIZE) == 0 &&
+ 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) &&
@@ -216,7 +216,7 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
return memcmp(parent_ctx.master_key_descriptor,
child_ctx.master_key_descriptor,
- FS_KEY_DESCRIPTOR_SIZE) == 0 &&
+ FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
(parent_ctx.contents_encryption_mode ==
child_ctx.contents_encryption_mode) &&
(parent_ctx.filenames_encryption_mode ==
@@ -254,7 +254,7 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
ctx.filenames_encryption_mode = ci->ci_filename_mode;
ctx.flags = ci->ci_flags;
memcpy(ctx.master_key_descriptor, ci->ci_master_key,
- FS_KEY_DESCRIPTOR_SIZE);
+ 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,
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index f7aa7d62e235..671ce57e4673 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -99,12 +99,12 @@ static inline bool fscrypt_dummy_context_enabled(struct inode *inode)
static inline bool fscrypt_valid_enc_modes(u32 contents_mode,
u32 filenames_mode)
{
- if (contents_mode == FS_ENCRYPTION_MODE_AES_128_CBC &&
- filenames_mode == FS_ENCRYPTION_MODE_AES_128_CTS)
+ if (contents_mode == FSCRYPT_MODE_AES_128_CBC &&
+ filenames_mode == FSCRYPT_MODE_AES_128_CTS)
return true;
- if (contents_mode == FS_ENCRYPTION_MODE_AES_256_XTS &&
- filenames_mode == FS_ENCRYPTION_MODE_AES_256_CTS)
+ if (contents_mode == FSCRYPT_MODE_AES_256_XTS &&
+ filenames_mode == FSCRYPT_MODE_AES_256_CTS)
return true;
return false;
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC PATCH 03/25] fscrypt: use FSCRYPT_* definitions, not FS_*
2017-10-23 21:40 ` [RFC PATCH 03/25] fscrypt: use FSCRYPT_* definitions, not FS_* Eric Biggers
@ 2017-10-27 18:06 ` Michael Halcrow
0 siblings, 0 replies; 33+ messages in thread
From: Michael Halcrow @ 2017-10-27 18:06 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
linux-mtd, linux-api, keyrings, Theodore Y . Ts'o,
Jaegeuk Kim, Gwendal Grignou, Ryo Hashimoto, Sarthak Kukreti,
Nick Desaulniers, Eric Biggers
On Mon, Oct 23, 2017 at 02:40:36PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Update the filesystem encryption kernel code to use the new names for
> the UAPI constants rather than the old names.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Michael Halcrow <mhalcrow@google.com>
> ---
> fs/crypto/fname.c | 4 ++--
> fs/crypto/fscrypt_private.h | 4 ++--
> fs/crypto/keyinfo.c | 36 +++++++++++++++++-------------------
> fs/crypto/policy.c | 14 +++++++-------
> include/linux/fscrypt.h | 8 ++++----
> 5 files changed, 32 insertions(+), 34 deletions(-)
>
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 2878289b3ed2..c91bcef65b9f 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 & FS_POLICY_FLAGS_PAD_MASK);
> + int padding = 4 << (ci->ci_flags & FSCRYPT_POLICY_FLAGS_PAD_MASK);
> 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 & FS_POLICY_FLAGS_PAD_MASK);
> + padding = 4 << (ci->ci_flags & FSCRYPT_POLICY_FLAGS_PAD_MASK);
> 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 a180981ee6d7..5cb80a2d39ea 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -43,7 +43,7 @@ struct fscrypt_context {
> 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];
> } __packed;
>
> @@ -59,7 +59,7 @@ struct fscrypt_info {
> u8 ci_flags;
> struct crypto_skcipher *ci_ctfm;
> struct crypto_cipher *ci_essiv_tfm;
> - u8 ci_master_key[FS_KEY_DESCRIPTOR_SIZE];
> + u8 ci_master_key[FSCRYPT_KEY_DESCRIPTOR_SIZE];
> };
>
> typedef enum {
> diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
> index 7aed93bcfb82..ac41f646e7b7 100644
> --- a/fs/crypto/keyinfo.c
> +++ b/fs/crypto/keyinfo.c
> @@ -38,7 +38,7 @@ static void derive_crypt_complete(struct crypto_async_request *req, int rc)
> */
> static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
> const struct fscrypt_key *source_key,
> - u8 derived_raw_key[FS_MAX_KEY_SIZE])
> + u8 derived_raw_key[FSCRYPT_MAX_KEY_SIZE])
> {
> int res = 0;
> struct skcipher_request *req = NULL;
> @@ -91,7 +91,7 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
> int res;
>
> description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
> - FS_KEY_DESCRIPTOR_SIZE,
> + FSCRYPT_KEY_DESCRIPTOR_SIZE,
> ctx->master_key_descriptor);
> if (!description)
> return -ENOMEM;
> @@ -121,7 +121,8 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
> 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 > FS_MAX_KEY_SIZE
> + if (master_key->size < min_keysize ||
> + master_key->size > FSCRYPT_MAX_KEY_SIZE
> || master_key->size % AES_BLOCK_SIZE != 0) {
Format nit: This makes the placement of "||" inconsistent across lines
in this same expression.
> printk_once(KERN_WARNING
> "%s: key size incorrect: %d\n",
> @@ -140,14 +141,10 @@ static const struct {
> const char *cipher_str;
> int keysize;
> } available_modes[] = {
> - [FS_ENCRYPTION_MODE_AES_256_XTS] = { "xts(aes)",
> - FS_AES_256_XTS_KEY_SIZE },
> - [FS_ENCRYPTION_MODE_AES_256_CTS] = { "cts(cbc(aes))",
> - FS_AES_256_CTS_KEY_SIZE },
> - [FS_ENCRYPTION_MODE_AES_128_CBC] = { "cbc(aes)",
> - FS_AES_128_CBC_KEY_SIZE },
> - [FS_ENCRYPTION_MODE_AES_128_CTS] = { "cts(cbc(aes))",
> - FS_AES_128_CTS_KEY_SIZE },
> + [FSCRYPT_MODE_AES_256_XTS] = { "xts(aes)", FS_AES_256_XTS_KEY_SIZE },
> + [FSCRYPT_MODE_AES_256_CTS] = { "cts(cbc(aes))", FS_AES_256_CTS_KEY_SIZE },
> + [FSCRYPT_MODE_AES_128_CBC] = { "cbc(aes)", FS_AES_128_CBC_KEY_SIZE },
> + [FSCRYPT_MODE_AES_128_CTS] = { "cts(cbc(aes))", FS_AES_128_CTS_KEY_SIZE },
> };
>
> static int determine_cipher_type(struct fscrypt_info *ci, struct inode *inode,
> @@ -278,9 +275,10 @@ int fscrypt_get_encryption_info(struct inode *inode)
> /* Fake up a context for an unencrypted directory */
> memset(&ctx, 0, sizeof(ctx));
> ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
> - ctx.contents_encryption_mode = FS_ENCRYPTION_MODE_AES_256_XTS;
> - ctx.filenames_encryption_mode = FS_ENCRYPTION_MODE_AES_256_CTS;
> - memset(ctx.master_key_descriptor, 0x42, FS_KEY_DESCRIPTOR_SIZE);
> + ctx.contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
> + ctx.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
> + memset(ctx.master_key_descriptor, 0x42,
> + FSCRYPT_KEY_DESCRIPTOR_SIZE);
> } else if (res != sizeof(ctx)) {
> return -EINVAL;
> }
> @@ -288,7 +286,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
> if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
> return -EINVAL;
>
> - if (ctx.flags & ~FS_POLICY_FLAGS_VALID)
> + if (ctx.flags & ~FSCRYPT_POLICY_FLAGS_VALID)
> return -EINVAL;
>
> crypt_info = kmem_cache_alloc(fscrypt_info_cachep, GFP_NOFS);
> @@ -312,12 +310,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
> * crypto API as part of key derivation.
> */
> res = -ENOMEM;
> - raw_key = kmalloc(FS_MAX_KEY_SIZE, GFP_NOFS);
> + raw_key = kmalloc(FSCRYPT_MAX_KEY_SIZE, GFP_NOFS);
> if (!raw_key)
> goto out;
>
> - res = validate_user_key(crypt_info, &ctx, raw_key, FS_KEY_DESC_PREFIX,
> - keysize);
> + 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,
> @@ -349,7 +347,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
> goto out;
>
> if (S_ISREG(inode->i_mode) &&
> - crypt_info->ci_data_mode == FS_ENCRYPTION_MODE_AES_128_CBC) {
> + crypt_info->ci_data_mode == FSCRYPT_MODE_AES_128_CBC) {
> res = init_essiv_generator(crypt_info, raw_key, keysize);
> if (res) {
> pr_debug("%s: error %d (inode %lu) allocating essiv tfm\n",
> diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
> index 6a63b8a0d46c..19332a6fd52d 100644
> --- a/fs/crypto/policy.c
> +++ b/fs/crypto/policy.c
> @@ -21,7 +21,7 @@ static bool is_encryption_context_consistent_with_policy(
> const struct fscrypt_policy *policy)
> {
> return memcmp(ctx->master_key_descriptor, policy->master_key_descriptor,
> - FS_KEY_DESCRIPTOR_SIZE) == 0 &&
> + FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
> (ctx->flags == policy->flags) &&
> (ctx->contents_encryption_mode ==
> policy->contents_encryption_mode) &&
> @@ -36,13 +36,13 @@ static int create_encryption_context_from_policy(struct inode *inode,
>
> ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
> memcpy(ctx.master_key_descriptor, policy->master_key_descriptor,
> - FS_KEY_DESCRIPTOR_SIZE);
> + FSCRYPT_KEY_DESCRIPTOR_SIZE);
>
> if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
> policy->filenames_encryption_mode))
> return -EINVAL;
>
> - if (policy->flags & ~FS_POLICY_FLAGS_VALID)
> + if (policy->flags & ~FSCRYPT_POLICY_FLAGS_VALID)
> return -EINVAL;
>
> ctx.contents_encryption_mode = policy->contents_encryption_mode;
> @@ -125,7 +125,7 @@ int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg)
> policy.filenames_encryption_mode = ctx.filenames_encryption_mode;
> policy.flags = ctx.flags;
> memcpy(policy.master_key_descriptor, ctx.master_key_descriptor,
> - FS_KEY_DESCRIPTOR_SIZE);
> + FSCRYPT_KEY_DESCRIPTOR_SIZE);
>
> if (copy_to_user(arg, &policy, sizeof(policy)))
> return -EFAULT;
> @@ -199,7 +199,7 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
>
> if (parent_ci && child_ci) {
> return memcmp(parent_ci->ci_master_key, child_ci->ci_master_key,
> - FS_KEY_DESCRIPTOR_SIZE) == 0 &&
> + 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) &&
> @@ -216,7 +216,7 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
>
> return memcmp(parent_ctx.master_key_descriptor,
> child_ctx.master_key_descriptor,
> - FS_KEY_DESCRIPTOR_SIZE) == 0 &&
> + FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
> (parent_ctx.contents_encryption_mode ==
> child_ctx.contents_encryption_mode) &&
> (parent_ctx.filenames_encryption_mode ==
> @@ -254,7 +254,7 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
> ctx.filenames_encryption_mode = ci->ci_filename_mode;
> ctx.flags = ci->ci_flags;
> memcpy(ctx.master_key_descriptor, ci->ci_master_key,
> - FS_KEY_DESCRIPTOR_SIZE);
> + 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,
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index f7aa7d62e235..671ce57e4673 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -99,12 +99,12 @@ static inline bool fscrypt_dummy_context_enabled(struct inode *inode)
> static inline bool fscrypt_valid_enc_modes(u32 contents_mode,
> u32 filenames_mode)
> {
> - if (contents_mode == FS_ENCRYPTION_MODE_AES_128_CBC &&
> - filenames_mode == FS_ENCRYPTION_MODE_AES_128_CTS)
> + if (contents_mode == FSCRYPT_MODE_AES_128_CBC &&
> + filenames_mode == FSCRYPT_MODE_AES_128_CTS)
> return true;
>
> - if (contents_mode == FS_ENCRYPTION_MODE_AES_256_XTS &&
> - filenames_mode == FS_ENCRYPTION_MODE_AES_256_CTS)
> + if (contents_mode == FSCRYPT_MODE_AES_256_XTS &&
> + filenames_mode == FSCRYPT_MODE_AES_256_CTS)
> return true;
>
> return false;
> --
> 2.15.0.rc0.271.g36b669edcc-goog
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 04/25] fscrypt: refactor finding and deriving key
2017-10-23 21:40 [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support Eric Biggers
` (2 preceding siblings ...)
2017-10-23 21:40 ` [RFC PATCH 03/25] fscrypt: use FSCRYPT_* definitions, not FS_* Eric Biggers
@ 2017-10-23 21:40 ` Eric Biggers
2017-10-27 18:23 ` Michael Halcrow
2017-10-23 21:40 ` [RFC PATCH 05/25] fs: add ->s_master_keys to struct super_block Eric Biggers
` (20 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Eric Biggers @ 2017-10-23 21:40 UTC (permalink / raw)
To: linux-fscrypt
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-mtd, linux-api,
keyrings, Theodore Y . Ts'o, Jaegeuk Kim, Gwendal Grignou,
Ryo Hashimoto, Sarthak Kukreti, Nick Desaulniers, Michael Halcrow,
Eric Biggers
From: Eric Biggers <ebiggers@google.com>
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 <ebiggers@google.com>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC PATCH 04/25] fscrypt: refactor finding and deriving key
2017-10-23 21:40 ` [RFC PATCH 04/25] fscrypt: refactor finding and deriving key Eric Biggers
@ 2017-10-27 18:23 ` Michael Halcrow
0 siblings, 0 replies; 33+ messages in thread
From: Michael Halcrow @ 2017-10-27 18:23 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
linux-mtd, linux-api, keyrings, Theodore Y . Ts'o,
Jaegeuk Kim, Gwendal Grignou, Ryo Hashimoto, Sarthak Kukreti,
Nick Desaulniers, Eric Biggers
On Mon, Oct 23, 2017 at 02:40:37PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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 <ebiggers@google.com>
Reviewed-by: Michael Halcrow <mhalcrow@google.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 05/25] fs: add ->s_master_keys to struct super_block
2017-10-23 21:40 [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support Eric Biggers
` (3 preceding siblings ...)
2017-10-23 21:40 ` [RFC PATCH 04/25] fscrypt: refactor finding and deriving key Eric Biggers
@ 2017-10-23 21:40 ` Eric Biggers
2017-10-27 18:26 ` Michael Halcrow
2017-10-23 21:40 ` [RFC PATCH 06/25] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl Eric Biggers
` (19 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Eric Biggers @ 2017-10-23 21:40 UTC (permalink / raw)
To: linux-fscrypt
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-mtd, linux-api,
keyrings, Theodore Y . Ts'o, Jaegeuk Kim, Gwendal Grignou,
Ryo Hashimoto, Sarthak Kukreti, Nick Desaulniers, Michael Halcrow,
Eric Biggers
From: Eric Biggers <ebiggers@google.com>
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 <ebiggers@google.com>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC PATCH 05/25] fs: add ->s_master_keys to struct super_block
2017-10-23 21:40 ` [RFC PATCH 05/25] fs: add ->s_master_keys to struct super_block Eric Biggers
@ 2017-10-27 18:26 ` Michael Halcrow
0 siblings, 0 replies; 33+ messages in thread
From: Michael Halcrow @ 2017-10-27 18:26 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
linux-mtd, linux-api, keyrings, Theodore Y . Ts'o,
Jaegeuk Kim, Gwendal Grignou, Ryo Hashimoto, Sarthak Kukreti,
Nick Desaulniers, Eric Biggers
On Mon, Oct 23, 2017 at 02:40:38PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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 <ebiggers@google.com>
Reviewed-by: Michael Halcrow <mhalcrow@google.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 06/25] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl
2017-10-23 21:40 [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support Eric Biggers
` (4 preceding siblings ...)
2017-10-23 21:40 ` [RFC PATCH 05/25] fs: add ->s_master_keys to struct super_block Eric Biggers
@ 2017-10-23 21:40 ` Eric Biggers
2017-10-27 20:14 ` Michael Halcrow
2017-10-23 21:40 ` [RFC PATCH 07/25] fs/inode.c: export inode_lru_list_del() Eric Biggers
` (18 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Eric Biggers @ 2017-10-23 21:40 UTC (permalink / raw)
To: linux-fscrypt
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-mtd, linux-api,
keyrings, Theodore Y . Ts'o, Jaegeuk Kim, Gwendal Grignou,
Ryo Hashimoto, Sarthak Kukreti, Nick Desaulniers, Michael Halcrow,
Eric Biggers
From: Eric Biggers <ebiggers@google.com>
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 <ebiggers@google.com>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC PATCH 06/25] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl
2017-10-23 21:40 ` [RFC PATCH 06/25] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl Eric Biggers
@ 2017-10-27 20:14 ` Michael Halcrow
0 siblings, 0 replies; 33+ messages in thread
From: Michael Halcrow @ 2017-10-27 20:14 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
linux-mtd, linux-api, keyrings, Theodore Y . Ts'o,
Jaegeuk Kim, Gwendal Grignou, Ryo Hashimoto, Sarthak Kukreti,
Nick Desaulniers, Eric Biggers
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 <ebiggers@google.com>
Reviewed-by: Michael Halcrow <mhalcrow@google.com>
> ---
> 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.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 07/25] fs/inode.c: export inode_lru_list_del()
2017-10-23 21:40 [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support Eric Biggers
` (5 preceding siblings ...)
2017-10-23 21:40 ` [RFC PATCH 06/25] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl Eric Biggers
@ 2017-10-23 21:40 ` Eric Biggers
2017-10-27 20:28 ` Michael Halcrow
2017-10-23 21:40 ` [RFC PATCH 08/25] fs/inode.c: rename and export dispose_list() Eric Biggers
` (17 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Eric Biggers @ 2017-10-23 21:40 UTC (permalink / raw)
To: linux-fscrypt
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-mtd, linux-api,
keyrings, Theodore Y . Ts'o, Jaegeuk Kim, Gwendal Grignou,
Ryo Hashimoto, Sarthak Kukreti, Nick Desaulniers, Michael Halcrow,
Eric Biggers
From: Eric Biggers <ebiggers@google.com>
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 <ebiggers@google.com>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC PATCH 07/25] fs/inode.c: export inode_lru_list_del()
2017-10-23 21:40 ` [RFC PATCH 07/25] fs/inode.c: export inode_lru_list_del() Eric Biggers
@ 2017-10-27 20:28 ` Michael Halcrow
0 siblings, 0 replies; 33+ messages in thread
From: Michael Halcrow @ 2017-10-27 20:28 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
linux-mtd, linux-api, keyrings, Theodore Y . Ts'o,
Jaegeuk Kim, Gwendal Grignou, Ryo Hashimoto, Sarthak Kukreti,
Nick Desaulniers, Eric Biggers
On Mon, Oct 23, 2017 at 02:40:40PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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 <ebiggers@google.com>
Reviewed-by: Michael Halcrow <mhalcrow@google.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 08/25] fs/inode.c: rename and export dispose_list()
2017-10-23 21:40 [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support Eric Biggers
` (6 preceding siblings ...)
2017-10-23 21:40 ` [RFC PATCH 07/25] fs/inode.c: export inode_lru_list_del() Eric Biggers
@ 2017-10-23 21:40 ` Eric Biggers
2017-10-23 21:40 ` [RFC PATCH 09/25] fs/dcache.c: add shrink_dcache_inode() Eric Biggers
` (16 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Eric Biggers @ 2017-10-23 21:40 UTC (permalink / raw)
To: linux-fscrypt
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-mtd, linux-api,
keyrings, Theodore Y . Ts'o, Jaegeuk Kim, Gwendal Grignou,
Ryo Hashimoto, Sarthak Kukreti, Nick Desaulniers, Michael Halcrow,
Eric Biggers
From: Eric Biggers <ebiggers@google.com>
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 <ebiggers@google.com>
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 09/25] fs/dcache.c: add shrink_dcache_inode()
2017-10-23 21:40 [RFC PATCH 00/25] fscrypt: filesystem-level keyring and v2 policy support Eric Biggers
` (7 preceding siblings ...)
2017-10-23 21:40 ` [RFC PATCH 08/25] fs/inode.c: rename and export dispose_list() Eric Biggers
@ 2017-10-23 21:40 ` Eric Biggers
2017-10-23 21:40 ` [RFC PATCH 10/25] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl Eric Biggers
` (15 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Eric Biggers @ 2017-10-23 21:40 UTC (permalink / raw)
To: linux-fscrypt
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-mtd, linux-api,
keyrings, Theodore Y . Ts'o, Jaegeuk Kim, Gwendal Grignou,
Ryo Hashimoto, Sarthak Kukreti, Nick Desaulniers, Michael Halcrow,
Eric Biggers
From: Eric Biggers <ebiggers@google.com>
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 <ebiggers@google.com>
---
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
^ permalink raw