diff mbox series

[v9,10/11] SELinux: Add selfattr hooks

Message ID 20230421174259.2458-11-casey@schaufler-ca.com (mailing list archive)
State Handled Elsewhere
Headers show
Series None | expand

Commit Message

Casey Schaufler April 21, 2023, 5:42 p.m. UTC 
Add hooks for setselfattr and getselfattr. These hooks are not very
different from their setprocattr and getprocattr equivalents, and
much of the code is shared.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: selinux@vger.kernel.org
Cc: Paul Moore <paul@paul-moore.com>
---
 security/selinux/hooks.c | 153 +++++++++++++++++++++++++++++++--------
 1 file changed, 123 insertions(+), 30 deletions(-)

Comments

Kees Cook April 21, 2023, 7:57 p.m. UTC | #1 
On Fri, Apr 21, 2023 at 10:42:58AM -0700, Casey Schaufler wrote:
> Add hooks for setselfattr and getselfattr. These hooks are not very
> different from their setprocattr and getprocattr equivalents, and
> much of the code is shared.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> Cc: selinux@vger.kernel.org
> Cc: Paul Moore <paul@paul-moore.com>
> ---
>  security/selinux/hooks.c | 153 +++++++++++++++++++++++++++++++--------
>  1 file changed, 123 insertions(+), 30 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9403aee75981..9bc6206fb1ef 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6348,8 +6348,8 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode)
>  		inode_doinit_with_dentry(inode, dentry);
>  }
>  
> -static int selinux_getprocattr(struct task_struct *p,
> -			       const char *name, char **value)
> +static int selinux_lsm_getattr(unsigned int attr, struct task_struct *p,
> +			       char **value)
>  {
>  	const struct task_security_struct *__tsec;
>  	u32 sid;
> @@ -6367,20 +6367,27 @@ static int selinux_getprocattr(struct task_struct *p,
>  			goto bad;
>  	}
>  
> -	if (!strcmp(name, "current"))
> +	switch (attr) {
> +	case LSM_ATTR_CURRENT:
>  		sid = __tsec->sid;
> -	else if (!strcmp(name, "prev"))
> +		break;
> +	case LSM_ATTR_PREV:
>  		sid = __tsec->osid;
> -	else if (!strcmp(name, "exec"))
> +		break;
> +	case LSM_ATTR_EXEC:
>  		sid = __tsec->exec_sid;
> -	else if (!strcmp(name, "fscreate"))
> +		break;
> +	case LSM_ATTR_FSCREATE:
>  		sid = __tsec->create_sid;
> -	else if (!strcmp(name, "keycreate"))
> +		break;
> +	case LSM_ATTR_KEYCREATE:
>  		sid = __tsec->keycreate_sid;
> -	else if (!strcmp(name, "sockcreate"))
> +		break;
> +	case LSM_ATTR_SOCKCREATE:
>  		sid = __tsec->sockcreate_sid;
> -	else {
> -		error = -EINVAL;
> +		break;
> +	default:
> +		error = -EOPNOTSUPP;
>  		goto bad;
>  	}
>  	rcu_read_unlock();
> @@ -6398,7 +6405,7 @@ static int selinux_getprocattr(struct task_struct *p,
>  	return error;
>  }
>  
> -static int selinux_setprocattr(const char *name, void *value, size_t size)
> +static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
>  {
>  	struct task_security_struct *tsec;
>  	struct cred *new;
> @@ -6409,28 +6416,36 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
>  	/*
>  	 * Basic control over ability to set these attributes at all.
>  	 */
> -	if (!strcmp(name, "exec"))
> +	switch (attr) {
> +	case LSM_ATTR_CURRENT:
> +		error = avc_has_perm(&selinux_state,
> +				     mysid, mysid, SECCLASS_PROCESS,
> +				     PROCESS__SETCURRENT, NULL);
> +		break;
> +	case LSM_ATTR_EXEC:
>  		error = avc_has_perm(&selinux_state,
>  				     mysid, mysid, SECCLASS_PROCESS,
>  				     PROCESS__SETEXEC, NULL);
> -	else if (!strcmp(name, "fscreate"))
> +		break;
> +	case LSM_ATTR_FSCREATE:
>  		error = avc_has_perm(&selinux_state,
>  				     mysid, mysid, SECCLASS_PROCESS,
>  				     PROCESS__SETFSCREATE, NULL);
> -	else if (!strcmp(name, "keycreate"))
> +		break;
> +	case LSM_ATTR_KEYCREATE:
>  		error = avc_has_perm(&selinux_state,
>  				     mysid, mysid, SECCLASS_PROCESS,
>  				     PROCESS__SETKEYCREATE, NULL);
> -	else if (!strcmp(name, "sockcreate"))
> +		break;
> +	case LSM_ATTR_SOCKCREATE:
>  		error = avc_has_perm(&selinux_state,
>  				     mysid, mysid, SECCLASS_PROCESS,
>  				     PROCESS__SETSOCKCREATE, NULL);
> -	else if (!strcmp(name, "current"))
> -		error = avc_has_perm(&selinux_state,
> -				     mysid, mysid, SECCLASS_PROCESS,
> -				     PROCESS__SETCURRENT, NULL);
> -	else
> -		error = -EINVAL;
> +		break;
> +	default:
> +		error = -EOPNOTSUPP;
> +		break;
> +	}
>  	if (error)
>  		return error;
>  
> @@ -6442,13 +6457,14 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
>  		}
>  		error = security_context_to_sid(&selinux_state, value, size,
>  						&sid, GFP_KERNEL);
> -		if (error == -EINVAL && !strcmp(name, "fscreate")) {
> +		if (error == -EINVAL && attr == LSM_ATTR_FSCREATE) {
>  			if (!has_cap_mac_admin(true)) {
>  				struct audit_buffer *ab;
>  				size_t audit_size;
>  
> -				/* We strip a nul only if it is at the end, otherwise the
> -				 * context contains a nul and we should audit that */
> +				/* We strip a nul only if it is at the end,
> +				 * otherwise the context contains a nul and
> +				 * we should audit that */
>  				if (str[size - 1] == '\0')
>  					audit_size = size - 1;
>  				else
> @@ -6459,7 +6475,8 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
>  				if (!ab)
>  					return error;
>  				audit_log_format(ab, "op=fscreate invalid_context=");
> -				audit_log_n_untrustedstring(ab, value, audit_size);
> +				audit_log_n_untrustedstring(ab, value,
> +							    audit_size);
>  				audit_log_end(ab);
>  
>  				return error;
> @@ -6483,11 +6500,11 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
>  	   checks and may_create for the file creation checks. The
>  	   operation will then fail if the context is not permitted. */
>  	tsec = selinux_cred(new);
> -	if (!strcmp(name, "exec")) {
> +	if (attr == LSM_ATTR_EXEC) {
>  		tsec->exec_sid = sid;
> -	} else if (!strcmp(name, "fscreate")) {
> +	} else if (attr == LSM_ATTR_FSCREATE) {
>  		tsec->create_sid = sid;
> -	} else if (!strcmp(name, "keycreate")) {
> +	} else if (attr == LSM_ATTR_KEYCREATE) {
>  		if (sid) {
>  			error = avc_has_perm(&selinux_state, mysid, sid,
>  					     SECCLASS_KEY, KEY__CREATE, NULL);
> @@ -6495,9 +6512,9 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
>  				goto abort_change;
>  		}
>  		tsec->keycreate_sid = sid;
> -	} else if (!strcmp(name, "sockcreate")) {
> +	} else if (attr == LSM_ATTR_SOCKCREATE) {
>  		tsec->sockcreate_sid = sid;
> -	} else if (!strcmp(name, "current")) {
> +	} else if (attr == LSM_ATTR_CURRENT) {
>  		error = -EINVAL;
>  		if (sid == 0)
>  			goto abort_change;
> @@ -6542,6 +6559,80 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
>  	return error;
>  }
>  
> +static int selinux_getselfattr(unsigned int __user attr,
> +			       struct lsm_ctx __user *ctx, size_t *size,
> +			       u32 __user flags)
> +{
> +	char *value;
> +	size_t total_len;
> +	int len;
> +	int rc = 1;
> +
> +	len = selinux_lsm_getattr(attr, current, &value);
> +	if (len < 0)
> +		return len;
> +
> +	total_len = ALIGN(len + sizeof(*ctx), 8);

struct_size(ctx, ctx, len)

> +
> +	if (total_len > *size)
> +		rc = -E2BIG;
> +	else
> +		lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0);
> +
> +	*size = total_len;
> +	return rc;
> +}
> +
> +static int selinux_setselfattr(unsigned int __user attr,
> +			       struct lsm_ctx __user *ctx, size_t __user size,
> +			       u32 __user flags)
> +{
> +	struct lsm_ctx *lctx;
> +	void *context;
> +	int rc;
> +
> +	context = kmalloc(size, GFP_KERNEL);
> +	if (context == NULL)
> +		return -ENOMEM;
> +
> +	lctx = (struct lsm_ctx *)context;
> +	if (copy_from_user(context, ctx, size))
> +		rc = -EFAULT;
> +	else if (lctx->ctx_len > size)
> +		rc = -EINVAL;
> +	else
> +		rc = selinux_lsm_setattr(attr, lctx + 1, lctx->ctx_len);

Same nits as before:
	- "context" isn't needed
	- lctx + 1 doesn't exist: lctx->ctx does
	- "u32 __user" isn't a sane type

> +
> +	kfree(context);
> +	if (rc > 0)
> +		return 0;
> +	return rc;
> +}
> +
> +static int selinux_getprocattr(struct task_struct *p,
> +			       const char *name, char **value)
> +{
> +	unsigned int attr = lsm_name_to_attr(name);
> +	int rc;
> +
> +	if (attr) {
> +		rc = selinux_lsm_getattr(attr, p, value);
> +		if (rc != -EOPNOTSUPP)
> +			return rc;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int selinux_setprocattr(const char *name, void *value, size_t size)
> +{
> +	int attr = lsm_name_to_attr(name);
> +
> +	if (attr)
> +		return selinux_lsm_setattr(attr, value, size);
> +	return -EINVAL;
> +}
> +
>  static int selinux_ismaclabel(const char *name)
>  {
>  	return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0);
> @@ -7183,6 +7274,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>  
>  	LSM_HOOK_INIT(d_instantiate, selinux_d_instantiate),
>  
> +	LSM_HOOK_INIT(getselfattr, selinux_getselfattr),
> +	LSM_HOOK_INIT(setselfattr, selinux_setselfattr),
>  	LSM_HOOK_INIT(getprocattr, selinux_getprocattr),
>  	LSM_HOOK_INIT(setprocattr, selinux_setprocattr),
>  
> -- 
> 2.39.2
>
diff mbox series

Patch

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9403aee75981..9bc6206fb1ef 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6348,8 +6348,8 @@  static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode)
 		inode_doinit_with_dentry(inode, dentry);
 }
 
-static int selinux_getprocattr(struct task_struct *p,
-			       const char *name, char **value)
+static int selinux_lsm_getattr(unsigned int attr, struct task_struct *p,
+			       char **value)
 {
 	const struct task_security_struct *__tsec;
 	u32 sid;
@@ -6367,20 +6367,27 @@  static int selinux_getprocattr(struct task_struct *p,
 			goto bad;
 	}
 
-	if (!strcmp(name, "current"))
+	switch (attr) {
+	case LSM_ATTR_CURRENT:
 		sid = __tsec->sid;
-	else if (!strcmp(name, "prev"))
+		break;
+	case LSM_ATTR_PREV:
 		sid = __tsec->osid;
-	else if (!strcmp(name, "exec"))
+		break;
+	case LSM_ATTR_EXEC:
 		sid = __tsec->exec_sid;
-	else if (!strcmp(name, "fscreate"))
+		break;
+	case LSM_ATTR_FSCREATE:
 		sid = __tsec->create_sid;
-	else if (!strcmp(name, "keycreate"))
+		break;
+	case LSM_ATTR_KEYCREATE:
 		sid = __tsec->keycreate_sid;
-	else if (!strcmp(name, "sockcreate"))
+		break;
+	case LSM_ATTR_SOCKCREATE:
 		sid = __tsec->sockcreate_sid;
-	else {
-		error = -EINVAL;
+		break;
+	default:
+		error = -EOPNOTSUPP;
 		goto bad;
 	}
 	rcu_read_unlock();
@@ -6398,7 +6405,7 @@  static int selinux_getprocattr(struct task_struct *p,
 	return error;
 }
 
-static int selinux_setprocattr(const char *name, void *value, size_t size)
+static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
 {
 	struct task_security_struct *tsec;
 	struct cred *new;
@@ -6409,28 +6416,36 @@  static int selinux_setprocattr(const char *name, void *value, size_t size)
 	/*
 	 * Basic control over ability to set these attributes at all.
 	 */
-	if (!strcmp(name, "exec"))
+	switch (attr) {
+	case LSM_ATTR_CURRENT:
+		error = avc_has_perm(&selinux_state,
+				     mysid, mysid, SECCLASS_PROCESS,
+				     PROCESS__SETCURRENT, NULL);
+		break;
+	case LSM_ATTR_EXEC:
 		error = avc_has_perm(&selinux_state,
 				     mysid, mysid, SECCLASS_PROCESS,
 				     PROCESS__SETEXEC, NULL);
-	else if (!strcmp(name, "fscreate"))
+		break;
+	case LSM_ATTR_FSCREATE:
 		error = avc_has_perm(&selinux_state,
 				     mysid, mysid, SECCLASS_PROCESS,
 				     PROCESS__SETFSCREATE, NULL);
-	else if (!strcmp(name, "keycreate"))
+		break;
+	case LSM_ATTR_KEYCREATE:
 		error = avc_has_perm(&selinux_state,
 				     mysid, mysid, SECCLASS_PROCESS,
 				     PROCESS__SETKEYCREATE, NULL);
-	else if (!strcmp(name, "sockcreate"))
+		break;
+	case LSM_ATTR_SOCKCREATE:
 		error = avc_has_perm(&selinux_state,
 				     mysid, mysid, SECCLASS_PROCESS,
 				     PROCESS__SETSOCKCREATE, NULL);
-	else if (!strcmp(name, "current"))
-		error = avc_has_perm(&selinux_state,
-				     mysid, mysid, SECCLASS_PROCESS,
-				     PROCESS__SETCURRENT, NULL);
-	else
-		error = -EINVAL;
+		break;
+	default:
+		error = -EOPNOTSUPP;
+		break;
+	}
 	if (error)
 		return error;
 
@@ -6442,13 +6457,14 @@  static int selinux_setprocattr(const char *name, void *value, size_t size)
 		}
 		error = security_context_to_sid(&selinux_state, value, size,
 						&sid, GFP_KERNEL);
-		if (error == -EINVAL && !strcmp(name, "fscreate")) {
+		if (error == -EINVAL && attr == LSM_ATTR_FSCREATE) {
 			if (!has_cap_mac_admin(true)) {
 				struct audit_buffer *ab;
 				size_t audit_size;
 
-				/* We strip a nul only if it is at the end, otherwise the
-				 * context contains a nul and we should audit that */
+				/* We strip a nul only if it is at the end,
+				 * otherwise the context contains a nul and
+				 * we should audit that */
 				if (str[size - 1] == '\0')
 					audit_size = size - 1;
 				else
@@ -6459,7 +6475,8 @@  static int selinux_setprocattr(const char *name, void *value, size_t size)
 				if (!ab)
 					return error;
 				audit_log_format(ab, "op=fscreate invalid_context=");
-				audit_log_n_untrustedstring(ab, value, audit_size);
+				audit_log_n_untrustedstring(ab, value,
+							    audit_size);
 				audit_log_end(ab);
 
 				return error;
@@ -6483,11 +6500,11 @@  static int selinux_setprocattr(const char *name, void *value, size_t size)
 	   checks and may_create for the file creation checks. The
 	   operation will then fail if the context is not permitted. */
 	tsec = selinux_cred(new);
-	if (!strcmp(name, "exec")) {
+	if (attr == LSM_ATTR_EXEC) {
 		tsec->exec_sid = sid;
-	} else if (!strcmp(name, "fscreate")) {
+	} else if (attr == LSM_ATTR_FSCREATE) {
 		tsec->create_sid = sid;
-	} else if (!strcmp(name, "keycreate")) {
+	} else if (attr == LSM_ATTR_KEYCREATE) {
 		if (sid) {
 			error = avc_has_perm(&selinux_state, mysid, sid,
 					     SECCLASS_KEY, KEY__CREATE, NULL);
@@ -6495,9 +6512,9 @@  static int selinux_setprocattr(const char *name, void *value, size_t size)
 				goto abort_change;
 		}
 		tsec->keycreate_sid = sid;
-	} else if (!strcmp(name, "sockcreate")) {
+	} else if (attr == LSM_ATTR_SOCKCREATE) {
 		tsec->sockcreate_sid = sid;
-	} else if (!strcmp(name, "current")) {
+	} else if (attr == LSM_ATTR_CURRENT) {
 		error = -EINVAL;
 		if (sid == 0)
 			goto abort_change;
@@ -6542,6 +6559,80 @@  static int selinux_setprocattr(const char *name, void *value, size_t size)
 	return error;
 }
 
+static int selinux_getselfattr(unsigned int __user attr,
+			       struct lsm_ctx __user *ctx, size_t *size,
+			       u32 __user flags)
+{
+	char *value;
+	size_t total_len;
+	int len;
+	int rc = 1;
+
+	len = selinux_lsm_getattr(attr, current, &value);
+	if (len < 0)
+		return len;
+
+	total_len = ALIGN(len + sizeof(*ctx), 8);
+
+	if (total_len > *size)
+		rc = -E2BIG;
+	else
+		lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0);
+
+	*size = total_len;
+	return rc;
+}
+
+static int selinux_setselfattr(unsigned int __user attr,
+			       struct lsm_ctx __user *ctx, size_t __user size,
+			       u32 __user flags)
+{
+	struct lsm_ctx *lctx;
+	void *context;
+	int rc;
+
+	context = kmalloc(size, GFP_KERNEL);
+	if (context == NULL)
+		return -ENOMEM;
+
+	lctx = (struct lsm_ctx *)context;
+	if (copy_from_user(context, ctx, size))
+		rc = -EFAULT;
+	else if (lctx->ctx_len > size)
+		rc = -EINVAL;
+	else
+		rc = selinux_lsm_setattr(attr, lctx + 1, lctx->ctx_len);
+
+	kfree(context);
+	if (rc > 0)
+		return 0;
+	return rc;
+}
+
+static int selinux_getprocattr(struct task_struct *p,
+			       const char *name, char **value)
+{
+	unsigned int attr = lsm_name_to_attr(name);
+	int rc;
+
+	if (attr) {
+		rc = selinux_lsm_getattr(attr, p, value);
+		if (rc != -EOPNOTSUPP)
+			return rc;
+	}
+
+	return -EINVAL;
+}
+
+static int selinux_setprocattr(const char *name, void *value, size_t size)
+{
+	int attr = lsm_name_to_attr(name);
+
+	if (attr)
+		return selinux_lsm_setattr(attr, value, size);
+	return -EINVAL;
+}
+
 static int selinux_ismaclabel(const char *name)
 {
 	return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0);
@@ -7183,6 +7274,8 @@  static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 
 	LSM_HOOK_INIT(d_instantiate, selinux_d_instantiate),
 
+	LSM_HOOK_INIT(getselfattr, selinux_getselfattr),
+	LSM_HOOK_INIT(setselfattr, selinux_setselfattr),
 	LSM_HOOK_INIT(getprocattr, selinux_getprocattr),
 	LSM_HOOK_INIT(setprocattr, selinux_setprocattr),