* Re: [PATCH v5] KEYS: encrypted: Instantiate key with user-provided decrypted data
From: Sumit Garg @ 2022-02-21 5:47 UTC (permalink / raw)
To: Yael Tzur
Cc: linux-integrity, jejb, jarkko, zohar, corbet, dhowells, jmorris,
serge, keyrings, linux-doc, linux-kernel, linux-security-module
In-Reply-To: <20220215141953.1557009-1-yaelt@google.com>
On Tue, 15 Feb 2022 at 19:50, Yael Tzur <yaelt@google.com> wrote:
>
> For availability and performance reasons master keys often need to be
> released outside of a Key Management Service (KMS) to clients. It
> would be beneficial to provide a mechanism where the
> wrapping/unwrapping of data encryption keys (DEKs) is not dependent
> on a remote call at runtime yet security is not (or only minimally)
> compromised. Master keys could be securely stored in the Kernel and
> be used to wrap/unwrap keys from Userspace.
>
> The encrypted.c class supports instantiation of encrypted keys with
> either an already-encrypted key material, or by generating new key
> material based on random numbers. This patch defines a new datablob
> format: [<format>] <master-key name> <decrypted data length>
> <decrypted data> that allows to inject and encrypt user-provided
> decrypted data. The decrypted data must be hex-ascii encoded.
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Yael Tzur <yaelt@google.com>
> ---
>
> Notes:
> v -> v2: fixed compilation error.
>
> v2 -> v3: modified documentation.
>
> v3 -> v4: modified commit message.
>
> v4 -> v5: added config option to enable feature, and modified input validation.
>
Reviewed-by: Sumit Garg <sumit.garg@linaro.org>
-Sumit
> .../security/keys/trusted-encrypted.rst | 25 +++++--
> security/keys/Kconfig | 19 +++--
> security/keys/encrypted-keys/encrypted.c | 72 ++++++++++++++-----
> 3 files changed, 87 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
> index 80d5a5af62a1..f614dad7de12 100644
> --- a/Documentation/security/keys/trusted-encrypted.rst
> +++ b/Documentation/security/keys/trusted-encrypted.rst
> @@ -107,12 +107,13 @@ Encrypted Keys
> --------------
>
> Encrypted keys do not depend on a trust source, and are faster, as they use AES
> -for encryption/decryption. New keys are created from kernel-generated random
> -numbers, and are encrypted/decrypted using a specified ‘master’ key. The
> -‘master’ key can either be a trusted-key or user-key type. The main disadvantage
> -of encrypted keys is that if they are not rooted in a trusted key, they are only
> -as secure as the user key encrypting them. The master user key should therefore
> -be loaded in as secure a way as possible, preferably early in boot.
> +for encryption/decryption. New keys are created either from kernel-generated
> +random numbers or user-provided decrypted data, and are encrypted/decrypted
> +using a specified ‘master’ key. The ‘master’ key can either be a trusted-key or
> +user-key type. The main disadvantage of encrypted keys is that if they are not
> +rooted in a trusted key, they are only as secure as the user key encrypting
> +them. The master user key should therefore be loaded in as secure a way as
> +possible, preferably early in boot.
>
>
> Usage
> @@ -199,6 +200,8 @@ Usage::
>
> keyctl add encrypted name "new [format] key-type:master-key-name keylen"
> ring
> + keyctl add encrypted name "new [format] key-type:master-key-name keylen
> + decrypted-data" ring
> keyctl add encrypted name "load hex_blob" ring
> keyctl update keyid "update key-type:master-key-name"
>
> @@ -303,6 +306,16 @@ Load an encrypted key "evm" from saved blob::
> 82dbbc55be2a44616e4959430436dc4f2a7a9659aa60bb4652aeb2120f149ed197c564e0
> 24717c64 5972dcb82ab2dde83376d82b2e3c09ffc
>
> +Instantiate an encrypted key "evm" using user-provided decrypted data::
> +
> + $ keyctl add encrypted evm "new default user:kmk 32 `cat evm_decrypted_data.blob`" @u
> + 794890253
> +
> + $ keyctl print 794890253
> + default user:kmk 32 2375725ad57798846a9bbd240de8906f006e66c03af53b1b382d
> + bbc55be2a44616e4959430436dc4f2a7a9659aa60bb4652aeb2120f149ed197c564e0247
> + 17c64 5972dcb82ab2dde83376d82b2e3c09ffc
> +
> Other uses for trusted and encrypted keys, such as for disk and file encryption
> are anticipated. In particular the new format 'ecryptfs' has been defined
> in order to use encrypted keys to mount an eCryptfs filesystem. More details
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index 969122c7b92f..0e30b361e1c1 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -98,10 +98,21 @@ config ENCRYPTED_KEYS
> select CRYPTO_RNG
> help
> This option provides support for create/encrypting/decrypting keys
> - in the kernel. Encrypted keys are kernel generated random numbers,
> - which are encrypted/decrypted with a 'master' symmetric key. The
> - 'master' key can be either a trusted-key or user-key type.
> - Userspace only ever sees/stores encrypted blobs.
> + in the kernel. Encrypted keys are instantiated using kernel
> + generated random numbers or provided decrypted data, and are
> + encrypted/decrypted with a 'master' symmetric key. The 'master'
> + key can be either a trusted-key or user-key type. Only encrypted
> + blobs are ever output to Userspace.
> +
> + If you are unsure as to whether this is required, answer N.
> +
> +config USER_DECRYPTED_DATA
> + bool "Allow encrypted keys with user decrypted data"
> + depends on ENCRYPTED_KEYS
> + help
> + This option provides support for instantiating encrypted keys using
> + user-provided decrypted data. The decrypted data must be hex-ascii
> + encoded.
>
> If you are unsure as to whether this is required, answer N.
>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 87432b35d771..ebfb8129fb92 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -78,6 +78,11 @@ static const match_table_t key_tokens = {
> {Opt_err, NULL}
> };
>
> +static bool user_decrypted_data = IS_ENABLED(CONFIG_USER_DECRYPTED_DATA);
> +module_param(user_decrypted_data, bool, 0);
> +MODULE_PARM_DESC(user_decrypted_data,
> + "Allow instantiation of encrypted keys using provided decrypted data");
> +
> static int aes_get_sizes(void)
> {
> struct crypto_skcipher *tfm;
> @@ -158,7 +163,7 @@ static int valid_master_desc(const char *new_desc, const char *orig_desc)
> * datablob_parse - parse the keyctl data
> *
> * datablob format:
> - * new [<format>] <master-key name> <decrypted data length>
> + * new [<format>] <master-key name> <decrypted data length> [<decrypted data>]
> * load [<format>] <master-key name> <decrypted data length>
> * <encrypted iv + data>
> * update <new-master-key name>
> @@ -170,7 +175,7 @@ static int valid_master_desc(const char *new_desc, const char *orig_desc)
> */
> static int datablob_parse(char *datablob, const char **format,
> char **master_desc, char **decrypted_datalen,
> - char **hex_encoded_iv)
> + char **hex_encoded_iv, char **decrypted_data)
> {
> substring_t args[MAX_OPT_ARGS];
> int ret = -EINVAL;
> @@ -231,6 +236,7 @@ static int datablob_parse(char *datablob, const char **format,
> "when called from .update method\n", keyword);
> break;
> }
> + *decrypted_data = strsep(&datablob, " \t");
> ret = 0;
> break;
> case Opt_load:
> @@ -595,7 +601,8 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload,
> static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
> const char *format,
> const char *master_desc,
> - const char *datalen)
> + const char *datalen,
> + const char *decrypted_data)
> {
> struct encrypted_key_payload *epayload = NULL;
> unsigned short datablob_len;
> @@ -604,6 +611,7 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
> unsigned int encrypted_datalen;
> unsigned int format_len;
> long dlen;
> + int i;
> int ret;
>
> ret = kstrtol(datalen, 10, &dlen);
> @@ -613,6 +620,24 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
> format_len = (!format) ? strlen(key_format_default) : strlen(format);
> decrypted_datalen = dlen;
> payload_datalen = decrypted_datalen;
> +
> + if (decrypted_data) {
> + if (!user_decrypted_data) {
> + pr_err("encrypted key: instantiation of keys using provided decrypted data is disabled since CONFIG_USER_DECRYPTED_DATA is set to false\n");
> + return ERR_PTR(-EINVAL);
> + }
> + if (strlen(decrypted_data) != decrypted_datalen) {
> + pr_err("encrypted key: decrypted data provided does not match decrypted data length provided\n");
> + return ERR_PTR(-EINVAL);
> + }
> + for (i = 0; i < strlen(decrypted_data); i++) {
> + if (!isxdigit(decrypted_data[i])) {
> + pr_err("encrypted key: decrypted data provided must contain only hexadecimal characters\n");
> + return ERR_PTR(-EINVAL);
> + }
> + }
> + }
> +
> if (format) {
> if (!strcmp(format, key_format_ecryptfs)) {
> if (dlen != ECRYPTFS_MAX_KEY_BYTES) {
> @@ -740,13 +766,14 @@ static void __ekey_init(struct encrypted_key_payload *epayload,
> /*
> * encrypted_init - initialize an encrypted key
> *
> - * For a new key, use a random number for both the iv and data
> - * itself. For an old key, decrypt the hex encoded data.
> + * For a new key, use either a random number or user-provided decrypted data in
> + * case it is provided. A random number is used for the iv in both cases. For
> + * an old key, decrypt the hex encoded data.
> */
> static int encrypted_init(struct encrypted_key_payload *epayload,
> const char *key_desc, const char *format,
> const char *master_desc, const char *datalen,
> - const char *hex_encoded_iv)
> + const char *hex_encoded_iv, const char *decrypted_data)
> {
> int ret = 0;
>
> @@ -760,21 +787,26 @@ static int encrypted_init(struct encrypted_key_payload *epayload,
> }
>
> __ekey_init(epayload, format, master_desc, datalen);
> - if (!hex_encoded_iv) {
> - get_random_bytes(epayload->iv, ivsize);
> -
> - get_random_bytes(epayload->decrypted_data,
> - epayload->decrypted_datalen);
> - } else
> + if (hex_encoded_iv) {
> ret = encrypted_key_decrypt(epayload, format, hex_encoded_iv);
> + } else if (decrypted_data) {
> + get_random_bytes(epayload->iv, ivsize);
> + memcpy(epayload->decrypted_data, decrypted_data,
> + epayload->decrypted_datalen);
> + } else {
> + get_random_bytes(epayload->iv, ivsize);
> + get_random_bytes(epayload->decrypted_data, epayload->decrypted_datalen);
> + }
> return ret;
> }
>
> /*
> * encrypted_instantiate - instantiate an encrypted key
> *
> - * Decrypt an existing encrypted datablob or create a new encrypted key
> - * based on a kernel random number.
> + * Instantiates the key:
> + * - by decrypting an existing encrypted datablob, or
> + * - by creating a new encrypted key based on a kernel random number, or
> + * - using provided decrypted data.
> *
> * On success, return 0. Otherwise return errno.
> */
> @@ -787,6 +819,7 @@ static int encrypted_instantiate(struct key *key,
> char *master_desc = NULL;
> char *decrypted_datalen = NULL;
> char *hex_encoded_iv = NULL;
> + char *decrypted_data = NULL;
> size_t datalen = prep->datalen;
> int ret;
>
> @@ -799,18 +832,18 @@ static int encrypted_instantiate(struct key *key,
> datablob[datalen] = 0;
> memcpy(datablob, prep->data, datalen);
> ret = datablob_parse(datablob, &format, &master_desc,
> - &decrypted_datalen, &hex_encoded_iv);
> + &decrypted_datalen, &hex_encoded_iv, &decrypted_data);
> if (ret < 0)
> goto out;
>
> epayload = encrypted_key_alloc(key, format, master_desc,
> - decrypted_datalen);
> + decrypted_datalen, decrypted_data);
> if (IS_ERR(epayload)) {
> ret = PTR_ERR(epayload);
> goto out;
> }
> ret = encrypted_init(epayload, key->description, format, master_desc,
> - decrypted_datalen, hex_encoded_iv);
> + decrypted_datalen, hex_encoded_iv, decrypted_data);
> if (ret < 0) {
> kfree_sensitive(epayload);
> goto out;
> @@ -860,7 +893,7 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep)
>
> buf[datalen] = 0;
> memcpy(buf, prep->data, datalen);
> - ret = datablob_parse(buf, &format, &new_master_desc, NULL, NULL);
> + ret = datablob_parse(buf, &format, &new_master_desc, NULL, NULL, NULL);
> if (ret < 0)
> goto out;
>
> @@ -869,7 +902,7 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep)
> goto out;
>
> new_epayload = encrypted_key_alloc(key, epayload->format,
> - new_master_desc, epayload->datalen);
> + new_master_desc, epayload->datalen, NULL);
> if (IS_ERR(new_epayload)) {
> ret = PTR_ERR(new_epayload);
> goto out;
> --
> 2.35.1.265.g69c8d7142f-goog
>
^ permalink raw reply
* Re: [PATCH v8 0/3] integrity: support including firmware ".platform" keys at build time
From: Nayna @ 2022-02-21 13:33 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, keyrings, dhowells, zohar, linux-security-module,
linux-kernel, dimitri.ledkov, seth, Nayna Jain
In-Reply-To: <YhLIhJF0aWZt+8op@iki.fi>
On 2/20/22 18:02, Jarkko Sakkinen wrote:
> On Tue, Jan 11, 2022 at 01:36:44PM -0500, Nayna Jain wrote:
>> Some firmware support secure boot by embedding static keys to verify the
>> Linux kernel during boot. However, these firmware do not expose an
>> interface for the kernel to load firmware keys onto the ".platform"
>> keyring, preventing the kernel from verifying the kexec kernel image
>> signature.
>>
>> This patchset exports load_certificate_list() and defines a new function
>> load_builtin_platform_cert() to load compiled in certificates onto the
>> ".platform" keyring.
>>
>> Changelog:
>>
>> v8:
>> * Includes Jarkko's feedback on patch description and removed Reported-by
>> for Patch 1.
>>
>> v7:
>> * Incldues Jarkko's feedback on patch description for Patch 1 and 3.
>>
>> v6:
>> * Includes Jarkko's feedback:
>> * Split Patch 2 into two.
>> * Update Patch description.
>>
>> v5:
>> * Renamed load_builtin_platform_cert() to load_platform_certificate_list()
>> and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as
>> suggested by Mimi Zohar.
>>
>> v4:
>> * Split into two patches as per Mimi Zohar and Dimitri John Ledkov
>> recommendation.
>>
>> v3:
>> * Included Jarkko's feedback
>> ** updated patch description to include approach.
>> ** removed extern for function declaration in the .h file.
>> * Included load_certificate_list() within #ifdef CONFIG_KEYS condition.
>>
>> v2:
>> * Fixed the error reported by kernel test robot
>> * Updated patch description based on Jarkko's feedback.
>>
>> Nayna Jain (3):
>> certs: export load_certificate_list() to be used outside certs/
>> integrity: make integrity_keyring_from_id() non-static
>> integrity: support including firmware ".platform" keys at build time
>>
>> certs/Makefile | 5 ++--
>> certs/blacklist.c | 1 -
>> certs/common.c | 2 +-
>> certs/common.h | 9 -------
>> certs/system_keyring.c | 1 -
>> include/keys/system_keyring.h | 6 +++++
>> security/integrity/Kconfig | 10 +++++++
>> security/integrity/Makefile | 17 +++++++++++-
>> security/integrity/digsig.c | 2 +-
>> security/integrity/integrity.h | 6 +++++
>> .../integrity/platform_certs/platform_cert.S | 23 ++++++++++++++++
>> .../platform_certs/platform_keyring.c | 26 +++++++++++++++++++
>> 12 files changed, 92 insertions(+), 16 deletions(-)
>> delete mode 100644 certs/common.h
>> create mode 100644 security/integrity/platform_certs/platform_cert.S
>>
>> --
>> 2.27.0
> To sort out tree conflicts: what if I pick these patches? They look fine
> to me now. I can try to fix the possible merge conflicts and you can check
> them before I make a PR.
Sounds good. Thanks !!
Thanks & Regards,
- Nayna
^ permalink raw reply
* [PATCH v1 2/7] landlock: Fix landlock_add_rule(2) signature
From: Mickaël Salaün @ 2022-02-21 15:53 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Jann Horn, Kees Cook,
Konstantin Meskhidze, Nathan Chancellor, Nick Desaulniers,
Paul Moore, Shuah Khan, linux-api, linux-kernel,
linux-security-module, Alejandro Colomar,
Mickaël Salaün
In-Reply-To: <20220221155311.166278-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Replace the enum landlock_rule_type with an int in the syscall signature
of landlock_add_rule to avoid an implementation-defined size. In
practice an enum type is like an int (at least with GCC and clang), but
compilers may accept options (e.g. -fshort-enums) that would have an
impact on that [1]. This change is mostly a cosmetic fix according to
the current kernel compilers and used options.
Link: https://lore.kernel.org/r/8a22a3c2-468c-e96c-6516-22a0f029aa34@gmail.com/ [1]
Reported-by: Alejandro Colomar <alx.manpages@gmail.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-3-mic@digikod.net
---
include/linux/syscalls.h | 3 +--
security/landlock/syscalls.c | 7 ++++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 819c0cb00b6d..a5956f91caf2 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -71,7 +71,6 @@ struct clone_args;
struct open_how;
struct mount_attr;
struct landlock_ruleset_attr;
-enum landlock_rule_type;
#include <linux/types.h>
#include <linux/aio_abi.h>
@@ -1053,7 +1052,7 @@ asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,
asmlinkage long sys_pidfd_getfd(int pidfd, int fd, unsigned int flags);
asmlinkage long sys_landlock_create_ruleset(const struct landlock_ruleset_attr __user *attr,
size_t size, __u32 flags);
-asmlinkage long sys_landlock_add_rule(int ruleset_fd, enum landlock_rule_type rule_type,
+asmlinkage long sys_landlock_add_rule(int ruleset_fd, int rule_type,
const void __user *rule_attr, __u32 flags);
asmlinkage long sys_landlock_restrict_self(int ruleset_fd, __u32 flags);
asmlinkage long sys_memfd_secret(unsigned int flags);
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index fd4b24022a06..3b40fc5d0216 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -277,8 +277,9 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
*
* @ruleset_fd: File descriptor tied to the ruleset that should be extended
* with the new rule.
- * @rule_type: Identify the structure type pointed to by @rule_attr (only
- * LANDLOCK_RULE_PATH_BENEATH for now).
+ * @rule_type: Identify the structure type pointed to by @rule_attr as defined
+ * by enum landlock_rule_type (only LANDLOCK_RULE_PATH_BENEATH for
+ * now).
* @rule_attr: Pointer to a rule (only of type &struct
* landlock_path_beneath_attr for now).
* @flags: Must be 0.
@@ -301,7 +302,7 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
* - EFAULT: @rule_attr inconsistency.
*/
SYSCALL_DEFINE4(landlock_add_rule,
- const int, ruleset_fd, const enum landlock_rule_type, rule_type,
+ const int, ruleset_fd, const int, rule_type,
const void __user *const, rule_attr, const __u32, flags)
{
struct landlock_path_beneath_attr path_beneath_attr;
--
2.35.1
^ permalink raw reply related
* [PATCH v1 0/7] Minor Landlock fixes and new tests
From: Mickaël Salaün @ 2022-02-21 15:53 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Jann Horn, Kees Cook,
Konstantin Meskhidze, Nathan Chancellor, Nick Desaulniers,
Paul Moore, Shuah Khan, linux-api, linux-kernel,
linux-security-module
Hi,
This series contains landlock_add_rule(2) (cosmetic) signature and
documentation fixes. There is also some miscellaneous new tests to
improve coverage and that may help for future access types (e.g.
networking).
Regards,
Mickaël Salaün (7):
landlock: Fix landlock_add_rule(2) documentation
landlock: Fix landlock_add_rule(2) signature
selftest/landlock: Make tests build with old libc
selftest/landlock: Extend tests for minimal valid attribute size
selftest/landlock: Add tests for unknown access rights
selftest/landlock: Extend access right tests to directories
selftest/landlock: Fully test file rename with "remove" access
include/linux/syscalls.h | 3 +-
include/uapi/linux/landlock.h | 5 +-
security/landlock/syscalls.c | 14 ++--
tools/testing/selftests/landlock/base_test.c | 5 ++
tools/testing/selftests/landlock/fs_test.c | 84 ++++++++++++++++----
5 files changed, 86 insertions(+), 25 deletions(-)
base-commit: cfb92440ee71adcc2105b0890bb01ac3cddb8507
--
2.35.1
^ permalink raw reply
* [PATCH v1 1/7] landlock: Fix landlock_add_rule(2) documentation
From: Mickaël Salaün @ 2022-02-21 15:53 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Jann Horn, Kees Cook,
Konstantin Meskhidze, Nathan Chancellor, Nick Desaulniers,
Paul Moore, Shuah Khan, linux-api, linux-kernel,
linux-security-module, Mickaël Salaün
In-Reply-To: <20220221155311.166278-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
It is not mandatory to pass a file descriptor obtained with the O_PATH
flag. Also, replace rule's accesses with ruleset's accesses.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-2-mic@digikod.net
---
include/uapi/linux/landlock.h | 5 +++--
security/landlock/syscalls.c | 7 +++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index b3d952067f59..c0390e318a65 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -60,8 +60,9 @@ struct landlock_path_beneath_attr {
*/
__u64 allowed_access;
/**
- * @parent_fd: File descriptor, open with ``O_PATH``, which identifies
- * the parent directory of a file hierarchy, or just a file.
+ * @parent_fd: File descriptor, preferably opened with ``O_PATH``,
+ * which identifies the parent directory of a file hierarchy, or just a
+ * file.
*/
__s32 parent_fd;
/*
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 32396962f04d..fd4b24022a06 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -290,14 +290,13 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
*
* - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
* - EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
- * &landlock_path_beneath_attr.allowed_access is not a subset of the rule's
- * accesses);
+ * &landlock_path_beneath_attr.allowed_access is not a subset of the
+ * ruleset handled accesses);
* - ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
* - EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
* member of @rule_attr is not a file descriptor as expected;
* - EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
- * @rule_attr is not the expected file descriptor type (e.g. file open
- * without O_PATH);
+ * @rule_attr is not the expected file descriptor type;
* - EPERM: @ruleset_fd has no write access to the underlying ruleset;
* - EFAULT: @rule_attr inconsistency.
*/
--
2.35.1
^ permalink raw reply related
* [PATCH v1 6/7] selftest/landlock: Extend access right tests to directories
From: Mickaël Salaün @ 2022-02-21 15:53 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Jann Horn, Kees Cook,
Konstantin Meskhidze, Nathan Chancellor, Nick Desaulniers,
Paul Moore, Shuah Khan, linux-api, linux-kernel,
linux-security-module, Mickaël Salaün
In-Reply-To: <20220221155311.166278-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Make sure that all filesystem access rights can be tied to directories.
Rename layout1/file_access_rights to layout1/file_and_dir_access_rights
to reflect this change.
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-7-mic@digikod.net
---
tools/testing/selftests/landlock/fs_test.c | 29 ++++++++++++++++------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 5506472a46ce..3736253c9582 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -409,11 +409,12 @@ TEST_F_FORK(layout1, inval)
LANDLOCK_ACCESS_FS_MAKE_BLOCK | \
ACCESS_LAST)
-TEST_F_FORK(layout1, file_access_rights)
+TEST_F_FORK(layout1, file_and_dir_access_rights)
{
__u64 access;
int err;
- struct landlock_path_beneath_attr path_beneath = {};
+ struct landlock_path_beneath_attr path_beneath_file = {},
+ path_beneath_dir = {};
struct landlock_ruleset_attr ruleset_attr = {
.handled_access_fs = ACCESS_ALL,
};
@@ -423,20 +424,32 @@ TEST_F_FORK(layout1, file_access_rights)
ASSERT_LE(0, ruleset_fd);
/* Tests access rights for files. */
- path_beneath.parent_fd = open(file1_s1d2, O_PATH | O_CLOEXEC);
- ASSERT_LE(0, path_beneath.parent_fd);
+ path_beneath_file.parent_fd = open(file1_s1d2, O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, path_beneath_file.parent_fd);
+
+ /* Tests access rights for directories. */
+ path_beneath_dir.parent_fd = open(dir_s1d2, O_PATH | O_DIRECTORY |
+ O_CLOEXEC);
+ ASSERT_LE(0, path_beneath_dir.parent_fd);
+
for (access = 1; access <= ACCESS_LAST; access <<= 1) {
- path_beneath.allowed_access = access;
+ path_beneath_dir.allowed_access = access;
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &path_beneath_dir, 0));
+
+ path_beneath_file.allowed_access = access;
err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
- &path_beneath, 0);
- if ((access | ACCESS_FILE) == ACCESS_FILE) {
+ &path_beneath_file, 0);
+ if (access & ACCESS_FILE) {
ASSERT_EQ(0, err);
} else {
ASSERT_EQ(-1, err);
ASSERT_EQ(EINVAL, errno);
}
}
- ASSERT_EQ(0, close(path_beneath.parent_fd));
+ ASSERT_EQ(0, close(path_beneath_file.parent_fd));
+ ASSERT_EQ(0, close(path_beneath_dir.parent_fd));
+ ASSERT_EQ(0, close(ruleset_fd));
}
TEST_F_FORK(layout1, unknown_access_rights)
--
2.35.1
^ permalink raw reply related
* [PATCH v1 3/7] selftest/landlock: Make tests build with old libc
From: Mickaël Salaün @ 2022-02-21 15:53 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Jann Horn, Kees Cook,
Konstantin Meskhidze, Nathan Chancellor, Nick Desaulniers,
Paul Moore, Shuah Khan, linux-api, linux-kernel,
linux-security-module, Mickaël Salaün
In-Reply-To: <20220221155311.166278-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Replace SYS_<syscall> with __NR_<syscall>. Using the __NR_<syscall>
notation, provided by UAPI, is useful to build tests on systems without
the SYS_<syscall> definitions.
Replace SYS_pivot_root with __NR_pivot_root, and SYS_move_mount with
__NR_move_mount.
Define renameat2() and RENAME_EXCHANGE if they are unknown to old build
systems.
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-4-mic@digikod.net
---
tools/testing/selftests/landlock/fs_test.c | 23 +++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 10c9a1e4ebd9..699cda25a12a 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -22,6 +22,19 @@
#include "common.h"
+#ifndef renameat2
+int renameat2(int olddirfd, const char *oldpath, int newdirfd,
+ const char *newpath, unsigned int flags)
+{
+ return syscall(__NR_renameat2, olddirfd, oldpath, newdirfd, newpath,
+ flags);
+}
+#endif
+
+#ifndef RENAME_EXCHANGE
+#define RENAME_EXCHANGE (1 << 1)
+#endif
+
#define TMP_DIR "tmp"
#define BINARY_PATH "./true"
@@ -1249,7 +1262,7 @@ TEST_F_FORK(layout1, rule_inside_mount_ns)
int ruleset_fd;
set_cap(_metadata, CAP_SYS_ADMIN);
- ASSERT_EQ(0, syscall(SYS_pivot_root, dir_s3d2, dir_s3d3)) {
+ ASSERT_EQ(0, syscall(__NR_pivot_root, dir_s3d2, dir_s3d3)) {
TH_LOG("Failed to pivot root: %s", strerror(errno));
};
ASSERT_EQ(0, chdir("/"));
@@ -1282,7 +1295,7 @@ TEST_F_FORK(layout1, mount_and_pivot)
set_cap(_metadata, CAP_SYS_ADMIN);
ASSERT_EQ(-1, mount(NULL, dir_s3d2, NULL, MS_RDONLY, NULL));
ASSERT_EQ(EPERM, errno);
- ASSERT_EQ(-1, syscall(SYS_pivot_root, dir_s3d2, dir_s3d3));
+ ASSERT_EQ(-1, syscall(__NR_pivot_root, dir_s3d2, dir_s3d3));
ASSERT_EQ(EPERM, errno);
clear_cap(_metadata, CAP_SYS_ADMIN);
}
@@ -1301,12 +1314,12 @@ TEST_F_FORK(layout1, move_mount)
ASSERT_LE(0, ruleset_fd);
set_cap(_metadata, CAP_SYS_ADMIN);
- ASSERT_EQ(0, syscall(SYS_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD,
+ ASSERT_EQ(0, syscall(__NR_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD,
dir_s1d2, 0)) {
TH_LOG("Failed to move mount: %s", strerror(errno));
}
- ASSERT_EQ(0, syscall(SYS_move_mount, AT_FDCWD, dir_s1d2, AT_FDCWD,
+ ASSERT_EQ(0, syscall(__NR_move_mount, AT_FDCWD, dir_s1d2, AT_FDCWD,
dir_s3d2, 0));
clear_cap(_metadata, CAP_SYS_ADMIN);
@@ -1314,7 +1327,7 @@ TEST_F_FORK(layout1, move_mount)
ASSERT_EQ(0, close(ruleset_fd));
set_cap(_metadata, CAP_SYS_ADMIN);
- ASSERT_EQ(-1, syscall(SYS_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD,
+ ASSERT_EQ(-1, syscall(__NR_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD,
dir_s1d2, 0));
ASSERT_EQ(EPERM, errno);
clear_cap(_metadata, CAP_SYS_ADMIN);
--
2.35.1
^ permalink raw reply related
* [PATCH v1 4/7] selftest/landlock: Extend tests for minimal valid attribute size
From: Mickaël Salaün @ 2022-02-21 15:53 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Jann Horn, Kees Cook,
Konstantin Meskhidze, Nathan Chancellor, Nick Desaulniers,
Paul Moore, Shuah Khan, linux-api, linux-kernel,
linux-security-module, Mickaël Salaün
In-Reply-To: <20220221155311.166278-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
This might be useful when the struct landlock_ruleset_attr will get more
fields.
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-5-mic@digikod.net
---
tools/testing/selftests/landlock/base_test.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index ca40abe9daa8..38fa1e0dfa33 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -34,6 +34,8 @@ TEST(inconsistent_attr) {
ASSERT_EQ(EINVAL, errno);
ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr, 1, 0));
ASSERT_EQ(EINVAL, errno);
+ ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr, 7, 0));
+ ASSERT_EQ(EINVAL, errno);
ASSERT_EQ(-1, landlock_create_ruleset(NULL, 1, 0));
/* The size if less than sizeof(struct landlock_attr_enforce). */
@@ -46,6 +48,9 @@ TEST(inconsistent_attr) {
ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr, page_size + 1, 0));
ASSERT_EQ(E2BIG, errno);
+ /* Checks minimal valid attribute size. */
+ ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr, 8, 0));
+ ASSERT_EQ(ENOMSG, errno);
ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr,
sizeof(struct landlock_ruleset_attr), 0));
ASSERT_EQ(ENOMSG, errno);
--
2.35.1
^ permalink raw reply related
* [PATCH v1 7/7] selftest/landlock: Fully test file rename with "remove" access
From: Mickaël Salaün @ 2022-02-21 15:53 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Jann Horn, Kees Cook,
Konstantin Meskhidze, Nathan Chancellor, Nick Desaulniers,
Paul Moore, Shuah Khan, linux-api, linux-kernel,
linux-security-module, Mickaël Salaün
In-Reply-To: <20220221155311.166278-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
These tests were missing to check the check_access_path() call with all
combinations of maybe_remove(old_dentry) and maybe_remove(new_dentry).
Extend layout1/link with a new complementary test.
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-8-mic@digikod.net
---
tools/testing/selftests/landlock/fs_test.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 3736253c9582..62b88406419d 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -1640,11 +1640,14 @@ TEST_F_FORK(layout1, link)
ASSERT_EQ(-1, link(file2_s1d1, file1_s1d1));
ASSERT_EQ(EACCES, errno);
+
/* Denies linking because of reparenting. */
ASSERT_EQ(-1, link(file1_s2d1, file1_s1d2));
ASSERT_EQ(EXDEV, errno);
ASSERT_EQ(-1, link(file2_s1d2, file1_s1d3));
ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, link(file2_s1d3, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);
ASSERT_EQ(0, link(file2_s1d2, file1_s1d2));
ASSERT_EQ(0, link(file2_s1d3, file1_s1d3));
@@ -1668,7 +1671,6 @@ TEST_F_FORK(layout1, rename_file)
ASSERT_LE(0, ruleset_fd);
- ASSERT_EQ(0, unlink(file1_s1d1));
ASSERT_EQ(0, unlink(file1_s1d2));
enforce_ruleset(_metadata, ruleset_fd);
@@ -1704,9 +1706,15 @@ TEST_F_FORK(layout1, rename_file)
ASSERT_EQ(-1, renameat2(AT_FDCWD, dir_s2d2, AT_FDCWD, file1_s2d1,
RENAME_EXCHANGE));
ASSERT_EQ(EACCES, errno);
+ /* Checks that file1_s2d1 cannot be removed (instead of ENOTDIR). */
+ ASSERT_EQ(-1, rename(dir_s2d2, file1_s2d1));
+ ASSERT_EQ(EACCES, errno);
ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s2d1, AT_FDCWD, dir_s2d2,
RENAME_EXCHANGE));
ASSERT_EQ(EACCES, errno);
+ /* Checks that file1_s1d1 cannot be removed (instead of EISDIR). */
+ ASSERT_EQ(-1, rename(file1_s1d1, dir_s1d2));
+ ASSERT_EQ(EACCES, errno);
/* Renames files with different parents. */
ASSERT_EQ(-1, rename(file1_s2d2, file1_s1d2));
@@ -1769,9 +1777,15 @@ TEST_F_FORK(layout1, rename_dir)
ASSERT_EQ(-1, renameat2(AT_FDCWD, dir_s1d1, AT_FDCWD, dir_s2d1,
RENAME_EXCHANGE));
ASSERT_EQ(EACCES, errno);
+ /* Checks that dir_s1d2 cannot be removed (instead of ENOTDIR). */
+ ASSERT_EQ(-1, rename(dir_s1d2, file1_s1d1));
+ ASSERT_EQ(EACCES, errno);
ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s1d1, AT_FDCWD, dir_s1d2,
RENAME_EXCHANGE));
ASSERT_EQ(EACCES, errno);
+ /* Checks that dir_s1d2 cannot be removed (instead of EISDIR). */
+ ASSERT_EQ(-1, rename(file1_s1d1, dir_s1d2));
+ ASSERT_EQ(EACCES, errno);
/*
* Exchanges and renames directory to the same parent, which allows
--
2.35.1
^ permalink raw reply related
* [PATCH v1 5/7] selftest/landlock: Add tests for unknown access rights
From: Mickaël Salaün @ 2022-02-21 15:53 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Jann Horn, Kees Cook,
Konstantin Meskhidze, Nathan Chancellor, Nick Desaulniers,
Paul Moore, Shuah Khan, linux-api, linux-kernel,
linux-security-module, Mickaël Salaün
In-Reply-To: <20220221155311.166278-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Make sure that trying to use unknown access rights returns an error.
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-6-mic@digikod.net
---
tools/testing/selftests/landlock/fs_test.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 699cda25a12a..5506472a46ce 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -439,6 +439,22 @@ TEST_F_FORK(layout1, file_access_rights)
ASSERT_EQ(0, close(path_beneath.parent_fd));
}
+TEST_F_FORK(layout1, unknown_access_rights)
+{
+ __u64 access_mask;
+
+ for (access_mask = 1ULL << 63; access_mask != ACCESS_LAST;
+ access_mask >>= 1) {
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = access_mask,
+ };
+
+ ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0));
+ ASSERT_EQ(EINVAL, errno);
+ }
+}
+
static void add_path_beneath(struct __test_metadata *const _metadata,
const int ruleset_fd, const __u64 allowed_access,
const char *const path)
--
2.35.1
^ permalink raw reply related
* Re: [PATCH v5] KEYS: encrypted: Instantiate key with user-provided decrypted data
From: Jarkko Sakkinen @ 2022-02-21 20:19 UTC (permalink / raw)
To: Mimi Zohar
Cc: Yael Tzur, linux-integrity, jejb, corbet, dhowells, jmorris,
serge, keyrings, linux-doc, linux-kernel, linux-security-module
In-Reply-To: <0aa47dfaada88f1cbd2162784f8b77f43566f626.camel@linux.ibm.com>
On Mon, Feb 21, 2022 at 12:11:22AM -0500, Mimi Zohar wrote:
> On Tue, 2022-02-15 at 09:19 -0500, Yael Tzur wrote:
> > For availability and performance reasons master keys often need to be
> > released outside of a Key Management Service (KMS) to clients. It
> > would be beneficial to provide a mechanism where the
> > wrapping/unwrapping of data encryption keys (DEKs) is not dependent
> > on a remote call at runtime yet security is not (or only minimally)
> > compromised. Master keys could be securely stored in the Kernel and
> > be used to wrap/unwrap keys from Userspace.
> >
> > The encrypted.c class supports instantiation of encrypted keys with
> > either an already-encrypted key material, or by generating new key
> > material based on random numbers. This patch defines a new datablob
> > format: [<format>] <master-key name> <decrypted data length>
> > <decrypted data> that allows to inject and encrypt user-provided
> > decrypted data. The decrypted data must be hex-ascii encoded.
> >
> > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > Signed-off-by: Yael Tzur <yaelt@google.com>
>
> Thanks, Yael.
>
> This patch is now queued in the #next-integrity-testing branch.
>
> --
> thanks,
>
> Mimi
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* [PATCH v1 01/11] landlock: Define access_mask_t to enforce a consistent access mask size
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Create and use the access_mask_t typedef to enforce a consistent access
mask size and uniformly use a 16-bits type. This will helps transition
to a 32-bits value one day.
Add a build check to make sure all (filesystem) access rights fit in.
This will be extended with a following commit.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-2-mic@digikod.net
---
security/landlock/fs.c | 19 ++++++++++---------
security/landlock/fs.h | 2 +-
security/landlock/limits.h | 2 ++
security/landlock/ruleset.c | 6 ++++--
security/landlock/ruleset.h | 17 +++++++++++++----
5 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 97b8e421f617..9de2a460a762 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -150,7 +150,7 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
* @path: Should have been checked by get_path_from_fd().
*/
int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
- const struct path *const path, u32 access_rights)
+ const struct path *const path, access_mask_t access_rights)
{
int err;
struct landlock_object *object;
@@ -182,8 +182,8 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
static inline u64 unmask_layers(
const struct landlock_ruleset *const domain,
- const struct path *const path, const u32 access_request,
- u64 layer_mask)
+ const struct path *const path,
+ const access_mask_t access_request, u64 layer_mask)
{
const struct landlock_rule *rule;
const struct inode *inode;
@@ -223,7 +223,8 @@ static inline u64 unmask_layers(
}
static int check_access_path(const struct landlock_ruleset *const domain,
- const struct path *const path, u32 access_request)
+ const struct path *const path,
+ const access_mask_t access_request)
{
bool allowed = false;
struct path walker_path;
@@ -308,7 +309,7 @@ static int check_access_path(const struct landlock_ruleset *const domain,
}
static inline int current_check_access_path(const struct path *const path,
- const u32 access_request)
+ const access_mask_t access_request)
{
const struct landlock_ruleset *const dom =
landlock_get_current_domain();
@@ -511,7 +512,7 @@ static int hook_sb_pivotroot(const struct path *const old_path,
/* Path hooks */
-static inline u32 get_mode_access(const umode_t mode)
+static inline access_mask_t get_mode_access(const umode_t mode)
{
switch (mode & S_IFMT) {
case S_IFLNK:
@@ -563,7 +564,7 @@ static int hook_path_link(struct dentry *const old_dentry,
get_mode_access(d_backing_inode(old_dentry)->i_mode));
}
-static inline u32 maybe_remove(const struct dentry *const dentry)
+static inline access_mask_t maybe_remove(const struct dentry *const dentry)
{
if (d_is_negative(dentry))
return 0;
@@ -631,9 +632,9 @@ static int hook_path_rmdir(const struct path *const dir,
/* File hooks */
-static inline u32 get_file_access(const struct file *const file)
+static inline access_mask_t get_file_access(const struct file *const file)
{
- u32 access = 0;
+ access_mask_t access = 0;
if (file->f_mode & FMODE_READ) {
/* A directory can only be opened in read mode. */
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index 187284b421c9..74be312aad96 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -65,6 +65,6 @@ static inline struct landlock_superblock_security *landlock_superblock(
__init void landlock_add_fs_hooks(void);
int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
- const struct path *const path, u32 access_hierarchy);
+ const struct path *const path, access_mask_t access_hierarchy);
#endif /* _SECURITY_LANDLOCK_FS_H */
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 2a0a1095ee27..458d1de32ed5 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -9,6 +9,7 @@
#ifndef _SECURITY_LANDLOCK_LIMITS_H
#define _SECURITY_LANDLOCK_LIMITS_H
+#include <linux/bitops.h>
#include <linux/limits.h>
#include <uapi/linux/landlock.h>
@@ -17,5 +18,6 @@
#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_MAKE_SYM
#define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
+#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
#endif /* _SECURITY_LANDLOCK_LIMITS_H */
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index ec72b9262bf3..4e7aa8024fff 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -44,7 +44,8 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
return new_ruleset;
}
-struct landlock_ruleset *landlock_create_ruleset(const u32 fs_access_mask)
+struct landlock_ruleset *landlock_create_ruleset(
+ const access_mask_t fs_access_mask)
{
struct landlock_ruleset *new_ruleset;
@@ -228,7 +229,8 @@ static void build_check_layer(void)
/* @ruleset must be locked by the caller. */
int landlock_insert_rule(struct landlock_ruleset *const ruleset,
- struct landlock_object *const object, const u32 access)
+ struct landlock_object *const object,
+ const access_mask_t access)
{
struct landlock_layer layers[] = {{
.access = access,
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 2d3ed7ec5a0a..7e7cac68e443 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -9,13 +9,20 @@
#ifndef _SECURITY_LANDLOCK_RULESET_H
#define _SECURITY_LANDLOCK_RULESET_H
+#include <linux/bitops.h>
+#include <linux/build_bug.h>
#include <linux/mutex.h>
#include <linux/rbtree.h>
#include <linux/refcount.h>
#include <linux/workqueue.h>
+#include "limits.h"
#include "object.h"
+typedef u16 access_mask_t;
+/* Makes sure all filesystem access rights can be stored. */
+static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
+
/**
* struct landlock_layer - Access rights for a given layer
*/
@@ -28,7 +35,7 @@ struct landlock_layer {
* @access: Bitfield of allowed actions on the kernel object. They are
* relative to the object type (e.g. %LANDLOCK_ACTION_FS_READ).
*/
- u16 access;
+ access_mask_t access;
};
/**
@@ -135,18 +142,20 @@ struct landlock_ruleset {
* layers are set once and never changed for the
* lifetime of the ruleset.
*/
- u16 fs_access_masks[];
+ access_mask_t fs_access_masks[];
};
};
};
-struct landlock_ruleset *landlock_create_ruleset(const u32 fs_access_mask);
+struct landlock_ruleset *landlock_create_ruleset(
+ const access_mask_t fs_access_mask);
void landlock_put_ruleset(struct landlock_ruleset *const ruleset);
void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset);
int landlock_insert_rule(struct landlock_ruleset *const ruleset,
- struct landlock_object *const object, const u32 access);
+ struct landlock_object *const object,
+ const access_mask_t access);
struct landlock_ruleset *landlock_merge_ruleset(
struct landlock_ruleset *const parent,
--
2.35.1
^ permalink raw reply related
* [PATCH v1 02/11] landlock: Reduce the maximum number of layers to 16
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
The maximum number of nested Landlock domains is currently 64. Because
of the following fix and to help reduce the stack size, let's reduce it
to 16. This seems large enough for a lot of use cases (e.g. sandboxed
init service, spawning a sandboxed SSH service, in nested sandboxed
containers). Reducing the number of nested domains may also help to
discover misuse of Landlock (e.g. creating a domain per rule).
Add and use a dedicated layer_mask_t typedef to fit with the number of
layers. This might be useful when changing it and to keep it consistent
with the maximum number of layers.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-3-mic@digikod.net
---
security/landlock/fs.c | 13 +++++--------
security/landlock/limits.h | 2 +-
security/landlock/ruleset.h | 4 ++++
tools/testing/selftests/landlock/fs_test.c | 2 +-
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 9de2a460a762..4048e3c04d75 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -180,10 +180,10 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
/* Access-control management */
-static inline u64 unmask_layers(
+static inline layer_mask_t unmask_layers(
const struct landlock_ruleset *const domain,
const struct path *const path,
- const access_mask_t access_request, u64 layer_mask)
+ const access_mask_t access_request, layer_mask_t layer_mask)
{
const struct landlock_rule *rule;
const struct inode *inode;
@@ -209,11 +209,11 @@ static inline u64 unmask_layers(
*/
for (i = 0; i < rule->num_layers; i++) {
const struct landlock_layer *const layer = &rule->layers[i];
- const u64 layer_level = BIT_ULL(layer->level - 1);
+ const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
/* Checks that the layer grants access to the full request. */
if ((layer->access & access_request) == access_request) {
- layer_mask &= ~layer_level;
+ layer_mask &= ~layer_bit;
if (layer_mask == 0)
return layer_mask;
@@ -228,12 +228,9 @@ static int check_access_path(const struct landlock_ruleset *const domain,
{
bool allowed = false;
struct path walker_path;
- u64 layer_mask;
+ layer_mask_t layer_mask;
size_t i;
- /* Make sure all layers can be checked. */
- BUILD_BUG_ON(BITS_PER_TYPE(layer_mask) < LANDLOCK_MAX_NUM_LAYERS);
-
if (!access_request)
return 0;
if (WARN_ON_ONCE(!domain || !path))
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 458d1de32ed5..126d1ec04d34 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -13,7 +13,7 @@
#include <linux/limits.h>
#include <uapi/linux/landlock.h>
-#define LANDLOCK_MAX_NUM_LAYERS 64
+#define LANDLOCK_MAX_NUM_LAYERS 16
#define LANDLOCK_MAX_NUM_RULES U32_MAX
#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_MAKE_SYM
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 7e7cac68e443..0128c56ee7ff 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -23,6 +23,10 @@ typedef u16 access_mask_t;
/* Makes sure all filesystem access rights can be stored. */
static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
+typedef u16 layer_mask_t;
+/* Makes sure all layers can be checked. */
+static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
+
/**
* struct landlock_layer - Access rights for a given layer
*/
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 10c9a1e4ebd9..99838cac970b 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -1080,7 +1080,7 @@ TEST_F_FORK(layout1, max_layers)
const int ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
ASSERT_LE(0, ruleset_fd);
- for (i = 0; i < 64; i++)
+ for (i = 0; i < 16; i++)
enforce_ruleset(_metadata, ruleset_fd);
for (i = 0; i < 2; i++) {
--
2.35.1
^ permalink raw reply related
* [PATCH v1 00/11] Landlock: file linking and renaming support
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module
Hi,
One of the most annoying limitations of Landlock is that sandboxed
processes can only link and rename files to the same directory (i.e.
file reparenting is always denied). Indeed, because of the unprivileged
nature of Landlock, file hierarchy are identified thanks to ephemeral
inode tagging, which may cause arbitrary renaming and linking to change
the security policy in an unexpected way.
This patch series brings a new access right, LANDLOCK_ACCESS_FS_REFER,
which enables to allow safe file linking and renaming. In a nutshell,
Landlock checks that the inherited access rights of a moved or renamed
file cannot increase but only reduce. Six new test suits cover file
renaming and linking, which brings coverage for security/landlock/ from
93.5% of lines to 94.4%.
The documentation and the tutorial is extended with this new access
right, along with more explanations about backward and forward
compatibility, good practices, and a bit about the current access
rights rational.
While developing this new feature, I also found an issue with the
current implementation of Landlock. In some (rare) cases, sandboxed
processes may be more restricted than intended. Indeed, because of the
current way to check file hierarchy access rights, composition of rules
may be incomplete when requesting multiple accesses at the same time.
This is fixed with a dedicated patch involving some refactoring. A new
test suite checks relevant new edge cases.
As a side effect, and to limit the increased use of the stack, I reduced
the number of Landlock nested domains from 64 to 16. I think this
should be more than enough for legitimate use cases, but feel free to
challenge this decision with real and legitimate use cases.
Because of the current path_rename security hook, Landlock cannot yet
return consistent error codes with RENAME_EXCHANGE. I plan to address
this issue with a next series.
This patch series was developed with some complementary new tests sent
in a standalone patch series:
https://lore.kernel.org/r/20220221155311.166278-1-mic@digikod.net
Additionally, a new dedicated syzkaller test has been developed to cover
new paths.
Regards,
Mickaël Salaün (11):
landlock: Define access_mask_t to enforce a consistent access mask
size
landlock: Reduce the maximum number of layers to 16
landlock: Create find_rule() from unmask_layers()
landlock: Fix same-layer rule unions
landlock: Move filesystem helpers and add a new one
landlock: Add support for file reparenting with
LANDLOCK_ACCESS_FS_REFER
selftest/landlock: Add 6 new test suites dedicated to file reparenting
samples/landlock: Add support for file reparenting
landlock: Document LANDLOCK_ACCESS_FS_REFER and ABI versioning
landlock: Document good practices about filesystem policies
landlock: Add design choices documentation for filesystem access
rights
Documentation/security/landlock.rst | 17 +-
Documentation/userspace-api/landlock.rst | 145 +++-
include/uapi/linux/landlock.h | 27 +-
samples/landlock/sandboxer.c | 37 +-
security/landlock/fs.c | 721 +++++++++++++++----
security/landlock/fs.h | 2 +-
security/landlock/limits.h | 6 +-
security/landlock/ruleset.c | 6 +-
security/landlock/ruleset.h | 23 +-
security/landlock/syscalls.c | 2 +-
tools/testing/selftests/landlock/base_test.c | 2 +-
tools/testing/selftests/landlock/fs_test.c | 634 +++++++++++++++-
12 files changed, 1447 insertions(+), 175 deletions(-)
base-commit: cfb92440ee71adcc2105b0890bb01ac3cddb8507
--
2.35.1
^ permalink raw reply
* [PATCH v1 03/11] landlock: Create find_rule() from unmask_layers()
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
This refactoring will be useful in a following commit.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-4-mic@digikod.net
---
security/landlock/fs.c | 39 +++++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 4048e3c04d75..0bcb27f2360a 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -180,23 +180,36 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
/* Access-control management */
-static inline layer_mask_t unmask_layers(
+/*
+ * The lifetime of the returned rule is tied to @domain.
+ *
+ * Returns NULL if no rule is found or if @dentry is negative.
+ */
+static inline const struct landlock_rule *find_rule(
const struct landlock_ruleset *const domain,
- const struct path *const path,
- const access_mask_t access_request, layer_mask_t layer_mask)
+ const struct dentry *const dentry)
{
const struct landlock_rule *rule;
const struct inode *inode;
- size_t i;
- if (d_is_negative(path->dentry))
- /* Ignore nonexistent leafs. */
- return layer_mask;
- inode = d_backing_inode(path->dentry);
+ /* Ignores nonexistent leafs. */
+ if (d_is_negative(dentry))
+ return NULL;
+
+ inode = d_backing_inode(dentry);
rcu_read_lock();
rule = landlock_find_rule(domain,
rcu_dereference(landlock_inode(inode)->object));
rcu_read_unlock();
+ return rule;
+}
+
+static inline layer_mask_t unmask_layers(
+ const struct landlock_rule *const rule,
+ const access_mask_t access_request, layer_mask_t layer_mask)
+{
+ size_t layer_level;
+
if (!rule)
return layer_mask;
@@ -207,8 +220,9 @@ static inline layer_mask_t unmask_layers(
* the remaining layers for each inode, from the first added layer to
* the last one.
*/
- for (i = 0; i < rule->num_layers; i++) {
- const struct landlock_layer *const layer = &rule->layers[i];
+ for (layer_level = 0; layer_level < rule->num_layers; layer_level++) {
+ const struct landlock_layer *const layer =
+ &rule->layers[layer_level];
const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
/* Checks that the layer grants access to the full request. */
@@ -266,8 +280,9 @@ static int check_access_path(const struct landlock_ruleset *const domain,
while (true) {
struct dentry *parent_dentry;
- layer_mask = unmask_layers(domain, &walker_path,
- access_request, layer_mask);
+ layer_mask = unmask_layers(find_rule(domain,
+ walker_path.dentry), access_request,
+ layer_mask);
if (layer_mask == 0) {
/* Stops when a rule from each layer grants access. */
allowed = true;
--
2.35.1
^ permalink raw reply related
* [PATCH v1 04/11] landlock: Fix same-layer rule unions
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
The original behavior was to check if the full set of requested accesses
was allowed by at least a rule of every relevant layer. This didn't
take into account requests for multiple accesses and same-layer rules
allowing the union of these accesses in a complementary way. As a
result, multiple accesses requested on a file hierarchy matching rules
that, together, allowed these accesses, but without a unique rule
allowing all of them, was illegitimately denied. This case should be
rare in practice and it can only be triggered by the path_rename or
file_open hook implementations.
For instance, if, for the same layer, a rule allows execution
beneath /a/b and another rule allows read beneath /a, requesting access
to read and execute at the same time for /a/b should be allowed for this
layer.
This was an inconsistency because the union of same-layer rule accesses
was already allowed if requested once at a time anyway.
This fix changes the way allowed accesses are gathered over a path walk.
To take into account all these rule accesses, we store in a matrix all
layer granting the set of requested accesses, according to the handled
accesses. To avoid heap allocation, we use an array on the stack which
is 2*13 bytes. A following commit bringing the LANDLOCK_ACCESS_FS_REFER
access right will increase this size to reach 84 bytes (2*14*3) in case
of link or rename actions.
Add a new layout1.layer_rule_unions test to check that accesses from
different rules pertaining to the same layer are ORed in a file
hierarchy. Also test that it is not the case for rules from different
layers.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-5-mic@digikod.net
---
security/landlock/fs.c | 77 ++++++++++-----
security/landlock/ruleset.h | 2 +
tools/testing/selftests/landlock/fs_test.c | 107 +++++++++++++++++++++
3 files changed, 160 insertions(+), 26 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 0bcb27f2360a..9662f9fb3cd0 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -204,45 +204,66 @@ static inline const struct landlock_rule *find_rule(
return rule;
}
-static inline layer_mask_t unmask_layers(
- const struct landlock_rule *const rule,
- const access_mask_t access_request, layer_mask_t layer_mask)
+/*
+ * @layer_masks is read and may be updated according to the access request and
+ * the matching rule.
+ *
+ * Returns true if the request is allowed (i.e. relevant layer masks for the
+ * request are empty).
+ */
+static inline bool unmask_layers(const struct landlock_rule *const rule,
+ const access_mask_t access_request,
+ layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
{
size_t layer_level;
+ if (!access_request || !layer_masks)
+ return true;
if (!rule)
- return layer_mask;
+ return false;
/*
* An access is granted if, for each policy layer, at least one rule
- * encountered on the pathwalk grants the requested accesses,
- * regardless of their position in the layer stack. We must then check
+ * encountered on the pathwalk grants the requested access,
+ * regardless of its position in the layer stack. We must then check
* the remaining layers for each inode, from the first added layer to
- * the last one.
+ * the last one. When there is multiple requested accesses, for each
+ * policy layer, the full set of requested accesses may not be granted
+ * by only one rule, but by the union (binary OR) of multiple rules.
+ * E.g. /a/b <execute> + /a <read> = /a/b <execute + read>
*/
for (layer_level = 0; layer_level < rule->num_layers; layer_level++) {
const struct landlock_layer *const layer =
&rule->layers[layer_level];
const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
+ const unsigned long access_req = access_request;
+ unsigned long access_bit;
+ bool is_empty;
- /* Checks that the layer grants access to the full request. */
- if ((layer->access & access_request) == access_request) {
- layer_mask &= ~layer_bit;
-
- if (layer_mask == 0)
- return layer_mask;
+ /*
+ * Records in @layer_masks which layer grants access to each
+ * requested access.
+ */
+ is_empty = true;
+ for_each_set_bit(access_bit, &access_req,
+ ARRAY_SIZE(*layer_masks)) {
+ if (layer->access & BIT_ULL(access_bit))
+ (*layer_masks)[access_bit] &= ~layer_bit;
+ is_empty = is_empty && !(*layer_masks)[access_bit];
}
+ if (is_empty)
+ return true;
}
- return layer_mask;
+ return false;
}
static int check_access_path(const struct landlock_ruleset *const domain,
const struct path *const path,
const access_mask_t access_request)
{
- bool allowed = false;
+ layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+ bool allowed = false, has_access = false;
struct path walker_path;
- layer_mask_t layer_mask;
size_t i;
if (!access_request)
@@ -262,13 +283,20 @@ static int check_access_path(const struct landlock_ruleset *const domain,
return -EACCES;
/* Saves all layers handling a subset of requested accesses. */
- layer_mask = 0;
for (i = 0; i < domain->num_layers; i++) {
- if (domain->fs_access_masks[i] & access_request)
- layer_mask |= BIT_ULL(i);
+ const unsigned long access_req = access_request;
+ unsigned long access_bit;
+
+ for_each_set_bit(access_bit, &access_req,
+ ARRAY_SIZE(layer_masks)) {
+ if (domain->fs_access_masks[i] & BIT_ULL(access_bit)) {
+ layer_masks[access_bit] |= BIT_ULL(i);
+ has_access = true;
+ }
+ }
}
/* An access request not handled by the domain is allowed. */
- if (layer_mask == 0)
+ if (!has_access)
return 0;
walker_path = *path;
@@ -280,14 +308,11 @@ static int check_access_path(const struct landlock_ruleset *const domain,
while (true) {
struct dentry *parent_dentry;
- layer_mask = unmask_layers(find_rule(domain,
- walker_path.dentry), access_request,
- layer_mask);
- if (layer_mask == 0) {
+ allowed = unmask_layers(find_rule(domain, walker_path.dentry),
+ access_request, &layer_masks);
+ if (allowed)
/* Stops when a rule from each layer grants access. */
- allowed = true;
break;
- }
jump_up:
if (walker_path.dentry == walker_path.mnt->mnt_root) {
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 0128c56ee7ff..fa17cc1f82db 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -22,6 +22,8 @@
typedef u16 access_mask_t;
/* Makes sure all filesystem access rights can be stored. */
static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
+/* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
+static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
typedef u16 layer_mask_t;
/* Makes sure all layers can be checked. */
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 99838cac970b..1ac41bfa7382 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -687,6 +687,113 @@ TEST_F_FORK(layout1, ruleset_overlap)
ASSERT_EQ(0, test_open(dir_s1d3, O_RDONLY | O_DIRECTORY));
}
+TEST_F_FORK(layout1, layer_rule_unions)
+{
+ const struct rule layer1[] = {
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE,
+ },
+ /* dir_s1d3 should allow READ_FILE and WRITE_FILE (O_RDWR). */
+ {
+ .path = dir_s1d3,
+ .access = LANDLOCK_ACCESS_FS_WRITE_FILE,
+ },
+ {}
+ };
+ const struct rule layer2[] = {
+ /* Doesn't change anything from layer1. */
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE,
+ },
+ {}
+ };
+ const struct rule layer3[] = {
+ /* Only allows write (but not read) to dir_s1d3. */
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_WRITE_FILE,
+ },
+ {}
+ };
+ int ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Checks s1d1 hierarchy with layer1. */
+ ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDWR));
+ ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+
+ /* Checks s1d2 hierarchy with layer1. */
+ ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDWR));
+ ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+
+ /* Checks s1d3 hierarchy with layer1. */
+ ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+ ASSERT_EQ(0, test_open(file1_s1d3, O_WRONLY));
+ /* dir_s1d3 should allow READ_FILE and WRITE_FILE (O_RDWR). */
+ ASSERT_EQ(0, test_open(file1_s1d3, O_RDWR));
+ ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+
+ /* Doesn't change anything from layer1. */
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer2);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Checks s1d1 hierarchy with layer2. */
+ ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDWR));
+ ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+
+ /* Checks s1d2 hierarchy with layer2. */
+ ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDWR));
+ ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+
+ /* Checks s1d3 hierarchy with layer2. */
+ ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+ ASSERT_EQ(0, test_open(file1_s1d3, O_WRONLY));
+ /* dir_s1d3 should allow READ_FILE and WRITE_FILE (O_RDWR). */
+ ASSERT_EQ(0, test_open(file1_s1d3, O_RDWR));
+ ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+
+ /* Only allows write (but not read) to dir_s1d3. */
+ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer3);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Checks s1d1 hierarchy with layer3. */
+ ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d1, O_RDWR));
+ ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+
+ /* Checks s1d2 hierarchy with layer3. */
+ ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+ ASSERT_EQ(EACCES, test_open(file1_s1d2, O_RDWR));
+ ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+
+ /* Checks s1d3 hierarchy with layer3. */
+ ASSERT_EQ(EACCES, test_open(file1_s1d3, O_RDONLY));
+ ASSERT_EQ(0, test_open(file1_s1d3, O_WRONLY));
+ /* dir_s1d3 should now deny READ_FILE and WRITE_FILE (O_RDWR). */
+ ASSERT_EQ(EACCES, test_open(file1_s1d3, O_RDWR));
+ ASSERT_EQ(EACCES, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+}
+
TEST_F_FORK(layout1, non_overlapping_accesses)
{
const struct rule layer1[] = {
--
2.35.1
^ permalink raw reply related
* [PATCH v1 05/11] landlock: Move filesystem helpers and add a new one
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Move the SB_NOUSER and IS_PRIVATE dentry check to a standalone
is_nouser_or_private() helper. This will be useful for a following
commit.
Move get_mode_access() and maybe_remove() to make them usable by new
code provided by a following commit.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-6-mic@digikod.net
---
security/landlock/fs.c | 87 ++++++++++++++++++++++--------------------
1 file changed, 46 insertions(+), 41 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 9662f9fb3cd0..3886f9ad1a60 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -257,6 +257,18 @@ static inline bool unmask_layers(const struct landlock_rule *const rule,
return false;
}
+static inline bool is_nouser_or_private(const struct dentry *dentry)
+{
+ /*
+ * Allows access to pseudo filesystems that will never be mountable
+ * (e.g. sockfs, pipefs), but can still be reachable through
+ * /proc/<pid>/fd/<file-descriptor> .
+ */
+ return (dentry->d_sb->s_flags & SB_NOUSER) ||
+ (d_is_positive(dentry) &&
+ unlikely(IS_PRIVATE(d_backing_inode(dentry))));
+}
+
static int check_access_path(const struct landlock_ruleset *const domain,
const struct path *const path,
const access_mask_t access_request)
@@ -270,14 +282,7 @@ static int check_access_path(const struct landlock_ruleset *const domain,
return 0;
if (WARN_ON_ONCE(!domain || !path))
return 0;
- /*
- * Allows access to pseudo filesystems that will never be mountable
- * (e.g. sockfs, pipefs), but can still be reachable through
- * /proc/<pid>/fd/<file-descriptor> .
- */
- if ((path->dentry->d_sb->s_flags & SB_NOUSER) ||
- (d_is_positive(path->dentry) &&
- unlikely(IS_PRIVATE(d_backing_inode(path->dentry)))))
+ if (is_nouser_or_private(path->dentry))
return 0;
if (WARN_ON_ONCE(domain->num_layers < 1))
return -EACCES;
@@ -356,6 +361,39 @@ static inline int current_check_access_path(const struct path *const path,
return check_access_path(dom, path, access_request);
}
+static inline access_mask_t get_mode_access(const umode_t mode)
+{
+ switch (mode & S_IFMT) {
+ case S_IFLNK:
+ return LANDLOCK_ACCESS_FS_MAKE_SYM;
+ case 0:
+ /* A zero mode translates to S_IFREG. */
+ case S_IFREG:
+ return LANDLOCK_ACCESS_FS_MAKE_REG;
+ case S_IFDIR:
+ return LANDLOCK_ACCESS_FS_MAKE_DIR;
+ case S_IFCHR:
+ return LANDLOCK_ACCESS_FS_MAKE_CHAR;
+ case S_IFBLK:
+ return LANDLOCK_ACCESS_FS_MAKE_BLOCK;
+ case S_IFIFO:
+ return LANDLOCK_ACCESS_FS_MAKE_FIFO;
+ case S_IFSOCK:
+ return LANDLOCK_ACCESS_FS_MAKE_SOCK;
+ default:
+ WARN_ON_ONCE(1);
+ return 0;
+ }
+}
+
+static inline access_mask_t maybe_remove(const struct dentry *const dentry)
+{
+ if (d_is_negative(dentry))
+ return 0;
+ return d_is_dir(dentry) ? LANDLOCK_ACCESS_FS_REMOVE_DIR :
+ LANDLOCK_ACCESS_FS_REMOVE_FILE;
+}
+
/* Inode hooks */
static void hook_inode_free_security(struct inode *const inode)
@@ -549,31 +587,6 @@ static int hook_sb_pivotroot(const struct path *const old_path,
/* Path hooks */
-static inline access_mask_t get_mode_access(const umode_t mode)
-{
- switch (mode & S_IFMT) {
- case S_IFLNK:
- return LANDLOCK_ACCESS_FS_MAKE_SYM;
- case 0:
- /* A zero mode translates to S_IFREG. */
- case S_IFREG:
- return LANDLOCK_ACCESS_FS_MAKE_REG;
- case S_IFDIR:
- return LANDLOCK_ACCESS_FS_MAKE_DIR;
- case S_IFCHR:
- return LANDLOCK_ACCESS_FS_MAKE_CHAR;
- case S_IFBLK:
- return LANDLOCK_ACCESS_FS_MAKE_BLOCK;
- case S_IFIFO:
- return LANDLOCK_ACCESS_FS_MAKE_FIFO;
- case S_IFSOCK:
- return LANDLOCK_ACCESS_FS_MAKE_SOCK;
- default:
- WARN_ON_ONCE(1);
- return 0;
- }
-}
-
/*
* Creating multiple links or renaming may lead to privilege escalations if not
* handled properly. Indeed, we must be sure that the source doesn't gain more
@@ -601,14 +614,6 @@ static int hook_path_link(struct dentry *const old_dentry,
get_mode_access(d_backing_inode(old_dentry)->i_mode));
}
-static inline access_mask_t maybe_remove(const struct dentry *const dentry)
-{
- if (d_is_negative(dentry))
- return 0;
- return d_is_dir(dentry) ? LANDLOCK_ACCESS_FS_REMOVE_DIR :
- LANDLOCK_ACCESS_FS_REMOVE_FILE;
-}
-
static int hook_path_rename(const struct path *const old_dir,
struct dentry *const old_dentry,
const struct path *const new_dir,
--
2.35.1
^ permalink raw reply related
* [PATCH v1 08/11] samples/landlock: Add support for file reparenting
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Add LANDLOCK_ACCESS_FS_REFER to the "roughly write" access rights and
leverage the Landlock ABI version to only try to enforce it if it is
supported by the running kernel.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-9-mic@digikod.net
---
samples/landlock/sandboxer.c | 37 +++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 7a15910d2171..8509543fcbbb 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -153,16 +153,21 @@ static int populate_ruleset(
LANDLOCK_ACCESS_FS_MAKE_SOCK | \
LANDLOCK_ACCESS_FS_MAKE_FIFO | \
LANDLOCK_ACCESS_FS_MAKE_BLOCK | \
- LANDLOCK_ACCESS_FS_MAKE_SYM)
+ LANDLOCK_ACCESS_FS_MAKE_SYM | \
+ LANDLOCK_ACCESS_FS_REFER)
+
+#define ACCESS_ABI_2 ( \
+ LANDLOCK_ACCESS_FS_REFER)
int main(const int argc, char *const argv[], char *const *const envp)
{
const char *cmd_path;
char *const *cmd_argv;
- int ruleset_fd;
+ int ruleset_fd, abi;
+ __u64 access_fs_ro = ACCESS_FS_ROUGHLY_READ,
+ access_fs_rw = ACCESS_FS_ROUGHLY_READ | ACCESS_FS_ROUGHLY_WRITE;
struct landlock_ruleset_attr ruleset_attr = {
- .handled_access_fs = ACCESS_FS_ROUGHLY_READ |
- ACCESS_FS_ROUGHLY_WRITE,
+ .handled_access_fs = access_fs_rw,
};
if (argc < 2) {
@@ -183,11 +188,11 @@ int main(const int argc, char *const argv[], char *const *const envp)
return 1;
}
- ruleset_fd = landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
- if (ruleset_fd < 0) {
+ abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
+ if (abi < 0) {
const int err = errno;
- perror("Failed to create a ruleset");
+ perror("Failed to check Landlock compatibility");
switch (err) {
case ENOSYS:
fprintf(stderr, "Hint: Landlock is not supported by the current kernel. "
@@ -205,12 +210,22 @@ int main(const int argc, char *const argv[], char *const *const envp)
}
return 1;
}
- if (populate_ruleset(ENV_FS_RO_NAME, ruleset_fd,
- ACCESS_FS_ROUGHLY_READ)) {
+ /* Best-effort security. */
+ if (abi < 2) {
+ ruleset_attr.handled_access_fs &= ~ACCESS_ABI_2;
+ access_fs_ro &= ~ACCESS_ABI_2;
+ access_fs_rw &= ~ACCESS_ABI_2;
+ }
+
+ ruleset_fd = landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ if (ruleset_fd < 0) {
+ perror("Failed to create a ruleset");
+ return 1;
+ }
+ if (populate_ruleset(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro)) {
goto err_close_ruleset;
}
- if (populate_ruleset(ENV_FS_RW_NAME, ruleset_fd,
- ACCESS_FS_ROUGHLY_READ | ACCESS_FS_ROUGHLY_WRITE)) {
+ if (populate_ruleset(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw)) {
goto err_close_ruleset;
}
if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
--
2.35.1
^ permalink raw reply related
* [PATCH v1 07/11] selftest/landlock: Add 6 new test suites dedicated to file reparenting
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
These test suites try to check all edge cases for directory and file
renaming or linking involving a new parent directory, with and without
LANDLOCK_ACCESS_FS_REFER and other access rights.
layout1:
* reparent_refer: Tests simple FS_REFER usage.
* reparent_link: Tests a mix of FS_MAKE_REG and FS_REFER with links.
* reparent_rename: Tests a mix of FS_MAKE_REG and FS_REFER with renames.
* reparent_exdev_layers: Tests with two layers.
* reparent_dom_superset: Tests access partial ordering.
layout1_bind:
* reparent_cross_mount: Tests FS_REFER propagation across mount points.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-8-mic@digikod.net
---
tools/testing/selftests/landlock/fs_test.c | 522 +++++++++++++++++++++
1 file changed, 522 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 0568d1193492..c42fcd9e62ec 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -1851,6 +1851,491 @@ TEST_F_FORK(layout1, rename_dir)
ASSERT_EQ(0, rmdir(dir_s1d3));
}
+TEST_F_FORK(layout1, reparent_refer)
+{
+ const struct rule layer1[] = {
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {
+ .path = dir_s2d2,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {}
+ };
+ int ruleset_fd = create_ruleset(_metadata, LANDLOCK_ACCESS_FS_REFER,
+ layer1);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d1));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d2));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d3));
+ ASSERT_EQ(EXDEV, errno);
+
+ ASSERT_EQ(-1, rename(dir_s1d3, dir_s2d1));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, rename(dir_s1d3, dir_s2d2));
+ ASSERT_EQ(EXDEV, errno);
+ /*
+ * Moving should only be allowed when the source and the destination
+ * parent directory have REFER.
+ */
+ ASSERT_EQ(-1, rename(dir_s1d3, dir_s2d3));
+ ASSERT_EQ(ENOTEMPTY, errno);
+ ASSERT_EQ(0, unlink(file1_s2d3));
+ ASSERT_EQ(0, unlink(file2_s2d3));
+ ASSERT_EQ(0, rename(dir_s1d3, dir_s2d3));
+}
+
+TEST_F_FORK(layout1, reparent_link)
+{
+ const struct rule layer1[] = {
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG,
+ },
+ {
+ .path = dir_s1d3,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {
+ .path = dir_s2d2,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {
+ .path = dir_s2d3,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG,
+ },
+ {}
+ };
+ const int ruleset_fd = create_ruleset(_metadata,
+ LANDLOCK_ACCESS_FS_MAKE_REG | LANDLOCK_ACCESS_FS_REFER,
+ layer1);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ ASSERT_EQ(0, unlink(file1_s1d1));
+ ASSERT_EQ(0, unlink(file1_s1d2));
+ ASSERT_EQ(0, unlink(file1_s1d3));
+
+ /* Denies linking because of missing MAKE_REG. */
+ ASSERT_EQ(-1, link(file2_s1d1, file1_s1d1));
+ ASSERT_EQ(EACCES, errno);
+ /* Denies linking because of missing source and destination REFER. */
+ ASSERT_EQ(-1, link(file1_s2d1, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);
+ /* Denies linking because of missing source REFER. */
+ ASSERT_EQ(-1, link(file1_s2d1, file1_s1d3));
+ ASSERT_EQ(EXDEV, errno);
+
+ /* Denies linking because of missing MAKE_REG. */
+ ASSERT_EQ(-1, link(file1_s2d2, file1_s1d1));
+ ASSERT_EQ(EACCES, errno);
+ /* Denies linking because of missing destination REFER. */
+ ASSERT_EQ(-1, link(file1_s2d2, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);
+
+ /* Allows linking because of REFER and MAKE_REG. */
+ ASSERT_EQ(0, link(file1_s2d2, file1_s1d3));
+ ASSERT_EQ(0, unlink(file1_s2d2));
+ /* Reverse linking denied because of missing MAKE_REG. */
+ ASSERT_EQ(-1, link(file1_s1d3, file1_s2d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(0, unlink(file1_s2d3));
+ /* Checks reverse linking. */
+ ASSERT_EQ(0, link(file1_s1d3, file1_s2d3));
+ ASSERT_EQ(0, unlink(file1_s1d3));
+
+ /*
+ * This is OK for a file link, but it should not be allowed for a
+ * directory rename (because of the superset of access rights.
+ */
+ ASSERT_EQ(0, link(file1_s2d3, file1_s1d3));
+ ASSERT_EQ(0, unlink(file1_s1d3));
+
+ ASSERT_EQ(-1, link(file2_s1d2, file1_s1d3));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, link(file2_s1d3, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);
+
+ ASSERT_EQ(0, link(file2_s1d2, file1_s1d2));
+ ASSERT_EQ(0, link(file2_s1d3, file1_s1d3));
+}
+
+TEST_F_FORK(layout1, reparent_rename)
+{
+ /* Same rules as for reparent_link. */
+ const struct rule layer1[] = {
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG,
+ },
+ {
+ .path = dir_s1d3,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {
+ .path = dir_s2d2,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {
+ .path = dir_s2d3,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG,
+ },
+ {}
+ };
+ const int ruleset_fd = create_ruleset(_metadata,
+ LANDLOCK_ACCESS_FS_MAKE_REG | LANDLOCK_ACCESS_FS_REFER,
+ layer1);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ ASSERT_EQ(0, unlink(file1_s1d2));
+ ASSERT_EQ(0, unlink(file1_s1d3));
+
+ /* Denies renaming because of missing MAKE_REG. */
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, file2_s1d1, AT_FDCWD, file1_s1d1,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s1d1, AT_FDCWD, file2_s1d1,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(0, unlink(file1_s1d1));
+ ASSERT_EQ(-1, rename(file2_s1d1, file1_s1d1));
+ ASSERT_EQ(EACCES, errno);
+ /* Even denies same file exchange. */
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, file2_s1d1, AT_FDCWD, file2_s1d1,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Denies renaming because of missing source and destination REFER. */
+ ASSERT_EQ(-1, rename(file1_s2d1, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);
+ /*
+ * Denies renaming because of missing MAKE_REG, source and destination
+ * REFER.
+ */
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s2d1, AT_FDCWD, file2_s1d1,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, file2_s1d1, AT_FDCWD, file1_s2d1,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Denies renaming because of missing source REFER. */
+ ASSERT_EQ(-1, rename(file1_s2d1, file1_s1d3));
+ ASSERT_EQ(EXDEV, errno);
+ /* Denies renaming because of missing MAKE_REG. */
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s2d1, AT_FDCWD, file2_s1d3,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EACCES, errno);
+
+ /* Denies renaming because of missing MAKE_REG. */
+ ASSERT_EQ(-1, rename(file1_s2d2, file1_s1d1));
+ ASSERT_EQ(EACCES, errno);
+ /* Denies renaming because of missing destination REFER*/
+ ASSERT_EQ(-1, rename(file1_s2d2, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);
+
+ /* Denies exchange because of one missing MAKE_REG. */
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s2d2, AT_FDCWD, file2_s1d3,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EACCES, errno);
+ /* Allows renaming because of REFER and MAKE_REG. */
+ ASSERT_EQ(0, rename(file1_s2d2, file1_s1d3));
+
+ /* Reverse renaming denied because of missing MAKE_REG. */
+ ASSERT_EQ(-1, rename(file1_s1d3, file1_s2d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(0, unlink(file1_s2d3));
+ ASSERT_EQ(0, rename(file1_s1d3, file1_s2d3));
+
+ /* Tests reverse renaming. */
+ ASSERT_EQ(0, rename(file1_s2d3, file1_s1d3));
+ ASSERT_EQ(0, renameat2(AT_FDCWD, file2_s2d3, AT_FDCWD, file1_s1d3,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(0, rename(file1_s1d3, file1_s2d3));
+
+ /*
+ * This is OK for a file rename, but it should not be allowed for a
+ * directory rename (because of the superset of access rights).
+ */
+ ASSERT_EQ(0, rename(file1_s2d3, file1_s1d3));
+ ASSERT_EQ(0, rename(file1_s1d3, file1_s2d3));
+
+ /*
+ * Tests superset restrictions applied to directories. Not only the
+ * dir_s2d3's parent (dir_s2d2) should be taken into account but also
+ * access rights tied to dir_s2d3. dir_s2d2 is missing one access right
+ * compared to dir_s1d3/file1_s1d3 (MAKE_REG) but it is provided
+ * directly by the moved dir_s2d3.
+ */
+ ASSERT_EQ(0, rename(dir_s2d3, file1_s1d3));
+ ASSERT_EQ(0, rename(file1_s1d3, dir_s2d3));
+ /*
+ * The first rename is allowed but not the exchange because dir_s1d3's
+ * parent (dir_s1d2) doesn't have REFER.
+ */
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s2d3, AT_FDCWD, dir_s1d3,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, renameat2(AT_FDCWD, dir_s1d3, AT_FDCWD, file1_s2d3,
+ RENAME_EXCHANGE));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, rename(file1_s2d3, dir_s1d3));
+ ASSERT_EQ(EXDEV, errno);
+
+ ASSERT_EQ(-1, rename(file2_s1d2, file1_s1d3));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, rename(file2_s1d3, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);
+
+ /* Renaming in the same directory is always allowed. */
+ ASSERT_EQ(0, rename(file2_s1d2, file1_s1d2));
+ ASSERT_EQ(0, rename(file2_s1d3, file1_s1d3));
+
+ ASSERT_EQ(0, unlink(file1_s1d2));
+ /* Denies because of missing source MAKE_REG and destination REFER. */
+ ASSERT_EQ(-1, rename(dir_s2d3, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);
+
+ ASSERT_EQ(0, unlink(file1_s1d3));
+ /* Denies because of missing source MAKE_REG and REFER. */
+ ASSERT_EQ(-1, rename(dir_s2d2, file1_s1d3));
+ ASSERT_EQ(EXDEV, errno);
+}
+
+TEST_F_FORK(layout1, reparent_exdev_layers)
+{
+ const struct rule layer1[] = {
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {
+ /* Interesting for the layer2 tests. */
+ .path = dir_s1d3,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG,
+ },
+ {
+ .path = dir_s2d2,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {
+ .path = dir_s2d3,
+ .access = LANDLOCK_ACCESS_FS_MAKE_REG,
+ },
+ {}
+ };
+ const struct rule layer2[] = {
+ {
+ .path = dir_s2d3,
+ .access = LANDLOCK_ACCESS_FS_MAKE_DIR,
+ },
+ {}
+ };
+ int ruleset_fd = create_ruleset(_metadata,
+ LANDLOCK_ACCESS_FS_MAKE_REG | LANDLOCK_ACCESS_FS_REFER,
+ layer1);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Checks EACCES predominance over EXDEV. */
+ ASSERT_EQ(-1, rename(file1_s1d1, file1_s2d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(file1_s1d2, file1_s2d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(file1_s1d1, file1_s2d3));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(0, rename(file1_s1d2, file1_s2d3));
+
+ /* Without REFER source. */
+ ASSERT_EQ(-1, rename(dir_s1d1, file1_s2d2));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(-1, rename(dir_s1d2, file1_s2d2));
+ ASSERT_EQ(EXDEV, errno);
+
+ /*
+ * Moving the dir_s1d3 directory below dir_s2d2 is allowed by Landlock
+ * because it doesn't inherit new access rights.
+ */
+ ASSERT_EQ(-1, rename(dir_s1d3, file1_s2d2));
+ ASSERT_EQ(ENOTDIR, errno);
+ ASSERT_EQ(0, unlink(file1_s2d2));
+ ASSERT_EQ(0, rename(dir_s1d3, file1_s2d2));
+ ASSERT_EQ(0, rename(file1_s2d2, dir_s1d3));
+
+ /*
+ * Moving the dir_s1d3 directory below dir_s2d3 is allowed, even if it
+ * gets a new inherited access rights (MAKE_REG), because MAKE_REG is
+ * already allowed for dir_s1d3.
+ */
+ ASSERT_EQ(-1, rename(dir_s1d3, file1_s2d3));
+ ASSERT_EQ(ENOTDIR, errno);
+ ASSERT_EQ(0, unlink(file1_s2d3));
+ ASSERT_EQ(0, rename(dir_s1d3, file1_s2d3));
+ ASSERT_EQ(0, rename(file1_s2d3, dir_s1d3));
+
+ /*
+ * However, moving the file1_s1d3 file below dir_s2d3 is allowed
+ * because it cannot inherit MAKE_REG right (which is dedicated to
+ * directories).
+ */
+ ASSERT_EQ(0, rename(file1_s1d3, file1_s2d3));
+
+ /*
+ * Same checks as before but with a second layer and a new MAKE_DIR
+ * rule (and no explicit handling of REFER).
+ */
+ ruleset_fd = create_ruleset(_metadata, LANDLOCK_ACCESS_FS_MAKE_DIR,
+ layer2);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Checks EACCES predominance over EXDEV. */
+ ASSERT_EQ(-1, rename(file1_s1d1, file1_s2d2));
+ ASSERT_EQ(EACCES, errno);
+ /* Checks with actual file2_s1d2. */
+ ASSERT_EQ(-1, rename(file2_s1d2, file1_s2d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(file1_s1d1, file1_s2d3));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(0, rename(file2_s1d2, file1_s2d3));
+
+ /* Without REFER source, EACCES wins over EXDEV. */
+ ASSERT_EQ(-1, rename(dir_s1d1, file1_s2d2));
+ ASSERT_EQ(EACCES, errno);
+ ASSERT_EQ(-1, rename(dir_s1d2, file1_s2d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Moving the dir_s1d3 directory below dir_s2d2 is now denied because
+ * MAKE_DIR is not tied to dir_s2d2.
+ */
+ ASSERT_EQ(-1, rename(dir_s1d3, file1_s2d2));
+ ASSERT_EQ(EACCES, errno);
+
+ /*
+ * Moving the dir_s1d3 directory below dir_s2d3 is forbidden because it
+ * would grants MAKE_REG and MAKE_DIR rights to it.
+ */
+ ASSERT_EQ(-1, rename(dir_s1d3, file1_s2d3));
+ ASSERT_EQ(EXDEV, errno);
+
+ /*
+ * However, moving the file2_s1d3 file below dir_s2d3 is allowed
+ * because it cannot inherit MAKE_REG nor MAKE_DIR rights (which are
+ * dedicated to directories).
+ */
+ ASSERT_EQ(0, rename(file2_s1d3, file1_s2d3));
+}
+
+TEST_F_FORK(layout1, reparent_dom_superset)
+{
+ const struct rule layer1[] = {
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {
+ .path = file1_s1d2,
+ .access = LANDLOCK_ACCESS_FS_EXECUTE,
+ },
+ {
+ .path = dir_s1d3,
+ .access = LANDLOCK_ACCESS_FS_MAKE_SOCK |
+ LANDLOCK_ACCESS_FS_EXECUTE,
+ },
+ {
+ .path = dir_s2d2,
+ .access = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_SOCK,
+ },
+ {
+ .path = dir_s2d3,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_FIFO,
+ },
+ {}
+ };
+ int ruleset_fd = create_ruleset(_metadata,
+ LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_SOCK |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_FIFO,
+ layer1);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ ASSERT_EQ(-1, rename(file1_s1d2, file1_s2d1));
+ ASSERT_EQ(EXDEV, errno);
+ /*
+ * Moving file1_s1d2 beneath dir_s2d3 would grant it the READ_FILE
+ * access right.
+ */
+ ASSERT_EQ(-1, rename(file1_s1d2, file1_s2d3));
+ ASSERT_EQ(EXDEV, errno);
+ /*
+ * Moving file1_s1d2 should be allowed even if dir_s2d2 grants a
+ * superset of access rights compared to dir_s1d2, because file1_s1d2
+ * already has these access rights anyway.
+ */
+ ASSERT_EQ(0, rename(file1_s1d2, file1_s2d2));
+ ASSERT_EQ(0, rename(file1_s2d2, file1_s1d2));
+
+ ASSERT_EQ(-1, rename(dir_s1d3, file1_s2d1));
+ ASSERT_EQ(EXDEV, errno);
+ /*
+ * Moving dir_s1d3 beneath dir_s2d3 would grant it the MAKE_FIFO access
+ * right.
+ */
+ ASSERT_EQ(-1, rename(dir_s1d3, file1_s2d3));
+ ASSERT_EQ(EXDEV, errno);
+ /*
+ * Moving dir_s1d3 should be allowed even if dir_s2d2 grants a superset
+ * of access rights compared to dir_s1d2, because dir_s1d3 already has
+ * these access rights anyway.
+ */
+ ASSERT_EQ(0, rename(dir_s1d3, file1_s2d2));
+ ASSERT_EQ(0, rename(file1_s2d2, dir_s1d3));
+
+ /*
+ * Moving file1_s2d3 beneath dir_s1d2 is allowed, but moving it back
+ * will be denied because the new inherited access rights from dir_s1d2
+ * will be less than the destination (original) dir_s2d3. This is a
+ * sinkhole scenario where we cannot move back files or directories.
+ */
+ ASSERT_EQ(0, rename(file1_s2d3, file2_s1d2));
+ ASSERT_EQ(-1, rename(file2_s1d2, file1_s2d3));
+ ASSERT_EQ(EXDEV, errno);
+ ASSERT_EQ(0, unlink(file2_s1d2));
+ ASSERT_EQ(0, unlink(file2_s2d3));
+ /*
+ * Checks similar directory one-way move: dir_s2d3 loses EXECUTE and
+ * MAKE_SOCK which were inherited from dir_s1d3.
+ */
+ ASSERT_EQ(0, rename(dir_s2d3, file2_s1d2));
+ ASSERT_EQ(-1, rename(file2_s1d2, dir_s2d3));
+ ASSERT_EQ(EXDEV, errno);
+}
+
TEST_F_FORK(layout1, remove_dir)
{
const struct rule rules[] = {
@@ -2390,6 +2875,43 @@ TEST_F_FORK(layout1_bind, same_content_same_file)
ASSERT_EQ(EACCES, test_open(bind_file1_s1d3, O_WRONLY));
}
+TEST_F_FORK(layout1_bind, reparent_cross_mount)
+{
+ const struct rule layer1[] = {
+ {
+ /* dir_s2d1 is beneath the dir_s2d2 mount point. */
+ .path = dir_s2d1,
+ .access = LANDLOCK_ACCESS_FS_REFER,
+ },
+ {
+ .path = bind_dir_s1d3,
+ .access = LANDLOCK_ACCESS_FS_EXECUTE,
+ },
+ {}
+ };
+ int ruleset_fd = create_ruleset(_metadata, LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_EXECUTE, layer1);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Checks basic denied move. */
+ ASSERT_EQ(-1, rename(file1_s1d1, file1_s1d2));
+ ASSERT_EQ(EXDEV, errno);
+
+ /* Checks real cross-mount move (Landlock is not involved). */
+ ASSERT_EQ(-1, rename(file1_s2d1, file1_s2d2));
+ ASSERT_EQ(EXDEV, errno);
+
+ /* Checks move that will give more accesses. */
+ ASSERT_EQ(-1, rename(file1_s2d2, bind_file1_s1d3));
+ ASSERT_EQ(EXDEV, errno);
+
+ /* Checks legitimate downgrade move. */
+ ASSERT_EQ(0, rename(bind_file1_s1d3, file1_s2d2));
+}
+
#define LOWER_BASE TMP_DIR "/lower"
#define LOWER_DATA LOWER_BASE "/data"
static const char lower_fl1[] = LOWER_DATA "/fl1";
--
2.35.1
^ permalink raw reply related
* [PATCH v1 10/11] landlock: Document good practices about filesystem policies
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-11-mic@digikod.net
---
Documentation/userspace-api/landlock.rst | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 97db09d36a5c..cc3b52f65f99 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -156,6 +156,27 @@ ruleset.
Full working code can be found in `samples/landlock/sandboxer.c`_.
+Good practices
+--------------
+
+It is recommended setting access rights to file hierarchy leaves as much as
+possible. For instance, it is better to be able to have ``~/doc/`` as a
+read-only hierarchy and ``~/tmp/`` as a read-write hierarchy, compared to
+``~/`` as a read-only hierarchy and ``~/tmp/`` as a read-write hierarchy.
+Following this good practice leads to self-sufficient hierarchies that don't
+depend on their location (i.e. parent directories). This is particularly
+relevant when we want to allow linking or renaming. Indeed, having consistent
+access rights per directory enables to change the location of such directory
+without relying on the destination directory access rights (except those that
+are required for this operation, see `LANDLOCK_ACCESS_FS_REFER` documentation).
+Having self-sufficient hierarchies also helps to tighten the required access
+rights to the minimal set of data. This also helps avoid sinkhole directories,
+i.e. directories where data can be linked to but not linked from. However,
+this depends on data organization, which might not be controlled by developers.
+In this case, granting read-write access to ``~/tmp/``, instead of write-only
+access, would potentially allow to move ``~/tmp/`` to a non-readable directory
+and still keep the ability to list the content of ``~/tmp/``.
+
Layers of file path access rights
---------------------------------
--
2.35.1
^ permalink raw reply related
* [PATCH v1 06/11] landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Add a new LANDLOCK_ACCESS_FS_REFER access right to enable policy writers
to allow sandboxed processes to link and rename files from and to a
specific set of file hierarchies. This access right should be composed
with LANDLOCK_ACCESS_FS_MAKE_* for the destination of a link or rename,
and with LANDLOCK_ACCESS_FS_REMOVE_* for a source of a rename. This
lift a Landlock limitation that always denied changing the parent of an
inode.
Renaming or linking to the same directory is still always allowed,
whatever LANDLOCK_ACCESS_FS_REFER is used or not, because it is not
considered a threat to user data.
However, creating multiple links or renaming to a different parent
directory may lead to privilege escalations if not handled properly.
Indeed, we must be sure that the source doesn't gain more privileges by
being accessible from the destination. This is handled by making sure
that the source hierarchy (including the referenced file or directory
itself) restricts at least as much the destination hierarchy. If it is
not the case, an EXDEV error is returned, making it potentially possible
for user space to copy the file hierarchy instead of moving or linking
it.
Instead of creating different access rights for the source and the
destination, we choose to make it simple and consistent for users.
Indeed, considering the previous constraint, it would be weird to
require such destination access right to be also granted to the source
(to make it a superset).
See the provided documentation for additional details.
New tests are provided with a following commit.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-7-mic@digikod.net
---
include/uapi/linux/landlock.h | 27 +-
security/landlock/fs.c | 550 ++++++++++++++++---
security/landlock/limits.h | 2 +-
security/landlock/syscalls.c | 2 +-
tools/testing/selftests/landlock/base_test.c | 2 +-
tools/testing/selftests/landlock/fs_test.c | 3 +-
6 files changed, 516 insertions(+), 70 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index b3d952067f59..f433d58a58f2 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -21,8 +21,14 @@ struct landlock_ruleset_attr {
/**
* @handled_access_fs: Bitmask of actions (cf. `Filesystem flags`_)
* that is handled by this ruleset and should then be forbidden if no
- * rule explicitly allow them. This is needed for backward
- * compatibility reasons.
+ * rule explicitly allow them: it is a deny-by-default list that should
+ * contain as much Landlock access rights as possible. Indeed, all
+ * Landlock filesystem access rights that are not part of
+ * handled_access_fs are allowed. This is needed for backward
+ * compatibility reasons. One exception is the
+ * LANDLOCK_ACCESS_FS_REFER access right, which is always implicitly
+ * handled, but must still be explicitly handled to add new rules with
+ * this access right.
*/
__u64 handled_access_fs;
};
@@ -109,6 +115,22 @@ struct landlock_path_beneath_attr {
* - %LANDLOCK_ACCESS_FS_MAKE_FIFO: Create (or rename or link) a named pipe.
* - %LANDLOCK_ACCESS_FS_MAKE_BLOCK: Create (or rename or link) a block device.
* - %LANDLOCK_ACCESS_FS_MAKE_SYM: Create (or rename or link) a symbolic link.
+ * - %LANDLOCK_ACCESS_FS_REFER: Link or rename a file from or to a different
+ * directory (i.e. reparent a file hierarchy). This access right is
+ * available since the second version of the Landlock ABI. This is also the
+ * only access right which is always considered handled by any ruleset in
+ * such a way that reparenting a file hierarchy is always denied by default.
+ * To avoid privilege escalation, it is not enough to add a rule with this
+ * access right. When linking or renaming a file, the destination directory
+ * hierarchy must also always have the same or a superset of restrictions of
+ * the source hierarchy. If it is not the case, or if the domain doesn't
+ * handle this access right, such actions are denied by default with errno
+ * set to EXDEV. Linking also requires a LANDLOCK_ACCESS_FS_MAKE_* access
+ * right on the destination directory, and renaming also requires a
+ * LANDLOCK_ACCESS_FS_REMOVE_* access right on the source's (file or
+ * directory) parent. Otherwise, such actions are denied with errno set to
+ * EACCES. The EACCES errno prevails over EXDEV to let user space
+ * efficiently deal with an unrecoverable error.
*
* .. warning::
*
@@ -133,5 +155,6 @@ struct landlock_path_beneath_attr {
#define LANDLOCK_ACCESS_FS_MAKE_FIFO (1ULL << 10)
#define LANDLOCK_ACCESS_FS_MAKE_BLOCK (1ULL << 11)
#define LANDLOCK_ACCESS_FS_MAKE_SYM (1ULL << 12)
+#define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)
#endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 3886f9ad1a60..c7c7ce4e7cd5 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -4,6 +4,7 @@
*
* Copyright © 2016-2020 Mickaël Salaün <mic@digikod.net>
* Copyright © 2018-2020 ANSSI
+ * Copyright © 2021-2022 Microsoft Corporation
*/
#include <linux/atomic.h>
@@ -269,16 +270,188 @@ static inline bool is_nouser_or_private(const struct dentry *dentry)
unlikely(IS_PRIVATE(d_backing_inode(dentry))));
}
-static int check_access_path(const struct landlock_ruleset *const domain,
- const struct path *const path,
+static inline access_mask_t get_handled_accesses(
+ const struct landlock_ruleset *const domain)
+{
+ access_mask_t access_dom = 0;
+ unsigned long access_bit;
+
+ for (access_bit = 0; access_bit < LANDLOCK_NUM_ACCESS_FS;
+ access_bit++) {
+ size_t layer_level;
+
+ for (layer_level = 0; layer_level < domain->num_layers;
+ layer_level++) {
+ if (domain->fs_access_masks[layer_level] &
+ BIT_ULL(access_bit)) {
+ access_dom |= BIT_ULL(access_bit);
+ break;
+ }
+ }
+ }
+ return access_dom;
+}
+
+static inline access_mask_t init_layer_masks(
+ const struct landlock_ruleset *const domain,
+ const access_mask_t access_request,
+ layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
+{
+ access_mask_t handled_accesses = 0;
+ size_t layer_level;
+
+ memset(layer_masks, 0, sizeof(*layer_masks));
+ if (WARN_ON_ONCE(!access_request))
+ return 0;
+
+ /* Saves all handled accesses per layer. */
+ for (layer_level = 0; layer_level < domain->num_layers;
+ layer_level++) {
+ const unsigned long access_req = access_request;
+ unsigned long access_bit;
+
+ for_each_set_bit(access_bit, &access_req,
+ ARRAY_SIZE(*layer_masks)) {
+ if (domain->fs_access_masks[layer_level] &
+ BIT_ULL(access_bit)) {
+ (*layer_masks)[access_bit] |=
+ BIT_ULL(layer_level);
+ handled_accesses |= BIT_ULL(access_bit);
+ }
+ }
+ }
+ return handled_accesses;
+}
+
+/*
+ * Check that a destination file hierarchy has more restrictions than a source
+ * file hierarchy. This is only used for link and rename actions.
+ */
+static inline bool is_superset(bool child_is_directory,
+ const layer_mask_t (*const
+ layer_masks_dst_parent)[LANDLOCK_NUM_ACCESS_FS],
+ const layer_mask_t (*const
+ layer_masks_src_parent)[LANDLOCK_NUM_ACCESS_FS],
+ const layer_mask_t (*const
+ layer_masks_child)[LANDLOCK_NUM_ACCESS_FS])
+{
+ unsigned long access_bit;
+
+ for (access_bit = 0; access_bit < ARRAY_SIZE(*layer_masks_dst_parent);
+ access_bit++) {
+ /* Ignores accesses that only make sense for directories. */
+ if (!child_is_directory && !(BIT_ULL(access_bit) & ACCESS_FILE))
+ continue;
+
+ /*
+ * Checks if the destination restrictions are a superset of the
+ * source ones (i.e. inherited access rights without child
+ * exceptions).
+ */
+ if ((((*layer_masks_src_parent)[access_bit] & (*layer_masks_child)[access_bit]) |
+ (*layer_masks_dst_parent)[access_bit]) !=
+ (*layer_masks_dst_parent)[access_bit])
+ return false;
+ }
+ return true;
+}
+
+/*
+ * Removes @layer_masks accesses that are not requested.
+ *
+ * Returns true if the request is allowed, false otherwise.
+ */
+static inline bool scope_to_request(const access_mask_t access_request,
+ layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
+{
+ const unsigned long access_req = access_request;
+ unsigned long access_bit;
+
+ if (WARN_ON_ONCE(!layer_masks))
+ return true;
+
+ for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
+ (*layer_masks)[access_bit] = 0;
+ return !memchr_inv(layer_masks, 0, sizeof(*layer_masks));
+}
+
+/*
+ * Returns true if there is at least one access right different than
+ * LANDLOCK_ACCESS_FS_REFER.
+ */
+static inline bool is_eacces(
+ const layer_mask_t (*const
+ layer_masks)[LANDLOCK_NUM_ACCESS_FS],
const access_mask_t access_request)
{
- layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
- bool allowed = false, has_access = false;
+ unsigned long access_bit;
+ /* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
+ const unsigned long access_check = access_request &
+ ~LANDLOCK_ACCESS_FS_REFER;
+
+ if (!layer_masks)
+ return false;
+
+ for_each_set_bit(access_bit, &access_check, ARRAY_SIZE(*layer_masks)) {
+ if ((*layer_masks)[access_bit])
+ return true;
+ }
+ return false;
+}
+
+/**
+ * check_access_path_dual - Check a source and a destination accesses
+ *
+ * @domain: Domain to check against.
+ * @path: File hierarchy to walk through.
+ * @child_is_directory: Must be set to true if the (original) leaf is a
+ * directory, false otherwise.
+ * @access_request_dst_parent: Accesses to check, once @layer_masks_dst_parent
+ * is equal to @layer_masks_src_parent (if any).
+ * @layer_masks_dst_parent: Pointer to a matrix of layer masks per access
+ * masks, identifying the layers that forbid a specific access. Bits from
+ * this matrix can be unset according to the @path walk. An empty matrix
+ * means that @domain allows all possible Landlock accesses (i.e. not only
+ * those identified by @access_request_dst_parent). This matrix can
+ * initially refer to domain layer masks and, when the accesses for the
+ * destination and source are the same, to request layer masks.
+ * @access_request_src_parent: Similar to @access_request_dst_parent but for an
+ * initial source path request. Only taken into account if
+ * @layer_masks_src_parent is not NULL.
+ * @layer_masks_src_parent: Similar to @layer_masks_dst_parent but for an
+ * initial source path walk. This can be NULL if only dealing with a
+ * destination access request (i.e. not a rename nor a link action).
+ * @layer_masks_child: Similar to @layer_masks_src_parent but only for the
+ * linked or renamed inode (without hierarchy). This is only used if
+ * @layer_masks_src_parent is not NULL.
+ *
+ * This helper first checks that the destination has a superset of restrictions
+ * compared to the source (if any) for a common path. It then checks that the
+ * collected accesses and the remaining ones are enough to allow the request.
+ *
+ * Returns:
+ * - 0 if the access request is granted;
+ * - -EACCES if it is denied because of access right other than
+ * LANDLOCK_ACCESS_FS_REFER;
+ * - -EXDEV if the renaming or linking would be a privileged escalation
+ * (according to each layered policies), or if LANDLOCK_ACCESS_FS_REFER is
+ * not allowed by the source or the destination.
+ */
+static int check_access_path_dual(const struct landlock_ruleset *const domain,
+ const struct path *const path,
+ bool child_is_directory,
+ const access_mask_t access_request_dst_parent,
+ layer_mask_t (*const
+ layer_masks_dst_parent)[LANDLOCK_NUM_ACCESS_FS],
+ const access_mask_t access_request_src_parent,
+ layer_mask_t (*layer_masks_src_parent)[LANDLOCK_NUM_ACCESS_FS],
+ layer_mask_t (*layer_masks_child)[LANDLOCK_NUM_ACCESS_FS])
+{
+ bool allowed_dst_parent = false, allowed_src_parent = false, is_dom_check;
struct path walker_path;
- size_t i;
+ access_mask_t access_masked_dst_parent, access_masked_src_parent;
- if (!access_request)
+ if (!access_request_dst_parent && !access_request_src_parent)
return 0;
if (WARN_ON_ONCE(!domain || !path))
return 0;
@@ -287,22 +460,20 @@ static int check_access_path(const struct landlock_ruleset *const domain,
if (WARN_ON_ONCE(domain->num_layers < 1))
return -EACCES;
- /* Saves all layers handling a subset of requested accesses. */
- for (i = 0; i < domain->num_layers; i++) {
- const unsigned long access_req = access_request;
- unsigned long access_bit;
-
- for_each_set_bit(access_bit, &access_req,
- ARRAY_SIZE(layer_masks)) {
- if (domain->fs_access_masks[i] & BIT_ULL(access_bit)) {
- layer_masks[access_bit] |= BIT_ULL(i);
- has_access = true;
- }
- }
+ BUILD_BUG_ON(!layer_masks_dst_parent);
+ if (layer_masks_src_parent) {
+ if (WARN_ON_ONCE(!layer_masks_child))
+ return -EACCES;
+ access_masked_dst_parent = access_masked_src_parent =
+ get_handled_accesses(domain);
+ is_dom_check = true;
+ } else {
+ if (WARN_ON_ONCE(layer_masks_child))
+ return -EACCES;
+ access_masked_dst_parent = access_request_dst_parent;
+ access_masked_src_parent = access_request_src_parent;
+ is_dom_check = false;
}
- /* An access request not handled by the domain is allowed. */
- if (!has_access)
- return 0;
walker_path = *path;
path_get(&walker_path);
@@ -312,11 +483,50 @@ static int check_access_path(const struct landlock_ruleset *const domain,
*/
while (true) {
struct dentry *parent_dentry;
+ const struct landlock_rule *rule;
+
+ /*
+ * If at least all accesses allowed on the destination are
+ * already allowed on the source, respectively if there is at
+ * least as much as restrictions on the destination than on the
+ * source, then we can safely refer files from the source to
+ * the destination without risking a privilege escalation.
+ * This is crucial for standalone multilayered security
+ * policies. Furthermore, this helps avoid policy writers to
+ * shoot themselves in the foot.
+ */
+ if (is_dom_check && is_superset(child_is_directory,
+ layer_masks_dst_parent,
+ layer_masks_src_parent,
+ layer_masks_child)) {
+ allowed_dst_parent =
+ scope_to_request(access_request_dst_parent,
+ layer_masks_dst_parent);
+ allowed_src_parent =
+ scope_to_request(access_request_src_parent,
+ layer_masks_src_parent);
+
+ /* Stops when all accesses are granted. */
+ if (allowed_dst_parent && allowed_src_parent)
+ break;
+
+ /*
+ * Downgrades checks from domain handled accesses to
+ * requested accesses.
+ */
+ is_dom_check = false;
+ access_masked_dst_parent = access_request_dst_parent;
+ access_masked_src_parent = access_request_src_parent;
+ }
+
+ rule = find_rule(domain, walker_path.dentry);
+ allowed_dst_parent = unmask_layers(rule, access_masked_dst_parent,
+ layer_masks_dst_parent);
+ allowed_src_parent = unmask_layers(rule, access_masked_src_parent,
+ layer_masks_src_parent);
- allowed = unmask_layers(find_rule(domain, walker_path.dentry),
- access_request, &layer_masks);
- if (allowed)
- /* Stops when a rule from each layer grants access. */
+ /* Stops when a rule from each layer grants access. */
+ if (allowed_dst_parent && allowed_src_parent)
break;
jump_up:
@@ -329,7 +539,7 @@ static int check_access_path(const struct landlock_ruleset *const domain,
* Stops at the real root. Denies access
* because not all layers have granted access.
*/
- allowed = false;
+ allowed_dst_parent = false;
break;
}
}
@@ -339,7 +549,8 @@ static int check_access_path(const struct landlock_ruleset *const domain,
* access to internal filesystems (e.g. nsfs, which is
* reachable through /proc/<pid>/ns/<namespace>).
*/
- allowed = !!(walker_path.mnt->mnt_flags & MNT_INTERNAL);
+ allowed_dst_parent = !!(walker_path.mnt->mnt_flags &
+ MNT_INTERNAL);
break;
}
parent_dentry = dget_parent(walker_path.dentry);
@@ -347,7 +558,40 @@ static int check_access_path(const struct landlock_ruleset *const domain,
walker_path.dentry = parent_dentry;
}
path_put(&walker_path);
- return allowed ? 0 : -EACCES;
+
+ if (allowed_dst_parent && allowed_src_parent)
+ return 0;
+
+ /*
+ * Unfortunately, we cannot prioritize EACCES over EXDEV for all
+ * RENAME_EXCHANGE cases because it depends on the source and
+ * destination order. This could be changed with a new
+ * security_path_rename hook implementation.
+ */
+ if (likely(is_eacces(layer_masks_dst_parent, access_request_dst_parent)
+ || is_eacces(layer_masks_src_parent,
+ access_request_src_parent)))
+ return -EACCES;
+
+ /*
+ * Gracefully forbids reparenting if the destination directory
+ * hierarchy is not a superset of restrictions of the source directory
+ * hierarchy, or if LANDLOCK_ACCESS_FS_REFER is not allowed by the
+ * source or the destination.
+ */
+ return -EXDEV;
+}
+
+static inline int check_access_path(const struct landlock_ruleset *const domain,
+ const struct path *const path,
+ access_mask_t access_request)
+{
+ layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+
+ access_request = init_layer_masks(domain, access_request,
+ &layer_masks);
+ return check_access_path_dual(domain, path, d_is_dir(path->dentry),
+ access_request, &layer_masks, 0, NULL, NULL);
}
static inline int current_check_access_path(const struct path *const path,
@@ -394,6 +638,217 @@ static inline access_mask_t maybe_remove(const struct dentry *const dentry)
LANDLOCK_ACCESS_FS_REMOVE_FILE;
}
+/**
+ * collect_domain_accesses - Walk through a file path and collect accesses
+ *
+ * @domain: Domain to check against.
+ * @mnt_root: Last directory to check.
+ * @dir: Directory to start the walk from.
+ * @layer_masks_dom: Where to store the collected accesses.
+ *
+ * This helper is useful to begin a path walk from the @dir directory to a
+ * @mnt_root directory used as a mount point. This mount point is the common
+ * ancestor between the source and the destination of a renamed and linked
+ * file. While walking from @dir to @mnt_root, we record all the domain's
+ * allowed accesses in @layer_masks_dom.
+ *
+ * This is similar to check_access_path_dual() but much simpler because it only
+ * handles walking on the same mount point and only check one set of accesses.
+ *
+ * Returns:
+ * - true if all the domain access rights are allowed for @dir;
+ * - false if the walk reached @mnt_root.
+ */
+static bool collect_domain_accesses(
+ const struct landlock_ruleset *const domain,
+ const struct dentry *const mnt_root, struct dentry *dir,
+ layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS])
+{
+ unsigned long access_dom;
+ bool ret = false;
+
+ BUILD_BUG_ON(!layer_masks_dom);
+ if (WARN_ON_ONCE(!domain || !mnt_root || !dir))
+ return true;
+ if (is_nouser_or_private(dir))
+ return true;
+
+ access_dom = init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
+ layer_masks_dom);
+
+ dget(dir);
+ while (true) {
+ struct dentry *parent_dentry;
+
+ /* Gets all layers allowing all domain accesses. */
+ if (unmask_layers(find_rule(domain, dir), access_dom,
+ layer_masks_dom)) {
+ /*
+ * Stops when all handled accesses are allowed by at
+ * least one rule in each layer.
+ */
+ ret = true;
+ break;
+ }
+
+ /* We should not reach a root other than @mnt_root. */
+ if (dir == mnt_root || WARN_ON_ONCE(IS_ROOT(dir))) {
+ ret = false;
+ break;
+ }
+
+ parent_dentry = dget_parent(dir);
+ dput(dir);
+ dir = parent_dentry;
+ }
+ dput(dir);
+ return ret;
+}
+
+/**
+ * current_check_refer_path - Check if a rename or link action is allowed
+ *
+ * @old_dentry: File or directory requested to be moved or linked.
+ * @new_dir: Destination parent directory.
+ * @new_dentry: Destination file or directory.
+ * @removable: Sets to true if it is a rename operation.
+ *
+ * Because of its unprivileged constraints, Landlock relies on file hierarchies
+ * (and not only inodes) to tie access rights to files. Being able to link or
+ * rename a file hierarchy brings some challenges. Indeed, moving or linking a
+ * file (i.e. creating a new reference to an inode) can have an impact on the
+ * actions allowed for a set of files if it would change its parent directory
+ * (i.e. reparenting).
+ *
+ * To avoid trivial access right bypasses, Landlock first checks if the file or
+ * directory requested to be moved would gain new access rights inherited from
+ * its new hierarchy. Before returning any error, Landlock then checks that
+ * the parent source hierarchy and the destination hierarchy would allow the
+ * link or rename action. If it is not the case, an error with EACCES is
+ * returned to inform user space that there is no way to remove or create the
+ * requested source file type. If it should be allowed but the new inherited
+ * access rights would be greater than the source access rights, then the
+ * kernel returns an error with EXDEV. Prioritizing EACCES over EXDEV enables
+ * user space to abort the whole operation if there is no way to do it, or to
+ * manually copy the source to the destination if this remains allowed, e.g.
+ * because file creation is allowed on the destination directory but not direct
+ * linking.
+ *
+ * To achieve this goal, the kernel needs to compare two file hierarchies: the
+ * one identifying the source file or directory (including itself), and the
+ * destination one. This can be seen as a multilayer partial ordering problem.
+ * The kernel walks through these paths and collect in a matrix the access
+ * rights that are denied per layer. These matrices are then compared to see
+ * if the destination one has more (or the same) restrictions as the source
+ * one. If this is the case, the requested action will not return EXDEV, which
+ * doesn't mean the action is allowed. The parent hierarchy of the source
+ * (i.e. parent directory), and the destination hierarchy must also be checked
+ * to verify that they explicitly allow such action (i.e. referencing,
+ * creation and potentially removal rights). The kernel implementation is then
+ * required to rely on three matrices of access rights: one for the source file
+ * or directory (i.e. the child), one for the source parent hierarchy and one
+ * for the destination hierarchy. These ephemeral matrices take some space on
+ * the stack, which limits the number of layers to a deemed reasonable number:
+ * 16.
+ *
+ * Returns:
+ * - 0 if access is allowed;
+ * - -EXDEV if @old_dentry would inherit new access rights from @new_dir;
+ * - -EACCES if file removal or creation is denied.
+ */
+static int current_check_refer_path(struct dentry *const old_dentry,
+ const struct path *const new_dir,
+ struct dentry *const new_dentry,
+ bool removable)
+{
+ const struct landlock_ruleset *const dom =
+ landlock_get_current_domain();
+ bool allow_dst_parent, allow_src_parent;
+ access_mask_t access_request_dst_parent, access_request_src_parent,
+ access_child;
+ struct path mnt_dir;
+ layer_mask_t layer_masks_dst_parent[LANDLOCK_NUM_ACCESS_FS],
+ layer_masks_src_parent[LANDLOCK_NUM_ACCESS_FS],
+ layer_masks_child[LANDLOCK_NUM_ACCESS_FS];
+
+ if (!dom)
+ return 0;
+ if (WARN_ON_ONCE(dom->num_layers < 1))
+ return -EACCES;
+ if (unlikely(d_is_negative(old_dentry)))
+ return -ENOENT;
+
+ access_request_dst_parent =
+ get_mode_access(d_backing_inode(old_dentry)->i_mode);
+ access_request_src_parent = 0;
+ if (removable) {
+ access_request_dst_parent |= maybe_remove(new_dentry);
+ access_request_src_parent |= maybe_remove(old_dentry);
+ }
+
+ /* The mount points are the same for old and new paths, cf. EXDEV. */
+ if (old_dentry->d_parent == new_dir->dentry) {
+ /*
+ * The LANDLOCK_ACCESS_FS_REFER access right is not required
+ * for same-directory referer (i.e. no reparenting).
+ */
+ access_request_dst_parent = init_layer_masks(dom,
+ access_request_dst_parent | access_request_src_parent,
+ &layer_masks_dst_parent);
+ return check_access_path_dual(dom, new_dir, d_is_dir(old_dentry),
+ access_request_dst_parent, &layer_masks_dst_parent,
+ 0, NULL, NULL);
+ }
+
+ /* Backward compatibility: no reparenting support. */
+ if (!(get_handled_accesses(dom) & LANDLOCK_ACCESS_FS_REFER))
+ return -EXDEV;
+
+ access_request_src_parent |= LANDLOCK_ACCESS_FS_REFER;
+ access_request_dst_parent |= LANDLOCK_ACCESS_FS_REFER;
+
+ /* Saves the common mount point. */
+ mnt_dir.mnt = new_dir->mnt;
+ mnt_dir.dentry = new_dir->mnt->mnt_root;
+
+ /* new_dir->dentry is equal to new_dentry->d_parent */
+ allow_dst_parent = collect_domain_accesses(dom, mnt_dir.dentry,
+ new_dir->dentry, &layer_masks_dst_parent);
+ allow_src_parent = collect_domain_accesses(dom, mnt_dir.dentry,
+ old_dentry->d_parent, &layer_masks_src_parent);
+
+ if (allow_src_parent) {
+ /* No need to go further if everything is allowed. */
+ if (allow_dst_parent)
+ return 0;
+
+ /* @new_dentry can only gain more restrictions. */
+ if (scope_to_request(access_request_dst_parent,
+ &layer_masks_dst_parent))
+ return 0;
+
+ return check_access_path_dual(dom, &mnt_dir, d_is_dir(old_dentry),
+ access_request_dst_parent, &layer_masks_dst_parent,
+ 0, NULL, NULL);
+ }
+
+ /*
+ * To be able to compare source and destination domain access rights,
+ * take into account the @old_dentry access rights aggregated with its
+ * parent access rights. This will be useful to compare with the
+ * destination parent access rights.
+ */
+ access_child = init_layer_masks(dom, LANDLOCK_MASK_ACCESS_FS,
+ &layer_masks_child);
+ unmask_layers(find_rule(dom, old_dentry), access_child,
+ &layer_masks_child);
+
+ return check_access_path_dual(dom, &mnt_dir, d_is_dir(old_dentry),
+ access_request_dst_parent, &layer_masks_dst_parent,
+ access_request_src_parent, &layer_masks_src_parent,
+ &layer_masks_child);
+}
+
/* Inode hooks */
static void hook_inode_free_security(struct inode *const inode)
@@ -587,31 +1042,11 @@ static int hook_sb_pivotroot(const struct path *const old_path,
/* Path hooks */
-/*
- * Creating multiple links or renaming may lead to privilege escalations if not
- * handled properly. Indeed, we must be sure that the source doesn't gain more
- * privileges by being accessible from the destination. This is getting more
- * complex when dealing with multiple layers. The whole picture can be seen as
- * a multilayer partial ordering problem. A future version of Landlock will
- * deal with that.
- */
static int hook_path_link(struct dentry *const old_dentry,
const struct path *const new_dir,
struct dentry *const new_dentry)
{
- const struct landlock_ruleset *const dom =
- landlock_get_current_domain();
-
- if (!dom)
- return 0;
- /* The mount points are the same for old and new paths, cf. EXDEV. */
- if (old_dentry->d_parent != new_dir->dentry)
- /* Gracefully forbids reparenting. */
- return -EXDEV;
- if (unlikely(d_is_negative(old_dentry)))
- return -ENOENT;
- return check_access_path(dom, new_dir,
- get_mode_access(d_backing_inode(old_dentry)->i_mode));
+ return current_check_refer_path(old_dentry, new_dir, new_dentry, false);
}
static int hook_path_rename(const struct path *const old_dir,
@@ -619,21 +1054,8 @@ static int hook_path_rename(const struct path *const old_dir,
const struct path *const new_dir,
struct dentry *const new_dentry)
{
- const struct landlock_ruleset *const dom =
- landlock_get_current_domain();
-
- if (!dom)
- return 0;
- /* The mount points are the same for old and new paths, cf. EXDEV. */
- if (old_dir->dentry != new_dir->dentry)
- /* Gracefully forbids reparenting. */
- return -EXDEV;
- if (unlikely(d_is_negative(old_dentry)))
- return -ENOENT;
- /* RENAME_EXCHANGE is handled because directories are the same. */
- return check_access_path(dom, old_dir, maybe_remove(old_dentry) |
- maybe_remove(new_dentry) |
- get_mode_access(d_backing_inode(old_dentry)->i_mode));
+ /* old_dir refers to old_dentry->d_parent and new_dir->mnt */
+ return current_check_refer_path(old_dentry, new_dir, new_dentry, true);
}
static int hook_path_mkdir(const struct path *const dir,
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 126d1ec04d34..26c8166d0265 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -16,7 +16,7 @@
#define LANDLOCK_MAX_NUM_LAYERS 16
#define LANDLOCK_MAX_NUM_RULES U32_MAX
-#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_MAKE_SYM
+#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_REFER
#define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 32396962f04d..fa14f09b6bf4 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -128,7 +128,7 @@ static const struct file_operations ruleset_fops = {
.write = fop_dummy_write,
};
-#define LANDLOCK_ABI_VERSION 1
+#define LANDLOCK_ABI_VERSION 2
/**
* sys_landlock_create_ruleset - Create a new ruleset
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index ca40abe9daa8..99aab93d50e1 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -67,7 +67,7 @@ TEST(abi_version) {
const struct landlock_ruleset_attr ruleset_attr = {
.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
};
- ASSERT_EQ(1, landlock_create_ruleset(NULL, 0,
+ ASSERT_EQ(2, landlock_create_ruleset(NULL, 0,
LANDLOCK_CREATE_RULESET_VERSION));
ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 1ac41bfa7382..0568d1193492 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -381,7 +381,7 @@ TEST_F_FORK(layout1, inval)
LANDLOCK_ACCESS_FS_WRITE_FILE | \
LANDLOCK_ACCESS_FS_READ_FILE)
-#define ACCESS_LAST LANDLOCK_ACCESS_FS_MAKE_SYM
+#define ACCESS_LAST LANDLOCK_ACCESS_FS_REFER
#define ACCESS_ALL ( \
ACCESS_FILE | \
@@ -394,6 +394,7 @@ TEST_F_FORK(layout1, inval)
LANDLOCK_ACCESS_FS_MAKE_SOCK | \
LANDLOCK_ACCESS_FS_MAKE_FIFO | \
LANDLOCK_ACCESS_FS_MAKE_BLOCK | \
+ LANDLOCK_ACCESS_FS_MAKE_SYM | \
ACCESS_LAST)
TEST_F_FORK(layout1, file_access_rights)
--
2.35.1
^ permalink raw reply related
* [PATCH v1 11/11] landlock: Add design choices documentation for filesystem access rights
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-12-mic@digikod.net
---
Documentation/security/landlock.rst | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/Documentation/security/landlock.rst b/Documentation/security/landlock.rst
index 3df68cb1d10f..621b2c1ac514 100644
--- a/Documentation/security/landlock.rst
+++ b/Documentation/security/landlock.rst
@@ -7,7 +7,7 @@ Landlock LSM: kernel documentation
==================================
:Author: Mickaël Salaün
-:Date: March 2021
+:Date: February 2022
Landlock's goal is to create scoped access-control (i.e. sandboxing). To
harden a whole system, this feature should be available to any process,
@@ -42,6 +42,21 @@ Guiding principles for safe access controls
* Computation related to Landlock operations (e.g. enforcing a ruleset) shall
only impact the processes requesting them.
+Design choices
+==============
+
+Filesystem access rights
+------------------------
+
+All access rights are tied to an inode and what can be accessed through it.
+Reading the content of a directory doesn't imply to be allowed to read the
+content of a listed inode. Indeed, a file name is local to its parent
+directory, and an inode can be referenced by multiple file names thanks to
+(hard) links. Being able to unlink a file only has a direct impact on the
+directory, not the unlinked inode. This is the reason why
+`LANDLOCK_ACCESS_FS_REMOVE_FILE` or `LANDLOCK_ACCESS_FS_REFER` are not allowed
+to be tied to files but only to directories.
+
Tests
=====
--
2.35.1
^ permalink raw reply related
* [PATCH v1 09/11] landlock: Document LANDLOCK_ACCESS_FS_REFER and ABI versioning
From: Mickaël Salaün @ 2022-02-21 21:25 UTC (permalink / raw)
To: James Morris, Serge E . Hallyn
Cc: Mickaël Salaün, Al Viro, Jann Horn, Kees Cook,
Konstantin Meskhidze, Paul Moore, Shuah Khan, linux-doc,
linux-fsdevel, linux-kernel, linux-security-module,
Mickaël Salaün
In-Reply-To: <20220221212522.320243-1-mic@digikod.net>
From: Mickaël Salaün <mic@linux.microsoft.com>
Add LANDLOCK_ACCESS_FS_REFER in the example and properly check to only
use it if the current kernel support it thanks to the Landlock ABI
version.
Move the file renaming and linking limitation to a new "Previous
limitations" section.
Improve documentation about the backward and forward compatibility,
including the rational for ruleset's handled_access_fs.
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221212522.320243-10-mic@digikod.net
---
Documentation/userspace-api/landlock.rst | 124 +++++++++++++++++++----
1 file changed, 104 insertions(+), 20 deletions(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index f35552ff19ba..97db09d36a5c 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -8,7 +8,7 @@ Landlock: unprivileged access control
=====================================
:Author: Mickaël Salaün
-:Date: March 2021
+:Date: February 2022
The goal of Landlock is to enable to restrict ambient rights (e.g. global
filesystem access) for a set of processes. Because Landlock is a stackable
@@ -29,14 +29,15 @@ the thread enforcing it, and its future children.
Defining and enforcing a security policy
----------------------------------------
-We first need to create the ruleset that will contain our rules. For this
+We first need to define the ruleset that will contain our rules. For this
example, the ruleset will contain rules that only allow read actions, but write
actions will be denied. The ruleset then needs to handle both of these kind of
-actions.
+actions. This is required for backward and forward compatibility (i.e. the
+kernel and user space may not know each other's supported restrictions), hence
+the need to be explicit about the denied-by-default access rights.
.. code-block:: c
- int ruleset_fd;
struct landlock_ruleset_attr ruleset_attr = {
.handled_access_fs =
LANDLOCK_ACCESS_FS_EXECUTE |
@@ -51,9 +52,34 @@ actions.
LANDLOCK_ACCESS_FS_MAKE_SOCK |
LANDLOCK_ACCESS_FS_MAKE_FIFO |
LANDLOCK_ACCESS_FS_MAKE_BLOCK |
- LANDLOCK_ACCESS_FS_MAKE_SYM,
+ LANDLOCK_ACCESS_FS_MAKE_SYM |
+ LANDLOCK_ACCESS_FS_REFER,
};
+Because we may not know on which kernel version an application will be
+executed, it is safer to follow a best-effort security approach. Indeed, we
+should try to protect users as much as possible whatever the kernel they are
+using. To avoid binary enforcement (i.e. either all security features or
+none), we can leverage a dedicated Landlock command to get the current version
+of the Landlock ABI and adapt the handled accesses. Let's check if we should
+remove the `LANDLOCK_ACCESS_FS_REFER` access right which is only supported
+starting with the second version of the ABI.
+
+.. code-block:: c
+
+ int abi;
+
+ abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
+ if (abi < 2) {
+ ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
+ }
+
+This enables to create an inclusive ruleset that will contain our rules.
+
+.. code-block:: c
+
+ int ruleset_fd;
+
ruleset_fd = landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
if (ruleset_fd < 0) {
perror("Failed to create a ruleset");
@@ -92,6 +118,11 @@ descriptor.
return 1;
}
+It may also be required to create rules following the same logic as explained
+for the ruleset creation, by filtering access rights according to the Landlock
+ABI version. In this example, this is not required because
+`LANDLOCK_ACCESS_FS_REFER` is not allowed by any rule.
+
We now have a ruleset with one rule allowing read access to ``/usr`` while
denying all other handled accesses for the filesystem. The next step is to
restrict the current thread from gaining more privileges (e.g. thanks to a SUID
@@ -192,6 +223,56 @@ To be allowed to use :manpage:`ptrace(2)` and related syscalls on a target
process, a sandboxed process should have a subset of the target process rules,
which means the tracee must be in a sub-domain of the tracer.
+Compatibility
+=============
+
+Backward and forward compatibility
+----------------------------------
+
+Landlock is designed to be compatible with past and future versions of the
+kernel. This is achieved thanks to the system call attributes and the
+associated bitflags, particularly the ruleset's `handled_access_fs`. Making
+handled access right explicit enables the kernel and user space to have a clear
+contract with each other. This is required to make sure sandboxing will not
+get stricter with a system update, which could break applications.
+
+Developers can subscribe to the `Landlock mailing list
+<https://subspace.kernel.org/lists.linux.dev.html>`_ to knowingly update and
+test their applications with the latest available features. In the interest of
+users, and because they may use different kernel versions, it is strongly
+encouraged to follow a best-effort security approach by checking the Landlock
+ABI version at runtime and only enforcing the supported features.
+
+Landlock ABI versions
+---------------------
+
+The Landlock ABI version can be read with the sys_landlock_create_ruleset()
+system call:
+
+.. code-block:: c
+
+ int abi;
+
+ abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
+ if (abi < 0) {
+ switch (errno) {
+ case ENOSYS:
+ printf("Landlock is not supported by the current kernel.\n");
+ break;
+ case EOPNOTSUPP:
+ printf("Landlock is currently disabled.\n");
+ break;
+ }
+ return 0;
+ }
+ if (abi >= 2) {
+ printf("Landlock supports LANDLOCK_ACCESS_FS_REFER.\n");
+ }
+
+The following kernel interfaces are implicitly supported by the first ABI
+version. Features only supported from a specific version are explicitly marked
+as such.
+
Kernel interface
================
@@ -228,21 +309,6 @@ Enforcing a ruleset
Current limitations
===================
-File renaming and linking
--------------------------
-
-Because Landlock targets unprivileged access controls, it is needed to properly
-handle composition of rules. Such property also implies rules nesting.
-Properly handling multiple layers of ruleset, each one of them able to restrict
-access to files, also implies to inherit the ruleset restrictions from a parent
-to its hierarchy. Because files are identified and restricted by their
-hierarchy, moving or linking a file from one directory to another implies to
-propagate the hierarchy constraints. To protect against privilege escalations
-through renaming or linking, and for the sake of simplicity, Landlock currently
-limits linking and renaming to the same directory. Future Landlock evolutions
-will enable more flexibility for renaming and linking, with dedicated ruleset
-flags.
-
Filesystem topology modification
--------------------------------
@@ -281,6 +347,24 @@ Memory usage
Kernel memory allocated to create rulesets is accounted and can be restricted
by the Documentation/admin-guide/cgroup-v1/memory.rst.
+Previous limitations
+====================
+
+File renaming and linking (ABI 1)
+---------------------------------
+
+Because Landlock targets unprivileged access controls, it is needed to properly
+handle composition of rules. Such property also implies rules nesting.
+Properly handling multiple layers of ruleset, each one of them able to restrict
+access to files, also implies to inherit the ruleset restrictions from a parent
+to its hierarchy. Because files are identified and restricted by their
+hierarchy, moving or linking a file from one directory to another implies to
+propagate the hierarchy constraints. To protect against privilege escalations
+through renaming or linking, and for the sake of simplicity, Landlock previously
+limited linking and renaming to the same directory. Starting with the Landlock
+ABI version 2, it is now possible to securely control renaming and linking
+thanks to the new `LANDLOCK_ACCESS_FS_REFER` access right.
+
Questions and answers
=====================
--
2.35.1
^ permalink raw reply related
* Re: [PATCH v1 06/11] landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER
From: kernel test robot @ 2022-02-22 3:16 UTC (permalink / raw)
To: Mickaël Salaün, James Morris, Serge E . Hallyn
Cc: llvm, kbuild-all, Mickaël Salaün, Al Viro, Jann Horn,
Kees Cook, Konstantin Meskhidze, Paul Moore, Shuah Khan,
linux-doc, linux-fsdevel, linux-kernel, linux-security-module
In-Reply-To: <20220221212522.320243-7-mic@digikod.net>
Hi "Mickaël,
I love your patch! Yet something to improve:
[auto build test ERROR on cfb92440ee71adcc2105b0890bb01ac3cddb8507]
url: https://github.com/0day-ci/linux/commits/Micka-l-Sala-n/Landlock-file-linking-and-renaming-support/20220222-051842
base: cfb92440ee71adcc2105b0890bb01ac3cddb8507
config: hexagon-randconfig-r002-20220221 (https://download.01.org/0day-ci/archive/20220222/202202221149.qLO9DEqo-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c68b879f54d6262963d435a18cedbc238b7faeaf
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Micka-l-Sala-n/Landlock-file-linking-and-renaming-support/20220222-051842
git checkout c68b879f54d6262963d435a18cedbc238b7faeaf
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> security/landlock/fs.c:463:2: error: call to __compiletime_assert_228 declared with 'error' attribute: BUILD_BUG_ON failed: !layer_masks_dst_parent
BUILD_BUG_ON(!layer_masks_dst_parent);
^
include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^
include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
include/linux/compiler_types.h:346:2: note: expanded from macro 'compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^
include/linux/compiler_types.h:334:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
include/linux/compiler_types.h:327:4: note: expanded from macro '__compiletime_assert'
prefix ## suffix(); \
^
<scratch space>:170:1: note: expanded from here
__compiletime_assert_228
^
>> security/landlock/fs.c:670:2: error: call to __compiletime_assert_229 declared with 'error' attribute: BUILD_BUG_ON failed: !layer_masks_dom
BUILD_BUG_ON(!layer_masks_dom);
^
include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^
include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
include/linux/compiler_types.h:346:2: note: expanded from macro 'compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^
include/linux/compiler_types.h:334:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
include/linux/compiler_types.h:327:4: note: expanded from macro '__compiletime_assert'
prefix ## suffix(); \
^
<scratch space>:174:1: note: expanded from here
__compiletime_assert_229
^
2 errors generated.
vim +/error +463 security/landlock/fs.c
401
402 /**
403 * check_access_path_dual - Check a source and a destination accesses
404 *
405 * @domain: Domain to check against.
406 * @path: File hierarchy to walk through.
407 * @child_is_directory: Must be set to true if the (original) leaf is a
408 * directory, false otherwise.
409 * @access_request_dst_parent: Accesses to check, once @layer_masks_dst_parent
410 * is equal to @layer_masks_src_parent (if any).
411 * @layer_masks_dst_parent: Pointer to a matrix of layer masks per access
412 * masks, identifying the layers that forbid a specific access. Bits from
413 * this matrix can be unset according to the @path walk. An empty matrix
414 * means that @domain allows all possible Landlock accesses (i.e. not only
415 * those identified by @access_request_dst_parent). This matrix can
416 * initially refer to domain layer masks and, when the accesses for the
417 * destination and source are the same, to request layer masks.
418 * @access_request_src_parent: Similar to @access_request_dst_parent but for an
419 * initial source path request. Only taken into account if
420 * @layer_masks_src_parent is not NULL.
421 * @layer_masks_src_parent: Similar to @layer_masks_dst_parent but for an
422 * initial source path walk. This can be NULL if only dealing with a
423 * destination access request (i.e. not a rename nor a link action).
424 * @layer_masks_child: Similar to @layer_masks_src_parent but only for the
425 * linked or renamed inode (without hierarchy). This is only used if
426 * @layer_masks_src_parent is not NULL.
427 *
428 * This helper first checks that the destination has a superset of restrictions
429 * compared to the source (if any) for a common path. It then checks that the
430 * collected accesses and the remaining ones are enough to allow the request.
431 *
432 * Returns:
433 * - 0 if the access request is granted;
434 * - -EACCES if it is denied because of access right other than
435 * LANDLOCK_ACCESS_FS_REFER;
436 * - -EXDEV if the renaming or linking would be a privileged escalation
437 * (according to each layered policies), or if LANDLOCK_ACCESS_FS_REFER is
438 * not allowed by the source or the destination.
439 */
440 static int check_access_path_dual(const struct landlock_ruleset *const domain,
441 const struct path *const path,
442 bool child_is_directory,
443 const access_mask_t access_request_dst_parent,
444 layer_mask_t (*const
445 layer_masks_dst_parent)[LANDLOCK_NUM_ACCESS_FS],
446 const access_mask_t access_request_src_parent,
447 layer_mask_t (*layer_masks_src_parent)[LANDLOCK_NUM_ACCESS_FS],
448 layer_mask_t (*layer_masks_child)[LANDLOCK_NUM_ACCESS_FS])
449 {
450 bool allowed_dst_parent = false, allowed_src_parent = false, is_dom_check;
451 struct path walker_path;
452 access_mask_t access_masked_dst_parent, access_masked_src_parent;
453
454 if (!access_request_dst_parent && !access_request_src_parent)
455 return 0;
456 if (WARN_ON_ONCE(!domain || !path))
457 return 0;
458 if (is_nouser_or_private(path->dentry))
459 return 0;
460 if (WARN_ON_ONCE(domain->num_layers < 1))
461 return -EACCES;
462
> 463 BUILD_BUG_ON(!layer_masks_dst_parent);
464 if (layer_masks_src_parent) {
465 if (WARN_ON_ONCE(!layer_masks_child))
466 return -EACCES;
467 access_masked_dst_parent = access_masked_src_parent =
468 get_handled_accesses(domain);
469 is_dom_check = true;
470 } else {
471 if (WARN_ON_ONCE(layer_masks_child))
472 return -EACCES;
473 access_masked_dst_parent = access_request_dst_parent;
474 access_masked_src_parent = access_request_src_parent;
475 is_dom_check = false;
476 }
477
478 walker_path = *path;
479 path_get(&walker_path);
480 /*
481 * We need to walk through all the hierarchy to not miss any relevant
482 * restriction.
483 */
484 while (true) {
485 struct dentry *parent_dentry;
486 const struct landlock_rule *rule;
487
488 /*
489 * If at least all accesses allowed on the destination are
490 * already allowed on the source, respectively if there is at
491 * least as much as restrictions on the destination than on the
492 * source, then we can safely refer files from the source to
493 * the destination without risking a privilege escalation.
494 * This is crucial for standalone multilayered security
495 * policies. Furthermore, this helps avoid policy writers to
496 * shoot themselves in the foot.
497 */
498 if (is_dom_check && is_superset(child_is_directory,
499 layer_masks_dst_parent,
500 layer_masks_src_parent,
501 layer_masks_child)) {
502 allowed_dst_parent =
503 scope_to_request(access_request_dst_parent,
504 layer_masks_dst_parent);
505 allowed_src_parent =
506 scope_to_request(access_request_src_parent,
507 layer_masks_src_parent);
508
509 /* Stops when all accesses are granted. */
510 if (allowed_dst_parent && allowed_src_parent)
511 break;
512
513 /*
514 * Downgrades checks from domain handled accesses to
515 * requested accesses.
516 */
517 is_dom_check = false;
518 access_masked_dst_parent = access_request_dst_parent;
519 access_masked_src_parent = access_request_src_parent;
520 }
521
522 rule = find_rule(domain, walker_path.dentry);
523 allowed_dst_parent = unmask_layers(rule, access_masked_dst_parent,
524 layer_masks_dst_parent);
525 allowed_src_parent = unmask_layers(rule, access_masked_src_parent,
526 layer_masks_src_parent);
527
528 /* Stops when a rule from each layer grants access. */
529 if (allowed_dst_parent && allowed_src_parent)
530 break;
531
532 jump_up:
533 if (walker_path.dentry == walker_path.mnt->mnt_root) {
534 if (follow_up(&walker_path)) {
535 /* Ignores hidden mount points. */
536 goto jump_up;
537 } else {
538 /*
539 * Stops at the real root. Denies access
540 * because not all layers have granted access.
541 */
542 allowed_dst_parent = false;
543 break;
544 }
545 }
546 if (unlikely(IS_ROOT(walker_path.dentry))) {
547 /*
548 * Stops at disconnected root directories. Only allows
549 * access to internal filesystems (e.g. nsfs, which is
550 * reachable through /proc/<pid>/ns/<namespace>).
551 */
552 allowed_dst_parent = !!(walker_path.mnt->mnt_flags &
553 MNT_INTERNAL);
554 break;
555 }
556 parent_dentry = dget_parent(walker_path.dentry);
557 dput(walker_path.dentry);
558 walker_path.dentry = parent_dentry;
559 }
560 path_put(&walker_path);
561
562 if (allowed_dst_parent && allowed_src_parent)
563 return 0;
564
565 /*
566 * Unfortunately, we cannot prioritize EACCES over EXDEV for all
567 * RENAME_EXCHANGE cases because it depends on the source and
568 * destination order. This could be changed with a new
569 * security_path_rename hook implementation.
570 */
571 if (likely(is_eacces(layer_masks_dst_parent, access_request_dst_parent)
572 || is_eacces(layer_masks_src_parent,
573 access_request_src_parent)))
574 return -EACCES;
575
576 /*
577 * Gracefully forbids reparenting if the destination directory
578 * hierarchy is not a superset of restrictions of the source directory
579 * hierarchy, or if LANDLOCK_ACCESS_FS_REFER is not allowed by the
580 * source or the destination.
581 */
582 return -EXDEV;
583 }
584
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* RE: [EXT] Re: [PATCH v4 5/5] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
From: Pankaj Gupta @ 2022-02-22 4:30 UTC (permalink / raw)
To: Matthias Schiffer, Ahmad Fatoum
Cc: kernel@pengutronix.de, David Gstir, tharvey@gateworks.com,
James Morris, Serge E. Hallyn, Horia Geanta, Aymen Sghaier,
Herbert Xu, David S. Miller, Udit Agarwal, Eric Biggers,
Jan Luebbe, Richard Weinberger, Franck Lenormand, Sumit Garg,
keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-doc@vger.kernel.org, linux-integrity@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, Jonathan Corbet,
David Howells, Jarkko Sakkinen, James Bottomley, Mimi Zohar
In-Reply-To: <4decdfb7d4395e967e1bf6c65212616400c8064a.camel@ew.tq-group.com>
Hi Ahmad,
> -----Original Message-----
> From: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> Sent: Monday, December 13, 2021 7:11 PM
> To: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: kernel@pengutronix.de; David Gstir <david@sigma-star.at>;
> tharvey@gateworks.com; James Morris <jmorris@namei.org>; Serge E.
> Hallyn <serge@hallyn.com>; Horia Geanta <horia.geanta@nxp.com>; Aymen
> Sghaier <aymen.sghaier@nxp.com>; Herbert Xu
> <herbert@gondor.apana.org.au>; David S. Miller <davem@davemloft.net>;
> Udit Agarwal <udit.agarwal@nxp.com>; Eric Biggers <ebiggers@kernel.org>;
> Jan Luebbe <j.luebbe@pengutronix.de>; Richard Weinberger
> <richard@nod.at>; Franck Lenormand <franck.lenormand@nxp.com>; Sumit
> Garg <sumit.garg@linaro.org>; keyrings@vger.kernel.org; linux-
> crypto@vger.kernel.org; linux-doc@vger.kernel.org; linux-
> integrity@vger.kernel.org; linux-kernel@vger.kernel.org; linux-security-
> module@vger.kernel.org; Jonathan Corbet <corbet@lwn.net>; David
> Howells <dhowells@redhat.com>; Jarkko Sakkinen <jarkko@kernel.org>;
> James Bottomley <jejb@linux.ibm.com>; Mimi Zohar <zohar@linux.ibm.com>
> Subject: [EXT] Re: [PATCH v4 5/5] KEYS: trusted: Introduce support for NXP
> CAAM-based trusted keys
>
> Caution: EXT Email
>
> On Mon, 2021-12-13 at 12:36 +0100, Ahmad Fatoum wrote:
> > Hello Matthias,
> >
> > On 13.12.21 12:00, Matthias Schiffer wrote:
> > > On Mon, 2021-10-11 at 12:02 +0200, Ahmad Fatoum wrote:
> > > > The Cryptographic Acceleration and Assurance Module (CAAM) is an
> > > > IP
> > > > Reviewed-by: David Gstir <david@sigma-star.at>
> > > > Tested-By: Tim Harvey <tharvey@gateworks.com>
> > > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > >
> > > Tested-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> >
> > Thanks for testing! Should I add your Tested-by: to the whole series,
> > or only this patch here?
>
> I didn't really do any tests regarding the RNG, so I think adding it to patches 4
> and 5 would be appropriate.
>
>
> >
> > > It is unfortunate that this implementation is incompatible with the
> > > "secure keys" feature found in linux-imx, as that would allow
> > > upgrading from linux-imx to mainline kernels in the future without
> > > losing access to keys. I did not follow the discussion of previous
> > > versions of this patch series, but I assume there is some reason why
> > > this code is not aligned with the linux-imx implementation?
> >
> > I don't use the vendor fork, so compatibility with it wasn't a
> > criteria for me. The format used in this series is very
> > straight-forward:
> > Key modifier is kernel:trusted and blob is exactly what's returned by
> > the CAAM. What would you change to make it linux-imx compatible?
>
> It seems that the only difference is the key modifier: linux-imx uses
> "SECURE_KEY". If I apply the following patch, I can load a key that was
> exported on linux-imx:
>
> --- a/security/keys/trusted-keys/trusted_caam.c
> +++ b/security/keys/trusted-keys/trusted_caam.c
> @@ -11,7 +11,7 @@
>
> static struct caam_blob_priv *blobifier;
>
> -#define KEYMOD "kernel:trusted"
> +#define KEYMOD "SECURE_KEY"
>
> static_assert(MAX_KEY_SIZE + CAAM_BLOB_OVERHEAD <=
> CAAM_BLOB_MAX_LEN); static_assert(MAX_BLOB_SIZE <=
> CAAM_BLOB_MAX_LEN);
> ---
>
>
> >
> > > Should the kernel emit some kind of warning if CAAM-based trusted
> > > keys are used, but the SoC has not been "closed" (if there is a nice
> > > way to detect that)? As the CAAM is using a common example key
> > > instead of the fused master key when HAB/secure boot are disabled,
> > > the kernel would basically be lying about the keys being trusted in
> > > this case.
> >
> > For now, this is pointed out in the documentation. If you have a
> > suggestion on a specific condition we should check and issue a
> > diagnostic on, I can incorporate it. An exhaustive if
> > WARN_ON(!secure()) is impossible, but having some warning for
> > unsuspecting users would indeed be nice.
>
> I don't know of any condition that doesn't involve looking at SoC- specific OTP
> registers - that's what U-Boot does to determine whether HAB is enabled...
>
Check the value fetched from the SEC Status Register (SSTA) (Offset 0xFD4h, bit 8,9 => 00b - Non-Secure, 01b - Secure, 10b - Trusted, 11b - Fail), for MOO (Mode of Operation).
And the warning can be issued accordingly.
It is to be noted that this register is part of CAAM page0, which might not be accessible to Linux, for all the iMX SoC(s).
For other SoC(s), this can be added.
> Regards,
> Matthias
>
>
> >
> > Cheers,
> > Ahmad
> >
> > > > ---
> > > > To: Jonathan Corbet <corbet@lwn.net>
> > > > To: David Howells <dhowells@redhat.com>
> > > > To: Jarkko Sakkinen <jarkko@kernel.org>
> > > > To: James Bottomley <jejb@linux.ibm.com>
> > > > To: Mimi Zohar <zohar@linux.ibm.com>
> > > > Cc: James Morris <jmorris@namei.org>
> > > > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> > > > Cc: "Horia Geantă" <horia.geanta@nxp.com>
> > > > Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
> > > > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > > > Cc: "David S. Miller" <davem@davemloft.net>
> > > > Cc: Udit Agarwal <udit.agarwal@nxp.com>
> > > > Cc: Eric Biggers <ebiggers@kernel.org>
> > > > Cc: Jan Luebbe <j.luebbe@pengutronix.de>
> > > > Cc: David Gstir <david@sigma-star.at>
> > > > Cc: Richard Weinberger <richard@nod.at>
> > > > Cc: Franck LENORMAND <franck.lenormand@nxp.com>
> > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > Cc: keyrings@vger.kernel.org
> > > > Cc: linux-crypto@vger.kernel.org
> > > > Cc: linux-doc@vger.kernel.org
> > > > Cc: linux-integrity@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Cc: linux-security-module@vger.kernel.org
> > > > ---
> > > > Documentation/admin-guide/kernel-parameters.txt | 1 +-
> > > > Documentation/security/keys/trusted-encrypted.rst | 42 ++++++++-
> > > > MAINTAINERS | 9 ++-
> > > > include/keys/trusted_caam.h | 11 ++-
> > > > security/keys/trusted-keys/Kconfig | 11 +-
> > > > security/keys/trusted-keys/Makefile | 2 +-
> > > > security/keys/trusted-keys/trusted_caam.c | 74
> > > > ++++++++++++++++-
> > > > security/keys/trusted-keys/trusted_core.c | 6 +-
> > > > 8 files changed, 152 insertions(+), 4 deletions(-) create mode
> > > > 100644 include/keys/trusted_caam.h create mode 100644
> > > > security/keys/trusted-keys/trusted_caam.c
> > > >
> > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > > > b/Documentation/admin-guide/kernel-parameters.txt
> > > > index d5969452f063..0ed1165e0f55 100644
> > > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > > @@ -5767,6 +5767,7 @@
> > > > sources:
> > > > - "tpm"
> > > > - "tee"
> > > > + - "caam"
> > > > If not specified then it defaults to iterating
> > > > through
> > > > the trust source list starting with TPM and
> > > > assigns the
> > > > first trust source as a backend which is
> > > > initialized diff --git
> > > > a/Documentation/security/keys/trusted-encrypted.rst
> > > > b/Documentation/security/keys/trusted-encrypted.rst
> > > > index 1d4b4b8f12f0..ad66573ca6fd 100644
> > > > --- a/Documentation/security/keys/trusted-encrypted.rst
> > > > +++ b/Documentation/security/keys/trusted-encrypted.rst
> > > > @@ -35,6 +35,13 @@ safe.
> > > > Rooted to Hardware Unique Key (HUK) which is generally
> > > > burnt in on-chip
> > > > fuses and is accessible to TEE only.
> > > >
> > > > + (3) CAAM (Cryptographic Acceleration and Assurance Module:
> > > > IP
> > > > on NXP SoCs)
> > > > +
> > > > + When High Assurance Boot (HAB) is enabled and the CAAM
> > > > is
> > > > in secure
> > > > + mode, trust is rooted to the OTPMK, a never-disclosed
> > > > 256-
> > > > bit key
> > > > + randomly generated and fused into each SoC at
> > > > manufacturing
> > > > time.
> > > > + Otherwise, a common fixed test key is used instead.
> > > > +
> > > > * Execution isolation
> > > >
> > > > (1) TPM
> > > > @@ -46,6 +53,10 @@ safe.
> > > > Customizable set of operations running in isolated
> > > > execution
> > > > environment verified via Secure/Trusted boot process.
> > > >
> > > > + (3) CAAM
> > > > +
> > > > + Fixed set of operations running in isolated execution
> > > > environment.
> > > > +
> > > > * Optional binding to platform integrity state
> > > >
> > > > (1) TPM
> > > > @@ -63,6 +74,11 @@ safe.
> > > > Relies on Secure/Trusted boot process for platform
> > > > integrity. It can
> > > > be extended with TEE based measured boot process.
> > > >
> > > > + (3) CAAM
> > > > +
> > > > + Relies on the High Assurance Boot (HAB) mechanism of
> > > > NXP
> > > > SoCs
> > > > + for platform integrity.
> > > > +
> > > > * Interfaces and APIs
> > > >
> > > > (1) TPM
> > > > @@ -74,10 +90,13 @@ safe.
> > > > TEEs have well-documented, standardized client interface
> > > > and APIs. For
> > > > more details refer to
> > > > ``Documentation/staging/tee.rst``.
> > > >
> > > > + (3) CAAM
> > > > +
> > > > + Interface is specific to silicon vendor.
> > > >
> > > > * Threat model
> > > >
> > > > - The strength and appropriateness of a particular TPM or TEE
> > > > for
> > > > a given
> > > > + The strength and appropriateness of a particular trust
> > > > source
> > > > for a given
> > > > purpose must be assessed when using them to protect
> > > > security-
> > > > relevant data.
> > > >
> > > >
> > > > @@ -104,8 +123,14 @@ selected trust source:
> > > > from platform specific hardware RNG or a software based
> > > > Fortuna CSPRNG
> > > > which can be seeded via multiple entropy sources.
> > > >
> > > > + * CAAM: Kernel RNG
> > > > +
> > > > + The normal kernel random number generator is used. To seed
> > > > it
> > > > from the
> > > > + CAAM HWRNG, enable CRYPTO_DEV_FSL_CAAM_RNG_API and
> ensure
> > > > the
> > > > device
> > > > + can be probed.
> > > > +
> > > > Optionally, users may specify ``trusted.kernel_rng=1`` on the
> > > > kernel -command-line to override the used RNG with the kernel's
> > > > random number pool.
> > > > +command-line to force use of the kernel's random number pool.
> > > >
> > > > Encrypted Keys
> > > > --------------
> > > > @@ -192,6 +217,19 @@ Usage::
> > > > specific to TEE device implementation. The key length for new
> > > > keys is always in bytes. Trusted Keys can be 32 - 128 bytes (256
> > > > - 1024 bits).
> > > >
> > > > +Trusted Keys usage: CAAM
> > > > +------------------------
> > > > +
> > > > +Usage::
> > > > +
> > > > + keyctl add trusted name "new keylen" ring
> > > > + keyctl add trusted name "load hex_blob" ring
> > > > + keyctl print keyid
> > > > +
> > > > +"keyctl print" returns an ASCII hex copy of the sealed key,
> > > > which is
> > > > in format
> > > > +specific to CAAM device implementation. The key length for new
> > > > keys
> > > > is always
> > > > +in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
> > > > +
> > > > Encrypted Keys usage
> > > > --------------------
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS index
> > > > a4a0c2baaf27..2c6514759222 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -10364,6 +10364,15 @@ S: Supported
> > > > F: include/keys/trusted_tee.h
> > > > F: security/keys/trusted-keys/trusted_tee.c
> > > >
> > > > +KEYS-TRUSTED-CAAM
> > > > +M: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > > +R: Pengutronix Kernel Team <kernel@pengutronix.de>
> > > > +L: linux-integrity@vger.kernel.org
> > > > +L: keyrings@vger.kernel.org
> > > > +S: Supported
> > > > +F: include/keys/trusted_caam.h
> > > > +F: security/keys/trusted-keys/trusted_caam.c
> > > > +
> > > > KEYS/KEYRINGS
> > > > M: David Howells <dhowells@redhat.com>
> > > > M: Jarkko Sakkinen <jarkko@kernel.org>
> > > > diff --git a/include/keys/trusted_caam.h
> > > > b/include/keys/trusted_caam.h new file mode 100644 index
> > > > 000000000000..2fba0996b0b0
> > > > --- /dev/null
> > > > +++ b/include/keys/trusted_caam.h
> > > > @@ -0,0 +1,11 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > > +/*
> > > > + * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <
> > > > kernel@pengutronix.de>
> > > > + */
> > > > +
> > > > +#ifndef __CAAM_TRUSTED_KEY_H
> > > > +#define __CAAM_TRUSTED_KEY_H
> > > > +
> > > > +extern struct trusted_key_ops caam_trusted_key_ops;
> > > > +
> > > > +#endif
> > > > diff --git a/security/keys/trusted-keys/Kconfig
> > > > b/security/keys/trusted-keys/Kconfig
> > > > index fc4abd581abb..dbfdd8536468 100644
> > > > --- a/security/keys/trusted-keys/Kconfig
> > > > +++ b/security/keys/trusted-keys/Kconfig
> > > > @@ -24,6 +24,15 @@ config TRUSTED_KEYS_TEE
> > > > Enable use of the Trusted Execution Environment (TEE) as
> > > > trusted
> > > > key backend.
> > > >
> > > > -if !TRUSTED_KEYS_TPM && !TRUSTED_KEYS_TEE
> > > > +config TRUSTED_KEYS_CAAM
> > > > + bool "CAAM-based trusted keys"
> > > > + depends on CRYPTO_DEV_FSL_CAAM_JR >= TRUSTED_KEYS select
> > > > +CRYPTO_DEV_FSL_CAAM_BLOB_GEN default y help
> > > > + Enable use of NXP's Cryptographic Accelerator and Assurance
> > > > Module
> > > > + (CAAM) as trusted key backend.
> > > > +
> > > > +if !TRUSTED_KEYS_TPM && !TRUSTED_KEYS_TEE
> && !TRUSTED_KEYS_CAAM
> > > > comment "No trust source selected!"
> > > > endif
> > > > diff --git a/security/keys/trusted-keys/Makefile
> > > > b/security/keys/trusted-keys/Makefile
> > > > index 2e2371eae4d5..735aa0bc08ef 100644
> > > > --- a/security/keys/trusted-keys/Makefile
> > > > +++ b/security/keys/trusted-keys/Makefile
> > > > @@ -12,3 +12,5 @@ trusted-$(CONFIG_TRUSTED_KEYS_TPM) +=
> > > > trusted_tpm2.o
> > > > trusted-$(CONFIG_TRUSTED_KEYS_TPM) += tpm2key.asn1.o
> > > >
> > > > trusted-$(CONFIG_TRUSTED_KEYS_TEE) += trusted_tee.o
> > > > +
> > > > +trusted-$(CONFIG_TRUSTED_KEYS_CAAM) += trusted_caam.o
> > > > diff --git a/security/keys/trusted-keys/trusted_caam.c
> > > > b/security/keys/trusted-keys/trusted_caam.c
> > > > new file mode 100644
> > > > index 000000000000..01adfd18adda
> > > > --- /dev/null
> > > > +++ b/security/keys/trusted-keys/trusted_caam.c
> > > > @@ -0,0 +1,74 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/*
> > > > + * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <
> > > > kernel@pengutronix.de>
> > > > + */
> > > > +
> > > > +#include <keys/trusted_caam.h>
> > > > +#include <keys/trusted-type.h>
> > > > +#include <linux/build_bug.h>
> > > > +#include <linux/key-type.h>
> > > > +#include <soc/fsl/caam-blob.h>
> > > > +
> > > > +static struct caam_blob_priv *blobifier;
> > > > +
> > > > +#define KEYMOD "kernel:trusted"
> > > > +
> > > > +static_assert(MAX_KEY_SIZE + CAAM_BLOB_OVERHEAD <=
> > > > CAAM_BLOB_MAX_LEN);
> > > > +static_assert(MAX_BLOB_SIZE <= CAAM_BLOB_MAX_LEN);
> > > > +
> > > > +static int trusted_caam_seal(struct trusted_key_payload *p, char
> > > > *datablob)
> > > > +{
> > > > + int length = p->key_len + CAAM_BLOB_OVERHEAD; int ret;
> > > > +
> > > > + ret = caam_encap_blob(blobifier, KEYMOD, p->key, p->blob,
> > > > length);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + p->blob_len = length;
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int trusted_caam_unseal(struct trusted_key_payload *p,
> > > > char
> > > > *datablob)
> > > > +{
> > > > + int length = p->blob_len;
> > > > + int ret;
> > > > +
> > > > + ret = caam_decap_blob(blobifier, KEYMOD, p->blob, p->key,
> > > > length);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + p->key_len = length - CAAM_BLOB_OVERHEAD;
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int trusted_caam_init(void) { int ret;
> > > > +
> > > > + blobifier = caam_blob_gen_init(); if (IS_ERR(blobifier)) {
> > > > + pr_err("Job Ring Device allocation for transform
> > > > failed\n");
> > > > + return PTR_ERR(blobifier); }
> > > > +
> > > > + ret = register_key_type(&key_type_trusted);
> > > > + if (ret)
> > > > + caam_blob_gen_exit(blobifier);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static void trusted_caam_exit(void) {
> > > > +unregister_key_type(&key_type_trusted);
> > > > + caam_blob_gen_exit(blobifier);
> > > > +}
> > > > +
> > > > +struct trusted_key_ops caam_trusted_key_ops = { .migratable = 0,
> > > > +/* non-migratable */ .init = trusted_caam_init, .seal =
> > > > +trusted_caam_seal, .unseal = trusted_caam_unseal, .exit =
> > > > +trusted_caam_exit, };
> > > > diff --git a/security/keys/trusted-keys/trusted_core.c
> > > > b/security/keys/trusted-keys/trusted_core.c
> > > > index d2b7626cde8b..305e44651180 100644
> > > > --- a/security/keys/trusted-keys/trusted_core.c
> > > > +++ b/security/keys/trusted-keys/trusted_core.c
> > > > @@ -9,6 +9,7 @@
> > > > #include <keys/user-type.h>
> > > > #include <keys/trusted-type.h>
> > > > #include <keys/trusted_tee.h>
> > > > +#include <keys/trusted_caam.h>
> > > > #include <keys/trusted_tpm.h>
> > > > #include <linux/capability.h>
> > > > #include <linux/err.h>
> > > > @@ -29,7 +30,7 @@ MODULE_PARM_DESC(kernel_rng, "Generate key
> > > > material from kernel RNG");
> > > >
> > > > static char *trusted_key_source;
> > > > module_param_named(source, trusted_key_source, charp, 0);
> > > > -MODULE_PARM_DESC(source, "Select trusted keys source (tpm or
> > > > tee)");
> > > > +MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee
> > > > or
> > > > caam)");
> > > >
> > > > static const struct trusted_key_source trusted_key_sources[] = {
> > > > #if defined(CONFIG_TRUSTED_KEYS_TPM) @@ -38,6 +39,9 @@ static
> > > > const struct trusted_key_source trusted_key_sources[] = { #if
> > > > defined(CONFIG_TRUSTED_KEYS_TEE)
> > > > { "tee", &trusted_key_tee_ops }, #endif
> > > > +#if defined(CONFIG_TRUSTED_KEYS_CAAM) { "caam",
> > > > +&caam_trusted_key_ops }, #endif
> > > > };
> > > >
> > > > DEFINE_STATIC_CALL_NULL(trusted_key_init,
> > > > *trusted_key_sources[0].ops->init);
> >
> >
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox