Smack labeling is always done in user space, not in kernel space.
Because kernel can't know which node or preocess should has the certain label
and it shouldn't be neither. Therefore there is a distinct time gap between
file creation and the labeling which might be about few miliseconds or even
indefinite period. This unavoidable and imprecise time gap produces unintended
Smack denial time to time. If it guerantees that labeling is done before any
other access, the time gap doesn't make matter. However if not, the system
will suffer from false alarm.
I've already exeperienced the situation with specific cases.
Mostly, it happens with devtmpfs where udevd applies xattr with defined udev
rules. When kernel module is loaded, device node is newly created and it sends
uevent and notification to processes in wait. However, somtimes uevent may be
handled lately then other processes's access. So, Smack module checks the
authority with uninitialized label, and prohibits the access even the access
authority exists in Smack rule.
At the first time of encounting the problem, I tries to fix the system and user
process to prevent the accesses in ahead of labeling but it wasn't the solution.
Performance is degraded because of compulsory delay.
Another candidate of solution is given label from kernel side.
However, kernel should not be aware of the label, the string, and the
relationship between specific node and label. So I left it in user space.
Label is still given from user space. Kernel only checks there is a pre-assigned
labeling rule for the node. If so, the file will acquire the label as default at
the creation. A file is created with pre-assigend label as like it's done
automatically. So it's called 'Auto Smack labeling'
Pre-assigned label can be given via additional file, 'autolabel' in smackfs.
echo '<File path> <Smack Label>' > /sys/fs/smackfs/autolabel
The label only activates when the file isn't created yet. If file already
exists, then just add the label relationship to table and deactivate it.
When the label is applied, it'll also be deactivated. The label turns to be
enabled again only when the file is removed.
This is a candidate of solution for the specific problem, and might be buggy or
hamful for system-wide security. So I gently request your opinion for more clear
and wise solution for the case that I faced.
Jonghwa Lee (1):
security: smack: Add support automatic Smack labeling
security/smack/Kconfig | 11 ++++
security/smack/smack.h | 23 ++++++++
security/smack/smack_lsm.c | 66 +++++++++++++++++++++
security/smack/smackfs.c | 136 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 236 insertions(+)
--
1.7.9.5
Current Smack object's label is always given by userspace.
So there might be a certain gap between the time of file creation
and the time of applying actual label. And because of the time gap,
it results unintended Smack denial time to time.
If accessing a file occurs ahead of labeling, Smack module will check
the authority with uninitialized label and will prohibit the access
as a result. It shouldn't be happened labeling is done in proper time.
For the case that system can't gueratee that Smack labeling is done
before any other accesses to newly created file, this patch introdues
new interface called 'Automatic Smack labling'.
The label can be defined before file creation with absolute path.
Then, when target file is created in generic manner, the pre-defined
label is applied automatically at once.
Pre-defined label format follows below form.
<File path> <Smack Label>
And it can add new entry to autolabel table via 'autolabel' in smackfs.
echo '<File path> <Smack Label>' > /sys/fs/smackfs/autolabel
To view entries of autolabel table,
$cat /sys/fs/smackfs/autolabel
/dev/device00 Label:A <enabled>
/run/userfile0 Label:B <disabled>
Signed-off-by: Jonghwa Lee <[email protected]>
---
security/smack/Kconfig | 11 ++++
security/smack/smack.h | 23 ++++++++
security/smack/smack_lsm.c | 66 +++++++++++++++++++++
security/smack/smackfs.c | 136 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 236 insertions(+)
diff --git a/security/smack/Kconfig b/security/smack/Kconfig
index 271adae..002aa01 100644
--- a/security/smack/Kconfig
+++ b/security/smack/Kconfig
@@ -30,6 +30,17 @@ config SECURITY_SMACK_BRINGUP
"permissive" mode of other systems.
If you are unsure how to answer this question, answer N.
+config SECURITY_SMACK_AUTOLABEL
+ bool "Enable Automatic Smack Labeling"
+ depends on SECURITY_SMACK
+ default n
+ help
+ To remove out the gap between file creation and actual labeling,
+ this option gives the additional interface for automatic labeling.
+ With this option enabled, a file will posseses the assigned label
+ in autolabel table at creation.
+ If you are unsure how to answer this question, answer N.
+
config SECURITY_SMACK_NETFILTER
bool "Packet marking using secmarks for netfilter"
depends on SECURITY_SMACK
diff --git a/security/smack/smack.h b/security/smack/smack.h
index fff0c61..8b7ee83 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -24,6 +24,7 @@
#include <linux/list.h>
#include <linux/rculist.h>
#include <linux/lsm_audit.h>
+#include <linux/namei.h>
/*
* Use IPv6 port labeling if IPv6 is enabled and secmarks
@@ -106,6 +107,9 @@ struct inode_smack {
struct smack_known *smk_inode; /* label of the fso */
struct smack_known *smk_task; /* label of the task */
struct smack_known *smk_mmap; /* label of the mmap domain */
+#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
+ struct smack_auto_label *smk_auto;
+#endif
struct mutex smk_lock; /* initialization lock */
int smk_flags; /* smack inode flags */
};
@@ -480,4 +484,23 @@ static inline void smk_ad_setfield_u_net_sk(struct smk_audit_info *a,
}
#endif
+#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
+struct smack_auto_label {
+ bool disabled;
+ char *fpath;
+ struct list_head entry;
+ struct smack_known *label;
+ struct inode *inode;
+};
+
+extern struct list_head smack_autolabel_list;
+extern struct mutex smack_autolabel_lock;
+
+extern struct smack_known *smk_inode_bind_autolabel(struct inode *);
+extern void smk_inode_unbind_autolabel(struct inode *);
+#else
+#define smk_inode_bind_autolabel(inode) NULL
+#define smk_inode_unbind_autolabel(inode) do { } while (0)
+#endif /* CONFIG_SECURITY_SMACK_AUTOLABEL */
+
#endif /* _SECURITY_SMACK_H */
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 996c889..851e8b4 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -249,6 +249,65 @@ static int smk_bu_credfile(const struct cred *cred, struct file *file,
#define smk_bu_credfile(cred, file, mode, RC) (RC)
#endif
+#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
+/**
+ * smk_inode_bind_autolabel - Check assigned label for this inode
+ * @ip : the object of inode
+ *
+ * Return pre-assigned smack_known when exists, NULL if not.
+ */
+struct smack_known *smk_inode_bind_autolabel(struct inode *ip)
+{
+ struct smack_auto_label *smk_auto;
+ struct inode_smack *isp;
+ struct smack_known *smk = NULL;
+ struct path p;
+
+ if (list_empty(&smack_autolabel_list))
+ goto out;
+
+ mutex_lock(&smack_autolabel_lock);
+ list_for_each_entry(smk_auto, &smack_autolabel_list, entry) {
+ if (smk_auto->disabled)
+ continue;
+
+ if (!smk_auto->inode) {
+ if (kern_path(smk_auto->fpath, 0, &p))
+ continue;
+ else
+ smk_auto->inode = p.dentry->d_inode;
+ }
+
+ if (smk_auto->inode == ip) {
+ isp = ip->i_security;
+ isp->smk_auto = smk_auto;
+ smk = smk_auto->label;
+ smk_auto->disabled = true;
+ break;
+ }
+ }
+ mutex_unlock(&smack_autolabel_lock);
+out:
+ return smk;
+}
+
+/**
+ * smk_inode_bind_autolabel - Re-activate autolabel for use in future
+ * @ip : the object of inode
+ */
+void smk_inode_unbind_autolabel(struct inode *ip)
+{
+ struct inode_smack *isp = ip->i_security;
+ struct smack_auto_label *smk_auto = isp->smk_auto;
+
+ if (smk_auto) {
+ smk_auto->inode = NULL;
+ smk_auto->disabled = false;
+ isp->smk_auto = NULL;
+ }
+}
+#endif /* CONFIG_SECURITY_SMACK_AUTOLABEL */
+
/**
* smk_fetch - Fetch the smack label from a file.
* @name: type of the label (attribute)
@@ -1089,6 +1148,8 @@ static int smack_inode_unlink(struct inode *dir, struct dentry *dentry)
smk_ad_setfield_u_fs_inode(&ad, dir);
rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
rc = smk_bu_inode(dir, MAY_WRITE, rc);
+ if (rc == 0)
+ smk_inode_unbind_autolabel(ip);
}
return rc;
}
@@ -1122,6 +1183,8 @@ static int smack_inode_rmdir(struct inode *dir, struct dentry *dentry)
smk_ad_setfield_u_fs_inode(&ad, dir);
rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
rc = smk_bu_inode(dir, MAY_WRITE, rc);
+ if (rc == 0)
+ smk_inode_unbind_autolabel(d_backing_inode(dentry));
}
return rc;
@@ -3445,6 +3508,9 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
if (!IS_ERR_OR_NULL(skp))
final = skp;
+ skp = smk_inode_bind_autolabel(inode);
+ if (skp != NULL)
+ final = skp;
/*
* Transmuting directory
*/
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index c20b154..28380ba 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -61,6 +61,9 @@ enum smk_inos {
#if IS_ENABLED(CONFIG_IPV6)
SMK_NET6ADDR = 23, /* single label IPv6 hosts */
#endif /* CONFIG_IPV6 */
+#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
+ SMK_AUTOLABEL = 24,
+#endif
};
/*
@@ -746,6 +749,135 @@ static void smk_cipso_doi(void)
}
}
+#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
+LIST_HEAD(smack_autolabel_list);
+DEFINE_MUTEX(smack_autolabel_lock);
+static int smk_add_autolabel(const char *fpath, const char *label)
+{
+ struct smack_auto_label *smk_auto;
+ struct smack_known *smk;
+ struct path p;
+
+ /*
+ * Check whether given file path already exists in entry
+ */
+ mutex_lock(&smack_autolabel_lock);
+ list_for_each_entry(smk_auto, &smack_autolabel_list, entry) {
+ if (!strcmp(smk_auto->fpath, fpath)) {
+ smk = smk_import_entry(label, 0);
+ if (!IS_ERR(smk))
+ smk_auto->label = smk;
+ mutex_unlock(&smack_autolabel_lock);
+ return 0;
+ }
+ }
+ mutex_unlock(&smack_autolabel_lock);
+
+ /*
+ * Allocation for new autolabel candidate
+ */
+ smk_auto = kzalloc(sizeof(*smk_auto), GFP_KERNEL);
+ if (smk_auto == NULL)
+ return -ENOMEM;
+
+ smk_auto->fpath = kzalloc(strlen(fpath), GFP_KERNEL);
+ if (smk_auto->fpath == NULL) {
+ kfree(smk_auto);
+ return -ENOMEM;
+ }
+
+ strcpy(smk_auto->fpath, fpath);
+ smk_auto->label = smk_import_entry(label, 0);
+ if (IS_ERR(smk_auto->label)) {
+ kfree(smk_auto->fpath);
+ kfree(smk_auto);
+ return PTR_ERR(smk_auto->label);
+ }
+
+ mutex_lock(&smack_autolabel_lock);
+ list_add(&smk_auto->entry, &smack_autolabel_list);
+ mutex_unlock(&smack_autolabel_lock);
+
+ /*
+ * If path is already existed, bind autolabel to it.
+ */
+ if (!kern_path(fpath, 0, &p))
+ smk_inode_bind_autolabel(p.dentry->d_inode);
+
+ return 0;
+}
+
+static void *autolabel_seq_start(struct seq_file *s, loff_t *pos)
+{
+ return smk_seq_start(s, pos, &smack_autolabel_list);
+}
+
+static void *autolabel_seq_next(struct seq_file *s, void *v, loff_t *pos)
+{
+ return smk_seq_next(s, v, pos, &smack_autolabel_list);
+}
+
+static int autolabel_seq_show(struct seq_file *s, void *v)
+{
+ struct list_head *list = v;
+ struct smack_auto_label *smk_auto =
+ list_entry(list, struct smack_auto_label, entry);
+
+ seq_printf(s, "%s %s %s\n",
+ smk_auto->fpath, smk_auto->label->smk_known,
+ smk_auto->disabled ? "(disabled)" : "(enabled)");
+
+ return 0;
+}
+
+static const struct seq_operations autolabel_seq_ops = {
+ .start = autolabel_seq_start,
+ .next = autolabel_seq_next,
+ .show = autolabel_seq_show,
+ .stop = smk_seq_stop,
+};
+
+static int smk_open_autolabel(struct inode *inode, struct file *file)
+{
+ return seq_open(file, &autolabel_seq_ops);
+}
+
+static ssize_t smk_write_autolabel(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ char *data;
+ char *tok[2];
+ int ret;
+
+ data = kzalloc(count + 1, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ if (copy_from_user(data, buf, count))
+ return -EFAULT;
+
+ if (data[0] != '/')
+ return -EINVAL;
+
+ tok[0] = strsep(&data, " ");
+ tok[1] = data;
+
+ ret = smk_add_autolabel(tok[0], tok[1]);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static const struct file_operations smk_autolabel_ops = {
+ .open = smk_open_autolabel,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .write = smk_write_autolabel,
+ .release = seq_release,
+};
+#endif /* CONFIG_SECURITY_SMACK_AUTOLABEL */
+
/**
* smk_unlbl_ambient - initialize the unlabeled domain
* @oldambient: previous domain string
@@ -2824,6 +2956,10 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
[SMK_NET6ADDR] = {
"ipv6host", &smk_net6addr_ops, S_IRUGO|S_IWUSR},
#endif /* CONFIG_IPV6 */
+#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
+ [SMK_AUTOLABEL] = {
+ "autolabel", &smk_autolabel_ops, S_IRUGO|S_IWUSR},
+#endif
/* last one */
{""}
};
--
1.7.9.5
On 8/26/2015 6:58 PM, Jonghwa Lee wrote:
> Current Smack object's label is always given by userspace.
> So there might be a certain gap between the time of file creation
> and the time of applying actual label. And because of the time gap,
> it results unintended Smack denial time to time.
>
> If accessing a file occurs ahead of labeling, Smack module will check
> the authority with uninitialized label and will prohibit the access
> as a result. It shouldn't be happened labeling is done in proper time.
The Smack label is assigned when the inode is created.
The value will depend on the filesystem type, the state
of transmute attributes, and the Smack label of the
creating process. But it will always be assigned.
>
> For the case that system can't gueratee that Smack labeling is done
> before any other accesses to newly created file, this patch introdues
> new interface called 'Automatic Smack labling'.
The system guarantees that the Smack labeling is done
before any other access can be attempted in all cases.
Can you describe a situation where this does not occur?
> The label can be defined before file creation with absolute path.
You could in a pathname based system like AppArmor or TOMOYO,
but Smack is an object based system. You can't count on the
pathname to identify the object you care about. There are too
many facilities that can manipulate the filesystem namespace.
You have to deal with chroot and filesystem namespaces just to
start.
That, and the performance impact would be prohibitive.
> Then, when target file is created in generic manner, the pre-defined
> label is applied automatically at once.
>
> Pre-defined label format follows below form.
>
> <File path> <Smack Label>
>
> And it can add new entry to autolabel table via 'autolabel' in smackfs.
>
> echo '<File path> <Smack Label>' > /sys/fs/smackfs/autolabel
>
> To view entries of autolabel table,
>
> $cat /sys/fs/smackfs/autolabel
> /dev/device00 Label:A <enabled>
> /run/userfile0 Label:B <disabled>
>
> Signed-off-by: Jonghwa Lee <[email protected]>
Nacked-by: Casey Schaufler <[email protected]>
> ---
> security/smack/Kconfig | 11 ++++
> security/smack/smack.h | 23 ++++++++
> security/smack/smack_lsm.c | 66 +++++++++++++++++++++
> security/smack/smackfs.c | 136 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 236 insertions(+)
>
> diff --git a/security/smack/Kconfig b/security/smack/Kconfig
> index 271adae..002aa01 100644
> --- a/security/smack/Kconfig
> +++ b/security/smack/Kconfig
> @@ -30,6 +30,17 @@ config SECURITY_SMACK_BRINGUP
> "permissive" mode of other systems.
> If you are unsure how to answer this question, answer N.
>
> +config SECURITY_SMACK_AUTOLABEL
> + bool "Enable Automatic Smack Labeling"
> + depends on SECURITY_SMACK
> + default n
> + help
> + To remove out the gap between file creation and actual labeling,
> + this option gives the additional interface for automatic labeling.
> + With this option enabled, a file will posseses the assigned label
> + in autolabel table at creation.
> + If you are unsure how to answer this question, answer N.
> +
> config SECURITY_SMACK_NETFILTER
> bool "Packet marking using secmarks for netfilter"
> depends on SECURITY_SMACK
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index fff0c61..8b7ee83 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -24,6 +24,7 @@
> #include <linux/list.h>
> #include <linux/rculist.h>
> #include <linux/lsm_audit.h>
> +#include <linux/namei.h>
>
> /*
> * Use IPv6 port labeling if IPv6 is enabled and secmarks
> @@ -106,6 +107,9 @@ struct inode_smack {
> struct smack_known *smk_inode; /* label of the fso */
> struct smack_known *smk_task; /* label of the task */
> struct smack_known *smk_mmap; /* label of the mmap domain */
> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
> + struct smack_auto_label *smk_auto;
> +#endif
> struct mutex smk_lock; /* initialization lock */
> int smk_flags; /* smack inode flags */
> };
> @@ -480,4 +484,23 @@ static inline void smk_ad_setfield_u_net_sk(struct smk_audit_info *a,
> }
> #endif
>
> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
> +struct smack_auto_label {
> + bool disabled;
> + char *fpath;
> + struct list_head entry;
> + struct smack_known *label;
> + struct inode *inode;
> +};
> +
> +extern struct list_head smack_autolabel_list;
> +extern struct mutex smack_autolabel_lock;
> +
> +extern struct smack_known *smk_inode_bind_autolabel(struct inode *);
> +extern void smk_inode_unbind_autolabel(struct inode *);
> +#else
> +#define smk_inode_bind_autolabel(inode) NULL
> +#define smk_inode_unbind_autolabel(inode) do { } while (0)
> +#endif /* CONFIG_SECURITY_SMACK_AUTOLABEL */
> +
> #endif /* _SECURITY_SMACK_H */
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 996c889..851e8b4 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -249,6 +249,65 @@ static int smk_bu_credfile(const struct cred *cred, struct file *file,
> #define smk_bu_credfile(cred, file, mode, RC) (RC)
> #endif
>
> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
> +/**
> + * smk_inode_bind_autolabel - Check assigned label for this inode
> + * @ip : the object of inode
> + *
> + * Return pre-assigned smack_known when exists, NULL if not.
> + */
> +struct smack_known *smk_inode_bind_autolabel(struct inode *ip)
> +{
> + struct smack_auto_label *smk_auto;
> + struct inode_smack *isp;
> + struct smack_known *smk = NULL;
> + struct path p;
> +
> + if (list_empty(&smack_autolabel_list))
> + goto out;
Poor coding style. You can safely return NULL. There is no reason
to goto out.
> +
> + mutex_lock(&smack_autolabel_lock);
> + list_for_each_entry(smk_auto, &smack_autolabel_list, entry) {
> + if (smk_auto->disabled)
> + continue;
> +
> + if (!smk_auto->inode) {
> + if (kern_path(smk_auto->fpath, 0, &p))
Do you have any idea of the impact this would have on
system performance?
> + continue;
> + else
> + smk_auto->inode = p.dentry->d_inode;
> + }
> +
> + if (smk_auto->inode == ip) {
> + isp = ip->i_security;
> + isp->smk_auto = smk_auto;
> + smk = smk_auto->label;
> + smk_auto->disabled = true;
> + break;
> + }
> + }
> + mutex_unlock(&smack_autolabel_lock);
> +out:
> + return smk;
> +}
> +
> +/**
> + * smk_inode_bind_autolabel - Re-activate autolabel for use in future
> + * @ip : the object of inode
> + */
> +void smk_inode_unbind_autolabel(struct inode *ip)
> +{
> + struct inode_smack *isp = ip->i_security;
> + struct smack_auto_label *smk_auto = isp->smk_auto;
> +
> + if (smk_auto) {
> + smk_auto->inode = NULL;
> + smk_auto->disabled = false;
> + isp->smk_auto = NULL;
> + }
> +}
> +#endif /* CONFIG_SECURITY_SMACK_AUTOLABEL */
> +
> /**
> * smk_fetch - Fetch the smack label from a file.
> * @name: type of the label (attribute)
> @@ -1089,6 +1148,8 @@ static int smack_inode_unlink(struct inode *dir, struct dentry *dentry)
> smk_ad_setfield_u_fs_inode(&ad, dir);
> rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
> rc = smk_bu_inode(dir, MAY_WRITE, rc);
> + if (rc == 0)
> + smk_inode_unbind_autolabel(ip);
> }
> return rc;
> }
> @@ -1122,6 +1183,8 @@ static int smack_inode_rmdir(struct inode *dir, struct dentry *dentry)
> smk_ad_setfield_u_fs_inode(&ad, dir);
> rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
> rc = smk_bu_inode(dir, MAY_WRITE, rc);
> + if (rc == 0)
> + smk_inode_unbind_autolabel(d_backing_inode(dentry));
> }
>
> return rc;
> @@ -3445,6 +3508,9 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
> if (!IS_ERR_OR_NULL(skp))
> final = skp;
>
> + skp = smk_inode_bind_autolabel(inode);
> + if (skp != NULL)
> + final = skp;
> /*
> * Transmuting directory
> */
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index c20b154..28380ba 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -61,6 +61,9 @@ enum smk_inos {
> #if IS_ENABLED(CONFIG_IPV6)
> SMK_NET6ADDR = 23, /* single label IPv6 hosts */
> #endif /* CONFIG_IPV6 */
> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
> + SMK_AUTOLABEL = 24,
> +#endif
> };
>
> /*
> @@ -746,6 +749,135 @@ static void smk_cipso_doi(void)
> }
> }
>
> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
> +LIST_HEAD(smack_autolabel_list);
> +DEFINE_MUTEX(smack_autolabel_lock);
> +static int smk_add_autolabel(const char *fpath, const char *label)
> +{
> + struct smack_auto_label *smk_auto;
> + struct smack_known *smk;
> + struct path p;
> +
> + /*
> + * Check whether given file path already exists in entry
> + */
> + mutex_lock(&smack_autolabel_lock);
> + list_for_each_entry(smk_auto, &smack_autolabel_list, entry) {
> + if (!strcmp(smk_auto->fpath, fpath)) {
> + smk = smk_import_entry(label, 0);
> + if (!IS_ERR(smk))
> + smk_auto->label = smk;
> + mutex_unlock(&smack_autolabel_lock);
> + return 0;
> + }
> + }
> + mutex_unlock(&smack_autolabel_lock);
> +
> + /*
> + * Allocation for new autolabel candidate
> + */
> + smk_auto = kzalloc(sizeof(*smk_auto), GFP_KERNEL);
> + if (smk_auto == NULL)
> + return -ENOMEM;
> +
> + smk_auto->fpath = kzalloc(strlen(fpath), GFP_KERNEL);
> + if (smk_auto->fpath == NULL) {
> + kfree(smk_auto);
> + return -ENOMEM;
> + }
> +
> + strcpy(smk_auto->fpath, fpath);
> + smk_auto->label = smk_import_entry(label, 0);
> + if (IS_ERR(smk_auto->label)) {
> + kfree(smk_auto->fpath);
> + kfree(smk_auto);
> + return PTR_ERR(smk_auto->label);
> + }
> +
> + mutex_lock(&smack_autolabel_lock);
> + list_add(&smk_auto->entry, &smack_autolabel_list);
> + mutex_unlock(&smack_autolabel_lock);
> +
> + /*
> + * If path is already existed, bind autolabel to it.
> + */
> + if (!kern_path(fpath, 0, &p))
> + smk_inode_bind_autolabel(p.dentry->d_inode);
> +
> + return 0;
> +}
> +
> +static void *autolabel_seq_start(struct seq_file *s, loff_t *pos)
> +{
> + return smk_seq_start(s, pos, &smack_autolabel_list);
> +}
> +
> +static void *autolabel_seq_next(struct seq_file *s, void *v, loff_t *pos)
> +{
> + return smk_seq_next(s, v, pos, &smack_autolabel_list);
> +}
> +
> +static int autolabel_seq_show(struct seq_file *s, void *v)
> +{
> + struct list_head *list = v;
> + struct smack_auto_label *smk_auto =
> + list_entry(list, struct smack_auto_label, entry);
> +
> + seq_printf(s, "%s %s %s\n",
> + smk_auto->fpath, smk_auto->label->smk_known,
> + smk_auto->disabled ? "(disabled)" : "(enabled)");
> +
> + return 0;
> +}
> +
> +static const struct seq_operations autolabel_seq_ops = {
> + .start = autolabel_seq_start,
> + .next = autolabel_seq_next,
> + .show = autolabel_seq_show,
> + .stop = smk_seq_stop,
> +};
> +
> +static int smk_open_autolabel(struct inode *inode, struct file *file)
> +{
> + return seq_open(file, &autolabel_seq_ops);
> +}
> +
> +static ssize_t smk_write_autolabel(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + char *data;
> + char *tok[2];
> + int ret;
> +
> + data = kzalloc(count + 1, GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + if (copy_from_user(data, buf, count))
> + return -EFAULT;
> +
> + if (data[0] != '/')
> + return -EINVAL;
> +
> + tok[0] = strsep(&data, " ");
> + tok[1] = data;
> +
> + ret = smk_add_autolabel(tok[0], tok[1]);
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +
> +static const struct file_operations smk_autolabel_ops = {
> + .open = smk_open_autolabel,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .write = smk_write_autolabel,
> + .release = seq_release,
> +};
> +#endif /* CONFIG_SECURITY_SMACK_AUTOLABEL */
> +
> /**
> * smk_unlbl_ambient - initialize the unlabeled domain
> * @oldambient: previous domain string
> @@ -2824,6 +2956,10 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
> [SMK_NET6ADDR] = {
> "ipv6host", &smk_net6addr_ops, S_IRUGO|S_IWUSR},
> #endif /* CONFIG_IPV6 */
> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
> + [SMK_AUTOLABEL] = {
> + "autolabel", &smk_autolabel_ops, S_IRUGO|S_IWUSR},
> +#endif
> /* last one */
> {""}
> };
On 2015년 08월 29일 02:32, Casey Schaufler wrote:
> On 8/26/2015 6:58 PM, Jonghwa Lee wrote:
>> Current Smack object's label is always given by userspace.
>> So there might be a certain gap between the time of file creation
>> and the time of applying actual label. And because of the time gap,
>> it results unintended Smack denial time to time.
>>
>> If accessing a file occurs ahead of labeling, Smack module will check
>> the authority with uninitialized label and will prohibit the access
>> as a result. It shouldn't be happened labeling is done in proper time.
> The Smack label is assigned when the inode is created.
> The value will depend on the filesystem type, the state
> of transmute attributes, and the Smack label of the
> creating process. But it will always be assigned.
>
>> For the case that system can't gueratee that Smack labeling is done
>> before any other accesses to newly created file, this patch introdues
>> new interface called 'Automatic Smack labling'.
> The system guarantees that the Smack labeling is done
> before any other access can be attempted in all cases.
> Can you describe a situation where this does not occur?
>
Hi, Casey,
First of all, I`m thank you for review my humble patch.
I already explained the situation in cover letter but it seemed not enough.
(RFC cover : https://lkml.org/lkml/2015/8/26/782)
Let me explain it in detail. Here is the case,
A rule is defined for a process, 'process A', in smack rule table.
...
Process A device::A arwx-
...
The object 'device::A' will be used to a device node that 'process A' will access.
However when the target device node is created it's labeled with default label
which is inherited from any of filesystem, ancestor, or creating process.
Let's say the default object label for devtmpfs is '_' which allows only read and
write access. So we need the specific labeling by the authorized process as like
udevd for the devtmpfs.
In normal, smack label and access control follow the sequences,
1. Kernel module driver loaded
2. New device node is created (/dev/aaa , '_')
3. Udevd gets uevent and appies udev rule (/dev/aaa, 'device::A')
4. 'Process A' accesses the device node ('Process A' ---> 'device::A', MAY_WRITE)
5. Access is permitted.
However, when labeling isn't done in proper time, result will be different,
1. Kernel module driver loaded
2. New device node is created (/dev/aaa , '_')
3. 'Process A' accesses the device node ('Process A' ---> '_', MAY_WRITE)
4. Access is prohibited
Can this situation be handled in current Smack subsystem?
If so, could you give me an idea how to handle it.
Best regards,
Jonghwa Lee
>> The label can be defined before file creation with absolute path.
> You could in a pathname based system like AppArmor or TOMOYO,
> but Smack is an object based system. You can't count on the
> pathname to identify the object you care about. There are too
> many facilities that can manipulate the filesystem namespace.
> You have to deal with chroot and filesystem namespaces just to
> start.
>
> That, and the performance impact would be prohibitive.
>
>> Then, when target file is created in generic manner, the pre-defined
>> label is applied automatically at once.
>>
>> Pre-defined label format follows below form.
>>
>> <File path> <Smack Label>
>>
>> And it can add new entry to autolabel table via 'autolabel' in smackfs.
>>
>> echo '<File path> <Smack Label>' > /sys/fs/smackfs/autolabel
>>
>> To view entries of autolabel table,
>>
>> $cat /sys/fs/smackfs/autolabel
>> /dev/device00 Label:A <enabled>
>> /run/userfile0 Label:B <disabled>
>>
>> Signed-off-by: Jonghwa Lee <[email protected]>
> Nacked-by: Casey Schaufler <[email protected]>
>
>> ---
>> security/smack/Kconfig | 11 ++++
>> security/smack/smack.h | 23 ++++++++
>> security/smack/smack_lsm.c | 66 +++++++++++++++++++++
>> security/smack/smackfs.c | 136 ++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 236 insertions(+)
>>
>> diff --git a/security/smack/Kconfig b/security/smack/Kconfig
>> index 271adae..002aa01 100644
>> --- a/security/smack/Kconfig
>> +++ b/security/smack/Kconfig
>> @@ -30,6 +30,17 @@ config SECURITY_SMACK_BRINGUP
>> "permissive" mode of other systems.
>> If you are unsure how to answer this question, answer N.
>>
>> +config SECURITY_SMACK_AUTOLABEL
>> + bool "Enable Automatic Smack Labeling"
>> + depends on SECURITY_SMACK
>> + default n
>> + help
>> + To remove out the gap between file creation and actual labeling,
>> + this option gives the additional interface for automatic labeling.
>> + With this option enabled, a file will posseses the assigned label
>> + in autolabel table at creation.
>> + If you are unsure how to answer this question, answer N.
>> +
>> config SECURITY_SMACK_NETFILTER
>> bool "Packet marking using secmarks for netfilter"
>> depends on SECURITY_SMACK
>> diff --git a/security/smack/smack.h b/security/smack/smack.h
>> index fff0c61..8b7ee83 100644
>> --- a/security/smack/smack.h
>> +++ b/security/smack/smack.h
>> @@ -24,6 +24,7 @@
>> #include <linux/list.h>
>> #include <linux/rculist.h>
>> #include <linux/lsm_audit.h>
>> +#include <linux/namei.h>
>>
>> /*
>> * Use IPv6 port labeling if IPv6 is enabled and secmarks
>> @@ -106,6 +107,9 @@ struct inode_smack {
>> struct smack_known *smk_inode; /* label of the fso */
>> struct smack_known *smk_task; /* label of the task */
>> struct smack_known *smk_mmap; /* label of the mmap domain */
>> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
>> + struct smack_auto_label *smk_auto;
>> +#endif
>> struct mutex smk_lock; /* initialization lock */
>> int smk_flags; /* smack inode flags */
>> };
>> @@ -480,4 +484,23 @@ static inline void smk_ad_setfield_u_net_sk(struct smk_audit_info *a,
>> }
>> #endif
>>
>> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
>> +struct smack_auto_label {
>> + bool disabled;
>> + char *fpath;
>> + struct list_head entry;
>> + struct smack_known *label;
>> + struct inode *inode;
>> +};
>> +
>> +extern struct list_head smack_autolabel_list;
>> +extern struct mutex smack_autolabel_lock;
>> +
>> +extern struct smack_known *smk_inode_bind_autolabel(struct inode *);
>> +extern void smk_inode_unbind_autolabel(struct inode *);
>> +#else
>> +#define smk_inode_bind_autolabel(inode) NULL
>> +#define smk_inode_unbind_autolabel(inode) do { } while (0)
>> +#endif /* CONFIG_SECURITY_SMACK_AUTOLABEL */
>> +
>> #endif /* _SECURITY_SMACK_H */
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 996c889..851e8b4 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -249,6 +249,65 @@ static int smk_bu_credfile(const struct cred *cred, struct file *file,
>> #define smk_bu_credfile(cred, file, mode, RC) (RC)
>> #endif
>>
>> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
>> +/**
>> + * smk_inode_bind_autolabel - Check assigned label for this inode
>> + * @ip : the object of inode
>> + *
>> + * Return pre-assigned smack_known when exists, NULL if not.
>> + */
>> +struct smack_known *smk_inode_bind_autolabel(struct inode *ip)
>> +{
>> + struct smack_auto_label *smk_auto;
>> + struct inode_smack *isp;
>> + struct smack_known *smk = NULL;
>> + struct path p;
>> +
>> + if (list_empty(&smack_autolabel_list))
>> + goto out;
> Poor coding style. You can safely return NULL. There is no reason
> to goto out.
>
>> +
>> + mutex_lock(&smack_autolabel_lock);
>> + list_for_each_entry(smk_auto, &smack_autolabel_list, entry) {
>> + if (smk_auto->disabled)
>> + continue;
>> +
>> + if (!smk_auto->inode) {
>> + if (kern_path(smk_auto->fpath, 0, &p))
> Do you have any idea of the impact this would have on
> system performance?
>
>> + continue;
>> + else
>> + smk_auto->inode = p.dentry->d_inode;
>> + }
>> +
>> + if (smk_auto->inode == ip) {
>> + isp = ip->i_security;
>> + isp->smk_auto = smk_auto;
>> + smk = smk_auto->label;
>> + smk_auto->disabled = true;
>> + break;
>> + }
>> + }
>> + mutex_unlock(&smack_autolabel_lock);
>> +out:
>> + return smk;
>> +}
>> +
>> +/**
>> + * smk_inode_bind_autolabel - Re-activate autolabel for use in future
>> + * @ip : the object of inode
>> + */
>> +void smk_inode_unbind_autolabel(struct inode *ip)
>> +{
>> + struct inode_smack *isp = ip->i_security;
>> + struct smack_auto_label *smk_auto = isp->smk_auto;
>> +
>> + if (smk_auto) {
>> + smk_auto->inode = NULL;
>> + smk_auto->disabled = false;
>> + isp->smk_auto = NULL;
>> + }
>> +}
>> +#endif /* CONFIG_SECURITY_SMACK_AUTOLABEL */
>> +
>> /**
>> * smk_fetch - Fetch the smack label from a file.
>> * @name: type of the label (attribute)
>> @@ -1089,6 +1148,8 @@ static int smack_inode_unlink(struct inode *dir, struct dentry *dentry)
>> smk_ad_setfield_u_fs_inode(&ad, dir);
>> rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
>> rc = smk_bu_inode(dir, MAY_WRITE, rc);
>> + if (rc == 0)
>> + smk_inode_unbind_autolabel(ip);
>> }
>> return rc;
>> }
>> @@ -1122,6 +1183,8 @@ static int smack_inode_rmdir(struct inode *dir, struct dentry *dentry)
>> smk_ad_setfield_u_fs_inode(&ad, dir);
>> rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
>> rc = smk_bu_inode(dir, MAY_WRITE, rc);
>> + if (rc == 0)
>> + smk_inode_unbind_autolabel(d_backing_inode(dentry));
>> }
>>
>> return rc;
>> @@ -3445,6 +3508,9 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
>> if (!IS_ERR_OR_NULL(skp))
>> final = skp;
>>
>> + skp = smk_inode_bind_autolabel(inode);
>> + if (skp != NULL)
>> + final = skp;
>> /*
>> * Transmuting directory
>> */
>> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
>> index c20b154..28380ba 100644
>> --- a/security/smack/smackfs.c
>> +++ b/security/smack/smackfs.c
>> @@ -61,6 +61,9 @@ enum smk_inos {
>> #if IS_ENABLED(CONFIG_IPV6)
>> SMK_NET6ADDR = 23, /* single label IPv6 hosts */
>> #endif /* CONFIG_IPV6 */
>> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
>> + SMK_AUTOLABEL = 24,
>> +#endif
>> };
>>
>> /*
>> @@ -746,6 +749,135 @@ static void smk_cipso_doi(void)
>> }
>> }
>>
>> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
>> +LIST_HEAD(smack_autolabel_list);
>> +DEFINE_MUTEX(smack_autolabel_lock);
>> +static int smk_add_autolabel(const char *fpath, const char *label)
>> +{
>> + struct smack_auto_label *smk_auto;
>> + struct smack_known *smk;
>> + struct path p;
>> +
>> + /*
>> + * Check whether given file path already exists in entry
>> + */
>> + mutex_lock(&smack_autolabel_lock);
>> + list_for_each_entry(smk_auto, &smack_autolabel_list, entry) {
>> + if (!strcmp(smk_auto->fpath, fpath)) {
>> + smk = smk_import_entry(label, 0);
>> + if (!IS_ERR(smk))
>> + smk_auto->label = smk;
>> + mutex_unlock(&smack_autolabel_lock);
>> + return 0;
>> + }
>> + }
>> + mutex_unlock(&smack_autolabel_lock);
>> +
>> + /*
>> + * Allocation for new autolabel candidate
>> + */
>> + smk_auto = kzalloc(sizeof(*smk_auto), GFP_KERNEL);
>> + if (smk_auto == NULL)
>> + return -ENOMEM;
>> +
>> + smk_auto->fpath = kzalloc(strlen(fpath), GFP_KERNEL);
>> + if (smk_auto->fpath == NULL) {
>> + kfree(smk_auto);
>> + return -ENOMEM;
>> + }
>> +
>> + strcpy(smk_auto->fpath, fpath);
>> + smk_auto->label = smk_import_entry(label, 0);
>> + if (IS_ERR(smk_auto->label)) {
>> + kfree(smk_auto->fpath);
>> + kfree(smk_auto);
>> + return PTR_ERR(smk_auto->label);
>> + }
>> +
>> + mutex_lock(&smack_autolabel_lock);
>> + list_add(&smk_auto->entry, &smack_autolabel_list);
>> + mutex_unlock(&smack_autolabel_lock);
>> +
>> + /*
>> + * If path is already existed, bind autolabel to it.
>> + */
>> + if (!kern_path(fpath, 0, &p))
>> + smk_inode_bind_autolabel(p.dentry->d_inode);
>> +
>> + return 0;
>> +}
>> +
>> +static void *autolabel_seq_start(struct seq_file *s, loff_t *pos)
>> +{
>> + return smk_seq_start(s, pos, &smack_autolabel_list);
>> +}
>> +
>> +static void *autolabel_seq_next(struct seq_file *s, void *v, loff_t *pos)
>> +{
>> + return smk_seq_next(s, v, pos, &smack_autolabel_list);
>> +}
>> +
>> +static int autolabel_seq_show(struct seq_file *s, void *v)
>> +{
>> + struct list_head *list = v;
>> + struct smack_auto_label *smk_auto =
>> + list_entry(list, struct smack_auto_label, entry);
>> +
>> + seq_printf(s, "%s %s %s\n",
>> + smk_auto->fpath, smk_auto->label->smk_known,
>> + smk_auto->disabled ? "(disabled)" : "(enabled)");
>> +
>> + return 0;
>> +}
>> +
>> +static const struct seq_operations autolabel_seq_ops = {
>> + .start = autolabel_seq_start,
>> + .next = autolabel_seq_next,
>> + .show = autolabel_seq_show,
>> + .stop = smk_seq_stop,
>> +};
>> +
>> +static int smk_open_autolabel(struct inode *inode, struct file *file)
>> +{
>> + return seq_open(file, &autolabel_seq_ops);
>> +}
>> +
>> +static ssize_t smk_write_autolabel(struct file *file, const char __user *buf,
>> + size_t count, loff_t *ppos)
>> +{
>> + char *data;
>> + char *tok[2];
>> + int ret;
>> +
>> + data = kzalloc(count + 1, GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + if (copy_from_user(data, buf, count))
>> + return -EFAULT;
>> +
>> + if (data[0] != '/')
>> + return -EINVAL;
>> +
>> + tok[0] = strsep(&data, " ");
>> + tok[1] = data;
>> +
>> + ret = smk_add_autolabel(tok[0], tok[1]);
>> + if (ret)
>> + return ret;
>> +
>> + return count;
>> +}
>> +
>> +static const struct file_operations smk_autolabel_ops = {
>> + .open = smk_open_autolabel,
>> + .read = seq_read,
>> + .llseek = seq_lseek,
>> + .write = smk_write_autolabel,
>> + .release = seq_release,
>> +};
>> +#endif /* CONFIG_SECURITY_SMACK_AUTOLABEL */
>> +
>> /**
>> * smk_unlbl_ambient - initialize the unlabeled domain
>> * @oldambient: previous domain string
>> @@ -2824,6 +2956,10 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
>> [SMK_NET6ADDR] = {
>> "ipv6host", &smk_net6addr_ops, S_IRUGO|S_IWUSR},
>> #endif /* CONFIG_IPV6 */
>> +#ifdef CONFIG_SECURITY_SMACK_AUTOLABEL
>> + [SMK_AUTOLABEL] = {
>> + "autolabel", &smk_autolabel_ops, S_IRUGO|S_IWUSR},
>> +#endif
>> /* last one */
>> {""}
>> };
>
On pon, 2015-08-31 at 15:13 +0900, [email protected] wrote:
> A rule is defined for a process, 'process A', in smack rule table.
>
> ...
> Process A device::A arwx-
> ...
>
> The object 'device::A' will be used to a device node that 'process A'
> will access.
> However when the target device node is created it's labeled with
> default label
> which is inherited from any of filesystem, ancestor, or creating
> process.
> Let's say the default object label for devtmpfs is '_' which allows
> only read and
> write access. So we need the specific labeling by the authorized
> process as like
> udevd for the devtmpfs.
>
> In normal, smack label and access control follow the sequences,
>
> 1. Kernel module driver loaded
> 2. New device node is created (/dev/aaa , '_')
> 3. Udevd gets uevent and appies udev rule (/dev/aaa, 'device::A')
> 4. 'Process A' accesses the device node ('Process A' --->
> 'device::A', MAY_WRITE)
> 5. Access is permitted.
>
> However, when labeling isn't done in proper time, result will be
> different,
>
> 1. Kernel module driver loaded
> 2. New device node is created (/dev/aaa , '_')
> 3. 'Process A' accesses the device node ('Process A' ---> '_',
> MAY_WRITE)
> 4. Access is prohibited
>
> Can this situation be handled in current Smack subsystem?
> If so, could you give me an idea how to handle it.
This doesn't seem to be a Smack problem. This isn't even a kernel
problem. It's userspace race. You should wait for a proper udev event
that notifies after all udev rules are applied.
I think there are 2 udev events. One that notifies that a device has
been added. Second that notifies where all the rules for the device has
been applied. You need to use the second one.
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics