| Message ID | 20201106155626.3395468-4-lokeshgidra@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | SELinux support for anonymous inodes and UFFD | expand |
On Fri, Nov 6, 2020 at 10:56 AM Lokesh Gidra <lokeshgidra@google.com> wrote: > > From: Daniel Colascione <dancol@google.com> > > This change uses the anon_inodes and LSM infrastructure introduced in > the previous patches to give SELinux the ability to control > anonymous-inode files that are created using the new > anon_inode_getfd_secure() function. > > A SELinux policy author detects and controls these anonymous inodes by > adding a name-based type_transition rule that assigns a new security > type to anonymous-inode files created in some domain. The name used > for the name-based transition is the name associated with the > anonymous inode for file listings --- e.g., "[userfaultfd]" or > "[perf_event]". > > Example: > > type uffd_t; > type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]"; > allow sysadm_t uffd_t:anon_inode { create }; > > (The next patch in this series is necessary for making userfaultfd > support this new interface. The example above is just > for exposition.) > > Signed-off-by: Daniel Colascione <dancol@google.com> > Signed-off-by: Lokesh Gidra <lokeshgidra@google.com> > --- > security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++ > security/selinux/include/classmap.h | 2 ++ > 2 files changed, 55 insertions(+) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 6b1826fc3658..1c0adcdce7a8 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2927,6 +2927,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, > return 0; > } > > +static int selinux_inode_init_security_anon(struct inode *inode, > + const struct qstr *name, > + const struct inode *context_inode) > +{ > + const struct task_security_struct *tsec = selinux_cred(current_cred()); > + struct common_audit_data ad; > + struct inode_security_struct *isec; > + int rc; > + > + if (unlikely(!selinux_initialized(&selinux_state))) > + return 0; > + > + isec = selinux_inode(inode); > + > + /* > + * We only get here once per ephemeral inode. The inode has > + * been initialized via inode_alloc_security but is otherwise > + * untouched. > + */ > + > + if (context_inode) { > + struct inode_security_struct *context_isec = > + selinux_inode(context_inode); > + isec->sclass = context_isec->sclass; > + isec->sid = context_isec->sid; I suppose this isn't a major concern given the limited usage at the moment, but I wonder if it would be a good idea to make sure the context_inode's SELinux label is valid before we assign it to the anonymous inode? If it is invalid, what should we do? Do we attempt to (re)validate it? Do we simply fallback to the transition approach? > + } else { > + isec->sclass = SECCLASS_ANON_INODE; > + rc = security_transition_sid( > + &selinux_state, tsec->sid, tsec->sid, > + isec->sclass, name, &isec->sid); > + if (rc) > + return rc; > + } > + > + isec->initialized = LABEL_INITIALIZED; > + > + /* > + * Now that we've initialized security, check whether we're > + * allowed to actually create this type of anonymous inode. > + */ > + > + ad.type = LSM_AUDIT_DATA_INODE; > + ad.u.inode = inode; > + > + return avc_has_perm(&selinux_state, > + tsec->sid, > + isec->sid, > + isec->sclass, > + FILE__CREATE, I believe you want to use ANON_INODE__CREATE here instead of FILE__CREATE, yes? This brings up another question, and requirement - what testing are you doing for this patchset? We require that new SELinux kernel functionality includes additions to the SELinux test suite to help verify the functionality. I'm also *strongly* encouraging that new contributions come with updates to The SELinux Notebook. If you are unsure about what to do for either, let us know and we can help get you started. * https://github.com/SELinuxProject/selinux-testsuite * https://github.com/SELinuxProject/selinux-notebook > + &ad); > +} > + > static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode) > { > return may_create(dir, dentry, SECCLASS_FILE); > @@ -6992,6 +7044,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { > > LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security), > LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security), > + LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon), > LSM_HOOK_INIT(inode_create, selinux_inode_create), > LSM_HOOK_INIT(inode_link, selinux_inode_link), > LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink), > diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h > index 40cebde62856..ba2e01a6955c 100644 > --- a/security/selinux/include/classmap.h > +++ b/security/selinux/include/classmap.h > @@ -249,6 +249,8 @@ struct security_class_mapping secclass_map[] = { > {"open", "cpu", "kernel", "tracepoint", "read", "write"} }, > { "lockdown", > { "integrity", "confidentiality", NULL } }, > + { "anon_inode", > + { COMMON_FILE_PERMS, NULL } }, > { NULL } > }; >
Thanks a lot Paul for the reviewing this patch. On Mon, Nov 9, 2020 at 7:12 PM Paul Moore <paul@paul-moore.com> wrote: > > On Fri, Nov 6, 2020 at 10:56 AM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > > From: Daniel Colascione <dancol@google.com> > > > > This change uses the anon_inodes and LSM infrastructure introduced in > > the previous patches to give SELinux the ability to control > > anonymous-inode files that are created using the new > > anon_inode_getfd_secure() function. > > > > A SELinux policy author detects and controls these anonymous inodes by > > adding a name-based type_transition rule that assigns a new security > > type to anonymous-inode files created in some domain. The name used > > for the name-based transition is the name associated with the > > anonymous inode for file listings --- e.g., "[userfaultfd]" or > > "[perf_event]". > > > > Example: > > > > type uffd_t; > > type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]"; > > allow sysadm_t uffd_t:anon_inode { create }; > > > > (The next patch in this series is necessary for making userfaultfd > > support this new interface. The example above is just > > for exposition.) > > > > Signed-off-by: Daniel Colascione <dancol@google.com> > > Signed-off-by: Lokesh Gidra <lokeshgidra@google.com> > > --- > > security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++ > > security/selinux/include/classmap.h | 2 ++ > > 2 files changed, 55 insertions(+) > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 6b1826fc3658..1c0adcdce7a8 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -2927,6 +2927,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, > > return 0; > > } > > > > +static int selinux_inode_init_security_anon(struct inode *inode, > > + const struct qstr *name, > > + const struct inode *context_inode) > > +{ > > + const struct task_security_struct *tsec = selinux_cred(current_cred()); > > + struct common_audit_data ad; > > + struct inode_security_struct *isec; > > + int rc; > > + > > + if (unlikely(!selinux_initialized(&selinux_state))) > > + return 0; > > + > > + isec = selinux_inode(inode); > > + > > + /* > > + * We only get here once per ephemeral inode. The inode has > > + * been initialized via inode_alloc_security but is otherwise > > + * untouched. > > + */ > > + > > + if (context_inode) { > > + struct inode_security_struct *context_isec = > > + selinux_inode(context_inode); > > + isec->sclass = context_isec->sclass; > > + isec->sid = context_isec->sid; > > I suppose this isn't a major concern given the limited usage at the > moment, but I wonder if it would be a good idea to make sure the > context_inode's SELinux label is valid before we assign it to the > anonymous inode? If it is invalid, what should we do? Do we attempt > to (re)validate it? Do we simply fallback to the transition approach? > Frankly, I'm not too familiar with SELinux. Originally this patch series was developed by Daniel, in consultation with Stephen Smalley. In my (probably naive) opinion we should fallback to transition approach. But I'd request you to tell me if this needs to be addressed now, and if so then what's the right approach. If the decision is to address this now, then what's the best way to check the SELinux label validity? > > + } else { > > + isec->sclass = SECCLASS_ANON_INODE; > > + rc = security_transition_sid( > > + &selinux_state, tsec->sid, tsec->sid, > > + isec->sclass, name, &isec->sid); > > + if (rc) > > + return rc; > > + } > > + > > + isec->initialized = LABEL_INITIALIZED; > > + > > + /* > > + * Now that we've initialized security, check whether we're > > + * allowed to actually create this type of anonymous inode. > > + */ > > + > > + ad.type = LSM_AUDIT_DATA_INODE; > > + ad.u.inode = inode; > > + > > + return avc_has_perm(&selinux_state, > > + tsec->sid, > > + isec->sid, > > + isec->sclass, > > + FILE__CREATE, > > I believe you want to use ANON_INODE__CREATE here instead of FILE__CREATE, yes? ANON_INODE__CREATE definitely seems more appropriate. I'll change it in the next revision. > > This brings up another question, and requirement - what testing are > you doing for this patchset? We require that new SELinux kernel > functionality includes additions to the SELinux test suite to help > verify the functionality. I'm also *strongly* encouraging that new > contributions come with updates to The SELinux Notebook. If you are > unsure about what to do for either, let us know and we can help get > you started. > > * https://github.com/SELinuxProject/selinux-testsuite > * https://github.com/SELinuxProject/selinux-notebook > I'd definitely need help with both of these. Kindly guide how to proceed. > > + &ad); > > +} > > + > > static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode) > > { > > return may_create(dir, dentry, SECCLASS_FILE); > > @@ -6992,6 +7044,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { > > > > LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security), > > LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security), > > + LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon), > > LSM_HOOK_INIT(inode_create, selinux_inode_create), > > LSM_HOOK_INIT(inode_link, selinux_inode_link), > > LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink), > > diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h > > index 40cebde62856..ba2e01a6955c 100644 > > --- a/security/selinux/include/classmap.h > > +++ b/security/selinux/include/classmap.h > > @@ -249,6 +249,8 @@ struct security_class_mapping secclass_map[] = { > > {"open", "cpu", "kernel", "tracepoint", "read", "write"} }, > > { "lockdown", > > { "integrity", "confidentiality", NULL } }, > > + { "anon_inode", > > + { COMMON_FILE_PERMS, NULL } }, > > { NULL } > > }; > > > > -- > paul moore > www.paul-moore.com
On Tue, Nov 10, 2020 at 1:24 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > On Mon, Nov 9, 2020 at 7:12 PM Paul Moore <paul@paul-moore.com> wrote: > > On Fri, Nov 6, 2020 at 10:56 AM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > > > > From: Daniel Colascione <dancol@google.com> > > > > > > This change uses the anon_inodes and LSM infrastructure introduced in > > > the previous patches to give SELinux the ability to control > > > anonymous-inode files that are created using the new > > > anon_inode_getfd_secure() function. > > > > > > A SELinux policy author detects and controls these anonymous inodes by > > > adding a name-based type_transition rule that assigns a new security > > > type to anonymous-inode files created in some domain. The name used > > > for the name-based transition is the name associated with the > > > anonymous inode for file listings --- e.g., "[userfaultfd]" or > > > "[perf_event]". > > > > > > Example: > > > > > > type uffd_t; > > > type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]"; > > > allow sysadm_t uffd_t:anon_inode { create }; > > > > > > (The next patch in this series is necessary for making userfaultfd > > > support this new interface. The example above is just > > > for exposition.) > > > > > > Signed-off-by: Daniel Colascione <dancol@google.com> > > > Signed-off-by: Lokesh Gidra <lokeshgidra@google.com> > > > --- > > > security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++ > > > security/selinux/include/classmap.h | 2 ++ > > > 2 files changed, 55 insertions(+) > > > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > > index 6b1826fc3658..1c0adcdce7a8 100644 > > > --- a/security/selinux/hooks.c > > > +++ b/security/selinux/hooks.c > > > @@ -2927,6 +2927,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, > > > return 0; > > > } > > > > > > +static int selinux_inode_init_security_anon(struct inode *inode, > > > + const struct qstr *name, > > > + const struct inode *context_inode) > > > +{ > > > + const struct task_security_struct *tsec = selinux_cred(current_cred()); > > > + struct common_audit_data ad; > > > + struct inode_security_struct *isec; > > > + int rc; > > > + > > > + if (unlikely(!selinux_initialized(&selinux_state))) > > > + return 0; > > > + > > > + isec = selinux_inode(inode); > > > + > > > + /* > > > + * We only get here once per ephemeral inode. The inode has > > > + * been initialized via inode_alloc_security but is otherwise > > > + * untouched. > > > + */ > > > + > > > + if (context_inode) { > > > + struct inode_security_struct *context_isec = > > > + selinux_inode(context_inode); > > > + isec->sclass = context_isec->sclass; > > > + isec->sid = context_isec->sid; > > > > I suppose this isn't a major concern given the limited usage at the > > moment, but I wonder if it would be a good idea to make sure the > > context_inode's SELinux label is valid before we assign it to the > > anonymous inode? If it is invalid, what should we do? Do we attempt > > to (re)validate it? Do we simply fallback to the transition approach? > > Frankly, I'm not too familiar with SELinux. Originally this patch > series was developed by Daniel, in consultation with Stephen Smalley. > In my (probably naive) opinion we should fallback to transition > approach. But I'd request you to tell me if this needs to be addressed > now, and if so then what's the right approach. > > If the decision is to address this now, then what's the best way to > check the SELinux label validity? You can check to see if an inode's label is valid by looking at the isec->initialized field; if it is LABEL_INITIALIZED then it is all set, if it is any other value then the label isn't entirely correct. It may have not have ever been fully initialized (and has a default value) or it may have live on a remote filesystem where the host has signaled that the label has changed (and the label is now outdated). This patchset includes support for userfaultfd, which means we don't really have to worry about the remote fs problem, but the never-fully-initialized problem could be real in this case. Normally we would revalidate an inode in SELinux by calling __inode_security_revalidate() which requires either a valid dentry or one that can be found via the inode; does d_find_alias() work on userfaultfd inodes? If all else fails, it seems like the safest approach would be to simply fail the selinux_inode_init_security_anon() call if a context_inode was supplied and the label wasn't valid. If we later decide to change it to falling back to the transition approach we can do that, we can't go the other way (from transition to error). > > > + } else { > > > + isec->sclass = SECCLASS_ANON_INODE; > > > + rc = security_transition_sid( > > > + &selinux_state, tsec->sid, tsec->sid, > > > + isec->sclass, name, &isec->sid); > > > + if (rc) > > > + return rc; > > > + } > > > + > > > + isec->initialized = LABEL_INITIALIZED; > > > + > > > + /* > > > + * Now that we've initialized security, check whether we're > > > + * allowed to actually create this type of anonymous inode. > > > + */ > > > + > > > + ad.type = LSM_AUDIT_DATA_INODE; > > > + ad.u.inode = inode; > > > + > > > + return avc_has_perm(&selinux_state, > > > + tsec->sid, > > > + isec->sid, > > > + isec->sclass, > > > + FILE__CREATE, > > > > I believe you want to use ANON_INODE__CREATE here instead of FILE__CREATE, yes? > > ANON_INODE__CREATE definitely seems more appropriate. I'll change it > in the next revision. > > > This brings up another question, and requirement - what testing are > > you doing for this patchset? We require that new SELinux kernel > > functionality includes additions to the SELinux test suite to help > > verify the functionality. I'm also *strongly* encouraging that new > > contributions come with updates to The SELinux Notebook. If you are > > unsure about what to do for either, let us know and we can help get > > you started. > > > > * https://github.com/SELinuxProject/selinux-testsuite > > * https://github.com/SELinuxProject/selinux-notebook > > > I'd definitely need help with both of these. Kindly guide how to proceed. Well, perhaps the best way to start is to explain how you have been testing this so far and then using that information to draft a test for the testsuite.
On Tue, Nov 10, 2020 at 6:13 PM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Nov 10, 2020 at 1:24 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > On Mon, Nov 9, 2020 at 7:12 PM Paul Moore <paul@paul-moore.com> wrote: > > > On Fri, Nov 6, 2020 at 10:56 AM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > > > > > > From: Daniel Colascione <dancol@google.com> > > > > > > > > This change uses the anon_inodes and LSM infrastructure introduced in > > > > the previous patches to give SELinux the ability to control > > > > anonymous-inode files that are created using the new > > > > anon_inode_getfd_secure() function. > > > > > > > > A SELinux policy author detects and controls these anonymous inodes by > > > > adding a name-based type_transition rule that assigns a new security > > > > type to anonymous-inode files created in some domain. The name used > > > > for the name-based transition is the name associated with the > > > > anonymous inode for file listings --- e.g., "[userfaultfd]" or > > > > "[perf_event]". > > > > > > > > Example: > > > > > > > > type uffd_t; > > > > type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]"; > > > > allow sysadm_t uffd_t:anon_inode { create }; > > > > > > > > (The next patch in this series is necessary for making userfaultfd > > > > support this new interface. The example above is just > > > > for exposition.) > > > > > > > > Signed-off-by: Daniel Colascione <dancol@google.com> > > > > Signed-off-by: Lokesh Gidra <lokeshgidra@google.com> > > > > --- > > > > security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++ > > > > security/selinux/include/classmap.h | 2 ++ > > > > 2 files changed, 55 insertions(+) > > > > > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > > > index 6b1826fc3658..1c0adcdce7a8 100644 > > > > --- a/security/selinux/hooks.c > > > > +++ b/security/selinux/hooks.c > > > > @@ -2927,6 +2927,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, > > > > return 0; > > > > } > > > > > > > > +static int selinux_inode_init_security_anon(struct inode *inode, > > > > + const struct qstr *name, > > > > + const struct inode *context_inode) > > > > +{ > > > > + const struct task_security_struct *tsec = selinux_cred(current_cred()); > > > > + struct common_audit_data ad; > > > > + struct inode_security_struct *isec; > > > > + int rc; > > > > + > > > > + if (unlikely(!selinux_initialized(&selinux_state))) > > > > + return 0; > > > > + > > > > + isec = selinux_inode(inode); > > > > + > > > > + /* > > > > + * We only get here once per ephemeral inode. The inode has > > > > + * been initialized via inode_alloc_security but is otherwise > > > > + * untouched. > > > > + */ > > > > + > > > > + if (context_inode) { > > > > + struct inode_security_struct *context_isec = > > > > + selinux_inode(context_inode); > > > > + isec->sclass = context_isec->sclass; > > > > + isec->sid = context_isec->sid; > > > > > > I suppose this isn't a major concern given the limited usage at the > > > moment, but I wonder if it would be a good idea to make sure the > > > context_inode's SELinux label is valid before we assign it to the > > > anonymous inode? If it is invalid, what should we do? Do we attempt > > > to (re)validate it? Do we simply fallback to the transition approach? > > > > Frankly, I'm not too familiar with SELinux. Originally this patch > > series was developed by Daniel, in consultation with Stephen Smalley. > > In my (probably naive) opinion we should fallback to transition > > approach. But I'd request you to tell me if this needs to be addressed > > now, and if so then what's the right approach. > > > > If the decision is to address this now, then what's the best way to > > check the SELinux label validity? > > You can check to see if an inode's label is valid by looking at the > isec->initialized field; if it is LABEL_INITIALIZED then it is all > set, if it is any other value then the label isn't entirely correct. > It may have not have ever been fully initialized (and has a default > value) or it may have live on a remote filesystem where the host has > signaled that the label has changed (and the label is now outdated). > > This patchset includes support for userfaultfd, which means we don't > really have to worry about the remote fs problem, but the > never-fully-initialized problem could be real in this case. Normally > we would revalidate an inode in SELinux by calling > __inode_security_revalidate() which requires either a valid dentry or > one that can be found via the inode; does d_find_alias() work on > userfaultfd inodes? > > If all else fails, it seems like the safest approach would be to > simply fail the selinux_inode_init_security_anon() call if a > context_inode was supplied and the label wasn't valid. If we later > decide to change it to falling back to the transition approach we can > do that, we can't go the other way (from transition to error). > I'm not sure about d_find_alias() on userfaultfd inodes. But it seems ok to fail selinux_inode_init_security_anon() to begin with. > > > > + } else { > > > > + isec->sclass = SECCLASS_ANON_INODE; > > > > + rc = security_transition_sid( > > > > + &selinux_state, tsec->sid, tsec->sid, > > > > + isec->sclass, name, &isec->sid); > > > > + if (rc) > > > > + return rc; > > > > + } > > > > + > > > > + isec->initialized = LABEL_INITIALIZED; > > > > + > > > > + /* > > > > + * Now that we've initialized security, check whether we're > > > > + * allowed to actually create this type of anonymous inode. > > > > + */ > > > > + > > > > + ad.type = LSM_AUDIT_DATA_INODE; > > > > + ad.u.inode = inode; > > > > + > > > > + return avc_has_perm(&selinux_state, > > > > + tsec->sid, > > > > + isec->sid, > > > > + isec->sclass, > > > > + FILE__CREATE, > > > > > > I believe you want to use ANON_INODE__CREATE here instead of FILE__CREATE, yes? > > > > ANON_INODE__CREATE definitely seems more appropriate. I'll change it > > in the next revision. > > > > > This brings up another question, and requirement - what testing are > > > you doing for this patchset? We require that new SELinux kernel > > > functionality includes additions to the SELinux test suite to help > > > verify the functionality. I'm also *strongly* encouraging that new > > > contributions come with updates to The SELinux Notebook. If you are > > > unsure about what to do for either, let us know and we can help get > > > you started. > > > > > > * https://github.com/SELinuxProject/selinux-testsuite > > > * https://github.com/SELinuxProject/selinux-notebook > > > > > I'd definitely need help with both of these. Kindly guide how to proceed. > > Well, perhaps the best way to start is to explain how you have been > testing this so far and then using that information to draft a test > for the testsuite. > As I said in my previous reply, Daniel worked on this patch along with Stephan Smalley. Here's the conversation regarding testing from back then: https://lore.kernel.org/lkml/CAEjxPJ4iquFSBfEj+UEFLUFHPsezuQ-Bzv09n+WgOWk38Nyw3w@mail.gmail.com/ There have been only minor changes (fixing comments/coding-style), except for addressing a double free issue with userfaultfd_ctx since last time it was tested as per the link above. > -- > paul moore > www.paul-moore.com
On Tue, Nov 10, 2020 at 10:30 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > On Tue, Nov 10, 2020 at 6:13 PM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Nov 10, 2020 at 1:24 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > On Mon, Nov 9, 2020 at 7:12 PM Paul Moore <paul@paul-moore.com> wrote: > > > > On Fri, Nov 6, 2020 at 10:56 AM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > > > > > > > > From: Daniel Colascione <dancol@google.com> > > > > > > > > > > This change uses the anon_inodes and LSM infrastructure introduced in > > > > > the previous patches to give SELinux the ability to control > > > > > anonymous-inode files that are created using the new > > > > > anon_inode_getfd_secure() function. > > > > > > > > > > A SELinux policy author detects and controls these anonymous inodes by > > > > > adding a name-based type_transition rule that assigns a new security > > > > > type to anonymous-inode files created in some domain. The name used > > > > > for the name-based transition is the name associated with the > > > > > anonymous inode for file listings --- e.g., "[userfaultfd]" or > > > > > "[perf_event]". > > > > > > > > > > Example: > > > > > > > > > > type uffd_t; > > > > > type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]"; > > > > > allow sysadm_t uffd_t:anon_inode { create }; > > > > > > > > > > (The next patch in this series is necessary for making userfaultfd > > > > > support this new interface. The example above is just > > > > > for exposition.) > > > > > > > > > > Signed-off-by: Daniel Colascione <dancol@google.com> > > > > > Signed-off-by: Lokesh Gidra <lokeshgidra@google.com> > > > > > --- > > > > > security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++ > > > > > security/selinux/include/classmap.h | 2 ++ > > > > > 2 files changed, 55 insertions(+) > > > > > > > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > > > > index 6b1826fc3658..1c0adcdce7a8 100644 > > > > > --- a/security/selinux/hooks.c > > > > > +++ b/security/selinux/hooks.c > > > > > @@ -2927,6 +2927,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, > > > > > return 0; > > > > > } > > > > > > > > > > +static int selinux_inode_init_security_anon(struct inode *inode, > > > > > + const struct qstr *name, > > > > > + const struct inode *context_inode) > > > > > +{ > > > > > + const struct task_security_struct *tsec = selinux_cred(current_cred()); > > > > > + struct common_audit_data ad; > > > > > + struct inode_security_struct *isec; > > > > > + int rc; > > > > > + > > > > > + if (unlikely(!selinux_initialized(&selinux_state))) > > > > > + return 0; > > > > > + > > > > > + isec = selinux_inode(inode); > > > > > + > > > > > + /* > > > > > + * We only get here once per ephemeral inode. The inode has > > > > > + * been initialized via inode_alloc_security but is otherwise > > > > > + * untouched. > > > > > + */ > > > > > + > > > > > + if (context_inode) { > > > > > + struct inode_security_struct *context_isec = > > > > > + selinux_inode(context_inode); > > > > > + isec->sclass = context_isec->sclass; > > > > > + isec->sid = context_isec->sid; > > > > > > > > I suppose this isn't a major concern given the limited usage at the > > > > moment, but I wonder if it would be a good idea to make sure the > > > > context_inode's SELinux label is valid before we assign it to the > > > > anonymous inode? If it is invalid, what should we do? Do we attempt > > > > to (re)validate it? Do we simply fallback to the transition approach? > > > > > > Frankly, I'm not too familiar with SELinux. Originally this patch > > > series was developed by Daniel, in consultation with Stephen Smalley. > > > In my (probably naive) opinion we should fallback to transition > > > approach. But I'd request you to tell me if this needs to be addressed > > > now, and if so then what's the right approach. > > > > > > If the decision is to address this now, then what's the best way to > > > check the SELinux label validity? > > > > You can check to see if an inode's label is valid by looking at the > > isec->initialized field; if it is LABEL_INITIALIZED then it is all > > set, if it is any other value then the label isn't entirely correct. > > It may have not have ever been fully initialized (and has a default > > value) or it may have live on a remote filesystem where the host has > > signaled that the label has changed (and the label is now outdated). > > > > This patchset includes support for userfaultfd, which means we don't > > really have to worry about the remote fs problem, but the > > never-fully-initialized problem could be real in this case. Normally > > we would revalidate an inode in SELinux by calling > > __inode_security_revalidate() which requires either a valid dentry or > > one that can be found via the inode; does d_find_alias() work on > > userfaultfd inodes? > > > > If all else fails, it seems like the safest approach would be to > > simply fail the selinux_inode_init_security_anon() call if a > > context_inode was supplied and the label wasn't valid. If we later > > decide to change it to falling back to the transition approach we can > > do that, we can't go the other way (from transition to error). > > I'm not sure about d_find_alias() on userfaultfd inodes. But it seems > ok to fail selinux_inode_init_security_anon() to begin with. I'm okay with simply failing here, but I'm growing a bit concerned that this patchset hasn't been well tested. That is a problem. > > > > This brings up another question, and requirement - what testing are > > > > you doing for this patchset? We require that new SELinux kernel > > > > functionality includes additions to the SELinux test suite to help > > > > verify the functionality. I'm also *strongly* encouraging that new > > > > contributions come with updates to The SELinux Notebook. If you are > > > > unsure about what to do for either, let us know and we can help get > > > > you started. > > > > > > > > * https://github.com/SELinuxProject/selinux-testsuite > > > > * https://github.com/SELinuxProject/selinux-notebook > > > > > > > I'd definitely need help with both of these. Kindly guide how to proceed. > > > > Well, perhaps the best way to start is to explain how you have been > > testing this so far and then using that information to draft a test > > for the testsuite. > > As I said in my previous reply, Daniel worked on this patch along with > Stephan Smalley. Here's the conversation regarding testing from back > then: > https://lore.kernel.org/lkml/CAEjxPJ4iquFSBfEj+UEFLUFHPsezuQ-Bzv09n+WgOWk38Nyw3w@mail.gmail.com/ > > There have been only minor changes (fixing comments/coding-style), > except for addressing a double free issue with userfaultfd_ctx since > last time it was tested as per the link above. I should probably be more clear. I honestly don't care who originally wrote the patch, the simple fact is that you are the one who is posting it *now* for inclusion in the kernel; at the very least I expect you to be able to demonstrate that you are able to reliably test this functionality and prove it is working. While being able to test this submission initially is important, it is far more important to have the tests and docs necessary to maintain this functionality long term. Perhaps you and/or Google will continue to contribute and support this functionality long term, but it would be irresponsible of me to assume that to be true; both people and companies come and go but code has a tendency to live forever. Let's start again; how have you been testing this code?
On Thu, Nov 12, 2020 at 4:13 PM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Nov 10, 2020 at 10:30 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > On Tue, Nov 10, 2020 at 6:13 PM Paul Moore <paul@paul-moore.com> wrote: > > > On Tue, Nov 10, 2020 at 1:24 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > > On Mon, Nov 9, 2020 at 7:12 PM Paul Moore <paul@paul-moore.com> wrote: > > > > > On Fri, Nov 6, 2020 at 10:56 AM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > > > > > > > > > > From: Daniel Colascione <dancol@google.com> > > > > > > > > > > > > This change uses the anon_inodes and LSM infrastructure introduced in > > > > > > the previous patches to give SELinux the ability to control > > > > > > anonymous-inode files that are created using the new > > > > > > anon_inode_getfd_secure() function. > > > > > > > > > > > > A SELinux policy author detects and controls these anonymous inodes by > > > > > > adding a name-based type_transition rule that assigns a new security > > > > > > type to anonymous-inode files created in some domain. The name used > > > > > > for the name-based transition is the name associated with the > > > > > > anonymous inode for file listings --- e.g., "[userfaultfd]" or > > > > > > "[perf_event]". > > > > > > > > > > > > Example: > > > > > > > > > > > > type uffd_t; > > > > > > type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]"; > > > > > > allow sysadm_t uffd_t:anon_inode { create }; > > > > > > > > > > > > (The next patch in this series is necessary for making userfaultfd > > > > > > support this new interface. The example above is just > > > > > > for exposition.) > > > > > > > > > > > > Signed-off-by: Daniel Colascione <dancol@google.com> > > > > > > Signed-off-by: Lokesh Gidra <lokeshgidra@google.com> > > > > > > --- > > > > > > security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++ > > > > > > security/selinux/include/classmap.h | 2 ++ > > > > > > 2 files changed, 55 insertions(+) > > > > > > > > > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > > > > > index 6b1826fc3658..1c0adcdce7a8 100644 > > > > > > --- a/security/selinux/hooks.c > > > > > > +++ b/security/selinux/hooks.c > > > > > > @@ -2927,6 +2927,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, > > > > > > return 0; > > > > > > } > > > > > > > > > > > > +static int selinux_inode_init_security_anon(struct inode *inode, > > > > > > + const struct qstr *name, > > > > > > + const struct inode *context_inode) > > > > > > +{ > > > > > > + const struct task_security_struct *tsec = selinux_cred(current_cred()); > > > > > > + struct common_audit_data ad; > > > > > > + struct inode_security_struct *isec; > > > > > > + int rc; > > > > > > + > > > > > > + if (unlikely(!selinux_initialized(&selinux_state))) > > > > > > + return 0; > > > > > > + > > > > > > + isec = selinux_inode(inode); > > > > > > + > > > > > > + /* > > > > > > + * We only get here once per ephemeral inode. The inode has > > > > > > + * been initialized via inode_alloc_security but is otherwise > > > > > > + * untouched. > > > > > > + */ > > > > > > + > > > > > > + if (context_inode) { > > > > > > + struct inode_security_struct *context_isec = > > > > > > + selinux_inode(context_inode); > > > > > > + isec->sclass = context_isec->sclass; > > > > > > + isec->sid = context_isec->sid; > > > > > > > > > > I suppose this isn't a major concern given the limited usage at the > > > > > moment, but I wonder if it would be a good idea to make sure the > > > > > context_inode's SELinux label is valid before we assign it to the > > > > > anonymous inode? If it is invalid, what should we do? Do we attempt > > > > > to (re)validate it? Do we simply fallback to the transition approach? > > > > > > > > Frankly, I'm not too familiar with SELinux. Originally this patch > > > > series was developed by Daniel, in consultation with Stephen Smalley. > > > > In my (probably naive) opinion we should fallback to transition > > > > approach. But I'd request you to tell me if this needs to be addressed > > > > now, and if so then what's the right approach. > > > > > > > > If the decision is to address this now, then what's the best way to > > > > check the SELinux label validity? > > > > > > You can check to see if an inode's label is valid by looking at the > > > isec->initialized field; if it is LABEL_INITIALIZED then it is all > > > set, if it is any other value then the label isn't entirely correct. > > > It may have not have ever been fully initialized (and has a default > > > value) or it may have live on a remote filesystem where the host has > > > signaled that the label has changed (and the label is now outdated). > > > > > > This patchset includes support for userfaultfd, which means we don't > > > really have to worry about the remote fs problem, but the > > > never-fully-initialized problem could be real in this case. Normally > > > we would revalidate an inode in SELinux by calling > > > __inode_security_revalidate() which requires either a valid dentry or > > > one that can be found via the inode; does d_find_alias() work on > > > userfaultfd inodes? > > > > > > If all else fails, it seems like the safest approach would be to > > > simply fail the selinux_inode_init_security_anon() call if a > > > context_inode was supplied and the label wasn't valid. If we later > > > decide to change it to falling back to the transition approach we can > > > do that, we can't go the other way (from transition to error). > > > > I'm not sure about d_find_alias() on userfaultfd inodes. But it seems > > ok to fail selinux_inode_init_security_anon() to begin with. > > I'm okay with simply failing here, but I'm growing a bit concerned > that this patchset hasn't been well tested. That is a problem. > > > > > > This brings up another question, and requirement - what testing are > > > > > you doing for this patchset? We require that new SELinux kernel > > > > > functionality includes additions to the SELinux test suite to help > > > > > verify the functionality. I'm also *strongly* encouraging that new > > > > > contributions come with updates to The SELinux Notebook. If you are > > > > > unsure about what to do for either, let us know and we can help get > > > > > you started. > > > > > > > > > > * https://github.com/SELinuxProject/selinux-testsuite > > > > > * https://github.com/SELinuxProject/selinux-notebook > > > > > > > > > I'd definitely need help with both of these. Kindly guide how to proceed. > > > > > > Well, perhaps the best way to start is to explain how you have been > > > testing this so far and then using that information to draft a test > > > for the testsuite. > > > > As I said in my previous reply, Daniel worked on this patch along with > > Stephan Smalley. Here's the conversation regarding testing from back > > then: > > https://lore.kernel.org/lkml/CAEjxPJ4iquFSBfEj+UEFLUFHPsezuQ-Bzv09n+WgOWk38Nyw3w@mail.gmail.com/ > > > > There have been only minor changes (fixing comments/coding-style), > > except for addressing a double free issue with userfaultfd_ctx since > > last time it was tested as per the link above. > > I should probably be more clear. I honestly don't care who originally > wrote the patch, the simple fact is that you are the one who is > posting it *now* for inclusion in the kernel; at the very least I > expect you to be able to demonstrate that you are able to reliably > test this functionality and prove it is working. While being able to > test this submission initially is important, it is far more important > to have the tests and docs necessary to maintain this functionality > long term. Perhaps you and/or Google will continue to contribute and > support this functionality long term, but it would be irresponsible of > me to assume that to be true; both people and companies come and go > but code has a tendency to live forever. > > Let's start again; how have you been testing this code? > I have created a cuttlefish build and have tested with the attached userfaultfd program: 1) Without these kernel patches the program executes without any restrictions vsoc_x86_64:/ $ ./system/bin/userfaultfdSimple api: 170 features: 511 ioctls: 9223372036854775811 read: Try again 2) With these patches applied but without any policy the 'permission denied' is thrown vsoc_x86_64:/ $ ./system/bin/userfaultfdSimple syscall(userfaultfd): Permission denied with the following logcat message: 11-18 14:21:44.041 3130 3130 W userfaultfdSimp: type=1400 audit(0.0:107): avc: denied { create } for dev="anon_inodefs" ino=45031 scontext=u:r:shell:s0 tcontext=u:object_r:shell:s0 tclass=anon_inode permissive=0 3) With the attached .te policy file in place the following output is observed, confirming that the patch is working as intended. vsoc_x86_64:/ $ ./vendor/bin/userfaultfdSimple UFFDIO_API: Permission denied with the following logcat message: 11-18 14:33:29.142 2028 2028 W userfaultfdSimp: type=1400 audit(0.0:104): avc: denied { ioctl } for path="anon_inode:[userfaultfd]" dev="anon_inodefs" ino=41169 ioctlcmd=0xaa3f scontext=u:r:userfaultfdSimple:s0 tcontext=u:object_r:uffd_t:s0 tclass=anon_inode permissive=0 > -- > paul moore > www.paul-moore.com
On Wed, Nov 18, 2020 at 5:39 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > I have created a cuttlefish build and have tested with the attached > userfaultfd program: Thanks, that's a good place to start, a few comments: - While we support Android as a distribution, it isn't a platform that we common use for development and testing. At the moment, Fedora is probably your best choice for that. - Your test program should be written in vanilla C for the selinux-testsuite. Looking at the userfaultfdSimple.cc code that should be a trivial conversion. - I think you have a good start on a test for the selinux-testsuite, please take a look at the test suite and submit a patch against that repo. Ondrej (CC'd) currently maintains the test suite and he may have some additional thoughts. * https://github.com/SELinuxProject/selinux-testsuite > 1) Without these kernel patches the program executes without any restrictions > > vsoc_x86_64:/ $ ./system/bin/userfaultfdSimple > api: 170 > features: 511 > ioctls: 9223372036854775811 > > read: Try again > > > 2) With these patches applied but without any policy the 'permission > denied' is thrown > > vsoc_x86_64:/ $ ./system/bin/userfaultfdSimple > syscall(userfaultfd): Permission denied > > with the following logcat message: > 11-18 14:21:44.041 3130 3130 W userfaultfdSimp: type=1400 > audit(0.0:107): avc: denied { create } for dev="anon_inodefs" > ino=45031 scontext=u:r:shell:s0 tcontext=u:object_r:shell:s0 > tclass=anon_inode permissive=0 > > > 3) With the attached .te policy file in place the following output is > observed, confirming that the patch is working as intended. > vsoc_x86_64:/ $ ./vendor/bin/userfaultfdSimple > UFFDIO_API: Permission denied > > with the following logcat message: > 11-18 14:33:29.142 2028 2028 W userfaultfdSimp: type=1400 > audit(0.0:104): avc: denied { ioctl } for > path="anon_inode:[userfaultfd]" dev="anon_inodefs" ino=41169 > ioctlcmd=0xaa3f scontext=u:r:userfaultfdSimple:s0 > tcontext=u:object_r:uffd_t:s0 tclass=anon_inode permissive=0
On Sun, Nov 22, 2020 at 3:14 PM Paul Moore <paul@paul-moore.com> wrote: > > On Wed, Nov 18, 2020 at 5:39 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > I have created a cuttlefish build and have tested with the attached > > userfaultfd program: > > Thanks, that's a good place to start, a few comments: > > - While we support Android as a distribution, it isn't a platform that > we common use for development and testing. At the moment, Fedora is > probably your best choice for that. > I tried setting up a debian/ubuntu system for testing using the instructions on the selinux-testsuite page, but the system kept freezing after 'setenforce 1'. I'll try with fedora now. > - Your test program should be written in vanilla C for the > selinux-testsuite. Looking at the userfaultfdSimple.cc code that > should be a trivial conversion. > > - I think you have a good start on a test for the selinux-testsuite, > please take a look at the test suite and submit a patch against that > repo. Ondrej (CC'd) currently maintains the test suite and he may > have some additional thoughts. > > * https://github.com/SELinuxProject/selinux-testsuite Thanks a lot for the inputs. I'll start working on this. > > > 1) Without these kernel patches the program executes without any restrictions > > > > vsoc_x86_64:/ $ ./system/bin/userfaultfdSimple > > api: 170 > > features: 511 > > ioctls: 9223372036854775811 > > > > read: Try again > > > > > > 2) With these patches applied but without any policy the 'permission > > denied' is thrown > > > > vsoc_x86_64:/ $ ./system/bin/userfaultfdSimple > > syscall(userfaultfd): Permission denied > > > > with the following logcat message: > > 11-18 14:21:44.041 3130 3130 W userfaultfdSimp: type=1400 > > audit(0.0:107): avc: denied { create } for dev="anon_inodefs" > > ino=45031 scontext=u:r:shell:s0 tcontext=u:object_r:shell:s0 > > tclass=anon_inode permissive=0 > > > > > > 3) With the attached .te policy file in place the following output is > > observed, confirming that the patch is working as intended. > > vsoc_x86_64:/ $ ./vendor/bin/userfaultfdSimple > > UFFDIO_API: Permission denied > > > > with the following logcat message: > > 11-18 14:33:29.142 2028 2028 W userfaultfdSimp: type=1400 > > audit(0.0:104): avc: denied { ioctl } for > > path="anon_inode:[userfaultfd]" dev="anon_inodefs" ino=41169 > > ioctlcmd=0xaa3f scontext=u:r:userfaultfdSimple:s0 > > tcontext=u:object_r:uffd_t:s0 tclass=anon_inode permissive=0 > > -- > paul moore > www.paul-moore.com
On Mon, Nov 23, 2020 at 2:21 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > On Sun, Nov 22, 2020 at 3:14 PM Paul Moore <paul@paul-moore.com> wrote: > > On Wed, Nov 18, 2020 at 5:39 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > I have created a cuttlefish build and have tested with the attached > > > userfaultfd program: > > > > Thanks, that's a good place to start, a few comments: > > > > - While we support Android as a distribution, it isn't a platform that > > we common use for development and testing. At the moment, Fedora is > > probably your best choice for that. > > > I tried setting up a debian/ubuntu system for testing using the > instructions on the selinux-testsuite page, but the system kept > freezing after 'setenforce 1'. I'll try with fedora now. I would expect you to have much better luck with Fedora. > > - Your test program should be written in vanilla C for the > > selinux-testsuite. Looking at the userfaultfdSimple.cc code that > > should be a trivial conversion. > > > > - I think you have a good start on a test for the selinux-testsuite, > > please take a look at the test suite and submit a patch against that > > repo. Ondrej (CC'd) currently maintains the test suite and he may > > have some additional thoughts. > > > > * https://github.com/SELinuxProject/selinux-testsuite > > Thanks a lot for the inputs. I'll start working on this. Great, let us know if you hit any problems. I think we would all like to see this upstream :)
On Mon, Nov 23, 2020 at 2:43 PM Paul Moore <paul@paul-moore.com> wrote: > > On Mon, Nov 23, 2020 at 2:21 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > On Sun, Nov 22, 2020 at 3:14 PM Paul Moore <paul@paul-moore.com> wrote: > > > On Wed, Nov 18, 2020 at 5:39 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > > I have created a cuttlefish build and have tested with the attached > > > > userfaultfd program: > > > > > > Thanks, that's a good place to start, a few comments: > > > > > > - While we support Android as a distribution, it isn't a platform that > > > we common use for development and testing. At the moment, Fedora is > > > probably your best choice for that. > > > > > I tried setting up a debian/ubuntu system for testing using the > > instructions on the selinux-testsuite page, but the system kept > > freezing after 'setenforce 1'. I'll try with fedora now. > > I would expect you to have much better luck with Fedora. Yes. It worked! > > > > - Your test program should be written in vanilla C for the > > > selinux-testsuite. Looking at the userfaultfdSimple.cc code that > > > should be a trivial conversion. > > > > > > - I think you have a good start on a test for the selinux-testsuite, > > > please take a look at the test suite and submit a patch against that > > > repo. Ondrej (CC'd) currently maintains the test suite and he may > > > have some additional thoughts. > > > > > > * https://github.com/SELinuxProject/selinux-testsuite > > > > Thanks a lot for the inputs. I'll start working on this. > > Great, let us know if you hit any problems. I think we would all like > to see this upstream :) > I have the patch ready. I couldn't find any instructions on the testsuite site about patch submission. Can you please tell me how to proceed. > -- > paul moore > www.paul-moore.com
On Tue, Nov 24, 2020 at 3:44 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > On Mon, Nov 23, 2020 at 2:43 PM Paul Moore <paul@paul-moore.com> wrote: > > On Mon, Nov 23, 2020 at 2:21 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > On Sun, Nov 22, 2020 at 3:14 PM Paul Moore <paul@paul-moore.com> wrote: > > > > On Wed, Nov 18, 2020 at 5:39 PM Lokesh Gidra <lokeshgidra@google.com> wrote: > > > > > I have created a cuttlefish build and have tested with the attached > > > > > userfaultfd program: > > > > > > > > Thanks, that's a good place to start, a few comments: > > > > > > > > - While we support Android as a distribution, it isn't a platform that > > > > we common use for development and testing. At the moment, Fedora is > > > > probably your best choice for that. > > > > > > > I tried setting up a debian/ubuntu system for testing using the > > > instructions on the selinux-testsuite page, but the system kept > > > freezing after 'setenforce 1'. I'll try with fedora now. > > > > I would expect you to have much better luck with Fedora. > > Yes. It worked! Excellent :) > > > > - Your test program should be written in vanilla C for the > > > > selinux-testsuite. Looking at the userfaultfdSimple.cc code that > > > > should be a trivial conversion. > > > > > > > > - I think you have a good start on a test for the selinux-testsuite, > > > > please take a look at the test suite and submit a patch against that > > > > repo. Ondrej (CC'd) currently maintains the test suite and he may > > > > have some additional thoughts. > > > > > > > > * https://github.com/SELinuxProject/selinux-testsuite > > > > > > Thanks a lot for the inputs. I'll start working on this. > > > > Great, let us know if you hit any problems. I think we would all like > > to see this upstream :) > > I have the patch ready. I couldn't find any instructions on the > testsuite site about patch submission. Can you please tell me how to > proceed. You can post it to the SELinux mailing list, much like you would do a SELinux kernel patch. I'll take a look and I'll make sure Ondrej looks at it too. Thanks!
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 6b1826fc3658..1c0adcdce7a8 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2927,6 +2927,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, return 0; } +static int selinux_inode_init_security_anon(struct inode *inode, + const struct qstr *name, + const struct inode *context_inode) +{ + const struct task_security_struct *tsec = selinux_cred(current_cred()); + struct common_audit_data ad; + struct inode_security_struct *isec; + int rc; + + if (unlikely(!selinux_initialized(&selinux_state))) + return 0; + + isec = selinux_inode(inode); + + /* + * We only get here once per ephemeral inode. The inode has + * been initialized via inode_alloc_security but is otherwise + * untouched. + */ + + if (context_inode) { + struct inode_security_struct *context_isec = + selinux_inode(context_inode); + isec->sclass = context_isec->sclass; + isec->sid = context_isec->sid; + } else { + isec->sclass = SECCLASS_ANON_INODE; + rc = security_transition_sid( + &selinux_state, tsec->sid, tsec->sid, + isec->sclass, name, &isec->sid); + if (rc) + return rc; + } + + isec->initialized = LABEL_INITIALIZED; + + /* + * Now that we've initialized security, check whether we're + * allowed to actually create this type of anonymous inode. + */ + + ad.type = LSM_AUDIT_DATA_INODE; + ad.u.inode = inode; + + return avc_has_perm(&selinux_state, + tsec->sid, + isec->sid, + isec->sclass, + FILE__CREATE, + &ad); +} + static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode) { return may_create(dir, dentry, SECCLASS_FILE); @@ -6992,6 +7044,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security), LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security), + LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon), LSM_HOOK_INIT(inode_create, selinux_inode_create), LSM_HOOK_INIT(inode_link, selinux_inode_link), LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink), diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 40cebde62856..ba2e01a6955c 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -249,6 +249,8 @@ struct security_class_mapping secclass_map[] = { {"open", "cpu", "kernel", "tracepoint", "read", "write"} }, { "lockdown", { "integrity", "confidentiality", NULL } }, + { "anon_inode", + { COMMON_FILE_PERMS, NULL } }, { NULL } };