[PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

2015-10-22 Thread Dmitry Kasatkin
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

2015-10-22 Thread Dmitry Kasatkin
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

2015-10-22 Thread Dmitry Kasatkin
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

2015-10-22 Thread Dmitry Kasatkin
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;

2015-10-22 Thread Dmitry Kasatkin
Hi Petko,

I have a question

On Fri, Oct 16, 2015 at 10:31 PM, Petko Manolov  wrote:
> 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

2015-10-22 Thread José Bollo
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

2015-10-22 Thread Andy Lutomirski
On Thu, Oct 22, 2015 at 1:45 PM, Eric W. Biederman
 wrote:
>
> 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

2015-10-22 Thread Eric W. Biederman
Andy Lutomirski  writes:

> 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

2015-10-22 Thread James Morris
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

2015-10-22 Thread José Bollo
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