| Message ID | 20200325230245.184786-3-dancol@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On 3/25/20 7:02 PM, Daniel Colascione wrote: > This change uses the anon_inodes and LSM infrastructure introduced in > the previous patch to give SELinux the ability to control > anonymous-inode files that are created using the new _secure() > anon_inodes functions. > > 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 : file uffd_t "[userfaultfd]"; > allow sysadm_t uffd_t:file { 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> > --- > security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++ > security/selinux/include/classmap.h | 2 ++ > 2 files changed, 56 insertions(+) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 1659b59fb5d7..b9eb45c2e4e5 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2915,6 +2915,59 @@ 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 file_operations *fops, > + 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_state.initialized)) > + return 0; This leaves secure anon inodes created before first policy load with the unlabeled SID rather than defaulting to the SID of the creating task (kernel SID in that situation). Is that what you want? Alternatively you can just remove this test and let it proceed; nothing should be break and the anon inodes will get the kernel SID. > + > + 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, > + SECCLASS_FILE, 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); > @@ -6923,6 +6976,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 986f3ac14282..263750b6aaac 100644 > --- a/security/selinux/include/classmap.h > +++ b/security/selinux/include/classmap.h > @@ -248,6 +248,8 @@ struct security_class_mapping secclass_map[] = { > {"open", "cpu", "kernel", "tracepoint", "read", "write"} }, > { "lockdown", > { "integrity", "confidentiality", NULL } }, > + { "anon_inode", > + { COMMON_FILE_PERMS, NULL } }, > { NULL } > }; > >
On 3/25/20 7:02 PM, Daniel Colascione wrote: > This change uses the anon_inodes and LSM infrastructure introduced in > the previous patch to give SELinux the ability to control > anonymous-inode files that are created using the new _secure() > anon_inodes functions. > > 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 : file uffd_t "[userfaultfd]"; > allow sysadm_t uffd_t:file { create }; These should use :anon_inode rather than :file since the class is no longer file. > > (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> > --- > security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++ > security/selinux/include/classmap.h | 2 ++ > 2 files changed, 56 insertions(+) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 1659b59fb5d7..b9eb45c2e4e5 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2915,6 +2915,59 @@ 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 file_operations *fops, > + 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_state.initialized)) > + 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, > + SECCLASS_FILE, name, &isec->sid); You should use isec->sclass == SECCLASS_ANON_INODE instead of SECCLASS_FILE here.
Thanks for taking a look! On Thu, Mar 26, 2020 at 6:57 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > > On 3/25/20 7:02 PM, Daniel Colascione wrote: > > This change uses the anon_inodes and LSM infrastructure introduced in > > the previous patch to give SELinux the ability to control > > anonymous-inode files that are created using the new _secure() > > anon_inodes functions. > > > > 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 : file uffd_t "[userfaultfd]"; > > allow sysadm_t uffd_t:file { create }; Oops. Will fix. > > (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> > > --- > > security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++ > > security/selinux/include/classmap.h | 2 ++ > > 2 files changed, 56 insertions(+) > > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 1659b59fb5d7..b9eb45c2e4e5 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -2915,6 +2915,59 @@ 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 file_operations *fops, > > + 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_state.initialized)) > > + return 0; > > This leaves secure anon inodes created before first policy load with the > unlabeled SID rather than defaulting to the SID of the creating task > (kernel SID in that situation). Is that what you want? Alternatively > you can just remove this test and let it proceed; nothing should be > break and the anon inodes will get the kernel SID. We talked about this decision on the last thread [1], and I think you mentioned that either the unlabeled or the kernel SID approach would be defensible. Using the unlabeled SID seems more "honest" to me than using the kernel SID: the unlabeled SID says "we don't know", while using kernel SID would be making an affirmative claim that the anonymous inode belongs to the kernel, and claim wouldn't be true. That's why I'm leaning toward the unlabeled approach right now. [1] https://lore.kernel.org/lkml/9ca03838-8686-0007-0971-ee63bf5031da@tycho.nsa.gov/
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 1659b59fb5d7..b9eb45c2e4e5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2915,6 +2915,59 @@ 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 file_operations *fops, + 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_state.initialized)) + 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, + SECCLASS_FILE, 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); @@ -6923,6 +6976,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 986f3ac14282..263750b6aaac 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -248,6 +248,8 @@ struct security_class_mapping secclass_map[] = { {"open", "cpu", "kernel", "tracepoint", "read", "write"} }, { "lockdown", { "integrity", "confidentiality", NULL } }, + { "anon_inode", + { COMMON_FILE_PERMS, NULL } }, { NULL } };
This change uses the anon_inodes and LSM infrastructure introduced in the previous patch to give SELinux the ability to control anonymous-inode files that are created using the new _secure() anon_inodes functions. 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 : file uffd_t "[userfaultfd]"; allow sysadm_t uffd_t:file { 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> --- security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++ security/selinux/include/classmap.h | 2 ++ 2 files changed, 56 insertions(+)