* [RFC PATCH 0/1] selinux: Add support for new key permissions
@ 2020-02-20 18:10 Richard Haines
2020-02-20 18:10 ` [RFC PATCH 1/1] " Richard Haines
0 siblings, 1 reply; 8+ messages in thread
From: Richard Haines @ 2020-02-20 18:10 UTC (permalink / raw)
To: dhowells; +Cc: selinux, sds, paul, Richard Haines
I've been running this patch on my system for a few weeks now with no
problems, therefore I conclude that the key service only passes one
permission at a time.
Listed below is the output from the kernel logs regarding the permission
translations.
key_perms polcap = 0
entry_perm: 0x0001 exit_perm: 0x0001 view
entry_perm: 0x0002 exit_perm: 0x0002 read
entry_perm: 0x0004 exit_perm: 0x0004 write
entry_perm: 0x0008 exit_perm: 0x0008 search
entry_perm: 0x0010 exit_perm: 0x0010 link
entry_perm: 0x0020 exit_perm: 0x0020 setsec
entry_perm: 0x0040 exit_perm: 0x0008 inval/search
entry_perm: 0x0080 exit_perm: 0x0004 revoke/write
entry_perm: 0x0100 exit_perm: 0x0008 join/search
entry_perm: 0x0200 exit_perm: 0x0004 clear/write
entry_perm: 0x0400 exit_perm: 0x0010 parent_join/link
key_perms polcap = 1
entry_perm: 0x0001 exit_perm: 0x0001 view
entry_perm: 0x0002 exit_perm: 0x0002 read
entry_perm: 0x0004 exit_perm: 0x0004 write
entry_perm: 0x0008 exit_perm: 0x0008 search
entry_perm: 0x0010 exit_perm: 0x0010 link
entry_perm: 0x0020 exit_perm: 0x0020 setsec
entry_perm: 0x0040 exit_perm: 0x0080 inval
entry_perm: 0x0080 exit_perm: 0x0100 revoke
entry_perm: 0x0100 exit_perm: 0x0200 join
entry_perm: 0x0200 exit_perm: 0x0400 clear
entry_perm: 0x0400 exit_perm: 0x0200 parent_join/join
<--- key.h ----> <-- av_permissions.h -->
KEY_NEED_VIEW 0x001 KEY__VIEW 0x00000001U
KEY_NEED_READ 0x002 KEY__READ 0x00000002U
KEY_NEED_WRITE 0x004 KEY__WRITE 0x00000004U
KEY_NEED_SEARCH 0x008 KEY__SEARCH 0x00000008U
KEY_NEED_LINK 0x010 KEY__LINK 0x00000010U
KEY_NEED_SETSEC 0x020 KEY__SETATTR 0x00000020U
KEY_NEED_INVAL 0x040 KEY__INVAL 0x00000080U
KEY_NEED_REVOKE 0x080 KEY__REVOKE 0x00000100U
KEY_NEED_JOIN 0x100 KEY__JOIN 0x00000200U
KEY_NEED_CLEAR 0x200 KEY__CLEAR 0x00000400U
KEY_NEED_PARENT_JOIN 0x400 KEY__JOIN 0x00000200U
Richard Haines (1):
selinux: Add support for new key permissions
security/selinux/hooks.c | 123 ++++++++++++++++------------
security/selinux/include/security.h | 10 +--
security/selinux/ss/services.c | 4 +-
3 files changed, 76 insertions(+), 61 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/1] selinux: Add support for new key permissions
2020-02-20 18:10 [RFC PATCH 0/1] selinux: Add support for new key permissions Richard Haines
@ 2020-02-20 18:10 ` Richard Haines
[not found] ` <c5d6ae72-4f5d-fe41-c025-0eaa4616b7eb@tycho.nsa.gov>
2020-02-28 15:55 ` David Howells
0 siblings, 2 replies; 8+ messages in thread
From: Richard Haines @ 2020-02-20 18:10 UTC (permalink / raw)
To: dhowells; +Cc: selinux, sds, paul, Richard Haines
Add a new 'key_perms' policy capability and support for the additional
key permissions: inval, revoke, join, clear
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 < current version: Change permissions on a keyring.
kernel >= current version: 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>
---
security/selinux/hooks.c | 123 ++++++++++++++++------------
security/selinux/include/security.h | 10 +--
security/selinux/ss/services.c | 4 +-
3 files changed, 76 insertions(+), 61 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 46a8f3e7d..af179442c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6538,8 +6538,7 @@ static int selinux_key_permission(key_ref_t key_ref,
{
struct key *key;
struct key_security_struct *ksec;
- unsigned int key_perm = 0, switch_perm = 0;
- int bit = 1, count = KEY_NEED_ALL;
+ unsigned int key_perm = 0;
u32 sid;
/* if no specific permissions are requested, we skip the
@@ -6549,60 +6548,73 @@ static int selinux_key_permission(key_ref_t key_ref,
return 0;
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;
+ 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;
+ default:
+ pr_err("BUG pcap = 1 entry_perm: 0x%04x\n", perm);
+ BUG();
+ break;
}
} 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;
+ switch (perm) {
+ case KEY_NEED_VIEW:
+ key_perm = KEY_NEED_VIEW;
+ break;
+ case KEY_NEED_READ:
+ key_perm = KEY_NEED_READ;
+ break;
+ case KEY_NEED_WRITE:
+ case KEY_NEED_REVOKE:
+ case KEY_NEED_CLEAR:
+ key_perm = KEY_NEED_WRITE;
+ break;
+ case KEY_NEED_SEARCH:
+ case KEY_NEED_INVAL:
+ case KEY_NEED_JOIN:
+ key_perm = KEY_NEED_SEARCH;
+ break;
+ case KEY_NEED_LINK:
+ case KEY_NEED_PARENT_JOIN:
+ key_perm = KEY_NEED_LINK;
+ break;
+ case KEY_NEED_SETSEC:
+ key_perm = OLD_KEY_NEED_SETATTR;
+ break;
+ default:
+ pr_err("BUG pcap = 0 entry_perm: 0x%04x\n", perm);
+ BUG();
+ break;
+ }
}
sid = cred_sid(cred);
@@ -6610,6 +6622,9 @@ static int selinux_key_permission(key_ref_t key_ref,
key = key_ref_to_ptr(key_ref);
ksec = key->security;
+ pr_info("entry_perm: 0x%04x exit_perm: 0x%04x\n",
+ perm, key_perm);
+
return avc_has_perm(&selinux_state,
sid, ksec->sid, SECCLASS_KEY, key_perm, NULL);
}
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index cba9610b8..c0998e79d 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -79,8 +79,8 @@ enum {
POLICYDB_CAPABILITY_ALWAYSNETWORK,
POLICYDB_CAPABILITY_CGROUPSECLABEL,
POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
- POLICYDB_CAPABILITY_KEYPERMS,
POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS,
+ POLICYDB_CAPABILITY_KEYPERMS,
__POLICYDB_CAPABILITY_MAX
};
#define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
@@ -215,18 +215,18 @@ 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)
+static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
{
struct selinux_state *state = &selinux_state;
- return state->policycap[POLICYDB_CAPABILITY_KEYPERMS];
+ return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
}
-static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
+static inline bool selinux_policycap_key_perms(void)
{
struct selinux_state *state = &selinux_state;
- return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
+ return state->policycap[POLICYDB_CAPABILITY_KEYPERMS];
}
int security_mls_enabled(struct selinux_state *state);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d4a05f34d..6efc86c47 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -73,8 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
"always_check_network",
"cgroup_seclabel",
"nnp_nosuid_transition",
- "key_perms",
- "genfs_seclabel_symlinks"
+ "genfs_seclabel_symlinks",
+ "key_perms"
};
static struct selinux_ss selinux_ss;
--
2.24.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] selinux: Add support for new key permissions
[not found] ` <c5d6ae72-4f5d-fe41-c025-0eaa4616b7eb@tycho.nsa.gov>
@ 2020-02-21 0:03 ` Paul Moore
2020-02-21 10:55 ` Richard Haines
2020-02-28 15:52 ` David Howells
2 siblings, 0 replies; 8+ messages in thread
From: Paul Moore @ 2020-02-21 0:03 UTC (permalink / raw)
To: dhowells; +Cc: Richard Haines, selinux, Stephen Smalley
On Thu, Feb 20, 2020 at 2:26 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 2/20/20 1:10 PM, Richard Haines wrote:
> > Add a new 'key_perms' policy capability and support for the additional
> > key permissions: inval, revoke, join, clear
> >
> > 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 < current version: Change permissions on a keyring.
> > kernel >= current version: 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>
>
> What is this relative to? Didn't apply for me on any of keys-acl or
> keys-next or selinux-next.
>
> Regardless, we need to revert the original patch and create a new one
> that addresses the KEY_NEED_PARENT_JOIN issue I mentioned and that adds
> the key_perms capability in the right place in the first place, not
> apply a fix on top.
Yes, you really need to revert this patch David, I mentioned this some
time ago when the linux-next conflict appeared. Also, future patches
like this *really* need to go in via the SELinux tree, not the keys
tree, as they affect the SELinux kernel ABI and if they aren't merged
via the same tree lots of bad things can happen if we aren't careful.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] selinux: Add support for new key permissions
[not found] ` <c5d6ae72-4f5d-fe41-c025-0eaa4616b7eb@tycho.nsa.gov>
2020-02-21 0:03 ` Paul Moore
@ 2020-02-21 10:55 ` Richard Haines
2020-02-28 15:52 ` David Howells
2 siblings, 0 replies; 8+ messages in thread
From: Richard Haines @ 2020-02-21 10:55 UTC (permalink / raw)
To: Stephen Smalley, dhowells; +Cc: selinux, paul
On Thu, 2020-02-20 at 14:28 -0500, Stephen Smalley wrote:
> On 2/20/20 1:10 PM, Richard Haines wrote:
> > Add a new 'key_perms' policy capability and support for the
> > additional
> > key permissions: inval, revoke, join, clear
> >
> > 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 < current version: Change permissions on a
> > keyring.
> > kernel >= current version: 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>
>
> What is this relative to? Didn't apply for me on any of keys-acl or
> keys-next or selinux-next.
I build this version of the patch on linux-next-next-20200214.tar.gz
>
> Regardless, we need to revert the original patch and create a new
> one
> that addresses the KEY_NEED_PARENT_JOIN issue I mentioned and that
> adds
> the key_perms capability in the right place in the first place, not
> apply a fix on top.
Yes I know. I was just making the point to David that as far as I can
see keys only ever passes one perm at a time.
>
> > ---
> > security/selinux/hooks.c | 123 ++++++++++++++++-------
> > -----
> > security/selinux/include/security.h | 10 +--
> > security/selinux/ss/services.c | 4 +-
> > 3 files changed, 76 insertions(+), 61 deletions(-)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 46a8f3e7d..af179442c 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -6538,8 +6538,7 @@ static int selinux_key_permission(key_ref_t
> > key_ref,
> > {
> > struct key *key;
> > struct key_security_struct *ksec;
> > - unsigned int key_perm = 0, switch_perm = 0;
> > - int bit = 1, count = KEY_NEED_ALL;
> > + unsigned int key_perm = 0;
> > u32 sid;
> >
> > /* if no specific permissions are requested, we skip the
> > @@ -6549,60 +6548,73 @@ static int selinux_key_permission(key_ref_t
> > key_ref,
> > return 0;
> >
> > 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;
> > + 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;
> > + default:
> > + pr_err("BUG pcap = 1 entry_perm: 0x%04x\n",
> > perm);
> > + BUG();
> > + break;
> > }
> > } 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;
> > + switch (perm) {
> > + case KEY_NEED_VIEW:
> > + key_perm = KEY_NEED_VIEW;
> > + break;
> > + case KEY_NEED_READ:
> > + key_perm = KEY_NEED_READ;
> > + break;
> > + case KEY_NEED_WRITE:
> > + case KEY_NEED_REVOKE:
> > + case KEY_NEED_CLEAR:
> > + key_perm = KEY_NEED_WRITE;
> > + break;
> > + case KEY_NEED_SEARCH:
> > + case KEY_NEED_INVAL:
> > + case KEY_NEED_JOIN:
> > + key_perm = KEY_NEED_SEARCH;
> > + break;
> > + case KEY_NEED_LINK:
> > + case KEY_NEED_PARENT_JOIN:
> > + key_perm = KEY_NEED_LINK;
> > + break;
> > + case KEY_NEED_SETSEC:
> > + key_perm = OLD_KEY_NEED_SETATTR;
> > + break;
> > + default:
> > + pr_err("BUG pcap = 0 entry_perm: 0x%04x\n",
> > perm);
> > + BUG();
> > + break;
> > + }
> > }
> >
> > sid = cred_sid(cred);
> > @@ -6610,6 +6622,9 @@ static int selinux_key_permission(key_ref_t
> > key_ref,
> > key = key_ref_to_ptr(key_ref);
> > ksec = key->security;
> >
> > + pr_info("entry_perm: 0x%04x exit_perm: 0x%04x\n",
> > + perm, key_perm);
> > +
> > return avc_has_perm(&selinux_state,
> > sid, ksec->sid, SECCLASS_KEY, key_perm,
> > NULL);
> > }
> > diff --git a/security/selinux/include/security.h
> > b/security/selinux/include/security.h
> > index cba9610b8..c0998e79d 100644
> > --- a/security/selinux/include/security.h
> > +++ b/security/selinux/include/security.h
> > @@ -79,8 +79,8 @@ enum {
> > POLICYDB_CAPABILITY_ALWAYSNETWORK,
> > POLICYDB_CAPABILITY_CGROUPSECLABEL,
> > POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
> > - POLICYDB_CAPABILITY_KEYPERMS,
> > POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS,
> > + POLICYDB_CAPABILITY_KEYPERMS,
> > __POLICYDB_CAPABILITY_MAX
> > };
> > #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
> > @@ -215,18 +215,18 @@ 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)
> > +static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
> > {
> > struct selinux_state *state = &selinux_state;
> >
> > - return state->policycap[POLICYDB_CAPABILITY_KEYPERMS];
> > + return state-
> > >policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
> > }
> >
> > -static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
> > +static inline bool selinux_policycap_key_perms(void)
> > {
> > struct selinux_state *state = &selinux_state;
> >
> > - return state-
> > >policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
> > + return state->policycap[POLICYDB_CAPABILITY_KEYPERMS];
> > }
> >
> > int security_mls_enabled(struct selinux_state *state);
> > diff --git a/security/selinux/ss/services.c
> > b/security/selinux/ss/services.c
> > index d4a05f34d..6efc86c47 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -73,8 +73,8 @@ const char
> > *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
> > "always_check_network",
> > "cgroup_seclabel",
> > "nnp_nosuid_transition",
> > - "key_perms",
> > - "genfs_seclabel_symlinks"
> > + "genfs_seclabel_symlinks",
> > + "key_perms"
> > };
> >
> > static struct selinux_ss selinux_ss;
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] selinux: Add support for new key permissions
[not found] ` <c5d6ae72-4f5d-fe41-c025-0eaa4616b7eb@tycho.nsa.gov>
2020-02-21 0:03 ` Paul Moore
2020-02-21 10:55 ` Richard Haines
@ 2020-02-28 15:52 ` David Howells
2020-02-28 16:08 ` Paul Moore
2 siblings, 1 reply; 8+ messages in thread
From: David Howells @ 2020-02-28 15:52 UTC (permalink / raw)
To: Paul Moore; +Cc: dhowells, Richard Haines, selinux, Stephen Smalley
Paul Moore <paul@paul-moore.com> wrote:
> Yes, you really need to revert this patch David, I mentioned this some
> time ago when the linux-next conflict appeared.
It is reverted.
> Also, future patches like this *really* need to go in via the SELinux tree,
> not the keys tree, as they affect the SELinux kernel ABI and if they aren't
> merged via the same tree lots of bad things can happen if we aren't careful.
Are you're willing to take the matching keyring changes with it? The SELinux
patch won't build without them - but they have to go in at the same time
otherwise the keyrings part will malfunction.
David
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] selinux: Add support for new key permissions
2020-02-20 18:10 ` [RFC PATCH 1/1] " Richard Haines
[not found] ` <c5d6ae72-4f5d-fe41-c025-0eaa4616b7eb@tycho.nsa.gov>
@ 2020-02-28 15:55 ` David Howells
2020-02-28 17:22 ` Stephen Smalley
1 sibling, 1 reply; 8+ messages in thread
From: David Howells @ 2020-02-28 15:55 UTC (permalink / raw)
To: Stephen Smalley; +Cc: dhowells, Richard Haines, selinux, paul
Stephen Smalley <sds@tycho.nsa.gov> wrote:
> Regardless, we need to revert the original patch and create a new one that
> addresses the KEY_NEED_PARENT_JOIN issue I mentioned and that adds the
> key_perms capability in the right place in the first place, not apply a fix on
> top.
I think the problem is that selinux_key_permission() is munging the new perm
set into the old perm set and then passing that to avc_has_perm(). Really, we
need to work backwards if the SELinux policy is described in terms of the old
perm set.
Is there any way to make that possible?
David
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] selinux: Add support for new key permissions
2020-02-28 15:52 ` David Howells
@ 2020-02-28 16:08 ` Paul Moore
0 siblings, 0 replies; 8+ messages in thread
From: Paul Moore @ 2020-02-28 16:08 UTC (permalink / raw)
To: David Howells; +Cc: Richard Haines, selinux, Stephen Smalley
On Fri, Feb 28, 2020 at 10:52 AM David Howells <dhowells@redhat.com> wrote:
> Paul Moore <paul@paul-moore.com> wrote:
>
> > Yes, you really need to revert this patch David, I mentioned this some
> > time ago when the linux-next conflict appeared.
>
> It is reverted.
Thank you.
[sorry, I forgot the reply-all]
> > Also, future patches like this *really* need to go in via the SELinux tree,
> > not the keys tree, as they affect the SELinux kernel ABI and if they aren't
> > merged via the same tree lots of bad things can happen if we aren't careful.
>
> Are you're willing to take the matching keyring changes with it? The SELinux
> patch won't build without them - but they have to go in at the same time
> otherwise the keyrings part will malfunction.
Assuming they've got the right ACKs from any subsystems that are affected, sure.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] selinux: Add support for new key permissions
2020-02-28 15:55 ` David Howells
@ 2020-02-28 17:22 ` Stephen Smalley
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Smalley @ 2020-02-28 17:22 UTC (permalink / raw)
To: David Howells; +Cc: Stephen Smalley, Richard Haines, SElinux list, Paul Moore
On Fri, Feb 28, 2020 at 10:56 AM David Howells <dhowells@redhat.com> wrote:
>
> Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> > Regardless, we need to revert the original patch and create a new one that
> > addresses the KEY_NEED_PARENT_JOIN issue I mentioned and that adds the
> > key_perms capability in the right place in the first place, not apply a fix on
> > top.
>
> I think the problem is that selinux_key_permission() is munging the new perm
> set into the old perm set and then passing that to avc_has_perm(). Really, we
> need to work backwards if the SELinux policy is described in terms of the old
> perm set.
>
> Is there any way to make that possible?
That's not the problem. The problem is that security_key_permission()
needs to be passed something (an additional flag or
a different permission) in order to differentiate the two different
callers so that SELinux can support both the old logic (when the
new key_perms capability is disabled) and the new logic (when it is
enabled). The old patch tried to do this by introducing a new
KEY_NEED_PARENT_JOIN permission. But it didn't expose this as a
KEY_ACE value and therefore creates a conflict/inconsistency
between the ACE permissions and the internal permissions. Either it
needs to be exposed as a legitimate ACE value too, or
it needs to be handled differently, e.g. as an additional
kernel-internal flag that gets passed down so SELinux can distinguish
them.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-02-28 17:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20 18:10 [RFC PATCH 0/1] selinux: Add support for new key permissions Richard Haines
2020-02-20 18:10 ` [RFC PATCH 1/1] " Richard Haines
[not found] ` <c5d6ae72-4f5d-fe41-c025-0eaa4616b7eb@tycho.nsa.gov>
2020-02-21 0:03 ` Paul Moore
2020-02-21 10:55 ` Richard Haines
2020-02-28 15:52 ` David Howells
2020-02-28 16:08 ` Paul Moore
2020-02-28 15:55 ` David Howells
2020-02-28 17:22 ` Stephen Smalley
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).