| Message ID | 20200203141343.29133-1-richard_c_haines@btinternet.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | security/selinux: Add support for new key permissions | expand |
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@btinternet.com> > Signed-off-by: David Howells <dhowells@redhat.com> > --- > include/linux/key.h | 3 +- > security/keys/keyctl.c | 3 +- > security/selinux/hooks.c | 73 ++++++++++++++++++++++++++++++------- > security/selinux/include/classmap.h | 2 +- > security/selinux/include/security.h | 8 ++++ > security/selinux/ss/services.c | 3 +- > 6 files changed, 74 insertions(+), 18 deletions(-) > > diff --git a/include/linux/key.h b/include/linux/key.h > index 24c69457783f..ddfc0709569b 100644 > --- a/include/linux/key.h > +++ b/include/linux/key.h > @@ -435,7 +435,8 @@ extern void key_free_user_ns(struct user_namespace *); > #define KEY_NEED_REVOKE 0x080 /* Require permission to revoke key */ > #define KEY_NEED_JOIN 0x100 /* Require permission to join keyring as session */ > #define KEY_NEED_CLEAR 0x200 /* Require permission to clear a keyring */ > -#define KEY_NEED_ALL 0x3ff > +#define KEY_NEED_PARENT_JOIN 0x400 /* Require permission to impose keyring on parent */ > +#define KEY_NEED_ALL 0x7ff > > #define OLD_KEY_NEED_SETATTR 0x20 /* Used to be Require permission to change attributes */ > > 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. > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index c8db5235b01f..a499bd7d9777 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -6565,18 +6566,62 @@ static int selinux_key_permission(key_ref_t key_ref, > if (perm == 0) > return 0; > > - oldstyle_perm = perm & (KEY_NEED_VIEW | KEY_NEED_READ | KEY_NEED_WRITE | > - KEY_NEED_SEARCH | KEY_NEED_LINK); > - if (perm & KEY_NEED_SETSEC) > - oldstyle_perm |= OLD_KEY_NEED_SETATTR; > - if (perm & KEY_NEED_INVAL) > - oldstyle_perm |= KEY_NEED_SEARCH; > - if (perm & KEY_NEED_REVOKE && !(perm & OLD_KEY_NEED_SETATTR)) > - oldstyle_perm |= KEY_NEED_WRITE; > - if (perm & KEY_NEED_JOIN) > - oldstyle_perm |= KEY_NEED_SEARCH; > - if (perm & KEY_NEED_CLEAR) > - oldstyle_perm |= KEY_NEED_WRITE; > + if (selinux_policycap_key_perms()) { > + while (count) { > + switch_perm = bit & perm; > + switch (switch_perm) { > + case KEY_NEED_VIEW: > + key_perm |= KEY__VIEW; > + break; > + case KEY_NEED_READ: > + key_perm |= KEY__READ; > + break; > + case KEY_NEED_WRITE: > + key_perm |= KEY__WRITE; > + break; > + case KEY_NEED_SEARCH: > + key_perm |= KEY__SEARCH; > + break; > + case KEY_NEED_LINK: > + key_perm |= KEY__LINK; > + break; > + case KEY_NEED_SETSEC: > + key_perm |= KEY__SETATTR; > + break; > + case KEY_NEED_INVAL: > + key_perm |= KEY__INVAL; > + break; > + case KEY_NEED_REVOKE: > + key_perm |= KEY__REVOKE; > + break; > + case KEY_NEED_JOIN: > + case KEY_NEED_PARENT_JOIN: > + key_perm |= KEY__JOIN; > + break; > + case KEY_NEED_CLEAR: > + key_perm |= KEY__CLEAR; > + break; > + } > + bit <<= 1; > + count >>= 1; > + } So I assume then that it is unreasonable to have key_permission() limit its inputs to single-permission-at-a-time checks and therefore we have to permit multiple permissions? David? If not, is there any concern about the performance overhead of this loop and if so should we optimize for the common case of a single permission? > 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@googlemail.com/ so we'll have to sort out which one goes first.
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@btinternet.com> > > Signed-off-by: David Howells <dhowells@redhat.com> > > --- > > include/linux/key.h | 3 +- > > security/keys/keyctl.c | 3 +- > > security/selinux/hooks.c | 73 > > ++++++++++++++++++++++++++++++------- > > security/selinux/include/classmap.h | 2 +- > > security/selinux/include/security.h | 8 ++++ > > security/selinux/ss/services.c | 3 +- > > 6 files changed, 74 insertions(+), 18 deletions(-) > > > > diff --git a/include/linux/key.h b/include/linux/key.h > > index 24c69457783f..ddfc0709569b 100644 > > --- a/include/linux/key.h > > +++ b/include/linux/key.h > > @@ -435,7 +435,8 @@ extern void key_free_user_ns(struct > > user_namespace *); > > #define KEY_NEED_REVOKE 0x080 /* Require permission to > > revoke key */ > > #define KEY_NEED_JOIN 0x100 /* Require permission to > > join keyring as session */ > > #define KEY_NEED_CLEAR 0x200 /* Require permission to > > clear a keyring */ > > -#define KEY_NEED_ALL 0x3ff > > +#define KEY_NEED_PARENT_JOIN 0x400 /* Require permission to impose > > keyring on parent */ > > +#define KEY_NEED_ALL 0x7ff > > > > #define OLD_KEY_NEED_SETATTR 0x20 /* Used to be Require > > permission to change attributes */ > > > > 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. > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index c8db5235b01f..a499bd7d9777 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -6565,18 +6566,62 @@ static int selinux_key_permission(key_ref_t > > key_ref, > > if (perm == 0) > > return 0; > > > > - oldstyle_perm = perm & (KEY_NEED_VIEW | KEY_NEED_READ | > > KEY_NEED_WRITE | > > - KEY_NEED_SEARCH | KEY_NEED_LINK); > > - if (perm & KEY_NEED_SETSEC) > > - oldstyle_perm |= OLD_KEY_NEED_SETATTR; > > - if (perm & KEY_NEED_INVAL) > > - oldstyle_perm |= KEY_NEED_SEARCH; > > - if (perm & KEY_NEED_REVOKE && !(perm & OLD_KEY_NEED_SETATTR)) > > - oldstyle_perm |= KEY_NEED_WRITE; > > - if (perm & KEY_NEED_JOIN) > > - oldstyle_perm |= KEY_NEED_SEARCH; > > - if (perm & KEY_NEED_CLEAR) > > - oldstyle_perm |= KEY_NEED_WRITE; > > + if (selinux_policycap_key_perms()) { > > + while (count) { > > + switch_perm = bit & perm; > > + switch (switch_perm) { > > + case KEY_NEED_VIEW: > > + key_perm |= KEY__VIEW; > > + break; > > + case KEY_NEED_READ: > > + key_perm |= KEY__READ; > > + break; > > + case KEY_NEED_WRITE: > > + key_perm |= KEY__WRITE; > > + break; > > + case KEY_NEED_SEARCH: > > + key_perm |= KEY__SEARCH; > > + break; > > + case KEY_NEED_LINK: > > + key_perm |= KEY__LINK; > > + break; > > + case KEY_NEED_SETSEC: > > + key_perm |= KEY__SETATTR; > > + break; > > + case KEY_NEED_INVAL: > > + key_perm |= KEY__INVAL; > > + break; > > + case KEY_NEED_REVOKE: > > + key_perm |= KEY__REVOKE; > > + break; > > + case KEY_NEED_JOIN: > > + case KEY_NEED_PARENT_JOIN: > > + key_perm |= KEY__JOIN; > > + break; > > + case KEY_NEED_CLEAR: > > + key_perm |= KEY__CLEAR; > > + break; > > + } > > + bit <<= 1; > > + count >>= 1; > > + } > > So I assume then that it is unreasonable to have key_permission() > limit > its inputs to single-permission-at-a-time checks and therefore we > have > to permit multiple permissions? David? If not, is there any concern > about the performance overhead of this loop and if so should we > optimize > for the common case of a single permission? I have a single perm patch as well if that is required > > > 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@googlemail.com/ > so we'll have to sort out which one goes first.
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@btinternet.com> >>> Signed-off-by: David Howells <dhowells@redhat.com> >>> --- >>> 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@googlemail.com/ >> 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.
On Thu, 2020-02-06 at 11:03 -0500, Stephen Smalley wrote: > 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@btinternet.com> > > > > Signed-off-by: David Howells <dhowells@redhat.com> > > > > --- > > > > 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@googlemail.com/ > > > 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. I'm still waiting for David to get back regarding the permission loop. Anyway I'll update the patch as soon as I see POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS in Paul's https://github.com/SELinuxProject/selinux-kernel/tree/next
diff --git a/include/linux/key.h b/include/linux/key.h index 24c69457783f..ddfc0709569b 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -435,7 +435,8 @@ extern void key_free_user_ns(struct user_namespace *); #define KEY_NEED_REVOKE 0x080 /* Require permission to revoke key */ #define KEY_NEED_JOIN 0x100 /* Require permission to join keyring as session */ #define KEY_NEED_CLEAR 0x200 /* Require permission to clear a keyring */ -#define KEY_NEED_ALL 0x3ff +#define KEY_NEED_PARENT_JOIN 0x400 /* Require permission to impose keyring on parent */ +#define KEY_NEED_ALL 0x7ff #define OLD_KEY_NEED_SETATTR 0x20 /* Used to be Require permission to change attributes */ 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); if (IS_ERR(keyring_r)) return PTR_ERR(keyring_r); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c8db5235b01f..a499bd7d9777 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6556,7 +6556,8 @@ static int selinux_key_permission(key_ref_t key_ref, { struct key *key; struct key_security_struct *ksec; - unsigned oldstyle_perm; + unsigned int key_perm = 0, switch_perm = 0; + int bit = 1, count = KEY_NEED_ALL; u32 sid; /* if no specific permissions are requested, we skip the @@ -6565,18 +6566,62 @@ static int selinux_key_permission(key_ref_t key_ref, if (perm == 0) return 0; - oldstyle_perm = perm & (KEY_NEED_VIEW | KEY_NEED_READ | KEY_NEED_WRITE | - KEY_NEED_SEARCH | KEY_NEED_LINK); - if (perm & KEY_NEED_SETSEC) - oldstyle_perm |= OLD_KEY_NEED_SETATTR; - if (perm & KEY_NEED_INVAL) - oldstyle_perm |= KEY_NEED_SEARCH; - if (perm & KEY_NEED_REVOKE && !(perm & OLD_KEY_NEED_SETATTR)) - oldstyle_perm |= KEY_NEED_WRITE; - if (perm & KEY_NEED_JOIN) - oldstyle_perm |= KEY_NEED_SEARCH; - if (perm & KEY_NEED_CLEAR) - oldstyle_perm |= KEY_NEED_WRITE; + if (selinux_policycap_key_perms()) { + while (count) { + switch_perm = bit & perm; + switch (switch_perm) { + case KEY_NEED_VIEW: + key_perm |= KEY__VIEW; + break; + case KEY_NEED_READ: + key_perm |= KEY__READ; + break; + case KEY_NEED_WRITE: + key_perm |= KEY__WRITE; + break; + case KEY_NEED_SEARCH: + key_perm |= KEY__SEARCH; + break; + case KEY_NEED_LINK: + key_perm |= KEY__LINK; + break; + case KEY_NEED_SETSEC: + key_perm |= KEY__SETATTR; + break; + case KEY_NEED_INVAL: + key_perm |= KEY__INVAL; + break; + case KEY_NEED_REVOKE: + key_perm |= KEY__REVOKE; + break; + case KEY_NEED_JOIN: + case KEY_NEED_PARENT_JOIN: + key_perm |= KEY__JOIN; + break; + case KEY_NEED_CLEAR: + key_perm |= KEY__CLEAR; + break; + } + bit <<= 1; + count >>= 1; + } + } else { + key_perm = perm & (KEY_NEED_VIEW | KEY_NEED_READ | + KEY_NEED_WRITE | KEY_NEED_SEARCH | + KEY_NEED_LINK); + if (perm & KEY_NEED_PARENT_JOIN) + key_perm |= KEY_NEED_LINK; + if (perm & KEY_NEED_SETSEC) + key_perm |= OLD_KEY_NEED_SETATTR; + if (perm & KEY_NEED_INVAL) + key_perm |= KEY_NEED_SEARCH; + if (perm & KEY_NEED_REVOKE && !(perm & OLD_KEY_NEED_SETATTR)) + key_perm |= KEY_NEED_WRITE; + if (perm & KEY_NEED_JOIN) + key_perm |= KEY_NEED_SEARCH; + if (perm & KEY_NEED_CLEAR) + key_perm |= KEY_NEED_WRITE; + } sid = cred_sid(cred); @@ -6584,7 +6629,7 @@ static int selinux_key_permission(key_ref_t key_ref, ksec = key->security; return avc_has_perm(&selinux_state, - sid, ksec->sid, SECCLASS_KEY, oldstyle_perm, NULL); + sid, ksec->sid, SECCLASS_KEY, key_perm, NULL); } static int selinux_key_getsecurity(struct key *key, char **_buffer) diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 7db24855e12d..038374bef526 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -159,7 +159,7 @@ struct security_class_mapping secclass_map[] = { { "send", "recv", "relabelto", "forward_in", "forward_out", NULL } }, { "key", { "view", "read", "write", "search", "link", "setattr", "create", - NULL } }, + "inval", "revoke", "join", "clear", NULL } }, { "dccp_socket", { COMMON_SOCK_PERMS, "node_bind", "name_connect", NULL } }, 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) return state->policycap[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION]; } +static inline bool selinux_policycap_key_perms(void) +{ + struct selinux_state *state = &selinux_state; + + return state->policycap[POLICYDB_CAPABILITY_KEYPERMS]; +} + int security_mls_enabled(struct selinux_state *state); int security_load_policy(struct selinux_state *state, void *data, size_t len); diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index a5813c7629c1..b31dcd54cde9 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -73,7 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { "extended_socket_class", "always_check_network", "cgroup_seclabel", - "nnp_nosuid_transition" + "nnp_nosuid_transition", + "key_perms" }; static struct selinux_ss selinux_ss;