On Mon, May 17, 2021 at 3:53 PM Casey Schaufler [off-list ref] wrote:
On 5/14/2021 12:23 PM, Kees Cook wrote:
quoted
On Thu, May 13, 2021 at 01:07:56PM -0700, Casey Schaufler wrote:
quoted
Create a new entry "interface_lsm" in the procfs attr directory for
controlling which LSM security information is displayed for a
process. A process can only read or write its own display value.
The name of an active LSM that supplies hooks for
human readable data may be written to "interface_lsm" to set the
value. The name of the LSM currently in use can be read from
"interface_lsm". At this point there can only be one LSM capable
of display active. A helper function lsm_task_ilsm() is
provided to get the interface lsm slot for a task_struct.
Setting the "interface_lsm" requires that all security modules using
setprocattr hooks allow the action. Each security module is
responsible for defining its policy.
AppArmor hook provided by John Johansen [off-list ref]
SELinux hook provided by Stephen Smalley [off-list ref]
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: Kees Cook <redacted>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: John Johansen <john.johansen@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <redacted>
Cc: <redacted>
---
.../ABI/testing/procfs-attr-lsm_display | 22 +++
Documentation/security/lsm.rst | 14 ++
fs/proc/base.c | 1 +
include/linux/lsm_hooks.h | 17 ++
security/apparmor/include/apparmor.h | 3 +-
security/apparmor/lsm.c | 32 ++++
security/security.c | 166 ++++++++++++++++--
security/selinux/hooks.c | 11 ++
security/selinux/include/classmap.h | 2 +-
security/smack/smack_lsm.c | 7 +
10 files changed, 256 insertions(+), 19 deletions(-)
create mode 100644 Documentation/ABI/testing/procfs-attr-lsm_display
...
quoted
quoted
@@ -2171,23 +2203,110 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
char **value)
{
struct security_hook_list *hp;
+ int ilsm = lsm_task_ilsm(current);
+ int slot = 0;
+
+ if (!strcmp(name, "interface_lsm")) {
+ /*
+ * lsm_slot will be 0 if there are no displaying modules.
+ */
+ if (lsm_slot == 0)
+ return -EINVAL;
+
+ /*
+ * Only allow getting the current process' interface_lsm.
+ * There are too few reasons to get another process'
+ * interface_lsm and too many LSM policy issues.
+ */
+ if (current != p)
+ return -EINVAL;
... but context isn't established by just checking "current", as this
file handle may have been given to another process.
I suspect the security_get/setprocattr needs to gain a pointer to "file"
so that the f_cred struct can be examined[1] (i.e. compare opener
against reader/writer).
[1] https://www.kernel.org/doc/html/latest/security/credentials.html#open-file-credentials
It's not credentials being checked here. The check is whether the task that
would be affected is "current". Process A can't open /proc/B/attr/interface_lsm
with write access. The only process that can open it for write access is B.
If process B opens /proc/B/attr/interface_lsm for write access it could send
the file handle to process A, but process A can't write to the file because
(current != p) that is, (A != B).
Agreed.
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul moore
www.paul-moore.com