On 2/3/20 10:42 AM, Richard Haines wrote:
On Mon, 2020-02-03 at 10:29 -0500, Stephen Smalley wrote:
On 2/3/20 9:13 AM, Richard Haines wrote:
Add a new 'key_perms' policy capability and support for the
additional
key permissions: inval, revoke, join, clear
For future reference, subject line should have [PATCH] prefix; git
send-email will do this for you automatically. Also, the subsystem
prefix would conventionally be "keys,selinux:" to indicates that it
touches the keys and selinux subsystems, no need for "security/".
Also fixes JOIN -> LINK permission translation when policy
capability 'keys_perms' = 0;
The current "setattr" perm name remains and is used for
KEY_NEED_SETSEC.
This gives the following permissions for the 'key' class:
create Create a key or keyring.
view View attributes.
read Read contents.
write Update or modify.
search Search (keyring) or find (key).
link Link a key into the keyring.
setattr kernel < 5.X Change permissions on a keyring.
kernel >= 5.X Set owner, group, ACL.
inval Invalidate key.
revoke Revoke key.
join Join keyring as session.
clear Clear a keyring.
Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
---
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index a0c97d4d8251..65e1c0c3feb1 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1592,7 +1592,8 @@ long keyctl_session_to_parent(void)
struct cred *cred;
int ret;
- keyring_r = lookup_user_key(KEY_SPEC_SESSION_KEYRING, 0,
KEY_NEED_JOIN);
+ keyring_r = lookup_user_key(KEY_SPEC_SESSION_KEYRING, 0,
+ KEY_NEED_PARENT_JOIN);
I'm unclear on how this works with the regular key permission
checking
(not SELinux). There is no KEY_ACE_PARENT_JOIN permission AFAICT
and
the check would fail if the regular permissions were only
KEY_ACE_JOIN.
What we need is an additional flag that will get ignored by
key_permission() for its permission checking but signify to SELinux
that
different handling is required here.
David will need to answer this.
Until this gets resolved, this patch must not go into mainline.
Otherwise we're looking at a potential userspace ABI issue when/if it
gets resolved.
diff --git a/security/selinux/include/security.h
b/security/selinux/include/security.h
index ae840634e3c7..6b264b6d9d31 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -79,6 +79,7 @@ enum {
POLICYDB_CAPABILITY_ALWAYSNETWORK,
POLICYDB_CAPABILITY_CGROUPSECLABEL,
POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
+ POLICYDB_CAPABILITY_KEYPERMS,
__POLICYDB_CAPABILITY_MAX
};
#define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
@@ -178,6 +179,13 @@ static inline bool
selinux_policycap_nnp_nosuid_transition(void)
This will collide with
https://lore.kernel.org/selinux/20200128191656.111902-1-cgzones@xxxxxxxxxxxxxx/
so we'll have to sort out which one goes first.
Already noted this elsewhere but for the sake of ensuring it is noted in
this thread too: the key_perms capability will need to be added after
the genfs_seclabel_symlinks capability since the latter is already
queued on selinux/next and the corresponding libsepol patch has been
merged upstream.
