* [PATCH 1/3] LSM: Add new hook for generic node initialization
2019-01-09 9:10 [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent Ondrej Mosnacek
@ 2019-01-09 9:10 ` Ondrej Mosnacek
2019-01-09 14:35 ` Stephen Smalley
2019-01-09 9:10 ` [PATCH 2/3] selinux: Implement the object_init_security hook Ondrej Mosnacek
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: Ondrej Mosnacek @ 2019-01-09 9:10 UTC (permalink / raw)
To: selinux, Paul Moore
Cc: Stephen Smalley, linux-security-module, Greg Kroah-Hartman,
Tejun Heo, linux-fsdevel, cgroups, Ondrej Mosnacek
This patch introduces a new security hook that is intended for
initializing the security data for newly created pseudo filesystem
objects (such as kernfs nodes) that provide a way of storing a
non-default security context, but need to operate independently from
mounts.
The main motivation is to allow kernfs nodes to inherit the context of
the parent under SELinux, similar to the behavior of
security_inode_init_security(). Other LSMs may implement their own logic
for handling the creation of new nodes.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
include/linux/lsm_hooks.h | 5 +++++
include/linux/security.h | 12 ++++++++++++
security/security.c | 8 ++++++++
3 files changed, 25 insertions(+)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index aaeb7fa24dc4..f2b4c0bf4a7b 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1556,6 +1556,10 @@ union security_list_options {
int (*inode_copy_up)(struct dentry *src, struct cred **new);
int (*inode_copy_up_xattr)(const char *name);
+ int (*object_init_security)(void *parent_ctx, u32 parent_ctxlen,
+ const struct qstr *qstr, u16 mode,
+ void **ctx, u32 *ctxlen);
+
int (*file_permission)(struct file *file, int mask);
int (*file_alloc_security)(struct file *file);
void (*file_free_security)(struct file *file);
@@ -1855,6 +1859,7 @@ struct security_hook_heads {
struct hlist_head inode_getsecid;
struct hlist_head inode_copy_up;
struct hlist_head inode_copy_up_xattr;
+ struct hlist_head object_init_security;
struct hlist_head file_permission;
struct hlist_head file_alloc_security;
struct hlist_head file_free_security;
diff --git a/include/linux/security.h b/include/linux/security.h
index d170a5b031f3..e20d1f378ea4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -315,6 +315,9 @@ int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer
void security_inode_getsecid(struct inode *inode, u32 *secid);
int security_inode_copy_up(struct dentry *src, struct cred **new);
int security_inode_copy_up_xattr(const char *name);
+int security_object_init_security(void *parent_ctx, u32 parent_ctxlen,
+ const struct qstr *qstr, u16 mode,
+ void **ctx, u32 *ctxlen);
int security_file_permission(struct file *file, int mask);
int security_file_alloc(struct file *file);
void security_file_free(struct file *file);
@@ -815,6 +818,15 @@ static inline int security_inode_copy_up_xattr(const char *name)
return -EOPNOTSUPP;
}
+static inline int security_object_init_security(void *parent_ctx,
+ u32 parent_ctxlen,
+ const struct qstr *qstr,
+ u16 mode, void **ctx,
+ u32 *ctxlen)
+{
+ return 0;
+}
+
static inline int security_file_permission(struct file *file, int mask)
{
return 0;
diff --git a/security/security.c b/security/security.c
index 04d173eb93f6..56e77368b87f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -879,6 +879,14 @@ int security_inode_copy_up_xattr(const char *name)
}
EXPORT_SYMBOL(security_inode_copy_up_xattr);
+int security_object_init_security(void *parent_ctx, u32 parent_ctxlen,
+ const struct qstr *qstr, u16 mode,
+ void **ctx, u32 *ctxlen)
+{
+ return call_int_hook(object_init_security, 0, parent_ctx, parent_ctxlen,
+ qstr, mode, ctx, ctxlen);
+}
+
int security_file_permission(struct file *file, int mask)
{
int ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] LSM: Add new hook for generic node initialization
2019-01-09 9:10 ` [PATCH 1/3] LSM: Add new hook for generic node initialization Ondrej Mosnacek
@ 2019-01-09 14:35 ` Stephen Smalley
2019-01-09 16:06 ` Ondrej Mosnacek
0 siblings, 1 reply; 19+ messages in thread
From: Stephen Smalley @ 2019-01-09 14:35 UTC (permalink / raw)
To: Ondrej Mosnacek, selinux, Paul Moore
Cc: linux-security-module, Greg Kroah-Hartman, Tejun Heo,
linux-fsdevel, cgroups
On 1/9/19 4:10 AM, Ondrej Mosnacek wrote:
> This patch introduces a new security hook that is intended for
> initializing the security data for newly created pseudo filesystem
> objects (such as kernfs nodes) that provide a way of storing a
> non-default security context, but need to operate independently from
> mounts.
>
> The main motivation is to allow kernfs nodes to inherit the context of
> the parent under SELinux, similar to the behavior of
> security_inode_init_security(). Other LSMs may implement their own logic
> for handling the creation of new nodes.
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
> include/linux/lsm_hooks.h | 5 +++++
> include/linux/security.h | 12 ++++++++++++
> security/security.c | 8 ++++++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index aaeb7fa24dc4..f2b4c0bf4a7b 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1556,6 +1556,10 @@ union security_list_options {
> int (*inode_copy_up)(struct dentry *src, struct cred **new);
> int (*inode_copy_up_xattr)(const char *name);
>
> + int (*object_init_security)(void *parent_ctx, u32 parent_ctxlen,
> + const struct qstr *qstr, u16 mode,
> + void **ctx, u32 *ctxlen);
You'll want to add a kerneldoc comment for the new hook; see the
existing ones for the other hooks at the top of lsm_hooks.h.
> +
> int (*file_permission)(struct file *file, int mask);
> int (*file_alloc_security)(struct file *file);
> void (*file_free_security)(struct file *file);
> @@ -1855,6 +1859,7 @@ struct security_hook_heads {
> struct hlist_head inode_getsecid;
> struct hlist_head inode_copy_up;
> struct hlist_head inode_copy_up_xattr;
> + struct hlist_head object_init_security;
> struct hlist_head file_permission;
> struct hlist_head file_alloc_security;
> struct hlist_head file_free_security;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index d170a5b031f3..e20d1f378ea4 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -315,6 +315,9 @@ int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer
> void security_inode_getsecid(struct inode *inode, u32 *secid);
> int security_inode_copy_up(struct dentry *src, struct cred **new);
> int security_inode_copy_up_xattr(const char *name);
> +int security_object_init_security(void *parent_ctx, u32 parent_ctxlen,
> + const struct qstr *qstr, u16 mode,
> + void **ctx, u32 *ctxlen);
> int security_file_permission(struct file *file, int mask);
> int security_file_alloc(struct file *file);
> void security_file_free(struct file *file);
> @@ -815,6 +818,15 @@ static inline int security_inode_copy_up_xattr(const char *name)
> return -EOPNOTSUPP;
> }
>
> +static inline int security_object_init_security(void *parent_ctx,
> + u32 parent_ctxlen,
> + const struct qstr *qstr,
> + u16 mode, void **ctx,
> + u32 *ctxlen)
> +{
> + return 0;
> +}
> +
> static inline int security_file_permission(struct file *file, int mask)
> {
> return 0;
> diff --git a/security/security.c b/security/security.c
> index 04d173eb93f6..56e77368b87f 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -879,6 +879,14 @@ int security_inode_copy_up_xattr(const char *name)
> }
> EXPORT_SYMBOL(security_inode_copy_up_xattr);
>
> +int security_object_init_security(void *parent_ctx, u32 parent_ctxlen,
> + const struct qstr *qstr, u16 mode,
> + void **ctx, u32 *ctxlen)
> +{
> + return call_int_hook(object_init_security, 0, parent_ctx, parent_ctxlen,
> + qstr, mode, ctx, ctxlen);
> +}
> +
> int security_file_permission(struct file *file, int mask)
> {
> int ret;
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] LSM: Add new hook for generic node initialization
2019-01-09 14:35 ` Stephen Smalley
@ 2019-01-09 16:06 ` Ondrej Mosnacek
0 siblings, 0 replies; 19+ messages in thread
From: Ondrej Mosnacek @ 2019-01-09 16:06 UTC (permalink / raw)
To: Stephen Smalley
Cc: selinux, Paul Moore, Linux Security Module list,
Greg Kroah-Hartman, Tejun Heo, linux-fsdevel, cgroups
On Wed, Jan 9, 2019 at 3:33 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 1/9/19 4:10 AM, Ondrej Mosnacek wrote:
> > This patch introduces a new security hook that is intended for
> > initializing the security data for newly created pseudo filesystem
> > objects (such as kernfs nodes) that provide a way of storing a
> > non-default security context, but need to operate independently from
> > mounts.
> >
> > The main motivation is to allow kernfs nodes to inherit the context of
> > the parent under SELinux, similar to the behavior of
> > security_inode_init_security(). Other LSMs may implement their own logic
> > for handling the creation of new nodes.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> > include/linux/lsm_hooks.h | 5 +++++
> > include/linux/security.h | 12 ++++++++++++
> > security/security.c | 8 ++++++++
> > 3 files changed, 25 insertions(+)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index aaeb7fa24dc4..f2b4c0bf4a7b 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1556,6 +1556,10 @@ union security_list_options {
> > int (*inode_copy_up)(struct dentry *src, struct cred **new);
> > int (*inode_copy_up_xattr)(const char *name);
> >
> > + int (*object_init_security)(void *parent_ctx, u32 parent_ctxlen,
> > + const struct qstr *qstr, u16 mode,
> > + void **ctx, u32 *ctxlen);
>
> You'll want to add a kerneldoc comment for the new hook; see the
> existing ones for the other hooks at the top of lsm_hooks.h.
Good point, will add that in v2, thanks.
>
> > +
> > int (*file_permission)(struct file *file, int mask);
> > int (*file_alloc_security)(struct file *file);
> > void (*file_free_security)(struct file *file);
> > @@ -1855,6 +1859,7 @@ struct security_hook_heads {
> > struct hlist_head inode_getsecid;
> > struct hlist_head inode_copy_up;
> > struct hlist_head inode_copy_up_xattr;
> > + struct hlist_head object_init_security;
> > struct hlist_head file_permission;
> > struct hlist_head file_alloc_security;
> > struct hlist_head file_free_security;
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index d170a5b031f3..e20d1f378ea4 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -315,6 +315,9 @@ int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer
> > void security_inode_getsecid(struct inode *inode, u32 *secid);
> > int security_inode_copy_up(struct dentry *src, struct cred **new);
> > int security_inode_copy_up_xattr(const char *name);
> > +int security_object_init_security(void *parent_ctx, u32 parent_ctxlen,
> > + const struct qstr *qstr, u16 mode,
> > + void **ctx, u32 *ctxlen);
> > int security_file_permission(struct file *file, int mask);
> > int security_file_alloc(struct file *file);
> > void security_file_free(struct file *file);
> > @@ -815,6 +818,15 @@ static inline int security_inode_copy_up_xattr(const char *name)
> > return -EOPNOTSUPP;
> > }
> >
> > +static inline int security_object_init_security(void *parent_ctx,
> > + u32 parent_ctxlen,
> > + const struct qstr *qstr,
> > + u16 mode, void **ctx,
> > + u32 *ctxlen)
> > +{
> > + return 0;
I just realized I will need to assign *ctx to NULL and *ctxlen to 0
here, since I chose to return 0 by default...
> > +}
> > +
> > static inline int security_file_permission(struct file *file, int mask)
> > {
> > return 0;
> > diff --git a/security/security.c b/security/security.c
> > index 04d173eb93f6..56e77368b87f 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -879,6 +879,14 @@ int security_inode_copy_up_xattr(const char *name)
> > }
> > EXPORT_SYMBOL(security_inode_copy_up_xattr);
> >
> > +int security_object_init_security(void *parent_ctx, u32 parent_ctxlen,
> > + const struct qstr *qstr, u16 mode,
> > + void **ctx, u32 *ctxlen)
> > +{
> > + return call_int_hook(object_init_security, 0, parent_ctx, parent_ctxlen,
> > + qstr, mode, ctx, ctxlen);
Same here, in case there is no object_init_security hook provided by the LSM.
> > +}
> > +
> > int security_file_permission(struct file *file, int mask)
> > {
> > int ret;
> >
>
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/3] selinux: Implement the object_init_security hook
2019-01-09 9:10 [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent Ondrej Mosnacek
2019-01-09 9:10 ` [PATCH 1/3] LSM: Add new hook for generic node initialization Ondrej Mosnacek
@ 2019-01-09 9:10 ` Ondrej Mosnacek
2019-01-09 14:40 ` Stephen Smalley
2019-01-09 9:10 ` [PATCH 3/3] kernfs: Initialize security of newly created nodes Ondrej Mosnacek
2019-01-11 20:50 ` [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent Tejun Heo
3 siblings, 1 reply; 19+ messages in thread
From: Ondrej Mosnacek @ 2019-01-09 9:10 UTC (permalink / raw)
To: selinux, Paul Moore
Cc: Stephen Smalley, linux-security-module, Greg Kroah-Hartman,
Tejun Heo, linux-fsdevel, cgroups, Ondrej Mosnacek
The hook applies the same logic as selinux_determine_inode_label(), with
the exception of the super_block handling, which will be enforced on the
actual inodes by other hooks.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
security/selinux/hooks.c | 41 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 7ce012d9ec51..29c038513504 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3526,6 +3526,45 @@ static int selinux_inode_copy_up_xattr(const char *name)
return -EOPNOTSUPP;
}
+/* file-like object operations */
+
+/* Used e.g. for kernfs_node for newly created nodes */
+static int selinux_object_init_security(void *parent_ctx, u32 parent_ctxlen,
+ const struct qstr *qstr, u16 mode,
+ void **ctx, u32 *ctxlen)
+{
+ const struct task_security_struct *tsec = current_security();
+ u32 parent_sid, newsid, clen;
+ int rc;
+ char *context;
+
+ rc = security_context_to_sid(&selinux_state, parent_ctx, parent_ctxlen,
+ &parent_sid, GFP_KERNEL);
+ if (rc)
+ return rc;
+
+ if (tsec->create_sid) {
+ newsid = tsec->create_sid;
+ } else {
+ u16 secclass = inode_mode_to_security_class(mode);
+
+ rc = security_transition_sid(&selinux_state, tsec->sid,
+ parent_sid, secclass, qstr,
+ &newsid);
+ if (rc)
+ return rc;
+ }
+
+ rc = security_sid_to_context_force(&selinux_state, newsid,
+ &context, &clen);
+ if (rc)
+ return rc;
+
+ *ctx = context;
+ *ctxlen = clen;
+ return 0;
+}
+
/* file security operations */
static int selinux_revalidate_file_permission(struct file *file, int mask)
@@ -6965,6 +7004,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up),
LSM_HOOK_INIT(inode_copy_up_xattr, selinux_inode_copy_up_xattr),
+ LSM_HOOK_INIT(object_init_security, selinux_object_init_security),
+
LSM_HOOK_INIT(file_permission, selinux_file_permission),
LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
LSM_HOOK_INIT(file_free_security, selinux_file_free_security),
--
2.20.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] selinux: Implement the object_init_security hook
2019-01-09 9:10 ` [PATCH 2/3] selinux: Implement the object_init_security hook Ondrej Mosnacek
@ 2019-01-09 14:40 ` Stephen Smalley
2019-01-11 1:58 ` Paul Moore
0 siblings, 1 reply; 19+ messages in thread
From: Stephen Smalley @ 2019-01-09 14:40 UTC (permalink / raw)
To: Ondrej Mosnacek, selinux, Paul Moore
Cc: linux-security-module, Greg Kroah-Hartman, Tejun Heo,
linux-fsdevel, cgroups
On 1/9/19 4:10 AM, Ondrej Mosnacek wrote:
> The hook applies the same logic as selinux_determine_inode_label(), with
> the exception of the super_block handling, which will be enforced on the
> actual inodes by other hooks.
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> security/selinux/hooks.c | 41 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7ce012d9ec51..29c038513504 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3526,6 +3526,45 @@ static int selinux_inode_copy_up_xattr(const char *name)
> return -EOPNOTSUPP;
> }
>
> +/* file-like object operations */
> +
> +/* Used e.g. for kernfs_node for newly created nodes */
> +static int selinux_object_init_security(void *parent_ctx, u32 parent_ctxlen,
> + const struct qstr *qstr, u16 mode,
> + void **ctx, u32 *ctxlen)
> +{
> + const struct task_security_struct *tsec = current_security();
> + u32 parent_sid, newsid, clen;
> + int rc;
> + char *context;
> +
> + rc = security_context_to_sid(&selinux_state, parent_ctx, parent_ctxlen,
> + &parent_sid, GFP_KERNEL);
> + if (rc)
> + return rc;
> +
> + if (tsec->create_sid) {
> + newsid = tsec->create_sid;
> + } else {
> + u16 secclass = inode_mode_to_security_class(mode);
> +
> + rc = security_transition_sid(&selinux_state, tsec->sid,
> + parent_sid, secclass, qstr,
> + &newsid);
> + if (rc)
> + return rc;
> + }
> +
> + rc = security_sid_to_context_force(&selinux_state, newsid,
> + &context, &clen);
> + if (rc)
> + return rc;
> +
> + *ctx = context;
> + *ctxlen = clen;
> + return 0;
> +}
> +
> /* file security operations */
>
> static int selinux_revalidate_file_permission(struct file *file, int mask)
> @@ -6965,6 +7004,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up),
> LSM_HOOK_INIT(inode_copy_up_xattr, selinux_inode_copy_up_xattr),
>
> + LSM_HOOK_INIT(object_init_security, selinux_object_init_security),
> +
> LSM_HOOK_INIT(file_permission, selinux_file_permission),
> LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
> LSM_HOOK_INIT(file_free_security, selinux_file_free_security),
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] selinux: Implement the object_init_security hook
2019-01-09 14:40 ` Stephen Smalley
@ 2019-01-11 1:58 ` Paul Moore
0 siblings, 0 replies; 19+ messages in thread
From: Paul Moore @ 2019-01-11 1:58 UTC (permalink / raw)
To: Stephen Smalley
Cc: Ondrej Mosnacek, selinux, linux-security-module,
Greg Kroah-Hartman, Tejun Heo, linux-fsdevel, cgroups
On Wed, Jan 9, 2019 at 9:38 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 1/9/19 4:10 AM, Ondrej Mosnacek wrote:
> > The hook applies the same logic as selinux_determine_inode_label(), with
> > the exception of the super_block handling, which will be enforced on the
> > actual inodes by other hooks.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>
> Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
Looks good to me too.
> > ---
> > security/selinux/hooks.c | 41 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 7ce012d9ec51..29c038513504 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -3526,6 +3526,45 @@ static int selinux_inode_copy_up_xattr(const char *name)
> > return -EOPNOTSUPP;
> > }
> >
> > +/* file-like object operations */
> > +
> > +/* Used e.g. for kernfs_node for newly created nodes */
> > +static int selinux_object_init_security(void *parent_ctx, u32 parent_ctxlen,
> > + const struct qstr *qstr, u16 mode,
> > + void **ctx, u32 *ctxlen)
> > +{
> > + const struct task_security_struct *tsec = current_security();
> > + u32 parent_sid, newsid, clen;
> > + int rc;
> > + char *context;
> > +
> > + rc = security_context_to_sid(&selinux_state, parent_ctx, parent_ctxlen,
> > + &parent_sid, GFP_KERNEL);
> > + if (rc)
> > + return rc;
> > +
> > + if (tsec->create_sid) {
> > + newsid = tsec->create_sid;
> > + } else {
> > + u16 secclass = inode_mode_to_security_class(mode);
> > +
> > + rc = security_transition_sid(&selinux_state, tsec->sid,
> > + parent_sid, secclass, qstr,
> > + &newsid);
> > + if (rc)
> > + return rc;
> > + }
> > +
> > + rc = security_sid_to_context_force(&selinux_state, newsid,
> > + &context, &clen);
> > + if (rc)
> > + return rc;
> > +
> > + *ctx = context;
> > + *ctxlen = clen;
> > + return 0;
> > +}
> > +
> > /* file security operations */
> >
> > static int selinux_revalidate_file_permission(struct file *file, int mask)
> > @@ -6965,6 +7004,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> > LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up),
> > LSM_HOOK_INIT(inode_copy_up_xattr, selinux_inode_copy_up_xattr),
> >
> > + LSM_HOOK_INIT(object_init_security, selinux_object_init_security),
> > +
> > LSM_HOOK_INIT(file_permission, selinux_file_permission),
> > LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
> > LSM_HOOK_INIT(file_free_security, selinux_file_free_security),
> >
>
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/3] kernfs: Initialize security of newly created nodes
2019-01-09 9:10 [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent Ondrej Mosnacek
2019-01-09 9:10 ` [PATCH 1/3] LSM: Add new hook for generic node initialization Ondrej Mosnacek
2019-01-09 9:10 ` [PATCH 2/3] selinux: Implement the object_init_security hook Ondrej Mosnacek
@ 2019-01-09 9:10 ` Ondrej Mosnacek
2019-01-09 15:44 ` Stephen Smalley
2019-01-11 20:50 ` [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent Tejun Heo
3 siblings, 1 reply; 19+ messages in thread
From: Ondrej Mosnacek @ 2019-01-09 9:10 UTC (permalink / raw)
To: selinux, Paul Moore
Cc: Stephen Smalley, linux-security-module, Greg Kroah-Hartman,
Tejun Heo, linux-fsdevel, cgroups, Ondrej Mosnacek
Use the new security_object_init_security() hook to allow LSMs to
possibly assign a non-default security context to newly created nodes
based on the context of their parent node.
This fixes an issue with cgroupfs under SELinux, where newly created
cgroup subdirectories would not inherit its parent's context if it had
been set explicitly to a non-default value (other than the genfs context
specified by the policy). This can be reproduced as follows:
# mkdir /sys/fs/cgroup/unified/test
# chcon -R system_u:object_r:cgroup_t:s0:c123 /sys/fs/cgroup/unified/test
# ls -lZ /sys/fs/cgroup/unified
total 0
-r--r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.controllers
-rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.max.depth
-rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.max.descendants
-rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.procs
-r--r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.stat
-rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.subtree_control
-rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.threads
drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0 0 Jan 8 04:54 init.scope
drwxr-xr-x. 25 root root system_u:object_r:cgroup_t:s0 0 Jan 8 04:54 system.slice
drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0:c123 0 Jan 8 04:59 test
drwxr-xr-x. 3 root root system_u:object_r:cgroup_t:s0 0 Jan 8 04:55 user.slice
# mkdir /sys/fs/cgroup/unified/test/subdir
Actual result:
# ls -ldZ /sys/fs/cgroup/unified/test/subdir
drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:10 /sys/fs/cgroup/unified/test/subdir
Expected result:
# ls -ldZ /sys/fs/cgroup/unified/test/subdir
drwxr-xr-x. 2 root root unconfined_u:object_r:cgroup_t:s0:c123 0 Jan 8 05:10 /sys/fs/cgroup/unified/test/subdir
Link: https://github.com/SELinuxProject/selinux-kernel/issues/39
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
fs/kernfs/dir.c | 49 ++++++++++++++++++++++++++++++++++---
fs/kernfs/inode.c | 9 +++----
fs/kernfs/kernfs-internal.h | 4 +++
3 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 4ca0b5c18192..8a678a934f65 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -15,6 +15,7 @@
#include <linux/slab.h>
#include <linux/security.h>
#include <linux/hash.h>
+#include <linux/stringhash.h>
#include "kernfs-internal.h"
@@ -617,7 +618,43 @@ struct kernfs_node *kernfs_node_from_dentry(struct dentry *dentry)
return NULL;
}
-static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
+static int kernfs_node_init_security(struct kernfs_node *parent,
+ struct kernfs_node *kn, umode_t mode)
+{
+ struct kernfs_iattrs *attrs;
+ struct qstr q;
+ void *ctx;
+ u32 ctxlen;
+ int ret;
+
+ /* If parent has no explicit context set, leave child unset as well */
+ if (!parent->iattr)
+ return 0;
+ if (!parent->iattr->ia_secdata || !parent->iattr->ia_secdata_len)
+ return 0;
+
+ q.name = kn->name;
+ q.hash_len = hashlen_string(parent, kn->name);
+
+ ret = security_object_init_security(parent->iattr->ia_secdata,
+ parent->iattr->ia_secdata_len,
+ &q, (u16)mode, &ctx, &ctxlen);
+ if (ret)
+ return ret;
+
+ attrs = kernfs_iattrs(kn);
+ if (!attrs) {
+ security_release_secctx(ctx, ctxlen);
+ return -ENOMEM;
+ }
+
+ kernfs_node_setsecdata(attrs, &ctx, &ctxlen);
+ /* The inode is fresh, so the returned ctx is always NULL. */
+ return 0;
+}
+
+static struct kernfs_node *__kernfs_new_node(struct kernfs_node *parent,
+ struct kernfs_root *root,
const char *name, umode_t mode,
kuid_t uid, kgid_t gid,
unsigned flags)
@@ -674,6 +711,12 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
goto err_out3;
}
+ if (parent) {
+ ret = kernfs_node_init_security(parent, kn, mode);
+ if (ret)
+ goto err_out3;
+ }
+
return kn;
err_out3:
@@ -692,7 +735,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
{
struct kernfs_node *kn;
- kn = __kernfs_new_node(kernfs_root(parent),
+ kn = __kernfs_new_node(parent, kernfs_root(parent),
name, mode, uid, gid, flags);
if (kn) {
kernfs_get(parent);
@@ -962,7 +1005,7 @@ struct kernfs_root *kernfs_create_root(struct kernfs_syscall_ops *scops,
INIT_LIST_HEAD(&root->supers);
root->next_generation = 1;
- kn = __kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO,
+ kn = __kernfs_new_node(NULL, root, "", S_IFDIR | S_IRUGO | S_IXUGO,
GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
KERNFS_DIR);
if (!kn) {
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index 80cebcd94c90..e6db8d23437b 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -31,7 +31,7 @@ static const struct inode_operations kernfs_iops = {
.listxattr = kernfs_iop_listxattr,
};
-static struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn)
+struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn)
{
static DEFINE_MUTEX(iattr_mutex);
struct kernfs_iattrs *ret;
@@ -135,8 +135,8 @@ out:
return error;
}
-static int kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
- u32 *secdata_len)
+void kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
+ u32 *secdata_len)
{
void *old_secdata;
size_t old_secdata_len;
@@ -149,7 +149,6 @@ static int kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
*secdata = old_secdata;
*secdata_len = old_secdata_len;
- return 0;
}
ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
@@ -365,7 +364,7 @@ static int kernfs_security_xattr_set(const struct xattr_handler *handler,
return error;
mutex_lock(&kernfs_mutex);
- error = kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
+ kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
mutex_unlock(&kernfs_mutex);
if (secdata)
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index 3d83b114bb08..f6fb2df24c30 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -92,6 +92,10 @@ int kernfs_iop_getattr(const struct path *path, struct kstat *stat,
ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size);
int __kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr);
+struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn);
+void kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
+ u32 *secdata_len);
+
/*
* dir.c
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] kernfs: Initialize security of newly created nodes
2019-01-09 9:10 ` [PATCH 3/3] kernfs: Initialize security of newly created nodes Ondrej Mosnacek
@ 2019-01-09 15:44 ` Stephen Smalley
2019-01-11 2:08 ` Paul Moore
0 siblings, 1 reply; 19+ messages in thread
From: Stephen Smalley @ 2019-01-09 15:44 UTC (permalink / raw)
To: Ondrej Mosnacek, selinux, Paul Moore
Cc: linux-security-module, Greg Kroah-Hartman, Tejun Heo,
linux-fsdevel, cgroups
On 1/9/19 4:10 AM, Ondrej Mosnacek wrote:
> Use the new security_object_init_security() hook to allow LSMs to
> possibly assign a non-default security context to newly created nodes
> based on the context of their parent node.
>
> This fixes an issue with cgroupfs under SELinux, where newly created
> cgroup subdirectories would not inherit its parent's context if it had
> been set explicitly to a non-default value (other than the genfs context
> specified by the policy). This can be reproduced as follows:
>
> # mkdir /sys/fs/cgroup/unified/test
> # chcon -R system_u:object_r:cgroup_t:s0:c123 /sys/fs/cgroup/unified/test
> # ls -lZ /sys/fs/cgroup/unified
> total 0
> -r--r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.controllers
> -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.max.depth
> -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.max.descendants
> -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.procs
> -r--r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.stat
> -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.subtree_control
> -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.threads
> drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0 0 Jan 8 04:54 init.scope
> drwxr-xr-x. 25 root root system_u:object_r:cgroup_t:s0 0 Jan 8 04:54 system.slice
> drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0:c123 0 Jan 8 04:59 test
> drwxr-xr-x. 3 root root system_u:object_r:cgroup_t:s0 0 Jan 8 04:55 user.slice
> # mkdir /sys/fs/cgroup/unified/test/subdir
>
> Actual result:
>
> # ls -ldZ /sys/fs/cgroup/unified/test/subdir
> drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:10 /sys/fs/cgroup/unified/test/subdir
>
> Expected result:
>
> # ls -ldZ /sys/fs/cgroup/unified/test/subdir
> drwxr-xr-x. 2 root root unconfined_u:object_r:cgroup_t:s0:c123 0 Jan 8 05:10 /sys/fs/cgroup/unified/test/subdir
>
> Link: https://github.com/SELinuxProject/selinux-kernel/issues/39
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> fs/kernfs/dir.c | 49 ++++++++++++++++++++++++++++++++++---
> fs/kernfs/inode.c | 9 +++----
> fs/kernfs/kernfs-internal.h | 4 +++
> 3 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index 4ca0b5c18192..8a678a934f65 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -15,6 +15,7 @@
> #include <linux/slab.h>
> #include <linux/security.h>
> #include <linux/hash.h>
> +#include <linux/stringhash.h>
>
> #include "kernfs-internal.h"
>
> @@ -617,7 +618,43 @@ struct kernfs_node *kernfs_node_from_dentry(struct dentry *dentry)
> return NULL;
> }
>
> -static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
> +static int kernfs_node_init_security(struct kernfs_node *parent,
> + struct kernfs_node *kn, umode_t mode)
> +{
> + struct kernfs_iattrs *attrs;
> + struct qstr q;
> + void *ctx;
> + u32 ctxlen;
> + int ret;
> +
> + /* If parent has no explicit context set, leave child unset as well */
> + if (!parent->iattr)
> + return 0;
> + if (!parent->iattr->ia_secdata || !parent->iattr->ia_secdata_len)
> + return 0;
> +
> + q.name = kn->name;
> + q.hash_len = hashlen_string(parent, kn->name);
> +
> + ret = security_object_init_security(parent->iattr->ia_secdata,
> + parent->iattr->ia_secdata_len,
> + &q, (u16)mode, &ctx, &ctxlen);
> + if (ret)
> + return ret;
> +
> + attrs = kernfs_iattrs(kn);
> + if (!attrs) {
> + security_release_secctx(ctx, ctxlen);
> + return -ENOMEM;
> + }
> +
> + kernfs_node_setsecdata(attrs, &ctx, &ctxlen);
> + /* The inode is fresh, so the returned ctx is always NULL. */
> + return 0;
> +}
> +
> +static struct kernfs_node *__kernfs_new_node(struct kernfs_node *parent,
> + struct kernfs_root *root,
> const char *name, umode_t mode,
> kuid_t uid, kgid_t gid,
> unsigned flags)
> @@ -674,6 +711,12 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
> goto err_out3;
> }
>
> + if (parent) {
> + ret = kernfs_node_init_security(parent, kn, mode);
> + if (ret)
> + goto err_out3;
> + }
> +
> return kn;
>
> err_out3:
> @@ -692,7 +735,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
> {
> struct kernfs_node *kn;
>
> - kn = __kernfs_new_node(kernfs_root(parent),
> + kn = __kernfs_new_node(parent, kernfs_root(parent),
> name, mode, uid, gid, flags);
> if (kn) {
> kernfs_get(parent);
> @@ -962,7 +1005,7 @@ struct kernfs_root *kernfs_create_root(struct kernfs_syscall_ops *scops,
> INIT_LIST_HEAD(&root->supers);
> root->next_generation = 1;
>
> - kn = __kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO,
> + kn = __kernfs_new_node(NULL, root, "", S_IFDIR | S_IRUGO | S_IXUGO,
> GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
> KERNFS_DIR);
> if (!kn) {
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index 80cebcd94c90..e6db8d23437b 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -31,7 +31,7 @@ static const struct inode_operations kernfs_iops = {
> .listxattr = kernfs_iop_listxattr,
> };
>
> -static struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn)
> +struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn)
> {
> static DEFINE_MUTEX(iattr_mutex);
> struct kernfs_iattrs *ret;
> @@ -135,8 +135,8 @@ out:
> return error;
> }
>
> -static int kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> - u32 *secdata_len)
> +void kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> + u32 *secdata_len)
> {
> void *old_secdata;
> size_t old_secdata_len;
> @@ -149,7 +149,6 @@ static int kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
>
> *secdata = old_secdata;
> *secdata_len = old_secdata_len;
> - return 0;
> }
>
> ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
> @@ -365,7 +364,7 @@ static int kernfs_security_xattr_set(const struct xattr_handler *handler,
> return error;
>
> mutex_lock(&kernfs_mutex);
> - error = kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
> + kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
> mutex_unlock(&kernfs_mutex);
>
> if (secdata)
> diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
> index 3d83b114bb08..f6fb2df24c30 100644
> --- a/fs/kernfs/kernfs-internal.h
> +++ b/fs/kernfs/kernfs-internal.h
> @@ -92,6 +92,10 @@ int kernfs_iop_getattr(const struct path *path, struct kstat *stat,
> ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size);
> int __kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr);
>
> +struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn);
> +void kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> + u32 *secdata_len);
> +
> /*
> * dir.c
> */
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] kernfs: Initialize security of newly created nodes
2019-01-09 15:44 ` Stephen Smalley
@ 2019-01-11 2:08 ` Paul Moore
0 siblings, 0 replies; 19+ messages in thread
From: Paul Moore @ 2019-01-11 2:08 UTC (permalink / raw)
To: Stephen Smalley
Cc: Ondrej Mosnacek, selinux, linux-security-module,
Greg Kroah-Hartman, Tejun Heo, linux-fsdevel, cgroups
On Wed, Jan 9, 2019 at 10:42 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 1/9/19 4:10 AM, Ondrej Mosnacek wrote:
> > Use the new security_object_init_security() hook to allow LSMs to
> > possibly assign a non-default security context to newly created nodes
> > based on the context of their parent node.
> >
> > This fixes an issue with cgroupfs under SELinux, where newly created
> > cgroup subdirectories would not inherit its parent's context if it had
> > been set explicitly to a non-default value (other than the genfs context
> > specified by the policy). This can be reproduced as follows:
> >
> > # mkdir /sys/fs/cgroup/unified/test
> > # chcon -R system_u:object_r:cgroup_t:s0:c123 /sys/fs/cgroup/unified/test
> > # ls -lZ /sys/fs/cgroup/unified
> > total 0
> > -r--r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.controllers
> > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.max.depth
> > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.max.descendants
> > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.procs
> > -r--r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.stat
> > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.subtree_control
> > -rw-r--r--. 1 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:00 cgroup.threads
> > drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0 0 Jan 8 04:54 init.scope
> > drwxr-xr-x. 25 root root system_u:object_r:cgroup_t:s0 0 Jan 8 04:54 system.slice
> > drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0:c123 0 Jan 8 04:59 test
> > drwxr-xr-x. 3 root root system_u:object_r:cgroup_t:s0 0 Jan 8 04:55 user.slice
> > # mkdir /sys/fs/cgroup/unified/test/subdir
> >
> > Actual result:
> >
> > # ls -ldZ /sys/fs/cgroup/unified/test/subdir
> > drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0 0 Jan 8 05:10 /sys/fs/cgroup/unified/test/subdir
> >
> > Expected result:
> >
> > # ls -ldZ /sys/fs/cgroup/unified/test/subdir
> > drwxr-xr-x. 2 root root unconfined_u:object_r:cgroup_t:s0:c123 0 Jan 8 05:10 /sys/fs/cgroup/unified/test/subdir
> >
> > Link: https://github.com/SELinuxProject/selinux-kernel/issues/39
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>
> Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
Looks okay to me as well.
Some part of me wonders if it might be smart to check for non-NULL
secdata being returned from kernfs_node_setsecdata and doing a WARN.
Maybe I'm overly pessimistic, but I'm pretty sure
kernfs_node_init_security() will get improperly used at some point.
At least we have a comment to *maybe* warn future abusers.
> > ---
> > fs/kernfs/dir.c | 49 ++++++++++++++++++++++++++++++++++---
> > fs/kernfs/inode.c | 9 +++----
> > fs/kernfs/kernfs-internal.h | 4 +++
> > 3 files changed, 54 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> > index 4ca0b5c18192..8a678a934f65 100644
> > --- a/fs/kernfs/dir.c
> > +++ b/fs/kernfs/dir.c
> > @@ -15,6 +15,7 @@
> > #include <linux/slab.h>
> > #include <linux/security.h>
> > #include <linux/hash.h>
> > +#include <linux/stringhash.h>
> >
> > #include "kernfs-internal.h"
> >
> > @@ -617,7 +618,43 @@ struct kernfs_node *kernfs_node_from_dentry(struct dentry *dentry)
> > return NULL;
> > }
> >
> > -static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
> > +static int kernfs_node_init_security(struct kernfs_node *parent,
> > + struct kernfs_node *kn, umode_t mode)
> > +{
> > + struct kernfs_iattrs *attrs;
> > + struct qstr q;
> > + void *ctx;
> > + u32 ctxlen;
> > + int ret;
> > +
> > + /* If parent has no explicit context set, leave child unset as well */
> > + if (!parent->iattr)
> > + return 0;
> > + if (!parent->iattr->ia_secdata || !parent->iattr->ia_secdata_len)
> > + return 0;
> > +
> > + q.name = kn->name;
> > + q.hash_len = hashlen_string(parent, kn->name);
> > +
> > + ret = security_object_init_security(parent->iattr->ia_secdata,
> > + parent->iattr->ia_secdata_len,
> > + &q, (u16)mode, &ctx, &ctxlen);
> > + if (ret)
> > + return ret;
> > +
> > + attrs = kernfs_iattrs(kn);
> > + if (!attrs) {
> > + security_release_secctx(ctx, ctxlen);
> > + return -ENOMEM;
> > + }
> > +
> > + kernfs_node_setsecdata(attrs, &ctx, &ctxlen);
> > + /* The inode is fresh, so the returned ctx is always NULL. */
> > + return 0;
> > +}
> > +
> > +static struct kernfs_node *__kernfs_new_node(struct kernfs_node *parent,
> > + struct kernfs_root *root,
> > const char *name, umode_t mode,
> > kuid_t uid, kgid_t gid,
> > unsigned flags)
> > @@ -674,6 +711,12 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
> > goto err_out3;
> > }
> >
> > + if (parent) {
> > + ret = kernfs_node_init_security(parent, kn, mode);
> > + if (ret)
> > + goto err_out3;
> > + }
> > +
> > return kn;
> >
> > err_out3:
> > @@ -692,7 +735,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
> > {
> > struct kernfs_node *kn;
> >
> > - kn = __kernfs_new_node(kernfs_root(parent),
> > + kn = __kernfs_new_node(parent, kernfs_root(parent),
> > name, mode, uid, gid, flags);
> > if (kn) {
> > kernfs_get(parent);
> > @@ -962,7 +1005,7 @@ struct kernfs_root *kernfs_create_root(struct kernfs_syscall_ops *scops,
> > INIT_LIST_HEAD(&root->supers);
> > root->next_generation = 1;
> >
> > - kn = __kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO,
> > + kn = __kernfs_new_node(NULL, root, "", S_IFDIR | S_IRUGO | S_IXUGO,
> > GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
> > KERNFS_DIR);
> > if (!kn) {
> > diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> > index 80cebcd94c90..e6db8d23437b 100644
> > --- a/fs/kernfs/inode.c
> > +++ b/fs/kernfs/inode.c
> > @@ -31,7 +31,7 @@ static const struct inode_operations kernfs_iops = {
> > .listxattr = kernfs_iop_listxattr,
> > };
> >
> > -static struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn)
> > +struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn)
> > {
> > static DEFINE_MUTEX(iattr_mutex);
> > struct kernfs_iattrs *ret;
> > @@ -135,8 +135,8 @@ out:
> > return error;
> > }
> >
> > -static int kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> > - u32 *secdata_len)
> > +void kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> > + u32 *secdata_len)
> > {
> > void *old_secdata;
> > size_t old_secdata_len;
> > @@ -149,7 +149,6 @@ static int kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> >
> > *secdata = old_secdata;
> > *secdata_len = old_secdata_len;
> > - return 0;
> > }
> >
> > ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
> > @@ -365,7 +364,7 @@ static int kernfs_security_xattr_set(const struct xattr_handler *handler,
> > return error;
> >
> > mutex_lock(&kernfs_mutex);
> > - error = kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
> > + kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
> > mutex_unlock(&kernfs_mutex);
> >
> > if (secdata)
> > diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
> > index 3d83b114bb08..f6fb2df24c30 100644
> > --- a/fs/kernfs/kernfs-internal.h
> > +++ b/fs/kernfs/kernfs-internal.h
> > @@ -92,6 +92,10 @@ int kernfs_iop_getattr(const struct path *path, struct kstat *stat,
> > ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size);
> > int __kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr);
> >
> > +struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn);
> > +void kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> > + u32 *secdata_len);
> > +
> > /*
> > * dir.c
> > */
> >
>
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent
2019-01-09 9:10 [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent Ondrej Mosnacek
` (2 preceding siblings ...)
2019-01-09 9:10 ` [PATCH 3/3] kernfs: Initialize security of newly created nodes Ondrej Mosnacek
@ 2019-01-11 20:50 ` Tejun Heo
2019-01-14 9:14 ` Ondrej Mosnacek
2019-01-15 14:36 ` Stephen Smalley
3 siblings, 2 replies; 19+ messages in thread
From: Tejun Heo @ 2019-01-11 20:50 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: selinux, Paul Moore, Stephen Smalley, linux-security-module,
Greg Kroah-Hartman, linux-fsdevel, cgroups
Hello,
On Wed, Jan 09, 2019 at 10:10:25AM +0100, Ondrej Mosnacek wrote:
> The main motivation for this change is that the userspace users of cgroupfs
> (which is built on kernfs) expect the usual security context inheritance
> to work under SELinux (see [1] and [2]). This functionality is required for
> better confinement of containers under SELinux.
Can you please go into details on what the expected use cases are like
for cgroupfs? It shows up as a filesystem but isn't a real one and
has its own permission scheme for delegation and stuff. If sysfs
hasn't needed selinux support, I'm having a bit of difficulty seeing
why cgroupfs would.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent
2019-01-11 20:50 ` [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent Tejun Heo
@ 2019-01-14 9:14 ` Ondrej Mosnacek
2019-01-14 9:29 ` Ondrej Mosnacek
2019-01-14 15:50 ` Tejun Heo
2019-01-15 14:36 ` Stephen Smalley
1 sibling, 2 replies; 19+ messages in thread
From: Ondrej Mosnacek @ 2019-01-14 9:14 UTC (permalink / raw)
To: Tejun Heo
Cc: selinux, Paul Moore, Stephen Smalley, Linux Security Module list,
Greg Kroah-Hartman, linux-fsdevel, cgroups, Daniel Walsh
On Fri, Jan 11, 2019 at 9:51 PM Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Wed, Jan 09, 2019 at 10:10:25AM +0100, Ondrej Mosnacek wrote:
> > The main motivation for this change is that the userspace users of cgroupfs
> > (which is built on kernfs) expect the usual security context inheritance
> > to work under SELinux (see [1] and [2]). This functionality is required for
> > better confinement of containers under SELinux.
>
> Can you please go into details on what the expected use cases are like
> for cgroupfs? It shows up as a filesystem but isn't a real one and
> has its own permission scheme for delegation and stuff. If sysfs
> hasn't needed selinux support, I'm having a bit of difficulty seeing
> why cgroupfs would.
I'm not sure what are the exact needs of the container people, but
IIUC the goal is to make it possible to have a subtree labeled with a
specific label (that gets inherited by newly created cgroups in that
subtree by default) so that container processes do not need to be
given permissions for the whole cgroupfs tree.
I'm cc'ing Dan Walsh, who should be able to explain the use cases in
more details. Dan, this is related to the cgroupfs labeling problem
([1] and [2]). See [3] for the root of this discussion.
[1] https://github.com/SELinuxProject/selinux-kernel/issues/39
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1553803
[3] https://lore.kernel.org/selinux/CAFqZXNsxfjwDaCWDrqxP736y_3Jm-r=twaHtkkTDtMuym774Jw@mail.gmail.com/T/
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent
2019-01-14 9:14 ` Ondrej Mosnacek
@ 2019-01-14 9:29 ` Ondrej Mosnacek
[not found] ` <64977013-e2a5-809d-7a3f-bffbda9276aa@redhat.com>
2019-01-14 15:50 ` Tejun Heo
1 sibling, 1 reply; 19+ messages in thread
From: Ondrej Mosnacek @ 2019-01-14 9:29 UTC (permalink / raw)
To: Daniel Walsh
Cc: selinux, Tejun Heo, Paul Moore, Stephen Smalley,
Linux Security Module list, Greg Kroah-Hartman, linux-fsdevel,
cgroups
On Mon, Jan 14, 2019 at 10:14 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> [...]
> [3] https://lore.kernel.org/selinux/CAFqZXNsxfjwDaCWDrqxP736y_3Jm-r=twaHtkkTDtMuym774Jw@mail.gmail.com/T/
Actually, this thread belongs to v1 of the patch series, which is archived here:
https://lore.kernel.org/selinux/CAFqZXNu-bHGmUi80UiyW3djcbedycC+0KUyiQuv9-8b+WmrYuA@mail.gmail.com/T/
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent
2019-01-14 9:14 ` Ondrej Mosnacek
2019-01-14 9:29 ` Ondrej Mosnacek
@ 2019-01-14 15:50 ` Tejun Heo
1 sibling, 0 replies; 19+ messages in thread
From: Tejun Heo @ 2019-01-14 15:50 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: selinux, Paul Moore, Stephen Smalley, Linux Security Module list,
Greg Kroah-Hartman, linux-fsdevel, cgroups, Daniel Walsh
Hello,
On Mon, Jan 14, 2019 at 10:14:32AM +0100, Ondrej Mosnacek wrote:
> I'm not sure what are the exact needs of the container people, but
> IIUC the goal is to make it possible to have a subtree labeled with a
> specific label (that gets inherited by newly created cgroups in that
> subtree by default) so that container processes do not need to be
> given permissions for the whole cgroupfs tree.
>
> I'm cc'ing Dan Walsh, who should be able to explain the use cases in
> more details. Dan, this is related to the cgroupfs labeling problem
> ([1] and [2]). See [3] for the root of this discussion.
Let's wait for Dan to respond but I'm pretty skeptical that this is a
good direction.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent
2019-01-11 20:50 ` [PATCH 0/3] Allow initializing the kernfs node's secctx based on its parent Tejun Heo
2019-01-14 9:14 ` Ondrej Mosnacek
@ 2019-01-15 14:36 ` Stephen Smalley
1 sibling, 0 replies; 19+ messages in thread
From: Stephen Smalley @ 2019-01-15 14:36 UTC (permalink / raw)
To: Tejun Heo, Ondrej Mosnacek
Cc: selinux, Paul Moore, linux-security-module, Greg Kroah-Hartman,
linux-fsdevel, cgroups, Jeffrey Vander Stoep, Daniel J Walsh
On 1/11/19 3:50 PM, Tejun Heo wrote:
> Hello,
>
> On Wed, Jan 09, 2019 at 10:10:25AM +0100, Ondrej Mosnacek wrote:
>> The main motivation for this change is that the userspace users of cgroupfs
>> (which is built on kernfs) expect the usual security context inheritance
>> to work under SELinux (see [1] and [2]). This functionality is required for
>> better confinement of containers under SELinux.
>
> Can you please go into details on what the expected use cases are like
> for cgroupfs? It shows up as a filesystem but isn't a real one and
> has its own permission scheme for delegation and stuff. If sysfs
> hasn't needed selinux support, I'm having a bit of difficulty seeing
> why cgroupfs would.
Just to clarify with respect to your last point about sysfs, sysfs
selinux support was first introduced in commit ddd29ec6597125c830f7
("sysfs: Add labeling support for sysfs") for use by libvirt, and this
support was carried over into kernfs, and is extensively used
particularly in Android for controlling access to sysfs files. The
patch set in this series is extending that support to enable inheritance
of security labels set via setxattr from parent to child when
appropriate, which has particularly been requested for cgroup but would
also be useful for sysfs.
^ permalink raw reply [flat|nested] 19+ messages in thread