[PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring
Require all keys added to the EVM keyring be signed by an existing trusted key on the system trusted keyring. This patch also switches IMA to use integrity_init_keyring(). Changes in v3: * Added 'init_keyring' config based variable to skip initializing keyring instead of using __integrity_init_keyring() wrapper. * Added dependency back to CONFIG_IMA_TRUSTED_KEYRING Changes in v2: * Replace CONFIG_EVM_TRUSTED_KEYRING with IMA and EVM common CONFIG_INTEGRITY_TRUSTED_KEYRING configuration option * Deprecate CONFIG_IMA_TRUSTED_KEYRING but keep it for config file compatibility. (Mimi Zohar) Signed-off-by: Dmitry Kasatkin--- security/integrity/Kconfig| 11 +++ security/integrity/digsig.c | 14 -- security/integrity/evm/evm_main.c | 8 +--- security/integrity/ima/Kconfig| 5 - security/integrity/ima/ima.h | 12 security/integrity/ima/ima_init.c | 2 +- security/integrity/integrity.h| 5 ++--- 7 files changed, 35 insertions(+), 22 deletions(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 73c457b..21d7568 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -41,6 +41,17 @@ config INTEGRITY_ASYMMETRIC_KEYS This option enables digital signature verification using asymmetric keys. +config INTEGRITY_TRUSTED_KEYRING + bool "Require all keys on the integrity keyrings be signed" + depends on SYSTEM_TRUSTED_KEYRING + depends on INTEGRITY_ASYMMETRIC_KEYS + select KEYS_DEBUG_PROC_KEYS + default y + help + This option requires that all keys added to the .ima and + .evm keyrings be signed by a key on the system trusted + keyring. + config INTEGRITY_AUDIT bool "Enables integrity auditing support " depends on AUDIT diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 5be9ffb..8ef1511 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -24,15 +24,22 @@ static struct key *keyring[INTEGRITY_KEYRING_MAX]; static const char *keyring_name[INTEGRITY_KEYRING_MAX] = { +#ifndef CONFIG_INTEGRITY_TRUSTED_KEYRING "_evm", - "_module", -#ifndef CONFIG_IMA_TRUSTED_KEYRING "_ima", #else + ".evm", ".ima", #endif + "_module", }; +#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING +static bool init_keyring __initdata = true; +#else +static bool init_keyring __initdata; +#endif + int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, const char *digest, int digestlen) { @@ -68,6 +75,9 @@ int __init integrity_init_keyring(const unsigned int id) const struct cred *cred = current_cred(); int err = 0; + if (!init_keyring) + return 0; + keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0), KGIDT_INIT(0), cred, ((KEY_POS_ALL & ~KEY_POS_SETATTR) | diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 1334e02..75b7e30 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -478,15 +478,17 @@ static int __init init_evm(void) evm_init_config(); + error = integrity_init_keyring(INTEGRITY_KEYRING_EVM); + if (error) + return error; + error = evm_init_secfs(); if (error < 0) { pr_info("Error registering secfs\n"); - goto err; + return error; } return 0; -err: - return error; } /* diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index df30334..a292b88 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -123,14 +123,17 @@ config IMA_APPRAISE If unsure, say N. config IMA_TRUSTED_KEYRING - bool "Require all keys on the .ima keyring be signed" + bool "Require all keys on the .ima keyring be signed (deprecated)" depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING depends on INTEGRITY_ASYMMETRIC_KEYS + select INTEGRITY_TRUSTED_KEYRING default y help This option requires that all keys added to the .ima keyring be signed by a key on the system trusted keyring. + This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING + config IMA_LOAD_X509 bool "Load X509 certificate onto the '.ima' trusted keyring" depends on IMA_TRUSTED_KEYRING diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index e2a60c3..9e82367 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -251,16 +251,4 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, return -EINVAL; } #endif /* CONFIG_IMA_LSM_RULES */ - -#ifdef
[PATCHv3 3/6] evm: enable EVM when X509 certificate is loaded
In order to enable EVM before starting 'init' process, evm_initialized needs to be non-zero. Before it was indicating that HMAC key is loaded. When EVM loads X509 before calling 'init', it is possible to enable EVM to start signature based verification. This patch defines bits to enable EVM if key of any type is loaded. Changes in v2: * EVM_STATE_KEY_SET replaced by EVM_INIT_HMAC * EVM_STATE_X509_SET replaced by EVM_INIT_X509 Signed-off-by: Dmitry Kasatkin--- security/integrity/evm/evm.h| 3 +++ security/integrity/evm/evm_crypto.c | 2 ++ security/integrity/evm/evm_main.c | 6 +- security/integrity/evm/evm_secfs.c | 4 ++-- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index 88bfe77..f5f1272 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -21,6 +21,9 @@ #include "../integrity.h" +#define EVM_INIT_HMAC 0x0001 +#define EVM_INIT_X509 0x0002 + extern int evm_initialized; extern char *evm_hmac; extern char *evm_hash; diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 159ef3e..34e1a6f 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -40,6 +40,8 @@ static struct shash_desc *init_desc(char type) struct shash_desc *desc; if (type == EVM_XATTR_HMAC) { + if (!(evm_initialized & EVM_INIT_HMAC)) + return ERR_PTR(-ENOKEY); tfm = _tfm; algo = evm_hmac; } else { diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 519de0a..420d94d 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -475,7 +475,11 @@ EXPORT_SYMBOL_GPL(evm_inode_init_security); #ifdef CONFIG_EVM_LOAD_X509 void __init evm_load_x509(void) { - integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH); + int rc; + + rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH); + if (!rc) + evm_initialized |= EVM_INIT_X509; } #endif diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index cf12a04..3f775df 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -64,7 +64,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, char temp[80]; int i, error; - if (!capable(CAP_SYS_ADMIN) || evm_initialized) + if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_INIT_HMAC)) return -EPERM; if (count >= sizeof(temp) || count == 0) @@ -80,7 +80,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, error = evm_init_key(); if (!error) { - evm_initialized = 1; + evm_initialized |= EVM_INIT_HMAC; pr_info("initialized\n"); } else pr_err("initialization failed\n"); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 4/6] evm: provide a function to set EVM key from the kernel
Crypto HW kernel module can possibly initialize EVM key from the kernel __init code to enable EVM before calling 'init' process. This patch provide a function evm_set_key() which can be used to set custom key directly to EVM without using KEY subsystem. Changes in v3: * error reporting moved to evm_set_key * EVM_INIT_HMAC moved to evm_set_key * added bitop to prevent key setting race Changes in v2: * use size_t for key size instead of signed int * provide EVM_MAX_KEY_SIZE macro in * provide EVM_MIN_KEY_SIZE macro in Signed-off-by: Dmitry Kasatkin--- include/linux/evm.h | 7 ++ security/integrity/evm/evm_crypto.c | 47 +++-- security/integrity/evm/evm_secfs.c | 10 +++- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/include/linux/evm.h b/include/linux/evm.h index 1fcb88c..35ed9a8 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -14,6 +14,7 @@ struct integrity_iint_cache; #ifdef CONFIG_EVM +extern int evm_set_key(void *key, size_t keylen); extern enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name, void *xattr_value, @@ -42,6 +43,12 @@ static inline int posix_xattr_acl(const char *xattrname) } #endif #else + +static inline int evm_set_key(void *key, size_t keylen) +{ + return -EOPNOTSUPP; +} + #ifdef CONFIG_INTEGRITY static inline enum integrity_status evm_verifyxattr(struct dentry *dentry, const char *xattr_name, diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 34e1a6f..7aec93e 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include "evm.h" @@ -32,6 +33,41 @@ struct crypto_shash *hash_tfm; static DEFINE_MUTEX(mutex); +#define EVM_SET_KEY_BUSY 0 + +static unsigned long evm_set_key_flags; + +/* evm_set_key - set EVM HMAC key from the kernel + * + * This function allows to set EVM HMAC key from the kernel + * without using key subsystem 'encrypted' keys. It can be used + * by the crypto HW kernel module which has own way of managing + * keys. + * + * key length should be between 32 and 128 bytes long + */ +int evm_set_key(void *key, size_t keylen) +{ + int rc; + + rc = -EBUSY; + if (test_and_set_bit(EVM_SET_KEY_BUSY, _set_key_flags)) + goto busy; + rc = -EINVAL; + if (keylen > MAX_KEY_SIZE) + goto inval; + memcpy(evmkey, key, keylen); + evm_initialized |= EVM_INIT_HMAC; + pr_info("key initialized\n"); + return 0; +inval: + clear_bit(EVM_SET_KEY_BUSY, _set_key_flags); +busy: + pr_err("key initialization failed\n"); + return rc; +} +EXPORT_SYMBOL_GPL(evm_set_key); + static struct shash_desc *init_desc(char type) { long rc; @@ -242,7 +278,7 @@ int evm_init_key(void) { struct key *evm_key; struct encrypted_key_payload *ekp; - int rc = 0; + int rc; evm_key = request_key(_type_encrypted, EVMKEY, NULL); if (IS_ERR(evm_key)) @@ -250,12 +286,9 @@ int evm_init_key(void) down_read(_key->sem); ekp = evm_key->payload.data; - if (ekp->decrypted_datalen > MAX_KEY_SIZE) { - rc = -EINVAL; - goto out; - } - memcpy(evmkey, ekp->decrypted_data, ekp->decrypted_datalen); -out: + + rc = evm_set_key(ekp->decrypted_data, ekp->decrypted_datalen); + /* burn the original key contents */ memset(ekp->decrypted_data, 0, ekp->decrypted_datalen); up_read(_key->sem); diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index 3f775df..c8dccd5 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -62,7 +62,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { char temp[80]; - int i, error; + int i; if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_INIT_HMAC)) return -EPERM; @@ -78,12 +78,8 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, if ((sscanf(temp, "%d", ) != 1) || (i != 1)) return -EINVAL; - error = evm_init_key(); - if (!error) { - evm_initialized |= EVM_INIT_HMAC; - pr_info("initialized\n"); - } else - pr_err("initialization failed\n"); + evm_init_key(); + return count; } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at
[PATCHv3 5/6] evm: define EVM key max and min sizes
This patch imposes minimum key size limit. It declares EVM_MIN_KEY_SIZE and EVM_MAX_KEY_SIZE in public header file. Signed-off-by: Dmitry Kasatkin--- include/linux/evm.h | 3 +++ security/integrity/evm/evm_crypto.c | 7 +++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/linux/evm.h b/include/linux/evm.h index 35ed9a8..0aeedec 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -11,6 +11,9 @@ #include #include +#define EVM_MAX_KEY_SIZE 128 +#define EVM_MIN_KEY_SIZE 64 + struct integrity_iint_cache; #ifdef CONFIG_EVM diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 7aec93e..cfb788c 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -24,9 +24,8 @@ #include "evm.h" #define EVMKEY "evm-key" -#define MAX_KEY_SIZE 128 -static unsigned char evmkey[MAX_KEY_SIZE]; -static int evmkey_len = MAX_KEY_SIZE; +static unsigned char evmkey[EVM_MAX_KEY_SIZE]; +static int evmkey_len = EVM_MAX_KEY_SIZE; struct crypto_shash *hmac_tfm; struct crypto_shash *hash_tfm; @@ -54,7 +53,7 @@ int evm_set_key(void *key, size_t keylen) if (test_and_set_bit(EVM_SET_KEY_BUSY, _set_key_flags)) goto busy; rc = -EINVAL; - if (keylen > MAX_KEY_SIZE) + if (keylen < EVM_MIN_KEY_SIZE || keylen > EVM_MAX_KEY_SIZE) goto inval; memcpy(evmkey, key, keylen); evm_initialized |= EVM_INIT_HMAC; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] Enable multiple writes to the IMA policy;
Hi Petko, I have a question On Fri, Oct 16, 2015 at 10:31 PM, Petko Manolovwrote: > IMA policy can now be updated multiple times. The new rules get appended > to the original policy. Have in mind that the rules are scanned in FIFO > order so be careful when you add new ones. > > The mutex locks are replaced with RCU, which should lead to faster policy > traversals. The new rules are first appended to a temporary list, which > on error gets released without disturbing the normal IMA operations. > > Signed-off-by: Petko Manolov > --- > security/integrity/ima/Kconfig | 14 > security/integrity/ima/ima_fs.c | 13 +++ > security/integrity/ima/ima_policy.c | 70 > + > 3 files changed, 74 insertions(+), 23 deletions(-) > > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig > index df30334..15264b7 100644 > --- a/security/integrity/ima/Kconfig > +++ b/security/integrity/ima/Kconfig > @@ -107,6 +107,20 @@ config IMA_DEFAULT_HASH > default "sha512" if IMA_DEFAULT_HASH_SHA512 > default "wp512" if IMA_DEFAULT_HASH_WP512 > > +config IMA_WRITE_POLICY > + bool "Enable multiple writes to the IMA policy" > + depends on IMA > + default n > + help > + IMA policy can now be updated multiple times. The new rules get > + appended to the original policy. Have in mind that the rules are > + scanned in FIFO order so be careful when you add new ones. > + > + WARNING: Potential security hole - should be used with care in > + production-grade kernels! > + > + If unsure, say N. > + > config IMA_APPRAISE > bool "Appraise integrity measurements" > depends on IMA > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index 816d175..a3cf5c0 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -25,6 +25,8 @@ > > #include "ima.h" > > +static DEFINE_MUTEX(ima_write_mutex); > + > static int valid_policy = 1; > #define TMPBUFLEN 12 > static ssize_t ima_show_htable_value(char __user *buf, size_t count, > @@ -261,6 +263,11 @@ static ssize_t ima_write_policy(struct file *file, const > char __user *buf, > { > char *data = NULL; > ssize_t result; > + int res; > + > + res = mutex_lock_interruptible(_write_mutex); > + if (res) > + return res; > You got rid of "ima_rules_mutex" in favor of RCU. I wonder why 'ima_write_mutex" is needed here? I do not see any other uses. ima_open_policy() provide "exclusive" access if (test_and_set_bit(IMA_FS_BUSY, _fs_flags)) return -EBUSY; > if (datalen >= PAGE_SIZE) > datalen = PAGE_SIZE - 1; > @@ -286,6 +293,8 @@ out: > if (result < 0) > valid_policy = 0; > kfree(data); > + mutex_unlock(_write_mutex); > + > return result; > } > > @@ -337,8 +346,12 @@ static int ima_release_policy(struct inode *inode, > struct file *file) > return 0; > } > ima_update_policy(); > +#ifndefCONFIG_IMA_WRITE_POLICY > securityfs_remove(ima_policy); > ima_policy = NULL; > +#else > + clear_bit(IMA_FS_BUSY, _fs_flags); > +#endif > return 0; > } > May be could actually just clear a flag and leave sysfs entry anyway #ifdefCONFIG_IMA_WRITE_POLICY clear_bit(IMA_FS_BUSY, _fs_flags); #endif otherwise flag will be busy and next open fails as well... Dmitry > diff --git a/security/integrity/ima/ima_policy.c > b/security/integrity/ima/ima_policy.c > index 3997e20..7ace4e4 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -16,6 +16,7 @@ > #include > #include > #include > +#include > #include > > #include "ima.h" > @@ -135,11 +136,11 @@ static struct ima_rule_entry default_appraise_rules[] = > { > > static LIST_HEAD(ima_default_rules); > static LIST_HEAD(ima_policy_rules); > +static LIST_HEAD(ima_temp_rules); > static struct list_head *ima_rules; > > -static DEFINE_MUTEX(ima_rules_mutex); > - > static int ima_policy __initdata; > + > static int __init default_measure_policy_setup(char *str) > { > if (ima_policy) > @@ -171,9 +172,8 @@ static int __init default_appraise_policy_setup(char *str) > __setup("ima_appraise_tcb", default_appraise_policy_setup); > > /* > - * Although the IMA policy does not change, the LSM policy can be > - * reloaded, leaving the IMA LSM based rules referring to the old, > - * stale LSM policy. > + * Blocking here is not legal as we hold an RCU lock. ima_update_policy() > + * should make it safe to walk the list at any time. > * > * Update the IMA LSM based rules to reflect the reloaded LSM policy. > * We assume the rules still exist; and BUG_ON() if they don't. > @@ -184,7 +184,6 @@ static void
[PATCH v2 1/1] Tags: Adding tagging feature to security modules
The Tags security module allows to attach tags to processes. Tags are accessed through the new files /proc/PID/attr/tags and /proc/PID/tasks/TID/attr/tags below named "tag file". Reading a tag file returns all the tags attached to the process (or thread). The tags are listed one per line, each followed by exactly one new-line character ('\n'). Writing a tag file allows under condition to change the tags of a process. When writting the file, it acts like a protocol accepting lines. The accepted lines are: - empty: "\n" only a new-line - comment: "#...anything...\n" a sharp followed by anything and a new-line - remove all tags: "-\n" a minus followed by a new-line - remove one tag: "-TAGNAME\n" a minus followed by the name of the tag and a new-line - add one tag: "+TAGNAME\n" a plus followed by the name of the tag and a new-line - query one tag: "?TAGNAME\n" a question mark followed by the name of the tag and a new-line Be aware that the current implementation doesn't trim any extra characters like spaces or tabs. Names of tags can have any ASCII printable character (in the ranges [33...126] that is from '!' to '~'. The minimum length of a tag's name is 1 and the maximum length is 1000 bytes. It exist 7 special tags. Their names are: - tags:add - tags:sub - tags:keep - tags:add-all - tags:sub-all - tags:keep-all - tags:set-others All other tags prefixed by "tags:" are reserved and can not be used. Normally a process can not remove tags from itself. A process can remove a tag from itself only if one or more of the following conditions is true: - the process has not the tag (it is not an error) - the process has the tag "tags:sub" and the tag to remove is not a special tag - the process has the tag "tags:sub-all" - the process is a kernel's thread - the process has the capability CAP_MAC_ADMIN Normally a process can not add tags to itself. A process can add a tag to itself only if one or more of the following conditions is true: - the process already has the tag (it is not an error) - the process has the tag "tags:add" and the tag to add is not a special tag - the process has the tag "tags:add-all" - the process is a kernel's thread - the process has the capability CAP_MAC_ADMIN Even if a process has the right to add tags, it can be canceled (ECANCELED) if the count of tag reached is too high. The current limit is 1000. This limit is global to the kernel, not by process. Normally a process can not add or remove tags of other processes. But it can do that if it has the tag "tags:set-others". Tags are copied to the children processes during 'clone'. During execution of 'execve', the following rules apply: - processes having the tag "tags:keep-all" keep all there tags; - otherwise, processes having "tags:keep" keep the tags that are not specials; - otherwise, processes only keep tags that are prefixed with the character * (star). Because changes only occur through tag files accesses, the notifications might be available to any possible observer. Signed-off-by: José Bollo--- fs/proc/base.c | 3 + security/Kconfig | 2 + security/Makefile | 2 + security/smack/Kconfig | 9 + security/smack/smack.h | 7 + security/smack/smack_lsm.c | 50 security/tags/Kconfig | 8 + security/tags/Makefile | 6 + security/tags/tags.c | 731 + security/tags/tags.h | 39 +++ 10 files changed, 857 insertions(+) create mode 100644 security/tags/Kconfig create mode 100644 security/tags/Makefile create mode 100644 security/tags/tags.c create mode 100644 security/tags/tags.h diff --git a/fs/proc/base.c b/fs/proc/base.c index b25eee4..b1a47daa 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2406,6 +2406,9 @@ static const struct pid_entry attr_dir_stuff[] = { REG("fscreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), REG("keycreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), REG("sockcreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), +#ifdef CONFIG_SECURITY_TAGS + REG("tags", S_IRUGO|S_IWUGO, proc_pid_attr_operations), +#endif }; static int proc_attr_dir_readdir(struct file *file, struct dir_context *ctx) diff --git a/security/Kconfig b/security/Kconfig index e452378..2e628d2 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -118,6 +118,8 @@ config LSM_MMAP_MIN_ADDR this low address space will need the permission specific to the systems running LSM. +source security/tags/Kconfig + source security/selinux/Kconfig source security/smack/Kconfig source security/tomoyo/Kconfig diff --git a/security/Makefile b/security/Makefile index c9bfbc8..9138482 100644 --- a/security/Makefile +++ b/security/Makefile @@ -8,6 +8,7 @@ subdir-$(CONFIG_SECURITY_SMACK) += smack subdir-$(CONFIG_SECURITY_TOMOYO)+= tomoyo
Re: [PATCH] userns/capability: Add user namespace capability
On Thu, Oct 22, 2015 at 1:45 PM, Eric W. Biedermanwrote: > > Thank you for a creative solution to a problem that you perceive. I > appreciate it when people aim to solve problems they see. > > Tobias Markus writes: > >> On 17.10.2015 23:55, Serge E. Hallyn wrote: >>> On Sat, Oct 17, 2015 at 05:58:04PM +0200, Tobias Markus wrote: Add capability CAP_SYS_USER_NS. Tasks having CAP_SYS_USER_NS are allowed to create a new user namespace when calling clone or unshare with CLONE_NEWUSER. Rationale: Linux 3.8 saw the introduction of unpriviledged user namespaces, allowing unpriviledged users (without CAP_SYS_ADMIN) to be a "fake" root inside a separate user namespace. Before that, any namespace creation required CAP_SYS_ADMIN (or, in practice, the user had to be root). Unfortunately, there have been some security-relevant bugs in the meantime. Because of the fairly complex nature of user namespaces, it is reasonable to say that future vulnerabilties can not be excluded. Some distributions even wholly disable user namespaces because of this. >>> >>> Fwiw I'm not in favor of this. Debian has a patch (I believe the one >>> I originally wrote for Ubuntu but which Ubuntu dropped long ago) adding a >>> sysctl, off by default, for enabling user namespaces. >> >> While it certainly works, enabling a feature like this at runtime >> doesn't seem like a long term solution. >> >> The fact that Debian added this patch in the first place already >> demonstrates that there is demand for a way to limit unpriviledged user >> namespace creation. Please, don't get me wrong: I would *really like* to >> see widespread adoption and continued development of user namespaces! >> But the status quo remains: Distributions outright disabling user >> namespaces (e.g. Arch Linux) won't make it easier. > > Let me say I applaud Arch Linux for not doing what so many distributions > do and enable every feature in the kernel. I appreciate a distribution > that does not enable interesting kernel features while they are still > having their bugs shaken out of them. > > I also think Debians approach to limit things while they mature is also > wisdom. > >>> Posix capabilities are intended for privileged actions, not for >>> actions which explicitly should not require privilege, but which >>> we feel are in development. >>> >> >> Certainly, in an ideal world, user namespaces will never lead to any >> kernel-level exploits. But reality is different: There *have been* >> serious kernel vulnerabilities due to user namespaces, and there *will >> be* serious kernel vulnerabilities due to user namespaces. > > When you start talk about the future that is not yet real you have > stopped talking about reality. That sounds like a pessimists world view > rather than reality. > > The reality is new features are buggy and take time to mature. It takes > time for understanding to percolate through peoples heads. > >> Now, those are the alternatives imho: >> >> * Status quo: Some distributions will disable user namespaces by default >> in some way or another. User wishing to use user namespaces will have to >> use a custom kernel or enable a sysctl flag that was patched in by the >> downstream developers. On distributions that enable user namespaces by >> default, even users that don't wish to use them in the first places will >> be affected by vulnerabilities. > > Again I disagree. I see distributions waiting to enable user namespaces > until they mature and until they are interesting enough. I do not see > rushing to enable the newest features as wisdom, unless that the point > of your distribution is to enable people to play with the latest > features. > > I suspect we are quickly coming to a point where user namespaces will be > sufficiently compelling that they will be enabled more widely. > > > At this point the most helpful things I can see to be done are. > - Verify all userns related fixes have made it back into 4.1.x > - Play with and/or audit the userns code to see if more bugs can be > found. > - Analyze user namespaces and see if they are uniquely worse than > anything else. > > I agree that if user namespaces pose a unique security challenge to > the kernel we should do something about them. I think it is a healthy > question to ask. For the conversation to be productive I think we need > numbers and analsysis, not just worst case analsysis based on fear. To > date all I see are teething pains. > > My back of the napkin analysis is that there are maybe 3,000 lines of > code executed in user namespaces (mostly from fs/namespace.c) that > are not otherwise reachable from unprivileged users, while there are > perhaps 100,000 - 250,000 lines of code reachable by unprivileged users > (not counting drivers). At the risk of pointing out a can of worms, the attack surface also includes things like the iptables configuration APIs,
Re: [PATCH] userns/capability: Add user namespace capability
Andy Lutomirskiwrites: > At the risk of pointing out a can of worms, the attack surface also > includes things like the iptables configuration APIs, parsers, and > filter/conntrack/action modules. It is worth noting that module auto-load does not happen if the triggering code does not have the proper permissions in the initial user namespace. I agree that is another piece of code that should be counted. How that compares to the other 130,000 or so lines of code in the network stack an unprivileged user can caused to be exercised already I don't know. In my back of the napkin swag I had totally forgotten to count anything in the network stack. A lot of the netfilter code that I have read and looked at is compartively simple and clean so I don't expect there is much risk except from sheer volume of code there. It is also tricky to count because the entire network side of the networking stack is exposed to hostile users on the internet so anything except the configuration is already exposed to hostile users. The average check entry is 15-20 lines long. There appear to be 117 unique check entry functions in the kernel so there may be another 2.5k lines of code there. Hmm. And we have not had any design issues with the network stack. Absent of design issues where the code even when implemented correctly has the wrong semantics, we are left with the probability of exploitable buggy code. I suspect we have enough code even without user namespaces enabled that the probability of exploitable buggy code someone in the code that unprivilged users can cause to be exercised run is > 50%. I wonder if there are any good statistical models that give realistic estimates of those things. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] KEYS: Miscellaneous patches for next
On Thu, 22 Oct 2015, David Howells wrote: > Hi James, > > Could you pull these changes into your next branch please? > > There are three groups: > > (1) Miscellaneous cleanups. > > (2) Add scripts for extracting system cert list and module sigs. > > (3) Condense the type-specific data in the key struct into the payload > data as it doesn't really make any sense to keep them separate. > Pulled. Have these been in next yet? -- James Morris-- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/1] Tagging: a new Security Module
INTRODUCTION Adding a feature in the kernel is not something free, it must have some interest. I will try here to explain the reasons why I am posting here a new bag of code. I studied the security of Tizen 3 [1] and modestly participated to it. Tizen 3 uses Smack as its security background. But managing API level authorisation with Smack is nor simple nor very efficient. The current implementation uses a component named "cynara" [2] to record (database) the authorisation (aka privilege) of applications. Services called can check wether an instance of an application has or not the authorisation for given user. Before cynara came on the scene, I made some studies. One of it tried to implement a keyring of authorisations using fuse [3]. The very basic idea is to securely and publicly tag processes with authorisations that they have. The important thing here The research based on fuse was interesting but it had big issues. The biggest being that it can not follows application's lifecycle: clone, execve, exit... I presented most aspects of that it at FOSDEM 2015 [4] (thanks to Mikos). Most of issues are merely disappearing when the keyring of authorisations is managed as a LSM (Linux Security Module). unfortunately, the current implementation of LSM isn't compatible with the stacking of LSM. I made a first proposal of the tagging system as a submodule of Smack for many convenient reasons. But after some mail exchanges with Casey Schaufler, I prefer to submit it as a new Security Module that should be stacked over some other security module. It is valuable because it is typically independant: it could be used "on" Smack or "on" SELinux or "on" AppArmor. So, it may help to promote effort to allow real stacking of security components. And I think that it is a good direction. Nevertheless, stacking is not really possible and currently activating tagging only works with the LSM Smack. Let me know if other implmentation is of interest. HOW TO ACTIVATE IT? === It is a sub-module of Smack and it can be activated/deactivated in the config using CONFIG_SECURITY_SMACK_TAGS. WHAT IS IT DOING? = Each process or thread can have a list of tags. This list is managed by a security component of the system and is part of a chain of trust. It can be used in many ways. The main idea is to tag processes. The list can be empty. The list of tags is copied (this is not shared) during 'clone' and mostly dropped during 'execve'. By default: - processes can NOT remove any tags for itself - processes can NOT add any tag to itself - processes can NOT alter (add or remove) tags of other processes - processes mostly lose their tags during 'execve' - processes can read tags of other processes when DAC/MAC allows it But some rules allow authorised processes: - to remove some or all of its tags - to add some or any tag to itself - to alter other processes list of tags - to keep some or all of their tags during 'exec' More accurate details are in the commit message. Except for few special tags, the meaning of the tag is free. WHAT IS THE IDEA BEHIND? An authorised process can add a tag X to itself or other process. Later, an other process can check wether a process has or not the tag X to adapt its behaviour. Mechanisms here given are allowing either a centralized service for tagging processes or a fork/exec model. A such module can be easily used as part of a cynara like authorisation system. LINKS = [1] https://wiki.tizen.org/wiki/Security [2] https://wiki.tizen.org/wiki/Security/Tizen_3.X_Cynara [3] https://github.com/jobol/keyzen [4] https://archive.fosdem.org/2015/schedule/event/sec_enforcement/ José Bollo (1): Tags: Adding tagging feature to security modules fs/proc/base.c | 3 + security/Kconfig | 2 + security/Makefile | 2 + security/smack/Kconfig | 9 + security/smack/smack.h | 7 + security/smack/smack_lsm.c | 50 security/tags/Kconfig | 8 + security/tags/Makefile | 6 + security/tags/tags.c | 731 + security/tags/tags.h | 39 +++ 10 files changed, 857 insertions(+) create mode 100644 security/tags/Kconfig create mode 100644 security/tags/Makefile create mode 100644 security/tags/tags.c create mode 100644 security/tags/tags.h -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html