* [PATCH v5 0/8] LSM: Three basic syscalls
[not found] <20230109180717.58855-1-casey.ref@schaufler-ca.com>
@ 2023-01-09 18:07 ` Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 1/8] LSM: Identify modules by more than name Casey Schaufler
` (7 more replies)
0 siblings, 8 replies; 30+ messages in thread
From: Casey Schaufler @ 2023-01-09 18:07 UTC (permalink / raw)
To: casey.schaufler, paul, linux-security-module
Cc: casey, jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, mic
Add three system calls for the Linux Security Module ABI.
lsm_get_self_attr() provides the security module specific attributes
that have previously been visible in the /proc/self/attr directory.
For each security module that uses the specified attribute on the
current process the system call will return an LSM identifier and
the value of the attribute. The LSM and attribute identifier values
are defined in include/uapi/linux/lsm.h
LSM identifiers are simple integers and reflect the order in which
the LSM was added to the mainline kernel. This is a convention, not
a promise of the API. LSM identifiers below the value of 100 are
reserved for unspecified future uses. That could include information
about the security infrastructure itself, or about how multiple LSMs
might interact with each other.
lsm_module_list() provides the LSM identifiers, in order, of the
security modules that are active on the system. This has been
available in the securityfs file /sys/kernel/security/lsm.
lsm_set_self_attr() changes the specified LSM attribute. Only one
attribute can be changed at a time, and then only if the specified
security module allows the change.
Patch 0001 changes the LSM registration from passing the name
of the module to passing a lsm_id structure that contains the
name of the module, an LSM identifier number and an attribute
identifier.
Patch 0002 adds the registered lsm_ids to a table.
Patch 0003 changes security_[gs]etprocattr() to use LSM IDs instead
of LSM names.
Patch 0004 implements lsm_get_self_attr().
Patch 0005 implements lsm_module_list().
Patch 0006 implements lsm_set_self_attr().
Patch 0007 wires up the syscalls.
Patch 0008 implements selftests for the three new syscalls.
https://github.com/cschaufler/lsm-stacking.git#lsm-syscalls-6.2-rc2-v5
v5: Correct syscall parameter data types.
v4: Restore "reserved" LSM ID values. Add explaination.
Squash patches that introduce fields in lsm_id.
Correct a wireup error.
v3: Add lsm_set_self_attr().
Rename lsm_self_attr() to lsm_get_self_attr().
Provide the values only for a specifed attribute in
lsm_get_self_attr().
Add selftests for the three new syscalls.
Correct some parameter checking.
v2: Use user-interface safe data types.
Remove "reserved" LSM ID values.
Improve kerneldoc comments
Include copyright dates
Use more descriptive name for LSM counter
Add documentation
Correct wireup errors
Casey Schaufler (8):
LSM: Identify modules by more than name
LSM: Maintain a table of LSM attribute data
proc: Use lsmids instead of lsm names for attrs
LSM: lsm_get_self_attr syscall for LSM self attributes
LSM: Create lsm_module_list system call
LSM: lsm_set_self_attr syscall for LSM self attributes
LSM: wireup Linux Security Module syscalls
LSM: selftests for Linux Security Module syscalls
Documentation/userspace-api/index.rst | 1 +
Documentation/userspace-api/lsm.rst | 70 ++++
arch/alpha/kernel/syscalls/syscall.tbl | 3 +
arch/arm/tools/syscall.tbl | 3 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 6 +
arch/ia64/kernel/syscalls/syscall.tbl | 3 +
arch/m68k/kernel/syscalls/syscall.tbl | 3 +
arch/microblaze/kernel/syscalls/syscall.tbl | 3 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 3 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 3 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 3 +
arch/parisc/kernel/syscalls/syscall.tbl | 3 +
arch/powerpc/kernel/syscalls/syscall.tbl | 3 +
arch/s390/kernel/syscalls/syscall.tbl | 3 +
arch/sh/kernel/syscalls/syscall.tbl | 3 +
arch/sparc/kernel/syscalls/syscall.tbl | 3 +
arch/x86/entry/syscalls/syscall_32.tbl | 3 +
arch/x86/entry/syscalls/syscall_64.tbl | 3 +
arch/xtensa/kernel/syscalls/syscall.tbl | 3 +
fs/proc/base.c | 29 +-
fs/proc/internal.h | 2 +-
include/linux/lsm_hooks.h | 18 +-
include/linux/security.h | 13 +-
include/linux/syscalls.h | 6 +
include/uapi/asm-generic/unistd.h | 11 +-
include/uapi/linux/lsm.h | 76 ++++
kernel/sys_ni.c | 5 +
security/Makefile | 1 +
security/apparmor/lsm.c | 9 +-
security/bpf/hooks.c | 13 +-
security/commoncap.c | 8 +-
security/landlock/cred.c | 2 +-
security/landlock/fs.c | 2 +-
security/landlock/ptrace.c | 2 +-
security/landlock/setup.c | 6 +
security/landlock/setup.h | 1 +
security/loadpin/loadpin.c | 9 +-
security/lockdown/lockdown.c | 8 +-
security/lsm_syscalls.c | 264 ++++++++++++++
security/safesetid/lsm.c | 9 +-
security/security.c | 63 +++-
security/selinux/hooks.c | 11 +-
security/smack/smack_lsm.c | 9 +-
security/tomoyo/tomoyo.c | 9 +-
security/yama/yama_lsm.c | 8 +-
.../arch/mips/entry/syscalls/syscall_n64.tbl | 3 +
.../arch/powerpc/entry/syscalls/syscall.tbl | 3 +
.../perf/arch/s390/entry/syscalls/syscall.tbl | 3 +
.../arch/x86/entry/syscalls/syscall_64.tbl | 3 +
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/lsm/Makefile | 12 +
tools/testing/selftests/lsm/config | 2 +
.../selftests/lsm/lsm_get_self_attr_test.c | 268 ++++++++++++++
.../selftests/lsm/lsm_module_list_test.c | 149 ++++++++
.../selftests/lsm/lsm_set_self_attr_test.c | 328 ++++++++++++++++++
56 files changed, 1438 insertions(+), 55 deletions(-)
create mode 100644 Documentation/userspace-api/lsm.rst
create mode 100644 include/uapi/linux/lsm.h
create mode 100644 security/lsm_syscalls.c
create mode 100644 tools/testing/selftests/lsm/Makefile
create mode 100644 tools/testing/selftests/lsm/config
create mode 100644 tools/testing/selftests/lsm/lsm_get_self_attr_test.c
create mode 100644 tools/testing/selftests/lsm/lsm_module_list_test.c
create mode 100644 tools/testing/selftests/lsm/lsm_set_self_attr_test.c
--
2.39.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5 1/8] LSM: Identify modules by more than name
2023-01-09 18:07 ` [PATCH v5 0/8] LSM: Three basic syscalls Casey Schaufler
@ 2023-01-09 18:07 ` Casey Schaufler
2023-01-11 21:00 ` Paul Moore
2023-01-09 18:07 ` [PATCH v5 2/8] LSM: Maintain a table of LSM attribute data Casey Schaufler
` (6 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2023-01-09 18:07 UTC (permalink / raw)
To: casey.schaufler, paul, linux-security-module
Cc: casey, jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, mic
Create a struct lsm_id to contain identifying information
about Linux Security Modules (LSMs). At inception this contains
the name of the module, an identifier associated with the security
module and an integer member "attrs_used" which identifies the API
related data associated with each security module. The initial set
of features maps to information that has traditionaly been available
in /proc/self/attr. They are documented in a new userspace-api file.
Change the security_add_hooks() interface to use this structure.
Change the individual modules to maintain their own struct lsm_id
and pass it to security_add_hooks().
The values are for LSM identifiers are defined in a new UAPI
header file linux/lsm.h. Each existing LSM has been updated to
include it's LSMID in the lsm_id.
The LSM ID values are sequential, with the oldest module
LSM_ID_CAPABILITY being the lowest value and the existing modules
numbered in the order they were included in the main line kernel.
This is an arbitrary convention for assigning the values, but
none better presents itself. The value 0 is defined as being invalid.
The values 1-99 are reserved for any special case uses which may
arise in the future. This may include attributes of the LSM
infrastructure itself, possibly related to namespacing or network
attribute management. A special range is identified for such attributes
to help reduce confusion for developers unfamiliar with LSMs.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
Documentation/userspace-api/index.rst | 1 +
Documentation/userspace-api/lsm.rst | 55 +++++++++++++++++++++++++++
include/linux/lsm_hooks.h | 18 ++++++++-
include/uapi/linux/lsm.h | 55 +++++++++++++++++++++++++++
security/apparmor/lsm.c | 9 ++++-
security/bpf/hooks.c | 13 ++++++-
security/commoncap.c | 8 +++-
security/landlock/cred.c | 2 +-
security/landlock/fs.c | 2 +-
security/landlock/ptrace.c | 2 +-
security/landlock/setup.c | 6 +++
security/landlock/setup.h | 1 +
security/loadpin/loadpin.c | 9 ++++-
security/lockdown/lockdown.c | 8 +++-
security/safesetid/lsm.c | 9 ++++-
security/security.c | 12 +++---
security/selinux/hooks.c | 11 +++++-
security/smack/smack_lsm.c | 9 ++++-
security/tomoyo/tomoyo.c | 9 ++++-
security/yama/yama_lsm.c | 8 +++-
20 files changed, 226 insertions(+), 21 deletions(-)
create mode 100644 Documentation/userspace-api/lsm.rst
create mode 100644 include/uapi/linux/lsm.h
diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
index f16337bdb852..54c0f54cde89 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -31,6 +31,7 @@ place where this information is gathered.
sysfs-platform_profile
vduse
futex2
+ lsm
.. only:: subproject and html
diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst
new file mode 100644
index 000000000000..6ddf5506110b
--- /dev/null
+++ b/Documentation/userspace-api/lsm.rst
@@ -0,0 +1,55 @@
+.. SPDX-License-Identifier: GPL-2.0
+.. Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
+.. Copyright (C) 2022 Intel Corporation
+
+=====================================
+Linux Security Modules
+=====================================
+
+:Author: Casey Schaufler
+:Date: November 2022
+
+Linux security modules (LSM) provide a mechanism to implement
+additional access controls to the Linux security policies.
+
+The various security modules may support any of these attributes:
+
+``LSM_ATTR_CURRENT`` is the current, active security context of the
+process.
+The proc filesystem provides this value in ``/proc/self/attr/current``.
+This is supported by the SELinux, Smack and AppArmor security modules.
+Smack also provides this value in ``/proc/self/attr/smack/current``.
+AppArmor also provides this value in ``/proc/self/attr/apparmor/current``.
+
+``LSM_ATTR_EXEC`` is the security context of the process at the time the
+current image was executed.
+The proc filesystem provides this value in ``/proc/self/attr/exec``.
+This is supported by the SELinux and AppArmor security modules.
+AppArmor also provides this value in ``/proc/self/attr/apparmor/exec``.
+
+``LSM_ATTR_FSCREATE`` is the security context of the process used when
+creating file system objects.
+The proc filesystem provides this value in ``/proc/self/attr/fscreate``.
+This is supported by the SELinux security module.
+
+``LSM_ATTR_KEYCREATE`` is the security context of the process used when
+creating key objects.
+The proc filesystem provides this value in ``/proc/self/attr/keycreate``.
+This is supported by the SELinux security module.
+
+``LSM_ATTR_PREV`` is the security context of the process at the time the
+current security context was set.
+The proc filesystem provides this value in ``/proc/self/attr/prev``.
+This is supported by the SELinux and AppArmor security modules.
+AppArmor also provides this value in ``/proc/self/attr/apparmor/prev``.
+
+``LSM_ATTR_SOCKCREATE`` is the security context of the process used when
+creating socket objects.
+The proc filesystem provides this value in ``/proc/self/attr/sockcreate``.
+This is supported by the SELinux security module.
+
+Additional documentation
+========================
+
+* Documentation/security/lsm.rst
+* Documentation/security/lsm-development.rst
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 0a5ba81f7367..6f2cabb79ec4 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1665,6 +1665,20 @@ struct security_hook_heads {
#undef LSM_HOOK
} __randomize_layout;
+/**
+ * struct lsm_id - identify a Linux Security Module.
+ * @lsm: Name of the LSM. Must be approved by the LSM maintainers.
+ * @id: LSM ID number from uapi/linux/lsm.h
+ * @attrs_used: Which attributes this LSM supports.
+ *
+ * Contains the information that identifies the LSM.
+ */
+struct lsm_id {
+ const u8 *lsm;
+ u32 id;
+ u64 attrs_used;
+};
+
/*
* Security module hook list structure.
* For use with generic list macros for common operations.
@@ -1673,7 +1687,7 @@ struct security_hook_list {
struct hlist_node list;
struct hlist_head *head;
union security_list_options hook;
- const char *lsm;
+ struct lsm_id *lsmid;
} __randomize_layout;
/*
@@ -1708,7 +1722,7 @@ extern struct security_hook_heads security_hook_heads;
extern char *lsm_names;
extern void security_add_hooks(struct security_hook_list *hooks, int count,
- const char *lsm);
+ struct lsm_id *lsmid);
#define LSM_FLAG_LEGACY_MAJOR BIT(0)
#define LSM_FLAG_EXCLUSIVE BIT(1)
diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
new file mode 100644
index 000000000000..61a91b7d946f
--- /dev/null
+++ b/include/uapi/linux/lsm.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Linux Security Modules (LSM) - User space API
+ *
+ * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
+ * Copyright (C) 2022 Intel Corporation
+ */
+
+#ifndef _UAPI_LINUX_LSM_H
+#define _UAPI_LINUX_LSM_H
+
+/*
+ * ID values to identify security modules.
+ * A system may use more than one security module.
+ *
+ * A value of 0 is considered invalid.
+ * Values 1-99 are reserved for future use.
+ * The interface is designed to extend to attributes beyond those which
+ * are active today. Currently all the attributes are specific to the
+ * individual modules. The LSM infrastructure itself has no variable state,
+ * but that may change. One proposal would allow loadable modules, in which
+ * case an attribute such as LSM_IS_LOADABLE might identify the dynamic
+ * modules. Another potential attribute could be which security modules is
+ * associated withnetwork labeling using netlabel. Another possible attribute
+ * could be related to stacking behavior in a namespaced environment.
+ * While it would be possible to intermingle the LSM infrastructure attribute
+ * values with the security module provided values, keeping them separate
+ * provides a clearer distinction.
+ */
+#define LSM_ID_CAPABILITY 100
+#define LSM_ID_SELINUX 101
+#define LSM_ID_SMACK 102
+#define LSM_ID_TOMOYO 103
+#define LSM_ID_IMA 104
+#define LSM_ID_APPARMOR 105
+#define LSM_ID_YAMA 106
+#define LSM_ID_LOADPIN 107
+#define LSM_ID_SAFESETID 108
+#define LSM_ID_LOCKDOWN 109
+#define LSM_ID_BPF 110
+#define LSM_ID_LANDLOCK 111
+
+/*
+ * LSM_ATTR_XXX values identify the /proc/.../attr entry that the
+ * context represents. Not all security modules provide all of these
+ * values. Some security modules provide none of them.
+ */
+#define LSM_ATTR_CURRENT 0x0001
+#define LSM_ATTR_EXEC 0x0002
+#define LSM_ATTR_FSCREATE 0x0004
+#define LSM_ATTR_KEYCREATE 0x0008
+#define LSM_ATTR_PREV 0x0010
+#define LSM_ATTR_SOCKCREATE 0x0020
+
+#endif /* _UAPI_LINUX_LSM_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c6728a629437..63ea2a995987 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -24,6 +24,7 @@
#include <linux/zstd.h>
#include <net/sock.h>
#include <uapi/linux/mount.h>
+#include <uapi/linux/lsm.h>
#include "include/apparmor.h"
#include "include/apparmorfs.h"
@@ -1217,6 +1218,12 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
.lbs_task = sizeof(struct aa_task_ctx),
};
+static struct lsm_id apparmor_lsmid __lsm_ro_after_init = {
+ .lsm = "apparmor",
+ .id = LSM_ID_APPARMOR,
+ .attrs_used = LSM_ATTR_CURRENT | LSM_ATTR_PREV | LSM_ATTR_EXEC,
+};
+
static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
@@ -1912,7 +1919,7 @@ static int __init apparmor_init(void)
goto buffers_out;
}
security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
- "apparmor");
+ &apparmor_lsmid);
/* Report that AppArmor successfully initialized */
apparmor_initialized = 1;
diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
index e5971fa74fd7..20983ae8d31f 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -5,6 +5,7 @@
*/
#include <linux/lsm_hooks.h>
#include <linux/bpf_lsm.h>
+#include <uapi/linux/lsm.h>
static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
@@ -15,9 +16,19 @@ static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(task_free, bpf_task_storage_free),
};
+/*
+ * slot has to be LSMBLOB_NEEDED because some of the hooks
+ * supplied by this module require a slot.
+ */
+struct lsm_id bpf_lsmid __lsm_ro_after_init = {
+ .lsm = "bpf",
+ .id = LSM_ID_BPF,
+};
+
static int __init bpf_lsm_init(void)
{
- security_add_hooks(bpf_lsm_hooks, ARRAY_SIZE(bpf_lsm_hooks), "bpf");
+ security_add_hooks(bpf_lsm_hooks, ARRAY_SIZE(bpf_lsm_hooks),
+ &bpf_lsmid);
pr_info("LSM support for eBPF active\n");
return 0;
}
diff --git a/security/commoncap.c b/security/commoncap.c
index 1164278b97fd..76c5a0af95d6 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -25,6 +25,7 @@
#include <linux/binfmts.h>
#include <linux/personality.h>
#include <linux/mnt_idmapping.h>
+#include <uapi/linux/lsm.h>
/*
* If a non-root user executes a setuid-root binary in
@@ -1445,6 +1446,11 @@ int cap_mmap_file(struct file *file, unsigned long reqprot,
#ifdef CONFIG_SECURITY
+static struct lsm_id capability_lsmid __lsm_ro_after_init = {
+ .lsm = "capability",
+ .id = LSM_ID_CAPABILITY,
+};
+
static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(capable, cap_capable),
LSM_HOOK_INIT(settime, cap_settime),
@@ -1469,7 +1475,7 @@ static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
static int __init capability_init(void)
{
security_add_hooks(capability_hooks, ARRAY_SIZE(capability_hooks),
- "capability");
+ &capability_lsmid);
return 0;
}
diff --git a/security/landlock/cred.c b/security/landlock/cred.c
index ec6c37f04a19..2eb1d65f10d6 100644
--- a/security/landlock/cred.c
+++ b/security/landlock/cred.c
@@ -42,5 +42,5 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
__init void landlock_add_cred_hooks(void)
{
security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
- LANDLOCK_NAME);
+ &landlock_lsmid);
}
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index adcea0fe7e68..fa0e6e76991c 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1307,5 +1307,5 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
__init void landlock_add_fs_hooks(void)
{
security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
- LANDLOCK_NAME);
+ &landlock_lsmid);
}
diff --git a/security/landlock/ptrace.c b/security/landlock/ptrace.c
index 4c5b9cd71286..eab35808f395 100644
--- a/security/landlock/ptrace.c
+++ b/security/landlock/ptrace.c
@@ -116,5 +116,5 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
__init void landlock_add_ptrace_hooks(void)
{
security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
- LANDLOCK_NAME);
+ &landlock_lsmid);
}
diff --git a/security/landlock/setup.c b/security/landlock/setup.c
index 3f196d2ce4f9..9104133d04ca 100644
--- a/security/landlock/setup.c
+++ b/security/landlock/setup.c
@@ -8,6 +8,7 @@
#include <linux/init.h>
#include <linux/lsm_hooks.h>
+#include <uapi/linux/lsm.h>
#include "common.h"
#include "cred.h"
@@ -24,6 +25,11 @@ struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
.lbs_superblock = sizeof(struct landlock_superblock_security),
};
+struct lsm_id landlock_lsmid __lsm_ro_after_init = {
+ .lsm = LANDLOCK_NAME,
+ .id = LSM_ID_LANDLOCK,
+};
+
static int __init landlock_init(void)
{
landlock_add_cred_hooks();
diff --git a/security/landlock/setup.h b/security/landlock/setup.h
index 1daffab1ab4b..38bce5b172dc 100644
--- a/security/landlock/setup.h
+++ b/security/landlock/setup.h
@@ -14,5 +14,6 @@
extern bool landlock_initialized;
extern struct lsm_blob_sizes landlock_blob_sizes;
+extern struct lsm_id landlock_lsmid;
#endif /* _SECURITY_LANDLOCK_SETUP_H */
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 110a5ab2b46b..d5c1373a096d 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -20,6 +20,7 @@
#include <linux/string_helpers.h>
#include <linux/dm-verity-loadpin.h>
#include <uapi/linux/loadpin.h>
+#include <uapi/linux/lsm.h>
#define VERITY_DIGEST_FILE_HEADER "# LOADPIN_TRUSTED_VERITY_ROOT_DIGESTS"
@@ -203,6 +204,11 @@ static int loadpin_load_data(enum kernel_load_data_id id, bool contents)
return loadpin_check(NULL, (enum kernel_read_file_id) id);
}
+static struct lsm_id loadpin_lsmid __lsm_ro_after_init = {
+ .lsm = "loadpin",
+ .id = LSM_ID_LOADPIN,
+};
+
static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
@@ -250,7 +256,8 @@ static int __init loadpin_init(void)
pr_info("ready to pin (currently %senforcing)\n",
enforce ? "" : "not ");
parse_exclude();
- security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
+ security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks),
+ &loadpin_lsmid);
return 0;
}
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index a79b985e917e..e8c41a0caf7d 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -13,6 +13,7 @@
#include <linux/security.h>
#include <linux/export.h>
#include <linux/lsm_hooks.h>
+#include <uapi/linux/lsm.h>
static enum lockdown_reason kernel_locked_down;
@@ -75,6 +76,11 @@ static struct security_hook_list lockdown_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(locked_down, lockdown_is_locked_down),
};
+static struct lsm_id lockdown_lsmid __lsm_ro_after_init = {
+ .lsm = "lockdown",
+ .id = LSM_ID_LOCKDOWN,
+};
+
static int __init lockdown_lsm_init(void)
{
#if defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY)
@@ -83,7 +89,7 @@ static int __init lockdown_lsm_init(void)
lock_kernel_down("Kernel configuration", LOCKDOWN_CONFIDENTIALITY_MAX);
#endif
security_add_hooks(lockdown_hooks, ARRAY_SIZE(lockdown_hooks),
- "lockdown");
+ &lockdown_lsmid);
return 0;
}
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index e806739f7868..8d0742ba045d 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -19,6 +19,7 @@
#include <linux/ptrace.h>
#include <linux/sched/task_stack.h>
#include <linux/security.h>
+#include <uapi/linux/lsm.h>
#include "lsm.h"
/* Flag indicating whether initialization completed */
@@ -261,6 +262,11 @@ static int safesetid_task_fix_setgroups(struct cred *new, const struct cred *old
return 0;
}
+static struct lsm_id safesetid_lsmid __lsm_ro_after_init = {
+ .lsm = "safesetid",
+ .id = LSM_ID_SAFESETID,
+};
+
static struct security_hook_list safesetid_security_hooks[] = {
LSM_HOOK_INIT(task_fix_setuid, safesetid_task_fix_setuid),
LSM_HOOK_INIT(task_fix_setgid, safesetid_task_fix_setgid),
@@ -271,7 +277,8 @@ static struct security_hook_list safesetid_security_hooks[] = {
static int __init safesetid_security_init(void)
{
security_add_hooks(safesetid_security_hooks,
- ARRAY_SIZE(safesetid_security_hooks), "safesetid");
+ ARRAY_SIZE(safesetid_security_hooks),
+ &safesetid_lsmid);
/* Report that SafeSetID successfully initialized */
safesetid_initialized = 1;
diff --git a/security/security.c b/security/security.c
index d1571900a8c7..07a8fe7f92bf 100644
--- a/security/security.c
+++ b/security/security.c
@@ -504,17 +504,17 @@ static int lsm_append(const char *new, char **result)
* security_add_hooks - Add a modules hooks to the hook lists.
* @hooks: the hooks to add
* @count: the number of hooks to add
- * @lsm: the name of the security module
+ * @lsmid: the identification information for the security module
*
* Each LSM has to register its hooks with the infrastructure.
*/
void __init security_add_hooks(struct security_hook_list *hooks, int count,
- const char *lsm)
+ struct lsm_id *lsmid)
{
int i;
for (i = 0; i < count; i++) {
- hooks[i].lsm = lsm;
+ hooks[i].lsmid = lsmid;
hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
}
@@ -523,7 +523,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
* and fix this up afterwards.
*/
if (slab_is_available()) {
- if (lsm_append(lsm, &lsm_names) < 0)
+ if (lsm_append(lsmid->lsm, &lsm_names) < 0)
panic("%s - Cannot get early memory.\n", __func__);
}
}
@@ -2145,7 +2145,7 @@ int security_getprocattr(struct task_struct *p, const char *lsm,
struct security_hook_list *hp;
hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsm))
+ if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
continue;
return hp->hook.getprocattr(p, name, value);
}
@@ -2158,7 +2158,7 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
struct security_hook_list *hp;
hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsm))
+ if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
continue;
return hp->hook.setprocattr(name, value, size);
}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3c5be76a9199..7398819a0036 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -92,6 +92,7 @@
#include <linux/fsnotify.h>
#include <linux/fanotify.h>
#include <linux/io_uring.h>
+#include <uapi/linux/lsm.h>
#include "avc.h"
#include "objsec.h"
@@ -7032,6 +7033,13 @@ static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
}
#endif /* CONFIG_IO_URING */
+static struct lsm_id selinux_lsmid __lsm_ro_after_init = {
+ .lsm = "selinux",
+ .id = LSM_ID_SELINUX,
+ .attrs_used = LSM_ATTR_CURRENT | LSM_ATTR_EXEC | LSM_ATTR_FSCREATE |
+ LSM_ATTR_KEYCREATE | LSM_ATTR_PREV | LSM_ATTR_SOCKCREATE,
+};
+
/*
* IMPORTANT NOTE: When adding new hooks, please be careful to keep this order:
* 1. any hooks that don't belong to (2.) or (3.) below,
@@ -7355,7 +7363,8 @@ static __init int selinux_init(void)
hashtab_cache_init();
- security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+ security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks),
+ &selinux_lsmid);
if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
panic("SELinux: Unable to register AVC netcache callback\n");
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 9a82a15685d1..8918b52cff43 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -43,6 +43,7 @@
#include <linux/fs_parser.h>
#include <linux/watch_queue.h>
#include <linux/io_uring.h>
+#include <uapi/linux/lsm.h>
#include "smack.h"
#define TRANS_TRUE "TRUE"
@@ -4856,6 +4857,12 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
.lbs_superblock = sizeof(struct superblock_smack),
};
+static struct lsm_id smack_lsmid __lsm_ro_after_init = {
+ .lsm = "smack",
+ .id = LSM_ID_SMACK,
+ .attrs_used = LSM_ATTR_CURRENT,
+};
+
static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme),
@@ -5062,7 +5069,7 @@ static __init int smack_init(void)
/*
* Register with LSM
*/
- security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+ security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), &smack_lsmid);
smack_enabled = 1;
pr_info("Smack: Initializing.\n");
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index af04a7b7eb28..a4658fb5ef0e 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -6,6 +6,7 @@
*/
#include <linux/lsm_hooks.h>
+#include <uapi/linux/lsm.h>
#include "common.h"
/**
@@ -542,6 +543,11 @@ static void tomoyo_task_free(struct task_struct *task)
}
}
+static struct lsm_id tomoyo_lsmid __lsm_ro_after_init = {
+ .lsm = "tomoyo",
+ .id = LSM_ID_TOMOYO,
+};
+
/*
* tomoyo_security_ops is a "struct security_operations" which is used for
* registering TOMOYO.
@@ -595,7 +601,8 @@ static int __init tomoyo_init(void)
struct tomoyo_task *s = tomoyo_task(current);
/* register ourselves with the security framework */
- security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo");
+ security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks),
+ &tomoyo_lsmid);
pr_info("TOMOYO Linux initialized\n");
s->domain_info = &tomoyo_kernel_domain;
atomic_inc(&tomoyo_kernel_domain.users);
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 06e226166aab..2487b8f847f3 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -18,6 +18,7 @@
#include <linux/task_work.h>
#include <linux/sched.h>
#include <linux/spinlock.h>
+#include <uapi/linux/lsm.h>
#define YAMA_SCOPE_DISABLED 0
#define YAMA_SCOPE_RELATIONAL 1
@@ -421,6 +422,11 @@ static int yama_ptrace_traceme(struct task_struct *parent)
return rc;
}
+static struct lsm_id yama_lsmid __lsm_ro_after_init = {
+ .lsm = "yama",
+ .id = LSM_ID_YAMA,
+};
+
static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
@@ -477,7 +483,7 @@ static inline void yama_init_sysctl(void) { }
static int __init yama_init(void)
{
pr_info("Yama: becoming mindful.\n");
- security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), "yama");
+ security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), &yama_lsmid);
yama_init_sysctl();
return 0;
}
--
2.39.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 2/8] LSM: Maintain a table of LSM attribute data
2023-01-09 18:07 ` [PATCH v5 0/8] LSM: Three basic syscalls Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 1/8] LSM: Identify modules by more than name Casey Schaufler
@ 2023-01-09 18:07 ` Casey Schaufler
2023-01-11 21:01 ` Paul Moore
2023-01-09 18:07 ` [PATCH v5 3/8] proc: Use lsmids instead of lsm names for attrs Casey Schaufler
` (5 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2023-01-09 18:07 UTC (permalink / raw)
To: casey.schaufler, paul, linux-security-module
Cc: casey, jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, mic
As LSMs are registered add their lsm_id pointers to a table.
This will be used later for attribute reporting.
Determine the number of possible security modules based on
their respective CONFIG options. This allows the number to be
known at build time. This allows data structures and tables
to use the constant.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/security.h | 2 ++
security/security.c | 44 +++++++++++++++++++++++++++++++++-------
2 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 5b67f208f7de..33ed1860b96f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -138,6 +138,8 @@ enum lockdown_reason {
};
extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
+extern u32 lsm_active_cnt;
+extern struct lsm_id *lsm_idlist[];
/* These functions are in security/commoncap.c */
extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
diff --git a/security/security.c b/security/security.c
index 07a8fe7f92bf..a590fa98ddd6 100644
--- a/security/security.c
+++ b/security/security.c
@@ -28,12 +28,29 @@
#include <linux/backing-dev.h>
#include <linux/string.h>
#include <linux/msg.h>
+#include <uapi/linux/lsm.h>
#include <net/flow.h>
#define MAX_LSM_EVM_XATTR 2
-/* How many LSMs were built into the kernel? */
-#define LSM_COUNT (__end_lsm_info - __start_lsm_info)
+/*
+ * How many LSMs are built into the kernel as determined at
+ * build time. Used to determine fixed array sizes.
+ * The capability module is accounted for by CONFIG_SECURITY
+ */
+#define LSM_COUNT ( \
+ (IS_ENABLED(CONFIG_SECURITY) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN_LSM) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0))
/*
* These are descriptions of the reasons that can be passed to the
@@ -90,7 +107,7 @@ static __initdata const char *chosen_major_lsm;
static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
/* Ordered list of LSMs to initialize. */
-static __initdata struct lsm_info **ordered_lsms;
+static __initdata struct lsm_info *ordered_lsms[LSM_COUNT + 1];
static __initdata struct lsm_info *exclusive;
static __initdata bool debug;
@@ -341,13 +358,16 @@ static void __init report_lsm_order(void)
pr_cont("\n");
}
+/*
+ * Current index to use while initializing the lsm id list.
+ */
+u32 lsm_active_cnt __lsm_ro_after_init;
+struct lsm_id *lsm_idlist[LSM_COUNT] __lsm_ro_after_init;
+
static void __init ordered_lsm_init(void)
{
struct lsm_info **lsm;
- ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
- GFP_KERNEL);
-
if (chosen_lsm_order) {
if (chosen_major_lsm) {
pr_warn("security=%s is ignored because it is superseded by lsm=%s\n",
@@ -388,7 +408,7 @@ static void __init ordered_lsm_init(void)
for (lsm = ordered_lsms; *lsm; lsm++)
initialize_lsm(*lsm);
- kfree(ordered_lsms);
+ init_debug("lsm count = %d\n", lsm_active_cnt);
}
int __init early_security_init(void)
@@ -513,6 +533,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
{
int i;
+ /*
+ * A security module may call security_add_hooks() more
+ * than once. Landlock is one such case.
+ */
+ if (lsm_active_cnt == 0 || lsm_idlist[lsm_active_cnt - 1] != lsmid)
+ lsm_idlist[lsm_active_cnt++] = lsmid;
+
+ if (lsm_active_cnt > LSM_COUNT)
+ panic("%s Too many LSMs registered.\n", __func__);
+
for (i = 0; i < count; i++) {
hooks[i].lsmid = lsmid;
hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
--
2.39.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 3/8] proc: Use lsmids instead of lsm names for attrs
2023-01-09 18:07 ` [PATCH v5 0/8] LSM: Three basic syscalls Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 1/8] LSM: Identify modules by more than name Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 2/8] LSM: Maintain a table of LSM attribute data Casey Schaufler
@ 2023-01-09 18:07 ` Casey Schaufler
2023-01-11 21:01 ` Paul Moore
2023-01-09 18:07 ` [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes Casey Schaufler
` (4 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2023-01-09 18:07 UTC (permalink / raw)
To: casey.schaufler, paul, linux-security-module
Cc: casey, jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, mic,
linux-fsdevel
Use the LSM ID number instead of the LSM name to identify which
security module's attibute data should be shown in /proc/self/attr.
The security_[gs]etprocattr() functions have been changed to expect
the LSM ID. The change from a string comparison to an integer comparison
in these functions will provide a minor performance improvement.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-fsdevel@vger.kernel.org
---
fs/proc/base.c | 29 +++++++++++++++--------------
fs/proc/internal.h | 2 +-
include/linux/security.h | 11 +++++------
security/security.c | 11 +++++------
4 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 9e479d7d202b..9328b6b07dfc 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -96,6 +96,7 @@
#include <linux/time_namespace.h>
#include <linux/resctrl.h>
#include <linux/cn_proc.h>
+#include <uapi/linux/lsm.h>
#include <trace/events/oom.h>
#include "internal.h"
#include "fd.h"
@@ -145,10 +146,10 @@ struct pid_entry {
NOD(NAME, (S_IFREG|(MODE)), \
NULL, &proc_single_file_operations, \
{ .proc_show = show } )
-#define ATTR(LSM, NAME, MODE) \
+#define ATTR(LSMID, NAME, MODE) \
NOD(NAME, (S_IFREG|(MODE)), \
NULL, &proc_pid_attr_operations, \
- { .lsm = LSM })
+ { .lsmid = LSMID })
/*
* Count the number of hardlinks for the pid_entry table, excluding the .
@@ -2730,7 +2731,7 @@ static ssize_t proc_pid_attr_read(struct file * file, char __user * buf,
if (!task)
return -ESRCH;
- length = security_getprocattr(task, PROC_I(inode)->op.lsm,
+ length = security_getprocattr(task, PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name,
&p);
put_task_struct(task);
@@ -2788,7 +2789,7 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
if (rv < 0)
goto out_free;
- rv = security_setprocattr(PROC_I(inode)->op.lsm,
+ rv = security_setprocattr(PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name, page,
count);
mutex_unlock(¤t->signal->cred_guard_mutex);
@@ -2837,27 +2838,27 @@ static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \
#ifdef CONFIG_SECURITY_SMACK
static const struct pid_entry smack_attr_dir_stuff[] = {
- ATTR("smack", "current", 0666),
+ ATTR(LSM_ID_SMACK, "current", 0666),
};
LSM_DIR_OPS(smack);
#endif
#ifdef CONFIG_SECURITY_APPARMOR
static const struct pid_entry apparmor_attr_dir_stuff[] = {
- ATTR("apparmor", "current", 0666),
- ATTR("apparmor", "prev", 0444),
- ATTR("apparmor", "exec", 0666),
+ ATTR(LSM_ID_APPARMOR, "current", 0666),
+ ATTR(LSM_ID_APPARMOR, "prev", 0444),
+ ATTR(LSM_ID_APPARMOR, "exec", 0666),
};
LSM_DIR_OPS(apparmor);
#endif
static const struct pid_entry attr_dir_stuff[] = {
- ATTR(NULL, "current", 0666),
- ATTR(NULL, "prev", 0444),
- ATTR(NULL, "exec", 0666),
- ATTR(NULL, "fscreate", 0666),
- ATTR(NULL, "keycreate", 0666),
- ATTR(NULL, "sockcreate", 0666),
+ ATTR(0, "current", 0666),
+ ATTR(0, "prev", 0444),
+ ATTR(0, "exec", 0666),
+ ATTR(0, "fscreate", 0666),
+ ATTR(0, "keycreate", 0666),
+ ATTR(0, "sockcreate", 0666),
#ifdef CONFIG_SECURITY_SMACK
DIR("smack", 0555,
proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index b701d0207edf..18db9722c81b 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -92,7 +92,7 @@ union proc_op {
int (*proc_show)(struct seq_file *m,
struct pid_namespace *ns, struct pid *pid,
struct task_struct *task);
- const char *lsm;
+ int lsmid;
};
struct proc_inode {
diff --git a/include/linux/security.h b/include/linux/security.h
index 33ed1860b96f..2d09e818a7d1 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -475,10 +475,9 @@ int security_sem_semctl(struct kern_ipc_perm *sma, int cmd);
int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
unsigned nsops, int alter);
void security_d_instantiate(struct dentry *dentry, struct inode *inode);
-int security_getprocattr(struct task_struct *p, const char *lsm, const char *name,
+int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
char **value);
-int security_setprocattr(const char *lsm, const char *name, void *value,
- size_t size);
+int security_setprocattr(int lsmid, const char *name, void *value, size_t size);
int security_netlink_send(struct sock *sk, struct sk_buff *skb);
int security_ismaclabel(const char *name);
int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
@@ -1346,14 +1345,14 @@ static inline void security_d_instantiate(struct dentry *dentry,
struct inode *inode)
{ }
-static inline int security_getprocattr(struct task_struct *p, const char *lsm,
+static inline int security_getprocattr(struct task_struct *p, int lsmid,
const char *name, char **value)
{
return -EINVAL;
}
-static inline int security_setprocattr(const char *lsm, char *name,
- void *value, size_t size)
+static inline int security_setprocattr(int lsmid, char *name, void *value,
+ size_t size)
{
return -EINVAL;
}
diff --git a/security/security.c b/security/security.c
index a590fa98ddd6..a0f4af2da5f3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2169,26 +2169,25 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode)
}
EXPORT_SYMBOL(security_d_instantiate);
-int security_getprocattr(struct task_struct *p, const char *lsm,
- const char *name, char **value)
+int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
+ char **value)
{
struct security_hook_list *hp;
hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
+ if (lsmid != 0 && lsmid != hp->lsmid->id)
continue;
return hp->hook.getprocattr(p, name, value);
}
return LSM_RET_DEFAULT(getprocattr);
}
-int security_setprocattr(const char *lsm, const char *name, void *value,
- size_t size)
+int security_setprocattr(int lsmid, const char *name, void *value, size_t size)
{
struct security_hook_list *hp;
hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
+ if (lsmid != 0 && lsmid != hp->lsmid->id)
continue;
return hp->hook.setprocattr(name, value, size);
}
--
2.39.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-01-09 18:07 ` [PATCH v5 0/8] LSM: Three basic syscalls Casey Schaufler
` (2 preceding siblings ...)
2023-01-09 18:07 ` [PATCH v5 3/8] proc: Use lsmids instead of lsm names for attrs Casey Schaufler
@ 2023-01-09 18:07 ` Casey Schaufler
2023-01-11 21:07 ` Paul Moore
` (3 more replies)
2023-01-09 18:07 ` [PATCH v5 5/8] LSM: Create lsm_module_list system call Casey Schaufler
` (3 subsequent siblings)
7 siblings, 4 replies; 30+ messages in thread
From: Casey Schaufler @ 2023-01-09 18:07 UTC (permalink / raw)
To: casey.schaufler, paul, linux-security-module
Cc: casey, jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, mic
Create a system call lsm_get_self_attr() to provide the security
module maintained attributes of the current process. Historically
these attributes have been exposed to user space via entries in
procfs under /proc/self/attr.
Attributes are provided as a collection of lsm_ctx structures
which are placed into a user supplied buffer. Each structure
identifys the size of the attribute, and the attribute value.
The format of the attribute value is defined by the security
module, but will always be \0 terminated. The ctx_len value
will always be strlen(ctx)+1.
---------------------------
| __u32 id |
---------------------------
| __u64 flags |
---------------------------
| __kernel_size_t ctx_len |
---------------------------
| __u8 ctx[ctx_len] |
---------------------------
| __u32 id |
---------------------------
| __u64 flags |
---------------------------
| __kernel_size_t ctx_len |
---------------------------
| __u8 ctx[ctx_len] |
---------------------------
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
Documentation/userspace-api/lsm.rst | 9 ++
include/linux/syscalls.h | 3 +
include/uapi/linux/lsm.h | 21 ++++
kernel/sys_ni.c | 3 +
security/Makefile | 1 +
security/lsm_syscalls.c | 182 ++++++++++++++++++++++++++++
6 files changed, 219 insertions(+)
create mode 100644 security/lsm_syscalls.c
diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst
index 6ddf5506110b..98a0c191b499 100644
--- a/Documentation/userspace-api/lsm.rst
+++ b/Documentation/userspace-api/lsm.rst
@@ -48,6 +48,15 @@ creating socket objects.
The proc filesystem provides this value in ``/proc/self/attr/sockcreate``.
This is supported by the SELinux security module.
+Kernel interface
+================
+
+Get the security attributes of the current process
+--------------------------------------------------
+
+.. kernel-doc:: security/lsm_syscalls.c
+ :identifiers: sys_lsm_get_self_attr
+
Additional documentation
========================
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 33a0ee3bcb2e..a89205c70ffa 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -71,6 +71,7 @@ struct clone_args;
struct open_how;
struct mount_attr;
struct landlock_ruleset_attr;
+struct lsm_ctx;
enum landlock_rule_type;
#include <linux/types.h>
@@ -1058,6 +1059,8 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
unsigned long home_node,
unsigned long flags);
+asmlinkage long sys_lsm_get_self_attr(struct lsm_ctx *ctx, size_t *size,
+ int flags);
/*
* Architecture-specific system calls
diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
index 61a91b7d946f..8674d8c6b326 100644
--- a/include/uapi/linux/lsm.h
+++ b/include/uapi/linux/lsm.h
@@ -9,6 +9,27 @@
#ifndef _UAPI_LINUX_LSM_H
#define _UAPI_LINUX_LSM_H
+#include <linux/types.h>
+#include <linux/unistd.h>
+
+/**
+ * struct lsm_ctx - LSM context
+ * @id: the LSM id number, see LSM_ID_XXX
+ * @flags: context specifier and LSM specific flags
+ * @ctx_len: the size of @ctx
+ * @ctx: the LSM context, a nul terminated string
+ *
+ * @ctx in a nul terminated string.
+ * (strlen(@ctx) < @ctx_len) is always true.
+ * (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
+ */
+struct lsm_ctx {
+ __u32 id;
+ __u64 flags;
+ __kernel_size_t ctx_len;
+ __u8 ctx[];
+};
+
/*
* ID values to identify security modules.
* A system may use more than one security module.
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 860b2dcf3ac4..7b2513d5605d 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -262,6 +262,9 @@ COND_SYSCALL_COMPAT(recvmsg);
/* mm/nommu.c, also with MMU */
COND_SYSCALL(mremap);
+/* security/lsm_syscalls.c */
+COND_SYSCALL(lsm_get_self_attr);
+
/* security/keys/keyctl.c */
COND_SYSCALL(add_key);
COND_SYSCALL(request_key);
diff --git a/security/Makefile b/security/Makefile
index 18121f8f85cd..59f238490665 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_KEYS) += keys/
# always enable default capabilities
obj-y += commoncap.o
+obj-$(CONFIG_SECURITY) += lsm_syscalls.o
obj-$(CONFIG_MMU) += min_addr.o
# Object file lists
diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
new file mode 100644
index 000000000000..55e8bf61ac8a
--- /dev/null
+++ b/security/lsm_syscalls.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * System calls implementing the Linux Security Module API.
+ *
+ * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
+ * Copyright (C) 2022 Intel Corporation
+ */
+
+#include <asm/current.h>
+#include <linux/compiler_types.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/security.h>
+#include <linux/stddef.h>
+#include <linux/syscalls.h>
+#include <linux/types.h>
+#include <linux/lsm_hooks.h>
+#include <uapi/linux/lsm.h>
+
+struct attrs_used_map {
+ char *name;
+ int attrs_used;
+};
+
+static const struct attrs_used_map lsm_attr_names[] = {
+ { .name = "current", .attrs_used = LSM_ATTR_CURRENT, },
+ { .name = "exec", .attrs_used = LSM_ATTR_EXEC, },
+ { .name = "fscreate", .attrs_used = LSM_ATTR_FSCREATE, },
+ { .name = "keycreate", .attrs_used = LSM_ATTR_KEYCREATE, },
+ { .name = "prev", .attrs_used = LSM_ATTR_PREV, },
+ { .name = "sockcreate", .attrs_used = LSM_ATTR_SOCKCREATE, },
+};
+
+static int attr_used_index(u32 flags)
+{
+ int i;
+
+ if (flags == 0)
+ return -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(lsm_attr_names); i++)
+ if ((lsm_attr_names[i].attrs_used & flags) == flags)
+ return i;
+
+ return -EINVAL;
+}
+
+/**
+ * sys_lsm_get_self_attr - Return current task's security module attributes
+ * @ctx: the LSM contexts
+ * @size: size of @ctx, updated on return
+ * @flags: which attribute to return
+ *
+ * Returns the calling task's LSM contexts. On success this
+ * function returns the number of @ctx array elements. This value
+ * may be zero if there are no LSM contexts assigned. If @size is
+ * insufficient to contain the return data -E2BIG is returned and
+ * @size is set to the minimum required size. In all other cases
+ * a negative value indicating the error is returned.
+ */
+SYSCALL_DEFINE3(lsm_get_self_attr,
+ struct lsm_ctx __user *, ctx,
+ size_t __user *, size,
+ u32, flags)
+{
+ int i;
+ int rc = 0;
+ int len;
+ int attr;
+ int count = 0;
+ void *curr;
+ char *cp;
+ char *np;
+ char **interum_ctx;
+ size_t total_size = 0;
+ struct lsm_ctx *ip;
+ struct lsm_ctx *interum;
+ struct lsm_ctx *final = NULL;
+
+ attr = attr_used_index(flags);
+ if (attr < 0)
+ return attr;
+
+ interum = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
+ sizeof(*interum), GFP_KERNEL);
+ if (interum == NULL)
+ return -ENOMEM;
+ ip = interum;
+
+ interum_ctx = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
+ sizeof(*interum_ctx), GFP_KERNEL);
+ if (interum_ctx == NULL) {
+ kfree(interum);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < lsm_active_cnt; i++) {
+ if ((lsm_idlist[i]->attrs_used &
+ lsm_attr_names[attr].attrs_used) == 0)
+ continue;
+
+ len = security_getprocattr(current, lsm_idlist[i]->id,
+ lsm_attr_names[attr].name,
+ &cp);
+ if (len <= 0)
+ continue;
+
+ ip->id = lsm_idlist[i]->id;
+ ip->flags = lsm_attr_names[attr].attrs_used;
+ interum_ctx[count] = cp;
+
+ /*
+ * A security module that returns a binary attribute
+ * will need to identify itself to prevent string
+ * processing.
+ *
+ * At least one security module adds a \n at the
+ * end of a context to make it look nicer. Change
+ * that to a \0 so that user space doesn't have to
+ * work around it.
+ *
+ * Security modules have been inconsistent about
+ * including the \0 terminator in the size. If it's
+ * not there make space for it.
+ *
+ * The length returned will reflect the length of
+ * the string provided by the security module, which
+ * may not match what getprocattr returned.
+ */
+ np = strnchr(cp, len, '\n');
+ if (np != NULL)
+ *np = '\0';
+ ip->ctx_len = strnlen(cp, len) + 1;
+ total_size += sizeof(*interum) + ip->ctx_len;
+ ip++;
+ count++;
+ }
+
+ if (count == 0)
+ goto free_out;
+
+ final = kzalloc(total_size, GFP_KERNEL);
+ if (final == NULL) {
+ rc = -ENOMEM;
+ goto free_out;
+ }
+
+ curr = final;
+ ip = interum;
+ for (i = 0; i < count; i++) {
+ memcpy(curr, ip, sizeof(*interum));
+ curr += sizeof(*interum);
+ if (ip->ctx_len > 1)
+ memcpy(curr, interum_ctx[i], ip->ctx_len - 1);
+ curr += ip->ctx_len;
+ ip++;
+ }
+
+ if (get_user(len, size)) {
+ rc = -EFAULT;
+ goto free_out;
+ }
+ if (total_size > len) {
+ rc = -ERANGE;
+ if (put_user(total_size, size) != 0)
+ rc = -EFAULT;
+ goto free_out;
+ }
+ if (copy_to_user(ctx, final, total_size) != 0 ||
+ put_user(total_size, size) != 0)
+ rc = -EFAULT;
+ else
+ rc = count;
+
+free_out:
+ for (i = 0; i < count; i++)
+ kfree(interum_ctx[i]);
+ kfree(interum_ctx);
+ kfree(interum);
+ kfree(final);
+ return rc;
+}
--
2.39.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 5/8] LSM: Create lsm_module_list system call
2023-01-09 18:07 ` [PATCH v5 0/8] LSM: Three basic syscalls Casey Schaufler
` (3 preceding siblings ...)
2023-01-09 18:07 ` [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes Casey Schaufler
@ 2023-01-09 18:07 ` Casey Schaufler
2023-01-11 21:07 ` Paul Moore
2023-01-09 18:07 ` [PATCH v5 6/8] LSM: lsm_set_self_attr syscall for LSM self attributes Casey Schaufler
` (2 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2023-01-09 18:07 UTC (permalink / raw)
To: casey.schaufler, paul, linux-security-module
Cc: casey, jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, mic
Create a system call to report the list of Linux Security Modules
that are active on the system. The list is provided as an array
of LSM ID numbers.
The calling application can use this list determine what LSM
specific actions it might take. That might include chosing an
output format, determining required privilege or bypassing
security module specific behavior.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
Documentation/userspace-api/lsm.rst | 3 +++
include/linux/syscalls.h | 1 +
kernel/sys_ni.c | 1 +
security/lsm_syscalls.c | 41 +++++++++++++++++++++++++++++
4 files changed, 46 insertions(+)
diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst
index 98a0c191b499..e342d75b99ab 100644
--- a/Documentation/userspace-api/lsm.rst
+++ b/Documentation/userspace-api/lsm.rst
@@ -57,6 +57,9 @@ Get the security attributes of the current process
.. kernel-doc:: security/lsm_syscalls.c
:identifiers: sys_lsm_get_self_attr
+.. kernel-doc:: security/lsm_syscalls.c
+ :identifiers: sys_lsm_module_list
+
Additional documentation
========================
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index a89205c70ffa..9eb4cb6bbeb1 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1061,6 +1061,7 @@ asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long l
unsigned long flags);
asmlinkage long sys_lsm_get_self_attr(struct lsm_ctx *ctx, size_t *size,
int flags);
+asmlinkage long sys_lsm_module_list(u32 *ids, size_t *size, int flags);
/*
* Architecture-specific system calls
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 7b2513d5605d..af1fd28c0420 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -264,6 +264,7 @@ COND_SYSCALL(mremap);
/* security/lsm_syscalls.c */
COND_SYSCALL(lsm_get_self_attr);
+COND_SYSCALL(lsm_module_list);
/* security/keys/keyctl.c */
COND_SYSCALL(add_key);
diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
index 55e8bf61ac8a..92af1fcaa654 100644
--- a/security/lsm_syscalls.c
+++ b/security/lsm_syscalls.c
@@ -180,3 +180,44 @@ SYSCALL_DEFINE3(lsm_get_self_attr,
kfree(final);
return rc;
}
+
+/**
+ * sys_lsm_module_list - Return a list of the active security modules
+ * @ids: the LSM module ids
+ * @size: size of @ids, updated on return
+ * @flags: reserved for future use, must be zero
+ *
+ * Returns a list of the active LSM ids. On success this function
+ * returns the number of @ids array elements. This value may be zero
+ * if there are no LSMs active. If @size is insufficient to contain
+ * the return data -E2BIG is returned and @size is set to the minimum
+ * required size. In all other cases a negative value indicating the
+ * error is returned.
+ */
+SYSCALL_DEFINE3(lsm_module_list,
+ u32 __user *, ids,
+ size_t __user *, size,
+ u64, flags)
+{
+ size_t total_size = lsm_active_cnt * sizeof(*ids);
+ size_t usize;
+ int i;
+
+ if (flags)
+ return -EINVAL;
+
+ if (get_user(usize, size))
+ return -EFAULT;
+
+ if (put_user(total_size, size) != 0)
+ return -EFAULT;
+
+ if (usize < total_size)
+ return -E2BIG;
+
+ for (i = 0; i < lsm_active_cnt; i++)
+ if (put_user(lsm_idlist[i]->id, ids++))
+ return -EFAULT;
+
+ return lsm_active_cnt;
+}
--
2.39.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 6/8] LSM: lsm_set_self_attr syscall for LSM self attributes
2023-01-09 18:07 ` [PATCH v5 0/8] LSM: Three basic syscalls Casey Schaufler
` (4 preceding siblings ...)
2023-01-09 18:07 ` [PATCH v5 5/8] LSM: Create lsm_module_list system call Casey Schaufler
@ 2023-01-09 18:07 ` Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 7/8] LSM: wireup Linux Security Module syscalls Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 8/8] LSM: selftests for " Casey Schaufler
7 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2023-01-09 18:07 UTC (permalink / raw)
To: casey.schaufler, paul, linux-security-module
Cc: casey, jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, mic
Create a system call lsm_set_self_attr() to set a security
module maintained attribute of the current process. Historically
these attributes have been exposed to user space via entries in
procfs under /proc/self/attr.
The attribute value is provided in a lsm_ctx structure. The structure
identifys the size of the attribute, and the attribute value. The format
of the attribute value is defined by the security module, but will always
be \0 terminated if it is a string. The ctx_len value must always be
strlen(ctx)+1 if the value is a string. The flags field is reserved for
future security module specific use and must be 0.
---------------------------
| __u32 id |
---------------------------
| __u64 flags |
---------------------------
| __kernel_size_t ctx_len |
---------------------------
| __u8 ctx[ctx_len] |
---------------------------
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
Documentation/userspace-api/lsm.rst | 3 +++
include/linux/syscalls.h | 2 ++
kernel/sys_ni.c | 1 +
security/lsm_syscalls.c | 41 +++++++++++++++++++++++++++++
4 files changed, 47 insertions(+)
diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst
index e342d75b99ab..c7da13801305 100644
--- a/Documentation/userspace-api/lsm.rst
+++ b/Documentation/userspace-api/lsm.rst
@@ -57,6 +57,9 @@ Get the security attributes of the current process
.. kernel-doc:: security/lsm_syscalls.c
:identifiers: sys_lsm_get_self_attr
+.. kernel-doc:: security/lsm_syscalls.c
+ :identifiers: sys_lsm_set_self_attr
+
.. kernel-doc:: security/lsm_syscalls.c
:identifiers: sys_lsm_module_list
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 9eb4cb6bbeb1..a9f1ec9942af 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1062,6 +1062,8 @@ asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long l
asmlinkage long sys_lsm_get_self_attr(struct lsm_ctx *ctx, size_t *size,
int flags);
asmlinkage long sys_lsm_module_list(u32 *ids, size_t *size, int flags);
+asmlinkage long sys_lsm_set_self_attr(struct lsm_ctx *ctx, size_t size,
+ int flags);
/*
* Architecture-specific system calls
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index af1fd28c0420..c3884c1c7339 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -265,6 +265,7 @@ COND_SYSCALL(mremap);
/* security/lsm_syscalls.c */
COND_SYSCALL(lsm_get_self_attr);
COND_SYSCALL(lsm_module_list);
+COND_SYSCALL(lsm_set_self_attr);
/* security/keys/keyctl.c */
COND_SYSCALL(add_key);
diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
index 92af1fcaa654..026a86674a1f 100644
--- a/security/lsm_syscalls.c
+++ b/security/lsm_syscalls.c
@@ -181,6 +181,47 @@ SYSCALL_DEFINE3(lsm_get_self_attr,
return rc;
}
+/**
+ * sys_lsm_set_self_attr - Set current task's security module attribute
+ * @ctx: the LSM contexts
+ * @size: size of @ctx
+ * @flags: which attribute to set
+ *
+ * Sets the calling task's LSM context. On success this function
+ * returns 0. If the attribute specified cannot be set a negative
+ * value indicating the reason for the error is returned.
+ */
+SYSCALL_DEFINE3(lsm_set_self_attr,
+ struct lsm_ctx __user *, ctx,
+ size_t, size,
+ u32, flags)
+{
+ int rc = -EINVAL;
+ int attr;
+ void *page;
+ struct lsm_ctx *ip;
+
+ if (size > PAGE_SIZE)
+ return -E2BIG;
+ if (size <= sizeof(*ip))
+ return -EINVAL;
+
+ attr = attr_used_index(flags);
+ if (attr < 0)
+ return attr;
+
+ page = memdup_user(ctx, size);
+ if (IS_ERR(page))
+ return PTR_ERR(page);
+
+ ip = page;
+ if (sizeof(*ip) + ip->ctx_len <= size)
+ rc = security_setprocattr(ip->id, lsm_attr_names[attr].name,
+ ip->ctx, ip->ctx_len);
+ kfree(page);
+ return (rc > 0) ? 0 : rc;
+}
+
/**
* sys_lsm_module_list - Return a list of the active security modules
* @ids: the LSM module ids
--
2.39.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 7/8] LSM: wireup Linux Security Module syscalls
2023-01-09 18:07 ` [PATCH v5 0/8] LSM: Three basic syscalls Casey Schaufler
` (5 preceding siblings ...)
2023-01-09 18:07 ` [PATCH v5 6/8] LSM: lsm_set_self_attr syscall for LSM self attributes Casey Schaufler
@ 2023-01-09 18:07 ` Casey Schaufler
2023-01-13 9:31 ` Geert Uytterhoeven
2023-01-09 18:07 ` [PATCH v5 8/8] LSM: selftests for " Casey Schaufler
7 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2023-01-09 18:07 UTC (permalink / raw)
To: casey.schaufler, paul, linux-security-module
Cc: casey, jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, mic
Wireup lsm_get_self_attr, lsm_set_self_attr and lsm_module_list
system calls.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-api@vger.kernel.org
---
arch/alpha/kernel/syscalls/syscall.tbl | 3 +++
arch/arm/tools/syscall.tbl | 3 +++
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 6 ++++++
arch/ia64/kernel/syscalls/syscall.tbl | 3 +++
arch/m68k/kernel/syscalls/syscall.tbl | 3 +++
arch/microblaze/kernel/syscalls/syscall.tbl | 3 +++
arch/mips/kernel/syscalls/syscall_n32.tbl | 3 +++
arch/mips/kernel/syscalls/syscall_n64.tbl | 3 +++
arch/mips/kernel/syscalls/syscall_o32.tbl | 3 +++
arch/parisc/kernel/syscalls/syscall.tbl | 3 +++
arch/powerpc/kernel/syscalls/syscall.tbl | 3 +++
arch/s390/kernel/syscalls/syscall.tbl | 3 +++
arch/sh/kernel/syscalls/syscall.tbl | 3 +++
arch/sparc/kernel/syscalls/syscall.tbl | 3 +++
arch/x86/entry/syscalls/syscall_32.tbl | 3 +++
arch/x86/entry/syscalls/syscall_64.tbl | 3 +++
arch/xtensa/kernel/syscalls/syscall.tbl | 3 +++
include/uapi/asm-generic/unistd.h | 11 ++++++++++-
tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl | 3 +++
tools/perf/arch/powerpc/entry/syscalls/syscall.tbl | 3 +++
tools/perf/arch/s390/entry/syscalls/syscall.tbl | 3 +++
tools/perf/arch/x86/entry/syscalls/syscall_64.tbl | 3 +++
23 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 8ebacf37a8cf..002e6a39fcb1 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -490,3 +490,6 @@
558 common process_mrelease sys_process_mrelease
559 common futex_waitv sys_futex_waitv
560 common set_mempolicy_home_node sys_ni_syscall
+561 common lsm_get_self_attr sys_lsm_get_self_attr
+562 common lsm_module_list sys_lsm_module_list
+563 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index ac964612d8b0..dca80a2d3927 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -464,3 +464,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
index 037feba03a51..6a28fb91b85d 100644
--- a/arch/arm64/include/asm/unistd.h
+++ b/arch/arm64/include/asm/unistd.h
@@ -39,7 +39,7 @@
#define __ARM_NR_compat_set_tls (__ARM_NR_COMPAT_BASE + 5)
#define __ARM_NR_COMPAT_END (__ARM_NR_COMPAT_BASE + 0x800)
-#define __NR_compat_syscalls 451
+#define __NR_compat_syscalls 454
#endif
#define __ARCH_WANT_SYS_CLONE
diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index 604a2053d006..cb4b3149024d 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -907,6 +907,12 @@ __SYSCALL(__NR_process_mrelease, sys_process_mrelease)
__SYSCALL(__NR_futex_waitv, sys_futex_waitv)
#define __NR_set_mempolicy_home_node 450
__SYSCALL(__NR_set_mempolicy_home_node, sys_set_mempolicy_home_node)
+#define __NR_lsm_get_self_attr 451
+__SYSCALL(__NR_lsm_get_self_attr, sys_lsm_get_self_attr)
+#define __NR_lsm_module_list 452
+__SYSCALL(__NR_lsm_module_list, sys_module_list)
+#define __NR_lsm_set_self_attr 453
+__SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
/*
* Please add new compat syscalls above this comment and update
diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
index 72c929d9902b..1a5d560a1317 100644
--- a/arch/ia64/kernel/syscalls/syscall.tbl
+++ b/arch/ia64/kernel/syscalls/syscall.tbl
@@ -371,3 +371,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index b1f3940bc298..0b7b01c90315 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -450,3 +450,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 820145e47350..b69d57014c7b 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -456,3 +456,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 253ff994ed2e..7c1ca6241b90 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -389,3 +389,6 @@
448 n32 process_mrelease sys_process_mrelease
449 n32 futex_waitv sys_futex_waitv
450 n32 set_mempolicy_home_node sys_set_mempolicy_home_node
+451 n32 lsm_get_self_attr sys_lsm_get_self_attr
+452 n32 lsm_module_list sys_lsm_module_list
+453 n32 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index 3f1886ad9d80..99453966d179 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -365,3 +365,6 @@
448 n64 process_mrelease sys_process_mrelease
449 n64 futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 n64 lsm_get_self_attr sys_lsm_get_self_attr
+452 n64 lsm_module_list sys_lsm_module_list
+453 n64 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 8f243e35a7b2..4ddb0ff66793 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -438,3 +438,6 @@
448 o32 process_mrelease sys_process_mrelease
449 o32 futex_waitv sys_futex_waitv
450 o32 set_mempolicy_home_node sys_set_mempolicy_home_node
+451 o32 lsm_get_self_attr sys_lsm_get_self_attr
+452 o32 lsm_module_list sys_lsm_module_list
+453 032 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index 0e42fceb2d5e..5ab1a5b22d8e 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -448,3 +448,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index a0be127475b1..8d31bb83d6a2 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -537,3 +537,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 nospu set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 799147658dee..bb7597be2e4f 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -453,3 +453,6 @@
448 common process_mrelease sys_process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index 2de85c977f54..43d468742916 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -453,3 +453,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index 4398cc6fb68d..c7791c7bdde4 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -496,3 +496,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 320480a8db4f..4f2e6577466e 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -455,3 +455,6 @@
448 i386 process_mrelease sys_process_mrelease
449 i386 futex_waitv sys_futex_waitv
450 i386 set_mempolicy_home_node sys_set_mempolicy_home_node
+451 i386 lsm_get_self_attr sys_lsm_get_self_attr
+452 i386 lsm_module_list sys_lsm_module_list
+453 i386 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index c84d12608cd2..3a7866f72042 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -372,6 +372,9 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
#
# Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 52c94ab5c205..e0a5b61c1f1a 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -421,3 +421,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 45fa180cc56a..3659b2b02f5a 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -886,8 +886,17 @@ __SYSCALL(__NR_futex_waitv, sys_futex_waitv)
#define __NR_set_mempolicy_home_node 450
__SYSCALL(__NR_set_mempolicy_home_node, sys_set_mempolicy_home_node)
+#define __NR_lsm_get_self_attr 451
+__SYSCALL(__NR_lsm_get_self_attr, sys_lsm_get_self_attr)
+
+#define __NR_lsm_module_list 452
+__SYSCALL(__NR_lsm_module_list, sys_lsm_module_list)
+
+#define __NR_lsm_set_self_attr 453
+__SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
+
#undef __NR_syscalls
-#define __NR_syscalls 451
+#define __NR_syscalls 454
/*
* 32 bit systems traditionally used different
diff --git a/tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl b/tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl
index 3f1886ad9d80..99453966d179 100644
--- a/tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl
+++ b/tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl
@@ -365,3 +365,6 @@
448 n64 process_mrelease sys_process_mrelease
449 n64 futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 n64 lsm_get_self_attr sys_lsm_get_self_attr
+452 n64 lsm_module_list sys_lsm_module_list
+453 n64 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
index a0be127475b1..8d31bb83d6a2 100644
--- a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
+++ b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
@@ -537,3 +537,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 nospu set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/tools/perf/arch/s390/entry/syscalls/syscall.tbl b/tools/perf/arch/s390/entry/syscalls/syscall.tbl
index 799147658dee..d69bd5550b46 100644
--- a/tools/perf/arch/s390/entry/syscalls/syscall.tbl
+++ b/tools/perf/arch/s390/entry/syscalls/syscall.tbl
@@ -453,3 +453,6 @@
448 common process_mrelease sys_process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl b/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
index c84d12608cd2..3a7866f72042 100644
--- a/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
@@ -372,6 +372,9 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_module_list sys_lsm_module_list
+453 common lsm_set_self_attr sys_lsm_set_self_attr
#
# Due to a historical design error, certain syscalls are numbered differently
--
2.39.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 8/8] LSM: selftests for Linux Security Module syscalls
2023-01-09 18:07 ` [PATCH v5 0/8] LSM: Three basic syscalls Casey Schaufler
` (6 preceding siblings ...)
2023-01-09 18:07 ` [PATCH v5 7/8] LSM: wireup Linux Security Module syscalls Casey Schaufler
@ 2023-01-09 18:07 ` Casey Schaufler
7 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2023-01-09 18:07 UTC (permalink / raw)
To: casey.schaufler, paul, linux-security-module
Cc: casey, jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, mic
Add selftests for the three system calls supporting the LSM
infrastructure.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/lsm/Makefile | 12 +
tools/testing/selftests/lsm/config | 2 +
.../selftests/lsm/lsm_get_self_attr_test.c | 268 ++++++++++++++
.../selftests/lsm/lsm_module_list_test.c | 149 ++++++++
.../selftests/lsm/lsm_set_self_attr_test.c | 328 ++++++++++++++++++
6 files changed, 760 insertions(+)
create mode 100644 tools/testing/selftests/lsm/Makefile
create mode 100644 tools/testing/selftests/lsm/config
create mode 100644 tools/testing/selftests/lsm/lsm_get_self_attr_test.c
create mode 100644 tools/testing/selftests/lsm/lsm_module_list_test.c
create mode 100644 tools/testing/selftests/lsm/lsm_set_self_attr_test.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 41b649452560..ea58c5018529 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -37,6 +37,7 @@ TARGETS += landlock
TARGETS += lib
TARGETS += livepatch
TARGETS += lkdtm
+TARGETS += lsm
TARGETS += membarrier
TARGETS += memfd
TARGETS += memory-hotplug
diff --git a/tools/testing/selftests/lsm/Makefile b/tools/testing/selftests/lsm/Makefile
new file mode 100644
index 000000000000..d567ea9756ea
--- /dev/null
+++ b/tools/testing/selftests/lsm/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# First run: make -C ../../../.. headers_install
+
+CFLAGS += -Wall -O2 $(KHDR_INCLUDES)
+
+TEST_GEN_PROGS := lsm_get_self_attr_test lsm_module_list_test \
+ lsm_set_self_attr_test
+
+include ../lib.mk
+
+$(TEST_GEN_PROGS):
diff --git a/tools/testing/selftests/lsm/config b/tools/testing/selftests/lsm/config
new file mode 100644
index 000000000000..afb887715f64
--- /dev/null
+++ b/tools/testing/selftests/lsm/config
@@ -0,0 +1,2 @@
+CONFIG_SYSFS=y
+CONFIG_SECURITY=y
diff --git a/tools/testing/selftests/lsm/lsm_get_self_attr_test.c b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c
new file mode 100644
index 000000000000..6f7f72c25cda
--- /dev/null
+++ b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Linux Security Module infrastructure tests
+ * Tests for the lsm_get_self_attr system call
+ *
+ * Copyright © 2022 Casey Schaufler <casey@schaufler-ca.com>
+ * Copyright © 2022 Intel Corporation
+ */
+
+#define _GNU_SOURCE
+#include <linux/lsm.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include "../kselftest_harness.h"
+
+#define PROCATTR "/proc/self/attr/"
+
+static int read_proc_attr(const char *attr, char *value, __kernel_size_t size)
+{
+ FILE *fp;
+ int len;
+ char *path;
+
+ len = strlen(PROCATTR) + strlen(attr) + 1;
+ path = calloc(len, 1);
+ if (path == NULL)
+ return -1;
+ sprintf(path, "%s%s", PROCATTR, attr);
+
+ fp = fopen(path, "r");
+ free(path);
+
+ if (fp == NULL)
+ return -1;
+ if (fread(value, 1, size, fp) <= 0)
+ return -1;
+ fclose(fp);
+
+ path = strchr(value, '\n');
+ if (path)
+ *path = '\0';
+
+ return 0;
+}
+
+static struct lsm_ctx *next_ctx(struct lsm_ctx *ctxp)
+{
+ void *vp;
+
+ vp = (void *)ctxp + sizeof(*ctxp) + ctxp->ctx_len;
+ return (struct lsm_ctx *)vp;
+}
+
+TEST(size_null_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, ctx, NULL,
+ LSM_ATTR_CURRENT));
+ ASSERT_EQ(EFAULT, errno);
+
+ free(ctx);
+}
+
+TEST(ctx_null_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ __kernel_size_t size = page_size;
+
+ ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, NULL, &size,
+ LSM_ATTR_CURRENT));
+ ASSERT_EQ(EFAULT, errno);
+ ASSERT_NE(1, size);
+}
+
+TEST(size_too_small_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = 1;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_CURRENT));
+ ASSERT_EQ(ERANGE, errno);
+ ASSERT_NE(1, size);
+
+ free(ctx);
+}
+
+TEST(flags_zero_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, ctx, &size, 0));
+ ASSERT_EQ(EINVAL, errno);
+ ASSERT_EQ(page_size, size);
+
+ free(ctx);
+}
+
+TEST(flags_overset_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_CURRENT | LSM_ATTR_PREV));
+ ASSERT_EQ(EINVAL, errno);
+ ASSERT_EQ(page_size, size);
+
+ free(ctx);
+}
+
+TEST(basic_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ __kernel_size_t size = page_size;
+ struct lsm_ctx *ctx = calloc(page_size, 1);
+ struct lsm_ctx *tctx = NULL;
+ __u32 *syscall_lsms = calloc(page_size, 1);
+ char *attr = calloc(page_size, 1);
+ int cnt_current = 0;
+ int cnt_exec = 0;
+ int cnt_fscreate = 0;
+ int cnt_keycreate = 0;
+ int cnt_prev = 0;
+ int cnt_sockcreate = 0;
+ int lsmcount;
+ int count;
+ int i;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_NE(NULL, syscall_lsms);
+
+ lsmcount = syscall(__NR_lsm_module_list, syscall_lsms, &size, 0);
+ ASSERT_LE(1, lsmcount);
+
+ for (i = 0; i < lsmcount; i++) {
+ switch (syscall_lsms[i]) {
+ case LSM_ID_SELINUX:
+ cnt_current++;
+ cnt_exec++;
+ cnt_fscreate++;
+ cnt_keycreate++;
+ cnt_prev++;
+ cnt_sockcreate++;
+ break;
+ case LSM_ID_SMACK:
+ cnt_current++;
+ break;
+ case LSM_ID_APPARMOR:
+ cnt_current++;
+ cnt_exec++;
+ cnt_prev++;
+ break;
+ default:
+ break;
+ }
+ }
+
+ if (cnt_current) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_CURRENT);
+ ASSERT_EQ(cnt_current, count);
+ tctx = ctx;
+ ASSERT_EQ(0, read_proc_attr("current", attr, page_size));
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ if (cnt_exec) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_EXEC);
+ ASSERT_GE(cnt_exec, count);
+ if (count > 0) {
+ tctx = ctx;
+ ASSERT_EQ(0, read_proc_attr("exec", attr, page_size));
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ }
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ if (cnt_fscreate) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_FSCREATE);
+ ASSERT_GE(cnt_fscreate, count);
+ if (count > 0) {
+ tctx = ctx;
+ ASSERT_EQ(0, read_proc_attr("fscreate", attr,
+ page_size));
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ }
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ if (cnt_keycreate) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_KEYCREATE);
+ ASSERT_GE(cnt_keycreate, count);
+ if (count > 0) {
+ tctx = ctx;
+ ASSERT_EQ(0, read_proc_attr("keycreate", attr,
+ page_size));
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ }
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ if (cnt_prev) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_PREV);
+ ASSERT_GE(cnt_prev, count);
+ if (count > 0) {
+ tctx = ctx;
+ ASSERT_EQ(0, read_proc_attr("prev", attr, page_size));
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ }
+ if (cnt_sockcreate) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_SOCKCREATE);
+ ASSERT_GE(cnt_sockcreate, count);
+ if (count > 0) {
+ tctx = ctx;
+ ASSERT_EQ(0, read_proc_attr("sockcreate", attr,
+ page_size));
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ }
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+
+ free(ctx);
+ free(attr);
+ free(syscall_lsms);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/lsm/lsm_module_list_test.c b/tools/testing/selftests/lsm/lsm_module_list_test.c
new file mode 100644
index 000000000000..c5675598b2a4
--- /dev/null
+++ b/tools/testing/selftests/lsm/lsm_module_list_test.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Linux Security Module infrastructure tests
+ * Tests for the lsm_module_list system call
+ *
+ * Copyright © 2022 Casey Schaufler <casey@schaufler-ca.com>
+ * Copyright © 2022 Intel Corporation
+ */
+
+#define _GNU_SOURCE
+#include <linux/lsm.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include "../kselftest_harness.h"
+
+static int read_sysfs_lsms(char *lsms, __kernel_size_t size)
+{
+ FILE *fp;
+
+ fp = fopen("/sys/kernel/security/lsm", "r");
+ if (fp == NULL)
+ return -1;
+ if (fread(lsms, 1, size, fp) <= 0)
+ return -1;
+ fclose(fp);
+ return 0;
+}
+
+TEST(size_null_lsm_module_list)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *syscall_lsms = calloc(page_size, 1);
+
+ ASSERT_NE(NULL, syscall_lsms);
+ ASSERT_EQ(-1, syscall(__NR_lsm_module_list, syscall_lsms, NULL, 0));
+ ASSERT_EQ(EFAULT, errno);
+
+ free(syscall_lsms);
+}
+
+TEST(ids_null_lsm_module_list)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ __kernel_size_t size = page_size;
+
+ ASSERT_EQ(-1, syscall(__NR_lsm_module_list, NULL, &size, 0));
+ ASSERT_EQ(EFAULT, errno);
+ ASSERT_NE(1, size);
+}
+
+TEST(size_too_small_lsm_module_list)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *syscall_lsms = calloc(page_size, 1);
+ __kernel_size_t size = 1;
+
+ ASSERT_NE(NULL, syscall_lsms);
+ ASSERT_EQ(-1, syscall(__NR_lsm_module_list, syscall_lsms, &size, 0));
+ ASSERT_EQ(E2BIG, errno);
+ ASSERT_NE(1, size);
+
+ free(syscall_lsms);
+}
+
+TEST(flags_set_lsm_module_list)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *syscall_lsms = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, syscall_lsms);
+ ASSERT_EQ(-1, syscall(__NR_lsm_module_list, syscall_lsms, &size, 7));
+ ASSERT_EQ(EINVAL, errno);
+ ASSERT_EQ(page_size, size);
+
+ free(syscall_lsms);
+}
+
+TEST(correct_lsm_module_list)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ __kernel_size_t size = page_size;
+ __u32 *syscall_lsms = calloc(page_size, 1);
+ char *sysfs_lsms = calloc(page_size, 1);
+ char *name;
+ char *cp;
+ int count;
+ int i;
+
+ ASSERT_NE(NULL, sysfs_lsms);
+ ASSERT_NE(NULL, syscall_lsms);
+ ASSERT_EQ(0, read_sysfs_lsms(sysfs_lsms, page_size));
+
+ count = syscall(__NR_lsm_module_list, syscall_lsms, &size, 0);
+ ASSERT_LE(1, count);
+ cp = sysfs_lsms;
+ for (i = 0; i < count; i++) {
+ switch (syscall_lsms[i]) {
+ case LSM_ID_CAPABILITY:
+ name = "capability";
+ break;
+ case LSM_ID_SELINUX:
+ name = "selinux";
+ break;
+ case LSM_ID_SMACK:
+ name = "smack";
+ break;
+ case LSM_ID_TOMOYO:
+ name = "tomoyo";
+ break;
+ case LSM_ID_IMA:
+ name = "ima";
+ break;
+ case LSM_ID_APPARMOR:
+ name = "apparmor";
+ break;
+ case LSM_ID_YAMA:
+ name = "yama";
+ break;
+ case LSM_ID_LOADPIN:
+ name = "loadpin";
+ break;
+ case LSM_ID_SAFESETID:
+ name = "safesetid";
+ break;
+ case LSM_ID_LOCKDOWN:
+ name = "lockdown";
+ break;
+ case LSM_ID_BPF:
+ name = "bpf";
+ break;
+ case LSM_ID_LANDLOCK:
+ name = "landlock";
+ break;
+ default:
+ name = "INVALID";
+ break;
+ }
+ ASSERT_EQ(0, strncmp(cp, name, strlen(name)));
+ cp += strlen(name) + 1;
+ }
+
+ free(sysfs_lsms);
+ free(syscall_lsms);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
new file mode 100644
index 000000000000..86f8a5952471
--- /dev/null
+++ b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
@@ -0,0 +1,328 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Linux Security Module infrastructure tests
+ * Tests for the lsm_set_self_attr system call
+ *
+ * Copyright © 2022 Casey Schaufler <casey@schaufler-ca.com>
+ * Copyright © 2022 Intel Corporation
+ */
+
+#define _GNU_SOURCE
+#include <linux/lsm.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include "../kselftest_harness.h"
+
+static struct lsm_ctx *next_ctx(struct lsm_ctx *tctx)
+{
+ void *vp;
+
+ vp = (void *)tctx + sizeof(*tctx) + tctx->ctx_len;
+ return (struct lsm_ctx *)vp;
+}
+
+TEST(ctx_null_lsm_set_self_attr)
+{
+ ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, NULL, _SC_PAGESIZE,
+ LSM_ATTR_CURRENT));
+ ASSERT_EQ(EFAULT, errno);
+}
+
+TEST(size_too_small_lsm_set_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ struct lsm_ctx *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_CURRENT));
+ ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, ctx, 1,
+ LSM_ATTR_CURRENT));
+ ASSERT_EQ(EINVAL, errno);
+
+ free(ctx);
+}
+
+TEST(flags_zero_lsm_set_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_CURRENT));
+ ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, ctx, size, 0));
+ ASSERT_EQ(EINVAL, errno);
+
+ free(ctx);
+}
+
+TEST(flags_overset_lsm_set_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+ struct lsm_ctx *tctx = (struct lsm_ctx *)ctx;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, tctx, &size,
+ LSM_ATTR_CURRENT));
+ ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_CURRENT | LSM_ATTR_PREV));
+ ASSERT_EQ(EINVAL, errno);
+
+ free(ctx);
+}
+
+TEST(basic_lsm_set_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ __kernel_size_t size = page_size;
+ struct lsm_ctx *ctx = calloc(page_size, 1);
+ struct lsm_ctx *tctx;
+ __u32 *syscall_lsms = calloc(page_size, 1);
+ char *attr = calloc(page_size, 1);
+ bool active_apparmor = false;
+ bool active_selinux = false;
+ bool active_smack = false;
+ int cnt_current = 0;
+ int cnt_exec = 0;
+ int cnt_fscreate = 0;
+ int cnt_keycreate = 0;
+ int cnt_prev = 0;
+ int cnt_sockcreate = 0;
+ int lsmcount;
+ int count;
+ int rc;
+ int i;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_NE(NULL, syscall_lsms);
+
+ lsmcount = syscall(__NR_lsm_module_list, syscall_lsms, &size, 0);
+ ASSERT_LE(1, lsmcount);
+
+ for (i = 0; i < lsmcount; i++) {
+ switch (syscall_lsms[i]) {
+ case LSM_ID_SELINUX:
+ active_selinux = true;
+ cnt_current++;
+ cnt_exec++;
+ cnt_fscreate++;
+ cnt_keycreate++;
+ cnt_prev++;
+ cnt_sockcreate++;
+ break;
+ case LSM_ID_SMACK:
+ active_smack = true;
+ cnt_current++;
+ break;
+ case LSM_ID_APPARMOR:
+ active_apparmor = true;
+ cnt_current++;
+ cnt_exec++;
+ cnt_prev++;
+ break;
+ default:
+ break;
+ }
+ }
+
+ if (cnt_current) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_CURRENT);
+ ASSERT_EQ(cnt_current, count);
+ tctx = ctx;
+
+ for (i = 0; i < count; i++) {
+ switch (tctx->id) {
+ case LSM_ID_SELINUX:
+ ASSERT_EQ(active_selinux, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_CURRENT);
+ ASSERT_EQ(0, rc);
+ tctx->ctx[0] = 'X';
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_CURRENT);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ break;
+ case LSM_ID_SMACK:
+ ASSERT_EQ(active_smack, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_CURRENT);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EPERM, errno);
+ break;
+ case LSM_ID_APPARMOR:
+ ASSERT_EQ(active_apparmor, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_CURRENT);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ break;
+ default:
+ }
+ tctx = next_ctx(tctx);
+ }
+ }
+ if (cnt_exec) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_EXEC);
+ ASSERT_GE(cnt_exec, count);
+ tctx = ctx;
+
+ for (i = 0; i < count; i++) {
+ switch (tctx->id) {
+ case LSM_ID_SELINUX:
+ ASSERT_EQ(active_selinux, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_EXEC);
+ ASSERT_EQ(0, rc);
+ tctx->ctx[0] = 'X';
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_EXEC);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ break;
+ case LSM_ID_APPARMOR:
+ ASSERT_EQ(active_apparmor, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_EXEC);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EPERM, errno);
+ break;
+ default:
+ break;
+ }
+ tctx = next_ctx(tctx);
+ }
+ }
+ if (cnt_prev) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_PREV);
+ ASSERT_GE(cnt_prev, count);
+ tctx = ctx;
+
+ for (i = 0; i < count; i++) {
+ switch (tctx->id) {
+ case LSM_ID_SELINUX:
+ ASSERT_EQ(active_selinux, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_PREV);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ tctx->ctx[0] = 'X';
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_PREV);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ break;
+ case LSM_ID_APPARMOR:
+ ASSERT_EQ(active_apparmor, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_PREV);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EPERM, errno);
+ break;
+ default:
+ break;
+ }
+ tctx = next_ctx(tctx);
+ }
+ }
+ if (cnt_fscreate) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_FSCREATE);
+ ASSERT_GE(cnt_fscreate, count);
+ tctx = ctx;
+
+ for (i = 0; i < count; i++) {
+ switch (tctx->id) {
+ case LSM_ID_SELINUX:
+ ASSERT_EQ(active_selinux, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_FSCREATE);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ tctx->ctx[0] = 'X';
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_FSCREATE);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ break;
+ default:
+ break;
+ }
+ tctx = next_ctx(tctx);
+ }
+ }
+ if (cnt_keycreate) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_KEYCREATE);
+ ASSERT_GE(cnt_keycreate, count);
+ tctx = ctx;
+
+ for (i = 0; i < count; i++) {
+ switch (tctx->id) {
+ case LSM_ID_SELINUX:
+ ASSERT_EQ(active_selinux, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_KEYCREATE);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ tctx->ctx[0] = 'X';
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_KEYCREATE);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ break;
+ default:
+ break;
+ }
+ tctx = next_ctx(tctx);
+ }
+ }
+ if (cnt_sockcreate) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, ctx, &size,
+ LSM_ATTR_SOCKCREATE);
+ ASSERT_GE(cnt_sockcreate, count);
+ tctx = ctx;
+
+ for (i = 0; i < count; i++) {
+ switch (tctx->id) {
+ case LSM_ID_SELINUX:
+ ASSERT_EQ(active_selinux, true);
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_SOCKCREATE);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ tctx->ctx[0] = 'X';
+ rc = syscall(__NR_lsm_set_self_attr, tctx, size,
+ LSM_ATTR_SOCKCREATE);
+ ASSERT_EQ(-1, rc);
+ ASSERT_EQ(EINVAL, errno);
+ break;
+ default:
+ break;
+ }
+ tctx = next_ctx(tctx);
+ }
+ }
+
+ free(ctx);
+ free(attr);
+ free(syscall_lsms);
+}
+
+TEST_HARNESS_MAIN
--
2.39.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v5 1/8] LSM: Identify modules by more than name
2023-01-09 18:07 ` [PATCH v5 1/8] LSM: Identify modules by more than name Casey Schaufler
@ 2023-01-11 21:00 ` Paul Moore
2023-01-12 0:05 ` Casey Schaufler
0 siblings, 1 reply; 30+ messages in thread
From: Paul Moore @ 2023-01-11 21:00 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Mon, Jan 9, 2023 at 1:07 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Create a struct lsm_id to contain identifying information
> about Linux Security Modules (LSMs). At inception this contains
> the name of the module, an identifier associated with the security
> module and an integer member "attrs_used" which identifies the API
> related data associated with each security module. The initial set
> of features maps to information that has traditionaly been available
> in /proc/self/attr. They are documented in a new userspace-api file.
> Change the security_add_hooks() interface to use this structure.
> Change the individual modules to maintain their own struct lsm_id
> and pass it to security_add_hooks().
>
> The values are for LSM identifiers are defined in a new UAPI
> header file linux/lsm.h. Each existing LSM has been updated to
> include it's LSMID in the lsm_id.
>
> The LSM ID values are sequential, with the oldest module
> LSM_ID_CAPABILITY being the lowest value and the existing modules
> numbered in the order they were included in the main line kernel.
> This is an arbitrary convention for assigning the values, but
> none better presents itself. The value 0 is defined as being invalid.
> The values 1-99 are reserved for any special case uses which may
> arise in the future. This may include attributes of the LSM
> infrastructure itself, possibly related to namespacing or network
> attribute management. A special range is identified for such attributes
> to help reduce confusion for developers unfamiliar with LSMs.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> Documentation/userspace-api/index.rst | 1 +
> Documentation/userspace-api/lsm.rst | 55 +++++++++++++++++++++++++++
> include/linux/lsm_hooks.h | 18 ++++++++-
> include/uapi/linux/lsm.h | 55 +++++++++++++++++++++++++++
> security/apparmor/lsm.c | 9 ++++-
> security/bpf/hooks.c | 13 ++++++-
> security/commoncap.c | 8 +++-
> security/landlock/cred.c | 2 +-
> security/landlock/fs.c | 2 +-
> security/landlock/ptrace.c | 2 +-
> security/landlock/setup.c | 6 +++
> security/landlock/setup.h | 1 +
> security/loadpin/loadpin.c | 9 ++++-
> security/lockdown/lockdown.c | 8 +++-
> security/safesetid/lsm.c | 9 ++++-
> security/security.c | 12 +++---
> security/selinux/hooks.c | 11 +++++-
> security/smack/smack_lsm.c | 9 ++++-
> security/tomoyo/tomoyo.c | 9 ++++-
> security/yama/yama_lsm.c | 8 +++-
> 20 files changed, 226 insertions(+), 21 deletions(-)
> create mode 100644 Documentation/userspace-api/lsm.rst
> create mode 100644 include/uapi/linux/lsm.h
Mostly just nitpicky stuff below ...
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 0a5ba81f7367..6f2cabb79ec4 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1665,6 +1665,20 @@ struct security_hook_heads {
> #undef LSM_HOOK
> } __randomize_layout;
>
> +/**
> + * struct lsm_id - identify a Linux Security Module.
According to the kernel-doc documentation it looks like "identify"
should be capitalized.
* https://docs.kernel.org/doc-guide/kernel-doc.html
> + * @lsm: Name of the LSM. Must be approved by the LSM maintainers.
> + * @id: LSM ID number from uapi/linux/lsm.h
> + * @attrs_used: Which attributes this LSM supports.
In a bit of a reversal to the above comment, it appears that the
parameter descriptions should not start with a capital and should not
end with punctuation:
* @lsm: name of the LSM, must be approved by the LSM maintainers
> + * Contains the information that identifies the LSM.
> + */
> +struct lsm_id {
> + const u8 *lsm;
> + u32 id;
> + u64 attrs_used;
> +};
> @@ -1708,7 +1722,7 @@ extern struct security_hook_heads security_hook_heads;
> extern char *lsm_names;
>
> extern void security_add_hooks(struct security_hook_list *hooks, int count,
> - const char *lsm);
> + struct lsm_id *lsmid);
We should be able to mark @lsmid as a const, right?
> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> new file mode 100644
> index 000000000000..61a91b7d946f
> --- /dev/null
> +++ b/include/uapi/linux/lsm.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Linux Security Modules (LSM) - User space API
> + *
> + * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
> + * Copyright (C) 2022 Intel Corporation
> + */
This file should be added to the SECURITY SUBSYSTEM section in MAINTAINERS:
F: include/uapi/linux/lsm.h
> +#ifndef _UAPI_LINUX_LSM_H
> +#define _UAPI_LINUX_LSM_H
> +
> +/*
> + * ID values to identify security modules.
> + * A system may use more than one security module.
> + *
> + * A value of 0 is considered invalid.
> + * Values 1-99 are reserved for future use.
> + * The interface is designed to extend to attributes beyond those which
> + * are active today. Currently all the attributes are specific to the
> + * individual modules. The LSM infrastructure itself has no variable state,
> + * but that may change. One proposal would allow loadable modules, in which
> + * case an attribute such as LSM_IS_LOADABLE might identify the dynamic
> + * modules. Another potential attribute could be which security modules is
> + * associated withnetwork labeling using netlabel. Another possible attribute
> + * could be related to stacking behavior in a namespaced environment.
> + * While it would be possible to intermingle the LSM infrastructure attribute
> + * values with the security module provided values, keeping them separate
> + * provides a clearer distinction.
> + */
As this is in a UAPI file, let's avoid speculation and stick to just
the current facts. Anything we write here with respect to the future
is likely to be wrong so let's not tempt fate.
Once I reached patch 3/8 I also realized that we may want to have more
than just 0/invalid as a sentinel value, or at the very least we need
to redefine 0 as something slightly different if for no other reason
than we in fs/proc/base.c. I know it seems a little trivial, but
since we're talking about values that will be used in the UAPI I think
we need to give it some thought and discussion. The only think I can
think of right now is to redefine 0 as "undefined", which isn't that
far removed from "invalid" and will not look too terrible in patch 3/8
- thoughts?
With all that in mind, I would suggest something like this:
/*
* ID tokens to identify Linux Security Modules (LSMs)
*
* These token values are used to uniquely identify specific LSMs
* in the kernel as well in the kernel's LSM userspace API.
*
* A value of zero/0 is considered undefined and should not be used
* outside of the kernel, values 1-99 are reserved for potential
* future use.
*/
#define LSM_ID_UNDEF 0
> +#define LSM_ID_CAPABILITY 100
> +#define LSM_ID_SELINUX 101
> +#define LSM_ID_SMACK 102
> +#define LSM_ID_TOMOYO 103
> +#define LSM_ID_IMA 104
> +#define LSM_ID_APPARMOR 105
> +#define LSM_ID_YAMA 106
> +#define LSM_ID_LOADPIN 107
> +#define LSM_ID_SAFESETID 108
> +#define LSM_ID_LOCKDOWN 109
> +#define LSM_ID_BPF 110
> +#define LSM_ID_LANDLOCK 111
> +
> +/*
> + * LSM_ATTR_XXX values identify the /proc/.../attr entry that the
> + * context represents. Not all security modules provide all of these
> + * values. Some security modules provide none of them.
> + */
I'd rather see text closer to this:
/*
* LSM attributes
*
* The LSM_ATTR_XXX definitions identify different LSM
* attributes which are used in the kernel's LSM userspace API.
* Support for these attributes vary across the different LSMs,
* none are required.
*/
> +#define LSM_ATTR_CURRENT 0x0001
> +#define LSM_ATTR_EXEC 0x0002
> +#define LSM_ATTR_FSCREATE 0x0004
> +#define LSM_ATTR_KEYCREATE 0x0008
> +#define LSM_ATTR_PREV 0x0010
> +#define LSM_ATTR_SOCKCREATE 0x0020
> +
> +#endif /* _UAPI_LINUX_LSM_H */
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index c6728a629437..63ea2a995987 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -24,6 +24,7 @@
> #include <linux/zstd.h>
> #include <net/sock.h>
> #include <uapi/linux/mount.h>
> +#include <uapi/linux/lsm.h>
>
> #include "include/apparmor.h"
> #include "include/apparmorfs.h"
> @@ -1217,6 +1218,12 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
> .lbs_task = sizeof(struct aa_task_ctx),
> };
>
> +static struct lsm_id apparmor_lsmid __lsm_ro_after_init = {
> + .lsm = "apparmor",
> + .id = LSM_ID_APPARMOR,
> + .attrs_used = LSM_ATTR_CURRENT | LSM_ATTR_PREV | LSM_ATTR_EXEC,
> +};
Perhaps mark this as const in addition to ro_after_init? This applies
to all the other @lsm_id defs in this patch too.
> diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
> index e5971fa74fd7..20983ae8d31f 100644
> --- a/security/bpf/hooks.c
> +++ b/security/bpf/hooks.c
> @@ -5,6 +5,7 @@
> */
> #include <linux/lsm_hooks.h>
> #include <linux/bpf_lsm.h>
> +#include <uapi/linux/lsm.h>
>
> static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
> #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> @@ -15,9 +16,19 @@ static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(task_free, bpf_task_storage_free),
> };
>
> +/*
> + * slot has to be LSMBLOB_NEEDED because some of the hooks
> + * supplied by this module require a slot.
> + */
I don't think we need the above comment here, right?
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/8] LSM: Maintain a table of LSM attribute data
2023-01-09 18:07 ` [PATCH v5 2/8] LSM: Maintain a table of LSM attribute data Casey Schaufler
@ 2023-01-11 21:01 ` Paul Moore
2023-01-12 0:36 ` Casey Schaufler
0 siblings, 1 reply; 30+ messages in thread
From: Paul Moore @ 2023-01-11 21:01 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Mon, Jan 9, 2023 at 1:07 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> As LSMs are registered add their lsm_id pointers to a table.
> This will be used later for attribute reporting.
>
> Determine the number of possible security modules based on
> their respective CONFIG options. This allows the number to be
> known at build time. This allows data structures and tables
> to use the constant.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/security.h | 2 ++
> security/security.c | 44 +++++++++++++++++++++++++++++++++-------
> 2 files changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 5b67f208f7de..33ed1860b96f 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -138,6 +138,8 @@ enum lockdown_reason {
> };
>
> extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
> +extern u32 lsm_active_cnt;
> +extern struct lsm_id *lsm_idlist[];
>
> /* These functions are in security/commoncap.c */
> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> diff --git a/security/security.c b/security/security.c
> index 07a8fe7f92bf..a590fa98ddd6 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -28,12 +28,29 @@
> #include <linux/backing-dev.h>
> #include <linux/string.h>
> #include <linux/msg.h>
> +#include <uapi/linux/lsm.h>
> #include <net/flow.h>
>
> #define MAX_LSM_EVM_XATTR 2
>
> -/* How many LSMs were built into the kernel? */
> -#define LSM_COUNT (__end_lsm_info - __start_lsm_info)
> +/*
> + * How many LSMs are built into the kernel as determined at
> + * build time. Used to determine fixed array sizes.
> + * The capability module is accounted for by CONFIG_SECURITY
> + */
> +#define LSM_COUNT ( \
> + (IS_ENABLED(CONFIG_SECURITY) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN_LSM) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \
> + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0))
>
> /*
> * These are descriptions of the reasons that can be passed to the
> @@ -90,7 +107,7 @@ static __initdata const char *chosen_major_lsm;
> static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
>
> /* Ordered list of LSMs to initialize. */
> -static __initdata struct lsm_info **ordered_lsms;
> +static __initdata struct lsm_info *ordered_lsms[LSM_COUNT + 1];
I'm guessing this 'LSM_COUNT + 1' logic is basically just copied from
ordered_lsm_init() - which is okay - but can you remind me why it is
'LSM_COUNT + 1' and not just 'LSM_COUNT'? Based on the LSM_COUNT
macro above it seems like LSM_COUNT should be enough, no?
> static __initdata struct lsm_info *exclusive;
>
> static __initdata bool debug;
> @@ -341,13 +358,16 @@ static void __init report_lsm_order(void)
> pr_cont("\n");
> }
>
> +/*
> + * Current index to use while initializing the lsm id list.
> + */
> +u32 lsm_active_cnt __lsm_ro_after_init;
> +struct lsm_id *lsm_idlist[LSM_COUNT] __lsm_ro_after_init;
> +
> static void __init ordered_lsm_init(void)
> {
> struct lsm_info **lsm;
>
> - ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
> - GFP_KERNEL);
> -
> if (chosen_lsm_order) {
> if (chosen_major_lsm) {
> pr_warn("security=%s is ignored because it is superseded by lsm=%s\n",
> @@ -388,7 +408,7 @@ static void __init ordered_lsm_init(void)
> for (lsm = ordered_lsms; *lsm; lsm++)
> initialize_lsm(*lsm);
>
> - kfree(ordered_lsms);
> + init_debug("lsm count = %d\n", lsm_active_cnt);
> }
Given 86ef3c735ec8 ("LSM: Better reporting of actual LSMs at boot"),
is this needed?
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 3/8] proc: Use lsmids instead of lsm names for attrs
2023-01-09 18:07 ` [PATCH v5 3/8] proc: Use lsmids instead of lsm names for attrs Casey Schaufler
@ 2023-01-11 21:01 ` Paul Moore
2023-01-12 0:37 ` Casey Schaufler
0 siblings, 1 reply; 30+ messages in thread
From: Paul Moore @ 2023-01-11 21:01 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic, linux-fsdevel
On Mon, Jan 9, 2023 at 1:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Use the LSM ID number instead of the LSM name to identify which
> security module's attibute data should be shown in /proc/self/attr.
> The security_[gs]etprocattr() functions have been changed to expect
> the LSM ID. The change from a string comparison to an integer comparison
> in these functions will provide a minor performance improvement.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> Cc: linux-fsdevel@vger.kernel.org
> ---
> fs/proc/base.c | 29 +++++++++++++++--------------
> fs/proc/internal.h | 2 +-
> include/linux/security.h | 11 +++++------
> security/security.c | 11 +++++------
> 4 files changed, 26 insertions(+), 27 deletions(-)
...
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 9e479d7d202b..9328b6b07dfc 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2837,27 +2838,27 @@ static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \
>
> #ifdef CONFIG_SECURITY_SMACK
> static const struct pid_entry smack_attr_dir_stuff[] = {
> - ATTR("smack", "current", 0666),
> + ATTR(LSM_ID_SMACK, "current", 0666),
> };
> LSM_DIR_OPS(smack);
> #endif
>
> #ifdef CONFIG_SECURITY_APPARMOR
> static const struct pid_entry apparmor_attr_dir_stuff[] = {
> - ATTR("apparmor", "current", 0666),
> - ATTR("apparmor", "prev", 0444),
> - ATTR("apparmor", "exec", 0666),
> + ATTR(LSM_ID_APPARMOR, "current", 0666),
> + ATTR(LSM_ID_APPARMOR, "prev", 0444),
> + ATTR(LSM_ID_APPARMOR, "exec", 0666),
> };
> LSM_DIR_OPS(apparmor);
> #endif
>
> static const struct pid_entry attr_dir_stuff[] = {
> - ATTR(NULL, "current", 0666),
> - ATTR(NULL, "prev", 0444),
> - ATTR(NULL, "exec", 0666),
> - ATTR(NULL, "fscreate", 0666),
> - ATTR(NULL, "keycreate", 0666),
> - ATTR(NULL, "sockcreate", 0666),
> + ATTR(0, "current", 0666),
> + ATTR(0, "prev", 0444),
> + ATTR(0, "exec", 0666),
> + ATTR(0, "fscreate", 0666),
> + ATTR(0, "keycreate", 0666),
> + ATTR(0, "sockcreate", 0666),
See the discussion in patch 1/8, we should use a macro instead of a 0
here (although the exact macro definition is very much up for
discussion):
ATTR(LSM_ID_UNDEF, "current", 0666),
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-01-09 18:07 ` [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes Casey Schaufler
@ 2023-01-11 21:07 ` Paul Moore
2023-01-12 1:37 ` Casey Schaufler
2023-01-12 14:40 ` Arnd Bergmann
` (2 subsequent siblings)
3 siblings, 1 reply; 30+ messages in thread
From: Paul Moore @ 2023-01-11 21:07 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Mon, Jan 9, 2023 at 1:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Create a system call lsm_get_self_attr() to provide the security
> module maintained attributes of the current process. Historically
> these attributes have been exposed to user space via entries in
> procfs under /proc/self/attr.
>
> Attributes are provided as a collection of lsm_ctx structures
> which are placed into a user supplied buffer. Each structure
> identifys the size of the attribute, and the attribute value.
^^^
identifies
> The format of the attribute value is defined by the security
> module, but will always be \0 terminated. The ctx_len value
> will always be strlen(ctx)+1.
I don't want to limit ourselves to only sending string values as
attributes as who knows what we might need to do in the future, and
the struct was originally designed to support both strings and binary
data. I would suggest changing the sentences above to something like
this:
The format of the attribute value is defined by the individual LSM,
with the attribute itself stored in @ctx and the length of the
attribute stored in @ctx_len. Both strings and arbitrary binary
attributes are supported, but strings should be NULL terminated and
@ctx_len should be equal to `strlen(@ctx) + 1`.
> ---------------------------
> | __u32 id |
> ---------------------------
> | __u64 flags |
> ---------------------------
> | __kernel_size_t ctx_len |
> ---------------------------
> | __u8 ctx[ctx_len] |
> ---------------------------
> | __u32 id |
> ---------------------------
> | __u64 flags |
> ---------------------------
> | __kernel_size_t ctx_len |
> ---------------------------
> | __u8 ctx[ctx_len] |
> ---------------------------
Don't repeat the structure layout in memory twice here, it's
confusing. I also think it would be easier to read, and arguably more
useful, to simply copy the struct definition into the description
instead of the ASCII art column.
Although, this has got me wondering if we should think about aligning
the lsm_ctx structs when we are populating them in the kernel; more on
this below ...
> ---
> Documentation/userspace-api/lsm.rst | 9 ++
> include/linux/syscalls.h | 3 +
> include/uapi/linux/lsm.h | 21 ++++
> kernel/sys_ni.c | 3 +
> security/Makefile | 1 +
> security/lsm_syscalls.c | 182 ++++++++++++++++++++++++++++
> 6 files changed, 219 insertions(+)
> create mode 100644 security/lsm_syscalls.c
...
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 33a0ee3bcb2e..a89205c70ffa 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -71,6 +71,7 @@ struct clone_args;
> struct open_how;
> struct mount_attr;
> struct landlock_ruleset_attr;
> +struct lsm_ctx;
> enum landlock_rule_type;
>
> #include <linux/types.h>
> @@ -1058,6 +1059,8 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
> asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
> unsigned long home_node,
> unsigned long flags);
> +asmlinkage long sys_lsm_get_self_attr(struct lsm_ctx *ctx, size_t *size,
> + int flags);
>
> /*
> * Architecture-specific system calls
> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> index 61a91b7d946f..8674d8c6b326 100644
> --- a/include/uapi/linux/lsm.h
> +++ b/include/uapi/linux/lsm.h
> @@ -9,6 +9,27 @@
> #ifndef _UAPI_LINUX_LSM_H
> #define _UAPI_LINUX_LSM_H
>
> +#include <linux/types.h>
> +#include <linux/unistd.h>
> +
> +/**
> + * struct lsm_ctx - LSM context
> + * @id: the LSM id number, see LSM_ID_XXX
As mentioned above, it occurred to me that we might want want to pad
out the lsm_ctx struct to ensure that the "array" of lsm_ctx is nicely
aligned. I know some systems used to complain about unaligned
accesses, and even on those that don't complain tend to be faster when
access are aligned. We can either implicitly align the individual
lsm_ctx structs or we can add a total length field (in addition to the
@ctx_len field) so that the padding/alignment is explicit.
Adding an explicit total length field could have some other advantages
in that it, in conjunction with the existing @flags field, would allow
an individual LSM to "extend" the lsm_ctx struct to provide additional
LSM specific information in the case where the single @ctx field was
not sufficient. Think of it as some additional future proofing in
addition to explicit padding.
> + * @flags: context specifier and LSM specific flags
* @flags: LSM specific flags
Only the individual LSM specified in @id should ever interpret @flags or @ctx.
> + * @ctx_len: the size of @ctx
> + * @ctx: the LSM context, a nul terminated string
* @ctx: the LSM context value
> + * @ctx in a nul terminated string.
> + * (strlen(@ctx) < @ctx_len) is always true.
> + * (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
> + */
Let's rework the extra description too based on the comments above.
For the sake of clarity, here is what I'm currently thinking (comments
and feedback are encouraged):
/**
* struct lsm_ctx - LSM context information
* @id: the LSM ID token, see LSM_ID_XXX
* @flags: LSM specific flags
* @len: length of the lsm_ctx struct + extra (?) + padding
* @ctx_len: the size of @ctx
* @ctx: the LSM context value
*
* The @len field MUST be equal to size of the lsm_ctx struct
* plus any additional padding and/or data placed after @ctx.
*
* In all cases @ctx_len MUST be equal to length of @ctx. If
* @ctx is a string value, it should be nul terminated with
* @ctx_len equal to `strlen(@ctx) + 1`. Binary @ctx values
* are supported.
*
* The @flags and @ctx fields SHOULD only be interpreted by the
* LSM specified by @id; they MUST be set to zero/0 when not used.
*/
struct lsm_ctx {
__u32 id;
__u64 flags;
__kernel_size_t len;
__kernel_size_t ctx_len;
__u8 ctx[];
};
> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> new file mode 100644
> index 000000000000..55e8bf61ac8a
> --- /dev/null
> +++ b/security/lsm_syscalls.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * System calls implementing the Linux Security Module API.
> + *
> + * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
> + * Copyright (C) 2022 Intel Corporation
> + */
> +
> +#include <asm/current.h>
> +#include <linux/compiler_types.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/security.h>
> +#include <linux/stddef.h>
> +#include <linux/syscalls.h>
> +#include <linux/types.h>
> +#include <linux/lsm_hooks.h>
> +#include <uapi/linux/lsm.h>
> +
> +struct attrs_used_map {
> + char *name;
> + int attrs_used;
Based on the usage below it really seems like @attrs_used should just
be @attr, yes? That said, I'm not too bothered by it either way so if
you really love @attrs_used that's fine.
> +};
> +
> +static const struct attrs_used_map lsm_attr_names[] = {
We can probably just call this "attr_map" right? I mean the "used"
portion is pretty inherent in the fact that we're defining a mapping
:)
> + { .name = "current", .attrs_used = LSM_ATTR_CURRENT, },
> + { .name = "exec", .attrs_used = LSM_ATTR_EXEC, },
> + { .name = "fscreate", .attrs_used = LSM_ATTR_FSCREATE, },
> + { .name = "keycreate", .attrs_used = LSM_ATTR_KEYCREATE, },
> + { .name = "prev", .attrs_used = LSM_ATTR_PREV, },
> + { .name = "sockcreate", .attrs_used = LSM_ATTR_SOCKCREATE, },
> +};
> +
> +static int attr_used_index(u32 flags)
Since you can only return one index value at a time in this function
you can't really support multiple attribute bits set in the @flags
parameter so why not change the prototype to better match the required
usage, example:
static int attr_index(u32 attr)
> +{
> + int i;
> +
> + if (flags == 0)
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(lsm_attr_names); i++)
> + if ((lsm_attr_names[i].attrs_used & flags) == flags)
> + return i;
Given the above, why not simplify the above test to this:
if (lsm_attr_name[i].attr == attr)
return i;
If we don't care about failing fast in the case of being passed 0 (why
would we?) we can define this function as follows:
static int attr_index(u32 attr)
{
int i;
for (i = 0; i < ARRAY_SIZE(lsm_attr_names); i++)
if (lsm_attr_names[i].attr == attr)
return i;
return -EINVAL;
}
If we wanted to streamline things even further we could define
attr_map a bit differently and drop the loop in attr_index(). Yes, it
does waste attr_map[0], but I don't think anyone is going to be too
upset about one wasted index if it scales better.
static const struct attr_map[] = {
[LSM_ATTR_CURRENT] = { .name = "current", .attr = LSM_ATTR_CURRENT },
[LSM_ATTR_EXEC] = { .name = "exec", .attr = LSM_ATTR_EXEC },
...
};
static int attr_index(u32 attr)
{
if (attr == 0 || attr >= ARRAY_SIZE(attr_map))
return -EINVAL;
return attr;
}
If you did this you could probably also convert attr_map from a struct
to a simple array of strings as the attribute value would be the
associated index.
> +/**
> + * sys_lsm_get_self_attr - Return current task's security module attributes
> + * @ctx: the LSM contexts
> + * @size: size of @ctx, updated on return
> + * @flags: which attribute to return
> + *
> + * Returns the calling task's LSM contexts. On success this
> + * function returns the number of @ctx array elements. This value
> + * may be zero if there are no LSM contexts assigned. If @size is
> + * insufficient to contain the return data -E2BIG is returned and
> + * @size is set to the minimum required size. In all other cases
> + * a negative value indicating the error is returned.
> + */
> +SYSCALL_DEFINE3(lsm_get_self_attr,
> + struct lsm_ctx __user *, ctx,
> + size_t __user *, size,
> + u32, flags)
> +{
> + int i;
> + int rc = 0;
> + int len;
> + int attr;
> + int count = 0;
> + void *curr;
> + char *cp;
> + char *np;
> + char **interum_ctx;
> + size_t total_size = 0;
> + struct lsm_ctx *ip;
> + struct lsm_ctx *interum;
> + struct lsm_ctx *final = NULL;
> +
> + attr = attr_used_index(flags);
> + if (attr < 0)
> + return attr;
> +
> + interum = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
> + sizeof(*interum), GFP_KERNEL);
> + if (interum == NULL)
> + return -ENOMEM;
> + ip = interum;
> +
> + interum_ctx = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
> + sizeof(*interum_ctx), GFP_KERNEL);
> + if (interum_ctx == NULL) {
> + kfree(interum);
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < lsm_active_cnt; i++) {
> + if ((lsm_idlist[i]->attrs_used &
> + lsm_attr_names[attr].attrs_used) == 0)
> + continue;
> +
> + len = security_getprocattr(current, lsm_idlist[i]->id,
> + lsm_attr_names[attr].name,
> + &cp);
> + if (len <= 0)
> + continue;
> +
> + ip->id = lsm_idlist[i]->id;
> + ip->flags = lsm_attr_names[attr].attrs_used;
> + interum_ctx[count] = cp;
> +
> + /*
> + * A security module that returns a binary attribute
> + * will need to identify itself to prevent string
> + * processing.
> + *
> + * At least one security module adds a \n at the
> + * end of a context to make it look nicer. Change
> + * that to a \0 so that user space doesn't have to
> + * work around it.
> + *
> + * Security modules have been inconsistent about
> + * including the \0 terminator in the size. If it's
> + * not there make space for it.
> + *
> + * The length returned will reflect the length of
> + * the string provided by the security module, which
> + * may not match what getprocattr returned.
> + */
> + np = strnchr(cp, len, '\n');
> + if (np != NULL)
> + *np = '\0';
> + ip->ctx_len = strnlen(cp, len) + 1;
> + total_size += sizeof(*interum) + ip->ctx_len;
> + ip++;
> + count++;
> + }
> +
> + if (count == 0)
> + goto free_out;
> +
> + final = kzalloc(total_size, GFP_KERNEL);
> + if (final == NULL) {
> + rc = -ENOMEM;
> + goto free_out;
> + }
> +
> + curr = final;
> + ip = interum;
> + for (i = 0; i < count; i++) {
> + memcpy(curr, ip, sizeof(*interum));
> + curr += sizeof(*interum);
> + if (ip->ctx_len > 1)
> + memcpy(curr, interum_ctx[i], ip->ctx_len - 1);
> + curr += ip->ctx_len;
> + ip++;
> + }
> +
> + if (get_user(len, size)) {
> + rc = -EFAULT;
> + goto free_out;
> + }
> + if (total_size > len) {
> + rc = -ERANGE;
> + if (put_user(total_size, size) != 0)
> + rc = -EFAULT;
> + goto free_out;
> + }
> + if (copy_to_user(ctx, final, total_size) != 0 ||
> + put_user(total_size, size) != 0)
> + rc = -EFAULT;
> + else
> + rc = count;
> +
> +free_out:
> + for (i = 0; i < count; i++)
> + kfree(interum_ctx[i]);
> + kfree(interum_ctx);
> + kfree(interum);
> + kfree(final);
> + return rc;
> +}
Hmm. That's all kinda painful isn't it? I think trying to reuse
security_getprocattr() is doing more harm than good with all the
awkward handling necessary to ensure consistent output. While it's
nice to be able to reuse existing interfaces, one of the main
motivations behind the LSM syscall effort is to create a cleaner
interface that was designed from the beginning to support multiple
LSMs and provide a level of extensibility that we do not currently
have with the procfs interface. Hacking together all our old crap to
make this happen seems very wrong to me.
With that in mind I would like to propose we introduce a new LSM hook
to populate a lsm_ctx struct based on a LSM_ATTR_XXX value:
int security_sys_getselfattr(u64 attr, struct lsm_ctx __user *ctx,
size_t *len);
The individual LSMs would be responsible for fully populating their
lsm_ctx struct specified by @ctx (note the __user tagging) and would
return 0 on success or negative values on failure. The maximum size
of the @ctx buffer would be passed in via @len and the used size would
be returned in @len; in the case of an too-small @ctx, -E2BIG would be
returned and the necessary size would be returned in @len (just as
discussed for the syscall itself). This way the LSM layer syscall
function would not need to worry about properly terminating the
lsm_ctx::ctx field, setting any LSM specific flags, etc. Passing the
__user pointer directly not only greatly simplifies the LSM layer, it
also has the potential to reduce the number of allocations/copies.
Taking this approach should shrink the LSM layer syscall function to
simply needing to validate the passed @flags before looping through
the LSMs calling security_sys_getselfattr(). The lsm_ctx pointer
would need to be incremented appropriately for each call, and a total
length/size count would need to be maintained in case the buffer is
too small, but those should be relatively minor things.
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/8] LSM: Create lsm_module_list system call
2023-01-09 18:07 ` [PATCH v5 5/8] LSM: Create lsm_module_list system call Casey Schaufler
@ 2023-01-11 21:07 ` Paul Moore
2023-01-12 1:39 ` Casey Schaufler
0 siblings, 1 reply; 30+ messages in thread
From: Paul Moore @ 2023-01-11 21:07 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Mon, Jan 9, 2023 at 1:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Create a system call to report the list of Linux Security Modules
> that are active on the system. The list is provided as an array
> of LSM ID numbers.
>
> The calling application can use this list determine what LSM
> specific actions it might take. That might include chosing an
> output format, determining required privilege or bypassing
> security module specific behavior.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> Documentation/userspace-api/lsm.rst | 3 +++
> include/linux/syscalls.h | 1 +
> kernel/sys_ni.c | 1 +
> security/lsm_syscalls.c | 41 +++++++++++++++++++++++++++++
> 4 files changed, 46 insertions(+)
...
> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> index 55e8bf61ac8a..92af1fcaa654 100644
> --- a/security/lsm_syscalls.c
> +++ b/security/lsm_syscalls.c
> @@ -180,3 +180,44 @@ SYSCALL_DEFINE3(lsm_get_self_attr,
> kfree(final);
> return rc;
> }
> +
> +/**
> + * sys_lsm_module_list - Return a list of the active security modules
> + * @ids: the LSM module ids
> + * @size: size of @ids, updated on return
> + * @flags: reserved for future use, must be zero
> + *
> + * Returns a list of the active LSM ids. On success this function
> + * returns the number of @ids array elements. This value may be zero
> + * if there are no LSMs active. If @size is insufficient to contain
> + * the return data -E2BIG is returned and @size is set to the minimum
> + * required size. In all other cases a negative value indicating the
> + * error is returned.
> + */
> +SYSCALL_DEFINE3(lsm_module_list,
> + u32 __user *, ids,
> + size_t __user *, size,
> + u64, flags)
> +{
> + size_t total_size = lsm_active_cnt * sizeof(*ids);
> + size_t usize;
> + int i;
> +
> + if (flags)
> + return -EINVAL;
> +
> + if (get_user(usize, size))
> + return -EFAULT;
> +
> + if (put_user(total_size, size) != 0)
> + return -EFAULT;
> +
> + if (usize < total_size)
> + return -E2BIG;
> +
> + for (i = 0; i < lsm_active_cnt; i++)
> + if (put_user(lsm_idlist[i]->id, ids++))
> + return -EFAULT;
> +
> + return lsm_active_cnt;
> +}
Similar to my comments in 4/8, I would probably create a new LSM hook
for this syscall so that the lsm_ctx is passed through the LSM layer
directly to the target LSM:
int security_sys_setselfattr(u64 attr, struct lsm_ctx __user *ctx,
size_t len);
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 1/8] LSM: Identify modules by more than name
2023-01-11 21:00 ` Paul Moore
@ 2023-01-12 0:05 ` Casey Schaufler
2023-01-12 20:30 ` Paul Moore
0 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2023-01-12 0:05 UTC (permalink / raw)
To: Paul Moore
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic, casey
On 1/11/2023 1:00 PM, Paul Moore wrote:
> On Mon, Jan 9, 2023 at 1:07 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Create a struct lsm_id to contain identifying information
>> about Linux Security Modules (LSMs). At inception this contains
>> the name of the module, an identifier associated with the security
>> module and an integer member "attrs_used" which identifies the API
>> related data associated with each security module. The initial set
>> of features maps to information that has traditionaly been available
>> in /proc/self/attr. They are documented in a new userspace-api file.
>> Change the security_add_hooks() interface to use this structure.
>> Change the individual modules to maintain their own struct lsm_id
>> and pass it to security_add_hooks().
>>
>> The values are for LSM identifiers are defined in a new UAPI
>> header file linux/lsm.h. Each existing LSM has been updated to
>> include it's LSMID in the lsm_id.
>>
>> The LSM ID values are sequential, with the oldest module
>> LSM_ID_CAPABILITY being the lowest value and the existing modules
>> numbered in the order they were included in the main line kernel.
>> This is an arbitrary convention for assigning the values, but
>> none better presents itself. The value 0 is defined as being invalid.
>> The values 1-99 are reserved for any special case uses which may
>> arise in the future. This may include attributes of the LSM
>> infrastructure itself, possibly related to namespacing or network
>> attribute management. A special range is identified for such attributes
>> to help reduce confusion for developers unfamiliar with LSMs.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> Documentation/userspace-api/index.rst | 1 +
>> Documentation/userspace-api/lsm.rst | 55 +++++++++++++++++++++++++++
>> include/linux/lsm_hooks.h | 18 ++++++++-
>> include/uapi/linux/lsm.h | 55 +++++++++++++++++++++++++++
>> security/apparmor/lsm.c | 9 ++++-
>> security/bpf/hooks.c | 13 ++++++-
>> security/commoncap.c | 8 +++-
>> security/landlock/cred.c | 2 +-
>> security/landlock/fs.c | 2 +-
>> security/landlock/ptrace.c | 2 +-
>> security/landlock/setup.c | 6 +++
>> security/landlock/setup.h | 1 +
>> security/loadpin/loadpin.c | 9 ++++-
>> security/lockdown/lockdown.c | 8 +++-
>> security/safesetid/lsm.c | 9 ++++-
>> security/security.c | 12 +++---
>> security/selinux/hooks.c | 11 +++++-
>> security/smack/smack_lsm.c | 9 ++++-
>> security/tomoyo/tomoyo.c | 9 ++++-
>> security/yama/yama_lsm.c | 8 +++-
>> 20 files changed, 226 insertions(+), 21 deletions(-)
>> create mode 100644 Documentation/userspace-api/lsm.rst
>> create mode 100644 include/uapi/linux/lsm.h
> Mostly just nitpicky stuff below ...
Nitpicky is fine.
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 0a5ba81f7367..6f2cabb79ec4 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1665,6 +1665,20 @@ struct security_hook_heads {
>> #undef LSM_HOOK
>> } __randomize_layout;
>>
>> +/**
>> + * struct lsm_id - identify a Linux Security Module.
> According to the kernel-doc documentation it looks like "identify"
> should be capitalized.
>
> * https://docs.kernel.org/doc-guide/kernel-doc.html
I'll fix this, and the next as well. Thanks for pointing it out.
>> + * @lsm: Name of the LSM. Must be approved by the LSM maintainers.
>> + * @id: LSM ID number from uapi/linux/lsm.h
>> + * @attrs_used: Which attributes this LSM supports.
> In a bit of a reversal to the above comment, it appears that the
> parameter descriptions should not start with a capital and should not
> end with punctuation:
>
> * @lsm: name of the LSM, must be approved by the LSM maintainers
>
>> + * Contains the information that identifies the LSM.
>> + */
>> +struct lsm_id {
>> + const u8 *lsm;
>> + u32 id;
>> + u64 attrs_used;
>> +};
>> @@ -1708,7 +1722,7 @@ extern struct security_hook_heads security_hook_heads;
>> extern char *lsm_names;
>>
>> extern void security_add_hooks(struct security_hook_list *hooks, int count,
>> - const char *lsm);
>> + struct lsm_id *lsmid);
> We should be able to mark @lsmid as a const, right?
At this point, yes, but the const would have to come off when
the "slot" field is added to lsm_id in the upcoming stacking patches.
I can mark it const for now if it is important.
>> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
>> new file mode 100644
>> index 000000000000..61a91b7d946f
>> --- /dev/null
>> +++ b/include/uapi/linux/lsm.h
>> @@ -0,0 +1,55 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/*
>> + * Linux Security Modules (LSM) - User space API
>> + *
>> + * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
>> + * Copyright (C) 2022 Intel Corporation
>> + */
> This file should be added to the SECURITY SUBSYSTEM section in MAINTAINERS:
>
> F: include/uapi/linux/lsm.h
S'truth.
>> +#ifndef _UAPI_LINUX_LSM_H
>> +#define _UAPI_LINUX_LSM_H
>> +
>> +/*
>> + * ID values to identify security modules.
>> + * A system may use more than one security module.
>> + *
>> + * A value of 0 is considered invalid.
>> + * Values 1-99 are reserved for future use.
>> + * The interface is designed to extend to attributes beyond those which
>> + * are active today. Currently all the attributes are specific to the
>> + * individual modules. The LSM infrastructure itself has no variable state,
>> + * but that may change. One proposal would allow loadable modules, in which
>> + * case an attribute such as LSM_IS_LOADABLE might identify the dynamic
>> + * modules. Another potential attribute could be which security modules is
>> + * associated withnetwork labeling using netlabel. Another possible attribute
>> + * could be related to stacking behavior in a namespaced environment.
>> + * While it would be possible to intermingle the LSM infrastructure attribute
>> + * values with the security module provided values, keeping them separate
>> + * provides a clearer distinction.
>> + */
> As this is in a UAPI file, let's avoid speculation and stick to just
> the current facts. Anything we write here with respect to the future
> is likely to be wrong so let's not tempt fate.
Sure. I'll leave the rationale to the take message.
> Once I reached patch 3/8 I also realized that we may want to have more
> than just 0/invalid as a sentinel value, or at the very least we need
> to redefine 0 as something slightly different if for no other reason
> than we in fs/proc/base.c. I know it seems a little trivial, but
> since we're talking about values that will be used in the UAPI I think
> we need to give it some thought and discussion. The only think I can
> think of right now is to redefine 0 as "undefined", which isn't that
> far removed from "invalid" and will not look too terrible in patch 3/8
> - thoughts?
I originally had LSM_ID_INVALID for 0, but there was an objection.
It's not really invalid or undefined, it is reserved as not being an LSM id.
How about LSM_ID_NALSMID or LSM_ID_NOTALSMID for 0? sort of like NAN for
Not A Number.
> With all that in mind, I would suggest something like this:
>
> /*
> * ID tokens to identify Linux Security Modules (LSMs)
> *
> * These token values are used to uniquely identify specific LSMs
> * in the kernel as well in the kernel's LSM userspace API.
> *
> * A value of zero/0 is considered undefined and should not be used
> * outside of the kernel, values 1-99 are reserved for potential
> * future use.
> */
> #define LSM_ID_UNDEF 0
Fine, although I'd go with LSM_ID_NALSMID
>> +#define LSM_ID_CAPABILITY 100
>> +#define LSM_ID_SELINUX 101
>> +#define LSM_ID_SMACK 102
>> +#define LSM_ID_TOMOYO 103
>> +#define LSM_ID_IMA 104
>> +#define LSM_ID_APPARMOR 105
>> +#define LSM_ID_YAMA 106
>> +#define LSM_ID_LOADPIN 107
>> +#define LSM_ID_SAFESETID 108
>> +#define LSM_ID_LOCKDOWN 109
>> +#define LSM_ID_BPF 110
>> +#define LSM_ID_LANDLOCK 111
>> +
>> +/*
>> + * LSM_ATTR_XXX values identify the /proc/.../attr entry that the
>> + * context represents. Not all security modules provide all of these
>> + * values. Some security modules provide none of them.
>> + */
> I'd rather see text closer to this:
>
> /*
> * LSM attributes
> *
> * The LSM_ATTR_XXX definitions identify different LSM
> * attributes which are used in the kernel's LSM userspace API.
> * Support for these attributes vary across the different LSMs,
> * none are required.
> */
If you like that better I'm completely willing to adopt it.
>> +#define LSM_ATTR_CURRENT 0x0001
>> +#define LSM_ATTR_EXEC 0x0002
>> +#define LSM_ATTR_FSCREATE 0x0004
>> +#define LSM_ATTR_KEYCREATE 0x0008
>> +#define LSM_ATTR_PREV 0x0010
>> +#define LSM_ATTR_SOCKCREATE 0x0020
>> +
>> +#endif /* _UAPI_LINUX_LSM_H */
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index c6728a629437..63ea2a995987 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -24,6 +24,7 @@
>> #include <linux/zstd.h>
>> #include <net/sock.h>
>> #include <uapi/linux/mount.h>
>> +#include <uapi/linux/lsm.h>
>>
>> #include "include/apparmor.h"
>> #include "include/apparmorfs.h"
>> @@ -1217,6 +1218,12 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
>> .lbs_task = sizeof(struct aa_task_ctx),
>> };
>>
>> +static struct lsm_id apparmor_lsmid __lsm_ro_after_init = {
>> + .lsm = "apparmor",
>> + .id = LSM_ID_APPARMOR,
>> + .attrs_used = LSM_ATTR_CURRENT | LSM_ATTR_PREV | LSM_ATTR_EXEC,
>> +};
> Perhaps mark this as const in addition to ro_after_init? This applies
> to all the other @lsm_id defs in this patch too.
As I mentioned above, the lsm_id will eventually get changed during the
registration process. I can add the const for now, knowing full well that
it will be removed before long.
>> diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
>> index e5971fa74fd7..20983ae8d31f 100644
>> --- a/security/bpf/hooks.c
>> +++ b/security/bpf/hooks.c
>> @@ -5,6 +5,7 @@
>> */
>> #include <linux/lsm_hooks.h>
>> #include <linux/bpf_lsm.h>
>> +#include <uapi/linux/lsm.h>
>>
>> static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
>> #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
>> @@ -15,9 +16,19 @@ static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(task_free, bpf_task_storage_free),
>> };
>>
>> +/*
>> + * slot has to be LSMBLOB_NEEDED because some of the hooks
>> + * supplied by this module require a slot.
>> + */
> I don't think we need the above comment here, right?
Whoops. I thought I'd gotten that one.
>
> --
> paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/8] LSM: Maintain a table of LSM attribute data
2023-01-11 21:01 ` Paul Moore
@ 2023-01-12 0:36 ` Casey Schaufler
2023-01-12 20:26 ` Paul Moore
0 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2023-01-12 0:36 UTC (permalink / raw)
To: Paul Moore
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On 1/11/2023 1:01 PM, Paul Moore wrote:
> On Mon, Jan 9, 2023 at 1:07 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> As LSMs are registered add their lsm_id pointers to a table.
>> This will be used later for attribute reporting.
>>
>> Determine the number of possible security modules based on
>> their respective CONFIG options. This allows the number to be
>> known at build time. This allows data structures and tables
>> to use the constant.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> include/linux/security.h | 2 ++
>> security/security.c | 44 +++++++++++++++++++++++++++++++++-------
>> 2 files changed, 39 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 5b67f208f7de..33ed1860b96f 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -138,6 +138,8 @@ enum lockdown_reason {
>> };
>>
>> extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
>> +extern u32 lsm_active_cnt;
>> +extern struct lsm_id *lsm_idlist[];
>>
>> /* These functions are in security/commoncap.c */
>> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
>> diff --git a/security/security.c b/security/security.c
>> index 07a8fe7f92bf..a590fa98ddd6 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -28,12 +28,29 @@
>> #include <linux/backing-dev.h>
>> #include <linux/string.h>
>> #include <linux/msg.h>
>> +#include <uapi/linux/lsm.h>
>> #include <net/flow.h>
>>
>> #define MAX_LSM_EVM_XATTR 2
>>
>> -/* How many LSMs were built into the kernel? */
>> -#define LSM_COUNT (__end_lsm_info - __start_lsm_info)
>> +/*
>> + * How many LSMs are built into the kernel as determined at
>> + * build time. Used to determine fixed array sizes.
>> + * The capability module is accounted for by CONFIG_SECURITY
>> + */
>> +#define LSM_COUNT ( \
>> + (IS_ENABLED(CONFIG_SECURITY) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN_LSM) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0))
>>
>> /*
>> * These are descriptions of the reasons that can be passed to the
>> @@ -90,7 +107,7 @@ static __initdata const char *chosen_major_lsm;
>> static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
>>
>> /* Ordered list of LSMs to initialize. */
>> -static __initdata struct lsm_info **ordered_lsms;
>> +static __initdata struct lsm_info *ordered_lsms[LSM_COUNT + 1];
> I'm guessing this 'LSM_COUNT + 1' logic is basically just copied from
> ordered_lsm_init() - which is okay - but can you remind me why it is
> 'LSM_COUNT + 1' and not just 'LSM_COUNT'? Based on the LSM_COUNT
> macro above it seems like LSM_COUNT should be enough, no?
Yup. I didn't spend a lot of time investigating why the + 1.
I'll look more deeply and correct if appropriate.
>> static __initdata struct lsm_info *exclusive;
>>
>> static __initdata bool debug;
>> @@ -341,13 +358,16 @@ static void __init report_lsm_order(void)
>> pr_cont("\n");
>> }
>>
>> +/*
>> + * Current index to use while initializing the lsm id list.
>> + */
>> +u32 lsm_active_cnt __lsm_ro_after_init;
>> +struct lsm_id *lsm_idlist[LSM_COUNT] __lsm_ro_after_init;
>> +
>> static void __init ordered_lsm_init(void)
>> {
>> struct lsm_info **lsm;
>>
>> - ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
>> - GFP_KERNEL);
>> -
>> if (chosen_lsm_order) {
>> if (chosen_major_lsm) {
>> pr_warn("security=%s is ignored because it is superseded by lsm=%s\n",
>> @@ -388,7 +408,7 @@ static void __init ordered_lsm_init(void)
>> for (lsm = ordered_lsms; *lsm; lsm++)
>> initialize_lsm(*lsm);
>>
>> - kfree(ordered_lsms);
>> + init_debug("lsm count = %d\n", lsm_active_cnt);
>> }
> Given 86ef3c735ec8 ("LSM: Better reporting of actual LSMs at boot"),
> is this needed?
None of what comes out from lsm.debug is strictly necessary, and
human or script can parse "initializing lsm=", but sometimes the
number of LSMs is interesting.
>
> --
> paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 3/8] proc: Use lsmids instead of lsm names for attrs
2023-01-11 21:01 ` Paul Moore
@ 2023-01-12 0:37 ` Casey Schaufler
0 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2023-01-12 0:37 UTC (permalink / raw)
To: Paul Moore
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic, linux-fsdevel, casey
On 1/11/2023 1:01 PM, Paul Moore wrote:
> On Mon, Jan 9, 2023 at 1:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Use the LSM ID number instead of the LSM name to identify which
>> security module's attibute data should be shown in /proc/self/attr.
>> The security_[gs]etprocattr() functions have been changed to expect
>> the LSM ID. The change from a string comparison to an integer comparison
>> in these functions will provide a minor performance improvement.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> Cc: linux-fsdevel@vger.kernel.org
>> ---
>> fs/proc/base.c | 29 +++++++++++++++--------------
>> fs/proc/internal.h | 2 +-
>> include/linux/security.h | 11 +++++------
>> security/security.c | 11 +++++------
>> 4 files changed, 26 insertions(+), 27 deletions(-)
> ..
>
>> diff --git a/fs/proc/base.c b/fs/proc/base.c
>> index 9e479d7d202b..9328b6b07dfc 100644
>> --- a/fs/proc/base.c
>> +++ b/fs/proc/base.c
>> @@ -2837,27 +2838,27 @@ static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \
>>
>> #ifdef CONFIG_SECURITY_SMACK
>> static const struct pid_entry smack_attr_dir_stuff[] = {
>> - ATTR("smack", "current", 0666),
>> + ATTR(LSM_ID_SMACK, "current", 0666),
>> };
>> LSM_DIR_OPS(smack);
>> #endif
>>
>> #ifdef CONFIG_SECURITY_APPARMOR
>> static const struct pid_entry apparmor_attr_dir_stuff[] = {
>> - ATTR("apparmor", "current", 0666),
>> - ATTR("apparmor", "prev", 0444),
>> - ATTR("apparmor", "exec", 0666),
>> + ATTR(LSM_ID_APPARMOR, "current", 0666),
>> + ATTR(LSM_ID_APPARMOR, "prev", 0444),
>> + ATTR(LSM_ID_APPARMOR, "exec", 0666),
>> };
>> LSM_DIR_OPS(apparmor);
>> #endif
>>
>> static const struct pid_entry attr_dir_stuff[] = {
>> - ATTR(NULL, "current", 0666),
>> - ATTR(NULL, "prev", 0444),
>> - ATTR(NULL, "exec", 0666),
>> - ATTR(NULL, "fscreate", 0666),
>> - ATTR(NULL, "keycreate", 0666),
>> - ATTR(NULL, "sockcreate", 0666),
>> + ATTR(0, "current", 0666),
>> + ATTR(0, "prev", 0444),
>> + ATTR(0, "exec", 0666),
>> + ATTR(0, "fscreate", 0666),
>> + ATTR(0, "keycreate", 0666),
>> + ATTR(0, "sockcreate", 0666),
> See the discussion in patch 1/8, we should use a macro instead of a 0
> here (although the exact macro definition is very much up for
> discussion):
>
> ATTR(LSM_ID_UNDEF, "current", 0666),
Or LSM_ID_NALSMID, or whatever. Agreed.
>
> --
> paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-01-11 21:07 ` Paul Moore
@ 2023-01-12 1:37 ` Casey Schaufler
2023-01-12 21:37 ` Paul Moore
0 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2023-01-12 1:37 UTC (permalink / raw)
To: Paul Moore
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic, casey
On 1/11/2023 1:07 PM, Paul Moore wrote:
> On Mon, Jan 9, 2023 at 1:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Create a system call lsm_get_self_attr() to provide the security
>> module maintained attributes of the current process. Historically
>> these attributes have been exposed to user space via entries in
>> procfs under /proc/self/attr.
>>
>> Attributes are provided as a collection of lsm_ctx structures
>> which are placed into a user supplied buffer. Each structure
>> identifys the size of the attribute, and the attribute value.
> ^^^
> identifies
Pluralses are hard in the vicinity of 'y's. ;)
>> The format of the attribute value is defined by the security
>> module, but will always be \0 terminated. The ctx_len value
>> will always be strlen(ctx)+1.
> I don't want to limit ourselves to only sending string values as
> attributes as who knows what we might need to do in the future, and
> the struct was originally designed to support both strings and binary
> data. I would suggest changing the sentences above to something like
> this:
Part of creating a sane and sensible API is setting rational limitations.
While a "context" has always allowed for a binary value it has always
been a user friendly, nul terminated string. The one case where someone
proposed otherwise was my "hideous" format for compound contexts, and we
know how well that was received. If we're serious about cleaning up our
API let's quit bending over to support something no one is using and that
we'd prefer they didn't.
That's my preference. Is there anyone who wants a binary interface, or any
really good reason to provide one? I'm not going to stand fast on a strings
only interface, but it would make it significantly cleaner if we had one.
>
> The format of the attribute value is defined by the individual LSM,
> with the attribute itself stored in @ctx and the length of the
> attribute stored in @ctx_len. Both strings and arbitrary binary
> attributes are supported, but strings should be NULL terminated and
> @ctx_len should be equal to `strlen(@ctx) + 1`.
>
>> ---------------------------
>> | __u32 id |
>> ---------------------------
>> | __u64 flags |
>> ---------------------------
>> | __kernel_size_t ctx_len |
>> ---------------------------
>> | __u8 ctx[ctx_len] |
>> ---------------------------
>> | __u32 id |
>> ---------------------------
>> | __u64 flags |
>> ---------------------------
>> | __kernel_size_t ctx_len |
>> ---------------------------
>> | __u8 ctx[ctx_len] |
>> ---------------------------
> Don't repeat the structure layout in memory twice here, it's
> confusing. I also think it would be easier to read, and arguably more
> useful, to simply copy the struct definition into the description
> instead of the ASCII art column.
OK on both.
> Although, this has got me wondering if we should think about aligning
> the lsm_ctx structs when we are populating them in the kernel; more on
> this below ...
As you say below, we'll need a total_len for this, but OK.
>> ---
>> Documentation/userspace-api/lsm.rst | 9 ++
>> include/linux/syscalls.h | 3 +
>> include/uapi/linux/lsm.h | 21 ++++
>> kernel/sys_ni.c | 3 +
>> security/Makefile | 1 +
>> security/lsm_syscalls.c | 182 ++++++++++++++++++++++++++++
>> 6 files changed, 219 insertions(+)
>> create mode 100644 security/lsm_syscalls.c
> ..
>
>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index 33a0ee3bcb2e..a89205c70ffa 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -71,6 +71,7 @@ struct clone_args;
>> struct open_how;
>> struct mount_attr;
>> struct landlock_ruleset_attr;
>> +struct lsm_ctx;
>> enum landlock_rule_type;
>>
>> #include <linux/types.h>
>> @@ -1058,6 +1059,8 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
>> asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
>> unsigned long home_node,
>> unsigned long flags);
>> +asmlinkage long sys_lsm_get_self_attr(struct lsm_ctx *ctx, size_t *size,
>> + int flags);
>>
>> /*
>> * Architecture-specific system calls
>> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
>> index 61a91b7d946f..8674d8c6b326 100644
>> --- a/include/uapi/linux/lsm.h
>> +++ b/include/uapi/linux/lsm.h
>> @@ -9,6 +9,27 @@
>> #ifndef _UAPI_LINUX_LSM_H
>> #define _UAPI_LINUX_LSM_H
>>
>> +#include <linux/types.h>
>> +#include <linux/unistd.h>
>> +
>> +/**
>> + * struct lsm_ctx - LSM context
>> + * @id: the LSM id number, see LSM_ID_XXX
> As mentioned above, it occurred to me that we might want want to pad
> out the lsm_ctx struct to ensure that the "array" of lsm_ctx is nicely
> aligned. I know some systems used to complain about unaligned
> accesses, and even on those that don't complain tend to be faster when
> access are aligned. We can either implicitly align the individual
> lsm_ctx structs or we can add a total length field (in addition to the
> @ctx_len field) so that the padding/alignment is explicit.
>
> Adding an explicit total length field could have some other advantages
> in that it, in conjunction with the existing @flags field, would allow
> an individual LSM to "extend" the lsm_ctx struct to provide additional
> LSM specific information in the case where the single @ctx field was
> not sufficient. Think of it as some additional future proofing in
> addition to explicit padding.
I'm not sure whether to call it future proofing or confusion assurance,
and I certainly wouldn't encourage such use. On the other hand, I wouldn't
let that get in the way of the performant aligned interface, so I'm good
with the len field in addition to the ctx_len.
>> + * @flags: context specifier and LSM specific flags
> * @flags: LSM specific flags
>
> Only the individual LSM specified in @id should ever interpret @flags or @ctx.
Sure.
>> + * @ctx_len: the size of @ctx
>> + * @ctx: the LSM context, a nul terminated string
> * @ctx: the LSM context value
As above, I would like to make this a string. I won't fight over it however.
>> + * @ctx in a nul terminated string.
>> + * (strlen(@ctx) < @ctx_len) is always true.
>> + * (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
>> + */
> Let's rework the extra description too based on the comments above.
> For the sake of clarity, here is what I'm currently thinking (comments
> and feedback are encouraged):
>
> /**
> * struct lsm_ctx - LSM context information
> * @id: the LSM ID token, see LSM_ID_XXX
> * @flags: LSM specific flags
> * @len: length of the lsm_ctx struct + extra (?) + padding
> * @ctx_len: the size of @ctx
> * @ctx: the LSM context value
> *
> * The @len field MUST be equal to size of the lsm_ctx struct
> * plus any additional padding and/or data placed after @ctx.
> *
> * In all cases @ctx_len MUST be equal to length of @ctx. If
> * @ctx is a string value, it should be nul terminated with
> * @ctx_len equal to `strlen(@ctx) + 1`. Binary @ctx values
> * are supported.
> *
> * The @flags and @ctx fields SHOULD only be interpreted by the
> * LSM specified by @id; they MUST be set to zero/0 when not used.
> */
> struct lsm_ctx {
> __u32 id;
> __u64 flags;
> __kernel_size_t len;
> __kernel_size_t ctx_len;
> __u8 ctx[];
> };
Yes, if there's a convincing argument for binary values.
>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>> new file mode 100644
>> index 000000000000..55e8bf61ac8a
>> --- /dev/null
>> +++ b/security/lsm_syscalls.c
>> @@ -0,0 +1,182 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * System calls implementing the Linux Security Module API.
>> + *
>> + * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
>> + * Copyright (C) 2022 Intel Corporation
>> + */
>> +
>> +#include <asm/current.h>
>> +#include <linux/compiler_types.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/security.h>
>> +#include <linux/stddef.h>
>> +#include <linux/syscalls.h>
>> +#include <linux/types.h>
>> +#include <linux/lsm_hooks.h>
>> +#include <uapi/linux/lsm.h>
>> +
>> +struct attrs_used_map {
>> + char *name;
>> + int attrs_used;
> Based on the usage below it really seems like @attrs_used should just
> be @attr, yes? That said, I'm not too bothered by it either way so if
> you really love @attrs_used that's fine.
Potato - Potatoe - Seeing the same field name in different structs at
the same time gives me a headache, but I'm not going to quibble.
>> +};
>> +
>> +static const struct attrs_used_map lsm_attr_names[] = {
> We can probably just call this "attr_map" right? I mean the "used"
> portion is pretty inherent in the fact that we're defining a mapping
> :)
Sure.
>> + { .name = "current", .attrs_used = LSM_ATTR_CURRENT, },
>> + { .name = "exec", .attrs_used = LSM_ATTR_EXEC, },
>> + { .name = "fscreate", .attrs_used = LSM_ATTR_FSCREATE, },
>> + { .name = "keycreate", .attrs_used = LSM_ATTR_KEYCREATE, },
>> + { .name = "prev", .attrs_used = LSM_ATTR_PREV, },
>> + { .name = "sockcreate", .attrs_used = LSM_ATTR_SOCKCREATE, },
>> +};
>> +
>> +static int attr_used_index(u32 flags)
> Since you can only return one index value at a time in this function
> you can't really support multiple attribute bits set in the @flags
> parameter so why not change the prototype to better match the required
> usage, example:
>
> static int attr_index(u32 attr)
>
>> +{
>> + int i;
>> +
>> + if (flags == 0)
>> + return -EINVAL;
>> +
>> + for (i = 0; i < ARRAY_SIZE(lsm_attr_names); i++)
>> + if ((lsm_attr_names[i].attrs_used & flags) == flags)
>> + return i;
> Given the above, why not simplify the above test to this:
>
> if (lsm_attr_name[i].attr == attr)
> return i;
That won't work in the case where an LSM supports more than one attribute.
>
> If we don't care about failing fast in the case of being passed 0 (why
> would we?) we can define this function as follows:
>
> static int attr_index(u32 attr)
> {
> int i;
> for (i = 0; i < ARRAY_SIZE(lsm_attr_names); i++)
> if (lsm_attr_names[i].attr == attr)
> return i;
> return -EINVAL;
> }
>
> If we wanted to streamline things even further we could define
> attr_map a bit differently and drop the loop in attr_index(). Yes, it
> does waste attr_map[0], but I don't think anyone is going to be too
> upset about one wasted index if it scales better.
>
> static const struct attr_map[] = {
> [LSM_ATTR_CURRENT] = { .name = "current", .attr = LSM_ATTR_CURRENT },
> [LSM_ATTR_EXEC] = { .name = "exec", .attr = LSM_ATTR_EXEC },
> ...
> };
>
> static int attr_index(u32 attr)
> {
> if (attr == 0 || attr >= ARRAY_SIZE(attr_map))
> return -EINVAL;
> return attr;
> }
>
> If you did this you could probably also convert attr_map from a struct
> to a simple array of strings as the attribute value would be the
> associated index.
>
>> +/**
>> + * sys_lsm_get_self_attr - Return current task's security module attributes
>> + * @ctx: the LSM contexts
>> + * @size: size of @ctx, updated on return
>> + * @flags: which attribute to return
>> + *
>> + * Returns the calling task's LSM contexts. On success this
>> + * function returns the number of @ctx array elements. This value
>> + * may be zero if there are no LSM contexts assigned. If @size is
>> + * insufficient to contain the return data -E2BIG is returned and
>> + * @size is set to the minimum required size. In all other cases
>> + * a negative value indicating the error is returned.
>> + */
>> +SYSCALL_DEFINE3(lsm_get_self_attr,
>> + struct lsm_ctx __user *, ctx,
>> + size_t __user *, size,
>> + u32, flags)
>> +{
>> + int i;
>> + int rc = 0;
>> + int len;
>> + int attr;
>> + int count = 0;
>> + void *curr;
>> + char *cp;
>> + char *np;
>> + char **interum_ctx;
>> + size_t total_size = 0;
>> + struct lsm_ctx *ip;
>> + struct lsm_ctx *interum;
>> + struct lsm_ctx *final = NULL;
>> +
>> + attr = attr_used_index(flags);
>> + if (attr < 0)
>> + return attr;
>> +
>> + interum = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
>> + sizeof(*interum), GFP_KERNEL);
>> + if (interum == NULL)
>> + return -ENOMEM;
>> + ip = interum;
>> +
>> + interum_ctx = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
>> + sizeof(*interum_ctx), GFP_KERNEL);
>> + if (interum_ctx == NULL) {
>> + kfree(interum);
>> + return -ENOMEM;
>> + }
>> +
>> + for (i = 0; i < lsm_active_cnt; i++) {
>> + if ((lsm_idlist[i]->attrs_used &
>> + lsm_attr_names[attr].attrs_used) == 0)
>> + continue;
>> +
>> + len = security_getprocattr(current, lsm_idlist[i]->id,
>> + lsm_attr_names[attr].name,
>> + &cp);
>> + if (len <= 0)
>> + continue;
>> +
>> + ip->id = lsm_idlist[i]->id;
>> + ip->flags = lsm_attr_names[attr].attrs_used;
>> + interum_ctx[count] = cp;
>> +
>> + /*
>> + * A security module that returns a binary attribute
>> + * will need to identify itself to prevent string
>> + * processing.
>> + *
>> + * At least one security module adds a \n at the
>> + * end of a context to make it look nicer. Change
>> + * that to a \0 so that user space doesn't have to
>> + * work around it.
>> + *
>> + * Security modules have been inconsistent about
>> + * including the \0 terminator in the size. If it's
>> + * not there make space for it.
>> + *
>> + * The length returned will reflect the length of
>> + * the string provided by the security module, which
>> + * may not match what getprocattr returned.
>> + */
>> + np = strnchr(cp, len, '\n');
>> + if (np != NULL)
>> + *np = '\0';
>> + ip->ctx_len = strnlen(cp, len) + 1;
>> + total_size += sizeof(*interum) + ip->ctx_len;
>> + ip++;
>> + count++;
>> + }
>> +
>> + if (count == 0)
>> + goto free_out;
>> +
>> + final = kzalloc(total_size, GFP_KERNEL);
>> + if (final == NULL) {
>> + rc = -ENOMEM;
>> + goto free_out;
>> + }
>> +
>> + curr = final;
>> + ip = interum;
>> + for (i = 0; i < count; i++) {
>> + memcpy(curr, ip, sizeof(*interum));
>> + curr += sizeof(*interum);
>> + if (ip->ctx_len > 1)
>> + memcpy(curr, interum_ctx[i], ip->ctx_len - 1);
>> + curr += ip->ctx_len;
>> + ip++;
>> + }
>> +
>> + if (get_user(len, size)) {
>> + rc = -EFAULT;
>> + goto free_out;
>> + }
>> + if (total_size > len) {
>> + rc = -ERANGE;
>> + if (put_user(total_size, size) != 0)
>> + rc = -EFAULT;
>> + goto free_out;
>> + }
>> + if (copy_to_user(ctx, final, total_size) != 0 ||
>> + put_user(total_size, size) != 0)
>> + rc = -EFAULT;
>> + else
>> + rc = count;
>> +
>> +free_out:
>> + for (i = 0; i < count; i++)
>> + kfree(interum_ctx[i]);
>> + kfree(interum_ctx);
>> + kfree(interum);
>> + kfree(final);
>> + return rc;
>> +}
> Hmm. That's all kinda painful isn't it?
It's not so bad as all that. Well, maybe.
> I think trying to reuse
> security_getprocattr() is doing more harm than good with all the
> awkward handling necessary to ensure consistent output. While it's
> nice to be able to reuse existing interfaces, one of the main
> motivations behind the LSM syscall effort is to create a cleaner
> interface that was designed from the beginning to support multiple
> LSMs and provide a level of extensibility that we do not currently
> have with the procfs interface. Hacking together all our old crap to
> make this happen seems very wrong to me.
>
> With that in mind I would like to propose we introduce a new LSM hook
> to populate a lsm_ctx struct based on a LSM_ATTR_XXX value:
>
> int security_sys_getselfattr(u64 attr, struct lsm_ctx __user *ctx,
> size_t *len);
>
> The individual LSMs would be responsible for fully populating their
> lsm_ctx struct specified by @ctx (note the __user tagging) and would
> return 0 on success or negative values on failure. The maximum size
> of the @ctx buffer would be passed in via @len and the used size would
> be returned in @len; in the case of an too-small @ctx, -E2BIG would be
> returned and the necessary size would be returned in @len (just as
> discussed for the syscall itself). This way the LSM layer syscall
> function would not need to worry about properly terminating the
> lsm_ctx::ctx field, setting any LSM specific flags, etc. Passing the
> __user pointer directly not only greatly simplifies the LSM layer, it
> also has the potential to reduce the number of allocations/copies.
That's going to be a lot of duplicate code to add to each LSM. Yes,
we can do that, but I don't see it as any cleaner.
> Taking this approach should shrink the LSM layer syscall function to
> simply needing to validate the passed @flags before looping through
> the LSMs calling security_sys_getselfattr(). The lsm_ctx pointer
> would need to be incremented appropriately for each call, and a total
> length/size count would need to be maintained in case the buffer is
> too small, but those should be relatively minor things.
I think this pushed the complexity downward. If we only had one LSM
it would be a wash. With each LSM having to provide this we're multiplying
the complexity rather than reducing it.
But again, I'll listen to reason.
>
> --
> paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/8] LSM: Create lsm_module_list system call
2023-01-11 21:07 ` Paul Moore
@ 2023-01-12 1:39 ` Casey Schaufler
2023-01-12 21:43 ` Paul Moore
0 siblings, 1 reply; 30+ messages in thread
From: Casey Schaufler @ 2023-01-12 1:39 UTC (permalink / raw)
To: Paul Moore
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic, casey
On 1/11/2023 1:07 PM, Paul Moore wrote:
> On Mon, Jan 9, 2023 at 1:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Create a system call to report the list of Linux Security Modules
>> that are active on the system. The list is provided as an array
>> of LSM ID numbers.
>>
>> The calling application can use this list determine what LSM
>> specific actions it might take. That might include chosing an
>> output format, determining required privilege or bypassing
>> security module specific behavior.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> Documentation/userspace-api/lsm.rst | 3 +++
>> include/linux/syscalls.h | 1 +
>> kernel/sys_ni.c | 1 +
>> security/lsm_syscalls.c | 41 +++++++++++++++++++++++++++++
>> 4 files changed, 46 insertions(+)
> ..
>
>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>> index 55e8bf61ac8a..92af1fcaa654 100644
>> --- a/security/lsm_syscalls.c
>> +++ b/security/lsm_syscalls.c
>> @@ -180,3 +180,44 @@ SYSCALL_DEFINE3(lsm_get_self_attr,
>> kfree(final);
>> return rc;
>> }
>> +
>> +/**
>> + * sys_lsm_module_list - Return a list of the active security modules
>> + * @ids: the LSM module ids
>> + * @size: size of @ids, updated on return
>> + * @flags: reserved for future use, must be zero
>> + *
>> + * Returns a list of the active LSM ids. On success this function
>> + * returns the number of @ids array elements. This value may be zero
>> + * if there are no LSMs active. If @size is insufficient to contain
>> + * the return data -E2BIG is returned and @size is set to the minimum
>> + * required size. In all other cases a negative value indicating the
>> + * error is returned.
>> + */
>> +SYSCALL_DEFINE3(lsm_module_list,
>> + u32 __user *, ids,
>> + size_t __user *, size,
>> + u64, flags)
>> +{
>> + size_t total_size = lsm_active_cnt * sizeof(*ids);
>> + size_t usize;
>> + int i;
>> +
>> + if (flags)
>> + return -EINVAL;
>> +
>> + if (get_user(usize, size))
>> + return -EFAULT;
>> +
>> + if (put_user(total_size, size) != 0)
>> + return -EFAULT;
>> +
>> + if (usize < total_size)
>> + return -E2BIG;
>> +
>> + for (i = 0; i < lsm_active_cnt; i++)
>> + if (put_user(lsm_idlist[i]->id, ids++))
>> + return -EFAULT;
>> +
>> + return lsm_active_cnt;
>> +}
> Similar to my comments in 4/8, I would probably create a new LSM hook
> for this syscall so that the lsm_ctx is passed through the LSM layer
> directly to the target LSM:
>
> int security_sys_setselfattr(u64 attr, struct lsm_ctx __user *ctx,
> size_t len);
That seems like a whole lot of work when you can just look it up
in an existing table.
> --
> paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-01-09 18:07 ` [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes Casey Schaufler
2023-01-11 21:07 ` Paul Moore
@ 2023-01-12 14:40 ` Arnd Bergmann
2023-01-12 21:39 ` Paul Moore
2023-02-02 4:53 ` Serge Hallyn (shallyn)
2023-02-14 17:41 ` Mickaël Salaün
3 siblings, 1 reply; 30+ messages in thread
From: Arnd Bergmann @ 2023-01-12 14:40 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, Paul Moore, linux-security-module
Cc: jmorris, Kees Cook, john.johansen, Tetsuo Handa,
stephen.smalley.work, linux-kernel, linux-api, mic
On Mon, Jan 9, 2023, at 19:07, Casey Schaufler wrote:
> +/**
> + * struct lsm_ctx - LSM context
> + * @id: the LSM id number, see LSM_ID_XXX
> + * @flags: context specifier and LSM specific flags
> + * @ctx_len: the size of @ctx
> + * @ctx: the LSM context, a nul terminated string
> + *
> + * @ctx in a nul terminated string.
> + * (strlen(@ctx) < @ctx_len) is always true.
> + * (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
> + */
> +struct lsm_ctx {
> + __u32 id;
> + __u64 flags;
> + __kernel_size_t ctx_len;
> + __u8 ctx[];
> +};
I think this should be changed to be the same layout on
all architectures regardless of __u64 alignment and
sizeof(__kernel_size_t) differences, to avoid the need
for compat syscalls and explicit clearing of the
internal padding.
Maybe just use __u64 fields for all three integers?
Arnd
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/8] LSM: Maintain a table of LSM attribute data
2023-01-12 0:36 ` Casey Schaufler
@ 2023-01-12 20:26 ` Paul Moore
0 siblings, 0 replies; 30+ messages in thread
From: Paul Moore @ 2023-01-12 20:26 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Wed, Jan 11, 2023 at 7:36 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/11/2023 1:01 PM, Paul Moore wrote:
> > On Mon, Jan 9, 2023 at 1:07 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> As LSMs are registered add their lsm_id pointers to a table.
> >> This will be used later for attribute reporting.
> >>
> >> Determine the number of possible security modules based on
> >> their respective CONFIG options. This allows the number to be
> >> known at build time. This allows data structures and tables
> >> to use the constant.
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >> include/linux/security.h | 2 ++
> >> security/security.c | 44 +++++++++++++++++++++++++++++++++-------
> >> 2 files changed, 39 insertions(+), 7 deletions(-)
...
> >> diff --git a/security/security.c b/security/security.c
> >> index 07a8fe7f92bf..a590fa98ddd6 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -388,7 +408,7 @@ static void __init ordered_lsm_init(void)
> >> for (lsm = ordered_lsms; *lsm; lsm++)
> >> initialize_lsm(*lsm);
> >>
> >> - kfree(ordered_lsms);
> >> + init_debug("lsm count = %d\n", lsm_active_cnt);
> >> }
> > Given 86ef3c735ec8 ("LSM: Better reporting of actual LSMs at boot"),
> > is this needed?
>
> None of what comes out from lsm.debug is strictly necessary, and
> human or script can parse "initializing lsm=", but sometimes the
> number of LSMs is interesting.
I guess what I was questioning is if printing the @lsm_active_cnt
variable provides any better information that what is already provided
by commit 86ef3c735ec8? We currently print the enabled/active LSMs
with lsm.debug, printing a count seems a bit redundant to me.
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 1/8] LSM: Identify modules by more than name
2023-01-12 0:05 ` Casey Schaufler
@ 2023-01-12 20:30 ` Paul Moore
0 siblings, 0 replies; 30+ messages in thread
From: Paul Moore @ 2023-01-12 20:30 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Wed, Jan 11, 2023 at 7:05 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/11/2023 1:00 PM, Paul Moore wrote:
> > On Mon, Jan 9, 2023 at 1:07 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> Create a struct lsm_id to contain identifying information
> >> about Linux Security Modules (LSMs). At inception this contains
> >> the name of the module, an identifier associated with the security
> >> module and an integer member "attrs_used" which identifies the API
> >> related data associated with each security module. The initial set
> >> of features maps to information that has traditionaly been available
> >> in /proc/self/attr. They are documented in a new userspace-api file.
> >> Change the security_add_hooks() interface to use this structure.
> >> Change the individual modules to maintain their own struct lsm_id
> >> and pass it to security_add_hooks().
> >>
> >> The values are for LSM identifiers are defined in a new UAPI
> >> header file linux/lsm.h. Each existing LSM has been updated to
> >> include it's LSMID in the lsm_id.
> >>
> >> The LSM ID values are sequential, with the oldest module
> >> LSM_ID_CAPABILITY being the lowest value and the existing modules
> >> numbered in the order they were included in the main line kernel.
> >> This is an arbitrary convention for assigning the values, but
> >> none better presents itself. The value 0 is defined as being invalid.
> >> The values 1-99 are reserved for any special case uses which may
> >> arise in the future. This may include attributes of the LSM
> >> infrastructure itself, possibly related to namespacing or network
> >> attribute management. A special range is identified for such attributes
> >> to help reduce confusion for developers unfamiliar with LSMs.
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >> Documentation/userspace-api/index.rst | 1 +
> >> Documentation/userspace-api/lsm.rst | 55 +++++++++++++++++++++++++++
> >> include/linux/lsm_hooks.h | 18 ++++++++-
> >> include/uapi/linux/lsm.h | 55 +++++++++++++++++++++++++++
> >> security/apparmor/lsm.c | 9 ++++-
> >> security/bpf/hooks.c | 13 ++++++-
> >> security/commoncap.c | 8 +++-
> >> security/landlock/cred.c | 2 +-
> >> security/landlock/fs.c | 2 +-
> >> security/landlock/ptrace.c | 2 +-
> >> security/landlock/setup.c | 6 +++
> >> security/landlock/setup.h | 1 +
> >> security/loadpin/loadpin.c | 9 ++++-
> >> security/lockdown/lockdown.c | 8 +++-
> >> security/safesetid/lsm.c | 9 ++++-
> >> security/security.c | 12 +++---
> >> security/selinux/hooks.c | 11 +++++-
> >> security/smack/smack_lsm.c | 9 ++++-
> >> security/tomoyo/tomoyo.c | 9 ++++-
> >> security/yama/yama_lsm.c | 8 +++-
> >> 20 files changed, 226 insertions(+), 21 deletions(-)
> >> create mode 100644 Documentation/userspace-api/lsm.rst
> >> create mode 100644 include/uapi/linux/lsm.h
...
> >> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> >> index 0a5ba81f7367..6f2cabb79ec4 100644
> >> --- a/include/linux/lsm_hooks.h
> >> +++ b/include/linux/lsm_hooks.h
> >> @@ -1708,7 +1722,7 @@ extern struct security_hook_heads security_hook_heads;
> >> extern char *lsm_names;
> >>
> >> extern void security_add_hooks(struct security_hook_list *hooks, int count,
> >> - const char *lsm);
> >> + struct lsm_id *lsmid);
> > We should be able to mark @lsmid as a const, right?
>
> At this point, yes, but the const would have to come off when
> the "slot" field is added to lsm_id in the upcoming stacking patches.
> I can mark it const for now if it is important.
Ah, right. Okay, just leave it as-is then.
> >> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> >> new file mode 100644
> >> index 000000000000..61a91b7d946f
> >> --- /dev/null
> >> +++ b/include/uapi/linux/lsm.h
> >> @@ -0,0 +1,55 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> >> +/*
> >> + * Linux Security Modules (LSM) - User space API
> >> + *
> >> + * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
> >> + * Copyright (C) 2022 Intel Corporation
> >> + */
> > This file should be added to the SECURITY SUBSYSTEM section in MAINTAINERS:
> >
> > F: include/uapi/linux/lsm.h
>
> S'truth.
>
> >> +#ifndef _UAPI_LINUX_LSM_H
> >> +#define _UAPI_LINUX_LSM_H
> >> +
> >> +/*
> >> + * ID values to identify security modules.
> >> + * A system may use more than one security module.
> >> + *
> >> + * A value of 0 is considered invalid.
> >> + * Values 1-99 are reserved for future use.
> >> + * The interface is designed to extend to attributes beyond those which
> >> + * are active today. Currently all the attributes are specific to the
> >> + * individual modules. The LSM infrastructure itself has no variable state,
> >> + * but that may change. One proposal would allow loadable modules, in which
> >> + * case an attribute such as LSM_IS_LOADABLE might identify the dynamic
> >> + * modules. Another potential attribute could be which security modules is
> >> + * associated withnetwork labeling using netlabel. Another possible attribute
> >> + * could be related to stacking behavior in a namespaced environment.
> >> + * While it would be possible to intermingle the LSM infrastructure attribute
> >> + * values with the security module provided values, keeping them separate
> >> + * provides a clearer distinction.
> >> + */
> > As this is in a UAPI file, let's avoid speculation and stick to just
> > the current facts. Anything we write here with respect to the future
> > is likely to be wrong so let's not tempt fate.
>
> Sure. I'll leave the rationale to the take message.
>
> > Once I reached patch 3/8 I also realized that we may want to have more
> > than just 0/invalid as a sentinel value, or at the very least we need
> > to redefine 0 as something slightly different if for no other reason
> > than we in fs/proc/base.c. I know it seems a little trivial, but
> > since we're talking about values that will be used in the UAPI I think
> > we need to give it some thought and discussion. The only think I can
> > think of right now is to redefine 0 as "undefined", which isn't that
> > far removed from "invalid" and will not look too terrible in patch 3/8
> > - thoughts?
>
> I originally had LSM_ID_INVALID for 0, but there was an objection.
> It's not really invalid or undefined, it is reserved as not being an LSM id.
> How about LSM_ID_NALSMID or LSM_ID_NOTALSMID for 0? sort of like NAN for
> Not A Number.
>
> > With all that in mind, I would suggest something like this:
> >
> > /*
> > * ID tokens to identify Linux Security Modules (LSMs)
> > *
> > * These token values are used to uniquely identify specific LSMs
> > * in the kernel as well in the kernel's LSM userspace API.
> > *
> > * A value of zero/0 is considered undefined and should not be used
> > * outside of the kernel, values 1-99 are reserved for potential
> > * future use.
> > */
> > #define LSM_ID_UNDEF 0
>
> Fine, although I'd go with LSM_ID_NALSMID
Hmm, I had to think a little on that to figure out the NALSMID bit.
Of the two I would prefer LSM_ID_UNDEF as UNDEF tends to be a bit more
common in the kernel and would be more likely to get people thinking
in the right direction.
> >> +#define LSM_ID_CAPABILITY 100
> >> +#define LSM_ID_SELINUX 101
> >> +#define LSM_ID_SMACK 102
> >> +#define LSM_ID_TOMOYO 103
> >> +#define LSM_ID_IMA 104
> >> +#define LSM_ID_APPARMOR 105
> >> +#define LSM_ID_YAMA 106
> >> +#define LSM_ID_LOADPIN 107
> >> +#define LSM_ID_SAFESETID 108
> >> +#define LSM_ID_LOCKDOWN 109
> >> +#define LSM_ID_BPF 110
> >> +#define LSM_ID_LANDLOCK 111
> >> +
> >> +/*
> >> + * LSM_ATTR_XXX values identify the /proc/.../attr entry that the
> >> + * context represents. Not all security modules provide all of these
> >> + * values. Some security modules provide none of them.
> >> + */
> > I'd rather see text closer to this:
> >
> > /*
> > * LSM attributes
> > *
> > * The LSM_ATTR_XXX definitions identify different LSM
> > * attributes which are used in the kernel's LSM userspace API.
> > * Support for these attributes vary across the different LSMs,
> > * none are required.
> > */
>
> If you like that better I'm completely willing to adopt it.
Please.
> >> +#define LSM_ATTR_CURRENT 0x0001
> >> +#define LSM_ATTR_EXEC 0x0002
> >> +#define LSM_ATTR_FSCREATE 0x0004
> >> +#define LSM_ATTR_KEYCREATE 0x0008
> >> +#define LSM_ATTR_PREV 0x0010
> >> +#define LSM_ATTR_SOCKCREATE 0x0020
> >> +
> >> +#endif /* _UAPI_LINUX_LSM_H */
> >> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> >> index c6728a629437..63ea2a995987 100644
> >> --- a/security/apparmor/lsm.c
> >> +++ b/security/apparmor/lsm.c
> >> @@ -24,6 +24,7 @@
> >> #include <linux/zstd.h>
> >> #include <net/sock.h>
> >> #include <uapi/linux/mount.h>
> >> +#include <uapi/linux/lsm.h>
> >>
> >> #include "include/apparmor.h"
> >> #include "include/apparmorfs.h"
> >> @@ -1217,6 +1218,12 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
> >> .lbs_task = sizeof(struct aa_task_ctx),
> >> };
> >>
> >> +static struct lsm_id apparmor_lsmid __lsm_ro_after_init = {
> >> + .lsm = "apparmor",
> >> + .id = LSM_ID_APPARMOR,
> >> + .attrs_used = LSM_ATTR_CURRENT | LSM_ATTR_PREV | LSM_ATTR_EXEC,
> >> +};
> > Perhaps mark this as const in addition to ro_after_init? This applies
> > to all the other @lsm_id defs in this patch too.
>
> As I mentioned above, the lsm_id will eventually get changed during the
> registration process. I can add the const for now, knowing full well that
> it will be removed before long.
No, that's okay. When I was reviewing this I forgot that changes
would be required here for stacking.
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-01-12 1:37 ` Casey Schaufler
@ 2023-01-12 21:37 ` Paul Moore
0 siblings, 0 replies; 30+ messages in thread
From: Paul Moore @ 2023-01-12 21:37 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Wed, Jan 11, 2023 at 8:37 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/11/2023 1:07 PM, Paul Moore wrote:
> > On Mon, Jan 9, 2023 at 1:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> Create a system call lsm_get_self_attr() to provide the security
> >> module maintained attributes of the current process. Historically
> >> these attributes have been exposed to user space via entries in
> >> procfs under /proc/self/attr.
> >>
> >> Attributes are provided as a collection of lsm_ctx structures
> >> which are placed into a user supplied buffer. Each structure
> >> identifys the size of the attribute, and the attribute value.
> > ^^^
> > identifies
>
> Pluralses are hard in the vicinity of 'y's. ;)
>
> >> The format of the attribute value is defined by the security
> >> module, but will always be \0 terminated. The ctx_len value
> >> will always be strlen(ctx)+1.
> > I don't want to limit ourselves to only sending string values as
> > attributes as who knows what we might need to do in the future, and
> > the struct was originally designed to support both strings and binary
> > data. I would suggest changing the sentences above to something like
> > this:
>
> Part of creating a sane and sensible API is setting rational limitations.
> While a "context" has always allowed for a binary value it has always
> been a user friendly, nul terminated string. The one case where someone
> proposed otherwise was my "hideous" format for compound contexts, and we
> know how well that was received. If we're serious about cleaning up our
> API let's quit bending over to support something no one is using and that
> we'd prefer they didn't.
I agree that I'm in no rush to move away from a simple string format
for context values, but as we are at a unique point in time where we
get to define a new API I think it is important to ensure that it has
enough flexibility to endure whatever changes might come along in the
next 10~20 years. I mean ~20 years ago we only had one LSM and the
concept of containers/namespaces in the kernel was just a fringe
concept (if at all!).
I think it's also important to remember that we still review code
around here, and just because the struct has the necessary provisions
to *support* a binary context, it doesn't mean we are going to allow
it to be used that way without a lot of discussion first. Any move to
a binary format would have to be done in a way that doesn't break
existing applications which would likely mean either a secondary LSM
ID for those LSMs which provided a new format, a new LSM specific
flag, and in the most extreme case a LSM specific override tacked to
the end of the struct after the ctx field (possible since we now have
the total length field).
> That's my preference. Is there anyone who wants a binary interface, or any
> really good reason to provide one? I'm not going to stand fast on a strings
> only interface, but it would make it significantly cleaner if we had one.
It's not about needing or even wanting it right now, it's about
keeping it as a possible option for some point in the future when we
might need it.
> > The format of the attribute value is defined by the individual LSM,
> > with the attribute itself stored in @ctx and the length of the
> > attribute stored in @ctx_len. Both strings and arbitrary binary
> > attributes are supported, but strings should be NULL terminated and
> > @ctx_len should be equal to `strlen(@ctx) + 1`.
> >
> >> ---------------------------
> >> | __u32 id |
> >> ---------------------------
> >> | __u64 flags |
> >> ---------------------------
> >> | __kernel_size_t ctx_len |
> >> ---------------------------
> >> | __u8 ctx[ctx_len] |
> >> ---------------------------
> >> | __u32 id |
> >> ---------------------------
> >> | __u64 flags |
> >> ---------------------------
> >> | __kernel_size_t ctx_len |
> >> ---------------------------
> >> | __u8 ctx[ctx_len] |
> >> ---------------------------
> > Don't repeat the structure layout in memory twice here, it's
> > confusing. I also think it would be easier to read, and arguably more
> > useful, to simply copy the struct definition into the description
> > instead of the ASCII art column.
>
> OK on both.
>
> > Although, this has got me wondering if we should think about aligning
> > the lsm_ctx structs when we are populating them in the kernel; more on
> > this below ...
>
> As you say below, we'll need a total_len for this, but OK.
>
>
> >> ---
> >> Documentation/userspace-api/lsm.rst | 9 ++
> >> include/linux/syscalls.h | 3 +
> >> include/uapi/linux/lsm.h | 21 ++++
> >> kernel/sys_ni.c | 3 +
> >> security/Makefile | 1 +
> >> security/lsm_syscalls.c | 182 ++++++++++++++++++++++++++++
> >> 6 files changed, 219 insertions(+)
> >> create mode 100644 security/lsm_syscalls.c
> > ..
> >
> >> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> >> index 33a0ee3bcb2e..a89205c70ffa 100644
> >> --- a/include/linux/syscalls.h
> >> +++ b/include/linux/syscalls.h
> >> @@ -71,6 +71,7 @@ struct clone_args;
> >> struct open_how;
> >> struct mount_attr;
> >> struct landlock_ruleset_attr;
> >> +struct lsm_ctx;
> >> enum landlock_rule_type;
> >>
> >> #include <linux/types.h>
> >> @@ -1058,6 +1059,8 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
> >> asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
> >> unsigned long home_node,
> >> unsigned long flags);
> >> +asmlinkage long sys_lsm_get_self_attr(struct lsm_ctx *ctx, size_t *size,
> >> + int flags);
> >>
> >> /*
> >> * Architecture-specific system calls
> >> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> >> index 61a91b7d946f..8674d8c6b326 100644
> >> --- a/include/uapi/linux/lsm.h
> >> +++ b/include/uapi/linux/lsm.h
> >> @@ -9,6 +9,27 @@
> >> #ifndef _UAPI_LINUX_LSM_H
> >> #define _UAPI_LINUX_LSM_H
> >>
> >> +#include <linux/types.h>
> >> +#include <linux/unistd.h>
> >> +
> >> +/**
> >> + * struct lsm_ctx - LSM context
> >> + * @id: the LSM id number, see LSM_ID_XXX
> > As mentioned above, it occurred to me that we might want want to pad
> > out the lsm_ctx struct to ensure that the "array" of lsm_ctx is nicely
> > aligned. I know some systems used to complain about unaligned
> > accesses, and even on those that don't complain tend to be faster when
> > access are aligned. We can either implicitly align the individual
> > lsm_ctx structs or we can add a total length field (in addition to the
> > @ctx_len field) so that the padding/alignment is explicit.
> >
> > Adding an explicit total length field could have some other advantages
> > in that it, in conjunction with the existing @flags field, would allow
> > an individual LSM to "extend" the lsm_ctx struct to provide additional
> > LSM specific information in the case where the single @ctx field was
> > not sufficient. Think of it as some additional future proofing in
> > addition to explicit padding.
>
> I'm not sure whether to call it future proofing or confusion assurance,
> and I certainly wouldn't encourage such use. On the other hand, I wouldn't
> let that get in the way of the performant aligned interface, so I'm good
> with the len field in addition to the ctx_len.
Whatever you want to call it is fine by me. The more I think about
it, the more I think this will be important to have.
> >> + * @flags: context specifier and LSM specific flags
> > * @flags: LSM specific flags
> >
> > Only the individual LSM specified in @id should ever interpret @flags or @ctx.
>
> Sure.
>
> >> + * @ctx_len: the size of @ctx
> >> + * @ctx: the LSM context, a nul terminated string
> > * @ctx: the LSM context value
>
> As above, I would like to make this a string. I won't fight over it however.
>
> >> + * @ctx in a nul terminated string.
> >> + * (strlen(@ctx) < @ctx_len) is always true.
> >> + * (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
> >> + */
> > Let's rework the extra description too based on the comments above.
> > For the sake of clarity, here is what I'm currently thinking (comments
> > and feedback are encouraged):
> >
> > /**
> > * struct lsm_ctx - LSM context information
> > * @id: the LSM ID token, see LSM_ID_XXX
> > * @flags: LSM specific flags
> > * @len: length of the lsm_ctx struct + extra (?) + padding
> > * @ctx_len: the size of @ctx
> > * @ctx: the LSM context value
> > *
> > * The @len field MUST be equal to size of the lsm_ctx struct
> > * plus any additional padding and/or data placed after @ctx.
> > *
> > * In all cases @ctx_len MUST be equal to length of @ctx. If
> > * @ctx is a string value, it should be nul terminated with
> > * @ctx_len equal to `strlen(@ctx) + 1`. Binary @ctx values
> > * are supported.
> > *
> > * The @flags and @ctx fields SHOULD only be interpreted by the
> > * LSM specified by @id; they MUST be set to zero/0 when not used.
> > */
> > struct lsm_ctx {
> > __u32 id;
> > __u64 flags;
> > __kernel_size_t len;
> > __kernel_size_t ctx_len;
> > __u8 ctx[];
> > };
>
> Yes, if there's a convincing argument for binary values.
>
> >> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> >> new file mode 100644
> >> index 000000000000..55e8bf61ac8a
> >> --- /dev/null
> >> +++ b/security/lsm_syscalls.c
> >> @@ -0,0 +1,182 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * System calls implementing the Linux Security Module API.
> >> + *
> >> + * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
> >> + * Copyright (C) 2022 Intel Corporation
> >> + */
> >> +
> >> +#include <asm/current.h>
> >> +#include <linux/compiler_types.h>
> >> +#include <linux/err.h>
> >> +#include <linux/errno.h>
> >> +#include <linux/security.h>
> >> +#include <linux/stddef.h>
> >> +#include <linux/syscalls.h>
> >> +#include <linux/types.h>
> >> +#include <linux/lsm_hooks.h>
> >> +#include <uapi/linux/lsm.h>
> >> +
> >> +struct attrs_used_map {
> >> + char *name;
> >> + int attrs_used;
> > Based on the usage below it really seems like @attrs_used should just
> > be @attr, yes? That said, I'm not too bothered by it either way so if
> > you really love @attrs_used that's fine.
>
> Potato - Potatoe - Seeing the same field name in different structs at
> the same time gives me a headache, but I'm not going to quibble.
>
>
> >> +};
> >> +
> >> +static const struct attrs_used_map lsm_attr_names[] = {
> > We can probably just call this "attr_map" right? I mean the "used"
> > portion is pretty inherent in the fact that we're defining a mapping
> > :)
>
> Sure.
>
>
> >> + { .name = "current", .attrs_used = LSM_ATTR_CURRENT, },
> >> + { .name = "exec", .attrs_used = LSM_ATTR_EXEC, },
> >> + { .name = "fscreate", .attrs_used = LSM_ATTR_FSCREATE, },
> >> + { .name = "keycreate", .attrs_used = LSM_ATTR_KEYCREATE, },
> >> + { .name = "prev", .attrs_used = LSM_ATTR_PREV, },
> >> + { .name = "sockcreate", .attrs_used = LSM_ATTR_SOCKCREATE, },
> >> +};
> >> +
> >> +static int attr_used_index(u32 flags)
> > Since you can only return one index value at a time in this function
> > you can't really support multiple attribute bits set in the @flags
> > parameter so why not change the prototype to better match the required
> > usage, example:
> >
> > static int attr_index(u32 attr)
> >
> >> +{
> >> + int i;
> >> +
> >> + if (flags == 0)
> >> + return -EINVAL;
> >> +
> >> + for (i = 0; i < ARRAY_SIZE(lsm_attr_names); i++)
> >> + if ((lsm_attr_names[i].attrs_used & flags) == flags)
> >> + return i;
> > Given the above, why not simplify the above test to this:
> >
> > if (lsm_attr_name[i].attr == attr)
> > return i;
>
> That won't work in the case where an LSM supports more than one attribute.
>
> >
> > If we don't care about failing fast in the case of being passed 0 (why
> > would we?) we can define this function as follows:
> >
> > static int attr_index(u32 attr)
> > {
> > int i;
> > for (i = 0; i < ARRAY_SIZE(lsm_attr_names); i++)
> > if (lsm_attr_names[i].attr == attr)
> > return i;
> > return -EINVAL;
> > }
> >
> > If we wanted to streamline things even further we could define
> > attr_map a bit differently and drop the loop in attr_index(). Yes, it
> > does waste attr_map[0], but I don't think anyone is going to be too
> > upset about one wasted index if it scales better.
> >
> > static const struct attr_map[] = {
> > [LSM_ATTR_CURRENT] = { .name = "current", .attr = LSM_ATTR_CURRENT },
> > [LSM_ATTR_EXEC] = { .name = "exec", .attr = LSM_ATTR_EXEC },
> > ...
> > };
> >
> > static int attr_index(u32 attr)
> > {
> > if (attr == 0 || attr >= ARRAY_SIZE(attr_map))
> > return -EINVAL;
> > return attr;
> > }
> >
> > If you did this you could probably also convert attr_map from a struct
> > to a simple array of strings as the attribute value would be the
> > associated index.
> >
> >> +/**
> >> + * sys_lsm_get_self_attr - Return current task's security module attributes
> >> + * @ctx: the LSM contexts
> >> + * @size: size of @ctx, updated on return
> >> + * @flags: which attribute to return
> >> + *
> >> + * Returns the calling task's LSM contexts. On success this
> >> + * function returns the number of @ctx array elements. This value
> >> + * may be zero if there are no LSM contexts assigned. If @size is
> >> + * insufficient to contain the return data -E2BIG is returned and
> >> + * @size is set to the minimum required size. In all other cases
> >> + * a negative value indicating the error is returned.
> >> + */
> >> +SYSCALL_DEFINE3(lsm_get_self_attr,
> >> + struct lsm_ctx __user *, ctx,
> >> + size_t __user *, size,
> >> + u32, flags)
> >> +{
> >> + int i;
> >> + int rc = 0;
> >> + int len;
> >> + int attr;
> >> + int count = 0;
> >> + void *curr;
> >> + char *cp;
> >> + char *np;
> >> + char **interum_ctx;
> >> + size_t total_size = 0;
> >> + struct lsm_ctx *ip;
> >> + struct lsm_ctx *interum;
> >> + struct lsm_ctx *final = NULL;
> >> +
> >> + attr = attr_used_index(flags);
> >> + if (attr < 0)
> >> + return attr;
> >> +
> >> + interum = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
> >> + sizeof(*interum), GFP_KERNEL);
> >> + if (interum == NULL)
> >> + return -ENOMEM;
> >> + ip = interum;
> >> +
> >> + interum_ctx = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
> >> + sizeof(*interum_ctx), GFP_KERNEL);
> >> + if (interum_ctx == NULL) {
> >> + kfree(interum);
> >> + return -ENOMEM;
> >> + }
> >> +
> >> + for (i = 0; i < lsm_active_cnt; i++) {
> >> + if ((lsm_idlist[i]->attrs_used &
> >> + lsm_attr_names[attr].attrs_used) == 0)
> >> + continue;
> >> +
> >> + len = security_getprocattr(current, lsm_idlist[i]->id,
> >> + lsm_attr_names[attr].name,
> >> + &cp);
> >> + if (len <= 0)
> >> + continue;
> >> +
> >> + ip->id = lsm_idlist[i]->id;
> >> + ip->flags = lsm_attr_names[attr].attrs_used;
> >> + interum_ctx[count] = cp;
> >> +
> >> + /*
> >> + * A security module that returns a binary attribute
> >> + * will need to identify itself to prevent string
> >> + * processing.
> >> + *
> >> + * At least one security module adds a \n at the
> >> + * end of a context to make it look nicer. Change
> >> + * that to a \0 so that user space doesn't have to
> >> + * work around it.
> >> + *
> >> + * Security modules have been inconsistent about
> >> + * including the \0 terminator in the size. If it's
> >> + * not there make space for it.
> >> + *
> >> + * The length returned will reflect the length of
> >> + * the string provided by the security module, which
> >> + * may not match what getprocattr returned.
> >> + */
> >> + np = strnchr(cp, len, '\n');
> >> + if (np != NULL)
> >> + *np = '\0';
> >> + ip->ctx_len = strnlen(cp, len) + 1;
> >> + total_size += sizeof(*interum) + ip->ctx_len;
> >> + ip++;
> >> + count++;
> >> + }
> >> +
> >> + if (count == 0)
> >> + goto free_out;
> >> +
> >> + final = kzalloc(total_size, GFP_KERNEL);
> >> + if (final == NULL) {
> >> + rc = -ENOMEM;
> >> + goto free_out;
> >> + }
> >> +
> >> + curr = final;
> >> + ip = interum;
> >> + for (i = 0; i < count; i++) {
> >> + memcpy(curr, ip, sizeof(*interum));
> >> + curr += sizeof(*interum);
> >> + if (ip->ctx_len > 1)
> >> + memcpy(curr, interum_ctx[i], ip->ctx_len - 1);
> >> + curr += ip->ctx_len;
> >> + ip++;
> >> + }
> >> +
> >> + if (get_user(len, size)) {
> >> + rc = -EFAULT;
> >> + goto free_out;
> >> + }
> >> + if (total_size > len) {
> >> + rc = -ERANGE;
> >> + if (put_user(total_size, size) != 0)
> >> + rc = -EFAULT;
> >> + goto free_out;
> >> + }
> >> + if (copy_to_user(ctx, final, total_size) != 0 ||
> >> + put_user(total_size, size) != 0)
> >> + rc = -EFAULT;
> >> + else
> >> + rc = count;
> >> +
> >> +free_out:
> >> + for (i = 0; i < count; i++)
> >> + kfree(interum_ctx[i]);
> >> + kfree(interum_ctx);
> >> + kfree(interum);
> >> + kfree(final);
> >> + return rc;
> >> +}
> > Hmm. That's all kinda painful isn't it?
>
> It's not so bad as all that. Well, maybe.
>
> > I think trying to reuse
> > security_getprocattr() is doing more harm than good with all the
> > awkward handling necessary to ensure consistent output. While it's
> > nice to be able to reuse existing interfaces, one of the main
> > motivations behind the LSM syscall effort is to create a cleaner
> > interface that was designed from the beginning to support multiple
> > LSMs and provide a level of extensibility that we do not currently
> > have with the procfs interface. Hacking together all our old crap to
> > make this happen seems very wrong to me.
> >
> > With that in mind I would like to propose we introduce a new LSM hook
> > to populate a lsm_ctx struct based on a LSM_ATTR_XXX value:
> >
> > int security_sys_getselfattr(u64 attr, struct lsm_ctx __user *ctx,
> > size_t *len);
> >
> > The individual LSMs would be responsible for fully populating their
> > lsm_ctx struct specified by @ctx (note the __user tagging) and would
> > return 0 on success or negative values on failure. The maximum size
> > of the @ctx buffer would be passed in via @len and the used size would
> > be returned in @len; in the case of an too-small @ctx, -E2BIG would be
> > returned and the necessary size would be returned in @len (just as
> > discussed for the syscall itself). This way the LSM layer syscall
> > function would not need to worry about properly terminating the
> > lsm_ctx::ctx field, setting any LSM specific flags, etc. Passing the
> > __user pointer directly not only greatly simplifies the LSM layer, it
> > also has the potential to reduce the number of allocations/copies.
>
> That's going to be a lot of duplicate code to add to each LSM. Yes,
> we can do that, but I don't see it as any cleaner.
It puts the individual LSMs in charge of setting their own lsm_ctx
struct which I think will be increasingly important as time goes on
and things evolve. I'd rather make sure the infrastructure is doing
the right thing now so we can avoid having to have this exact same
discussion each time an individual LSM wants to do something a little
different with their lsm_ctx struct :)
If you need something more concrete and immediate to justify this
work, think about how a LSM could set a bit in the lsm_ctx::flags
field using the current approach of reusing the procfs hook.
> > Taking this approach should shrink the LSM layer syscall function to
> > simply needing to validate the passed @flags before looping through
> > the LSMs calling security_sys_getselfattr(). The lsm_ctx pointer
> > would need to be incremented appropriately for each call, and a total
> > length/size count would need to be maintained in case the buffer is
> > too small, but those should be relatively minor things.
>
> I think this pushed the complexity downward. If we only had one LSM
> it would be a wash. With each LSM having to provide this we're multiplying
> the complexity rather than reducing it.
>
> But again, I'll listen to reason.
I think we are all better off if the LSM layer is as thin as possible.
I believe we've seen the number of LSMs grow and the variety of
security models expand largely in part of this approach; while a thin
LSM layer may force the LSMs to do some extra work, it allows varied
approaches that might not be possible if the LSM layer enforced more
of a specific world-view.
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-01-12 14:40 ` Arnd Bergmann
@ 2023-01-12 21:39 ` Paul Moore
2023-02-14 16:48 ` Mickaël Salaün
0 siblings, 1 reply; 30+ messages in thread
From: Paul Moore @ 2023-01-12 21:39 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Casey Schaufler, casey.schaufler, linux-security-module, jmorris,
Kees Cook, john.johansen, Tetsuo Handa, stephen.smalley.work,
linux-kernel, linux-api, mic
On Thu, Jan 12, 2023 at 9:40 AM Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Jan 9, 2023, at 19:07, Casey Schaufler wrote:
> > +/**
> > + * struct lsm_ctx - LSM context
> > + * @id: the LSM id number, see LSM_ID_XXX
> > + * @flags: context specifier and LSM specific flags
> > + * @ctx_len: the size of @ctx
> > + * @ctx: the LSM context, a nul terminated string
> > + *
> > + * @ctx in a nul terminated string.
> > + * (strlen(@ctx) < @ctx_len) is always true.
> > + * (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
> > + */
> > +struct lsm_ctx {
> > + __u32 id;
> > + __u64 flags;
> > + __kernel_size_t ctx_len;
> > + __u8 ctx[];
> > +};
>
> I think this should be changed to be the same layout on
> all architectures regardless of __u64 alignment and
> sizeof(__kernel_size_t) differences, to avoid the need
> for compat syscalls and explicit clearing of the
> internal padding.
>
> Maybe just use __u64 fields for all three integers?
I have no problem with that ... the ctx[] field is variable length
anyway so keeping it as a __u8 should be fine.
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/8] LSM: Create lsm_module_list system call
2023-01-12 1:39 ` Casey Schaufler
@ 2023-01-12 21:43 ` Paul Moore
0 siblings, 0 replies; 30+ messages in thread
From: Paul Moore @ 2023-01-12 21:43 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Wed, Jan 11, 2023 at 8:39 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/11/2023 1:07 PM, Paul Moore wrote:
> > On Mon, Jan 9, 2023 at 1:09 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> Create a system call to report the list of Linux Security Modules
> >> that are active on the system. The list is provided as an array
> >> of LSM ID numbers.
> >>
> >> The calling application can use this list determine what LSM
> >> specific actions it might take. That might include chosing an
> >> output format, determining required privilege or bypassing
> >> security module specific behavior.
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >> Documentation/userspace-api/lsm.rst | 3 +++
> >> include/linux/syscalls.h | 1 +
> >> kernel/sys_ni.c | 1 +
> >> security/lsm_syscalls.c | 41 +++++++++++++++++++++++++++++
> >> 4 files changed, 46 insertions(+)
> > ..
> >
> >> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> >> index 55e8bf61ac8a..92af1fcaa654 100644
> >> --- a/security/lsm_syscalls.c
> >> +++ b/security/lsm_syscalls.c
> >> @@ -180,3 +180,44 @@ SYSCALL_DEFINE3(lsm_get_self_attr,
> >> kfree(final);
> >> return rc;
> >> }
> >> +
> >> +/**
> >> + * sys_lsm_module_list - Return a list of the active security modules
> >> + * @ids: the LSM module ids
> >> + * @size: size of @ids, updated on return
> >> + * @flags: reserved for future use, must be zero
> >> + *
> >> + * Returns a list of the active LSM ids. On success this function
> >> + * returns the number of @ids array elements. This value may be zero
> >> + * if there are no LSMs active. If @size is insufficient to contain
> >> + * the return data -E2BIG is returned and @size is set to the minimum
> >> + * required size. In all other cases a negative value indicating the
> >> + * error is returned.
> >> + */
> >> +SYSCALL_DEFINE3(lsm_module_list,
> >> + u32 __user *, ids,
> >> + size_t __user *, size,
> >> + u64, flags)
> >> +{
> >> + size_t total_size = lsm_active_cnt * sizeof(*ids);
> >> + size_t usize;
> >> + int i;
> >> +
> >> + if (flags)
> >> + return -EINVAL;
> >> +
> >> + if (get_user(usize, size))
> >> + return -EFAULT;
> >> +
> >> + if (put_user(total_size, size) != 0)
> >> + return -EFAULT;
> >> +
> >> + if (usize < total_size)
> >> + return -E2BIG;
> >> +
> >> + for (i = 0; i < lsm_active_cnt; i++)
> >> + if (put_user(lsm_idlist[i]->id, ids++))
> >> + return -EFAULT;
> >> +
> >> + return lsm_active_cnt;
> >> +}
> > Similar to my comments in 4/8, I would probably create a new LSM hook
> > for this syscall so that the lsm_ctx is passed through the LSM layer
> > directly to the target LSM:
> >
> > int security_sys_setselfattr(u64 attr, struct lsm_ctx __user *ctx,
> > size_t len);
>
> That seems like a whole lot of work when you can just look it up
> in an existing table.
D'oh! Sorry, this comment was intended for patch 6/8, the
lsm_set_self_attr() syscall patch. I agree, it would be very silly to
have a dedicated hook for lsm_module_list() :)
--
paul-moore.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 7/8] LSM: wireup Linux Security Module syscalls
2023-01-09 18:07 ` [PATCH v5 7/8] LSM: wireup Linux Security Module syscalls Casey Schaufler
@ 2023-01-13 9:31 ` Geert Uytterhoeven
0 siblings, 0 replies; 30+ messages in thread
From: Geert Uytterhoeven @ 2023-01-13 9:31 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, paul, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Mon, Jan 9, 2023 at 7:15 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> Wireup lsm_get_self_attr, lsm_set_self_attr and lsm_module_list
> system calls.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> Cc: linux-api@vger.kernel.org
> arch/m68k/kernel/syscalls/syscall.tbl | 3 +++
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> [m68k]
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-01-09 18:07 ` [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes Casey Schaufler
2023-01-11 21:07 ` Paul Moore
2023-01-12 14:40 ` Arnd Bergmann
@ 2023-02-02 4:53 ` Serge Hallyn (shallyn)
2023-02-14 17:41 ` Mickaël Salaün
3 siblings, 0 replies; 30+ messages in thread
From: Serge Hallyn (shallyn) @ 2023-02-02 4:53 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, paul, linux-security-module, jmorris, keescook,
john.johansen, penguin-kernel, stephen.smalley.work,
linux-kernel, linux-api, mic
On Mon, Jan 09, 2023 at 10:07:13AM -0800, Casey Schaufler wrote:
> +/**
> + * sys_lsm_get_self_attr - Return current task's security module attributes
> + * @ctx: the LSM contexts
> + * @size: size of @ctx, updated on return
> + * @flags: which attribute to return
> + *
> + * Returns the calling task's LSM contexts. On success this
> + * function returns the number of @ctx array elements. This value
> + * may be zero if there are no LSM contexts assigned. If @size is
> + * insufficient to contain the return data -E2BIG is returned and
Technicality: You say -E2BIG, which is -7, but in fact belog you return
-ERANGE, which is -34.
> + * @size is set to the minimum required size. In all other cases
> + * a negative value indicating the error is returned.
> + */
> +SYSCALL_DEFINE3(lsm_get_self_attr,
> + struct lsm_ctx __user *, ctx,
> + size_t __user *, size,
> + u32, flags)
> +{
> + int i;
> + int rc = 0;
> + int len;
> + int attr;
> + int count = 0;
> + void *curr;
> + char *cp;
> + char *np;
> + char **interum_ctx;
> + size_t total_size = 0;
> + struct lsm_ctx *ip;
> + struct lsm_ctx *interum;
> + struct lsm_ctx *final = NULL;
> +
> + attr = attr_used_index(flags);
> + if (attr < 0)
> + return attr;
> +
> + interum = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
> + sizeof(*interum), GFP_KERNEL);
> + if (interum == NULL)
> + return -ENOMEM;
> + ip = interum;
> +
> + interum_ctx = kzalloc(ARRAY_SIZE(lsm_attr_names) * lsm_active_cnt *
> + sizeof(*interum_ctx), GFP_KERNEL);
> + if (interum_ctx == NULL) {
> + kfree(interum);
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < lsm_active_cnt; i++) {
> + if ((lsm_idlist[i]->attrs_used &
> + lsm_attr_names[attr].attrs_used) == 0)
> + continue;
> +
> + len = security_getprocattr(current, lsm_idlist[i]->id,
> + lsm_attr_names[attr].name,
> + &cp);
> + if (len <= 0)
> + continue;
> +
> + ip->id = lsm_idlist[i]->id;
> + ip->flags = lsm_attr_names[attr].attrs_used;
> + interum_ctx[count] = cp;
> +
> + /*
> + * A security module that returns a binary attribute
> + * will need to identify itself to prevent string
> + * processing.
> + *
> + * At least one security module adds a \n at the
> + * end of a context to make it look nicer. Change
> + * that to a \0 so that user space doesn't have to
> + * work around it.
> + *
> + * Security modules have been inconsistent about
> + * including the \0 terminator in the size. If it's
> + * not there make space for it.
> + *
> + * The length returned will reflect the length of
> + * the string provided by the security module, which
> + * may not match what getprocattr returned.
> + */
> + np = strnchr(cp, len, '\n');
> + if (np != NULL)
> + *np = '\0';
> + ip->ctx_len = strnlen(cp, len) + 1;
> + total_size += sizeof(*interum) + ip->ctx_len;
> + ip++;
> + count++;
> + }
> +
> + if (count == 0)
> + goto free_out;
> +
> + final = kzalloc(total_size, GFP_KERNEL);
> + if (final == NULL) {
> + rc = -ENOMEM;
> + goto free_out;
> + }
> +
> + curr = final;
> + ip = interum;
> + for (i = 0; i < count; i++) {
> + memcpy(curr, ip, sizeof(*interum));
> + curr += sizeof(*interum);
> + if (ip->ctx_len > 1)
> + memcpy(curr, interum_ctx[i], ip->ctx_len - 1);
> + curr += ip->ctx_len;
> + ip++;
> + }
> +
> + if (get_user(len, size)) {
> + rc = -EFAULT;
> + goto free_out;
> + }
> + if (total_size > len) {
> + rc = -ERANGE;
> + if (put_user(total_size, size) != 0)
> + rc = -EFAULT;
> + goto free_out;
> + }
> + if (copy_to_user(ctx, final, total_size) != 0 ||
> + put_user(total_size, size) != 0)
> + rc = -EFAULT;
> + else
> + rc = count;
> +
> +free_out:
> + for (i = 0; i < count; i++)
> + kfree(interum_ctx[i]);
> + kfree(interum_ctx);
> + kfree(interum);
> + kfree(final);
> + return rc;
> +}
> --
> 2.39.0
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-01-12 21:39 ` Paul Moore
@ 2023-02-14 16:48 ` Mickaël Salaün
0 siblings, 0 replies; 30+ messages in thread
From: Mickaël Salaün @ 2023-02-14 16:48 UTC (permalink / raw)
To: Paul Moore, Arnd Bergmann
Cc: Casey Schaufler, casey.schaufler, linux-security-module, jmorris,
Kees Cook, john.johansen, Tetsuo Handa, stephen.smalley.work,
linux-kernel, linux-api
On 12/01/2023 22:39, Paul Moore wrote:
> On Thu, Jan 12, 2023 at 9:40 AM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, Jan 9, 2023, at 19:07, Casey Schaufler wrote:
>>> +/**
>>> + * struct lsm_ctx - LSM context
>>> + * @id: the LSM id number, see LSM_ID_XXX
>>> + * @flags: context specifier and LSM specific flags
>>> + * @ctx_len: the size of @ctx
>>> + * @ctx: the LSM context, a nul terminated string
>>> + *
>>> + * @ctx in a nul terminated string.
>>> + * (strlen(@ctx) < @ctx_len) is always true.
>>> + * (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
>>> + */
>>> +struct lsm_ctx {
>>> + __u32 id;
There is a hole here for 64-bit architectures.
>>> + __u64 flags;
>>> + __kernel_size_t ctx_len;
This is an architecture-related size, which makes the struct size
different according to architectures. We should avoid that.
>>> + __u8 ctx[];
>>> +};
I suggest packing this struct.
>>
>> I think this should be changed to be the same layout on
>> all architectures regardless of __u64 alignment and
>> sizeof(__kernel_size_t) differences, to avoid the need
>> for compat syscalls and explicit clearing of the
>> internal padding.
>>
>> Maybe just use __u64 fields for all three integers?
>
> I have no problem with that ... the ctx[] field is variable length
> anyway so keeping it as a __u8 should be fine.
>
For Landlock, we make sure the UAPI structs don't contain holes, are
packed, and have the same size for all architectures. We can check that
with pahole but for strong guarantee I suggest the same build check as
for Landlock's build_check_abi():
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/security/landlock/syscalls.c#n68
We don't need to use 64-bit fields everywhere.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-01-09 18:07 ` [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes Casey Schaufler
` (2 preceding siblings ...)
2023-02-02 4:53 ` Serge Hallyn (shallyn)
@ 2023-02-14 17:41 ` Mickaël Salaün
2023-02-14 18:06 ` Casey Schaufler
3 siblings, 1 reply; 30+ messages in thread
From: Mickaël Salaün @ 2023-02-14 17:41 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, paul, linux-security-module
Cc: jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api
On 09/01/2023 19:07, Casey Schaufler wrote:
> Create a system call lsm_get_self_attr() to provide the security
> module maintained attributes of the current process. Historically
> these attributes have been exposed to user space via entries in
> procfs under /proc/self/attr.
>
> Attributes are provided as a collection of lsm_ctx structures
> which are placed into a user supplied buffer. Each structure
> identifys the size of the attribute, and the attribute value.
> The format of the attribute value is defined by the security
> module, but will always be \0 terminated. The ctx_len value
> will always be strlen(ctx)+1.
>
> ---------------------------
> | __u32 id |
> ---------------------------
> | __u64 flags |
> ---------------------------
> | __kernel_size_t ctx_len |
> ---------------------------
> | __u8 ctx[ctx_len] |
> ---------------------------
> | __u32 id |
> ---------------------------
> | __u64 flags |
> ---------------------------
> | __kernel_size_t ctx_len |
> ---------------------------
> | __u8 ctx[ctx_len] |
> ---------------------------
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> Documentation/userspace-api/lsm.rst | 9 ++
> include/linux/syscalls.h | 3 +
> include/uapi/linux/lsm.h | 21 ++++
> kernel/sys_ni.c | 3 +
> security/Makefile | 1 +
> security/lsm_syscalls.c | 182 ++++++++++++++++++++++++++++
> 6 files changed, 219 insertions(+)
> create mode 100644 security/lsm_syscalls.c
For new files (e.g. lsm_syscalls.c), it would be nice to auto-format
them with clang-format. It helps maintenance by keeping a consistent
style across commits, which should also help backports, and it avoids
nitpicking on style issues.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes
2023-02-14 17:41 ` Mickaël Salaün
@ 2023-02-14 18:06 ` Casey Schaufler
0 siblings, 0 replies; 30+ messages in thread
From: Casey Schaufler @ 2023-02-14 18:06 UTC (permalink / raw)
To: Mickaël Salaün, casey.schaufler, paul, linux-security-module
Cc: jmorris, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, linux-api, casey
On 2/14/2023 9:41 AM, Mickaël Salaün wrote:
>
> On 09/01/2023 19:07, Casey Schaufler wrote:
>> Create a system call lsm_get_self_attr() to provide the security
>> module maintained attributes of the current process. Historically
>> these attributes have been exposed to user space via entries in
>> procfs under /proc/self/attr.
>>
>> Attributes are provided as a collection of lsm_ctx structures
>> which are placed into a user supplied buffer. Each structure
>> identifys the size of the attribute, and the attribute value.
>> The format of the attribute value is defined by the security
>> module, but will always be \0 terminated. The ctx_len value
>> will always be strlen(ctx)+1.
>>
>> ---------------------------
>> | __u32 id |
>> ---------------------------
>> | __u64 flags |
>> ---------------------------
>> | __kernel_size_t ctx_len |
>> ---------------------------
>> | __u8 ctx[ctx_len] |
>> ---------------------------
>> | __u32 id |
>> ---------------------------
>> | __u64 flags |
>> ---------------------------
>> | __kernel_size_t ctx_len |
>> ---------------------------
>> | __u8 ctx[ctx_len] |
>> ---------------------------
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> Documentation/userspace-api/lsm.rst | 9 ++
>> include/linux/syscalls.h | 3 +
>> include/uapi/linux/lsm.h | 21 ++++
>> kernel/sys_ni.c | 3 +
>> security/Makefile | 1 +
>> security/lsm_syscalls.c | 182 ++++++++++++++++++++++++++++
>> 6 files changed, 219 insertions(+)
>> create mode 100644 security/lsm_syscalls.c
>
> For new files (e.g. lsm_syscalls.c), it would be nice to auto-format
> them with clang-format. It helps maintenance by keeping a consistent
> style across commits, which should also help backports, and it avoids
> nitpicking on style issues.
Good idea.
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2023-02-14 18:06 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20230109180717.58855-1-casey.ref@schaufler-ca.com>
2023-01-09 18:07 ` [PATCH v5 0/8] LSM: Three basic syscalls Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 1/8] LSM: Identify modules by more than name Casey Schaufler
2023-01-11 21:00 ` Paul Moore
2023-01-12 0:05 ` Casey Schaufler
2023-01-12 20:30 ` Paul Moore
2023-01-09 18:07 ` [PATCH v5 2/8] LSM: Maintain a table of LSM attribute data Casey Schaufler
2023-01-11 21:01 ` Paul Moore
2023-01-12 0:36 ` Casey Schaufler
2023-01-12 20:26 ` Paul Moore
2023-01-09 18:07 ` [PATCH v5 3/8] proc: Use lsmids instead of lsm names for attrs Casey Schaufler
2023-01-11 21:01 ` Paul Moore
2023-01-12 0:37 ` Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self attributes Casey Schaufler
2023-01-11 21:07 ` Paul Moore
2023-01-12 1:37 ` Casey Schaufler
2023-01-12 21:37 ` Paul Moore
2023-01-12 14:40 ` Arnd Bergmann
2023-01-12 21:39 ` Paul Moore
2023-02-14 16:48 ` Mickaël Salaün
2023-02-02 4:53 ` Serge Hallyn (shallyn)
2023-02-14 17:41 ` Mickaël Salaün
2023-02-14 18:06 ` Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 5/8] LSM: Create lsm_module_list system call Casey Schaufler
2023-01-11 21:07 ` Paul Moore
2023-01-12 1:39 ` Casey Schaufler
2023-01-12 21:43 ` Paul Moore
2023-01-09 18:07 ` [PATCH v5 6/8] LSM: lsm_set_self_attr syscall for LSM self attributes Casey Schaufler
2023-01-09 18:07 ` [PATCH v5 7/8] LSM: wireup Linux Security Module syscalls Casey Schaufler
2023-01-13 9:31 ` Geert Uytterhoeven
2023-01-09 18:07 ` [PATCH v5 8/8] LSM: selftests for " Casey Schaufler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).