* [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
[parent not found: <c5d6ae72-4f5d-fe41-c025-0eaa4616b7eb@tycho.nsa.gov>]
* 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-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-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: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).