From 7412301b76bd53ee53b860f611fc3b5b1c2245b5 Mon Sep 17 00:00:00 2001 From: Marcin Lis Date: Thu, 22 Jan 2015 15:40:33 +0100 Subject: Smack: Assign smack_known_web as default smk_in label for kernel thread's socket This change fixes the bug associated with sockets owned by kernel threads. These sockets, created usually by network devices' drivers tasks, received smk_in label from the task that created them - the "floor" label in the most cases. The result was that they were not able to receive data packets because of missing smack rules. The main reason of the access deny is that the socket smk_in label is placed as the object during smk check, kernel thread's capabilities are omitted. Signed-off-by: Marcin Lis --- security/smack/smack_lsm.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'security/smack') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index c934311812f1..a097dc7d4669 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2452,7 +2452,21 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name, static int smack_socket_post_create(struct socket *sock, int family, int type, int protocol, int kern) { - if (family != PF_INET || sock->sk == NULL) + struct socket_smack *ssp; + + if (sock->sk == NULL) + return 0; + + /* + * Sockets created by kernel threads receive web label. + */ + if (unlikely(current->flags & PF_KTHREAD)) { + ssp = sock->sk->sk_security; + ssp->smk_in = &smack_known_web; + ssp->smk_out = &smack_known_web; + } + + if (family != PF_INET) return 0; /* * Set the outbound netlbl. -- cgit v1.2.3-59-g8ed1b From 7fc5f36e980a8f4830efdae3858f6e64eee538b7 Mon Sep 17 00:00:00 2001 From: José Bollo Date: Tue, 17 Feb 2015 15:41:22 +0100 Subject: Smack: getting the Smack security context of keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this commit, the LSM Smack implements the LSM side part of the system call keyctl with the action code KEYCTL_GET_SECURITY. It is now possible to get the context of, for example, the user session key using the command "keyctl security @s". The original patch has been modified for merge. Signed-off-by: José Bollo Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'security/smack') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index a097dc7d4669..e2d1a7b073c0 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4000,6 +4000,36 @@ static int smack_key_permission(key_ref_t key_ref, rc = smk_bu_note("key access", tkp, keyp->security, request, rc); return rc; } + +/* + * smack_key_getsecurity - Smack label tagging the key + * @key points to the key to be queried + * @_buffer points to a pointer that should be set to point to the + * resulting string (if no label or an error occurs). + * Return the length of the string (including terminating NUL) or -ve if + * an error. + * May also return 0 (and a NULL buffer pointer) if there is no label. + */ +static int smack_key_getsecurity(struct key *key, char **_buffer) +{ + struct smack_known *skp = key->security; + size_t length; + char *copy; + + if (key->security == NULL) { + *_buffer = NULL; + return 0; + } + + copy = kstrdup(skp->smk_known, GFP_KERNEL); + if (copy == NULL) + return -ENOMEM; + length = strlen(copy) + 1; + + *_buffer = copy; + return length; +} + #endif /* CONFIG_KEYS */ /* @@ -4324,6 +4354,7 @@ struct security_operations smack_ops = { .key_alloc = smack_key_alloc, .key_free = smack_key_free, .key_permission = smack_key_permission, + .key_getsecurity = smack_key_getsecurity, #endif /* CONFIG_KEYS */ /* Audit hooks */ -- cgit v1.2.3-59-g8ed1b From bf4b2fee99799780ea3dbb6d79d1909b3e32be13 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Sat, 21 Mar 2015 18:26:40 -0700 Subject: Smack: Allow an unconfined label in bringup mode I have vehemently opposed adding a "permissive" mode to Smack for the simple reasons that it would be subject to massive abuse and that developers refuse to turn it off come product release. I still believe that this is true, and still refuse to add a general "permissive mode". So don't ask again. Bumjin Im suggested an approach that addresses most of the concerns, and I have implemented it here. I still believe that we'd be better off without this sort of thing, but it looks like this minimizes the abuse potential. Firstly, you have to configure Smack Bringup Mode. That allows for "release" software to be ammune from abuse. Second, only one label gets to be "permissive" at a time. You can use it for debugging, but that's about it. A label written to smackfs/unconfined is treated specially. If either the subject or object label of an access check matches the "unconfined" label, and the access would not have been allowed otherwise an audit record and a console message are generated. The audit record "request" string is marked with either "(US)" or "(UO)", to indicate that the request was granted because of an unconfined label. The fact that an inode was accessed by an unconfined label is remembered, and subsequent accesses to that "impure" object are noted in the log. The impurity is not stored in the filesystem, so a file mislabled as a side effect of using an unconfined label may still cause concern after a reboot. So, it's there, it's dangerous, but so many application developers seem incapable of living without it I have given in. I've tried to make it as safe as I can, but in the end it's still a chain saw. Signed-off-by: Casey Schaufler --- security/smack/smack.h | 8 ++++ security/smack/smack_access.c | 43 ++++++++++++++----- security/smack/smack_lsm.c | 52 +++++++++++++++++++---- security/smack/smackfs.c | 96 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 17 deletions(-) (limited to 'security/smack') diff --git a/security/smack/smack.h b/security/smack/smack.h index 67ccb7b2b89b..49eada6266ec 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -105,6 +105,7 @@ struct task_smack { #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */ #define SMK_INODE_TRANSMUTE 0x02 /* directory is transmuting */ #define SMK_INODE_CHANGED 0x04 /* smack was transmuted */ +#define SMK_INODE_IMPURE 0x08 /* involved in an impure transaction */ /* * A label access rule. @@ -193,6 +194,10 @@ struct smk_port_label { #define MAY_LOCK 0x00002000 /* Locks should be writes, but ... */ #define MAY_BRINGUP 0x00004000 /* Report use of this rule */ +#define SMACK_BRINGUP_ALLOW 1 /* Allow bringup mode */ +#define SMACK_UNCONFINED_SUBJECT 2 /* Allow unconfined label */ +#define SMACK_UNCONFINED_OBJECT 3 /* Allow unconfined label */ + /* * Just to make the common cases easier to deal with */ @@ -254,6 +259,9 @@ extern int smack_cipso_mapped; extern struct smack_known *smack_net_ambient; extern struct smack_known *smack_onlycap; extern struct smack_known *smack_syslog_label; +#ifdef CONFIG_SECURITY_SMACK_BRINGUP +extern struct smack_known *smack_unconfined; +#endif extern struct smack_known smack_cipso_option; extern int smack_ptrace_rule; diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 1158430f5bb9..0f410fc56e33 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -130,7 +130,8 @@ int smk_access(struct smack_known *subject, struct smack_known *object, /* * Hardcoded comparisons. - * + */ + /* * A star subject can't access any object. */ if (subject == &smack_known_star) { @@ -189,10 +190,20 @@ int smk_access(struct smack_known *subject, struct smack_known *object, * succeed because of "b" rules. */ if (may & MAY_BRINGUP) - rc = MAY_BRINGUP; + rc = SMACK_BRINGUP_ALLOW; #endif out_audit: + +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + if (rc < 0) { + if (object == smack_unconfined) + rc = SMACK_UNCONFINED_OBJECT; + if (subject == smack_unconfined) + rc = SMACK_UNCONFINED_SUBJECT; + } +#endif + #ifdef CONFIG_AUDIT if (a) smack_log(subject->smk_known, object->smk_known, @@ -338,19 +349,16 @@ static void smack_log_callback(struct audit_buffer *ab, void *a) void smack_log(char *subject_label, char *object_label, int request, int result, struct smk_audit_info *ad) { +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + char request_buffer[SMK_NUM_ACCESS_TYPE + 5]; +#else char request_buffer[SMK_NUM_ACCESS_TYPE + 1]; +#endif struct smack_audit_data *sad; struct common_audit_data *a = &ad->a; -#ifdef CONFIG_SECURITY_SMACK_BRINGUP - /* - * The result may be positive in bringup mode. - */ - if (result > 0) - result = 0; -#endif /* check if we have to log the current event */ - if (result != 0 && (log_policy & SMACK_AUDIT_DENIED) == 0) + if (result < 0 && (log_policy & SMACK_AUDIT_DENIED) == 0) return; if (result == 0 && (log_policy & SMACK_AUDIT_ACCEPT) == 0) return; @@ -364,6 +372,21 @@ void smack_log(char *subject_label, char *object_label, int request, smack_str_from_perm(request_buffer, request); sad->subject = subject_label; sad->object = object_label; +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + /* + * The result may be positive in bringup mode. + * A positive result is an allow, but not for normal reasons. + * Mark it as successful, but don't filter it out even if + * the logging policy says to do so. + */ + if (result == SMACK_UNCONFINED_SUBJECT) + strcat(request_buffer, "(US)"); + else if (result == SMACK_UNCONFINED_OBJECT) + strcat(request_buffer, "(UO)"); + + if (result > 0) + result = 0; +#endif sad->request = request_buffer; sad->result = result; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index e2d1a7b073c0..6f3c7d866d04 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -57,6 +57,13 @@ static struct kmem_cache *smack_inode_cache; int smack_enabled; #ifdef CONFIG_SECURITY_SMACK_BRINGUP +static char *smk_bu_mess[] = { + "Bringup Error", /* Unused */ + "Bringup", /* SMACK_BRINGUP_ALLOW */ + "Unconfined Subject", /* SMACK_UNCONFINED_SUBJECT */ + "Unconfined Object", /* SMACK_UNCONFINED_OBJECT */ +}; + static void smk_bu_mode(int mode, char *s) { int i = 0; @@ -87,9 +94,11 @@ static int smk_bu_note(char *note, struct smack_known *sskp, if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) %s\n", + pr_info("Smack %s: (%s %s %s) %s\n", smk_bu_mess[rc], sskp->smk_known, oskp->smk_known, acc, note); return 0; } @@ -106,9 +115,11 @@ static int smk_bu_current(char *note, struct smack_known *oskp, if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) %s %s\n", + pr_info("Smack %s: (%s %s %s) %s %s\n", smk_bu_mess[rc], tsp->smk_task->smk_known, oskp->smk_known, acc, current->comm, note); return 0; @@ -126,9 +137,11 @@ static int smk_bu_task(struct task_struct *otp, int mode, int rc) if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) %s to %s\n", + pr_info("Smack %s: (%s %s %s) %s to %s\n", smk_bu_mess[rc], tsp->smk_task->smk_known, smk_task->smk_known, acc, current->comm, otp->comm); return 0; @@ -141,14 +154,25 @@ static int smk_bu_task(struct task_struct *otp, int mode, int rc) static int smk_bu_inode(struct inode *inode, int mode, int rc) { struct task_smack *tsp = current_security(); + struct inode_smack *isp = inode->i_security; char acc[SMK_NUM_ACCESS_TYPE + 1]; + if (isp->smk_flags & SMK_INODE_IMPURE) + pr_info("Smack Unconfined Corruption: inode=(%s %ld) %s\n", + inode->i_sb->s_id, inode->i_ino, current->comm); + if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; + if (rc == SMACK_UNCONFINED_SUBJECT && + (mode & (MAY_WRITE | MAY_APPEND))) + isp->smk_flags |= SMK_INODE_IMPURE; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) inode=(%s %ld) %s\n", - tsp->smk_task->smk_known, smk_of_inode(inode)->smk_known, acc, + + pr_info("Smack %s: (%s %s %s) inode=(%s %ld) %s\n", smk_bu_mess[rc], + tsp->smk_task->smk_known, isp->smk_inode->smk_known, acc, inode->i_sb->s_id, inode->i_ino, current->comm); return 0; } @@ -162,13 +186,20 @@ static int smk_bu_file(struct file *file, int mode, int rc) struct task_smack *tsp = current_security(); struct smack_known *sskp = tsp->smk_task; struct inode *inode = file_inode(file); + struct inode_smack *isp = inode->i_security; char acc[SMK_NUM_ACCESS_TYPE + 1]; + if (isp->smk_flags & SMK_INODE_IMPURE) + pr_info("Smack Unconfined Corruption: inode=(%s %ld) %s\n", + inode->i_sb->s_id, inode->i_ino, current->comm); + if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) file=(%s %ld %pD) %s\n", + pr_info("Smack %s: (%s %s %s) file=(%s %ld %pD) %s\n", smk_bu_mess[rc], sskp->smk_known, smk_of_inode(inode)->smk_known, acc, inode->i_sb->s_id, inode->i_ino, file, current->comm); @@ -185,13 +216,20 @@ static int smk_bu_credfile(const struct cred *cred, struct file *file, struct task_smack *tsp = cred->security; struct smack_known *sskp = tsp->smk_task; struct inode *inode = file->f_inode; + struct inode_smack *isp = inode->i_security; char acc[SMK_NUM_ACCESS_TYPE + 1]; + if (isp->smk_flags & SMK_INODE_IMPURE) + pr_info("Smack Unconfined Corruption: inode=(%s %ld) %s\n", + inode->i_sb->s_id, inode->i_ino, current->comm); + if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) file=(%s %ld %pD) %s\n", + pr_info("Smack %s: (%s %s %s) file=(%s %ld %pD) %s\n", smk_bu_mess[rc], sskp->smk_known, smk_of_inode(inode)->smk_known, acc, inode->i_sb->s_id, inode->i_ino, file, current->comm); diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index bce4e8f1b267..deb3d3bfbbf3 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -54,6 +54,9 @@ enum smk_inos { SMK_CHANGE_RULE = 19, /* change or add rules (long labels) */ SMK_SYSLOG = 20, /* change syslog label) */ SMK_PTRACE = 21, /* set ptrace rule */ +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + SMK_UNCONFINED = 22, /* define an unconfined label */ +#endif }; /* @@ -95,6 +98,16 @@ int smack_cipso_mapped = SMACK_CIPSO_MAPPED_DEFAULT; */ struct smack_known *smack_onlycap; +#ifdef CONFIG_SECURITY_SMACK_BRINGUP +/* + * Allow one label to be unconfined. This is for + * debugging and application bring-up purposes only. + * It is bad and wrong, but everyone seems to expect + * to have it. + */ +struct smack_known *smack_unconfined; +#endif + /* * If this value is set restrict syslog use to the label specified. * It can be reset via smackfs/syslog @@ -1717,6 +1730,85 @@ static const struct file_operations smk_onlycap_ops = { .llseek = default_llseek, }; +#ifdef CONFIG_SECURITY_SMACK_BRINGUP +/** + * smk_read_unconfined - read() for smackfs/unconfined + * @filp: file pointer, not actually used + * @buf: where to put the result + * @cn: maximum to send along + * @ppos: where to start + * + * Returns number of bytes read or error code, as appropriate + */ +static ssize_t smk_read_unconfined(struct file *filp, char __user *buf, + size_t cn, loff_t *ppos) +{ + char *smack = ""; + ssize_t rc = -EINVAL; + int asize; + + if (*ppos != 0) + return 0; + + if (smack_unconfined != NULL) + smack = smack_unconfined->smk_known; + + asize = strlen(smack) + 1; + + if (cn >= asize) + rc = simple_read_from_buffer(buf, cn, ppos, smack, asize); + + return rc; +} + +/** + * smk_write_unconfined - write() for smackfs/unconfined + * @file: file pointer, not actually used + * @buf: where to get the data from + * @count: bytes sent + * @ppos: where to start + * + * Returns number of bytes written or error code, as appropriate + */ +static ssize_t smk_write_unconfined(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + char *data; + int rc = count; + + if (!smack_privileged(CAP_MAC_ADMIN)) + return -EPERM; + + data = kzalloc(count + 1, GFP_KERNEL); + if (data == NULL) + return -ENOMEM; + + /* + * Should the null string be passed in unset the unconfined value. + * This seems like something to be careful with as usually + * smk_import only expects to return NULL for errors. It + * is usually the case that a nullstring or "\n" would be + * bad to pass to smk_import but in fact this is useful here. + * + * smk_import will also reject a label beginning with '-', + * so "-confine" will also work. + */ + if (copy_from_user(data, buf, count) != 0) + rc = -EFAULT; + else + smack_unconfined = smk_import_entry(data, count); + + kfree(data); + return rc; +} + +static const struct file_operations smk_unconfined_ops = { + .read = smk_read_unconfined, + .write = smk_write_unconfined, + .llseek = default_llseek, +}; +#endif /* CONFIG_SECURITY_SMACK_BRINGUP */ + /** * smk_read_logging - read() for /smack/logging * @filp: file pointer, not actually used @@ -2384,6 +2476,10 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent) "syslog", &smk_syslog_ops, S_IRUGO|S_IWUSR}, [SMK_PTRACE] = { "ptrace", &smk_ptrace_ops, S_IRUGO|S_IWUSR}, +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + [SMK_UNCONFINED] = { + "unconfined", &smk_unconfined_ops, S_IRUGO|S_IWUSR}, +#endif /* last one */ {""} }; -- cgit v1.2.3-59-g8ed1b From f43b65bad6d54df7562c522a13d30efddae91234 Mon Sep 17 00:00:00 2001 From: Paul Gortmaker Date: Mon, 23 Mar 2015 14:03:17 -0400 Subject: smack: Fix gcc warning from unused smack_syslog_lock mutex in smackfs.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In commit 00f84f3f2e9d088f06722f4351d67f5f577abe22 ("Smack: Make the syslog control configurable") this mutex was added, but the rest of the final commit never actually made use of it, resulting in: In file included from include/linux/mutex.h:29:0, from include/linux/notifier.h:13, from include/linux/memory_hotplug.h:6, from include/linux/mmzone.h:821, from include/linux/gfp.h:5, from include/linux/slab.h:14, from include/linux/security.h:27, from security/smack/smackfs.c:21: security/smack/smackfs.c:63:21: warning: ‘smack_syslog_lock’ defined but not used [-Wunused-variable] static DEFINE_MUTEX(smack_syslog_lock); ^ A git grep shows no other instances/references to smack_syslog_lock. Delete it, assuming that the mutex addition was just a leftover from an earlier work in progress version of the change. Signed-off-by: Paul Gortmaker --- security/smack/smackfs.c | 1 - 1 file changed, 1 deletion(-) (limited to 'security/smack') diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index deb3d3bfbbf3..06f719ed63c9 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -64,7 +64,6 @@ enum smk_inos { */ static DEFINE_MUTEX(smack_cipso_lock); static DEFINE_MUTEX(smack_ambient_lock); -static DEFINE_MUTEX(smack_syslog_lock); static DEFINE_MUTEX(smk_netlbladdr_lock); /* -- cgit v1.2.3-59-g8ed1b