Re: [PATCH v4 3/7] security: Make inode argument of inode_getsecid non-const

2015-10-29 Thread Stephen Smalley

On 10/28/2015 08:47 PM, Andreas Gruenbacher wrote:

Make the inode argument of the inode_getsecid hook non-const so that we
can use it to revalidate invalid security labels.

Signed-off-by: Andreas Gruenbacher 


Acked-by: Stephen Smalley 


---
  include/linux/audit.h  | 8 
  include/linux/lsm_hooks.h  | 2 +-
  include/linux/security.h   | 4 ++--
  kernel/audit.c | 2 +-
  kernel/audit.h | 2 +-
  kernel/auditsc.c   | 6 +++---
  security/security.c| 2 +-
  security/selinux/hooks.c   | 2 +-
  security/smack/smack_lsm.c | 2 +-
  9 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index b2abc99..7a9e0d7 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -137,7 +137,7 @@ extern void __audit_getname(struct filename *name);
  extern void __audit_inode(struct filename *name, const struct dentry *dentry,
unsigned int flags);
  extern void __audit_file(const struct file *);
-extern void __audit_inode_child(const struct inode *parent,
+extern void __audit_inode_child(struct inode *parent,
const struct dentry *dentry,
const unsigned char type);
  extern void __audit_seccomp(unsigned long syscall, long signr, int code);
@@ -202,7 +202,7 @@ static inline void audit_inode_parent_hidden(struct 
filename *name,
__audit_inode(name, dentry,
AUDIT_INODE_PARENT | AUDIT_INODE_HIDDEN);
  }
-static inline void audit_inode_child(const struct inode *parent,
+static inline void audit_inode_child(struct inode *parent,
 const struct dentry *dentry,
 const unsigned char type) {
if (unlikely(!audit_dummy_context()))
@@ -359,7 +359,7 @@ static inline void __audit_inode(struct filename *name,
const struct dentry *dentry,
unsigned int flags)
  { }
-static inline void __audit_inode_child(const struct inode *parent,
+static inline void __audit_inode_child(struct inode *parent,
const struct dentry *dentry,
const unsigned char type)
  { }
@@ -373,7 +373,7 @@ static inline void audit_file(struct file *file)
  static inline void audit_inode_parent_hidden(struct filename *name,
const struct dentry *dentry)
  { }
-static inline void audit_inode_child(const struct inode *parent,
+static inline void audit_inode_child(struct inode *parent,
 const struct dentry *dentry,
 const unsigned char type)
  { }
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index bdd0a3a..4c48227 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1420,7 +1420,7 @@ union security_list_options {
int flags);
int (*inode_listsecurity)(struct inode *inode, char *buffer,
size_t buffer_size);
-   void (*inode_getsecid)(const struct inode *inode, u32 *secid);
+   void (*inode_getsecid)(struct inode *inode, u32 *secid);

int (*file_permission)(struct file *file, int mask);
int (*file_alloc_security)(struct file *file);
diff --git a/include/linux/security.h b/include/linux/security.h
index 9ee61b2..e79149a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -273,7 +273,7 @@ int security_inode_killpriv(struct dentry *dentry);
  int security_inode_getsecurity(struct inode *inode, const char *name, void 
**buffer, bool alloc);
  int security_inode_setsecurity(struct inode *inode, const char *name, const 
void *value, size_t size, int flags);
  int security_inode_listsecurity(struct inode *inode, char *buffer, size_t 
buffer_size);
-void security_inode_getsecid(const struct inode *inode, u32 *secid);
+void security_inode_getsecid(struct inode *inode, u32 *secid);
  int security_file_permission(struct file *file, int mask);
  int security_file_alloc(struct file *file);
  void security_file_free(struct file *file);
@@ -734,7 +734,7 @@ static inline int security_inode_listsecurity(struct inode 
*inode, char *buffer,
return 0;
  }

-static inline void security_inode_getsecid(const struct inode *inode, u32 
*secid)
+static inline void security_inode_getsecid(struct inode *inode, u32 *secid)
  {
*secid = 0;
  }
diff --git a/kernel/audit.c b/kernel/audit.c
index 662c007..d20f674 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1708,7 +1708,7 @@ static inline int audit_copy_fcaps(struct audit_names 
*name,

  /* Copy inode data into an audit_names. */
  void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
- const 

Re: [PATCH v4 4/7] selinux: Add accessor functions for inode->i_security

2015-10-29 Thread Stephen Smalley

On 10/28/2015 08:47 PM, Andreas Gruenbacher wrote:

Add functions dentry_security and inode_security for accessing
inode->i_security.  These functions initially don't do much, but they
will later be used to revalidate the security labels when necessary.

Signed-off-by: Andreas Gruenbacher 


Acked-by: Stephen Smalley 


---
  security/selinux/hooks.c | 97 
  1 file changed, 56 insertions(+), 41 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a8f09af..48d1908 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -241,6 +241,24 @@ static int inode_alloc_security(struct inode *inode)
return 0;
  }

+/*
+ * Get the security label of an inode.
+ */
+static struct inode_security_struct *inode_security(struct inode *inode)
+{
+   return inode->i_security;
+}
+
+/*
+ * Get the security label of a dentry's backing inode.
+ */
+static struct inode_security_struct *backing_inode_security(struct dentry 
*dentry)
+{
+   struct inode *inode = d_backing_inode(dentry);
+
+   return inode->i_security;
+}
+
  static void inode_free_rcu(struct rcu_head *head)
  {
struct inode_security_struct *isec;
@@ -564,8 +582,8 @@ static int selinux_get_mnt_opts(const struct super_block 
*sb,
opts->mnt_opts_flags[i++] = DEFCONTEXT_MNT;
}
if (sbsec->flags & ROOTCONTEXT_MNT) {
-   struct inode *root = d_backing_inode(sbsec->sb->s_root);
-   struct inode_security_struct *isec = root->i_security;
+   struct dentry *root = sbsec->sb->s_root;
+   struct inode_security_struct *isec = 
backing_inode_security(root);

rc = security_sid_to_context(isec->sid, , );
if (rc)
@@ -620,8 +638,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
int rc = 0, i;
struct superblock_security_struct *sbsec = sb->s_security;
const char *name = sb->s_type->name;
-   struct inode *inode = d_backing_inode(sbsec->sb->s_root);
-   struct inode_security_struct *root_isec = inode->i_security;
+   struct dentry *root = sbsec->sb->s_root;
+   struct inode_security_struct *root_isec = backing_inode_security(root);
u32 fscontext_sid = 0, context_sid = 0, rootcontext_sid = 0;
u32 defcontext_sid = 0;
char **mount_options = opts->mnt_opts;
@@ -852,8 +870,8 @@ static int selinux_cmp_sb_context(const struct super_block 
*oldsb,
if ((oldflags & DEFCONTEXT_MNT) && old->def_sid != new->def_sid)
goto mismatch;
if (oldflags & ROOTCONTEXT_MNT) {
-   struct inode_security_struct *oldroot = 
d_backing_inode(oldsb->s_root)->i_security;
-   struct inode_security_struct *newroot = 
d_backing_inode(newsb->s_root)->i_security;
+   struct inode_security_struct *oldroot = 
backing_inode_security(oldsb->s_root);
+   struct inode_security_struct *newroot = 
backing_inode_security(newsb->s_root);
if (oldroot->sid != newroot->sid)
goto mismatch;
}
@@ -903,17 +921,14 @@ static int selinux_sb_clone_mnt_opts(const struct 
super_block *oldsb,
if (!set_fscontext)
newsbsec->sid = sid;
if (!set_rootcontext) {
-   struct inode *newinode = d_backing_inode(newsb->s_root);
-   struct inode_security_struct *newisec = 
newinode->i_security;
+   struct inode_security_struct *newisec = 
backing_inode_security(newsb->s_root);
newisec->sid = sid;
}
newsbsec->mntpoint_sid = sid;
}
if (set_rootcontext) {
-   const struct inode *oldinode = d_backing_inode(oldsb->s_root);
-   const struct inode_security_struct *oldisec = 
oldinode->i_security;
-   struct inode *newinode = d_backing_inode(newsb->s_root);
-   struct inode_security_struct *newisec = newinode->i_security;
+   const struct inode_security_struct *oldisec = 
backing_inode_security(oldsb->s_root);
+   struct inode_security_struct *newisec = 
backing_inode_security(newsb->s_root);

newisec->sid = oldisec->sid;
}
@@ -1712,13 +1727,13 @@ out:
  /*
   * Determine the label for an inode that might be unioned.
   */
-static int selinux_determine_inode_label(const struct inode *dir,
+static int selinux_determine_inode_label(struct inode *dir,
 const struct qstr *name,
 u16 tclass,
 u32 *_new_isid)
  {
const struct superblock_security_struct *sbsec = dir->i_sb->s_security;
-   const struct inode_security_struct *dsec = dir->i_security;
+   const struct inode_security_struct 

Re: [PATCH v4 6/7] selinux: Revalidate invalid inode security labels

2015-10-29 Thread Stephen Smalley

On 10/28/2015 08:47 PM, Andreas Gruenbacher wrote:

When fetching an inode's security label, check if it is still valid, and
try reloading it if it is not. Reloading will fail when we are in RCU
context which doesn't allow sleeping, or when we can't find a dentry for
the inode.  (Reloading happens via iop->getxattr which takes a dentry
parameter.)  When reloading fails, continue using the old, invalid
label.

Signed-off-by: Andreas Gruenbacher 


Could probably use inode_security_novalidate() for all of the 
SOCK_INODE() cases, right?  Otherwise,

Acked-by: Stephen Smalley 


---
  security/selinux/hooks.c | 70 
  1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dcac6dc..47dc704 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -241,11 +241,63 @@ static int inode_alloc_security(struct inode *inode)
return 0;
  }

+static int inode_doinit_with_dentry(struct inode *inode, struct dentry 
*opt_dentry);
+
+/*
+ * Try reloading inode security labels that have been marked as invalid.  The
+ * @may_sleep parameter indicates when sleeping and thus reloading labels is
+ * allowed; when set to false, returns ERR_PTR(-ECHILD) when the label is
+ * invalid.  The @opt_dentry parameter should be set to a dentry of the inode;
+ * when no dentry is available, set it to NULL instead.
+ */
+static int __inode_security_revalidate(struct inode *inode,
+  struct dentry *opt_dentry,
+  bool may_sleep)
+{
+   struct inode_security_struct *isec = inode->i_security;
+
+   might_sleep_if(may_sleep);
+
+   if (isec->initialized == LABEL_INVALID) {
+   if (!may_sleep)
+   return -ECHILD;
+
+   /*
+* Try reloading the inode security label.  This will fail if
+* @opt_dentry is NULL and no dentry for this inode can be
+* found; in that case, continue using the old label.
+*/
+   inode_doinit_with_dentry(inode, opt_dentry);
+   }
+   return 0;
+}
+
+static void inode_security_revalidate(struct inode *inode)
+{
+   __inode_security_revalidate(inode, NULL, true);
+}
+
+static struct inode_security_struct *inode_security_novalidate(struct inode 
*inode)
+{
+   return inode->i_security;
+}
+
+static struct inode_security_struct *inode_security_rcu(struct inode *inode, 
bool rcu)
+{
+   int error;
+
+   error = __inode_security_revalidate(inode, NULL, !rcu);
+   if (error)
+   return ERR_PTR(error);
+   return inode->i_security;
+}
+
  /*
   * Get the security label of an inode.
   */
  static struct inode_security_struct *inode_security(struct inode *inode)
  {
+   __inode_security_revalidate(inode, NULL, true);
return inode->i_security;
  }

@@ -256,6 +308,7 @@ static struct inode_security_struct 
*backing_inode_security(struct dentry *dentr
  {
struct inode *inode = d_backing_inode(dentry);

+   __inode_security_revalidate(inode, dentry, true);
return inode->i_security;
  }

@@ -362,8 +415,6 @@ static const char *labeling_behaviors[7] = {
"uses native labeling",
  };

-static int inode_doinit_with_dentry(struct inode *inode, struct dentry 
*opt_dentry);
-
  static inline int inode_doinit(struct inode *inode)
  {
return inode_doinit_with_dentry(inode, NULL);
@@ -1655,6 +1706,7 @@ static inline int dentry_has_perm(const struct cred *cred,

ad.type = LSM_AUDIT_DATA_DENTRY;
ad.u.dentry = dentry;
+   __inode_security_revalidate(inode, dentry, true);
return inode_has_perm(cred, inode, av, );
  }

@@ -1670,6 +1722,7 @@ static inline int path_has_perm(const struct cred *cred,

ad.type = LSM_AUDIT_DATA_PATH;
ad.u.path = *path;
+   __inode_security_revalidate(inode, path->dentry, true);
return inode_has_perm(cred, inode, av, );
  }

@@ -2874,7 +2927,9 @@ static int selinux_inode_follow_link(struct dentry 
*dentry, struct inode *inode,
ad.type = LSM_AUDIT_DATA_DENTRY;
ad.u.dentry = dentry;
sid = cred_sid(cred);
-   isec = inode_security(inode);
+   isec = inode_security_rcu(inode, rcu);
+   if (IS_ERR(isec))
+   return PTR_ERR(isec);

return avc_has_perm_flags(sid, isec->sid, isec->sclass, FILE__READ, ,
  rcu ? MAY_NOT_BLOCK : 0);
@@ -2926,7 +2981,9 @@ static int selinux_inode_permission(struct inode *inode, 
int mask)
perms = file_mask_to_av(inode->i_mode, mask);

sid = cred_sid(cred);
-   isec = inode_security(inode);
+   isec = inode_security_rcu(inode, flags & MAY_NOT_BLOCK);
+   if (IS_ERR(isec))
+   return PTR_ERR(isec);

rc = avc_has_perm_noaudit(sid, isec->sid, isec->sclass, 

[PATCH v1 1/4] crypto: add entry for sm3-256

2015-10-29 Thread Jarkko Sakkinen
Added entry for sm3-256 to the following tables:

* hash_algo_name
* hash_digest_size

Needed for TPM 2.0 trusted key sealing.

Signed-off-by: Jarkko Sakkinen 
---
 crypto/hash_info.c | 2 ++
 include/crypto/hash_info.h | 3 +++
 include/uapi/linux/hash_info.h | 1 +
 3 files changed, 6 insertions(+)

diff --git a/crypto/hash_info.c b/crypto/hash_info.c
index 3e7ff46..6f3a113 100644
--- a/crypto/hash_info.c
+++ b/crypto/hash_info.c
@@ -31,6 +31,7 @@ const char *const hash_algo_name[HASH_ALGO__LAST] = {
[HASH_ALGO_TGR_128] = "tgr128",
[HASH_ALGO_TGR_160] = "tgr160",
[HASH_ALGO_TGR_192] = "tgr192",
+   [HASH_ALGO_SM3_256] = "sm3-256",
 };
 EXPORT_SYMBOL_GPL(hash_algo_name);
 
@@ -52,5 +53,6 @@ const int hash_digest_size[HASH_ALGO__LAST] = {
[HASH_ALGO_TGR_128] = TGR128_DIGEST_SIZE,
[HASH_ALGO_TGR_160] = TGR160_DIGEST_SIZE,
[HASH_ALGO_TGR_192] = TGR192_DIGEST_SIZE,
+   [HASH_ALGO_SM3_256] = SM3_256_DIGEST_SIZE,
 };
 EXPORT_SYMBOL_GPL(hash_digest_size);
diff --git a/include/crypto/hash_info.h b/include/crypto/hash_info.h
index e1e5a3e..d86e050 100644
--- a/include/crypto/hash_info.h
+++ b/include/crypto/hash_info.h
@@ -34,6 +34,9 @@
 #define TGR160_DIGEST_SIZE 20
 #define TGR192_DIGEST_SIZE 24
 
+/* not defined in include/crypto/ */
+#define SM3_256_DIGEST_SIZE 32
+
 extern const char *const hash_algo_name[HASH_ALGO__LAST];
 extern const int hash_digest_size[HASH_ALGO__LAST];
 
diff --git a/include/uapi/linux/hash_info.h b/include/uapi/linux/hash_info.h
index ca18c45..ebf8fd8 100644
--- a/include/uapi/linux/hash_info.h
+++ b/include/uapi/linux/hash_info.h
@@ -31,6 +31,7 @@ enum hash_algo {
HASH_ALGO_TGR_128,
HASH_ALGO_TGR_160,
HASH_ALGO_TGR_192,
+   HASH_ALGO_SM3_256,
HASH_ALGO__LAST
 };
 
-- 
2.5.0

--
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 v1 4/4] keys, trusted: update documentation for 'hash=' option

2015-10-29 Thread Jarkko Sakkinen
Documented 'hash=' option.

Signed-off-by: Jarkko Sakkinen 
---
 Documentation/security/keys-trusted-encrypted.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/security/keys-trusted-encrypted.txt 
b/Documentation/security/keys-trusted-encrypted.txt
index e105ae9..fd2565b 100644
--- a/Documentation/security/keys-trusted-encrypted.txt
+++ b/Documentation/security/keys-trusted-encrypted.txt
@@ -38,6 +38,9 @@ Usage:
pcrlock=  pcr number to be extended to "lock" blob
migratable= 0|1 indicating permission to reseal to new PCR values,
default 1 (resealing allowed)
+   hash=  hash algorithm name as a string. For TPM 1.x the only
+  allowed value is sha1. For TPM 2.x the allowed values
+ are sha1, sha256, sha384, sha512 and sm3-256.
 
 "keyctl print" returns an ascii hex copy of the sealed key, which is in 
standard
 TPM_STORED_DATA format.  The key length for new keys are always in bytes.
-- 
2.5.0

--
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 v1 0/4] TPM2: select hash algorithm for a trusted key

2015-10-29 Thread Jarkko Sakkinen
Jarkko Sakkinen (4):
  crypto: add entry for sm3-256
  tpm: choose hash algorithm for sealing when using TPM 2.0
  keys, trusted: select the hash algorithm
  keys, trusted: update documentation for 'hash=' option

 Documentation/security/keys-trusted-encrypted.txt |  3 ++
 crypto/hash_info.c|  2 ++
 drivers/char/tpm/tpm.h| 10 --
 drivers/char/tpm/tpm2-cmd.c   | 42 +--
 include/crypto/hash_info.h|  3 ++
 include/keys/trusted-type.h   |  1 +
 include/uapi/linux/hash_info.h|  1 +
 security/keys/trusted.c   | 20 ++-
 8 files changed, 75 insertions(+), 7 deletions(-)

-- 
2.5.0

--
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 v1 3/4] keys, trusted: select the hash algorithm

2015-10-29 Thread Jarkko Sakkinen
Added 'hash=' option for selecting the hash algorithm for add_key()
syscall.

Signed-off-by: Jarkko Sakkinen 
---
 security/keys/trusted.c | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index d3633cf..7a87bcd 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -11,6 +11,7 @@
  * See Documentation/security/keys-trusted-encrypted.txt
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -710,7 +711,8 @@ enum {
Opt_err = -1,
Opt_new, Opt_load, Opt_update,
Opt_keyhandle, Opt_keyauth, Opt_blobauth,
-   Opt_pcrinfo, Opt_pcrlock, Opt_migratable
+   Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
+   Opt_hash,
 };
 
 static const match_table_t key_tokens = {
@@ -723,6 +725,7 @@ static const match_table_t key_tokens = {
{Opt_pcrinfo, "pcrinfo=%s"},
{Opt_pcrlock, "pcrlock=%s"},
{Opt_migratable, "migratable=%s"},
+   {Opt_hash, "hash=%s"},
{Opt_err, NULL}
 };
 
@@ -736,6 +739,7 @@ static int getoptions(char *c, struct trusted_key_payload 
*pay,
int res;
unsigned long handle;
unsigned long lock;
+   int i;
 
while ((p = strsep(, " \t"))) {
if (*p == '\0' || *p == ' ' || *p == '\t')
@@ -787,6 +791,20 @@ static int getoptions(char *c, struct trusted_key_payload 
*pay,
return -EINVAL;
opt->pcrlock = lock;
break;
+   case Opt_hash:
+   for (i = 0; i < HASH_ALGO__LAST; i++) {
+   if (!strcmp(args[0].from, hash_algo_name[i])) {
+   opt->hash = i;
+   break;
+   }
+   }
+   res = tpm_is_tpm2(TPM_ANY_NUM);
+   if (res < 0)
+   return res;
+   if (i == HASH_ALGO__LAST ||
+   (!res && i != HASH_ALGO_SHA1))
+   return -EINVAL;
+   break;
default:
return -EINVAL;
}
-- 
2.5.0

--
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 v1 2/4] tpm: choose hash algorithm for sealing when using TPM 2.0

2015-10-29 Thread Jarkko Sakkinen
Added hash member to the struct trusted_key_options for choosing the
hash algorithm and support for the following hash algorithms to the TPM
2.0 sealing code:

* sha1
* sha256
* sha384
* sha512
* sm3-256

The hash algorithm can be selected by using HASH_ALGO_* constants in
include/uapi/linux/hash_info.h.

Signed-off-by: Jarkko Sakkinen 
---
 drivers/char/tpm/tpm.h  | 10 +++---
 drivers/char/tpm/tpm2-cmd.c | 42 +++---
 include/keys/trusted-type.h |  1 +
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a4257a3..cdd49cd 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -83,16 +83,20 @@ enum tpm2_structures {
 };
 
 enum tpm2_return_codes {
-   TPM2_RC_INITIALIZE  = 0x0100,
-   TPM2_RC_TESTING = 0x090A,
+   TPM2_RC_HASH= 0x0083, /* RC_FMT1 */
+   TPM2_RC_INITIALIZE  = 0x0100, /* RC_VER1 */
TPM2_RC_DISABLED= 0x0120,
+   TPM2_RC_TESTING = 0x090A, /* RC_WARN */
 };
 
 enum tpm2_algorithms {
TPM2_ALG_SHA1   = 0x0004,
TPM2_ALG_KEYEDHASH  = 0x0008,
TPM2_ALG_SHA256 = 0x000B,
-   TPM2_ALG_NULL   = 0x0010
+   TPM2_ALG_SHA384 = 0x000C,
+   TPM2_ALG_SHA512 = 0x000D,
+   TPM2_ALG_NULL   = 0x0010,
+   TPM2_ALG_SM3_256= 0x0012,
 };
 
 enum tpm2_command_codes {
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index bd7039f..bc2564e 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -16,6 +16,7 @@
  */
 
 #include "tpm.h"
+#include 
 #include 
 
 enum tpm2_object_attributes {
@@ -104,6 +105,21 @@ struct tpm2_cmd {
union tpm2_cmd_params   params;
 } __packed;
 
+struct tpm2_hash {
+   unsigned int crypto_id;
+   unsigned int tpm_id;
+};
+
+static struct tpm2_hash tpm2_hash_map[] = {
+   {HASH_ALGO_SHA1, TPM2_ALG_SHA1},
+   {HASH_ALGO_SHA256, TPM2_ALG_SHA256},
+   {HASH_ALGO_SHA384, TPM2_ALG_SHA384},
+   {HASH_ALGO_SHA512, TPM2_ALG_SHA512},
+   {HASH_ALGO_SM3_256, TPM2_ALG_SM3_256},
+};
+
+#define TPM2_HASH_COUNT (sizeof(tpm2_hash_map) / sizeof(tpm2_hash_map[1]))
+
 /*
  * Array with one entry per ordinal defining the maximum amount
  * of time the chip could take to return the result. The values
@@ -429,8 +445,24 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
unsigned int blob_len;
struct tpm_buf buf;
+   u32 hash = TPM2_ALG_SHA256;
+   int i;
int rc;
 
+   if (options->hash) {
+   for (i = 0; i < TPM2_HASH_COUNT; i++) {
+   if (options->hash == tpm2_hash_map[i].crypto_id) {
+   hash = tpm2_hash_map[i].tpm_id;
+   dev_dbg(chip->pdev, "%s: hash: %s 0x%08X\n",
+   __func__, hash_algo_name[i], hash);
+   break;
+   }
+   }
+
+   if (i == TPM2_HASH_COUNT)
+   return -EINVAL;
+   }
+
rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
if (rc)
return rc;
@@ -454,7 +486,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
tpm_buf_append_u16(, 14);
 
tpm_buf_append_u16(, TPM2_ALG_KEYEDHASH);
-   tpm_buf_append_u16(, TPM2_ALG_SHA256);
+   tpm_buf_append_u16(, hash);
tpm_buf_append_u32(, TPM2_ATTR_USER_WITH_AUTH);
tpm_buf_append_u16(, 0); /* policy digest size */
tpm_buf_append_u16(, TPM2_ALG_NULL);
@@ -487,8 +519,12 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 out:
tpm_buf_destroy();
 
-   if (rc > 0)
-   rc = -EPERM;
+   if (rc > 0) {
+   if ((rc & TPM2_RC_HASH) == TPM2_RC_HASH)
+   rc = -EINVAL;
+   else
+   rc = -EPERM;
+   }
 
return rc;
 }
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index f91ecd9..8fed58d 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -36,6 +36,7 @@ struct trusted_key_options {
uint32_t pcrinfo_len;
unsigned char pcrinfo[MAX_PCRINFO_SIZE];
int pcrlock;
+   unsigned int hash;
 };
 
 extern struct key_type key_type_trusted;
-- 
2.5.0

--
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 5/7] security: Add hook to invalidate inode security labels

2015-10-29 Thread Stephen Smalley

On 10/28/2015 08:47 PM, Andreas Gruenbacher wrote:

Add a hook to invalidate an inode's security label when the cached
information becomes invalid.

Add the new hook in selinux: set a flag when a security label becomes
invalid.

Signed-off-by: Andreas Gruenbacher 
Reviewed-by: James Morris 


Acked-by: Stephen Smalley 


---
  include/linux/lsm_hooks.h |  6 ++
  include/linux/security.h  |  5 +
  security/security.c   |  8 
  security/selinux/hooks.c  | 30 --
  security/selinux/include/objsec.h |  6 ++
  5 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4c48227..71969de 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1261,6 +1261,10 @@
   *audit_rule_init.
   *@rule contains the allocated rule
   *
+ * @inode_invalidate_secctx:
+ * Notify the security module that it must revalidate the security context
+ * of an inode.
+ *
   * @inode_notifysecctx:
   *Notify the security module of what the security context of an inode
   *should be.  Initializes the incore security context managed by the
@@ -1516,6 +1520,7 @@ union security_list_options {
int (*secctx_to_secid)(const char *secdata, u32 seclen, u32 *secid);
void (*release_secctx)(char *secdata, u32 seclen);

+   void (*inode_invalidate_secctx)(struct inode *inode);
int (*inode_notifysecctx)(struct inode *inode, void *ctx, u32 ctxlen);
int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
@@ -1757,6 +1762,7 @@ struct security_hook_heads {
struct list_head secid_to_secctx;
struct list_head secctx_to_secid;
struct list_head release_secctx;
+   struct list_head inode_invalidate_secctx;
struct list_head inode_notifysecctx;
struct list_head inode_setsecctx;
struct list_head inode_getsecctx;
diff --git a/include/linux/security.h b/include/linux/security.h
index e79149a..4824a4c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -353,6 +353,7 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 
*seclen);
  int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
  void security_release_secctx(char *secdata, u32 seclen);

+void security_inode_invalidate_secctx(struct inode *inode);
  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
  int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
@@ -1093,6 +1094,10 @@ static inline void security_release_secctx(char 
*secdata, u32 seclen)
  {
  }

+static inline void security_inode_invalidate_secctx(struct inode *inode)
+{
+}
+
  static inline int security_inode_notifysecctx(struct inode *inode, void *ctx, 
u32 ctxlen)
  {
return -EOPNOTSUPP;
diff --git a/security/security.c b/security/security.c
index c5beb7e..e8ffd92 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1161,6 +1161,12 @@ void security_release_secctx(char *secdata, u32 seclen)
  }
  EXPORT_SYMBOL(security_release_secctx);

+void security_inode_invalidate_secctx(struct inode *inode)
+{
+   call_void_hook(inode_invalidate_secctx, inode);
+}
+EXPORT_SYMBOL(security_inode_invalidate_secctx);
+
  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
  {
return call_int_hook(inode_notifysecctx, 0, inode, ctx, ctxlen);
@@ -1763,6 +1769,8 @@ struct security_hook_heads security_hook_heads = {
LIST_HEAD_INIT(security_hook_heads.secctx_to_secid),
.release_secctx =
LIST_HEAD_INIT(security_hook_heads.release_secctx),
+   .inode_invalidate_secctx =
+   LIST_HEAD_INIT(security_hook_heads.inode_invalidate_secctx),
.inode_notifysecctx =
LIST_HEAD_INIT(security_hook_heads.inode_notifysecctx),
.inode_setsecctx =
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 48d1908..dcac6dc 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -820,7 +820,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
goto out;

root_isec->sid = rootcontext_sid;
-   root_isec->initialized = 1;
+   root_isec->initialized = LABEL_INITIALIZED;
}

if (defcontext_sid) {
@@ -1308,11 +1308,11 @@ static int inode_doinit_with_dentry(struct inode 
*inode, struct dentry *opt_dent
unsigned len = 0;
int rc = 0;

-   if (isec->initialized)
+   if (isec->initialized == LABEL_INITIALIZED)
goto out;

mutex_lock(>lock);
-   if (isec->initialized)
+   if (isec->initialized 

Re: [PATCH v1 2/4] tpm: choose hash algorithm for sealing when using TPM 2.0

2015-10-29 Thread Jarkko Sakkinen
On Thu, Oct 29, 2015 at 05:59:26PM +0200, Jarkko Sakkinen wrote:
> Added hash member to the struct trusted_key_options for choosing the
> hash algorithm and support for the following hash algorithms to the TPM
> 2.0 sealing code:
> 
> * sha1
> * sha256
> * sha384
> * sha512
> * sm3-256
> 
> The hash algorithm can be selected by using HASH_ALGO_* constants in
> include/uapi/linux/hash_info.h.
> 
> Signed-off-by: Jarkko Sakkinen 
> ---
>  drivers/char/tpm/tpm.h  | 10 +++---
>  drivers/char/tpm/tpm2-cmd.c | 42 +++---
>  include/keys/trusted-type.h |  1 +
>  3 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index a4257a3..cdd49cd 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -83,16 +83,20 @@ enum tpm2_structures {
>  };
>  
>  enum tpm2_return_codes {
> - TPM2_RC_INITIALIZE  = 0x0100,
> - TPM2_RC_TESTING = 0x090A,
> + TPM2_RC_HASH= 0x0083, /* RC_FMT1 */
> + TPM2_RC_INITIALIZE  = 0x0100, /* RC_VER1 */
>   TPM2_RC_DISABLED= 0x0120,
> + TPM2_RC_TESTING = 0x090A, /* RC_WARN */
>  };
>  
>  enum tpm2_algorithms {
>   TPM2_ALG_SHA1   = 0x0004,
>   TPM2_ALG_KEYEDHASH  = 0x0008,
>   TPM2_ALG_SHA256 = 0x000B,
> - TPM2_ALG_NULL   = 0x0010
> + TPM2_ALG_SHA384 = 0x000C,
> + TPM2_ALG_SHA512 = 0x000D,
> + TPM2_ALG_NULL   = 0x0010,
> + TPM2_ALG_SM3_256= 0x0012,
>  };
>  
>  enum tpm2_command_codes {
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index bd7039f..bc2564e 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -16,6 +16,7 @@
>   */
>  
>  #include "tpm.h"
> +#include 
>  #include 
>  
>  enum tpm2_object_attributes {
> @@ -104,6 +105,21 @@ struct tpm2_cmd {
>   union tpm2_cmd_params   params;
>  } __packed;
>  
> +struct tpm2_hash {
> + unsigned int crypto_id;
> + unsigned int tpm_id;
> +};
> +
> +static struct tpm2_hash tpm2_hash_map[] = {
> + {HASH_ALGO_SHA1, TPM2_ALG_SHA1},
> + {HASH_ALGO_SHA256, TPM2_ALG_SHA256},
> + {HASH_ALGO_SHA384, TPM2_ALG_SHA384},
> + {HASH_ALGO_SHA512, TPM2_ALG_SHA512},
> + {HASH_ALGO_SM3_256, TPM2_ALG_SM3_256},
> +};
> +
> +#define TPM2_HASH_COUNT (sizeof(tpm2_hash_map) / sizeof(tpm2_hash_map[1]))
> +
>  /*
>   * Array with one entry per ordinal defining the maximum amount
>   * of time the chip could take to return the result. The values
> @@ -429,8 +445,24 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  {
>   unsigned int blob_len;
>   struct tpm_buf buf;
> + u32 hash = TPM2_ALG_SHA256;
> + int i;
>   int rc;
>  
> + if (options->hash) {
> + for (i = 0; i < TPM2_HASH_COUNT; i++) {
> + if (options->hash == tpm2_hash_map[i].crypto_id) {
> + hash = tpm2_hash_map[i].tpm_id;
> + dev_dbg(chip->pdev, "%s: hash: %s 0x%08X\n",
> + __func__, hash_algo_name[i], hash);
> + break;
> + }
> + }
> +
> + if (i == TPM2_HASH_COUNT)
> + return -EINVAL;
> + }
> +
>   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
>   if (rc)
>   return rc;
> @@ -454,7 +486,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>   tpm_buf_append_u16(, 14);
>  
>   tpm_buf_append_u16(, TPM2_ALG_KEYEDHASH);
> - tpm_buf_append_u16(, TPM2_ALG_SHA256);
> + tpm_buf_append_u16(, hash);
>   tpm_buf_append_u32(, TPM2_ATTR_USER_WITH_AUTH);
>   tpm_buf_append_u16(, 0); /* policy digest size */
>   tpm_buf_append_u16(, TPM2_ALG_NULL);
> @@ -487,8 +519,12 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  out:
>   tpm_buf_destroy();
>  
> - if (rc > 0)
> - rc = -EPERM;
> + if (rc > 0) {
> + if ((rc & TPM2_RC_HASH) == TPM2_RC_HASH)
> + rc = -EINVAL;
> + else
> + rc = -EPERM;
> + }
>  
>   return rc;
>  }
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index f91ecd9..8fed58d 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -36,6 +36,7 @@ struct trusted_key_options {
>   uint32_t pcrinfo_len;
>   unsigned char pcrinfo[MAX_PCRINFO_SIZE];
>   int pcrlock;
> + unsigned int hash;

uint32_t probably here just for the sake of consistency.

>  };
>  
>  extern struct key_type key_type_trusted;
> -- 
> 2.5.0
> 

/Jarkko
--
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 6/7] selinux: Revalidate invalid inode security labels

2015-10-29 Thread Andreas Gruenbacher
On Thu, Oct 29, 2015 at 4:21 PM, Stephen Smalley  wrote:
> On 10/28/2015 08:47 PM, Andreas Gruenbacher wrote:
>>
>> When fetching an inode's security label, check if it is still valid, and
>> try reloading it if it is not. Reloading will fail when we are in RCU
>> context which doesn't allow sleeping, or when we can't find a dentry for
>> the inode.  (Reloading happens via iop->getxattr which takes a dentry
>> parameter.)  When reloading fails, continue using the old, invalid
>> label.
>>
>> Signed-off-by: Andreas Gruenbacher 
>
>
> Could probably use inode_security_novalidate() for all of the SOCK_INODE()
> cases, right?

I guess, yes.

>  Otherwise,
> Acked-by: Stephen Smalley 

Thanks,
Andreas
--
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 v1 3/4] keys, trusted: select the hash algorithm

2015-10-29 Thread kbuild test robot
Hi Jarkko,

[auto build test ERROR on next-20151022 -- if it's inappropriate base, please 
suggest rules for selecting the more suitable base]

url:
https://github.com/0day-ci/linux/commits/Jarkko-Sakkinen/TPM2-select-hash-algorithm-for-a-trusted-key/20151030-000439
config: x86_64-acpi-redef (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   security/built-in.o: In function `getoptions.isra.0':
>> trusted.c:(.text+0x6678): undefined reference to `hash_algo_name'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data


Re: RFC rdma cgroup

2015-10-29 Thread Parav Pandit
Hi Haggai,

On Thu, Oct 29, 2015 at 8:27 PM, Haggai Eran  wrote:
> On 28/10/2015 10:29, Parav Pandit wrote:
>> 3. Resources are not defined by the RDMA cgroup. Resources are defined
>> by RDMA/IB subsystem and optionally by HCA vendor device drivers.
>> Rationale: This allows rdma cgroup to remain constant while RDMA/IB
>> subsystem can evolve without the need of rdma cgroup update. A new
>> resource can be easily added by the RDMA/IB subsystem without touching
>> rdma cgroup.
> Resources exposed by the cgroup are basically a UAPI, so we have to be
> careful to make it stable when it evolves. I understand the need for
> vendor specific resources, following the discussion on the previous
> proposal, but could you write on how you plan to allow these set of
> resources to evolve?

Its fairly simple.
Here is the code snippet on how resources are defined in my tree.
It doesn't have the RSS work queues yet, but can be added right after
this patch.

Resource are defined as index and as match_table_t.

enum rdma_resource_type {
RDMA_VERB_RESOURCE_UCTX,
RDMA_VERB_RESOURCE_AH,
RDMA_VERB_RESOURCE_PD,
RDMA_VERB_RESOURCE_CQ,
RDMA_VERB_RESOURCE_MR,
RDMA_VERB_RESOURCE_MW,
RDMA_VERB_RESOURCE_SRQ,
RDMA_VERB_RESOURCE_QP,
RDMA_VERB_RESOURCE_FLOW,
RDMA_VERB_RESOURCE_MAX,
};
So UAPI RDMA resources can evolve by just adding more entries here.

>
>> 8. Typically each RDMA cgroup will have 0 to 4 RDMA devices. Therefore
>> each cgroup will have 0 to 4 verbs resource pool and optionally 0 to 4
>> hw resource pool per such device.
>> (Nothing stops to have more devices and pools, but design is around
>> this use case).
> In what way does the design depend on this assumption?

Current code when performs resource charging/uncharging, it needs to
identify the resource pool which one to charge to.
This resource pool is maintained as list_head and so its linear search
per device.
If we are thinking of 100 of RDMA devices per container, than liner
search will not be good way and different data structure needs to be
deployed.


>
>> 9. Resource pool object is created in following situations.
>> (a) administrative operation is done to set the limit and no previous
>> resource pool exist for the device of interest for the cgroup.
>> (b) no resource limits were configured, but IB/RDMA subsystem tries to
>> charge the resource. so that when applications are running without
>> limits and later on when limits are enforced, during uncharging, it
>> correctly uncharges them, otherwise usage count will drop to negative.
>> This is done using default resource pool.
>> Instead of implementing any sort of time markers, default pool
>> simplifies the design.
> Having a default resource pool kind of implies there is a non-default
> one. Is the only difference between the default and non-default the fact
> that the second was created with an administrative operation and has
> specified limits or is there some other difference?
>
You described it correctly.

>> (c) When process migrate from one to other cgroup, resource is
>> continue to be owned by the creator cgroup (rather css).
>> After process migration, whenever new resource is created in new
>> cgroup, it will be owned by new cgroup.
> It sounds a little different from how other cgroups behave. I agree that
> mostly processes will create the resources in their cgroup and won't
> migrate, but why not move the charge during migration?
>
With fork() process doesn't really own the resource (unlike other file
and socket descriptors).
Parent process might have died also.
There is possibly no clear way to transfer resource to right child.
Child that cgroup picks might not even want to own RDMA resources.
RDMA resources might be allocated by one process and freed by other
process (though this might not be the way they use it).
Its pretty similar to other cgroups with exception in migration area,
such exception comes from different behavior of how RDMA resources are
owned, created and used.
Recent unified hierarchy patch from Tejun equally highlights to not
frequently migrate processes among cgroups.

So in current implementation, (like other),
if process created a RDMA resource, forked a child.
child and parent both can allocate and free more resources.
child moved to different cgroup. But resource is shared among them.
child can free also the resource. All crazy combinations are possible
in theory (without much use cases).
So at best they are charged to the first cgroup css in which
parent/child are created and reference is hold to CSS.
cgroup, process can die, cut css remains until RDMA resources are freed.
This is similar to process behavior where task struct is release but
id is hold up for a while.


> I finally wanted to ask about other limitations an RDMA cgroup could
> handle. It would be great to be able to limit a container to be allowed
> to use only a subset of the MAC/VLAN pairs programmed 

Re: [RFC PATCH v3 2/5] lsm: introduce hooks for kdbus

2015-10-29 Thread Paul Moore
On Tuesday, October 20, 2015 04:41:14 PM Stephen Smalley wrote:
> On Mon, Oct 19, 2015 at 6:29 PM, Paul Moore  wrote:
> > On Friday, October 09, 2015 10:56:12 AM Stephen Smalley wrote:
> >> On 10/07/2015 07:08 PM, Paul Moore wrote:
> >> > diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> >> > index ef63d65..1cb87b3 100644
> >> > --- a/ipc/kdbus/connection.c
> >> > +++ b/ipc/kdbus/connection.c
> >> > @@ -108,6 +109,14 @@ static struct kdbus_conn *kdbus_conn_new(struct
> >> > kdbus_ep *ep,>
> >> > 
> >> > if (!owner && (creds || pids || seclabel))
> >> > 
> >> > return ERR_PTR(-EPERM);
> >> > 
> >> > +   ret = security_kdbus_conn_new(get_cred(file->f_cred),
> >> 
> >> You only need to use get_cred() if saving a reference; otherwise, you'll
> >> leak one here.
> > 
> > Yes, that was a typo on my part, thanks.
> > 
> >> Also, do we want file->f_cred here or ep->bus->node.creds (the latter is
> >> what is used by their own checks; the former is typically the same as
> >> current cred IIUC).  For that matter, what about ep->node.creds vs
> >> ep->bus-
> >> node.creds vs. ep->bus->domain->node.creds?  Can they differ?  Do we
> >> care?
> > 
> > We don't want file->f_cred, per our previous discussions.  I was working
> > on this patchset in small chunks and while I added credential storing in
> > the nodes, I forgot to update the hooks before I hit send, my apologies.
> > 
> > My current thinking is to pass both the endpoint and bus credentials, as I
> > believe they can differ.  Both the bus and the endpoint inherit their
> > security labels from their creator and while I don't have any specifics,
> > I think it is reasonable to imagine those two processes having different
> > security labels. Assuming we pass both credentials down to the LSM, I'm
> > currently thinking of> 
> > the following SELinux access controls for this hook:
> >   allow  bus_t:kdbus { connect };
> >   allow  ep_t:kdbus { use privileged activator monitor policy };
> 
> I think it would be simpler to apply an associate check when the
> endpoint is created between the endpoint label and the bus label
> (which will typically be the same), and then only check based on
> endpoint label for all subsequent permission checks involving that
> endpoint.  Then you don't have to worry about which label to use for
> all the other permission checks. And you get finer-grained control -
> per-endpoint rather than only per-bus.

After thinking about this for a bit, I agree.

> > ... besides the additional label, I added the kdbus:use permission and
> > dropped the kdbus:owner permission.  Considering that the endpoint label,
> > ep_t, in the examples above, could be different from the current process,
> > it seemed reasonable to want to control that interaction and I felt the
> > fd:use permission was the closest existing control so I reused the
> > permission name. I decided to drop the "owner" permission as it really
> > wasn't the useful for anything anymore, it simply indicates that the
> > current task is the DAC owner of the endpoint.
> 
> Can you 'use' an endpoint in any way other than to connect via it?
> If not, I'd just call that connect (won't conflict if you get rid of
> the separate bus check above), or distinguish it via separate classes
> or as connectthrough vs connectto.

I don't believe so; my understanding is that the main point of endpoints is to 
define special kdbus DAC policy.
 
> conn->owner is used to determine whether the caller can fake
> credentials, skip kdbus policy checking, create an activator, monitor,
> or policy holder connection, etc.  Our options are:
>
> 1. Apply a SELinux check when it is set to see if the caller is
> allowed to own the bus based on MAC labels and policy, and if not,
> refuse to create the connection (that's what checking the owner
> permission was doing).
> 
> 2. Separately apply MAC checks over each of those abilities (fake
> creds, override policy, create an activator, monitor, or policy
> holder, etc) when there is an attempt to exercise them (not all during
> connection creation), and selectively deny that ability.  More
> invasive, more potential for breakage for applications that don't
> expect failure if they could create the connection in the first place.
> 
> 3. Treat faking of DAC credentials and skipping of kdbus policy
> checking as not of interest to MAC, leaving it only controlled by the
> existing uid match or CAP_IPC_OWNER check.  Simple, but seems to lose
> some of the potential benefit of using SELinux for confining processes
> using kdbus.

I agree with you on #3 so let's rule that out.  The same with #2 as I fear we 
would likely get a nasty surprise at some point when the kdbus folks changed 
something and forgot to update the LSM hooks.  That leaves us with option #1 
and the following operations:

* fake credentials
We are already checking this with the kdbus:impersonate permission.

* skip kdbus policy checks
Being an "owner" 

Re: [PATCH v4 02/11] lsm: /proc/$PID/attr/label_map file and getprocattr_seq hook

2015-10-29 Thread Casey Schaufler
On 10/14/2015 5:41 AM, Lukasz Pawelczyk wrote:
> This commit adds a new proc attribute, label_map that is required by an
> upcoming Smack namespace. In general it can be used to hold a map of
> labels, e.g. to be used in namespaces.
>
> Due to the nature of this file, the standard getprocattr hook might not
> be enough to handle it. The map's output can in principle be greater
> than page size to which the aforementioned hook is limited.
> To handle this properly a getprocattr_seq LSM hook has been added that
> makes it possible to handle any chosen proc attr by seq operations.
>
> See the documentation in the patch below for the details about how to
> use the hook.
>
> Signed-off-by: Lukasz Pawelczyk 
> Acked-by: Serge Hallyn 

Acked-by: Casey Schaufler 



> ---
>  fs/proc/base.c| 81 
> +++
>  include/linux/lsm_hooks.h | 15 +
>  include/linux/security.h  |  9 ++
>  security/security.c   |  8 +
>  4 files changed, 107 insertions(+), 6 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index b25eee4..9ec88b8 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2327,20 +2327,77 @@ out:
>  }
>  
>  #ifdef CONFIG_SECURITY
> +static int proc_pid_attr_open(struct inode *inode, struct file *file)
> +{
> + const char *name = file->f_path.dentry->d_name.name;
> + const struct seq_operations *ops;
> + struct task_struct *task;
> + struct seq_file *seq;
> + int ret;
> +
> + file->private_data = NULL;
> +
> + task = get_proc_task(inode);
> + if (!task)
> + return -ESRCH;
> +
> + /* don't use seq_ops if they are not provided by LSM */
> + ret = security_getprocattr_seq(task, name, );
> + if (ret == -EOPNOTSUPP) {
> + ret = 0;
> + goto put_task;
> + }
> + if (ret)
> + goto put_task;
> +
> + ret = seq_open(file, ops);
> + if (ret)
> + goto put_task;
> +
> + seq = file->private_data;
> + seq->private = task;
> +
> + return 0;
> +
> +put_task:
> + put_task_struct(task);
> + return ret;
> +}
> +
> +static int proc_pid_attr_release(struct inode *inode, struct file *file)
> +{
> + struct seq_file *seq;
> + struct task_struct *task;
> +
> + /* don't use seq_ops if they were not provided by LSM */
> + if (file->private_data == NULL)
> + return 0;
> +
> + seq = file->private_data;
> + task = seq->private;
> + put_task_struct(task);
> +
> + return seq_release(inode, file);
> +}
> +
>  static ssize_t proc_pid_attr_read(struct file * file, char __user * buf,
> size_t count, loff_t *ppos)
>  {
> - struct inode * inode = file_inode(file);
> + struct inode *inode = file_inode(file);
> + const char *name = file->f_path.dentry->d_name.name;
>   char *p = NULL;
>   ssize_t length;
> - struct task_struct *task = get_proc_task(inode);
> + struct task_struct *task;
>  
> + /* use seq_ops if they were provided by LSM */
> + if (file->private_data)
> + return seq_read(file, buf, count, ppos);
> +
> + task = get_proc_task(inode);
>   if (!task)
>   return -ESRCH;
>  
> - length = security_getprocattr(task,
> -   (char*)file->f_path.dentry->d_name.name,
> -   );
> + length = security_getprocattr(task, (char *)name, );
>   put_task_struct(task);
>   if (length > 0)
>   length = simple_read_from_buffer(buf, count, ppos, p, length);
> @@ -2348,6 +2405,15 @@ static ssize_t proc_pid_attr_read(struct file * file, 
> char __user * buf,
>   return length;
>  }
>  
> +static loff_t proc_pid_attr_lseek(struct file *file, loff_t offset, int 
> whence)
> +{
> + /* use seq_ops if they were provided by LSM */
> + if (file->private_data)
> + return seq_lseek(file, offset, whence);
> +
> + return generic_file_llseek(file, offset, whence);
> +}
> +
>  static ssize_t proc_pid_attr_write(struct file * file, const char __user * 
> buf,
>  size_t count, loff_t *ppos)
>  {
> @@ -2394,9 +2460,11 @@ out_no_task:
>  }
>  
>  static const struct file_operations proc_pid_attr_operations = {
> + .open   = proc_pid_attr_open,
> + .release= proc_pid_attr_release,
>   .read   = proc_pid_attr_read,
> + .llseek = proc_pid_attr_lseek,
>   .write  = proc_pid_attr_write,
> - .llseek = generic_file_llseek,
>  };
>  
>  static const struct pid_entry attr_dir_stuff[] = {
> @@ -2406,6 +2474,7 @@ 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", 

Re: [PATCH v4 10/11] smack: namespace implementation

2015-10-29 Thread Casey Schaufler
On 10/14/2015 5:42 AM, Lukasz Pawelczyk wrote:
> This commit uses all the changes introduced in "namespace groundwork"
> and previous preparation patches and makes smack aware of its namespace
> and mapped labels.
>
> It modifies the following functions to be namespace aware:
> - smk_access
> - smk_find_label_name
> - smk_get_label
>
> And all functions that use them (e.g. smk_tskacc).
>
> It also adds another function that is used throughout Smack LSM hooks:
> - smk_labels_valid - it checks whether both, subject and object labels
>   are properly mapped in a namespace where they are to be used. This
>   function is used mostly together with a capability check when there is
>   no proper access check that usually checks for that.
>
> All the Smack LSM hooks have been adapted to be namespace aware.
>
> The capabilities (CAP_MAC_ADMIN, CAP_MAC_OVERRIDE) has been allowed in
> the namespace for few cases. Check the documentation for the details.
>
> Signed-off-by: Lukasz Pawelczyk 
> Reviewed-by: Casey Schaufler 

Acked-by: Casey Schaufler 


> ---
>  security/smack/smack.h|  29 +++-
>  security/smack/smack_access.c | 109 ++--
>  security/smack/smack_lsm.c| 390 
> ++
>  security/smack/smack_ns.c |  39 +
>  security/smack/smackfs.c  |  63 ---
>  5 files changed, 483 insertions(+), 147 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 4b7489f..3d432f4 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -119,6 +119,7 @@ struct superblock_smack {
>   struct smack_known  *smk_floor;
>   struct smack_known  *smk_hat;
>   struct smack_known  *smk_default;
> + struct user_namespace   *smk_ns;
>   int smk_initialized;
>  };
>  
> @@ -126,6 +127,7 @@ struct socket_smack {
>   struct smack_known  *smk_out;   /* outbound label */
>   struct smack_known  *smk_in;/* inbound label */
>   struct smack_known  *smk_packet;/* TCP peer label */
> + struct user_namespace   *smk_ns;/* user namespace */
>  };
>  
>  /*
> @@ -146,6 +148,14 @@ struct task_smack {
>   struct mutexsmk_rules_lock; /* lock for the rules */
>  };
>  
> +/*
> + * Used for IPC objects (sem, shm, etc)
> + */
> +struct ipc_smack {
> + struct smack_known  *smk_known; /* label for access control */
> + struct user_namespace   *smk_ns;/* user namespace */
> +};
> +
>  #define  SMK_INODE_INSTANT   0x01/* inode is instantiated */
>  #define  SMK_INODE_TRANSMUTE 0x02/* directory is transmuting */
>  #define  SMK_INODE_CHANGED   0x04/* smack was transmuted */
> @@ -319,10 +329,11 @@ struct smk_audit_info {
>   */
>  int smk_access_entry(char *, char *, struct list_head *);
>  int smk_access(struct smack_known *, struct smack_known *,
> -int, struct smk_audit_info *);
> +struct user_namespace *, int, struct smk_audit_info *);
>  int smk_tskacc(struct task_struct *, struct smack_known *,
> +struct user_namespace *, u32, struct smk_audit_info *);
> +int smk_curacc(struct smack_known *, struct user_namespace *,
>  u32, struct smk_audit_info *);
> -int smk_curacc(struct smack_known *, u32, struct smk_audit_info *);
>  struct smack_known *smack_from_secid(const u32);
>  char *smk_parse_smack(const char *string, int len, bool *allocated);
>  int smk_netlbl_mls(int, char *, struct netlbl_lsm_secattr *, int);
> @@ -335,8 +346,9 @@ int smack_has_ns_privilege(struct task_struct *task,
>  int smack_has_privilege(struct task_struct *task, int cap);
>  int smack_ns_privileged(struct user_namespace *user_ns, int cap);
>  int smack_privileged(int cap);
> -char *smk_find_label_name(struct smack_known *skp);
> -struct smack_known *smk_get_label(const char *string, int len, bool import);
> +char *smk_find_label_name(struct smack_known *skp, struct user_namespace 
> *ns);
> +struct smack_known *smk_get_label(const char *string, int len, bool import,
> +   struct user_namespace *ns);
>  
>  /*
>   * These functions are in smack_ns.c
> @@ -350,6 +362,15 @@ struct smack_known *smk_find_unmapped(const char 
> *string, int len,
>  extern const struct seq_operations proc_label_map_seq_operations;
>  ssize_t proc_label_map_write(struct task_struct *p, const struct cred 
> *f_cred,
>void *value, size_t size);
> +bool smk_labels_valid(struct smack_known *sbj, struct smack_known *obj,
> +   struct user_namespace *ns);
> +#else
> +static inline bool smk_labels_valid(struct smack_known *sbj,
> + struct smack_known *obj,
> + struct user_namespace *ns)
> +{
> + return true;
> +}
>  #endif /* CONFIG_SECURITY_SMACK_NS */
>  
>  /*
> diff 

Re: [PATCH v4 09/11] smack: namespace groundwork

2015-10-29 Thread Casey Schaufler
On 10/14/2015 5:42 AM, Lukasz Pawelczyk wrote:
> This commit introduces several changes to Smack to prepare it for
> namespace implementation. All the changes are related to namespaces.
>
> Overview of the changes:
> - Adds required data structures for mapped labels and functions to
>   operate on them.
> - Implements the proc interface /proc/$PID/attr/label_map that can be
>   used for remapping of labels for a specific namespace. Also for
>   checking the map.
> - Modifies handling of special built-in labels. Detects them on import
>   and assigns the same char* pointer regardless whether it's used in a
>   normal or a mapped label. This way we can always compare them by ==
>   instead of strcmp().
> - Adds User namespace hooks implementation
>
> This patch introduces both internal and user-space visible APIs to
> handle namespaced labels and Smack namespaces but the behaviour of Smack
> should not be changed. The APIs are there, but they have no impact yet.
>
> Signed-off-by: Lukasz Pawelczyk 
> Reviewed-by: Casey Schaufler 

Acked-by: Casey Schaufler 


> ---
>  security/smack/Kconfig|  10 ++
>  security/smack/Makefile   |   1 +
>  security/smack/smack.h|  45 -
>  security/smack/smack_access.c |  47 -
>  security/smack/smack_lsm.c| 134 +-
>  security/smack/smack_ns.c | 404 
> ++
>  6 files changed, 626 insertions(+), 15 deletions(-)
>  create mode 100644 security/smack/smack_ns.c
>
> diff --git a/security/smack/Kconfig b/security/smack/Kconfig
> index 271adae..b19a7fb 100644
> --- a/security/smack/Kconfig
> +++ b/security/smack/Kconfig
> @@ -40,3 +40,13 @@ config SECURITY_SMACK_NETFILTER
> This enables security marking of network packets using
> Smack labels.
> If you are unsure how to answer this question, answer N.
> +
> +config SECURITY_SMACK_NS
> + bool "Smack namespace"
> + depends on SECURITY_SMACK
> + depends on USER_NS
> + help
> +   This enables Smack namespace that makes it possible to map
> +   specific labels within user namespace (analogously to mapping
> +   UIDs) and to gain MAC capabilities over them.
> +   If you are unsure how to answer this question, answer N.
> diff --git a/security/smack/Makefile b/security/smack/Makefile
> index ee2ebd5..5faebd7 100644
> --- a/security/smack/Makefile
> +++ b/security/smack/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_SECURITY_SMACK) := smack.o
>  
>  smack-y := smack_lsm.o smack_access.o smackfs.o
>  smack-$(CONFIG_SECURITY_SMACK_NETFILTER) += smack_netfilter.o
> +smack-$(CONFIG_SECURITY_SMACK_NS) += smack_ns.o
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 98bb676..4b7489f 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   * Use IPv6 port labeling if IPv6 is enabled and secmarks
> @@ -74,8 +75,36 @@ struct smack_known {
>   struct netlbl_lsm_secattr   smk_netlabel;   /* on wire labels */
>   struct list_headsmk_rules;  /* access rules */
>   struct mutexsmk_rules_lock; /* lock for rules */
> +#ifdef CONFIG_SECURITY_SMACK_NS
> + struct list_headsmk_mapped; /* namespaced labels */
> + struct mutexsmk_mapped_lock;
> +#endif /* CONFIG_SECURITY_SMACK_NS */
>  };
>  
> +#ifdef CONFIG_SECURITY_SMACK_NS
> +
> +/*
> + * User namespace security pointer content.
> + */
> +struct smack_ns {
> + struct list_headsmk_mapped; /* namespaced labels */
> + struct mutexsmk_mapped_lock;
> +};
> +
> +/*
> + * A single entry for a namespaced/mapped label.
> + */
> +struct smack_known_ns {
> + struct list_headsmk_list_known;
> + struct list_headsmk_list_ns;
> + struct user_namespace   *smk_ns;
> + char*smk_mapped;
> + struct smack_known  *smk_unmapped;
> + boolsmk_allocated;
> +};
> +
> +#endif /* CONFIG_SECURITY_SMACK_NS */
> +
>  /*
>   * Maximum number of bytes for the levels in a CIPSO IP option.
>   * Why 23? CIPSO is constrained to 30, so a 32 byte buffer is
> @@ -295,7 +324,7 @@ int smk_tskacc(struct task_struct *, struct smack_known *,
>  u32, struct smk_audit_info *);
>  int smk_curacc(struct smack_known *, u32, struct smk_audit_info *);
>  struct smack_known *smack_from_secid(const u32);
> -char *smk_parse_smack(const char *string, int len);
> +char *smk_parse_smack(const char *string, int len, bool *allocated);
>  int smk_netlbl_mls(int, char *, struct netlbl_lsm_secattr *, int);
>  struct smack_known *smk_import_entry(const char *, int);
>  void smk_insert_entry(struct smack_known *skp);
> @@ -310,6 +339,20 @@ char *smk_find_label_name(struct smack_known *skp);
>  struct 

Re: [PATCH v4 08/11] smack: misc cleanups in preparation for a namespace patch

2015-10-29 Thread Casey Schaufler
On 10/14/2015 5:42 AM, Lukasz Pawelczyk wrote:
> This patch does some small miscellaneous cleanups and additions that
> should not change the code behaviour in any way. Its only purpose is to
> shape the code in a way that the smack namespace patches would be
> smaller and easier to understand.
>
> Changes:
> - four small helper functions added
> - minor code reformatting in several places for readability
> - unnecessarily increasing string size has been fixed
>
> This patch should not change the behaviour of the Smack in any way.
>
> Signed-off-by: Lukasz Pawelczyk 
> Reviewed-by: Casey Schaufler 

Acked-by: Casey Schaufler 


> ---
>  security/smack/smack.h| 47 ++-
>  security/smack/smack_access.c | 18 +-
>  security/smack/smack_lsm.c| 58 
> ---
>  security/smack/smackfs.c  |  4 +--
>  4 files changed, 81 insertions(+), 46 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 091efc2..98bb676 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -291,7 +291,7 @@ struct smk_audit_info {
>  int smk_access_entry(char *, char *, struct list_head *);
>  int smk_access(struct smack_known *, struct smack_known *,
>  int, struct smk_audit_info *);
> -int smk_tskacc(struct task_smack *, struct smack_known *,
> +int smk_tskacc(struct task_struct *, struct smack_known *,
>  u32, struct smk_audit_info *);
>  int smk_curacc(struct smack_known *, u32, struct smk_audit_info *);
>  struct smack_known *smack_from_secid(const u32);
> @@ -348,6 +348,7 @@ extern struct hlist_head 
> smack_known_hash[SMACK_HASH_SLOTS];
>  static inline int smk_inode_transmutable(const struct inode *isp)
>  {
>   struct inode_smack *sip = isp->i_security;
> +
>   return (sip->smk_flags & SMK_INODE_TRANSMUTE) != 0;
>  }
>  
> @@ -357,10 +358,31 @@ static inline int smk_inode_transmutable(const struct 
> inode *isp)
>  static inline struct smack_known *smk_of_inode(const struct inode *isp)
>  {
>   struct inode_smack *sip = isp->i_security;
> +
>   return sip->smk_inode;
>  }
>  
>  /*
> + * Present a pointer to the smack label entry in an inode blob for an exec.
> + */
> +static inline struct smack_known *smk_of_exec(const struct inode *isp)
> +{
> + struct inode_smack *sip = isp->i_security;
> +
> + return sip->smk_task;
> +}
> +
> +/*
> + * Present a pointer to the smack label entry in an inode blob for an mmap.
> + */
> +static inline struct smack_known *smk_of_mmap(const struct inode *isp)
> +{
> + struct inode_smack *sip = isp->i_security;
> +
> + return sip->smk_mmap;
> +}
> +
> +/*
>   * Present a pointer to the smack label entry in an task blob.
>   */
>  static inline struct smack_known *smk_of_task(const struct task_smack *tsp)
> @@ -395,6 +417,29 @@ static inline struct smack_known *smk_of_current(void)
>  }
>  
>  /*
> + * Present a pointer to the user namespace entry in an task blob.
> + */
> +static inline
> +struct user_namespace *ns_of_task_struct(const struct task_struct *t)
> +{
> + struct user_namespace *ns;
> +
> + rcu_read_lock();
> + ns = __task_cred(t)->user_ns;
> + rcu_read_unlock();
> +
> + return ns;
> +}
> +
> +/*
> + * Present a pointer to the user namespace entry in the current task blob.
> + */
> +static inline struct user_namespace *ns_of_current(void)
> +{
> + return current_user_ns();
> +}
> +
> +/*
>   * logging functions
>   */
>  #define SMACK_AUDIT_DENIED 0x1
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index 131c742..750aa9c 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -167,6 +167,7 @@ int smk_access(struct smack_known *subject, struct 
> smack_known *object,
>   if (subject == _known_hat)
>   goto out_audit;
>   }
> +
>   /*
>* Beyond here an explicit relationship is required.
>* If the requested access is contained in the available
> @@ -183,6 +184,7 @@ int smk_access(struct smack_known *subject, struct 
> smack_known *object,
>   rc = -EACCES;
>   goto out_audit;
>   }
> +
>  #ifdef CONFIG_SECURITY_SMACK_BRINGUP
>   /*
>* Return a positive value if using bringup mode.
> @@ -225,10 +227,10 @@ out_audit:
>   * non zero otherwise. It allows that the task may have the capability
>   * to override the rules.
>   */
> -int smk_tskacc(struct task_smack *tsp, struct smack_known *obj_known,
> +int smk_tskacc(struct task_struct *task, struct smack_known *obj_known,
>  u32 mode, struct smk_audit_info *a)
>  {
> - struct smack_known *sbj_known = smk_of_task(tsp);
> + struct smack_known *sbj_known = smk_of_task_struct(task);
>   int may;
>   int rc;
>  
> @@ -237,13 +239,19 @@ int smk_tskacc(struct task_smack *tsp, 

Re: [PATCH v4 01/11] user_ns: 3 new LSM hooks for user namespace operations

2015-10-29 Thread Casey Schaufler
On 10/14/2015 5:41 AM, Lukasz Pawelczyk wrote:
> This commit implements 3 new LSM hooks that provide the means for LSMs
> to embed their own security context within user namespace, effectively
> creating some sort of a user_ns related security namespace.
>
> The first one to take advantage of this mechanism is Smack.
>
> The hooks has been documented in the in the security.h below.
>
> Signed-off-by: Lukasz Pawelczyk 
> Reviewed-by: Casey Schaufler 
> Acked-by: Paul Moore 

Acked-by: Casey Schaufler 

> ---
>  include/linux/lsm_hooks.h  | 28 
>  include/linux/security.h   | 23 +++
>  include/linux/user_namespace.h |  4 
>  kernel/user.c  |  3 +++
>  kernel/user_namespace.c| 18 ++
>  security/security.c| 28 
>  6 files changed, 104 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index ec3a6ba..18c9160 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1261,6 +1261,23 @@
>   *   audit_rule_init.
>   *   @rule contains the allocated rule
>   *
> + * @userns_create:
> + *   Allocates and fills the security part of a new user namespace.
> + *   @ns points to a newly created user namespace.
> + *   Returns 0 or an error code.
> + *
> + * @userns_free:
> + *   Deallocates the security part of a user namespace.
> + *   @ns points to a user namespace about to be destroyed.
> + *
> + * @userns_setns:
> + *   Run during a setns syscall to add a process to an already existing
> + *   user namespace. Returning failure here will block the operation
> + *   requested from userspace (setns() with CLONE_NEWUSER).
> + *   @nsproxy contains nsproxy to which the user namespace will be assigned.
> + *   @ns contains user namespace that is to be incorporated to the nsproxy.
> + *   Returns 0 or an error code.
> + *
>   * @inode_notifysecctx:
>   *   Notify the security module of what the security context of an inode
>   *   should be.  Initializes the incore security context managed by the
> @@ -1613,6 +1630,12 @@ union security_list_options {
>   struct audit_context *actx);
>   void (*audit_rule_free)(void *lsmrule);
>  #endif /* CONFIG_AUDIT */
> +
> +#ifdef CONFIG_USER_NS
> + int (*userns_create)(struct user_namespace *ns);
> + void (*userns_free)(struct user_namespace *ns);
> + int (*userns_setns)(struct nsproxy *nsproxy, struct user_namespace *ns);
> +#endif /* CONFIG_USER_NS */
>  };
>  
>  struct security_hook_heads {
> @@ -1824,6 +1847,11 @@ struct security_hook_heads {
>   struct list_head audit_rule_match;
>   struct list_head audit_rule_free;
>  #endif /* CONFIG_AUDIT */
> +#ifdef CONFIG_USER_NS
> + struct list_head userns_create;
> + struct list_head userns_free;
> + struct list_head userns_setns;
> +#endif /* CONFIG_USER_NS */
>  };
>  
>  /*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 2f4c1f7..91ffba2 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1584,6 +1584,29 @@ static inline void security_audit_rule_free(void 
> *lsmrule)
>  #endif /* CONFIG_SECURITY */
>  #endif /* CONFIG_AUDIT */
>  
> +#ifdef CONFIG_USER_NS
> +int security_userns_create(struct user_namespace *ns);
> +void security_userns_free(struct user_namespace *ns);
> +int security_userns_setns(struct nsproxy *nsproxy, struct user_namespace 
> *ns);
> +
> +#else
> +
> +static inline int security_userns_create(struct user_namespace *ns)
> +{
> + return 0;
> +}
> +
> +static inline void security_userns_free(struct user_namespace *ns)
> +{ }
> +
> +static inline int security_userns_setns(struct nsproxy *nsproxy,
> + struct user_namespace *ns)
> +{
> + return 0;
> +}
> +
> +#endif /* CONFIG_USER_NS */
> +
>  #ifdef CONFIG_SECURITYFS
>  
>  extern struct dentry *securityfs_create_file(const char *name, umode_t mode,
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 8297e5b..a9400cc 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -39,6 +39,10 @@ struct user_namespace {
>   struct key  *persistent_keyring_register;
>   struct rw_semaphore persistent_keyring_register_sem;
>  #endif
> +
> +#ifdef CONFIG_SECURITY
> + void *security;
> +#endif
>  };
>  
>  extern struct user_namespace init_user_ns;
> diff --git a/kernel/user.c b/kernel/user.c
> index b069ccb..ce5419e 100644
> --- a/kernel/user.c
> +++ b/kernel/user.c
> @@ -59,6 +59,9 @@ struct user_namespace init_user_ns = {
>   .persistent_keyring_register_sem =
>   __RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
>  #endif
> +#ifdef CONFIG_SECURITY
> + .security = NULL,
> +#endif
>  };
>  

Re: [PATCH v4 06/11] smack: don't use implicit star to display smackfs/syslog

2015-10-29 Thread Casey Schaufler
On 10/14/2015 5:42 AM, Lukasz Pawelczyk wrote:
> Smackfs/syslog is analogous to onlycap and unconfined. When not filled
> they don't do anything. In such cases onlycap and unconfined displayed
> nothing when read, but syslog unconditionally displayed star. This
> doesn't work well with namespaces where the star could have been
> unmapped. Besides the meaning of this star was different then a star
> that could be written to this file. This was misleading.
>
> This also brings syslog read/write functions on par with onlycap and
> unconfined where it is possible to reset the value to NULL as should be
> possible according to comment in smackfs.c describing smack_syslog_label
> variable.
>
> Before that the initial state was to allow (smack_syslog_label was
> NULL), but after writing star to it the current had to be labeled star
> as well to have an access, even thought reading the smackfs/syslog
> returned the same result in both cases.
>
> Signed-off-by: Lukasz Pawelczyk 
> Acked-by: Serge Hallyn 

Acked-by: Casey Schaufler 


> ---
>  security/smack/smackfs.c | 42 +++---
>  1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index ce8d503..05e09ee2 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -2634,23 +2634,20 @@ static const struct file_operations 
> smk_change_rule_ops = {
>  static ssize_t smk_read_syslog(struct file *filp, char __user *buf,
>   size_t cn, loff_t *ppos)
>  {
> - struct smack_known *skp;
> + char *smack = "";
>   ssize_t rc = -EINVAL;
>   int asize;
>  
>   if (*ppos != 0)
>   return 0;
>  
> - if (smack_syslog_label == NULL)
> - skp = _known_star;
> - else
> - skp = smack_syslog_label;
> + if (smack_syslog_label != NULL)
> + smack = smack_syslog_label->smk_known;
>  
> - asize = strlen(skp->smk_known) + 1;
> + asize = strlen(smack) + 1;
>  
>   if (cn >= asize)
> - rc = simple_read_from_buffer(buf, cn, ppos, skp->smk_known,
> - asize);
> + rc = simple_read_from_buffer(buf, cn, ppos, smack, asize);
>  
>   return rc;
>  }
> @@ -2678,16 +2675,31 @@ static ssize_t smk_write_syslog(struct file *file, 
> const char __user *buf,
>   if (data == NULL)
>   return -ENOMEM;
>  
> - if (copy_from_user(data, buf, count) != 0)
> + if (copy_from_user(data, buf, count) != 0) {
>   rc = -EFAULT;
> - else {
> - skp = smk_import_entry(data, count);
> - if (IS_ERR(skp))
> - rc = PTR_ERR(skp);
> - else
> - smack_syslog_label = skp;
> + goto freeout;
>   }
>  
> + /*
> +  * Clear the smack_syslog_label on invalid label errors. This means
> +  * that we can pass a null string to unset the syslog value.
> +  *
> +  * Importing will also reject a label beginning with '-',
> +  * so "-syslog" will also work.
> +  *
> +  * But do so only on invalid label, not on system errors.
> +  */
> + skp = smk_import_entry(data, count);
> + if (PTR_ERR(skp) == -EINVAL)
> + skp = NULL;
> + else if (IS_ERR(skp)) {
> + rc = PTR_ERR(skp);
> + goto freeout;
> + }
> +
> + smack_syslog_label = skp;
> +
> +freeout:
>   kfree(data);
>   return rc;
>  }

--
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 03/11] lsm: add file opener's cred to a setprocattr arguments

2015-10-29 Thread Casey Schaufler
On 10/14/2015 5:41 AM, Lukasz Pawelczyk wrote:
> setprocattr hook for Smack's label_map attribute needs to know the
> capabilities of file opener. Add those credentials to the hook's
> arguments.
>
> While at it add documentation on get/setprocattr hooks.
>
> Signed-off-by: Lukasz Pawelczyk 
> Acked-by: Serge Hallyn 

Acked-by: Casey Schaufler 


> ---
>  fs/proc/base.c |  2 +-
>  include/linux/lsm_hooks.h  | 18 --
>  include/linux/security.h   |  7 +--
>  security/apparmor/lsm.c|  5 +++--
>  security/security.c|  6 --
>  security/selinux/hooks.c   |  2 +-
>  security/smack/smack_lsm.c |  4 ++--
>  7 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 9ec88b8..2b38969 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2447,7 +2447,7 @@ static ssize_t proc_pid_attr_write(struct file * file, 
> const char __user * buf,
>   if (length < 0)
>   goto out_free;
>  
> - length = security_setprocattr(task,
> + length = security_setprocattr(task, file->f_cred,
> (char*)file->f_path.dentry->d_name.name,
> (void*)page, count);
>   mutex_unlock(>signal->cred_guard_mutex);
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 7049db0..4f16640 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1220,6 +1220,20 @@
>   *   Return 0 if @name is to be handled by seq, EOPNOTSUPP if getprocattr()
>   *   should be used. Other errors will be passed to user-space.
>   *
> + * @getprocattr:
> + *   Get a value of a proc security attribute in /proc/$PID/attr/.
> + *   @p a task associated with the proc file.
> + *   @name a name of the file in question.
> + *   @value a pointer where to return the attribute's value.
> + *
> + * @setprocattr:
> + *   Set a value of a proc security attribute in /proc/$PID/attr/.
> + *   @p a task associated with the proc file.
> + *   @f_cred credentials of a file's opener.
> + *   @name a name of the file in question.
> + *   @value a pointer where a value to set is kept.
> + *   @size a number of bytes to read from the @value pointer.
> + *
>   * @secid_to_secctx:
>   *   Convert secid to security context.  If secdata is NULL the length of
>   *   the result will be returned in seclen, but no secdata will be returned.
> @@ -1540,8 +1554,8 @@ union security_list_options {
>   int (*getprocattr_seq)(struct task_struct *p, const char *name,
>  const struct seq_operations **ops);
>   int (*getprocattr)(struct task_struct *p, char *name, char **value);
> - int (*setprocattr)(struct task_struct *p, char *name, void *value,
> - size_t size);
> + int (*setprocattr)(struct task_struct *p, const struct cred *f_cred,
> +char *name, void *value, size_t size);
>   int (*ismaclabel)(const char *name);
>   int (*secid_to_secctx)(u32 secid, char **secdata, u32 *seclen);
>   int (*secctx_to_secid)(const char *secdata, u32 seclen, u32 *secid);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index dddea2f..12bd011 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -348,7 +348,8 @@ void security_d_instantiate(struct dentry *dentry, struct 
> inode *inode);
>  int security_getprocattr_seq(struct task_struct *p, const char *name,
>const struct seq_operations **ops);
>  int security_getprocattr(struct task_struct *p, char *name, char **value);
> -int security_setprocattr(struct task_struct *p, char *name, void *value, 
> size_t size);
> +int security_setprocattr(struct task_struct *p, const struct cred *f_cred,
> +  char *name, void *value, size_t size);
>  int security_netlink_send(struct sock *sk, struct sk_buff *skb);
>  int security_ismaclabel(const char *name);
>  int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
> @@ -1071,7 +1072,9 @@ static inline int security_getprocattr(struct 
> task_struct *p, char *name, char *
>   return -EINVAL;
>  }
>  
> -static inline int security_setprocattr(struct task_struct *p, char *name, 
> void *value, size_t size)
> +static inline int security_setprocattr(struct task_struct *p,
> +const struct cred *f_cred,
> +char *name, void *value, size_t size)
>  {
>   return -EINVAL;
>  }
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index dec607c..1212927 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -518,8 +518,9 @@ static int apparmor_getprocattr(struct task_struct *task, 
> char *name,
>   return error;
>  }
>  
> -static int apparmor_setprocattr(struct task_struct *task, char *name,
> - 

Re: [PATCH v4 04/11] lsm: inode_pre_setxattr hook

2015-10-29 Thread Casey Schaufler
On 10/14/2015 5:41 AM, Lukasz Pawelczyk wrote:
> Add a new LSM hook called before inode's setxattr. It is required for
> LSM to be able to reliably replace the xattr's value to be set to
> filesystem in __vfs_setxattr_noperm(). Useful for mapped values, like in
> the upcoming Smack namespace patches.
>
> Signed-off-by: Lukasz Pawelczyk 
> Acked-by: Serge Hallyn 

Acked-by: Casey Schaufler 


> ---
>  fs/xattr.c| 10 ++
>  include/linux/lsm_hooks.h |  9 +
>  include/linux/security.h  | 10 ++
>  security/security.c   | 12 
>  4 files changed, 41 insertions(+)
>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 072fee1..cbc8d19 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -100,12 +100,22 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const 
> char *name,
>   if (issec)
>   inode->i_flags &= ~S_NOSEC;
>   if (inode->i_op->setxattr) {
> + bool alloc = false;
> +
> + error = security_inode_pre_setxattr(dentry, name, ,
> + , flags, );
> + if (error)
> + return error;
> +
>   error = inode->i_op->setxattr(dentry, name, value, size, flags);
>   if (!error) {
>   fsnotify_xattr(dentry);
>   security_inode_post_setxattr(dentry, name, value,
>size, flags);
>   }
> +
> + if (alloc)
> + kfree(value);
>   } else if (issec) {
>   const char *suffix = name + XATTR_SECURITY_PREFIX_LEN;
>   error = security_inode_setsecurity(inode, suffix, value,
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 4f16640..85bfdde 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -349,6 +349,11 @@
>   *   Check permission before setting the extended attributes
>   *   @value identified by @name for @dentry.
>   *   Return 0 if permission is granted.
> + * @inode_pre_setxattr:
> + *   Be able to do some operation before setting the @value identified
> + *   by @name on the filesystem. Replacing the @value and its @size is
> + *   possible. Useful for mapped values. Set @alloc to true if @value
> + *   needs to be kfreed afterwards.
>   * @inode_post_setxattr:
>   *   Update inode security field after successful setxattr operation.
>   *   @value identified by @name for @dentry.
> @@ -1448,6 +1453,9 @@ union security_list_options {
>   int (*inode_getattr)(const struct path *path);
>   int (*inode_setxattr)(struct dentry *dentry, const char *name,
>   const void *value, size_t size, int flags);
> + int (*inode_pre_setxattr)(struct dentry *dentry, const char *name,
> +   const void **value, size_t *size,
> +   int flags, bool *alloc);
>   void (*inode_post_setxattr)(struct dentry *dentry, const char *name,
>   const void *value, size_t size,
>   int flags);
> @@ -1730,6 +1738,7 @@ struct security_hook_heads {
>   struct list_head inode_setattr;
>   struct list_head inode_getattr;
>   struct list_head inode_setxattr;
> + struct list_head inode_pre_setxattr;
>   struct list_head inode_post_setxattr;
>   struct list_head inode_getxattr;
>   struct list_head inode_listxattr;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 12bd011..4de4865 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -263,6 +263,9 @@ int security_inode_setattr(struct dentry *dentry, struct 
> iattr *attr);
>  int security_inode_getattr(const struct path *path);
>  int security_inode_setxattr(struct dentry *dentry, const char *name,
>   const void *value, size_t size, int flags);
> +int security_inode_pre_setxattr(struct dentry *dentry, const char *name,
> + const void **value, size_t *size, int flags,
> + bool *alloc);
>  void security_inode_post_setxattr(struct dentry *dentry, const char *name,
> const void *value, size_t size, int flags);
>  int security_inode_getxattr(struct dentry *dentry, const char *name);
> @@ -691,6 +694,13 @@ static inline int security_inode_setxattr(struct dentry 
> *dentry,
>   return cap_inode_setxattr(dentry, name, value, size, flags);
>  }
>  
> +static inline int security_inode_pre_setxattr(struct dentry *dentry,
> + const char *name, const void **value,
> + size_t *size, int flags, bool *alloc)
> +{
> + return 0;
> +}
> +
>  static inline void security_inode_post_setxattr(struct dentry *dentry,
>   const char 

[PATCH v3] selinux: export validatetrans decisions

2015-10-29 Thread Andrew Perepechko
Make validatetrans decisions available through selinuxfs.
"/validatetrans" is added to selinuxfs for this purpose.
This functionality is needed by file system servers
implemented in userspace or kernelspace without the VFS
layer.

Writing "$oldcontext $newcontext $tclass $taskcontext"
to /validatetrans is expected to return 0 if the transition
is allowed and -EPERM otherwise.

Signed-off-by: Andrew Perepechko 
CC: andrew.perepec...@seagate.com
---
 security/selinux/include/classmap.h |  2 +-
 security/selinux/include/security.h |  3 ++
 security/selinux/selinuxfs.c| 80 +
 security/selinux/ss/services.c  | 34 
 4 files changed, 111 insertions(+), 8 deletions(-)

diff --git a/security/selinux/include/classmap.h 
b/security/selinux/include/classmap.h
index 5a4eef5..ef83c4b 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -21,7 +21,7 @@ struct security_class_mapping secclass_map[] = {
  { "compute_av", "compute_create", "compute_member",
"check_context", "load_policy", "compute_relabel",
"compute_user", "setenforce", "setbool", "setsecparam",
-   "setcheckreqprot", "read_policy", NULL } },
+   "setcheckreqprot", "read_policy", "validate_trans", NULL } },
{ "process",
  { "fork", "transition", "sigchld", "sigkill",
"sigstop", "signull", "signal", "ptrace", "getsched", "setsched",
diff --git a/security/selinux/include/security.h 
b/security/selinux/include/security.h
index 223e9fd..38feb55 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -187,6 +187,9 @@ int security_node_sid(u16 domain, void *addr, u32 addrlen,
 int security_validate_transition(u32 oldsid, u32 newsid, u32 tasksid,
 u16 tclass);
 
+int security_validate_transition_user(u32 oldsid, u32 newsid, u32 tasksid,
+ u16 tclass);
+
 int security_bounded_transition(u32 oldsid, u32 newsid);
 
 int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid);
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index c02da25..0dc407d 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -116,6 +116,7 @@ enum sel_inos {
SEL_DENY_UNKNOWN, /* export unknown deny handling to userspace */
SEL_STATUS, /* export current status using mmap() */
SEL_POLICY, /* allow userspace to read the in kernel policy */
+   SEL_VALIDATE_TRANS, /* compute validatetrans decision */
SEL_INO_NEXT,   /* The next inode number to use */
 };
 
@@ -653,6 +654,83 @@ static const struct file_operations sel_checkreqprot_ops = 
{
.llseek = generic_file_llseek,
 };
 
+static ssize_t sel_write_validatetrans(struct file *file,
+   const char __user *buf,
+   size_t count, loff_t *ppos)
+{
+   char *oldcon = NULL, *newcon = NULL, *taskcon = NULL;
+   char *req = NULL;
+   u32 osid, nsid, tsid;
+   u16 tclass;
+   int rc;
+
+   rc = task_has_security(current, SECURITY__VALIDATE_TRANS);
+   if (rc)
+   goto out;
+
+   rc = -ENOMEM;
+   if (count >= PAGE_SIZE)
+   goto out;
+
+   /* No partial writes. */
+   rc = -EINVAL;
+   if (*ppos != 0)
+   goto out;
+
+   rc = -ENOMEM;
+   req = kzalloc(count + 1, GFP_KERNEL);
+   if (!req)
+   goto out;
+
+   rc = -EFAULT;
+   if (copy_from_user(req, buf, count))
+   goto out;
+
+   rc = -ENOMEM;
+   oldcon = kzalloc(count + 1, GFP_KERNEL);
+   if (!oldcon)
+   goto out;
+
+   newcon = kzalloc(count + 1, GFP_KERNEL);
+   if (!newcon)
+   goto out;
+
+   taskcon = kzalloc(count + 1, GFP_KERNEL);
+   if (!taskcon)
+   goto out;
+
+   rc = -EINVAL;
+   if (sscanf(req, "%s %s %hu %s", oldcon, newcon, , taskcon) != 4)
+   goto out;
+
+   rc = security_context_str_to_sid(oldcon, , GFP_KERNEL);
+   if (rc)
+   goto out;
+
+   rc = security_context_str_to_sid(newcon, , GFP_KERNEL);
+   if (rc)
+   goto out;
+
+   rc = security_context_str_to_sid(taskcon, , GFP_KERNEL);
+   if (rc)
+   goto out;
+
+   rc = security_validate_transition_user(osid, nsid, tsid, tclass);
+   if (!rc)
+   rc = count;
+out:
+   kfree(req);
+   kfree(oldcon);
+   kfree(newcon);
+   kfree(taskcon);
+   return rc;
+}
+
+static const struct file_operations sel_transition_ops = {
+   .write  = sel_write_validatetrans,
+   .llseek = generic_file_llseek,
+};
+
 /*
  * Remaining nodes use transaction based IO methods like nfsd/nfsctl.c
  */
@@ -1759,6 +1837,8 @@ static int