[PATCH 0/3] Proposed changes to ptrace in smack

[PATCH 0/3] Proposed changes to ptrace in smack

[Lukasz Pawelczyk]

This is a follow up of an e-mail discussion you had with Rafal Krypa on
December.  This patch set implements "ptrace" smackfs interface, like
you proposed, for tuning of Smack behavior for ptrace. It also fixes
some issues around ptrace in Smack that were found in the process.

Lukasz Pawelczyk (3):
  Smack: fix the subject/object order in smack_ptrace_traceme()
  Smack: unify all ptrace accesses in the smack
  Smack: adds smackfs/ptrace interface

 Documentation/security/Smack.txt |  10 ++++
 security/smack/smack.h           |  10 ++++
 security/smack/smack_access.c    |  38 +++++++++++---
 security/smack/smack_lsm.c       | 106 +++++++++++++++++++++++++++++++++------
 security/smack/smackfs.c         |  74 +++++++++++++++++++++++++++
 5 files changed, 216 insertions(+), 22 deletions(-)

-- 
1.8.5.3

[PATCH 1/3] Smack: fix the subject/object order in smack_ptrace_traceme()

[Lukasz Pawelczyk]

The order of subject/object is currently reversed in
smack_ptrace_traceme(). It is currently checked if the tracee has a
capability to trace tracer and according to this rule a decision is made
whether the tracer will be allowed to trace tracee.

Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
---
 security/smack/smack.h        |  1 +
 security/smack/smack_access.c | 33 ++++++++++++++++++++++++++-------
 security/smack/smack_lsm.c    |  4 ++--
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index d072fd3..b9dfc4e 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -225,6 +225,7 @@ struct inode_smack *new_inode_smack(char *);
  */
 int smk_access_entry(char *, char *, struct list_head *);
 int smk_access(struct smack_known *, char *, int, struct smk_audit_info *);
+int smk_tskacc(struct task_smack *, char *, u32, struct smk_audit_info *);
 int smk_curacc(char *, u32, struct smk_audit_info *);
 struct smack_known *smack_from_secid(const u32);
 char *smk_parse_smack(const char *string, int len);
diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index 14293cd..f161deb 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -192,20 +192,21 @@ out_audit:
 }
 
 /**
- * smk_curacc - determine if current has a specific access to an object
+ * smk_tskacc - determine if a task has a specific access to an object
+ * @tsp: a pointer to the subject task
  * @obj_label: a pointer to the object's Smack label
  * @mode: the access requested, in "MAY" format
  * @a : common audit data
  *
- * This function checks the current subject label/object label pair
+ * This function checks the subject task's label/object label pair
  * in the access rule list and returns 0 if the access is permitted,
- * non zero otherwise. It allows that current may have the capability
+ * non zero otherwise. It allows that the task may have the capability
  * to override the rules.
  */
-int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a)
+int smk_tskacc(struct task_smack *subject, char *obj_label,
+	       u32 mode, struct smk_audit_info *a)
 {
-	struct task_smack *tsp = current_security();
-	struct smack_known *skp = smk_of_task(tsp);
+	struct smack_known *skp = smk_of_task(subject);
 	int may;
 	int rc;
 
@@ -219,7 +220,7 @@ int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a)
 		 * it can further restrict access.
 		 */
 		may = smk_access_entry(skp->smk_known, obj_label,
-					&tsp->smk_rules);
+					&subject->smk_rules);
 		if (may < 0)
 			goto out_audit;
 		if ((mode & may) == mode)
@@ -241,6 +242,24 @@ out_audit:
 	return rc;
 }
 
+/**
+ * smk_curacc - determine if current has a specific access to an object
+ * @obj_label: a pointer to the object's Smack label
+ * @mode: the access requested, in "MAY" format
+ * @a : common audit data
+ *
+ * This function checks the current subject label/object label pair
+ * in the access rule list and returns 0 if the access is permitted,
+ * non zero otherwise. It allows that current may have the capability
+ * to override the rules.
+ */
+int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a)
+{
+	struct task_smack *tsp = current_security();
+
+	return smk_tskacc(tsp, obj_label, mode, a);
+}
+
 #ifdef CONFIG_AUDIT
 /**
  * smack_str_from_perm : helper to transalate an int to a
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index d814e35..48d61f6 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -207,11 +207,11 @@ static int smack_ptrace_traceme(struct task_struct *ptp)
 	if (rc != 0)
 		return rc;
 
-	skp = smk_of_task(task_security(ptp));
+	skp = smk_of_task(current_security());
 	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
 	smk_ad_setfield_u_tsk(&ad, ptp);
 
-	rc = smk_curacc(skp->smk_known, MAY_READWRITE, &ad);
+	rc = smk_tskacc(ptp, skp->smk_known, MAY_READWRITE, &ad);
 	return rc;
 }
 
-- 
1.8.5.3

[PATCH 2/3] Smack: unify all ptrace accesses in the smack

[Lukasz Pawelczyk]

The decision whether we can trace a process is made in the following
functions:
	smack_ptrace_traceme()
	smack_ptrace_access_check()
	smack_bprm_set_creds() (in case the proces is traced)

This patch unifies all those decisions by introducing one function that
checks whether ptrace is allowed: smk_ptrace_rule_check().

This makes possible to actually trace with TRACEME where first the
TRACEME itself must be allowed and then exec() on a traced process.

Additional bugs fixed:
- The decision is made according to the mode parameter that is now correctly
  translated from PTRACE_MODE_* to MAY_* instead of being treated 1:1.
  PTRACE_MODE_READ requires MAY_READ.
  PTRACE_MODE_ATTACH requires MAY_READWRITE.
- Add a smack audit log in case of exec() refused by bprm_set_creds().
- Honor the PTRACE_MODE_NOAUDIT flag and don't put smack audit info
  in case this flag is set.

Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
---
 security/smack/smack_lsm.c | 84 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 71 insertions(+), 13 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 48d61f6..3da13fd 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -157,6 +157,54 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead,
 	return rc;
 }
 
+/**
+ * smk_ptrace_mode - helper function for converting PTRACE_MODE_* into MAY_*
+ * @mode - input mode in form of PTRACE_MODE_*
+ *
+ * Returns a converted MAY_* mode usable by smack rules
+ */
+static inline unsigned int smk_ptrace_mode(unsigned int mode)
+{
+	switch (mode) {
+	case PTRACE_MODE_READ:
+		return MAY_READ;
+	case PTRACE_MODE_ATTACH:
+		return MAY_READWRITE;
+	}
+
+	return 0;
+}
+
+/**
+ * smk_ptrace_rule_check - helper for ptrace access
+ * @tracer: tracer process
+ * @tracee_label: label of the process that's about to be traced
+ * @mode: ptrace attachment mode (PTRACE_MODE_*)
+ * @func: name of the function that called us, used for audit
+ *
+ * Returns 0 on access granted, -error on error
+ */
+static int smk_ptrace_rule_check(struct task_struct *tracer, char *tracee_label,
+				 unsigned int mode, const char *func)
+{
+	int rc;
+	struct smk_audit_info ad, *saip = NULL;
+	struct task_smack *tsp;
+	struct smack_known *skp;
+
+	if ((mode & PTRACE_MODE_NOAUDIT) == 0) {
+		smk_ad_init(&ad, func, LSM_AUDIT_DATA_TASK);
+		smk_ad_setfield_u_tsk(&ad, tracer);
+		saip = &ad;
+	}
+
+	tsp = task_security(tracer);
+	skp = smk_of_task(tsp);
+
+	rc = smk_tskacc(tsp, tracee_label, smk_ptrace_mode(mode), saip);
+	return rc;
+}
+
 /*
  * LSM hooks.
  * We he, that is fun!
@@ -165,16 +213,15 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead,
 /**
  * smack_ptrace_access_check - Smack approval on PTRACE_ATTACH
  * @ctp: child task pointer
- * @mode: ptrace attachment mode
+ * @mode: ptrace attachment mode (PTRACE_MODE_*)
  *
  * Returns 0 if access is OK, an error code otherwise
  *
- * Do the capability checks, and require read and write.
+ * Do the capability checks.
  */
 static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
 {
 	int rc;
-	struct smk_audit_info ad;
 	struct smack_known *skp;
 
 	rc = cap_ptrace_access_check(ctp, mode);
@@ -182,10 +229,8 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
 		return rc;
 
 	skp = smk_of_task(task_security(ctp));
-	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
-	smk_ad_setfield_u_tsk(&ad, ctp);
 
-	rc = smk_curacc(skp->smk_known, mode, &ad);
+	rc = smk_ptrace_rule_check(current, skp->smk_known, mode, __func__);
 	return rc;
 }
 
@@ -195,12 +240,11 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
  *
  * Returns 0 if access is OK, an error code otherwise
  *
- * Do the capability checks, and require read and write.
+ * Do the capability checks, and require PTRACE_MODE_ATTACH.
  */
 static int smack_ptrace_traceme(struct task_struct *ptp)
 {
 	int rc;
-	struct smk_audit_info ad;
 	struct smack_known *skp;
 
 	rc = cap_ptrace_traceme(ptp);
@@ -208,10 +252,9 @@ static int smack_ptrace_traceme(struct task_struct *ptp)
 		return rc;
 
 	skp = smk_of_task(current_security());
-	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
-	smk_ad_setfield_u_tsk(&ad, ptp);
 
-	rc = smk_tskacc(ptp, skp->smk_known, MAY_READWRITE, &ad);
+	rc = smk_ptrace_rule_check(ptp, skp->smk_known,
+				   PTRACE_MODE_ATTACH, __func__);
 	return rc;
 }
 
@@ -453,7 +496,7 @@ static int smack_sb_statfs(struct dentry *dentry)
  * smack_bprm_set_creds - set creds for exec
  * @bprm: the exec information
  *
- * Returns 0 if it gets a blob, -ENOMEM otherwise
+ * Returns 0 if it gets a blob, -EPERM if exec forbidden and -ENOMEM otherwise
  */
 static int smack_bprm_set_creds(struct linux_binprm *bprm)
 {
@@ -473,7 +516,22 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
 	if (isp->smk_task == NULL || isp->smk_task == bsp->smk_task)
 		return 0;
 
-	if (bprm->unsafe)
+	if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) {
+		struct task_struct *tracer;
+		rc = 0;
+
+		rcu_read_lock();
+		tracer = ptrace_parent(current);
+		if (likely(tracer != NULL))
+			rc = smk_ptrace_rule_check(tracer,
+						   isp->smk_task->smk_known,
+						   PTRACE_MODE_ATTACH,
+						   __func__);
+		rcu_read_unlock();
+
+		if (rc != 0)
+			return rc;
+	} else if (bprm->unsafe)
 		return -EPERM;
 
 	bsp->smk_task = isp->smk_task;
-- 
1.8.5.3

[PATCH 3/3] Smack: adds smackfs/ptrace interface

[Lukasz Pawelczyk]

This allows to limit ptrace beyond the regular smack access rules.
It adds a smackfs/ptrace interface that allows smack to be configured
to require equal smack labels for PTRACE_MODE_ATTACH access.
See the changes in Documentation/security/Smack.txt below for details.

Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
---
 Documentation/security/Smack.txt | 10 ++++++
 security/smack/smack.h           |  9 +++++
 security/smack/smack_access.c    |  5 ++-
 security/smack/smack_lsm.c       | 22 +++++++++++-
 security/smack/smackfs.c         | 74 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 118 insertions(+), 2 deletions(-)

diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
index 7a2d30c..5597917 100644
--- a/Documentation/security/Smack.txt
+++ b/Documentation/security/Smack.txt
@@ -204,6 +204,16 @@ onlycap
 	these capabilities are effective at for processes with any
 	label. The value is set by writing the desired label to the
 	file or cleared by writing "-" to the file.
+ptrace
+	This is used to define the current ptrace policy
+	0 - default: this is the policy that relies on smack access rules.
+	    For the PTRACE_READ a subject needs to have a read access on
+	    object. For the PTRACE_ATTACH a read-write access is required.
+	1 - exact: this is the policy that limits PTRACE_ATTACH. Attach is
+	    only allowed when subject's and object's labels are equal.
+	    PTRACE_READ is not affected. Can be overriden with CAP_SYS_PTRACE.
+	2 - draconian: this policy behaves like the 'exact' above with an
+	    exception that it can't be overriden with CAP_SYS_PTRACE.
 revoke-subject
 	Writing a Smack label here sets the access to '-' for all access
 	rules with that subject label.
diff --git a/security/smack/smack.h b/security/smack/smack.h
index b9dfc4e..fade085 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -177,6 +177,14 @@ struct smk_port_label {
 #define SMACK_CIPSO_MAXCATNUM           184     /* 23 * 8 */
 
 /*
+ * Ptrace rules
+ */
+#define SMACK_PTRACE_DEFAULT	0
+#define SMACK_PTRACE_EXACT	1
+#define SMACK_PTRACE_DRACONIAN	2
+#define SMACK_PTRACE_MAX	SMACK_PTRACE_DRACONIAN
+
+/*
  * Flags for untraditional access modes.
  * It shouldn't be necessary to avoid conflicts with definitions
  * in fs.h, but do so anyway.
@@ -245,6 +253,7 @@ extern struct smack_known *smack_net_ambient;
 extern struct smack_known *smack_onlycap;
 extern struct smack_known *smack_syslog_label;
 extern const char *smack_cipso_option;
+extern int smack_ptrace_rule;
 
 extern struct smack_known smack_known_floor;
 extern struct smack_known smack_known_hat;
diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index f161deb..c062e94 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -304,7 +304,10 @@ static void smack_log_callback(struct audit_buffer *ab, void *a)
 	audit_log_untrustedstring(ab, sad->subject);
 	audit_log_format(ab, " object=");
 	audit_log_untrustedstring(ab, sad->object);
-	audit_log_format(ab, " requested=%s", sad->request);
+	if (sad->request[0] == '\0')
+		audit_log_format(ab, " labels_differ");
+	else
+		audit_log_format(ab, " requested=%s", sad->request);
 }
 
 /**
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 3da13fd..1876bfc 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -178,7 +178,8 @@ static inline unsigned int smk_ptrace_mode(unsigned int mode)
 /**
  * smk_ptrace_rule_check - helper for ptrace access
  * @tracer: tracer process
- * @tracee_label: label of the process that's about to be traced
+ * @tracee_label: label of the process that's about to be traced,
+ *                the pointer must originate from smack structures
  * @mode: ptrace attachment mode (PTRACE_MODE_*)
  * @func: name of the function that called us, used for audit
  *
@@ -201,6 +202,25 @@ static int smk_ptrace_rule_check(struct task_struct *tracer, char *tracee_label,
 	tsp = task_security(tracer);
 	skp = smk_of_task(tsp);
 
+	if ((mode & PTRACE_MODE_ATTACH) &&
+	    (smack_ptrace_rule == SMACK_PTRACE_EXACT ||
+	     smack_ptrace_rule == SMACK_PTRACE_DRACONIAN)) {
+		if (skp->smk_known == tracee_label)
+			rc = 0;
+		else if (smack_ptrace_rule == SMACK_PTRACE_DRACONIAN)
+			rc = -EACCES;
+		else if (capable(CAP_SYS_PTRACE))
+			rc = 0;
+		else
+			rc = -EACCES;
+
+		if (saip)
+			smack_log(skp->smk_known, tracee_label, 0, rc, saip);
+
+		return rc;
+	}
+
+	/* In case of rule==SMACK_PTRACE_DEFAULT or mode==PTRACE_MODE_READ */
 	rc = smk_tskacc(tsp, tracee_label, smk_ptrace_mode(mode), saip);
 	return rc;
 }
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 3198cfe..177d878 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -53,6 +53,7 @@ enum smk_inos {
 	SMK_REVOKE_SUBJ	= 18,	/* set rules with subject label to '-' */
 	SMK_CHANGE_RULE	= 19,	/* change or add rules (long labels) */
 	SMK_SYSLOG	= 20,	/* change syslog label) */
+	SMK_PTRACE	= 21,	/* set ptrace rule */
 };
 
 /*
@@ -101,6 +102,15 @@ struct smack_known *smack_onlycap;
 struct smack_known *smack_syslog_label;
 
 /*
+ * Ptrace current rule
+ * SMACK_PTRACE_DEFAULT    regular smack ptrace rules (/proc based)
+ * SMACK_PTRACE_EXACT      labels must match, but can be overriden with
+ *			   CAP_SYS_PTRACE
+ * SMACK_PTRACE_DRACONIAN  lables must match, CAP_SYS_PTRACE has no effect
+ */
+int smack_ptrace_rule = SMACK_PTRACE_DEFAULT;
+
+/*
  * Certain IP addresses may be designated as single label hosts.
  * Packets are sent there unlabeled, but only from tasks that
  * can write to the specified label.
@@ -2244,6 +2254,68 @@ static const struct file_operations smk_syslog_ops = {
 
 
 /**
+ * smk_read_ptrace - read() for /smack/ptrace
+ * @filp: file pointer, not actually used
+ * @buf: where to put the result
+ * @count: maximum to send along
+ * @ppos: where to start
+ *
+ * Returns number of bytes read or error code, as appropriate
+ */
+static ssize_t smk_read_ptrace(struct file *filp, char __user *buf,
+			       size_t count, loff_t *ppos)
+{
+	char temp[32];
+	ssize_t rc;
+
+	if (*ppos != 0)
+		return 0;
+
+	sprintf(temp, "%d\n", smack_ptrace_rule);
+	rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+	return rc;
+}
+
+/**
+ * smk_write_ptrace - write() for /smack/ptrace
+ * @file: file pointer
+ * @buf: data from user space
+ * @count: bytes sent
+ * @ppos: where to start - must be 0
+ */
+static ssize_t smk_write_ptrace(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	char temp[32];
+	int i;
+
+	if (!smack_privileged(CAP_MAC_ADMIN))
+		return -EPERM;
+
+	if (*ppos != 0 || count >= sizeof(temp) || count == 0)
+		return -EINVAL;
+
+	if (copy_from_user(temp, buf, count) != 0)
+		return -EFAULT;
+
+	temp[count] = '\0';
+
+	if (sscanf(temp, "%d", &i) != 1)
+		return -EINVAL;
+	if (i < SMACK_PTRACE_DEFAULT || i > SMACK_PTRACE_MAX)
+		return -EINVAL;
+	smack_ptrace_rule = i;
+
+	return count;
+}
+
+static const struct file_operations smk_ptrace_ops = {
+	.write		= smk_write_ptrace,
+	.read		= smk_read_ptrace,
+	.llseek		= default_llseek,
+};
+
+/**
  * smk_fill_super - fill the smackfs superblock
  * @sb: the empty superblock
  * @data: unused
@@ -2296,6 +2368,8 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
 			"change-rule", &smk_change_rule_ops, S_IRUGO|S_IWUSR},
 		[SMK_SYSLOG] = {
 			"syslog", &smk_syslog_ops, S_IRUGO|S_IWUSR},
+		[SMK_PTRACE] = {
+			"ptrace", &smk_ptrace_ops, S_IRUGO|S_IWUSR},
 		/* last one */
 			{""}
 	};
-- 
1.8.5.3

Re: [PATCH 3/3] Smack: adds smackfs/ptrace interface

[Casey Schaufler]

On 3/11/2014 9:07 AM, Lukasz Pawelczyk wrote:
> This allows to limit ptrace beyond the regular smack access rules.
> It adds a smackfs/ptrace interface that allows smack to be configured
> to require equal smack labels for PTRACE_MODE_ATTACH access.
> See the changes in Documentation/security/Smack.txt below for details.
>
> Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>
> Signed-off-by: Rafal Krypa <r.krypa@samsung.com>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

Applied to git://git.gitorious.org/smack-next/kernel.git smack-for-3.16

> ---
>  Documentation/security/Smack.txt | 10 ++++++
>  security/smack/smack.h           |  9 +++++
>  security/smack/smack_access.c    |  5 ++-
>  security/smack/smack_lsm.c       | 22 +++++++++++-
>  security/smack/smackfs.c         | 74 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 118 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
> index 7a2d30c..5597917 100644
> --- a/Documentation/security/Smack.txt
> +++ b/Documentation/security/Smack.txt
> @@ -204,6 +204,16 @@ onlycap
>  	these capabilities are effective at for processes with any
>  	label. The value is set by writing the desired label to the
>  	file or cleared by writing "-" to the file.
> +ptrace
> +	This is used to define the current ptrace policy
> +	0 - default: this is the policy that relies on smack access rules.
> +	    For the PTRACE_READ a subject needs to have a read access on
> +	    object. For the PTRACE_ATTACH a read-write access is required.
> +	1 - exact: this is the policy that limits PTRACE_ATTACH. Attach is
> +	    only allowed when subject's and object's labels are equal.
> +	    PTRACE_READ is not affected. Can be overriden with CAP_SYS_PTRACE.
> +	2 - draconian: this policy behaves like the 'exact' above with an
> +	    exception that it can't be overriden with CAP_SYS_PTRACE.
>  revoke-subject
>  	Writing a Smack label here sets the access to '-' for all access
>  	rules with that subject label.
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index b9dfc4e..fade085 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -177,6 +177,14 @@ struct smk_port_label {
>  #define SMACK_CIPSO_MAXCATNUM           184     /* 23 * 8 */
>  
>  /*
> + * Ptrace rules
> + */
> +#define SMACK_PTRACE_DEFAULT	0
> +#define SMACK_PTRACE_EXACT	1
> +#define SMACK_PTRACE_DRACONIAN	2
> +#define SMACK_PTRACE_MAX	SMACK_PTRACE_DRACONIAN
> +
> +/*
>   * Flags for untraditional access modes.
>   * It shouldn't be necessary to avoid conflicts with definitions
>   * in fs.h, but do so anyway.
> @@ -245,6 +253,7 @@ extern struct smack_known *smack_net_ambient;
>  extern struct smack_known *smack_onlycap;
>  extern struct smack_known *smack_syslog_label;
>  extern const char *smack_cipso_option;
> +extern int smack_ptrace_rule;
>  
>  extern struct smack_known smack_known_floor;
>  extern struct smack_known smack_known_hat;
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index f161deb..c062e94 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -304,7 +304,10 @@ static void smack_log_callback(struct audit_buffer *ab, void *a)
>  	audit_log_untrustedstring(ab, sad->subject);
>  	audit_log_format(ab, " object=");
>  	audit_log_untrustedstring(ab, sad->object);
> -	audit_log_format(ab, " requested=%s", sad->request);
> +	if (sad->request[0] == '\0')
> +		audit_log_format(ab, " labels_differ");
> +	else
> +		audit_log_format(ab, " requested=%s", sad->request);
>  }
>  
>  /**
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 3da13fd..1876bfc 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -178,7 +178,8 @@ static inline unsigned int smk_ptrace_mode(unsigned int mode)
>  /**
>   * smk_ptrace_rule_check - helper for ptrace access
>   * @tracer: tracer process
> - * @tracee_label: label of the process that's about to be traced
> + * @tracee_label: label of the process that's about to be traced,
> + *                the pointer must originate from smack structures
>   * @mode: ptrace attachment mode (PTRACE_MODE_*)
>   * @func: name of the function that called us, used for audit
>   *
> @@ -201,6 +202,25 @@ static int smk_ptrace_rule_check(struct task_struct *tracer, char *tracee_label,
>  	tsp = task_security(tracer);
>  	skp = smk_of_task(tsp);
>  
> +	if ((mode & PTRACE_MODE_ATTACH) &&
> +	    (smack_ptrace_rule == SMACK_PTRACE_EXACT ||
> +	     smack_ptrace_rule == SMACK_PTRACE_DRACONIAN)) {
> +		if (skp->smk_known == tracee_label)
> +			rc = 0;
> +		else if (smack_ptrace_rule == SMACK_PTRACE_DRACONIAN)
> +			rc = -EACCES;
> +		else if (capable(CAP_SYS_PTRACE))
> +			rc = 0;
> +		else
> +			rc = -EACCES;
> +
> +		if (saip)
> +			smack_log(skp->smk_known, tracee_label, 0, rc, saip);
> +
> +		return rc;
> +	}
> +
> +	/* In case of rule==SMACK_PTRACE_DEFAULT or mode==PTRACE_MODE_READ */
>  	rc = smk_tskacc(tsp, tracee_label, smk_ptrace_mode(mode), saip);
>  	return rc;
>  }
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 3198cfe..177d878 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -53,6 +53,7 @@ enum smk_inos {
>  	SMK_REVOKE_SUBJ	= 18,	/* set rules with subject label to '-' */
>  	SMK_CHANGE_RULE	= 19,	/* change or add rules (long labels) */
>  	SMK_SYSLOG	= 20,	/* change syslog label) */
> +	SMK_PTRACE	= 21,	/* set ptrace rule */
>  };
>  
>  /*
> @@ -101,6 +102,15 @@ struct smack_known *smack_onlycap;
>  struct smack_known *smack_syslog_label;
>  
>  /*
> + * Ptrace current rule
> + * SMACK_PTRACE_DEFAULT    regular smack ptrace rules (/proc based)
> + * SMACK_PTRACE_EXACT      labels must match, but can be overriden with
> + *			   CAP_SYS_PTRACE
> + * SMACK_PTRACE_DRACONIAN  lables must match, CAP_SYS_PTRACE has no effect
> + */
> +int smack_ptrace_rule = SMACK_PTRACE_DEFAULT;
> +
> +/*
>   * Certain IP addresses may be designated as single label hosts.
>   * Packets are sent there unlabeled, but only from tasks that
>   * can write to the specified label.
> @@ -2244,6 +2254,68 @@ static const struct file_operations smk_syslog_ops = {
>  
>  
>  /**
> + * smk_read_ptrace - read() for /smack/ptrace
> + * @filp: file pointer, not actually used
> + * @buf: where to put the result
> + * @count: maximum to send along
> + * @ppos: where to start
> + *
> + * Returns number of bytes read or error code, as appropriate
> + */
> +static ssize_t smk_read_ptrace(struct file *filp, char __user *buf,
> +			       size_t count, loff_t *ppos)
> +{
> +	char temp[32];
> +	ssize_t rc;
> +
> +	if (*ppos != 0)
> +		return 0;
> +
> +	sprintf(temp, "%d\n", smack_ptrace_rule);
> +	rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
> +	return rc;
> +}
> +
> +/**
> + * smk_write_ptrace - write() for /smack/ptrace
> + * @file: file pointer
> + * @buf: data from user space
> + * @count: bytes sent
> + * @ppos: where to start - must be 0
> + */
> +static ssize_t smk_write_ptrace(struct file *file, const char __user *buf,
> +				size_t count, loff_t *ppos)
> +{
> +	char temp[32];
> +	int i;
> +
> +	if (!smack_privileged(CAP_MAC_ADMIN))
> +		return -EPERM;
> +
> +	if (*ppos != 0 || count >= sizeof(temp) || count == 0)
> +		return -EINVAL;
> +
> +	if (copy_from_user(temp, buf, count) != 0)
> +		return -EFAULT;
> +
> +	temp[count] = '\0';
> +
> +	if (sscanf(temp, "%d", &i) != 1)
> +		return -EINVAL;
> +	if (i < SMACK_PTRACE_DEFAULT || i > SMACK_PTRACE_MAX)
> +		return -EINVAL;
> +	smack_ptrace_rule = i;
> +
> +	return count;
> +}
> +
> +static const struct file_operations smk_ptrace_ops = {
> +	.write		= smk_write_ptrace,
> +	.read		= smk_read_ptrace,
> +	.llseek		= default_llseek,
> +};
> +
> +/**
>   * smk_fill_super - fill the smackfs superblock
>   * @sb: the empty superblock
>   * @data: unused
> @@ -2296,6 +2368,8 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
>  			"change-rule", &smk_change_rule_ops, S_IRUGO|S_IWUSR},
>  		[SMK_SYSLOG] = {
>  			"syslog", &smk_syslog_ops, S_IRUGO|S_IWUSR},
> +		[SMK_PTRACE] = {
> +			"ptrace", &smk_ptrace_ops, S_IRUGO|S_IWUSR},
>  		/* last one */
>  			{""}
>  	};

Re: [PATCH 1/3] Smack: fix the subject/object order in smack_ptrace_traceme()

[Casey Schaufler]

On 3/11/2014 9:07 AM, Lukasz Pawelczyk wrote:
> The order of subject/object is currently reversed in
> smack_ptrace_traceme(). It is currently checked if the tracee has a
> capability to trace tracer and according to this rule a decision is made
> whether the tracer will be allowed to trace tracee.
>
> Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>
> Signed-off-by: Rafal Krypa <r.krypa@samsung.com>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

Applied to git://git.gitorious.org/smack-next/kernel.git smack-for-3.16

> ---
>  security/smack/smack.h        |  1 +
>  security/smack/smack_access.c | 33 ++++++++++++++++++++++++++-------
>  security/smack/smack_lsm.c    |  4 ++--
>  3 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index d072fd3..b9dfc4e 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -225,6 +225,7 @@ struct inode_smack *new_inode_smack(char *);
>   */
>  int smk_access_entry(char *, char *, struct list_head *);
>  int smk_access(struct smack_known *, char *, int, struct smk_audit_info *);
> +int smk_tskacc(struct task_smack *, char *, u32, struct smk_audit_info *);
>  int smk_curacc(char *, u32, struct smk_audit_info *);
>  struct smack_known *smack_from_secid(const u32);
>  char *smk_parse_smack(const char *string, int len);
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index 14293cd..f161deb 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -192,20 +192,21 @@ out_audit:
>  }
>  
>  /**
> - * smk_curacc - determine if current has a specific access to an object
> + * smk_tskacc - determine if a task has a specific access to an object
> + * @tsp: a pointer to the subject task
>   * @obj_label: a pointer to the object's Smack label
>   * @mode: the access requested, in "MAY" format
>   * @a : common audit data
>   *
> - * This function checks the current subject label/object label pair
> + * This function checks the subject task's label/object label pair
>   * in the access rule list and returns 0 if the access is permitted,
> - * non zero otherwise. It allows that current may have the capability
> + * non zero otherwise. It allows that the task may have the capability
>   * to override the rules.
>   */
> -int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a)
> +int smk_tskacc(struct task_smack *subject, char *obj_label,
> +	       u32 mode, struct smk_audit_info *a)
>  {
> -	struct task_smack *tsp = current_security();
> -	struct smack_known *skp = smk_of_task(tsp);
> +	struct smack_known *skp = smk_of_task(subject);
>  	int may;
>  	int rc;
>  
> @@ -219,7 +220,7 @@ int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a)
>  		 * it can further restrict access.
>  		 */
>  		may = smk_access_entry(skp->smk_known, obj_label,
> -					&tsp->smk_rules);
> +					&subject->smk_rules);
>  		if (may < 0)
>  			goto out_audit;
>  		if ((mode & may) == mode)
> @@ -241,6 +242,24 @@ out_audit:
>  	return rc;
>  }
>  
> +/**
> + * smk_curacc - determine if current has a specific access to an object
> + * @obj_label: a pointer to the object's Smack label
> + * @mode: the access requested, in "MAY" format
> + * @a : common audit data
> + *
> + * This function checks the current subject label/object label pair
> + * in the access rule list and returns 0 if the access is permitted,
> + * non zero otherwise. It allows that current may have the capability
> + * to override the rules.
> + */
> +int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a)
> +{
> +	struct task_smack *tsp = current_security();
> +
> +	return smk_tskacc(tsp, obj_label, mode, a);
> +}
> +
>  #ifdef CONFIG_AUDIT
>  /**
>   * smack_str_from_perm : helper to transalate an int to a
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d814e35..48d61f6 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -207,11 +207,11 @@ static int smack_ptrace_traceme(struct task_struct *ptp)
>  	if (rc != 0)
>  		return rc;
>  
> -	skp = smk_of_task(task_security(ptp));
> +	skp = smk_of_task(current_security());
>  	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
>  	smk_ad_setfield_u_tsk(&ad, ptp);
>  
> -	rc = smk_curacc(skp->smk_known, MAY_READWRITE, &ad);
> +	rc = smk_tskacc(ptp, skp->smk_known, MAY_READWRITE, &ad);
>  	return rc;
>  }
>  

Re: [PATCH 2/3] Smack: unify all ptrace accesses in the smack

[Casey Schaufler]

On 3/11/2014 9:07 AM, Lukasz Pawelczyk wrote:
> The decision whether we can trace a process is made in the following
> functions:
> 	smack_ptrace_traceme()
> 	smack_ptrace_access_check()
> 	smack_bprm_set_creds() (in case the proces is traced)
>
> This patch unifies all those decisions by introducing one function that
> checks whether ptrace is allowed: smk_ptrace_rule_check().
>
> This makes possible to actually trace with TRACEME where first the
> TRACEME itself must be allowed and then exec() on a traced process.
>
> Additional bugs fixed:
> - The decision is made according to the mode parameter that is now correctly
>   translated from PTRACE_MODE_* to MAY_* instead of being treated 1:1.
>   PTRACE_MODE_READ requires MAY_READ.
>   PTRACE_MODE_ATTACH requires MAY_READWRITE.
> - Add a smack audit log in case of exec() refused by bprm_set_creds().
> - Honor the PTRACE_MODE_NOAUDIT flag and don't put smack audit info
>   in case this flag is set.
>
> Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>
> Signed-off-by: Rafal Krypa <r.krypa@samsung.com>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

Applied to git://git.gitorious.org/smack-next/kernel.git smack-for-3.16

> ---
>  security/smack/smack_lsm.c | 84 +++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 71 insertions(+), 13 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 48d61f6..3da13fd 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -157,6 +157,54 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead,
>  	return rc;
>  }
>  
> +/**
> + * smk_ptrace_mode - helper function for converting PTRACE_MODE_* into MAY_*
> + * @mode - input mode in form of PTRACE_MODE_*
> + *
> + * Returns a converted MAY_* mode usable by smack rules
> + */
> +static inline unsigned int smk_ptrace_mode(unsigned int mode)
> +{
> +	switch (mode) {
> +	case PTRACE_MODE_READ:
> +		return MAY_READ;
> +	case PTRACE_MODE_ATTACH:
> +		return MAY_READWRITE;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * smk_ptrace_rule_check - helper for ptrace access
> + * @tracer: tracer process
> + * @tracee_label: label of the process that's about to be traced
> + * @mode: ptrace attachment mode (PTRACE_MODE_*)
> + * @func: name of the function that called us, used for audit
> + *
> + * Returns 0 on access granted, -error on error
> + */
> +static int smk_ptrace_rule_check(struct task_struct *tracer, char *tracee_label,
> +				 unsigned int mode, const char *func)
> +{
> +	int rc;
> +	struct smk_audit_info ad, *saip = NULL;
> +	struct task_smack *tsp;
> +	struct smack_known *skp;
> +
> +	if ((mode & PTRACE_MODE_NOAUDIT) == 0) {
> +		smk_ad_init(&ad, func, LSM_AUDIT_DATA_TASK);
> +		smk_ad_setfield_u_tsk(&ad, tracer);
> +		saip = &ad;
> +	}
> +
> +	tsp = task_security(tracer);
> +	skp = smk_of_task(tsp);
> +
> +	rc = smk_tskacc(tsp, tracee_label, smk_ptrace_mode(mode), saip);
> +	return rc;
> +}
> +
>  /*
>   * LSM hooks.
>   * We he, that is fun!
> @@ -165,16 +213,15 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead,
>  /**
>   * smack_ptrace_access_check - Smack approval on PTRACE_ATTACH
>   * @ctp: child task pointer
> - * @mode: ptrace attachment mode
> + * @mode: ptrace attachment mode (PTRACE_MODE_*)
>   *
>   * Returns 0 if access is OK, an error code otherwise
>   *
> - * Do the capability checks, and require read and write.
> + * Do the capability checks.
>   */
>  static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
>  {
>  	int rc;
> -	struct smk_audit_info ad;
>  	struct smack_known *skp;
>  
>  	rc = cap_ptrace_access_check(ctp, mode);
> @@ -182,10 +229,8 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
>  		return rc;
>  
>  	skp = smk_of_task(task_security(ctp));
> -	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
> -	smk_ad_setfield_u_tsk(&ad, ctp);
>  
> -	rc = smk_curacc(skp->smk_known, mode, &ad);
> +	rc = smk_ptrace_rule_check(current, skp->smk_known, mode, __func__);
>  	return rc;
>  }
>  
> @@ -195,12 +240,11 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
>   *
>   * Returns 0 if access is OK, an error code otherwise
>   *
> - * Do the capability checks, and require read and write.
> + * Do the capability checks, and require PTRACE_MODE_ATTACH.
>   */
>  static int smack_ptrace_traceme(struct task_struct *ptp)
>  {
>  	int rc;
> -	struct smk_audit_info ad;
>  	struct smack_known *skp;
>  
>  	rc = cap_ptrace_traceme(ptp);
> @@ -208,10 +252,9 @@ static int smack_ptrace_traceme(struct task_struct *ptp)
>  		return rc;
>  
>  	skp = smk_of_task(current_security());
> -	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
> -	smk_ad_setfield_u_tsk(&ad, ptp);
>  
> -	rc = smk_tskacc(ptp, skp->smk_known, MAY_READWRITE, &ad);
> +	rc = smk_ptrace_rule_check(ptp, skp->smk_known,
> +				   PTRACE_MODE_ATTACH, __func__);
>  	return rc;
>  }
>  
> @@ -453,7 +496,7 @@ static int smack_sb_statfs(struct dentry *dentry)
>   * smack_bprm_set_creds - set creds for exec
>   * @bprm: the exec information
>   *
> - * Returns 0 if it gets a blob, -ENOMEM otherwise
> + * Returns 0 if it gets a blob, -EPERM if exec forbidden and -ENOMEM otherwise
>   */
>  static int smack_bprm_set_creds(struct linux_binprm *bprm)
>  {
> @@ -473,7 +516,22 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
>  	if (isp->smk_task == NULL || isp->smk_task == bsp->smk_task)
>  		return 0;
>  
> -	if (bprm->unsafe)
> +	if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) {
> +		struct task_struct *tracer;
> +		rc = 0;
> +
> +		rcu_read_lock();
> +		tracer = ptrace_parent(current);
> +		if (likely(tracer != NULL))
> +			rc = smk_ptrace_rule_check(tracer,
> +						   isp->smk_task->smk_known,
> +						   PTRACE_MODE_ATTACH,
> +						   __func__);
> +		rcu_read_unlock();
> +
> +		if (rc != 0)
> +			return rc;
> +	} else if (bprm->unsafe)
>  		return -EPERM;
>  
>  	bsp->smk_task = isp->smk_task;