Re: [PATCH] libselinux: Replace selabel_digest hash function

2015-10-21 Thread Stephen Smalley

On 10/21/2015 11:35 AM, Richard Haines wrote:

This replaces the openssl library with SHA1 hash functions
extracted from [1] as this is a public domain implementation.

util/selabel_digest -v option still compares the result with
the openssl command "openssl dgst -sha1 -hex .." for validation.

[1] https://github.com/WaterJuice/CryptLib

Signed-off-by: Richard Haines 


Need to hide the symbols defined by sha1.c or they'll be exported by 
libselinux.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH v3 1/7] selinux: Remove unused variable in selinux_inode_init_security

2015-10-27 Thread Stephen Smalley

On 10/26/2015 05:15 PM, Andreas Gruenbacher wrote:

Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>


Acked-by:  Stephen Smalley <s...@tycho.nsa.gov>


---
  security/selinux/hooks.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e4369d8..fc8f626 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2756,13 +2756,11 @@ static int selinux_inode_init_security(struct inode 
*inode, struct inode *dir,
   void **value, size_t *len)
  {
const struct task_security_struct *tsec = current_security();
-   struct inode_security_struct *dsec;
struct superblock_security_struct *sbsec;
u32 sid, newsid, clen;
int rc;
char *context;

-   dsec = dir->i_security;
sbsec = dir->i_sb->s_security;

sid = tsec->sid;



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH v3 0/7] Inode security label invalidation

2015-10-27 Thread Stephen Smalley

On 10/26/2015 05:15 PM, Andreas Gruenbacher wrote:

Here is another version of the patch queue to make gfs2 and similar file
systems work with SELinux.  As suggested by Stephen Smalley [*], the relevant
uses of inode->security are wrapped in function calls that try to revalidate
invalid labels.

   [*] http://marc.info/?l=linux-kernel=144416710207686=2

The patches are looking good from my point of view; is there anything else that
needs addressing?

Does SELinux have test suites that these patches could be tested agains?


git clone https://github.com/SELinuxProject/selinux-testsuite
sudo yum install perl-Test perl-Test-Harness selinux-policy-devel gcc 
libselinux-devel net-tools netlabel_tools iptables

cd selinux-testsuite
sudo make test



Thanks,
Andreas

Andreas Gruenbacher (7):
   selinux: Remove unused variable in selinux_inode_init_security
   selinux: Add accessor functions for inode->i_security
   selinux: Get rid of file_path_has_perm
   selinux: Push dentry down from {dentry,path,file}_has_perm
   security: Add hook to invalidate inode security labels
   selinux: Revalidate invalid inode security labels
   gfs2: Invalide security labels of inodes when they go invalid

  fs/gfs2/glops.c   |   2 +
  include/linux/lsm_hooks.h |   6 ++
  include/linux/security.h  |   5 +
  security/security.c   |   8 ++
  security/selinux/hooks.c  | 213 ++
  security/selinux/include/objsec.h |   6 ++
  6 files changed, 152 insertions(+), 88 deletions(-)



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] Load libsepol.so.1 instead of libsepol.so

2015-10-28 Thread Stephen Smalley

On 10/27/2015 06:41 PM, Laurent Bigonville wrote:

From: Laurent Bigonville 

libsepol.so symlink is usually part of the development package, try to
load the library directly instead.


Thanks, applied.  Next time, please remember to add your Signed-off-by 
and include a prefix in the subject line indicating the component 
affected (e.g. [PATCH] libsemanage: semanage_migrate_store: Load 
libsepol.so.1 instead of libsepol.so).



---
  libsemanage/utils/semanage_migrate_store | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libsemanage/utils/semanage_migrate_store 
b/libsemanage/utils/semanage_migrate_store
index b5eefaa..0ebd285 100755
--- a/libsemanage/utils/semanage_migrate_store
+++ b/libsemanage/utils/semanage_migrate_store
@@ -10,7 +10,7 @@ from optparse import OptionParser

  import ctypes

-sepol = ctypes.cdll.LoadLibrary('libsepol.so')
+sepol = ctypes.cdll.LoadLibrary('libsepol.so.1')

  try:
import selinux



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH v3] libselinux: label_file: fix memory leaks and uninitialized jump

2015-10-28 Thread Stephen Smalley

On 10/27/2015 05:50 PM, william.c.robe...@intel.com wrote:

From: William Roberts 

Some error's were reported by valgrind (below) fix them. The test
cases on which these leaks were detected:

1. properly formed file_contexts file.
2. malformed file_contexts file, unknown type.
3. malformed file_contexts file, type that fails on validate callback.
4. malformed file_contexts file, invalid regex.
5. malformed file_contexts file, invalid mode.

==3819== Conditional jump or move depends on uninitialised value(s)
==3819==at 0x12A682: closef (label_file.c:577)
==3819==by 0x12A196: selabel_close (label.c:163)
==3819==by 0x10A2FD: cleanup (checkfc.c:218)
==3819==by 0x5089258: __run_exit_handlers (exit.c:82)
==3819==by 0x50892A4: exit (exit.c:104)
==3819==by 0x10A231: main (checkfc.c:361)
==3819==  Uninitialised value was created by a heap allocation
==3819==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3819==by 0x4C2CF1F: realloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3819==by 0x12BB31: process_file (label_file.h:273)
==3819==by 0x12A2BA: selabel_file_init (label_file.c:522)
==3819==by 0x12A0BB: selabel_open (label.c:88)
==3819==by 0x10A038: main (checkfc.c:292)
==3819==
==3819==
==3819== HEAP SUMMARY:
==3819== in use at exit: 729 bytes in 19 blocks
==3819==   total heap usage: 21,126 allocs, 21,107 frees, 923,854 bytes 
allocated
==3819==
==3819== 81 bytes in 1 blocks are definitely lost in loss record 1 of 2
==3819==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3819==by 0x50D5839: strdup (strdup.c:42)
==3819==by 0x12A2A6: selabel_file_init (label_file.c:517)
==3819==by 0x12A0BB: selabel_open (label.c:88)
==3819==by 0x10A038: main (checkfc.c:292)
==3819==

==4238== 40 bytes in 1 blocks are definitely lost in loss record 1 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x12A1D2: selabel_file_init (label_file.c:886)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 81 bytes in 1 blocks are definitely lost in loss record 2 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x50D5839: strdup (strdup.c:42)
==4238==by 0x12A2A6: selabel_file_init (label_file.c:517)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 386 bytes in 24 blocks are definitely lost in loss record 3 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x50D5889: strndup (strndup.c:45)
==4238==by 0x12CDDF: read_spec_entries (label_support.c:37)
==4238==by 0x12B72D: process_file (label_file.h:392)
==4238==by 0x12A2BA: selabel_file_init (label_file.c:522)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 648 bytes in 18 blocks are definitely lost in loss record 4 of 6
==4238==at 0x4C2CC70: calloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x117C9B: avtab_insert_node (avtab.c:105)
==4238==by 0x117C10: avtab_insert (avtab.c:163)
==4238==by 0x11880A: avtab_read_item (avtab.c:566)
==4238==by 0x118BD3: avtab_read (avtab.c:600)
==4238==by 0x125BDD: policydb_read (policydb.c:3854)
==4238==by 0x109F87: main (checkfc.c:273)
==4238==
==4238== 1,095 bytes in 12 blocks are definitely lost in loss record 5 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x12D8D1: pcre_compile2 (pcre_compile.c:9217)
==4238==by 0x12B239: compile_regex (label_file.h:357)
==4238==by 0x12B9C7: process_file (label_file.h:429)
==4238==by 0x12A2BA: selabel_file_init (label_file.c:522)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 1,296 bytes in 12 blocks are definitely lost in loss record 6 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x13EBE5: pcre_study (pcre_study.c:1565)
==4238==by 0x12B25D: compile_regex (label_file.h:366)
==4238==by 0x12B9C7: process_file (label_file.h:429)
==4238==by 0x12A2BA: selabel_file_init (label_file.c:522)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)

Signed-off-by: William Roberts 


Thanks, applied.


---
  libselinux/src/label.c  |  1 +
  libselinux/src/label_file.c |  5 -
  libselinux/src/label_file.h | 20 +---
  3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/libselinux/src/label.c b/libselinux/src/label.c
index c656fda..963bfcb 100644
--- a/libselinux/src/label.c
+++ 

Re: [PATCH v3] selinux: export validatetrans decisions

2015-10-29 Thread Stephen Smalley

On 10/29/2015 07:01 AM, Andrew Perepechko wrote:

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 <anser...@ya.ru>


Acked-by:  Stephen Smalley <s...@tycho.nsa.gov>


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

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 <agrue...@redhat.com>


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

Acked-by: Stephen Smalley <s...@tycho.nsa.gov>


---
  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);

   

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 <agrue...@redhat.com>


Acked-by: Stephen Smalley <s...@tycho.nsa.gov>


---
  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,
-  

Re: [PATCH v4 2/7] security: Make inode argument of inode_getsecurity 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_getsecurity hook non-const so that
we can use it to revalidate invalid security labels.

Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>


Acked-by:  Stephen Smalley <s...@tycho.nsa.gov>


---
  include/linux/lsm_hooks.h  | 2 +-
  include/linux/security.h   | 4 ++--
  security/security.c| 2 +-
  security/selinux/hooks.c   | 2 +-
  security/smack/smack_lsm.c | 2 +-
  5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index ec3a6ba..bdd0a3a 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1413,7 +1413,7 @@ union security_list_options {
int (*inode_removexattr)(struct dentry *dentry, const char *name);
int (*inode_need_killpriv)(struct dentry *dentry);
int (*inode_killpriv)(struct dentry *dentry);
-   int (*inode_getsecurity)(const struct inode *inode, const char *name,
+   int (*inode_getsecurity)(struct inode *inode, const char *name,
void **buffer, bool alloc);
int (*inode_setsecurity)(struct inode *inode, const char *name,
const void *value, size_t size,
diff --git a/include/linux/security.h b/include/linux/security.h
index 2f4c1f7..9ee61b2 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -270,7 +270,7 @@ int security_inode_listxattr(struct dentry *dentry);
  int security_inode_removexattr(struct dentry *dentry, const char *name);
  int security_inode_need_killpriv(struct dentry *dentry);
  int security_inode_killpriv(struct dentry *dentry);
-int security_inode_getsecurity(const struct inode *inode, const char *name, 
void **buffer, bool alloc);
+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);
@@ -719,7 +719,7 @@ static inline int security_inode_killpriv(struct dentry 
*dentry)
return cap_inode_killpriv(dentry);
  }

-static inline int security_inode_getsecurity(const struct inode *inode, const 
char *name, void **buffer, bool alloc)
+static inline int security_inode_getsecurity(struct inode *inode, const char 
*name, void **buffer, bool alloc)
  {
return -EOPNOTSUPP;
  }
diff --git a/security/security.c b/security/security.c
index 46f405c..73514c9 100644
--- a/security/security.c
+++ b/security/security.c
@@ -697,7 +697,7 @@ int security_inode_killpriv(struct dentry *dentry)
return call_int_hook(inode_killpriv, 0, dentry);
  }

-int security_inode_getsecurity(const struct inode *inode, const char *name, 
void **buffer, bool alloc)
+int security_inode_getsecurity(struct inode *inode, const char *name, void 
**buffer, bool alloc)
  {
if (unlikely(IS_PRIVATE(inode)))
return -EOPNOTSUPP;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index fc8f626..adec2e2 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3110,7 +3110,7 @@ static int selinux_inode_removexattr(struct dentry 
*dentry, const char *name)
   *
   * Permission check is handled by selinux_inode_getxattr hook.
   */
-static int selinux_inode_getsecurity(const struct inode *inode, const char 
*name, void **buffer, bool alloc)
+static int selinux_inode_getsecurity(struct inode *inode, const char *name, 
void **buffer, bool alloc)
  {
u32 size;
int error;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 996c889..07d0344 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1435,7 +1435,7 @@ static int smack_inode_removexattr(struct dentry *dentry, 
const char *name)
   *
   * Returns the size of the attribute or an error code
   */
-static int smack_inode_getsecurity(const struct inode *inode,
+static int smack_inode_getsecurity(struct inode *inode,
   const char *name, void **buffer,
   bool alloc)
  {



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


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 <agrue...@redhat.com>
Reviewed-by: James Morris <james.l.mor...@oracle.com>


Acked-by: Stephen Smalley <s...@tycho.nsa.gov>


---
  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->ini

Re: [PATCH] fix memory leaks and uninitialized jump

2015-10-27 Thread Stephen Smalley

On 10/26/2015 02:42 PM, Roberts, William C wrote:

Shouldn't;
   compat_validate(rec, _arr[nspec].lr, path, lineno);

in process_line() cause a failure? Right now the return code is being ignored.


I think it is historical.  Originally we had it bail on error.  Red Hat 
had problems with that because they would ship a file_contexts file that 
had an invalid context or the user would have one bad entry in 
file_contexts.local or a local module (maybe due to a policy change that 
removed a type, so it might have been valid originally and then became 
invalid), and suddenly restorecon/setfiles/rpm/... couldn't label 
anything at all, leading to unlabeled files and possibly an unbootable 
system.  So then we took the error handling to the validate callback, 
where setfiles or sefcontext_compile can make it a fatal error but you 
can still label most files (the ones that match valid entries) when they 
get installed via rpm or using restorecon.




Bill


-Original Message-
From: Roberts, William C
Sent: Monday, October 26, 2015 11:33 AM
To: selinux@tycho.nsa.gov
Cc: s...@tycho.nsa.gov; Roberts, William C
Subject: [PATCH] fix memory leaks and uninitialized jump

From: William Roberts 

 Some error's were reported by valgrind (below) fix them. The test
 cases on which these leaks were detected:

 1. properly formed file_contexts file.
 2. malformed file_contexts file, unknown type.
 3. malformed file_contexts file, type that fails on validate callback.
 4. malformed file_contexts file, invalid regex.
 5. malformed file_contexts file, invalid mode.

 ==3819== Conditional jump or move depends on uninitialised value(s)
 ==3819==at 0x12A682: closef (label_file.c:577)
 ==3819==by 0x12A196: selabel_close (label.c:163)
 ==3819==by 0x10A2FD: cleanup (checkfc.c:218)
 ==3819==by 0x5089258: __run_exit_handlers (exit.c:82)
 ==3819==by 0x50892A4: exit (exit.c:104)
 ==3819==by 0x10A231: main (checkfc.c:361)
 ==3819==  Uninitialised value was created by a heap allocation
 ==3819==at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
amd64-linux.so)
 ==3819==by 0x4C2CF1F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-
amd64-linux.so)
 ==3819==by 0x12BB31: process_file (label_file.h:273)
 ==3819==by 0x12A2BA: selabel_file_init (label_file.c:522)
 ==3819==by 0x12A0BB: selabel_open (label.c:88)
 ==3819==by 0x10A038: main (checkfc.c:292)
 ==3819==
 ==3819==
 ==3819== HEAP SUMMARY:
 ==3819== in use at exit: 729 bytes in 19 blocks
 ==3819==   total heap usage: 21,126 allocs, 21,107 frees, 923,854 bytes 
allocated
 ==3819==
 ==3819== 81 bytes in 1 blocks are definitely lost in loss record 1 of 2
 ==3819==at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
amd64-linux.so)
 ==3819==by 0x50D5839: strdup (strdup.c:42)
 ==3819==by 0x12A2A6: selabel_file_init (label_file.c:517)
 ==3819==by 0x12A0BB: selabel_open (label.c:88)
 ==3819==by 0x10A038: main (checkfc.c:292)
 ==3819==

 ==4238== 40 bytes in 1 blocks are definitely lost in loss record 1 of 6
 ==4238==at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
amd64-linux.so)
 ==4238==by 0x12A1D2: selabel_file_init (label_file.c:886)
 ==4238==by 0x12A0BB: selabel_open (label.c:88)
 ==4238==by 0x10A038: main (checkfc.c:292)
 ==4238==
 ==4238== 81 bytes in 1 blocks are definitely lost in loss record 2 of 6
 ==4238==at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
amd64-linux.so)
 ==4238==by 0x50D5839: strdup (strdup.c:42)
 ==4238==by 0x12A2A6: selabel_file_init (label_file.c:517)
 ==4238==by 0x12A0BB: selabel_open (label.c:88)
 ==4238==by 0x10A038: main (checkfc.c:292)
 ==4238==
 ==4238== 386 bytes in 24 blocks are definitely lost in loss record 3 of 6
 ==4238==at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
amd64-linux.so)
 ==4238==by 0x50D5889: strndup (strndup.c:45)
 ==4238==by 0x12CDDF: read_spec_entries (label_support.c:37)
 ==4238==by 0x12B72D: process_file (label_file.h:392)
 ==4238==by 0x12A2BA: selabel_file_init (label_file.c:522)
 ==4238==by 0x12A0BB: selabel_open (label.c:88)
 ==4238==by 0x10A038: main (checkfc.c:292)
 ==4238==
 ==4238== 648 bytes in 18 blocks are definitely lost in loss record 4 of 6
 ==4238==at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-
amd64-linux.so)
 ==4238==by 0x117C9B: avtab_insert_node (avtab.c:105)
 ==4238==by 0x117C10: avtab_insert (avtab.c:163)
 ==4238==by 0x11880A: avtab_read_item (avtab.c:566)
 ==4238==by 0x118BD3: avtab_read (avtab.c:600)
 ==4238==by 0x125BDD: policydb_read (policydb.c:3854)
 ==4238==by 0x109F87: main 

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

2015-10-27 Thread Stephen Smalley

On 10/26/2015 05:15 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 
---
  security/selinux/hooks.c | 101 ++-
  1 file changed, 57 insertions(+), 44 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index fc8f626..65e8689 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 a dentry's inode.
+ */
+static struct inode_security_struct *dentry_security(struct dentry *dentry)
+{
+   struct inode *inode = d_backing_inode(dentry);
+
+   return inode->i_security;
+}
+
+/*
+ * Get the security label of an inode.
+ */
+static struct inode_security_struct *inode_security(struct inode *inode)
+{
+   return inode->i_security;
+}
+
  static void inode_free_rcu(struct rcu_head *head)
  {
struct inode_security_struct *isec;



@@ -2207,7 +,6 @@ static int selinux_bprm_set_creds(struct linux_binprm 
*bprm)
struct task_security_struct *new_tsec;
struct inode_security_struct *isec;
struct common_audit_data ad;
-   struct inode *inode = file_inode(bprm->file);
int rc;

/* SELinux context only depends on initial program or script and not
@@ -2217,7 +2231,7 @@ static int selinux_bprm_set_creds(struct linux_binprm 
*bprm)

old_tsec = current_security();
new_tsec = bprm->cred->security;
-   isec = inode->i_security;
+   isec = dentry_security(bprm->file->f_path.dentry);


IIUC, this could change which inode label gets used when using overlayfs 
(the overlay inode or the underlying inode).  Not sure whether the 
current code is correct for overlayfs (overlayfs + SELinux support still 
in progress).



@@ -3154,7 +3168,7 @@ out_nofree:
  static int selinux_inode_setsecurity(struct inode *inode, const char *name,
 const void *value, size_t size, int flags)
  {
-   struct inode_security_struct *isec = inode->i_security;
+   struct inode_security_struct *isec = inode_security(inode);


Was it intentional to not do this for selinux_inode_getsecurity() and 
selinux_inode_getsecid()?



@@ -3241,8 +3254,8 @@ int ioctl_has_perm(const struct cred *cred, struct file 
*file,
  {
struct common_audit_data ad;
struct file_security_struct *fsec = file->f_security;
-   struct inode *inode = file_inode(file);
-   struct inode_security_struct *isec = inode->i_security;
+   struct dentry *dentry = file->f_path.dentry;
+   struct inode_security_struct *isec = dentry_security(dentry);
struct lsm_ioctlop_audit ioctl;
u32 ssid = cred_sid(cred);
int rc;
@@ -3263,7 +3276,7 @@ int ioctl_has_perm(const struct cred *cred, struct file 
*file,
goto out;
}

-   if (unlikely(IS_PRIVATE(inode)))
+   if (unlikely(IS_PRIVATE(dentry->d_inode)))
return 0;

rc = avc_has_extended_perms(ssid, isec->sid, isec->sclass,
@@ -3506,7 +3519,7 @@ static int selinux_file_open(struct file *file, const 
struct cred *cred)
struct inode_security_struct *isec;

fsec = file->f_security;
-   isec = file_inode(file)->i_security;
+   isec = dentry_security(file->f_path.dentry);


Similarly for these cases, switching from file_inode(file) to 
d_backing_inode(dentry) could affect overlayfs interaction IIUC.  cc'd 
David for clarification.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] sepolgen: Reset line numbers when parsing files

2015-10-27 Thread Stephen Smalley

On 10/24/2015 02:43 PM, Nicolas Iooss wrote:

When running sepolgen-ifgen on refpolicy (git master branch), the
following messages show up:

 /usr/share/selinux/refpolicy/include/kernel/selinux.if: Syntax error
 on line 3369 gen_context [type=GEN_CONTEXT]
 /usr/share/selinux/refpolicy/include/system/init.if: Syntax error on
 line 188379 ' [type=SQUOTE]
 /usr/share/selinux/refpolicy/include/system/init.if: Syntax error on
 line 188385 ' [type=SQUOTE]

The line numbers are incorrect because the lineno member of the lexer
object is not resetted after each file has been processed.  After fixing
this, the messages are nicer:

 /usr/share/selinux/refpolicy/include/kernel/selinux.if: Syntax error
 on line 43 gen_context [type=GEN_CONTEXT]
 /usr/share/selinux/refpolicy/include/system/init.if: Syntax error on
 line 1416 ' [type=SQUOTE]
 /usr/share/selinux/refpolicy/include/system/init.if: Syntax error on
 line 1422 ' [type=SQUOTE]

As line 43 of kernel/selinux.if contains a genfscon statement with a
gen_context component, the reported line numbers are now correct.

Signed-off-by: Nicolas Iooss 


Thanks, applied.


---
  sepolgen/src/sepolgen/refparser.py | 1 +
  1 file changed, 1 insertion(+)

diff --git a/sepolgen/src/sepolgen/refparser.py 
b/sepolgen/src/sepolgen/refparser.py
index b50112ccd900..3132c6fe7109 100644
--- a/sepolgen/src/sepolgen/refparser.py
+++ b/sepolgen/src/sepolgen/refparser.py
@@ -1003,6 +1003,7 @@ def parse(text, module=None, support=None, debug=False):
  create_globals(module, support, debug)
  global error, parser, lexer, success

+lexer.lineno = 1
  success = True

  try:



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] selinux: export validatetrans decisions

2015-10-27 Thread Stephen Smalley

On 10/27/2015 01:07 PM, Andrew Perepechko wrote:

Make validatetrans decisions available through selinuxfs.
"/transition" 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 /transition 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/hooks.c|  2 +-
  security/selinux/include/security.h |  2 +-
  security/selinux/selinuxfs.c| 81 +
  security/selinux/ss/services.c  | 20 +
  4 files changed, 95 insertions(+), 10 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e4369d8..f40f4b4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3043,7 +3043,7 @@ static int selinux_inode_setxattr(struct dentry *dentry, 
const char *name,
return rc;

rc = security_validate_transition(isec->sid, newsid, sid,
- isec->sclass);
+ isec->sclass, 0);


Follow the example of security_transition_sid(), i.e. introduce a _user 
interface() and re-factor the existing security_validate_transition() 
into a common helper that takes a bool argument.



if (rc)
return rc;

diff --git a/security/selinux/include/security.h 
b/security/selinux/include/security.h
index 6a681d2..84dec31 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -183,7 +183,7 @@ int security_node_sid(u16 domain, void *addr, u32 addrlen,
u32 *out_sid);

  int security_validate_transition(u32 oldsid, u32 newsid, u32 tasksid,
-u16 tclass);
+u16 tclass, int user);

  int security_bounded_transition(u32 oldsid, u32 newsid);

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 5bed7716..eb487ff 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_TRANSITION, /* compute validatetrans decision */


Could be confusing since it is for validate_transition not transition_sid.
Probably ought to put validate into the name in some form.


SEL_INO_NEXT,   /* The next inode number to use */
  };

@@ -653,6 +654,85 @@ static const struct file_operations sel_checkreqprot_ops = 
{
.llseek = generic_file_llseek,
  };

+static ssize_t sel_write_transition(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__COMPUTE_AV);


Just define a new permission in include/classmap.h (and in your policy, 
of course) and use it.



+   if (rc)
+   goto out;
+
+   rc = -ENOMEM;
+   if (count >= PAGE_SIZE - 1)
+   goto out;


Why PAGE_SIZE-1?


+
+   /* 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_to_sid(oldcon, strlen(oldcon) + 1, ,
+GFP_KERNEL);


#next has security_context_str_to_sid() as a convenient helper for this.


+   if (rc)
+   goto out;
+
+   rc = security_context_to_sid(newcon, strlen(newcon) + 1, ,
+GFP_KERNEL);
+   if (rc)
+   goto out;
+
+   rc = security_context_to_sid(taskcon, strlen(taskcon) + 1, ,
+GFP_KERNEL);
+   if (rc)
+   goto out;
+
+   rc = security_validate_transition(osid, nsid, tsid, tclass, 1);
+   if (!rc)
+   rc = count;
+out:
+   kfree(req);
+   kfree(oldcon);
+   kfree(newcon);
+   

Re: [PATCH v2] fix memory leaks and uninitialized jump

2015-10-27 Thread Stephen Smalley

On 10/27/2015 02:49 PM, william.c.robe...@intel.com wrote:

From: William Roberts 


Subject line after [PATCH] should start with "libselinux: label_file:" 
or similar prefix identifying affected component.




Some error's were reported by valgrind (below) fix them. The test
cases on which these leaks were detected:

1. properly formed file_contexts file.
2. malformed file_contexts file, unknown type.
3. malformed file_contexts file, type that fails on validate callback.
4. malformed file_contexts file, invalid regex.
5. malformed file_contexts file, invalid mode.

==3819== Conditional jump or move depends on uninitialised value(s)
==3819==at 0x12A682: closef (label_file.c:577)
==3819==by 0x12A196: selabel_close (label.c:163)
==3819==by 0x10A2FD: cleanup (checkfc.c:218)
==3819==by 0x5089258: __run_exit_handlers (exit.c:82)
==3819==by 0x50892A4: exit (exit.c:104)
==3819==by 0x10A231: main (checkfc.c:361)
==3819==  Uninitialised value was created by a heap allocation
==3819==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3819==by 0x4C2CF1F: realloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3819==by 0x12BB31: process_file (label_file.h:273)
==3819==by 0x12A2BA: selabel_file_init (label_file.c:522)
==3819==by 0x12A0BB: selabel_open (label.c:88)
==3819==by 0x10A038: main (checkfc.c:292)
==3819==
==3819==
==3819== HEAP SUMMARY:
==3819== in use at exit: 729 bytes in 19 blocks
==3819==   total heap usage: 21,126 allocs, 21,107 frees, 923,854 bytes 
allocated
==3819==
==3819== 81 bytes in 1 blocks are definitely lost in loss record 1 of 2
==3819==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3819==by 0x50D5839: strdup (strdup.c:42)
==3819==by 0x12A2A6: selabel_file_init (label_file.c:517)
==3819==by 0x12A0BB: selabel_open (label.c:88)
==3819==by 0x10A038: main (checkfc.c:292)
==3819==

==4238== 40 bytes in 1 blocks are definitely lost in loss record 1 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x12A1D2: selabel_file_init (label_file.c:886)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 81 bytes in 1 blocks are definitely lost in loss record 2 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x50D5839: strdup (strdup.c:42)
==4238==by 0x12A2A6: selabel_file_init (label_file.c:517)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 386 bytes in 24 blocks are definitely lost in loss record 3 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x50D5889: strndup (strndup.c:45)
==4238==by 0x12CDDF: read_spec_entries (label_support.c:37)
==4238==by 0x12B72D: process_file (label_file.h:392)
==4238==by 0x12A2BA: selabel_file_init (label_file.c:522)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 648 bytes in 18 blocks are definitely lost in loss record 4 of 6
==4238==at 0x4C2CC70: calloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x117C9B: avtab_insert_node (avtab.c:105)
==4238==by 0x117C10: avtab_insert (avtab.c:163)
==4238==by 0x11880A: avtab_read_item (avtab.c:566)
==4238==by 0x118BD3: avtab_read (avtab.c:600)
==4238==by 0x125BDD: policydb_read (policydb.c:3854)
==4238==by 0x109F87: main (checkfc.c:273)
==4238==
==4238== 1,095 bytes in 12 blocks are definitely lost in loss record 5 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x12D8D1: pcre_compile2 (pcre_compile.c:9217)
==4238==by 0x12B239: compile_regex (label_file.h:357)
==4238==by 0x12B9C7: process_file (label_file.h:429)
==4238==by 0x12A2BA: selabel_file_init (label_file.c:522)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)
==4238==
==4238== 1,296 bytes in 12 blocks are definitely lost in loss record 6 of 6
==4238==at 0x4C2AB80: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4238==by 0x13EBE5: pcre_study (pcre_study.c:1565)
==4238==by 0x12B25D: compile_regex (label_file.h:366)
==4238==by 0x12B9C7: process_file (label_file.h:429)
==4238==by 0x12A2BA: selabel_file_init (label_file.c:522)
==4238==by 0x12A0BB: selabel_open (label.c:88)
==4238==by 0x10A038: main (checkfc.c:292)

Signed-off-by: William Roberts 
---
  libselinux/src/label.c  |  1 +
  libselinux/src/label_file.c |  5 -
  libselinux/src/label_file.h | 21 ++---
  3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/libselinux/src/label.c 

Re: [PATCH V2] libselinux: Replace selabel_digest hash function

2015-10-22 Thread Stephen Smalley

On 10/22/2015 07:19 AM, Richard Haines wrote:

This replaces the openssl library with SHA1 hash functions
extracted from [1] as this is a public domain implementation.

util/selabel_digest -v option still compares the result with
the openssl command "openssl dgst -sha1 -hex .." for validation.

[1] https://github.com/WaterJuice/CryptLib

Signed-off-by: Richard Haines 


Thanks, applied.


---
V2 change - Hide Sha1* symbols.

  libselinux/src/Makefile |   2 +-
  libselinux/src/label_internal.h |   4 +-
  libselinux/src/label_support.c  |   6 +-
  libselinux/src/sha1.c   | 215 
  libselinux/src/sha1.h   |  86 
  libselinux/utils/Makefile   |   2 +-
  6 files changed, 310 insertions(+), 5 deletions(-)
  create mode 100644 libselinux/src/sha1.c
  create mode 100644 libselinux/src/sha1.h

diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
index 8e2223e..feab561 100644
--- a/libselinux/src/Makefile
+++ b/libselinux/src/Makefile
@@ -112,7 +112,7 @@ $(LIBA): $(OBJS)
$(RANLIB) $@

  $(LIBSO): $(LOBJS)
-   $(CC) $(CFLAGS) -shared -o $@ $^ -lpcre -ldl -lcrypto $(LDFLAGS) 
-L$(LIBDIR) -Wl,-soname,$(LIBSO),-z,defs,-z,relro
+   $(CC) $(CFLAGS) -shared -o $@ $^ -lpcre -ldl $(LDFLAGS) -L$(LIBDIR) 
-Wl,-soname,$(LIBSO),-z,defs,-z,relro
ln -sf $@ $(TARGET)

  $(LIBPC): $(LIBPC).in ../VERSION
diff --git a/libselinux/src/label_internal.h b/libselinux/src/label_internal.h
index 2aa7a7b..cefa80b 100644
--- a/libselinux/src/label_internal.h
+++ b/libselinux/src/label_internal.h
@@ -11,10 +11,10 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 
  #include "dso.h"
+#include "sha1.h"

  /*
   * Installed backends
@@ -51,7 +51,7 @@ struct selabel_sub {
   * calculate the hash the hashbuf will hold a concatenation of all the files
   * used. This is released once the value has been calculated.
   */
-#define DIGEST_SPECFILE_SIZE SHA_DIGEST_LENGTH
+#define DIGEST_SPECFILE_SIZE SHA1_HASH_SIZE
  #define DIGEST_FILES_MAX 8
  struct selabel_digest {
unsigned char *digest;  /* SHA1 digest of specfiles */
diff --git a/libselinux/src/label_support.c b/libselinux/src/label_support.c
index ac52885..324dc51 100644
--- a/libselinux/src/label_support.c
+++ b/libselinux/src/label_support.c
@@ -100,11 +100,15 @@ int hidden read_spec_entries(char *line_buf, int 
num_args, ...)
  /* Once all the specfiles are in the hash_buf, generate the hash. */
  void hidden digest_gen_hash(struct selabel_digest *digest)
  {
+   Sha1Context context;
+
/* If SELABEL_OPT_DIGEST not set then just return */
if (!digest)
return;

-   SHA1(digest->hashbuf, digest->hashbuf_size, digest->digest);
+   Sha1Initialise();
+   Sha1Update(, digest->hashbuf, digest->hashbuf_size);
+   Sha1Finalise(, (SHA1_HASH *)digest->digest);
free(digest->hashbuf);
digest->hashbuf = NULL;
return;
diff --git a/libselinux/src/sha1.c b/libselinux/src/sha1.c
new file mode 100644
index 000..db64404
--- /dev/null
+++ b/libselinux/src/sha1.c
@@ -0,0 +1,215 @@
+///
+//  LibSha1
+//
+//  Implementation of SHA1 hash function.
+//  Original author:  Steve Reid 
+//  Contributions by: James H. Brown , Saul Kravitz 
,
+//  and Ralph Giles 
+//  Modified by WaterJuice retaining Public Domain license.
+//
+//  This is free and unencumbered software released into the public domain - 
June 2013 waterjuice.org
+//  Modified to stop symbols being exported for libselinux shared library - 
October 2015
+//Richard Haines 

+///
+
+///
+//  IMPORTS
+///
+
+#include "sha1.h"
+#include "dso.h"
+#include 
+
+///
+//  TYPES
+///
+
+typedef union
+{
+uint8_t c [64];
+uint32_tl [16];
+} CHAR64LONG16;
+
+///
+//  INTERNAL FUNCTIONS
+///
+

Re: get_default_context() hit the SIMPLE_TRANSACTION_LIMIT

2015-11-09 Thread Stephen Smalley

On 11/09/2015 08:43 AM, Miroslav Grepl wrote:

We are trying to get pam_selinux + systemd-user working on Fedora
Rawhide to avoid systemd-user running with init_t. The problem is with
init_t domain which is unconfined domain by default on Fedora.


echo -n system_u:system_r:init_t:s0 unconfined_u > /sys/fs/selinux/user
sh: echo: write error: Numerical result out of range


causes failsafe_context is used for SELinux user context as a result of
pam_selinux. With disabled unconfined.pp module it works as expected.

The problem is also described here

https://bugzilla.redhat.com/show_bug.cgi?id=1274345


In the past, I have suggested not using security_compute_user() anymore 
and taking a simplified version of this logic entirely to userspace,

http://marc.info/?t=13305487561=1=2

Obviously we could increase the kernel limit, but think about what the 
get_ordered_context_list() code is doing:  it is asking the kernel to 
compute the complete set of reachable contexts (which is this case is 
huge because you are going from an unconfined domain to a user 
authorized for the unconfined role) and then throwing away the vast 
majority of the returned contexts because they don't match anything in 
/etc/selinux/targeted/contexts/default_contexts or 
/etc/selinux/targeted/contexts/users/ and then ultimately only 
using the first (highest priority) context from the ordered list.  So 
the kernel computation is mostly wasted.  Better to just cut it out 
entirely.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: selinux-testsuite: mmap execmod test failure on RHEL6.7 s390x

2015-11-05 Thread Stephen Smalley

On 11/05/2015 08:27 AM, Jan Stancek wrote:





- Original Message -

From: "Paul Moore" <p...@paul-moore.com>
To: "Stephen Smalley" <s...@tycho.nsa.gov>
Cc: "Jan Stancek" <jstan...@redhat.com>, selinux@tycho.nsa.gov
Sent: Wednesday, 4 November, 2015 10:51:15 PM
Subject: Re: selinux-testsuite: mmap execmod test failure on RHEL6.7 s390x

On Wed, Nov 4, 2015 at 3:49 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:

On 11/04/2015 03:32 PM, Paul Moore wrote:


On Wed, Nov 4, 2015 at 2:21 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:


selinux-testsuite exercises the individual kernel permission checks using
its own privately defined test domains and types, so a failure indicates
a
kernel bug or a bug in the test policy or test code, not a bug in the
distribution policy package.  It could be that a change in the
distribution
policy has a side effect (e.g. allowing some permission to all domains
that
we are trying to test such that we cannot trigger a failure, as in this
case), but the testsuite tries to work around such cases by setting any
necessary global booleans for the test duration and/or custom defining
the
test domain in such a way that it does not inherit anything from the
distribution policy.

The exec* checks can be disabled on certain architectures if they default
to
executable data but that would affect more than just execmod (and s390
does
not default to executable data).

Can you check that execmod permission is NOT granted to
test_no_execmod_t:
$ sesearch -AC -s test_no_execmod_t -p execmod

Can you confirm that the test program is not marked with executable stack
flag:
$ execstack -q tests/mmap/mprotect_file_private_execmod

Otherwise, I think you need some kernel instrumentation / tracing to see
what is happening, particularly the selinux_file_mprotect() function.



I have been working with Jan on this and it appears that the issue is
due to the READ_IMPLIES_EXEC personality being set on the affected
s390x kernels.



Hmmm...doesn't explain why only execmod failed - that should create
failures
for multiple tests.


Yes, it turns out I spoke too soon.


If I add a call to personality(0), the testcase can pass
(mprotect fails with EACCES):

diff --git a/tests/mmap/mprotect_file_private_execmod.c 
b/tests/mmap/mprotect_file_private_execmod.c
index ade1981..b2efa05 100644
--- a/tests/mmap/mprotect_file_private_execmod.c
+++ b/tests/mmap/mprotect_file_private_execmod.c
@@ -4,6 +4,7 @@
  #include 
  #include 
  #include 
+#include 

  int main(int argc, char **argv)
  {
@@ -11,6 +12,9 @@ int main(int argc, char **argv)
 int rc;
 int fd;

+
+   personality(0);
+
 if (argc != 2) {
 fprintf(stderr, "usage: %s file\n", argv[0]);
 exit(1);

(strace log)
...
[pid  2976] 08:21:53.928872 personality(PER_LINUX) = 4194304
[pid  2976] 08:21:53.936870 open("./temp_file", O_RDONLY) = 3
[pid  2976] 08:21:53.936898 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE, 
3, 0) = 0x3fffd4fa000
[pid  2976] 08:21:53.936925 mprotect(0x3fffd4fa000, 4096, PROT_READ|PROT_EXEC) 
= -1 EACCES (Permission denied)

(strace log from original)
...
[pid  3165] 08:24:31.480187 open("./temp_file", O_RDONLY) = 3
[pid  3165] 08:24:31.480294 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE, 
3, 0) = 0x3fffd196000
[pid  3165] 08:24:31.480318 mprotect(0x3fffd196000, 4096, PROT_READ|PROT_EXEC) 
= 0

# uname -r
2.6.32-584.el6.s390x

# cat /etc/redhat-release
Red Hat Enterprise Linux Server release 6.7 (Santiago)


Hmm...you have no similar issue with mprotect_stack or mprotect_heap?  I 
guess mprotect_file_private_rx and mprotect_file_private_rwx aren't 
affected because we always apply the file execute check even if the 
mapping is already executable, and likewise for mprotect_anon_* and 
execmem.  Only the checks directly in selinux_file_mprotect() are gated 
by a !(vma->vm_flags & VM_EXEC) test; the ones in file_map_prot_check() 
that are shared with the mmap hook are unconditional.


What does /proc/self/maps look like?

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: selinux-testsuite: mmap execmod test failure on RHEL6.7 s390x

2015-11-05 Thread Stephen Smalley

On 11/05/2015 10:45 AM, Jan Stancek wrote:





- Original Message -

From: "Stephen Smalley" <s...@tycho.nsa.gov>
To: "Jan Stancek" <jstan...@redhat.com>, "Paul Moore" <p...@paul-moore.com>
Cc: selinux@tycho.nsa.gov
Sent: Thursday, 5 November, 2015 3:37:33 PM
Subject: Re: selinux-testsuite: mmap execmod test failure on RHEL6.7 s390x

On 11/05/2015 08:27 AM, Jan Stancek wrote:





- Original Message -

From: "Paul Moore" <p...@paul-moore.com>
To: "Stephen Smalley" <s...@tycho.nsa.gov>
Cc: "Jan Stancek" <jstan...@redhat.com>, selinux@tycho.nsa.gov
Sent: Wednesday, 4 November, 2015 10:51:15 PM
Subject: Re: selinux-testsuite: mmap execmod test failure on RHEL6.7 s390x

On Wed, Nov 4, 2015 at 3:49 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:

On 11/04/2015 03:32 PM, Paul Moore wrote:


On Wed, Nov 4, 2015 at 2:21 PM, Stephen Smalley <s...@tycho.nsa.gov>
wrote:


selinux-testsuite exercises the individual kernel permission checks
using
its own privately defined test domains and types, so a failure
indicates
a
kernel bug or a bug in the test policy or test code, not a bug in the
distribution policy package.  It could be that a change in the
distribution
policy has a side effect (e.g. allowing some permission to all domains
that
we are trying to test such that we cannot trigger a failure, as in this
case), but the testsuite tries to work around such cases by setting any
necessary global booleans for the test duration and/or custom defining
the
test domain in such a way that it does not inherit anything from the
distribution policy.

The exec* checks can be disabled on certain architectures if they
default
to
executable data but that would affect more than just execmod (and s390
does
not default to executable data).

Can you check that execmod permission is NOT granted to
test_no_execmod_t:
$ sesearch -AC -s test_no_execmod_t -p execmod

Can you confirm that the test program is not marked with executable
stack
flag:
$ execstack -q tests/mmap/mprotect_file_private_execmod

Otherwise, I think you need some kernel instrumentation / tracing to
see
what is happening, particularly the selinux_file_mprotect() function.



I have been working with Jan on this and it appears that the issue is
due to the READ_IMPLIES_EXEC personality being set on the affected
s390x kernels.



Hmmm...doesn't explain why only execmod failed - that should create
failures
for multiple tests.


Yes, it turns out I spoke too soon.


If I add a call to personality(0), the testcase can pass
(mprotect fails with EACCES):

diff --git a/tests/mmap/mprotect_file_private_execmod.c
b/tests/mmap/mprotect_file_private_execmod.c
index ade1981..b2efa05 100644
--- a/tests/mmap/mprotect_file_private_execmod.c
+++ b/tests/mmap/mprotect_file_private_execmod.c
@@ -4,6 +4,7 @@
   #include 
   #include 
   #include 
+#include 

   int main(int argc, char **argv)
   {
@@ -11,6 +12,9 @@ int main(int argc, char **argv)
  int rc;
  int fd;

+
+   personality(0);
+
  if (argc != 2) {
  fprintf(stderr, "usage: %s file\n", argv[0]);
  exit(1);

(strace log)
...
[pid  2976] 08:21:53.928872 personality(PER_LINUX) = 4194304
[pid  2976] 08:21:53.936870 open("./temp_file", O_RDONLY) = 3
[pid  2976] 08:21:53.936898 mmap(NULL, 4096, PROT_READ|PROT_WRITE,
MAP_PRIVATE, 3, 0) = 0x3fffd4fa000
[pid  2976] 08:21:53.936925 mprotect(0x3fffd4fa000, 4096,
PROT_READ|PROT_EXEC) = -1 EACCES (Permission denied)

(strace log from original)
...
[pid  3165] 08:24:31.480187 open("./temp_file", O_RDONLY) = 3
[pid  3165] 08:24:31.480294 mmap(NULL, 4096, PROT_READ|PROT_WRITE,
MAP_PRIVATE, 3, 0) = 0x3fffd196000
[pid  3165] 08:24:31.480318 mprotect(0x3fffd196000, 4096,
PROT_READ|PROT_EXEC) = 0

# uname -r
2.6.32-584.el6.s390x

# cat /etc/redhat-release
Red Hat Enterprise Linux Server release 6.7 (Santiago)


Hmm...you have no similar issue with mprotect_stack or mprotect_heap?  I


No, these work as expected. systemtap trace shows that
vma->vm_flags doesn't contain VM_EXEC:

mprotect_heap selinux_file_mprotect vma=0x789e7680 reqprot=0x5 prot=0x5, flags: 
100073
mprotect_heap selinux_file_mprotect return=0xfff3

Here is a strace log of "runcon -t test_execmem_t $basedir/mprotect_heap":

[pid  4775] 10:31:59.987008 brk(0)  = 0xb824a000
[pid  4775] 10:31:59.987021 brk(0xb826d000) = 0xb826d000
[pid  4775] 10:31:59.987042 rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0
[pid  4775] 10:31:59.987060 rt_sigaction(SIGCHLD, NULL, {SIG_DFL, [], 0}, 8) = 0
[pid  4775] 10:31:59.987080 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
[pid  4775] 10:31:59.987094 nanosleep({30, 0}, 0x3aafed0) = 0
   ^^ I added this sleep so I can capture /proc/pid/maps
[pid  4775] 10:32:29.987868 mprotect(0xb824b000, 4096, PROT_READ|PROT_EXEC) = 
-1 EACCES (Permission denied)

# cat /proc/47

Re: does load_policy default to loading the lowest polvers available?

2015-10-14 Thread Stephen Smalley

On 10/14/2015 10:29 AM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 10:17:04AM -0400, Stephen Smalley wrote:

On 10/14/2015 10:11 AM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 09:56:04AM -0400, Stephen Smalley wrote:

On 10/14/2015 09:34 AM, Dominick Grift wrote:


I had some issue that just confused me (to say the least) It seems that
I have now solved this.

There were two policy.X files in my /etc/selinux/SELINUXTYPE/policy dir,
on 29 an one 30. The 29 seemingly had a bug in it.

It seems that load_policy (or its libselinux equivalent) defaults to
the lowest policy available (29 instead of 30 in this case)

Why is that?

I fixed the issue by removing the policy.29 file (i think at least)



What policy versions were supported by your kernel (cat
/sys/fs/selinux/policyvers) and by your libsepol (checkpolicy -V)?


/sys/fs/selinux/policyvers says: version 30, and checkpolicy says: 29 
(compatibility range 29-15)

That is weird because i have the latest libsepol installed (atleast
pretty recent):

# rpm -qa {libsepol*,libselinux*}
libselinux-utils-2.4-.git5aeb4c3.fc24.x86_64
libselinux-2.4-.git5aeb4c3.fc24.x86_64
libsepol-2.4-.git5aeb4c3.fc24.x86_64



Last release of libsepol predated policy 30 support.



However, if your kernel supports it, it should still be loaded.
The logic is in selinux/libselinux/src/load_policy.c:
selinux_mkload_policy().  With any modern kernel and configuration,
libselinux should not need to patch in local definitions or booleans
(already applied by libsemanage or preserved by the kernel), so maxvers
should be set to the max of the kernel version (/sys/fs/selinux/policyvers)
and the libsepol-supported version, and that should get loaded.



strace of load_policy might be interesting.


That is the thing indeed. It works fine if i manually run
load_policy. But when i reboot it seemed to go back to the old one. (I am
not sure how fedora currently loads the policy)

I removed the policy.29 now so i can't easily reproduce it now. and i do
not think an strace of a manual load_policy will reveal much as that
works fine and as expected. The problem only occurred when i rebooted
(when fedora load policy instead of me)

Ohh , hmm maybe its a fedora initramfs issue... they probably have some
old stuff in there


AFAIK, systemd just calls selinux_init_load_policy() in libselinux (aka 
load_policy -i).  And the approach to selecting a policy version has 
been stable for quite a while, so I wouldn't expect the libselinux in 
the initramfs to differ in this respect.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: does load_policy default to loading the lowest polvers available?

2015-10-14 Thread Stephen Smalley

On 10/14/2015 10:11 AM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 09:56:04AM -0400, Stephen Smalley wrote:

On 10/14/2015 09:34 AM, Dominick Grift wrote:


I had some issue that just confused me (to say the least) It seems that
I have now solved this.

There were two policy.X files in my /etc/selinux/SELINUXTYPE/policy dir,
on 29 an one 30. The 29 seemingly had a bug in it.

It seems that load_policy (or its libselinux equivalent) defaults to
the lowest policy available (29 instead of 30 in this case)

Why is that?

I fixed the issue by removing the policy.29 file (i think at least)



What policy versions were supported by your kernel (cat
/sys/fs/selinux/policyvers) and by your libsepol (checkpolicy -V)?


/sys/fs/selinux/policyvers says: version 30, and checkpolicy says: 29 
(compatibility range 29-15)

That is weird because i have the latest libsepol installed (atleast
pretty recent):

# rpm -qa {libsepol*,libselinux*}
libselinux-utils-2.4-.git5aeb4c3.fc24.x86_64
libselinux-2.4-.git5aeb4c3.fc24.x86_64
libsepol-2.4-.git5aeb4c3.fc24.x86_64


Last release of libsepol predated policy 30 support.

However, if your kernel supports it, it should still be loaded.
The logic is in selinux/libselinux/src/load_policy.c: 
selinux_mkload_policy().  With any modern kernel and configuration, 
libselinux should not need to patch in local definitions or booleans 
(already applied by libsemanage or preserved by the kernel), so maxvers 
should be set to the max of the kernel version 
(/sys/fs/selinux/policyvers) and the libsepol-supported version, and 
that should get loaded.


strace of load_policy might be interesting.





___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: does load_policy default to loading the lowest polvers available?

2015-10-14 Thread Stephen Smalley

On 10/14/2015 11:48 AM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 11:44:00AM -0400, Stephen Smalley wrote:

On 10/14/2015 10:29 AM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 10:17:04AM -0400, Stephen Smalley wrote:

On 10/14/2015 10:11 AM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 09:56:04AM -0400, Stephen Smalley wrote:

On 10/14/2015 09:34 AM, Dominick Grift wrote:


I had some issue that just confused me (to say the least) It seems that
I have now solved this.

There were two policy.X files in my /etc/selinux/SELINUXTYPE/policy dir,
on 29 an one 30. The 29 seemingly had a bug in it.

It seems that load_policy (or its libselinux equivalent) defaults to
the lowest policy available (29 instead of 30 in this case)

Why is that?

I fixed the issue by removing the policy.29 file (i think at least)



What policy versions were supported by your kernel (cat
/sys/fs/selinux/policyvers) and by your libsepol (checkpolicy -V)?


/sys/fs/selinux/policyvers says: version 30, and checkpolicy says: 29 
(compatibility range 29-15)

That is weird because i have the latest libsepol installed (atleast
pretty recent):

# rpm -qa {libsepol*,libselinux*}
libselinux-utils-2.4-.git5aeb4c3.fc24.x86_64
libselinux-2.4-.git5aeb4c3.fc24.x86_64
libsepol-2.4-.git5aeb4c3.fc24.x86_64



Last release of libsepol predated policy 30 support.



However, if your kernel supports it, it should still be loaded.
The logic is in selinux/libselinux/src/load_policy.c:
selinux_mkload_policy().  With any modern kernel and configuration,
libselinux should not need to patch in local definitions or booleans
(already applied by libsemanage or preserved by the kernel), so maxvers
should be set to the max of the kernel version (/sys/fs/selinux/policyvers)
and the libsepol-supported version, and that should get loaded.



strace of load_policy might be interesting.


That is the thing indeed. It works fine if i manually run
load_policy. But when i reboot it seemed to go back to the old one. (I am
not sure how fedora currently loads the policy)

I removed the policy.29 now so i can't easily reproduce it now. and i do
not think an strace of a manual load_policy will reveal much as that
works fine and as expected. The problem only occurred when i rebooted
(when fedora load policy instead of me)

Ohh , hmm maybe its a fedora initramfs issue... they probably have some
old stuff in there



AFAIK, systemd just calls selinux_init_load_policy() in libselinux (aka
load_policy -i).  And the approach to selecting a policy version has been
stable for quite a while, so I wouldn't expect the libselinux in the
initramfs to differ in this respect.


Yes, its something else because in permissive mode it seemed to work. So
i suppose what ever it needed to do, it was denied somehow. ofcourse no
AVC denials or any other related messages in the logs. (i suppose
because it happens so early)


Prior to initial policy load, everything is allowed (even if enforcing), 
so it can't be a SELinux permission denial.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: does load_policy default to loading the lowest polvers available?

2015-10-14 Thread Stephen Smalley

On 10/14/2015 01:38 PM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 07:34:16PM +0200, Dominick Grift wrote:


Setools(4) doesnt work with my policy (it can't deal with cil namespaces
seemingly, and returns non-sense)



Besides. did you know that setools (4) does not use
/sys/fs/selinux/policy? It uses /etc/selinux/SELINUXTYPE/policy/policy.X
instead. This sounded to me like a bad idea. Mainly because you don't
know if the /etc/selinux/SELINUXTYPE/policy/policy.X is the policy that
is currently actually loaded into the system.


It should use selinux_current_policy_path() to find the policy.

In any event, did you try compute_av from libselinux on the system in 
question?





___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: does load_policy default to loading the lowest polvers available?

2015-10-14 Thread Stephen Smalley

On 10/14/2015 01:34 PM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 12:53:06PM -0400, Stephen Smalley wrote:

On 10/14/2015 12:41 PM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 12:05:27PM -0400, Stephen Smalley wrote:



AFAIK, systemd just calls selinux_init_load_policy() in libselinux (aka
load_policy -i).  And the approach to selecting a policy version has been
stable for quite a while, so I wouldn't expect the libselinux in the
initramfs to differ in this respect.


I just reboot that machine, and it happened again! So the dangling 29
file was not at all related.

This issue is so weird, and so hard to narrow down.

I have about 7 systems all with the same policy, same selinux userspace, 
different form factors,
2 laptops (one rawhide, on fedora 23), one worksstation (rawhide) and
4 qemu/kvm guests (all rawhide)

Theyre pretty much all identical from a config point of view except that
the workstation is a hypervisor and router

The workstation is the issue. I am getting avc denials for the same
access vectors (but only on the workstation):

system {status start }

(obivously the rules to allow it are present in the policy)



You say "obviously"; how have you verified?  You could run sesearch on the
kernel's view of the policy (/sys/fs/selinux/policy), or you could run
compute_av from libselinux.



If allowed by policy but denied by systemd (since those are systemd
permissions, not kernel ones, and unfortunately use a kernel class), then
I've only seen that on a policy reload that alters the class definitions.
That issue should be fixed by the patch I posted a while back for
libselinux, which I believe should now be in rawhide.



Yes well setools (3) needs to be updated (only supports up to version 29),
but it does not build without a patch and i was hoping fedora would
update its setools (theres a bugzilla open for that). so far it hasnt
done so, and I haven't done so myself either (was hoping they would do
it instead)

Setools(4) doesnt work with my policy (it can't deal with cil namespaces
seemingly, and returns non-sense)

However I did query the, what should be, same policy on my fedora 23
system and the rules are in there. So that is why i said "obviously"

My libselinux is uptodate to 5aeb4c350b5cba13a68fc11e365bede82ad2cafb
This means that it has your fix for the policy reload issue

Could it be that this fix actually introduced this? I do not believe
that but I am not sure. (at this point I am not sure of anything)


I don't see why, but you can certainly test that easily enough - just 
revert that change and rebuild your libselinux, see if you continue to 
get the denials on that system.  If not, then re-apply that change, 
rebuild your libselinux, and confirm that you once again get denials on 
that system.


You could also add some instrumentation to selinux_check_access() and 
avc_has_perm() to try to discover more about why it is failing.  You can 
just add further avc_log() calls and they should go to the audit and 
journal logs.  You could log the class -> sclass and perm -> av mappings 
obtained by selinux_check_access() prior to calling avc_has_perm().



Why does it work sometimes but fail most of the time. why does this only
happen on one system. I am pretty sure i do not have any faulty RAM. Also 
everything else works fine and the
issue always on applies to the "system { start stop status }" access
vectors. any other ones work fine for example "service { start stop
status }" works fine.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: did libselinux grow a new build dependency? (openssl-devel: openssl.h)

2015-10-19 Thread Stephen Smalley

On 10/19/2015 02:09 PM, Stephen Smalley wrote:

On 10/18/2015 11:00 AM, Richard Haines wrote:




On Sunday, 18 October 2015, 15:07, Dominick Grift
<dac.overr...@gmail.com> wrote:



-BEGIN PGP SIGNED MESSAGE-

Hash: SHA512

On Sun, Oct 18, 2015 at 12:48:12PM +, Richard Haines wrote:

  I added openssl to libselinux to support the new selabel_digest(3)
  function.

  I'm not aware of any issues between openssl and gnutls, however as

  selabel_digest was only added last week I guess not much testing.
  Well apart from myself as I'm currently adding the selinux_restorecon
  feature that makes use of it.



Thanks for clarifying, I am not hitting any issues with it just
wondering if instead of openssl, gnutls could be used for this and if



so, if this should be somehow supported or not.


I tried using gnutls after I read your initial email, however I
could not find a way to generate the same digest as openssl
(I changed the SHA1 function to gnutls_hmac_fast(3) with various
algorithms and used the selabel_digest util to compare digests).
It could be that I should use some other function but I could

not find any useful info on this (including web searches).
If anyone knows how to resolve this please let me know.

I guess what is supported (openssl or gnutls) would be down to
the maintainers.


Wondering if dependency on openssl might be a license issue for Debian
or others.  Apparently openssl license is considered GPL-incompatible
[1] [2], and obviously libselinux is linked by a variety of GPL-licensed
programs.  Fedora seems to view this as falling under the system library
exception [3] but not clear that other distributions would view it that
way.  On the other hand, using gnutls would be subject to the reverse
problem; it would make libselinux depend on a LGPL library, and that
could create issues for non-GPL programs that statically link
libselinux.  We might need to revert this change and revisit how to
solve this in a manner that avoids such issues.

[1] http://www.gnu.org/licenses/license-list.en.html#OpenSSL

[2] https://people.gnome.org/~markmc/openssl-and-the-gpl.html

[3]
https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/FAQ#What.27s_the_deal_with_the_OpenSSL_license.3F)


Also, aside from license issues, we likely ought to dlopen libcrypto.so 
so that we don't bring this dependency to all users of libselinux but 
only those that actually use the digest functionality.






___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


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

2015-10-20 Thread Stephen Smalley
On Mon, Oct 19, 2015 at 6:29 PM, Paul Moore <pmo...@redhat.com> 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.  In the common case, the labels
will be the same and it won't matter, but if you want to support some
MAC form of their 'custom endpoints' with further restrictions, you
can do that by labeling the endpoints uniquely and using those labels
in your permission checks.

Otherwise, I have to ask whether you need two different classes above
(bus vs endpoint), and whether the privileged/activator/monitor/policy
permissions properly belong with the endpoint label or the bus label.
At least some of those are bus-wide properties, not endpoint-specific,
but that's ok as long as we have established a relationship between
the endpoint label and bus label.

> ... 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.

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 C

Re: does load_policy default to loading the lowest polvers available?

2015-10-14 Thread Stephen Smalley

On 10/14/2015 12:41 PM, Dominick Grift wrote:

On Wed, Oct 14, 2015 at 12:05:27PM -0400, Stephen Smalley wrote:



AFAIK, systemd just calls selinux_init_load_policy() in libselinux (aka
load_policy -i).  And the approach to selecting a policy version has been
stable for quite a while, so I wouldn't expect the libselinux in the
initramfs to differ in this respect.


I just reboot that machine, and it happened again! So the dangling 29
file was not at all related.

This issue is so weird, and so hard to narrow down.

I have about 7 systems all with the same policy, same selinux userspace, 
different form factors,
2 laptops (one rawhide, on fedora 23), one worksstation (rawhide) and
4 qemu/kvm guests (all rawhide)

Theyre pretty much all identical from a config point of view except that
the workstation is a hypervisor and router

The workstation is the issue. I am getting avc denials for the same
access vectors (but only on the workstation):

system {status start }

(obivously the rules to allow it are present in the policy)


You say "obviously"; how have you verified?  You could run sesearch on 
the kernel's view of the policy (/sys/fs/selinux/policy), or you could 
run compute_av from libselinux.


If allowed by policy but denied by systemd (since those are systemd 
permissions, not kernel ones, and unfortunately use a kernel class), 
then I've only seen that on a policy reload that alters the class 
definitions.  That issue should be fixed by the patch I posted a while 
back for libselinux, which I believe should now be in rawhide.




Is it Linux 4.3 related -> then why does it work on my rawhide laptop,
and kvm guests fine
Is it my policy -> then why does it work on all my other systems fine
Is it hardware related -> seems to be the only explanation but then why
does it not happen consistently? (it happens most of the time when boot
but not always)
Maybe it is a combination of hardware + linux 4.3?

So many questions and so hard to debug...




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: does load_policy default to loading the lowest polvers available?

2015-10-14 Thread Stephen Smalley

On 10/14/2015 09:34 AM, Dominick Grift wrote:


I had some issue that just confused me (to say the least) It seems that
I have now solved this.

There were two policy.X files in my /etc/selinux/SELINUXTYPE/policy dir,
on 29 an one 30. The 29 seemingly had a bug in it.

It seems that load_policy (or its libselinux equivalent) defaults to
the lowest policy available (29 instead of 30 in this case)

Why is that?

I fixed the issue by removing the policy.29 file (i think at least)


What policy versions were supported by your kernel (cat 
/sys/fs/selinux/policyvers) and by your libsepol (checkpolicy -V)?


load_policy will try to use the highest policy version that is supported 
by the kernel or by your libsepol.  If supported by the kernel, it can 
just load the file directly.  Otherwise, it can use libsepol to 
downgrade the policy to the highest version supported by the kernel and 
then load the result.  If the version is not supported by either the 
kernel or your libsepol, then it cannot be loaded and it will fall back 
to an older version.




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: did libselinux grow a new build dependency? (openssl-devel: openssl.h)

2015-10-20 Thread Stephen Smalley

On 10/20/2015 09:42 AM, Joshua Brindle wrote:

Stephen Smalley wrote:



Wondering if dependency on openssl might be a license issue for Debian
or others. Apparently openssl license is considered GPL-incompatible [1]
[2], and obviously libselinux is linked by a variety of GPL-licensed
programs. Fedora seems to view this as falling under the system library
exception [3] but not clear that other distributions would view it that
way. On the other hand, using gnutls would be subject to the reverse
problem; it would make libselinux depend on a LGPL library, and that
could create issues for non-GPL programs that statically link
libselinux. We might need to revert this change and revisit how to solve
this in a manner that avoids such issues.


LGPL explicitly allows non-GPL programs to link against an LGPL licensed
library without tainting the non-GPL program, which is the whole point
of the LGPL. Is there some other issue with static linking or something?


Yes, that's the concern.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: did libselinux grow a new build dependency? (openssl-devel: openssl.h)

2015-10-20 Thread Stephen Smalley

On 10/20/2015 08:27 AM, Richard Haines wrote:






On Monday, 19 October 2015, 19:10, Stephen Smalley <s...@tycho.nsa.gov> wrote:

On 10/18/2015 11:00 AM, Richard Haines wrote:



  On Sunday, 18 October 2015, 15:07, Dominick Grift

<dac.overr...@gmail.com> wrote:



  -BEGIN PGP SIGNED MESSAGE-

  Hash: SHA512

  On Sun, Oct 18, 2015 at 12:48:12PM +, Richard Haines wrote:

I added openssl to libselinux to support the new

selabel_digest(3)

function.

I'm not aware of any issues between openssl and gnutls,

however as


selabel_digest was only added last week I guess not much testing.
Well apart from myself as I'm currently adding the

selinux_restorecon

feature that makes use of it.



  Thanks for clarifying, I am not hitting any issues with it just
  wondering if instead of openssl, gnutls could be used for this and if



  so, if this should be somehow supported or not.


  I tried using gnutls after I read your initial email, however I
  could not find a way to generate the same digest as openssl
  (I changed the SHA1 function to gnutls_hmac_fast(3) with various
  algorithms and used the selabel_digest util to compare digests).
  It could be that I should use some other function but I could

  not find any useful info on this (including web searches).
  If anyone knows how to resolve this please let me know.

  I guess what is supported (openssl or gnutls) would be down to
  the maintainers.


Wondering if dependency on openssl might be a license issue for Debian
or others.  Apparently openssl license is considered GPL-incompatible
[1] [2], and obviously libselinux is linked by a variety of GPL-licensed
programs.  Fedora seems to view this as falling under the system library
exception [3] but not clear that other distributions would view it that
way.  On the other hand, using gnutls would be subject to the reverse
problem; it would make libselinux depend on a LGPL library, and that
could create issues for non-GPL programs that statically link
libselinux.  We might need to revert this change and revisit how to



solve this in a manner that avoids such issues.



Would building with the Android mincrypt SHA functions help regarding the
licensing issues ??? I've attached a quick patch that seems to work okay
using Android system/core/libmincrypt/sha.c


That looks BSD-licensed and thus broadly compatible.  We would need to 
amend libselinux/LICENSE to add that license information and we would 
need to hide those functions from being exposed outside of the library. 
 Other alternative would be to look for a public domain SHA 
implementation and use that.



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] security: selinux: Use a kmem_cache for allocation struct file_security_struct

2015-10-07 Thread Stephen Smalley
On 10/05/2015 01:45 AM, Sangwoo wrote:
> The size of struct file_security_struct is 16byte at my setup.
> But, the real allocation size for per each file_security_struct
> is 64bytes in my setup that kmalloc min size is 64bytes
> because ARCH_DMA_MINALIGN is 64.
> 
> This allocation is called every times at file allocation(alloc_file()).
> So, the total slack memory size(allocated size - request size)
> is increased exponentially.
> 
> E.g) Min Kmalloc Size : 64bytes, Unit : bytes
>   Allocated Size | Request Size | Slack Size | Allocation Count
> ---
>  770048  |192512|   577536   |  12032
> 
> At the result, this change reduce memory usage 42bytes per each
> file_security_struct
> 
> Signed-off-by: Sangwoo <sangwoo2.p...@lge.com>

Acked-by:  Stephen Smalley <s...@tycho.nsa.gov>

> ---
>  security/selinux/hooks.c |8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3f8d567..c20e082 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -126,6 +126,7 @@ int selinux_enabled = 1;
>  #endif
>  
>  static struct kmem_cache *sel_inode_cache;
> +static struct kmem_cache *file_security_cache;
>  
>  /**
>   * selinux_secmark_enabled - Check to see if SECMARK is currently enabled
> @@ -287,7 +288,7 @@ static int file_alloc_security(struct file *file)
>   struct file_security_struct *fsec;
>   u32 sid = current_sid();
>  
> - fsec = kzalloc(sizeof(struct file_security_struct), GFP_KERNEL);
> + fsec = kmem_cache_zalloc(file_security_cache, GFP_KERNEL);
>   if (!fsec)
>   return -ENOMEM;
>  
> @@ -302,7 +303,7 @@ static void file_free_security(struct file *file)
>  {
>   struct file_security_struct *fsec = file->f_security;
>   file->f_security = NULL;
> - kfree(fsec);
> + kmem_cache_free(file_security_cache, fsec);
>  }
>  
>  static int superblock_alloc_security(struct super_block *sb)
> @@ -6086,6 +6087,9 @@ static __init int selinux_init(void)
>   sel_inode_cache = kmem_cache_create("selinux_inode_security",
>   sizeof(struct 
> inode_security_struct),
>   0, SLAB_PANIC, NULL);
> + file_security_cache = kmem_cache_create("selinux_file_security",
> + sizeof(struct file_security_struct),
> + 0, SLAB_PANIC, NULL);
>   avc_init();
>  
>   security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks));
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH v2 1/2] security: Add hook to invalidate inode security labels

2015-10-06 Thread Stephen Smalley
On 10/05/2015 05:56 PM, Andreas Gruenbacher wrote:
> On Mon, Oct 5, 2015 at 5:08 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> Not fond of these magic initialized values.
> 
> That should be a solvable problem.
> 
>> Is it always safe to call inode_doinit() from all callers of
>> inode_has_perm()?
> 
> As long as inode_has_perm is only used in contexts in which a file
> permission check / acl check would be possible, I don't see why not.
> 
>> What about the cases where isec->sid is used without going through
>> inode_has_perm()?
> 
> inode_has_perm seems to be called frequently and invalid labels seem
> to be reload quickly, so this change may make SELinux work well enough
> to be useful on top of gfs2 or similar. More checks would of course be
> better. The ideal case would be to always reload invalid labels, but
> that currently won't be possible because we don't have dentries
> everywhere.
> 
> I can't tell if this is this good enough to provide a useful level of
> protection. In any case, without a patch like this, on gfs2 and
> similar file systems, SELinux currently doesn't work at all.
> 
> How we can make progress with this problem?

I think we'd need to wrap all uses of inode->i_security with a helper that
applies this test.  FWIW, many/most of them seem to have a dentry
available, including all callers of inode_has_perm itself, so you could
just use inode_doinit_with_dentry() for all of those cases.  Maybe just
inline inode_has_perm() and get rid of it.

Need to deal appropriately with situations like selinux_inode_permission with
MAY_NOT_BLOCK.






___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [RFC PATCH v3 1/5] kdbus: add creator credentials to the endpoints

2015-10-09 Thread Stephen Smalley

On 10/07/2015 07:08 PM, Paul Moore wrote:

In order to effectively enforce LSM based access controls we need to
have more information about the kdbus endpoint creator than the
uid/gid currently stored in the kdbus_node_type struct.  This patch
replaces the uid/gid values with a reference to the node creator's
credential struct which serves the needs of both the kdbus DAC access
controls as well as the LSM's access controls.

Two macros have also been created, kdbus_node_[uid,gid](), which can
be used to easily extract the euid/egid information from the new
credential reference.  The effective uid/gid is used as it was used
in all areas of the previous kdbus code except for areas where the
uid/gid was never set beyond the basic initialization to zero/root;
I expect this was a bug that was never caught as the node creator in
these cases was always expect to be root.

Signed-off-by: Paul Moore 

---
ChangeLog:
- v3
  * Ported to the 4.3-rc4 based kdbus tree
- v2
  * Initial draft
---
  ipc/kdbus/bus.c  |   13 +
  ipc/kdbus/endpoint.c |   14 --
  ipc/kdbus/endpoint.h |3 +--
  ipc/kdbus/fs.c   |4 ++--
  ipc/kdbus/node.c |   11 ---
  ipc/kdbus/node.h |5 +++--
  6 files changed, 19 insertions(+), 31 deletions(-)




diff --git a/ipc/kdbus/node.c b/ipc/kdbus/node.c
index 89f58bc..cd0c1a0 100644
--- a/ipc/kdbus/node.c
+++ b/ipc/kdbus/node.c
@@ -12,6 +12,7 @@
   */

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -170,13 +171,7 @@
   * node initialization. They must remain constant. If
   * NULL, they're skipped.
   *
- * * node->mode: filesystem access modes


mode still remains
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [RFC PATCH V2] libselinux: Add selinux_restorecon function

2015-10-09 Thread Stephen Smalley

On 10/01/2015 11:18 AM, Richard Haines wrote:

On Tuesday, 29 September 2015, 21:25, Stephen Smalley <s...@tycho.nsa.gov> 
wrote:

On 09/27/2015 08:06 AM, Richard Haines wrote:
  The selinux_restorecon(3) man page details this function that relies
  on the selabel_digest(3) function available from [1] (as not yet
  part of upstream libselinux).

  It has been built using the work from Android where an SHA1 hash
  of the specfiles is held in an extended attribute to enhance
  performance. Also contains components from policycoreutils/setfiles.

  The utils/selinux_restorecon.c utility demonstrates the functionality.

  [1] http://marc.info/?l=selinux=144274383217343=2

  Signed-off-by: Richard Haines <richard_c_hai...@btinternet.com>



  ---

 snip --

  +
  +extern int selinux_restorecon(const char **pathname_list,
  +const char **exclude_list,
  +const char *fc_path,
  +unsigned int restorecon_flags);





This is a more cumbersome interface for typical users than the Android one.


To make this easier would you prefer it to just take a single pathname and the
flags (and maybe the fc_path as well, or add another interface to take it as
discussed below)


Yes, it would be preferably to keep the default interface simple, and 
provide other interfaces to set non-default behaviors that go beyond 
simple flags.



The only reason I put the exclude_list is to allow filesystems that don't have
xattr support to be excluded by the caller. This could probably be resolved by
always setting the FTS_XDEV flag with the caller ensuring they cover their
relevant filesystems.


Well, setfiles does have the -e option, so providing a way to set an 
exclude list makes sense; it just doesn't necessarily have to be part of 
the default interface as opposed to a separate set_exclude_list 
interface called before invoking restorecon.




 snip --


  +fc_sehandle = selabel_open(SELABEL_CTX_FILE, fc_opts,

NUM_SELABEL_OPTS);

  +if (!fc_sehandle) {
  +selinux_log(SELINUX_ERROR,
  +"Error obtaining file context handle: %s\n",
  +strerror(errno));
  +return -1;
  +}


Android only does this once, not on every call to restorecon.
Caller that wants to use selabel_open() itself with custom options can
use selinux_android_set_sethandle() after selabel_open() call;



otherwise, callers don't ever have to specify selabel_open() args.


I could implement a similar interface to selinux_android_file_context_handle
(I guess that is what you are referring to) that would also take the fc_path
if this would be useful, it then keep selinux_restorecon simple and in line
with Android.


No, you would need an interface like set_sehandle(), which sets the 
handle used internally by restorecon to a handle provided by the caller 
(where that handle might originate from a direct selabel_open call by 
the caller if it wants full control, or from a call to 
selinux_android_file_context_handle if it just needs a reference to the 
handle for use elsewhere).




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: 答复: got some problems with the type_transition rules

2015-09-08 Thread Stephen Smalley
On 09/08/2015 05:06 AM, kuangjiou wrote:
> According to this webpage,
> http://selinuxproject.org/page/TypeRules
> 
> Policy versions 25 and above also support a 'name transition' rule
> 
> But the policy versions of my os is 26,I don't know why the type_trasition 
> rule didn't work

To see what version is supported by your kernel:
cat /selinux/policyvers or
cat /sys/fs/selinux/policyvers
(depending on where you have selinuxfs mounted)

Make sure you don't have CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
set in your kernel .config.  That was a legacy option for backward
compatibility with Fedora 3 and 4, and forces the kernel to an old
policy version.  You don't want it.








___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH] libselinux: Free memory when processing media and x specfiles

2015-09-15 Thread Stephen Smalley
On 09/15/2015 09:51 AM, Richard Haines wrote:
> Ensure all memory is freed - checked using valgrind
> 
> Signed-off-by: Richard Haines 

Thanks, applied.

> ---
>  libselinux/src/label_media.c | 2 +-
>  libselinux/src/label_x.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libselinux/src/label_media.c b/libselinux/src/label_media.c
> index 725d5bd..c589ca3 100644
> --- a/libselinux/src/label_media.c
> +++ b/libselinux/src/label_media.c
> @@ -161,7 +161,7 @@ static void close(struct selabel_handle *rec)
>   if (spec_arr)
>   free(spec_arr);
>  
> - memset(data, 0, sizeof(*data));
> + free(data);
>  }
>  
>  static struct selabel_lookup_rec *lookup(struct selabel_handle *rec,
> diff --git a/libselinux/src/label_x.c b/libselinux/src/label_x.c
> index 3f2ee1c..3d13b23 100644
> --- a/libselinux/src/label_x.c
> +++ b/libselinux/src/label_x.c
> @@ -188,7 +188,7 @@ static void close(struct selabel_handle *rec)
>   if (spec_arr)
>   free(spec_arr);
>  
> - memset(data, 0, sizeof(*data));
> + free(data);
>  }
>  
>  static struct selabel_lookup_rec *lookup(struct selabel_handle *rec,
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: newrole not working when built with LSPP_PRIV=y

2015-10-01 Thread Stephen Smalley

On 10/01/2015 03:51 AM, Laurent Bigonville wrote:



Le 29/09/15 21:35, Stephen Smalley a écrit :

On 09/26/2015 09:10 PM, Laurent Bigonville wrote:

[...]

The patch seems to break an other thing, it Fedora the newrole
executable is not setuid root, but it is granted a bunch of capabilities
explicitly, if I setuid this executable instead of granting these
capabilities, I get yet an other error:

Sorry, newrole failed to drop capabilities: Operation not permitted

So I guess something need to be fixed here.


Yes, the current code just seems to be wrong here.  The setresuid()
call will drop all capabilities if newrole is setuid-root and the
caller is non-root, so it will end up dropping all capabilities
immediately.  Then the attempt to further set the capabilities will
fail (as above), as will any subsequent privileged operations.  As
currently written, this can only work if not setuid-root and using
file-caps.  And in that case, the setresuid() call doesn't make sense.

Dan?


Apparently libcapng has a capng_change_id(3) function that can be used
to "change the credentials retaining capabilities".


If you just delete the setresuid() call entirely from the 
NAMESPACE_PRIV=y drop_capabilities() function, then it works (on top of 
the Fedora patch you cited).  That function wasn't supposed to be doing 
setresuid() in the first place; that is deferred to 
transition_to_caller_uid(), _after_ pam_namespace has run.  I'm guessing 
that pam_namespace is broken in Fedora.


Note that the Fedora patch you cited has at least one bug; it reverts an 
upstream fix to open stdin read/write to avoid breaking programs like 
tmux that assume that if stdin is a tty, then it can be both read and 
written.













___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


[PATCH 1/3] Fix newrole to not drop capabilities from the bounding set.

2015-10-01 Thread Stephen Smalley
From: Dan Walsh 

Stop dropping capabilities from its children.
Add better error messages.

Signed-off-by: Dan Walsh 
---
 policycoreutils/newrole/newrole.c | 60 +--
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/policycoreutils/newrole/newrole.c 
b/policycoreutils/newrole/newrole.c
index 1882cc8..55e8d39 100644
--- a/policycoreutils/newrole/newrole.c
+++ b/policycoreutils/newrole/newrole.c
@@ -546,9 +546,7 @@ static int drop_capabilities(int full)
if (!uid) return 0;
 
capng_setpid(getpid());
-   capng_clear(CAPNG_SELECT_BOTH);
-   if (capng_lock() < 0) 
-   return -1;
+   capng_clear(CAPNG_SELECT_CAPS);
 
/* Change uid */
if (setresuid(uid, uid, uid)) {
@@ -557,7 +555,7 @@ static int drop_capabilities(int full)
}
if (! full) 
capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED, 
CAP_AUDIT_WRITE);
-   return capng_apply(CAPNG_SELECT_BOTH);
+   return capng_apply(CAPNG_SELECT_CAPS);
 }
 #elif defined(NAMESPACE_PRIV)
 /**
@@ -575,20 +573,21 @@ static int drop_capabilities(int full)
  */
 static int drop_capabilities(int full)
 {
+   uid_t uid = getuid();
+   if (!uid) return 0;
+
capng_setpid(getpid());
-   capng_clear(CAPNG_SELECT_BOTH);
-   if (capng_lock() < 0) 
-   return -1;
+   capng_clear(CAPNG_SELECT_CAPS);
 
-   uid_t uid = getuid();
/* Change uid */
if (setresuid(uid, uid, uid)) {
fprintf(stderr, _("Error changing uid, aborting.\n"));
return -1;
}
if (! full) 
-   capng_updatev(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED, 
CAP_SYS_ADMIN , CAP_FOWNER , CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_SETPCAP, -1);
-   return capng_apply(CAPNG_SELECT_BOTH);
+   capng_updatev(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED, 
CAP_SYS_ADMIN , CAP_FOWNER , CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_AUDIT_WRITE, -1);
+   
+   return capng_apply(CAPNG_SELECT_CAPS);
 }
 
 #else
@@ -679,7 +678,7 @@ static int relabel_tty(const char *ttyn, security_context_t 
new_context,
   security_context_t * tty_context,
   security_context_t * new_tty_context)
 {
-   int fd;
+   int fd, rc;
int enforcing = security_getenforce();
security_context_t tty_con = NULL;
security_context_t new_tty_con = NULL;
@@ -698,7 +697,13 @@ static int relabel_tty(const char *ttyn, 
security_context_t new_context,
fprintf(stderr, _("Error!  Could not open %s.\n"), ttyn);
return fd;
}
-   fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+   /* this craziness is to make sure we cann't block on open and deadlock 
*/
+   rc = fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+   if (rc) {
+   fprintf(stderr, _("Error!  Could not clear O_NONBLOCK on 
%s\n"), ttyn);
+   close(fd);
+   return rc;
+   }
 
if (fgetfilecon(fd, _con) < 0) {
fprintf(stderr, _("%s!  Could not get current context "
@@ -1009,9 +1014,9 @@ int main(int argc, char *argv[])
int fd;
pid_t childPid = 0;
char *shell_argv0 = NULL;
+   int rc;
 
 #ifdef USE_PAM
-   int rc;
int pam_status; /* pam return code */
pam_handle_t *pam_handle;   /* opaque handle used by all PAM 
functions */
 
@@ -1222,18 +1227,26 @@ int main(int argc, char *argv[])
fprintf(stderr, _("Could not close descriptors.\n"));
goto err_close_pam;
}
-   fd = open(ttyn, O_RDWR | O_NONBLOCK);
+   fd = open(ttyn, O_RDONLY | O_NONBLOCK);
if (fd != 0)
goto err_close_pam;
-   fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+   rc = fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+   if (rc)
+   goto err_close_pam;
+
fd = open(ttyn, O_RDWR | O_NONBLOCK);
if (fd != 1)
goto err_close_pam;
-   fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+   rc = fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+   if (rc)
+   goto err_close_pam;
+
fd = open(ttyn, O_RDWR | O_NONBLOCK);
if (fd != 2)
goto err_close_pam;
-   fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+   rc = fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
+   if (rc)
+   goto err_close_pam;
 
}
/*
@@ -1267,19 +1280,24 @@ int main(int argc, char *argv[])
}
 #endif
 
-   if (send_audit_message(1, old_context, new_context, 

Re: av_decision on audit callback

2015-10-02 Thread Stephen Smalley

On 10/02/2015 04:22 PM, Roberts, William C wrote:




-Original Message-
From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
Sent: Friday, October 2, 2015 1:13 PM
To: Roberts, William C; seandroid-l...@tycho.nsa.gov; selinux@tycho.nsa.gov
Subject: Re: av_decision on audit callback

On 10/02/2015 04:07 PM, Roberts, William C wrote:




-Original Message-
From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
Sent: Friday, October 2, 2015 12:12 PM
To: Roberts, William C; seandroid-l...@tycho.nsa.gov;
selinux@tycho.nsa.gov
Subject: Re: av_decision on audit callback

On 10/02/2015 02:54 PM, Stephen Smalley wrote:

On 10/02/2015 02:48 PM, Roberts, William C wrote:

I would like to be able to gather the result of permissive mode per
domain from a check_access() call for the userspace object managers
on Android.

   From what I can tell check_access() calls avc_has_perm with a
NULL 5th argument. That argument is for the struct avc_entry_ref.

That structure has a pointer to an opaque type, avc_entry. Which
contains struct av_decision.

Which contains flags that have a permissive flag:

struct av_decision {

   access_vector_t allowed;

   access_vector_t decided;

   access_vector_t auditallow;

   access_vector_t auditdeny;

   unsigned int seqno;

   unsigned int flags;

};

/* Definitions of av_decision.flags */

#define SELINUX_AVD_FLAGS_PERMISSIVE0x0001

It looks like if check_access just passes this structure and then
avc_has_perm() when it calls avc_audit, it could supply the
av_decision structure to the avc_suppl_audit() call. We could then
have an audit2 callback that takes this parameter.

Is this mostly right, seem sane? Better way to do this?


It doesn't need to be exposed at that level; the libselinux
avc_audit() routine can log it, similar to what is done in the kernel.
It already has the av_decision structure available to it.


To clarify, anything directly known to the AVC, like the permissive
flag, can be directly logged by it.  The audit callback is for
logging auxiliary audit information not known to the AVC (the pid of the client

process being a good example).


I was wondering if we could just dump permissive=0|1 from the AVC
logging routine, but that would affect everyone.  I guess then you
would be ok with that? Does order matter with the fields wrt parsing?
I don't want to break any desktop tooling I am aware of, would we upstream

this change as well?

Just put it at the end (i.e. log_append() after the avc_dump_query() call), 
like we
do in the kernel.  Shouldn't be a problem.  Technically order shouldn't matter 
but
safer to just append it to the current fields.

Yes, we'd upstream it.


Done. Ill post back with a patch on android-review. And once merged there I can 
send one to the
Mailing list or you can cherry-pick. Are you ok with that?


Yes.  As with the kernel, only add permissive= to avc: denied messages, 
not avc: granted ones.  See the kernel logic (but note that the code and 
data structures aren't the same as the libselinux AVC anymore).


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: av_decision on audit callback

2015-10-02 Thread Stephen Smalley

On 10/02/2015 04:07 PM, Roberts, William C wrote:




-Original Message-
From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
Sent: Friday, October 2, 2015 12:12 PM
To: Roberts, William C; seandroid-l...@tycho.nsa.gov; selinux@tycho.nsa.gov
Subject: Re: av_decision on audit callback

On 10/02/2015 02:54 PM, Stephen Smalley wrote:

On 10/02/2015 02:48 PM, Roberts, William C wrote:

I would like to be able to gather the result of permissive mode per
domain from a check_access() call for the userspace object managers
on Android.

  From what I can tell check_access() calls avc_has_perm with a NULL
5th argument. That argument is for the struct avc_entry_ref.

That structure has a pointer to an opaque type, avc_entry. Which
contains struct av_decision.

Which contains flags that have a permissive flag:

struct av_decision {

  access_vector_t allowed;

  access_vector_t decided;

  access_vector_t auditallow;

  access_vector_t auditdeny;

  unsigned int seqno;

  unsigned int flags;

};

/* Definitions of av_decision.flags */

#define SELINUX_AVD_FLAGS_PERMISSIVE0x0001

It looks like if check_access just passes this structure and then
avc_has_perm() when it calls avc_audit, it could supply the
av_decision structure to the avc_suppl_audit() call. We could then
have an audit2 callback that takes this parameter.

Is this mostly right, seem sane? Better way to do this?


It doesn't need to be exposed at that level; the libselinux
avc_audit() routine can log it, similar to what is done in the kernel.
It already has the av_decision structure available to it.


To clarify, anything directly known to the AVC, like the permissive flag, can be
directly logged by it.  The audit callback is for logging auxiliary audit 
information
not known to the AVC (the pid of the client process being a good example).


I was wondering if we could just dump permissive=0|1 from the AVC logging 
routine, but that
would affect everyone.  I guess then you would be ok with that? Does order 
matter with
the fields wrt parsing? I don't want to break any desktop tooling I am aware 
of, would we upstream
this change as well?


Just put it at the end (i.e. log_append() after the avc_dump_query() 
call), like we do in the kernel.  Shouldn't be a problem.  Technically 
order shouldn't matter but safer to just append it to the current fields.


Yes, we'd upstream it.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: av_decision on audit callback

2015-10-02 Thread Stephen Smalley

On 10/02/2015 02:54 PM, Stephen Smalley wrote:

On 10/02/2015 02:48 PM, Roberts, William C wrote:

I would like to be able to gather the result of permissive mode per
domain from a check_access() call for the userspace object managers on
Android.

 From what I can tell check_access() calls avc_has_perm with a NULL 5th
argument. That argument is for the struct avc_entry_ref.

That structure has a pointer to an opaque type, avc_entry. Which
contains struct av_decision.

Which contains flags that have a permissive flag:

struct av_decision {

 access_vector_t allowed;

 access_vector_t decided;

 access_vector_t auditallow;

 access_vector_t auditdeny;

 unsigned int seqno;

 unsigned int flags;

};

/* Definitions of av_decision.flags */

#define SELINUX_AVD_FLAGS_PERMISSIVE0x0001

It looks like if check_access just passes this structure and then
avc_has_perm() when it calls avc_audit, it could supply the av_decision
structure to the avc_suppl_audit() call. We could then have an audit2
callback that takes this parameter.

Is this mostly right, seem sane? Better way to do this?


It doesn't need to be exposed at that level; the libselinux avc_audit()
routine can log it, similar to what is done in the kernel.  It already
has the av_decision structure available to it.


To clarify, anything directly known to the AVC, like the permissive 
flag, can be directly logged by it.  The audit callback is for logging 
auxiliary audit information not known to the AVC (the pid of the client 
process being a good example).


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH 1/5] selinux: introduce security_context_str_to_sid

2015-09-29 Thread Stephen Smalley

On 09/25/2015 06:34 PM, Rasmus Villemoes wrote:

There seems to be a little confusion as to whether the scontext_len
parameter of security_context_to_sid() includes the nul-byte or
not. Reading security_context_to_sid_core(), it seems that the
expectation is that it does not (both the string copying and the test
for scontext_len being zero hint at that).

Introduce the helper security_context_str_to_sid() to do the strlen()
call and fix all callers.

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>


Acked-by:  Stephen Smalley <s...@tycho.nsa.gov>


---
  security/selinux/hooks.c| 12 
  security/selinux/include/security.h |  2 ++
  security/selinux/selinuxfs.c| 26 +-
  security/selinux/ss/services.c  |  5 +
  4 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e4369d86e588..fd50cd5ac2ec 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -674,10 +674,9 @@ static int selinux_set_mnt_opts(struct super_block *sb,

if (flags[i] == SBLABEL_MNT)
continue;
-   rc = security_context_to_sid(mount_options[i],
-strlen(mount_options[i]), , 
GFP_KERNEL);
+   rc = security_context_str_to_sid(mount_options[i], , 
GFP_KERNEL);
if (rc) {
-   printk(KERN_WARNING "SELinux: security_context_to_sid"
+   printk(KERN_WARNING "SELinux: 
security_context_str_to_sid"
   "(%s) failed for (dev %s, type %s) errno=%d\n",
   mount_options[i], sb->s_id, name, rc);
goto out;
@@ -2617,15 +2616,12 @@ static int selinux_sb_remount(struct super_block *sb, 
void *data)

for (i = 0; i < opts.num_mnt_opts; i++) {
u32 sid;
-   size_t len;

if (flags[i] == SBLABEL_MNT)
continue;
-   len = strlen(mount_options[i]);
-   rc = security_context_to_sid(mount_options[i], len, ,
-GFP_KERNEL);
+   rc = security_context_str_to_sid(mount_options[i], , 
GFP_KERNEL);
if (rc) {
-   printk(KERN_WARNING "SELinux: security_context_to_sid"
+   printk(KERN_WARNING "SELinux: 
security_context_str_to_sid"
   "(%s) failed for (dev %s, type %s) errno=%d\n",
   mount_options[i], sb->s_id, sb->s_type->name, 
rc);
goto out_free_opts;
diff --git a/security/selinux/include/security.h 
b/security/selinux/include/security.h
index 6a681d26bf20..223e9fd15d66 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -166,6 +166,8 @@ int security_sid_to_context_force(u32 sid, char **scontext, 
u32 *scontext_len);
  int security_context_to_sid(const char *scontext, u32 scontext_len,
u32 *out_sid, gfp_t gfp);

+int security_context_str_to_sid(const char *scontext, u32 *out_sid, gfp_t gfp);
+
  int security_context_to_sid_default(const char *scontext, u32 scontext_len,
u32 *out_sid, u32 def_sid, gfp_t gfp_flags);

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 5bed7716f8ab..c02da25d7b63 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -731,13 +731,11 @@ static ssize_t sel_write_access(struct file *file, char 
*buf, size_t size)
if (sscanf(buf, "%s %s %hu", scon, tcon, ) != 3)
goto out;

-   length = security_context_to_sid(scon, strlen(scon) + 1, ,
-GFP_KERNEL);
+   length = security_context_str_to_sid(scon, , GFP_KERNEL);
if (length)
goto out;

-   length = security_context_to_sid(tcon, strlen(tcon) + 1, ,
-GFP_KERNEL);
+   length = security_context_str_to_sid(tcon, , GFP_KERNEL);
if (length)
goto out;

@@ -819,13 +817,11 @@ static ssize_t sel_write_create(struct file *file, char 
*buf, size_t size)
objname = namebuf;
}

-   length = security_context_to_sid(scon, strlen(scon) + 1, ,
-GFP_KERNEL);
+   length = security_context_str_to_sid(scon, , GFP_KERNEL);
if (length)
goto out;

-   length = security_context_to_sid(tcon, strlen(tcon) + 1, ,
-GFP_KERNEL);
+   length = security_context_str_to_sid(tcon, , GFP_KERNEL);
if (length)
goto out;

@@ -882,13 +878,11 @@ static ssize_t sel_write_relabel(st

Re: [PATCH 0/5] selinux: minor cleanup suggestions

2015-09-29 Thread Stephen Smalley

On 09/25/2015 06:34 PM, Rasmus Villemoes wrote:

A few random things I stumbled on.

While I'm pretty sure of the change in 1/5, I'm also confused, because
the doc for the reverse security_sid_to_context state that
@scontext_len is set to "the length of the string", which one would
normally interpret as being what strlen() would give (i.e., without
the \0). However, security_sid_to_context_core clearly includes the \0
in the return value, and I think callers rely on that.


It is historical; originally security_context_to_sid() required 
@scontext to be NUL-terminated and @scontext_len to include the NUL byte 
in the length, and security_sid_to_context() returned a NUL-terminated 
@scontext and included the NUL byte in the returned length.  However, 
when we switched SELinux to using xattrs rather than its own persistent 
label mapping, security_context_to_sid() was changed to accept contexts 
that did not already include the NUL because setfattr did not consider 
the NUL to be part of the attribute value for strings.  So presently it 
accepts either form, although we prefer them to be NUL-terminated and 
canonicalize them to that form before returning to userspace.




Rasmus Villemoes (5):
   selinux: introduce security_context_str_to_sid
   selinux: remove pointless cast in selinux_inode_setsecurity()
   selinux: use kmemdup in security_sid_to_context_core()
   selinux: use kstrdup() in security_get_bools()
   selinux: use sprintf return value

  security/selinux/hooks.c| 14 +-
  security/selinux/include/security.h |  2 ++
  security/selinux/selinuxfs.c| 26 +-
  security/selinux/ss/services.c  | 22 +-
  4 files changed, 25 insertions(+), 39 deletions(-)



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: newrole not working when built with LSPP_PRIV=y

2015-09-29 Thread Stephen Smalley

On 09/26/2015 09:10 PM, Laurent Bigonville wrote:

Hi,

Running newrole executable compiled with LSPP_PRIV=y I get the following
error while it's trying to switch role:

Error sending audit message.

It seems that the CAP_AUDIT_WRITE capability is not set [0]. Adding this
capability to the list doesn't seems enough, I then get the following error:

failed to exec shell: Operation not permitted

Looking at the fedora tree, I've found this patch[1] (which is not
merged upstream) that seems to fix both issues.

The patch seems to break an other thing, it Fedora the newrole
executable is not setuid root, but it is granted a bunch of capabilities
explicitly, if I setuid this executable instead of granting these
capabilities, I get yet an other error:

Sorry, newrole failed to drop capabilities: Operation not permitted

So I guess something need to be fixed here.


Yes, the current code just seems to be wrong here.  The setresuid() call 
will drop all capabilities if newrole is setuid-root and the caller is 
non-root, so it will end up dropping all capabilities immediately.  Then 
the attempt to further set the capabilities will fail (as above), as 
will any subsequent privileged operations.  As currently written, this 
can only work if not setuid-root and using file-caps.  And in that case, 
the setresuid() call doesn't make sense.


Dan?



Cheers,

Laurent Bigonville

[0]
https://github.com/SELinuxProject/selinux/blob/master/policycoreutils/newrole/newrole.c#L590

[1]
https://github.com/fedora-selinux/selinux/commit/339a6fed0b37f8b82e4382bc6a5c9367119ed92b


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [RFC PATCH V2] libselinux: Add selinux_restorecon function

2015-09-29 Thread Stephen Smalley

On 09/27/2015 08:06 AM, Richard Haines wrote:

The selinux_restorecon(3) man page details this function that relies
on the selabel_digest(3) function available from [1] (as not yet
part of upstream libselinux).

It has been built using the work from Android where an SHA1 hash
of the specfiles is held in an extended attribute to enhance
performance. Also contains components from policycoreutils/setfiles.

The utils/selinux_restorecon.c utility demonstrates the functionality.

[1] http://marc.info/?l=selinux=144274383217343=2

Signed-off-by: Richard Haines 
---
V2 Changes:
Added exclude_list to function, updated util to test and man page
Fixed xdev to use fts correctly.

  libselinux/include/selinux/selinux.h |  28 ++
  libselinux/man/man3/selinux_restorecon.3 | 200 ++
  libselinux/src/Makefile  |   2 +-
  libselinux/src/selinux_restorecon.c  | 443 +++
  libselinux/utils/Makefile|   2 +
  libselinux/utils/selinux_restorecon.c| 215 +++
  6 files changed, 889 insertions(+), 1 deletion(-)
  create mode 100644 libselinux/man/man3/selinux_restorecon.3
  create mode 100644 libselinux/src/selinux_restorecon.c
  create mode 100644 libselinux/utils/selinux_restorecon.c

diff --git a/libselinux/include/selinux/selinux.h 
b/libselinux/include/selinux/selinux.h
index 4beb170..c5228a6 100644
--- a/libselinux/include/selinux/selinux.h
+++ b/libselinux/include/selinux/selinux.h
@@ -663,6 +663,34 @@ extern int selinux_lsetfilecon_default(const char *path);
   */
  extern void selinux_reset_config(void);

+
+/* Force the checking of labels even if the stored SHA1
+ * digest matches the specfiles SHA1 digest. */
+#define SELINUX_RESTORECON_FORCE_CHECK 1


Why not just FORCE?  Or, alternatively, IGNORE_DIGEST.


+/* Do not change file labels */
+#define SELINUX_RESTORECON_NOCHANGE2
+/* If set change the files label to that in spec file.
+ * If not set only change the type component to that in spec file. */
+#define SELINUX_RESTORECON_CHANGE_USERROLETYPELEVEL4


Isn't this just part of restorecon -F upstream, and thus can be part of 
a FORCE option flag that is shared with the one below?



+/* Reset customizable types */
+#define SELINUX_RESTORECON_CHANGE_CUSTOMIZABLE 8
+/* Recurse through the directory path */


Recursively descend directories.


+#define SELINUX_RESTORECON_RECURSE 16
+/* Log changes and show specfiles SHA1 digest */
+#define SELINUX_RESTORECON_VERBOSE 32
+/* Set selabel_open(3) option SELABEL_OPT_VALIDATE */


Validate all contexts in the file_contexts file up front (not lazily on 
use).



+#define SELINUX_RESTORECON_VALIDATE64
+/* Convert passed-in pathname to canonical pathname */
+#define SELINUX_RESTORECON_REALPATH128


Note: This logic has changed recently in Android libselinux as it did 
not correctly handle the case of calling restorecon on a symlink with 
the intent of relabeling that symlink.  matchpathcon has a different yet 
similar approach via realpath_not_final(), but I think Android's 
implementation is probably more correct.



+/* Prevent descending into directories that have a different
+ * device number than the pathname from which the descent began */
+#define SELINUX_RESTORECON_XDEV256
+
+extern int selinux_restorecon(const char **pathname_list,
+   const char **exclude_list,
+   const char *fc_path,
+   unsigned int restorecon_flags);


This is a more cumbersome interface for typical users than the Android one.


+
  #ifdef __cplusplus
  }
  #endif
diff --git a/libselinux/man/man3/selinux_restorecon.3 
b/libselinux/man/man3/selinux_restorecon.3
new file mode 100644
index 000..3dfdbd3
--- /dev/null
+++ b/libselinux/man/man3/selinux_restorecon.3
@@ -0,0 +1,200 @@
+.TH "selinux_restorecon" "3" "22 Sept 2015" "Security Enhanced Linux" "SELinux API 
documentation"
+
+.SH "NAME"
+selinux_restorecon \- restore file(s) default SELinux security contexts
+.
+.SH "SYNOPSIS"
+.B #include 
+.sp
+.BI "int selinux_restorecon(const char **" pathname_list ,
+.in +\w'int selinux_restorecon('u
+.BI "const char **" exclude_list ,
+.br
+.BI "const char *" fc_path ,
+.br
+.BI "unsigned int " restorecon_flags ");"
+.in
+.
+.SH "DESCRIPTION"
+.BR selinux_restorecon ()
+restores file default security contexts based on:
+.sp
+.RS
+.IR pathname_list
+containing a
+.B NULL
+terminated list of one or more directories or files to be relabeled.
+.br
+If the
+.IR restorecon_flags
+.B SELINUX_RESTORECON_RECURSE
+has been set (for decending through directories), then for each
+.IR pathname_list
+entry specified
+.BR selinux_restorecon ()
+will write an SHA1 digest of the combined specfiles (see the
+.B NOTES
+section for details) to an 

Re: [RFC PATCH v1 1/3] lsm: introduce hooks for kdbus

2015-09-24 Thread Stephen Smalley
On 09/23/2015 05:44 PM, Paul Moore wrote:
> Add LSM access control hooks to kdbus; several new hooks are added and
> the existing security_file_receive() hook is reused.  The new hooks
> are listed below:
> 
>  * security_kdbus_conn_new
>Check if the current task is allowed to create a new kdbus
>connection.
>  * security_kdbus_own_name
>Check if a connection is allowed to own a kdbus service name.
>  * security_kdbus_conn_talk
>Check if a connection is allowed to talk to a kdbus peer.
>  * security_kdbus_conn_see
>Check if a connection can see a kdbus peer.
>  * security_kdbus_conn_see_name
>Check if a connection can see a kdbus service name.
>  * security_kdbus_conn_see_notification
>Check if a connection can receive notifications.
>  * security_kdbus_proc_permission
>Check if a connection can access another task's pid namespace info.
> 
> Signed-off-by: Paul Moore 
> ---
>  include/linux/security.h |  113 
> ++
>  ipc/kdbus/connection.c   |   73 --
>  ipc/kdbus/message.c  |   19 ++--
>  ipc/kdbus/metadata.c |6 +-
>  security/security.c  |   45 ++
>  5 files changed, 223 insertions(+), 33 deletions(-)
> 

> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
> index ef63d65..be8d210 100644
> --- a/ipc/kdbus/connection.c
> +++ b/ipc/kdbus/connection.c
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -213,6 +214,13 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep 
> *ep,
>   goto exit_unref;
>   }
>  
> + ret = security_kdbus_conn_new(conn->cred, creds, pids, seclabel,
> +   owner, privileged,
> +   is_activator, is_monitor,
> +   is_policy_holder);
> + if (ret < 0)
> + goto exit_unref;
> +
>   /*
>* Account the connection against the current user (UID), or for
>* custom endpoints use the anonymous user assigned to the endpoint.

It occurs to me that we aren't passing any information about the
endpoint or bus here.  conn->cred will be file->f_cred.  That will
generally be the same as the current cred since the process will have
opened the endpoint file in kdbusfs and then invoked ioctl with the
KDBUS_CMD_HELLO command on that open file.  So if I do a check against
that cred, it will always be to my own label, not to the actual creator
of the bus or the endpoint.

In comparison, kdbus_ep_is_owner(), which is called by this function,
checks file->f_cred->euid against ep->bus->node.uid.  Unfortunately the
kdbus_node struct does not contain a complete cred, just uid/gid/mode
information that gets used in various places, including for setting up
the pseudo file in kdbusfs that is used to access the bus or endpoint.
That seems to be kdbus' basic means of isolating different users; by
default, any bus or endpoint I create will only be accessible by my own
uid unless I choose to make it group or world accessible. current_euid()
and current_egid() are saved in the nodes when the bus or endpoint is
created, and later used in populating the inode and for uid comparisons.

If we want the same for MAC, I guess we either need kdbus_node to hold a
ref to a cred (and then we can pass ep->node->cred to the hooks), or
just add our own security field to kdbus_node.  The former seems cleaner
to me; we can then just take an additional reference to the bus or
endpoint creator's cred at creation time.  And then we need kdbusfs to
call a new hook on the inode and the cred in order to set the
inode->i_security to something appropriate for the bus or endpoint
creator.  Otherwise, we don't get any control over the ability to open
any given endpoint or bus in kdbusfs, as that is only subject to the
inode permission checks.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: MAP_STACK and execstack

2015-10-05 Thread Stephen Smalley

On 10/02/2015 04:44 PM, Nick Kralevich wrote:

Currently, SELinux implements the "execstack" capability using the
following code:

security/selinux/hooks.c
function: selinux_file_mprotect()

} else if (!vma->vm_file &&
   vma->vm_start <= vma->vm_mm->start_stack &&
   vma->vm_end >= vma->vm_mm->start_stack) {
 rc = current_has_perm(current, PROCESS__EXECSTACK);
}

However, at least on Android, this check doesn't work for pthread
allocated stacks. Those stacks are allocated in libc via mmap(), and
aren't accounted for in the kernel as stack pages. As a result,
attempting to mprotect(PROT_EXEC) a pthread stack page requires the
"execmem" permission, not the "execstack" permission.

"man mmap" defines MAP_STACK, which is currently a no-op in the kernel
indicating that the memory is intended to be used as a stack. In
theory, Android's libc could set this flag for memory intended to be
used as a stack, but doing so is useless if the kernel ignores it.

Is there any reason why SELinux shouldn't use MAP_STACK to determine
whether the execmem or execstack capability is checked? In Android,
this would be a net security win, since nobody is granted execstack
today.


To clarify, execstack is a check on mprotect() in addition to (not 
instead of) execmem when the mapping is part of the stack.  execmem is 
always required.


At the time execstack was added, /proc/pid/maps / show_map_vma only 
identified the main process stack and the same logic was used to 
identify it for the execstack check.


Since that time, /proc/pid/maps has been augmented to also identify 
thread stacks, so the logic from show_map_vma could be reused for the 
execstack check as well.


Using MAP_STACK as the basis for the check seems less desirable, as it 
is entirely at the discretion of the program (so can be easily omitted, 
thereby making the check opt-in), is only passed on mmap() not 
mprotect() and doesn't appear to be saved anywhere, so it isn't 
available on mprotect() calls, and is ignored by the kernel.  The commit 
that added it said:


commit cd98a04a59e2f94fa64d5bf1e26498d27427d5e7
Author: Ingo Molnar 
Date:   Wed Aug 13 18:02:18 2008 +0200

x86: add MAP_STACK mmap flag

as per this discussion:

   http://lkml.org/lkml/2008/8/12/423

Pardo reported that 64-bit threaded apps, if their stacks exceed the
combined size of ~4GB, slow down drastically in pthread_create() - 
because

glibc uses MAP_32BIT to allocate the stacks. The use of MAP_32BIT is
a legacy hack - to speed up context switching on certain early model
64-bit P4 CPUs.

So introduce a new flag to be used by glibc instead, to not constrain
64-bit apps like this.

glibc can switch to this new flag straight away - it will be ignored
by the kernel. If those old CPUs ever matter to anyone, support for
it can be implemented.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Performance issues - huge amount of AVC misses

2015-12-08 Thread Stephen Smalley

On 12/08/2015 05:25 AM, Michal Marciniszyn wrote:

Hello,

we are heavy SELinux shop and we recently run into AVC related
performance issue. I was trying to find an answer on freenode IRC chat
but I was sent here by multiple guys. We're running on Scientific Linux
6.6 (upgrade to 6.7 ongoing) and we see this on some of our nodes:

# cat /selinux/avc/cache_stats
lookups hits misses allocations reclaims frees
3976846641 3626568307 350278334 350303465 344833264 346344169
3474274460 3092218096 382056364 382081270 381170512 382671551
2037181411 1655679702 381501709 381527148 380680320 382162477
1943162363 1651603455 291558908 291584892 288099840 289631602
829213467 406079951 423133516 423158604 422311024 423847681
1963015875 1555848944 407166931 407192104 406718592 408227742
3490131033 3117047653 373083380 373108386 372270880 373862706
940880689 549698684 391182005 391207388 390339328 391888374
4098417807 3712068859 386348948 386373592 385604096 387172806
3931378773 3549502965 381875808 381901074 381059904 382628308


FWIW, avcstat would summarize that for you.
Those stats seem very unusual.  You said you see this on some nodes. 
Anything to distinguish these nodes from the others that don't exhibit 
this behavior?




Also we see

# cat /selinux/avc/hash_stats
entries: 499
buckets used: 257/512
longest chain: 6

Some times under load we see SELinux consuming about 30% of CPU time.
There is about 16% of cache misses on these nodes (and sometimes it goes
as high as 30%). The lates article about the issue is from RHEL 5 times
-
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/4/html/SELinux_Guide/rhlcommon-section-0102.html
. We do not feel this to be too relevant in this case.

Are there any recommendations on cache sizing for SELinux? We can resize
cache to 1024 or 2048 entries, but would this help to resolve the issue?


Yes, increasing the cache threshold should help as you are evidently 
thrashing the cache.



I'm attaching seinfo from node with our policy and then for comparison
from node without any policy.


What do you mean by "our policy" versus "without any policy"?  Do you 
mean that the former has some local policy modules that you have added 
and the latter is the stock SL6.6 policy?



With policy:
# seinfo

Statistics for policy file: /etc/selinux/targeted/policy/policy.24
Policy Version & Type: v.24 (binary, mls)

Classes:81Permissions:   238
Sensitivities:   1Categories:   1024
Types:4273Attributes:295
Users:   9Roles:  12
Booleans:  234Cond. Expr.:   274
Allow:  352554Neverallow:  0
Auditallow:140Dontaudit:  321786
Type_trans:  42813Type_change:38
Type_member:48Role allow: 19
Role_trans:409Range_trans:  6421
Constraints:90Validatetrans:   0
Initial SIDs:   27Fs_use: 23
Genfscon:   84Portcon:   505
Netifcon:0Nodecon: 0
Permissives:91Polcap:  2



Without policy:

seinfo

Statistics for policy file: /etc/selinux/targeted/policy/policy.24
Policy Version & Type: v.24 (binary, mls)

Classes:81Permissions:   238
Sensitivities:   1Categories:   1024
Types:3926Attributes:295
Users:   9Roles:  12
Booleans:  234Cond. Expr.:   274
Allow:  320969Neverallow:  0
Auditallow:140Dontaudit:  273256
Type_trans:  41915Type_change:38
Type_member:48Role allow: 19
Role_trans:386Range_trans:  6069
Constraints:90Validatetrans:   0
Initial SIDs:   27Fs_use: 23
Genfscon:   84Portcon:   479
Netifcon:0Nodecon: 0
Permissives:91Polcap:  2


Any help or guidance would be very much appreciated, if there is more
in-depth info needed I'll be more than happy to provide it.



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Performance issues - huge amount of AVC misses

2015-12-08 Thread Stephen Smalley

On 12/08/2015 09:56 AM, Michal Marciniszyn wrote:

Hi Dominic,

while there is quite a lot of dontaudit rules around, the amount for
domains running on this node is not high. Is there any way how to
monitor which rules are loaded and released from the cache? Anything
better than plain aggregated stats? We would bot care about performance
of such monitoring tool if it provides some useful answer. For instance,
is there a way how to use system tap or similar kernel profiling to get
the data?

I'll do a profiling on how many rules actually apply for the domains on
the node (i.e. use sesearch to find out). If doing so, does the rule in
cache hold whole vector (i.e. A is allowed to do X, Y, Z on B or is one
cache entry A can do X on B)?


One cache entry holds the entire access vector.  However, they are 
unique per (source context, target context, target class) triple.


Are you using categories on this system (i.e. running processes in 
specific category sets, assigning specific categories to files), or just 
types?


How many unique domains are running in your workload?  How many file 
types are typically accessed by your workload?  How many different kinds 
of files (regular, directory, symbolic link, block device, ...) are part 
of your workload?




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: continuation of systemd/SELinux discussion from Github

2015-12-02 Thread Stephen Smalley

On 12/02/2015 05:18 AM, Dominick Grift wrote:

Let's continue the discussion here.

The last answered questionnaire is below, any further questions or
comments?:



 "systemd --user" concept is broken as we can see/read from this
 thread from SELinux point of view.

It was working fine except that it was trying to log to the audit system
which unprivileged processes arent allowed to do.


What's the dbus solution for this issue?



 Are we able truly explain the enforcement here?

I think i was able to at least identify some of it, but i probably have
overlooked other compelling arguments:

Any processes that needs to be able to "control" any system-wide systemd
user unit, will be able to "control" all system-wide systemd user units.

In practice that means that a process may be able to for example start a
application even though it does not have access to execute that
application by telling systemd --user do start that applications on its
behalf


Can systemd-run --user be used to escape any of the following:
a) SELinux sandbox,
b) libvirt / svirt confinement,
c) newrole -r to a different, more restricted role or -l to a different 
level,

d) runcon to a more restricted domain.

If so, then there is a problem with systemd --user not applying its own 
access controls over its operations.




A process may, for example, be able to use this to stop a sensitive
process bypassing SELinux access control

 And the point is if we have big benefits of this level of
 isolation?

With access control the benefits often depend on the requirements. but
in general since selinux touts flexibility and one of the big benefits
here is that we can contain access of processes to units actions.

without this processes are potentially able to bypass selinux by telling
systemd --user to do stuff on their behalf

 Do we have real examples how it prevents possible security
 flaws?

No, and why should we? SELinux does not prevent possible security
flaws. SELinux is Linux access control extension
It enforces integrity and optional confidentiality or
comparmentalization.

 Do we talk about confined desktop here?

No we are talking about a set of components that without SELinux access
control extension potentionally allows processes
to bypass SELinux access control restrictions in some cases

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] policycoreutils: fix 'semanage permissive -l' subcommand

2015-12-01 Thread Stephen Smalley

On 11/30/2015 08:57 AM, Petr Lautrbach wrote:

This reverts the commit 97d06737 which introduced a regression on '-l'
which started to require at least one argument and fixes the original
problem other way. A args.parser value is set now and handlePermissive
function uses it to print an usage message when args.type is not set.

Fixes: semanage permissive -l
   usage: semanage permissive [-h] (-a | -d | -l) [-n] [-N] [-S STORE]
  type [type ...]
   semanage permissive: error: the following arguments are required: type

Signed-off-by: Petr Lautrbach 


Thanks, applied.


---
  policycoreutils/semanage/semanage | 16 +++-
  1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/policycoreutils/semanage/semanage 
b/policycoreutils/semanage/semanage
index ed48c11..7489955 100644
--- a/policycoreutils/semanage/semanage
+++ b/policycoreutils/semanage/semanage
@@ -700,12 +700,17 @@ def handlePermissive(args):
  OBJECT = object_dict['permissive']()
  OBJECT.set_reload(args.noreload)

-if args.action is "add":
-OBJECT.add(args.type)
  if args.action is "list":
  OBJECT.list(args.noheading)
-if args.action is "delete":
-OBJECT.delete(args.type)
+elif args.type != None:
+if args.action is "add":
+OBJECT.add(args.type)
+if args.action is "delete":
+OBJECT.delete(args.type)
+else:
+args.parser.print_usage(sys.stderr)
+sys.stderr.write(_('semanage permissive: error: the following argument 
is required: type\n'))
+sys.exit(1)


  def setupPermissiveParser(subparsers):
@@ -721,8 +726,9 @@ def setupPermissiveParser(subparsers):
  parser_add_noheading(permissiveParser, "permissive")
  parser_add_noreload(permissiveParser, "permissive")
  parser_add_store(permissiveParser, "permissive")
-permissiveParser.add_argument('type', nargs='+', default=None, 
help=_('type'))
+permissiveParser.add_argument('type', nargs='?', default=None, 
help=_('type'))
  permissiveParser.set_defaults(func=handlePermissive)
+permissiveParser.set_defaults(parser=permissiveParser)


  def handleDontaudit(args):



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: continuation of systemd/SELinux discussion from Github

2015-12-02 Thread Stephen Smalley

On 12/02/2015 02:47 PM, Dominick Grift wrote:

On Wed, Dec 02, 2015 at 01:20:30PM -0500, Stephen Smalley wrote:

On 12/02/2015 05:18 AM, Dominick Grift wrote:

Let's continue the discussion here.

The last answered questionnaire is below, any further questions or
comments?:



 "systemd --user" concept is broken as we can see/read from this
 thread from SELinux point of view.

It was working fine except that it was trying to log to the audit system
which unprivileged processes arent allowed to do.



What's the dbus solution for this issue?


Here is some chatter about the above i believe:

https://bugs.freedesktop.org/show_bug.cgi?id=83856


The code is easier to follow. bus/audit.c:bus_audit_init() will only 
open the audit socket if the process has CAP_AUDIT_WRITE. 
bus/selinux.c:log_callback() will only try to send the message to audit 
if the audit socket was opened, but will always send the message to 
syslog regardless.  Thus, if the dbus-daemon instance has 
CAP_AUDIT_WRITE, you get the messages logged to audit and syslog; 
otherwise, you only get them in syslog.  This is the most general 
approach, as it supports both privileged and unprivileged dbus-daemon 
instances, including the case where the system-wide bus is running in a 
container and does not have CAP_AUDIT_WRITE (even though it thinks it is 
root / uid 0) and the case where the session bus has CAP_AUDIT_WRITE (if 
one were to set cap_audit_write in the file capabilities on the 
dbus-daemon executable, not recommended but possible), as well as the 
more common cases of a system-wide bus with CAP_AUDIT_WRITE and a 
session bus without it.  systemd should likely do something similar, but 
using the journal.







 Are we able truly explain the enforcement here?

I think i was able to at least identify some of it, but i probably have
overlooked other compelling arguments:

Any processes that needs to be able to "control" any system-wide systemd
user unit, will be able to "control" all system-wide systemd user units.

In practice that means that a process may be able to for example start a
application even though it does not have access to execute that
application by telling systemd --user do start that applications on its
behalf



Can systemd-run --user be used to escape any of the following:
a) SELinux sandbox,
b) libvirt / svirt confinement,
c) newrole -r to a different, more restricted role or -l to a different
level,
d) runcon to a more restricted domain.



If so, then there is a problem with systemd --user not applying its own
access controls over its operations.



I cannot answer above currently. I can tell you that obviously one needs
to be able to talk to ones systemd --user instance to be able to tell it
to do anything on ones behalf obviously. So by default all of the above
will likely not work, since they most likely do not have the permissions
to talk to systemd --user even if they were able to execute systemctl
and/or systemd-run.

But even then, things also depend on whether systemd --user runs in a
private domain or the login user domain (in my policy it runs in a
private domain except for unconfined users)

Those are good questions but i do not see how they are directly related
to the question whether systemd --user should be a selinux user space object
manager or not (in my view it obviously should but i am not trusted by
systemd maintainers, walsh is trusted and walsh gave systemd maintainers
the impression that systemd --user does not have to be an selinux object
manager) I strongly suspect that is wrong.


I cited those examples because they were applicable to the stock Fedora 
policy and, if valid, would provide a rationale for restoring the 
controls regardless of what one might believe about confining desktop 
applications because they would indicate vulnerabilities or regressions 
in functionality provided by Fedora.  That seemed more likely to be 
convincing to the Fedora SELinux maintainers.


For sandbox, they don't appear to allow it to connect in the first place:

$ sandbox /bin/bash
bash-4.3$ systemd-run --user id
Failed to create bus connection: Permission denied

So there the systemd access controls wouldn't come into play.

For confined user roles, systemd-run --user  failed on Fedora 
22 with:


Failed to start transient service unit: Access denied

and journalctl showed:

systemd[15007]: Can't send to audit system: USER_AVC avc:  denied  { 
start } for auid=N uid=N gid=N 
path="/run/user/N/systemd/user/run-PID.service" cmdline="systemd-run 
--user id" scontext=staff_u:staff_r:staff_t:s0-s0:c0.c1023 
tcontext=system_u:object_r:user_tmp_t:s0 tclass=service


So removing the systemd --user controls is a regression in the 
protection being provided in Fedora, IIUC, although I'll let the Fedora 
SELinux maintainers speak to that.

___
Selinux maili

Re: Performance issues - huge amount of AVC misses

2015-12-09 Thread Stephen Smalley

On 12/09/2015 08:15 AM, Michal Marciniszyn wrote:

Hi,

after increasing the cache, I do not see many reclaims, like couple of
them here and there. The cache size had to be increased to 2048 to get
ti this state.

# avcstat 15

 537645 537623 22 22 32 32
 942916 942912  4  4  0  0
 604466 604457  9  9  0  0
 451737 451730  7  7 16 16
 457669 457669  0  0  0  0
 519135 519133  2  2  0  0
 517288 517288  0  0  0  0
 380376 380376  0  0  0  0
 464272 464269  3  3  0  0
 531484 531482  2  2  0  0
14224221422421  1  1  0  0
13809321380932  0  0  0  0
 512999 512999  0  0  0  0


Is it ok if I get longest chain length 13 in hash stats (It was higher
in the beginning - 19, but got to 13 after 2 hours)?


I wouldn't worry about that; it's insignificant compared to the cost of 
an AVC miss.




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: chcat is using getlogin() function that sometimes returns null/empty string

2015-12-07 Thread Stephen Smalley

On 12/07/2015 01:01 PM, Laurent Bigonville wrote:

Hi,

So apparently gnome-terminal developers have decided to stop updating
utmp[0] file and this is breaking chcat -Ll with the following error:

Traceback (most recent call last):
   File "/usr/bin/chcat", line 409, in 
 sys.exit(listusercats(cmds))
   File "/usr/bin/chcat", line 352, in listusercats
 users.append(os.getlogin())
OSError: [Errno 2] No such file or directory

getlogin(3) manpage suggest to rely on the LOGNAME environment variable,
so maybe chcat should fallback to this if getlogin() is not returning
anything?

Cheers,

Laurent Bigonville

[0] https://bugzilla.gnome.org/show_bug.cgi?id=747046


Maybe just use:
pwd.getpwuid(os.getuid()).pw_name



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Exposing secid to secctx mapping to user-space

2015-12-11 Thread Stephen Smalley

On 12/11/2015 02:55 PM, Paul Moore wrote:

On Fri, Dec 11, 2015 at 1:37 PM, Daniel Cashman  wrote:

Hello,

I would like to write a patch that would expose, via selinuxfs, the
mapping between secids in the kernel and security contexts to
user-space, but before doing so wanted to get some feedback as to
whether or not such an endeavor could have any support upstream.  The
direct motivation for this is the desire to communicate calling security
ids/contexts over binder IPC on android for use in a user-space object
manager.  Passing the security ids themselves would be simpler and more
efficient in the critical kernel path, but they currently have no
user-space meaning.


In general we try to avoid exposing the secid tokens outside the
kernel, I view them as an implementation hack designed to make it
easier to manage and operate on the security labels in the kernel.  I
suspect you will hear something very similar from Casey and the other
Smack developers.  Another consideration is the long standing LSM
stacking effort, they have several good reasons for wanting to abolish
the secid token, propagating it to userspace would make that all but
impossible.

While I'm sympathetic to your desire for less complexity and better
performance in passing security labels, from a kernel perspective I
think we lose too much in exporting the secid tokens outside the LSM.


To clarify, security identifiers were by design provided in the Flask 
architecture and SELinux as lightweight, non-persistent handles to 
security contexts, and exposed to userspace by the original SELinux API 
(pre-2.6, of course).  They were only removed when we transitioned to 
using extended attributes and the xattr API for file security labels, 
and we made all of the other APIs consistent as passing context strings 
seemed acceptable for proc and selinuxfs as well.  There was some 
thought to turning SIDs into proper reference-counted objects or even 
wrapping them with descriptors so that their lifecycles could be fully 
managed by the kernel, but that never happened.


Passing a security context string on every binder IPC may be too costly 
from a performance point of view (numbers would be helpful here).  I 
think this situation differs from that of stream sockets (i.e. 
getsockopt SO_PEERSEC), since they are looking at enabling passing of 
sender security label for every binder IPC (not just specific 
applications) and since binder IPC is quite different from stream socket 
IPC in its semantics and its performance.


Perhaps we could provide a new fixed-size tokenized version of the 
security context string for export to userspace that could be embedded 
in the binder transaction structure?  This could avoid both the 
limitations of the current secid (e.g. limited to 32 bits, no 
stackability) and the overhead of copying context strings on every IPC.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Exposing secid to secctx mapping to user-space

2015-12-15 Thread Stephen Smalley

On 12/15/2015 11:06 AM, Casey Schaufler wrote:

On 12/15/2015 7:00 AM, Stephen Smalley wrote:

On 12/14/2015 05:57 PM, Roberts, William C wrote:




If I understand correctly, the goal here is to avoid the lookup from
pid to context. If we somehow Had the context or a token to a context
during the ipc transaction to userspace, we could just use that In
computing the access decision. If that is correct, then since we have
PID, why not just extend the SE Linux compute av decision interface to support

passing of PID and then it can do the lookup in the Kernel?

That's no less racy than getpidcon().



I got a bounce from when I sent this from gmail, resending.

True, but in this case the binder transaction would be dead...

Why not just pass ctx? It's less than ideal, but it might be good enough for 
now until contexts get unwieldy big.

grep -rn '^type ' * | grep domain | cut -d' ' -f 2-2 | sed s/','//g | awk ' {  
thislen=length($0); printf("%-5s %dn", NR, thislen); totlen+=thislen}
END { printf("average: %dn", totlen/NR); } '

The avg type length for domain types in external/sepolicy is 7. Add the full 
ctx:

u:r:xxx:s0(cat)

1. We're looking at like 18 or so bytes, how do we know this won't be "fast 
enough"?
2. What's the current perf numbers, and what's agreed upon on what you need to 
hit to be fast enough?
3. I'm assuming the use case is in service manager, but would a userspace cache 
of AVD's help? Then you can (possibly) avoid both kernel trips, and you can 
invalidate the cache on policy reload and on PID deaths? In the case of service 
manager would it always be a miss based on usage pattern? I'm assuming things 
would say hey resolve this once, and be done. However, you could only do the 
ctx lookup and do the avd based on the policy in user space, thus avoiding 1 of 
two trips.


1. I don't think it is the size of the context that is the concern but rather 
the fact that it is a variable-length string, whereas current binder commands 
use fixed-size arguments and encode the size in the command value (common for 
ioctls).  Supporting passing a variable-length string would be a change to the 
protocol and would complicate the code.  On the performance side, it means 
generating a context string from the secid and then copying it to userspace on 
each IPC (versus just getting the secid and putting that in the existing 
binder_transaction_data that is already copied to userspace).


I have long wondered why SELinux generates the context string
of the secid more than once. Audit performance alone would
justify keeping it around. The variable length issue isn't
so difficult as you make it out. As William pointed out earlier,
most SELinux contexts are short. Two protocols, one with a
fixed length of 16 chars (typical is 7) and one with a fixed
length of 256 (for abnormal contexts) solve the problem without
complicating the code hardly at all.

If it's such a problem, why do we have SO_PEERSEC return a
variable length string? That's been around forever and seems
to work just fine.


Generating an audit record means you are already on the slow path, so 
adding the cost of generating a context at that point shouldn't be 
significant (but if one has performance data to the contrary, that would 
always be of interest). Keeping complete context strings in kernel 
memory at all times wasn't viewed as desirable, particularly when you 
consider the next point.


There is no internal limit on SELinux context size and one can in fact 
generate a SELinux context string that exceeds 256 bytes (just create a 
category set with many different, non-contiguous categories, e.g. a 
category set with every other category), so you can't just pick a couple 
of sizes (16 and 256 in your example) and be done with it.  It may be 
unusual to exceed 256 but it isn't impossible, and users of MLS systems 
have in fact run up against even the page size limitations of the 
/proc/self/attr interfaces as a pain point.


getsockopt SO_PEERSEC, as I mentioned earlier, is for stream sockets, so 
you are dealing with a connection-oriented model (with its attendant 
baseline overhead), server applications opt into getting the security 
label after the connection is established via getsockopt (which means 
that the overhead is not imposed on all connections, and which provides 
a straightforward way of passing variable-length data already), and all 
of the payload data is typically being copied through the kernel (to and 
from socket buffers).  With binder, we are talking about a synchronous 
IPC mechanism where the kernel copies the sender information (currently 
PID and UID) to the receiver on every IPC, the protocol currently passes 
no variable-length data, and the data is copied directly from the 
sender's memory into kernel-managed buffers that are mapped read-only 
into the receiver.  So introducing variable-length string copy to binder 
is a more substantial change, and the baseline perform

Re: Exposing secid to secctx mapping to user-space

2015-12-14 Thread Stephen Smalley

On 12/14/2015 04:29 PM, Roberts, William C wrote:



Subject: Re: Exposing secid to secctx mapping to user-space

On 12/13/2015 2:06 PM, Paul Moore wrote:

On Friday, December 11, 2015 05:14:38 PM Stephen Smalley wrote:

Perhaps we could provide a new fixed-size tokenized version of the
security context string for export to userspace that could be
embedded in the binder transaction structure?  This could avoid both
the limitations of the current secid (e.g. limited to 32 bits, no
stackability) and the overhead of copying context strings on every IPC.

On Friday, December 11, 2015 04:24:48 PM Casey Schaufler wrote:

How about this: Provide an alias mechanism for secctx. There would
then be a secid (32bits) a secctx (arbitrary text string) and a
secalias which could be a limited string of some length. You could
use the alias in place of the secctx anywhere you liked.

My initial reaction to the secalias idea isn't overly positive.  It
seems like a kludge with a lot of duplication, both in terms of code
and concept, and a lot of risk for confusion both by users and policy
writers.  I think if we really wanted to limit the security label
string format to a small size we should have done that from the start, it's too

late now.

The alias would be a user space controlled mapping. The kernel code would only
be involved at the border. I would never expect policy to be written using 
aliases.
As for being a kludge, yeah, there's some of that, but I think that's true with 
the
secid, too.


Assuming we see some binder performance numbers, and the numbers are
bad, I'm a little more open to doing something with the secid token.
Up to this point we haven't made any guarantees about the token and we
haven't exported it outside the kernel so there is some ability to change it to 
fit

our needs.

Granted, this isn't perfect solution either, and perhaps ultimately we
would need something else, but I think it is worth looking into this
first before we introduce another string label.


I agree with getting numbers before someone dashes off to make a premature
optimization that exposes secids. If the numbers are bad I would hope that the
developers would look at fixing binder rather than exposing (and forever
requiring) secids.



If I understand correctly, the goal here is to avoid the lookup from pid to 
context. If we somehow
Had the context or a token to a context during the ipc transaction to 
userspace, we could just use that
In computing the access decision. If that is correct, then since we have PID, 
why not just extend the
SE Linux compute av decision interface to support passing of PID and then it 
can do the lookup in the
Kernel?


That's no less racy than getpidcon().

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Exposing secid to secctx mapping to user-space

2015-12-15 Thread Stephen Smalley

On 12/14/2015 05:57 PM, Roberts, William C wrote:




If I understand correctly, the goal here is to avoid the lookup from
pid to context. If we somehow Had the context or a token to a context
during the ipc transaction to userspace, we could just use that In
computing the access decision. If that is correct, then since we have
PID, why not just extend the SE Linux compute av decision interface to support

passing of PID and then it can do the lookup in the Kernel?

That's no less racy than getpidcon().



I got a bounce from when I sent this from gmail, resending.

True, but in this case the binder transaction would be dead...

Why not just pass ctx? It's less than ideal, but it might be good enough for 
now until contexts get unwieldy big.

grep -rn '^type ' * | grep domain | cut -d' ' -f 2-2 | sed s/','//g | awk ' {  
thislen=length($0); printf("%-5s %dn", NR, thislen); totlen+=thislen}
END { printf("average: %dn", totlen/NR); } '

The avg type length for domain types in external/sepolicy is 7. Add the full 
ctx:

u:r:xxx:s0(cat)

1. We're looking at like 18 or so bytes, how do we know this won't be "fast 
enough"?
2. What's the current perf numbers, and what's agreed upon on what you need to 
hit to be fast enough?
3. I'm assuming the use case is in service manager, but would a userspace cache 
of AVD's help? Then you can (possibly) avoid both kernel trips, and you can 
invalidate the cache on policy reload and on PID deaths? In the case of service 
manager would it always be a miss based on usage pattern? I'm assuming things 
would say hey resolve this once, and be done. However, you could only do the 
ctx lookup and do the avd based on the policy in user space, thus avoiding 1 of 
two trips.


1. I don't think it is the size of the context that is the concern but 
rather the fact that it is a variable-length string, whereas current 
binder commands use fixed-size arguments and encode the size in the 
command value (common for ioctls).  Supporting passing a variable-length 
string would be a change to the protocol and would complicate the code. 
 On the performance side, it means generating a context string from the 
secid and then copying it to userspace on each IPC (versus just getting 
the secid and putting that in the existing binder_transaction_data that 
is already copied to userspace).


2. Don't know; deferring to Daniel to run whatever binder IPC benchmarks 
might exist with and without the current patch that copies the context 
string.


3. It is for any binder-based service that wants to apply SELinux access 
checks, which presently includes servicemanager and keystore.
We already have a userspace AVC (in libselinux) that gets used 
automatically when you use selinux_check_access(), but you still need to 
get the sender security context in some manner.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] selinux: Inode label revalidation performance fix

2016-01-06 Thread Stephen Smalley

On 01/05/2016 05:12 PM, Andreas Gruenbacher wrote:

Commit 5d226df4 has introduced a performance regression of about
10% in the UnixBench pipe benchmark.  It turns out that the call
to inode_security in selinux_file_permission can be moved below
the zero-mask test and that inode_security_revalidate can be
removed entirely, which brings us back to roughly the original
performance.

Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>


Acked-by:  Stephen Smalley <s...@tycho.nsa.gov>


---
  security/selinux/hooks.c | 10 ++
  1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 40e071a..f8110cf 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -273,11 +273,6 @@ static int __inode_security_revalidate(struct inode *inode,
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;
@@ -3277,19 +3272,19 @@ static int selinux_file_permission(struct file *file, 
int mask)
  {
struct inode *inode = file_inode(file);
struct file_security_struct *fsec = file->f_security;
-   struct inode_security_struct *isec = inode_security(inode);
+   struct inode_security_struct *isec;
u32 sid = current_sid();

if (!mask)
/* No permission to check.  Existence test. */
return 0;

+   isec = inode_security(inode);
if (sid == fsec->sid && fsec->isid == isec->sid &&
fsec->pseqno == avc_policy_seqno())
/* No change since file_open check. */
return 0;

-   inode_security_revalidate(inode);
return selinux_revalidate_file_permission(file, mask);
  }

@@ -3595,7 +3590,6 @@ static int selinux_file_open(struct file *file, const 
struct cred *cred)
 * new inode label or new policy.
 * This check is not redundant - do not remove.
 */
-   inode_security_revalidate(file_inode(file));
return file_path_has_perm(cred, file, open_file_to_av(file));
  }




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Labeling nsfs filesystem

2016-01-07 Thread Stephen Smalley

On 01/07/2016 03:36 PM, Nicolas Iooss wrote:

Hello,

Since Linux 3.19 targets of /proc/PID/ns/* symlinks have lived in a fs
separated from /proc, named nsfs [1].  These targets are used to enter
the namespace of another process by using setns() syscall [2].  On old
kernels, they were labeled with procfs default type (for example
"getfilecon /proc/self/ns/uts" returned system_u:object_r:proc_t:s0).
When using a recent kernel with a policy without nsfs support, the
inodes are not labeled, as reported for example in Fedora bug #1234757
[3].  As I encounter this issue on my systems, I asked yesterday on the
refpolicy ML how nsfs inodes should be labeled [4].

After digging a little bit about the possibilities, here is a summary of
the options I have considered so far.

Option 1: define a new type to label nsfs inodes, nsfs_t.  This works as
expected (c.f. [5] for more details).

Option 2: "fs_use_task nsfs gen_context(system_u:object_r:fs_t,s0);".
Even though this works well for /proc/self/ns/*, this behaves in a weird
way with other processes in the initial namespaces. Here is a shell
session with such a configuration (on a system running in permissive mode):

   # runcon system_u:system_r:init_t sleep 100&
   [1] 26633
   # ls -lZ /proc/26633/ns/uts
   lrwxrwxrwx. 1 root root system_u:system_r:init_t 0 Jan  7 19:49
   /proc/26633/ns/uts -> uts:[4026531838]
   # getfilecon /proc/26633/ns/uts
   /proc/26633/ns/uts sysadm_u:sysadm_r:sysadm_t
   # runcon user_u:user_r:user_t getfilecon /proc/26633/ns/uts
   /proc/26633/ns/uts user_u:user_r:user_t

In short, nsfs inodes get created with the context of the running task.
This is because the inodes do not exist before getfilecon() opens them
(c.f. ns_get_path() function in fs/nsfs.c [6] and
inode_doinit_with_dentry() in security/selinux/hooks.c, which does
"isec->sid = current_sid()" in SECURITY_FS_USE_TASK case [7]).  This
issue does not appear with Docker and the network namespace used by
systemd services for PrivateNetwork feature because a file descriptor to
the network namespace is kept open, so the inode is created by the task
"owning" the namespace and its label is stable.


Option 3: do not add anything in the policy and add
"security_task_to_inode(task, inode);" right after the inode
initialization in ns_get_path() (line 90 of [6]), which is what /proc
uses to make /proc/PID inodes have the context of their tasks.  Then
"getfilecon /proc/PID/ns/uts" returns the context of task PID (and not
the context that is used by getfilecon command), but as this inode is
per-namespace and not per-task, there can be situations like this:

   # id -Z ; getfilecon /proc/self/ns/uts
   sysadm_u:sysadm_r:sysadm_t
   /proc/self/ns/uts sysadm_u:sysadm_r:sysadm_t

   # runcon 'user_u:user_r:user_t' bash -c 'exec 3 uts:[4026531838]

   # getfilecon "/proc/$P/fd/3" "/proc/$P/ns/uts"
   /proc/803/fd/3   user_u:user_r:user_t
   /proc/803/ns/uts user_u:user_r:user_t

   # getfilecon /proc/self/ns/uts
   /proc/self/ns/uts user_u:user_r:user_t

   # fg
   ^C
   # getfilecon /proc/self/ns/uts
   /proc/self/ns/uts sysadm_u:sysadm_r:sysadm_t

So in fact each /proc/self/ns/* symlink target is labeled accordingly to
the first process which opened it.  I do not know whether this behaviour
fits with the "real world usage" of namespaces (e.g. with containers) or
can be considered as a side effect which can be ignored.


Option 4 (not tested): add a sid field to struct ns_common and make
every namespace labeled from the process which creates it, maybe with a
type transition mechanism. This would be quite heavy to handle.


How should nsfs be handled in the kernel and in SELinux policy?


Only option 1 makes sense to me.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: security_bounded_transition fails

2015-12-18 Thread Stephen Smalley

On 12/18/2015 01:12 AM, Hannu Savolainen wrote:

Hi,

I'm having a problem with a multithreaded application. It does lengthy  
initialization in advance under relatively privileged context and then switches 
to a less privileged one after the moment when the actual request arrives. 
After that it will create a chrooted container and join all threads to a new 
SELinux context.

However the transition fails with audit message "op=security_bounded_transition 
result=denied oldcontext=old_context newcontext=new_context".

Is there any policy rule that could be used to fix this or is this just not 
supported?


First, it is easier and safer to perform the privileged initialization 
and switch to the unprivileged context _before_ spawning other threads. 
 Then you won't have this problem at all.


If for some reason you cannot do that, then the requirement for a 
multi-threaded process is that you can only setcon to a domain bounded 
by your current domain, where these bounding relationships are defined 
through the use of the typebounds statement in policy.  The 
child/bounded domain is then restricted to a subset of the permissions 
of the parent/boundary domain, so you cannot allow the child/bounded 
domain any permission not allowed to the parent/boundary domain.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: security_bounded_transition fails

2015-12-18 Thread Stephen Smalley

On 12/18/2015 10:05 AM, Dominick Grift wrote:

On Fri, Dec 18, 2015 at 11:27:13AM +, Hannu Savolainen wrote:

Many thanks,



Adding the allow rules seem to be enough (have to verify that one more time 
next week). Fortunately the typebounds rule doesn't seem to be necessary since 
it triggered avalanche of dependency problems everywhere.



Best regards,



Hannu


Cc: selinux list

I suspect though that you eventually really need to add the typebounds
statement.


typebounds is required for a multi-threaded process to setcon().

And note that setcon() only changes the context of the current thread, 
not all threads in the process.



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] libselinux, policycoreutils: Man page warning fixes

2015-11-24 Thread Stephen Smalley

On 11/07/2015 04:20 AM, Ville Skyttä wrote:

Signed-off-by: Ville Skyttä 


Thanks, applied.


---
  libselinux/man/man3/security_load_booleans.3| 2 +-
  libselinux/man/man3/selinux_binary_policy_path.3| 2 +-
  libselinux/man/man8/avcstat.8   | 2 +-
  libselinux/man/man8/booleans.8  | 2 +-
  libselinux/man/man8/getenforce.8| 2 +-
  libselinux/man/man8/getsebool.8 | 2 +-
  libselinux/man/man8/matchpathcon.8  | 2 +-
  libselinux/man/man8/selinux.8   | 2 +-
  libselinux/man/man8/selinuxenabled.8| 2 +-
  libselinux/man/man8/selinuxexeccon.8| 2 +-
  libselinux/man/man8/setenforce.8| 2 +-
  libselinux/man/man8/togglesebool.8  | 2 +-
  policycoreutils/semodule/semodule.8 | 4 ++--
  policycoreutils/semodule_package/semodule_package.8 | 2 +-
  policycoreutils/sepolicy/sepolicy-communicate.8 | 2 +-
  policycoreutils/setsebool/setsebool.8   | 2 +-
  16 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/libselinux/man/man3/security_load_booleans.3 
b/libselinux/man/man3/security_load_booleans.3
index 3dc963d..3b0bbea 100644
--- a/libselinux/man/man3/security_load_booleans.3
+++ b/libselinux/man/man3/security_load_booleans.3
@@ -56,7 +56,7 @@ commits all pending values for the booleans.
  Where not otherwise stated, functions described in this manual page return
  zero on success or \-1 on error.
  .
-.SH AUTHOR 
+.SH AUTHOR
  This manual page was written by Dan Walsh .
  .
  .SH "SEE ALSO"
diff --git a/libselinux/man/man3/selinux_binary_policy_path.3 
b/libselinux/man/man3/selinux_binary_policy_path.3
index 503c52c..edaa3b8 100644
--- a/libselinux/man/man3/selinux_binary_policy_path.3
+++ b/libselinux/man/man3/selinux_binary_policy_path.3
@@ -108,7 +108,7 @@ returns the defines tty types for newrole securettys.
  .BR selinux_booleans_path ()
  returns the initial policy boolean settings.
  .
-.SH AUTHOR 
+.SH AUTHOR
  This manual page was written by Dan Walsh .
  .
  .SH "SEE ALSO"
diff --git a/libselinux/man/man8/avcstat.8 b/libselinux/man/man8/avcstat.8
index 6251591..204687d 100644
--- a/libselinux/man/man8/avcstat.8
+++ b/libselinux/man/man8/avcstat.8
@@ -27,7 +27,7 @@ Display the cumulative values.
  Specifies the location of the AVC statistics file, defaulting to
  .IR /selinux/avc/cache_stats .
  .
-.SH AUTHOR 
+.SH AUTHOR
  This manual page was written by Dan Walsh .
  The program was written by James Morris .
  .
diff --git a/libselinux/man/man8/booleans.8 b/libselinux/man/man8/booleans.8
index 9c4dbc3..d4ca9c4 100644
--- a/libselinux/man/man8/booleans.8
+++ b/libselinux/man/man8/booleans.8
@@ -47,7 +47,7 @@ unless the
  .B \-P
  option is used to setsebool.
  .
-.SH AUTHOR 
+.SH AUTHOR
  This manual page was written by Dan Walsh .
  The SELinux conditional policy support was developed by Tresys Technology.
  .
diff --git a/libselinux/man/man8/getenforce.8 b/libselinux/man/man8/getenforce.8
index e0924d8..70b9921 100644
--- a/libselinux/man/man8/getenforce.8
+++ b/libselinux/man/man8/getenforce.8
@@ -9,7 +9,7 @@ getenforce \- get the current mode of SELinux
  .B getenforce
  reports whether SELinux is enforcing, permissive, or disabled.
  .
-.SH AUTHOR 
+.SH AUTHOR
  Dan Walsh, 
  .
  .SH "SEE ALSO"
diff --git a/libselinux/man/man8/getsebool.8 b/libselinux/man/man8/getsebool.8
index 6353a2a..d70bf1e 100644
--- a/libselinux/man/man8/getsebool.8
+++ b/libselinux/man/man8/getsebool.8
@@ -27,7 +27,7 @@ their pending values as desired and then committing once.
  .B \-a
  Show all SELinux booleans.
  .
-.SH AUTHOR 
+.SH AUTHOR
  This manual page was written by Dan Walsh .
  The program was written by Tresys Technology.
  .
diff --git a/libselinux/man/man8/matchpathcon.8 
b/libselinux/man/man8/matchpathcon.8
index 5d60789..50c0d39 100644
--- a/libselinux/man/man8/matchpathcon.8
+++ b/libselinux/man/man8/matchpathcon.8
@@ -54,7 +54,7 @@ Use alternate policy root path
  .B \-V
  Verify file context on disk matches defaults
  .
-.SH AUTHOR 
+.SH AUTHOR
  This manual page was written by Dan Walsh .
  .
  .SH "SEE ALSO"
diff --git a/libselinux/man/man8/selinux.8 b/libselinux/man/man8/selinux.8
index 9e3bdc4..6f1034b 100644
--- a/libselinux/man/man8/selinux.8
+++ b/libselinux/man/man8/selinux.8
@@ -77,7 +77,7 @@ also has this capability.  The
  .BR restorecon / fixfiles
  commands are also available for relabeling files.
  .
-.SH AUTHOR 
+.SH AUTHOR
  This manual page was written by Dan Walsh .
  .
  .SH FILES
diff --git a/libselinux/man/man8/selinuxenabled.8 
b/libselinux/man/man8/selinuxenabled.8
index ac20587..5cd7a62 100644
--- 

Re: [PATCH] libselinux: Correct line count for property and service contexts files

2015-11-24 Thread Stephen Smalley

On 11/23/2015 08:52 AM, Richard Haines wrote:

When a line number is displayed for context errors they are
x2 the correct value, so reset line count for each pass.

Signed-off-by: Richard Haines 


Thanks, applied.


---
  libselinux/src/label_android_property.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libselinux/src/label_android_property.c 
b/libselinux/src/label_android_property.c
index 712eecb..fea1f8f 100644
--- a/libselinux/src/label_android_property.c
+++ b/libselinux/src/label_android_property.c
@@ -132,7 +132,7 @@ static int init(struct selabel_handle *rec, const struct 
selinux_opt *opts,
const char *path = NULL;
FILE *fp;
char line_buf[BUFSIZ];
-   unsigned int lineno = 0, maxnspec, pass;
+   unsigned int lineno, maxnspec, pass;
int status = -1;
struct stat sb;

@@ -166,6 +166,7 @@ static int init(struct selabel_handle *rec, const struct 
selinux_opt *opts,
maxnspec = UINT_MAX / sizeof(spec_t);
for (pass = 0; pass < 2; pass++) {
data->nspec = 0;
+   lineno = 0;

while (fgets(line_buf, sizeof(line_buf) - 1, fp)
   && data->nspec < maxnspec) {



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Obtaining Default Context for SELinux Users

2015-11-20 Thread Stephen Smalley

On 11/18/2015 07:26 PM, Mike Palmiotto wrote:

On Wed, Nov 18, 2015 at 5:09 PM, Mike Palmiotto
 wrote:

We're currently running into issues attempting to get a default
context for a newly added SELinux user.

The user has been added with semanage, and associated with a few
roles. There are role declarations and allows (to and from the "scon"
role) in place in the policy. We've also added entries to
/etc/selinux/mls/contexts/{default_contexts,users/foo_u} to facilitate
getting a default context for the SELinux user.

The desire is to switch security labels based on the specified user's
default context, like so:
root:staff_r:staff_t:s0 -> foo_u:foo_r:foo_t:s0

We're using a call to `get_default_context("foo_u",
"root:staff_r:staff_t:s0", _context)` to get the default, but that
doesn't seem to be finding working.

In testing with a more verbose version of security_compute_user_raw,
we noticed that the list of available contexts for foo_u are empty.
This behavior has been noted for staff_u as well.

Curious to know if there's something glaringly obvious that we're missing.


Other than this discussion (and previous discussions on using
security_compute_user), of course:
http://marc.info/?l=selinux=144707899910491=2

I'm still curious as to why the get_default_context mechanism is not
finding any reachable contexts.


The underlying logic is computing the set of possible contexts and then 
filtering them based on whether the source/from context has process 
transition permission to each of them (if not, then they aren't 
reachable).  As we don't normally allow a regular user domain to 
directly transition to a different user identity or role (that being 
reserved to programs like login/gdm/sshd and su/sudo/newrole), 
root:staff_r:staff_r:s0 would not normally be able to directly 
transition to foo_u at all.  Those restrictions are defined through 
constraints in the policy, and typically only domains assigned specific 
attributes (e.g. can_change_process_identity, can_change_process_role) 
are exempted.  Those attributes are assigned through refpolicy 
interfaces.  But you don't want to directly assign them to staff_t, or 
that staff_t process can arbitrarily change to another identity/role 
without going through any kind of gatekeeper program to ensure proper 
authentication, sanitization, protection of the new user/role from undue 
influence/control by the caller, etc.




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Labeling nsfs filesystem

2016-01-08 Thread Stephen Smalley

On 01/08/2016 08:00 AM, Christopher J. PeBenito wrote:

On 1/7/2016 4:19 PM, Stephen Smalley wrote:

On 01/07/2016 03:36 PM, Nicolas Iooss wrote:

Hello,

Since Linux 3.19 targets of /proc/PID/ns/* symlinks have lived in a fs
separated from /proc, named nsfs [1].  These targets are used to enter
the namespace of another process by using setns() syscall [2].  On old
kernels, they were labeled with procfs default type (for example
"getfilecon /proc/self/ns/uts" returned system_u:object_r:proc_t:s0).
When using a recent kernel with a policy without nsfs support, the
inodes are not labeled, as reported for example in Fedora bug #1234757
[3].  As I encounter this issue on my systems, I asked yesterday on the
refpolicy ML how nsfs inodes should be labeled [4].

After digging a little bit about the possibilities, here is a summary of
the options I have considered so far.

Option 1: define a new type to label nsfs inodes, nsfs_t.  This works as
expected (c.f. [5] for more details).

Option 2: "fs_use_task nsfs gen_context(system_u:object_r:fs_t,s0);".
Even though this works well for /proc/self/ns/*, this behaves in a weird
way with other processes in the initial namespaces. Here is a shell
session with such a configuration (on a system running in permissive
mode):

# runcon system_u:system_r:init_t sleep 100&
[1] 26633
# ls -lZ /proc/26633/ns/uts
lrwxrwxrwx. 1 root root system_u:system_r:init_t 0 Jan  7 19:49
/proc/26633/ns/uts -> uts:[4026531838]
# getfilecon /proc/26633/ns/uts
/proc/26633/ns/uts sysadm_u:sysadm_r:sysadm_t
# runcon user_u:user_r:user_t getfilecon /proc/26633/ns/uts
/proc/26633/ns/uts user_u:user_r:user_t

In short, nsfs inodes get created with the context of the running task.
This is because the inodes do not exist before getfilecon() opens them
(c.f. ns_get_path() function in fs/nsfs.c [6] and
inode_doinit_with_dentry() in security/selinux/hooks.c, which does
"isec->sid = current_sid()" in SECURITY_FS_USE_TASK case [7]).  This
issue does not appear with Docker and the network namespace used by
systemd services for PrivateNetwork feature because a file descriptor to
the network namespace is kept open, so the inode is created by the task
"owning" the namespace and its label is stable.


Option 3: do not add anything in the policy and add
"security_task_to_inode(task, inode);" right after the inode
initialization in ns_get_path() (line 90 of [6]), which is what /proc
uses to make /proc/PID inodes have the context of their tasks.  Then
"getfilecon /proc/PID/ns/uts" returns the context of task PID (and not
the context that is used by getfilecon command), but as this inode is
per-namespace and not per-task, there can be situations like this:

# id -Z ; getfilecon /proc/self/ns/uts
sysadm_u:sysadm_r:sysadm_t
/proc/self/ns/uts sysadm_u:sysadm_r:sysadm_t

# runcon 'user_u:user_r:user_t' bash -c 'exec 3 uts:[4026531838]

# getfilecon "/proc/$P/fd/3" "/proc/$P/ns/uts"
/proc/803/fd/3   user_u:user_r:user_t
/proc/803/ns/uts user_u:user_r:user_t

# getfilecon /proc/self/ns/uts
/proc/self/ns/uts user_u:user_r:user_t

# fg
^C
# getfilecon /proc/self/ns/uts
/proc/self/ns/uts sysadm_u:sysadm_r:sysadm_t

So in fact each /proc/self/ns/* symlink target is labeled accordingly to
the first process which opened it.  I do not know whether this behaviour
fits with the "real world usage" of namespaces (e.g. with containers) or
can be considered as a side effect which can be ignored.


Option 4 (not tested): add a sid field to struct ns_common and make
every namespace labeled from the process which creates it, maybe with a
type transition mechanism. This would be quite heavy to handle.


How should nsfs be handled in the kernel and in SELinux policy?


Only option 1 makes sense to me.


I don't understand the usage of nsfs which makes this confusing, but why
doesn't option 3 make sense?  Since it's under a particular /proc/pid
entry, doesn't it make sense to label the object as the domain's type?


The symlink is under a particular /proc/pid directory, but the target is 
per-namespace, not per-process.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [RFC 1/2] selinux: Stop looking up dentries from inodes

2016-06-03 Thread Stephen Smalley
On 06/01/2016 05:46 PM, Andreas Gruenbacher wrote:
> On Wed, Jun 1, 2016 at 3:44 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>> On 05/31/2016 11:22 AM, Andreas Gruenbacher wrote:
>>> With that fixed, could you possibly put this change to test?
>>
>> Falls over during boot in generic_getxattr(), which still needs a
>> non-NULL dentry in the work.selinux branch.
> 
> dentry->d_sb needs to be changed to inode->i_sb there.
> 
>> Is there a reason that this being done separately from work.xattr?
> 
> I don't know how much work.xattr will shift still (and what I can
> still add there), and this change is unrelated, at least so far.
> 
>> Also, if we aren't going to call d_find_alias() there, we can likely
>> also drop the dget() and dput().
> 
> Ah, yes. I'll remove those, thanks.

Looks like you lost the assignment for dentry entirely when you removed
the dget/dput.  Still need to set it to opt_dentry or just use
opt_dentry directly.

BTW, SELinux will presently never call getxattr for 9p or cifs; those
filesystem types are not configured for xattrs in policy because they do
not truly support labeling (and if they did, we would probably use
SECURITY_LSM_NATIVE_LABELS => SECURITY_FS_USE_NATIVE as with nfsv4
rather than FS_USE_XATTR).  Just because they support xattrs does not
mean that they support security labeling.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] LSM: Reorder security_capset to do access checks properly

2016-06-01 Thread Stephen Smalley
On 06/01/2016 04:30 PM, Casey Schaufler wrote:
> On 6/1/2016 1:06 PM, Stephen Smalley wrote:
>> On 06/01/2016 03:27 PM, Casey Schaufler wrote:
>>> Subject: [PATCH] LSM: Reorder security_capset to do access checks properly
>>>
>>> The security module hooks that check whether a process should
>>> be able to set a new capset are currently called after the new
>>> values are set in cap_capset(). This change reverses the order.
>>> The capability module no longer adds cap_capset to the module list.
>>> Instead, it is invoked directly by the LSM infrastructure.
>>> This isn't an approach that generalizes well.
>> Is this change necessary?  The fact that cap_capset() modifies new
>> before the other hooks are called does no harm, because if any hook
>> returns an error, then the caller returns that error and never commits
>> the new cred.  It is actually possibly beneficial for the other security
>> hooks to be called after cap_capset() so that they can adjust the new
>> values if desired (e.g. to reduce them) before they are finally committed.
> 
> The existing code will set the new credential values before the
> security modules do their checks. Even if it's harmless, it's sloppy.
> Currently there's only one caller, but with Serge's work on ns_capabilities
> I'm looking to make this safer.

It's intentional.  cap_capset() does two things: it validates the
proposed capability sets (a permission check, returning -EPERM on
failure) and if valid under its own logic, it then updates new.  But the
update does not take effect until the caller of security_capset() calls
commit_creds() and that only happens if all of the hooks pass.  By
moving cap_capset() to the end, you are reversing the order of checks
from the norm (DAC before MAC) and you aren't allowing other security
modules to vet and possibly reduce new further.  Also, it is obvious
from the patch below that doing so requires a massive hack to what was
otherwise working fine for stacking.

If you are going to insist on reversing the order, then I think you need
to split security_capset into two hooks, one which only validates and
one which sets, and only use your alternative ordering for the latter.
But that's a lot of work for no apparent gain...

> 
>>
>>> Signed-off-by: Casey Schaufler <ca...@schaufler-ca.com>
>>> ---
>>>  security/commoncap.c |  2 +-
>>>  security/security.c  | 24 ++--
>>>  2 files changed, 23 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/security/commoncap.c b/security/commoncap.c
>>> index 48071ed..f5bce18 100644
>>> --- a/security/commoncap.c
>>> +++ b/security/commoncap.c
>>> @@ -1073,7 +1073,7 @@ struct security_hook_list capability_hooks[] = {
>>> LSM_HOOK_INIT(ptrace_access_check, cap_ptrace_access_check),
>>> LSM_HOOK_INIT(ptrace_traceme, cap_ptrace_traceme),
>>> LSM_HOOK_INIT(capget, cap_capget),
>>> -   LSM_HOOK_INIT(capset, cap_capset),
>>> +   /* Carefull! Do not include cap_capset! */
>>> LSM_HOOK_INIT(bprm_set_creds, cap_bprm_set_creds),
>>> LSM_HOOK_INIT(bprm_secureexec, cap_bprm_secureexec),
>>> LSM_HOOK_INIT(inode_need_killpriv, cap_inode_need_killpriv),
>>> diff --git a/security/security.c b/security/security.c
>>> index 92cd1d8..1610be8 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -177,8 +177,28 @@ int security_capset(struct cred *new, const struct 
>>> cred *old,
>>> const kernel_cap_t *inheritable,
>>> const kernel_cap_t *permitted)
>>>  {
>>> -   return call_int_hook(capset, 0, new, old,
>>> -   effective, inheritable, permitted);
>>> +   struct security_hook_list *hp;
>>> +   int rc;
>>> +
>>> +   /*
>>> +* Special case handling because the "new" capabilities
>>> +* should not be set until it has been determined that
>>> +* all modules approve of the change. Passing NULL pointers
>>> +* to all modules except the capabilty module as it is
>>> +* expected that only the capability modules needs the
>>> +* result pointers.
>>> +*
>>> +* cap_capset() must not be in the capability module hook list!
>>> +*/
>>> +   list_for_each_entry(hp, _hook_heads.capset, list) {
>>> +   rc = hp->hook.capset(new, old, NULL, NULL, NULL);
>>> +   if (rc != 0)
>>> +   return rc;
>>> +   }
>>> +   /*
>>> +* Call cap_capset now to updat

Re: Possible problem with e6afc8ac ("udp: remove headers from UDP packets before queueing")

2016-06-01 Thread Stephen Smalley
On 06/01/2016 03:18 PM, Eric Dumazet wrote:
> On Wed, 2016-06-01 at 15:01 -0400, Paul Moore wrote:
>> Hello,
>>
>> I'm currently trying to debug a problem with 4.7-rc1 and labeled
>> networking over UDP.  I'm having some difficulty with the latest
>> 4.7-rc1 builds on my test system at the moment so I haven't been able
>> to concisely identify the problem, but looking through the commits in
>> 4.7-rc1 I think there may be a problem with the following:
>>
>>   commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac
>>   Author: samanthakumar <samanthaku...@google.com>
>>   Date:   Tue Apr 5 12:41:15 2016 -0400
>>
>>udp: remove headers from UDP packets before queueing
>>
>>Remove UDP transport headers before queueing packets for reception.
>>This change simplifies a follow-up patch to add MSG_PEEK support.
>>
>>Signed-off-by: Sam Kumar <samanthaku...@google.com>
>>Signed-off-by: Willem de Bruijn <will...@google.com>
>>Signed-off-by: David S. Miller <da...@davemloft.net>
>>
>> ... it appears that this commit changes things so that sk_filter() is
>> only called when sk->sk_filter is not NULL.  While this is fine for
>> the traditional socket filter case, it causes problems with LSMs that
>> make use of security_sock_rcv_skb() to enforce per-packet access
>> controls.
>>
>> Hopefully I'll get 4.7-rc1 booting soon and I can do a proper
>> bisection test around this patch, but I wanted to mention this now in
>> case others are seeing the same problem.
>>
> 
> Thanks for the report. Please try following fix.
> 
> sk_filter() got additional features like the skb_pfmemalloc() things and
> security_sock_rcv_skb()

This resolved the SELinux regression for me.

Tested-by: Stephen Smalley <s...@tycho.nsa.gov>

> 
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index d56c0559b477..0ff31d97d485 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1618,12 +1618,12 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff 
> *skb)
>   }
>   }
>  
> - if (rcu_access_pointer(sk->sk_filter)) {
> - if (udp_lib_checksum_complete(skb))
> + if (rcu_access_pointer(sk->sk_filter) &&
> + udp_lib_checksum_complete(skb))
>   goto csum_error;
> - if (sk_filter(sk, skb))
> - goto drop;
> - }
> +
> + if (sk_filter(sk, skb))
> + goto drop;
>  
>   udp_csum_pull_header(skb);
>   if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) {
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 2da1896af934..f421c9f23c5b 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -653,12 +653,12 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff 
> *skb)
>   }
>   }
>  
> - if (rcu_access_pointer(sk->sk_filter)) {
> - if (udp_lib_checksum_complete(skb))
> - goto csum_error;
> - if (sk_filter(sk, skb))
> - goto drop;
> - }
> + if (rcu_access_pointer(sk->sk_filter) &&
> + udp_lib_checksum_complete(skb))
> + goto csum_error;
> +
> + if (sk_filter(sk, skb))
> + goto drop;
>  
>   udp_csum_pull_header(skb);
>   if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) {
> 
> 
> ___
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH 3/3] policycoreutils: setfiles - Modify to use selinux_restorecon

2016-05-31 Thread Stephen Smalley
On 05/31/2016 09:01 AM, Richard Haines wrote:
> 
> 
> 
> 
> 
>> On Thursday, 19 May 2016, 19:24, Stephen Smalley <s...@tycho.nsa.gov> wrote:
>>> On 05/10/2016 11:24 AM, Richard Haines wrote:
>>>  Modify setfiles and restorecon to make use of the libselinux
>>>  selinux_restorecon* set of functions.
>>>
>>>  The output from these commands should be much the same as before
>>>  with some minor wording changes, the only exceptions being:
>>>  1) The -p option does not output the percentage, just * for every
>>>  1000 files (but does state approx file count if mass relabel
>>>  and verbose).
>>
>> Seems like it might be a regression for usability on e.g. an autorelabel
>> at boot.
> 
> The main reason I did not implement this is that I would either need to
> pass over the approx amount of files to selinux_restorecon() or implement
> the exclude_non_seclabel_mounts() function in selinux_restorecon().
> I guess if this is required then adding exclude_non_seclabel_mounts(),
> add_exclude() and remove_exclude() to selinux_restorecon may be the best
> option as that resolves one of your queries on [PATCH 2/3]. Any views on
> the best way forward.

I guess the latter IIUC; it makes sense for selinux_restorecon() to
compute the estimated total number of files internally for use in the
progress output.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [RFC 1/2] selinux: Stop looking up dentries from inodes

2016-05-31 Thread Stephen Smalley
On 05/30/2016 09:59 AM, Andreas Gruenbacher wrote:
> SELinux sometimes needs to load the security label of an inode without
> knowing which dentry belongs to that inode (for example, in the
> inode_permission hook).  The security label is stored in an xattr;
> getxattr currently requires both the dentry and the inode.
> 
> So far, SELinux has been using d_find_alias to find any dentry for the
> inode; there is no guarantee that d_find_alias finds a suitable dentry
> on all types of filesystems, though.
> 
> This patch changes SELinux calls getxattr with a NULL dentry when the
> dentry is unknown.  On filesystems that require a dentry, getxattr fails
> with -ECHILD; on all others, it succeeds.
> 
> Signed-off-by: Andreas Gruenbacher 
> ---
>  fs/9p/acl.c  |  3 +++
>  fs/9p/xattr.c|  3 +++
>  fs/cifs/xattr.c  |  9 +++--
>  fs/ecryptfs/inode.c  |  8 ++--
>  fs/overlayfs/inode.c |  6 +-
>  net/socket.c |  3 +++
>  security/selinux/hooks.c | 43 +++
>  7 files changed, 42 insertions(+), 33 deletions(-)
> 

> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 9d153b6..dd90e5d 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -1043,8 +1043,12 @@ static ssize_t
>  ecryptfs_getxattr(struct dentry *dentry, struct inode *inode,
> const char *name, void *value, size_t size)
>  {
> - return ecryptfs_getxattr_lower(ecryptfs_dentry_to_lower(dentry),
> -ecryptfs_inode_to_lower(inode),
> + struct dentry *lower_dentry;
> + struct inode *lower_inode;
> +
> + lower_dentry = dentry ? ecryptfs_dentry_to_lower(dentry) : NULL;
> + lower_inode = ecryptfs_inode_to_lower(inode);
> + return ecryptfs_getxattr_lower(lower_dentry, lower_inode,
>  name, value, size);

ecryptfs_getxattr_lower() doesn't appear to handle a NULL lower_dentry.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index a86d537..dd509c8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1387,33 +1387,7 @@ static int inode_doinit_with_dentry(struct inode 
> *inode, struct dentry *opt_dent
>   case SECURITY_FS_USE_NATIVE:
>   break;
>   case SECURITY_FS_USE_XATTR:
> - if (!inode->i_op->getxattr) {
> - isec->sid = sbsec->def_sid;
> - break;
> - }

I don't think we can remove the above or we'll end up invoking
inode->i_op->getxattr == NULL below.

> -
> - /* Need a dentry, since the xattr API requires one.
> -Life would be simpler if we could just pass the inode. */
> - if (opt_dentry) {
> - /* Called from d_instantiate or d_splice_alias. */
> - dentry = dget(opt_dentry);
> - } else {
> - /* Called from selinux_complete_init, try to find a 
> dentry. */
> - dentry = d_find_alias(inode);
> - }
> - if (!dentry) {
> - /*
> -  * this is can be hit on boot when a file is accessed
> -  * before the policy is loaded.  When we load policy we
> -  * may find inodes that have no dentry on the
> -  * sbsec->isec_head list.  No reason to complain as 
> these
> -  * will get fixed up the next time we go through
> -  * inode_doinit with a dentry, before these inodes could
> -  * be used again by userspace.
> -  */
> - goto out_unlock;
> - }
> -
> + dentry = dget(opt_dentry);
>   len = INITCONTEXTLEN;
>   context = kmalloc(len+1, GFP_NOFS);
>   if (!context) {
> @@ -1448,7 +1422,20 @@ static int inode_doinit_with_dentry(struct inode 
> *inode, struct dentry *opt_dent
>   }
>   dput(dentry);
>   if (rc < 0) {
> - if (rc != -ENODATA) {
> + if (rc == -ECHILD) {
> + /*
> +  * The dentry is NULL and this ->getxattr
> +  * requires a dentry.  We will keep trying
> +  * until we have a dentry and the label can be
> +  * loaded.
> +  *
> +  * (We could also remember when >getxattr
> +  * requires a dentry in the superblock if
> +  * retrying becomes too inefficient.)
> +  */
> + kfree(context);
> + goto out_unlock;
> + } else if (rc != -EOPNOTSUPP && rc != -ENODATA) {

Re: abnormal SELinux context labels

2016-06-22 Thread Stephen Smalley
On 06/22/2016 02:05 PM, Bond Masuda wrote:
> I'm installing CentOS 7 in a chroot'd environment to build new images of
> CentOS 7 for a private cloud environment. I've done this successfully
> before with CentOS 6 (with help from this list) and we have an automated
> process of doing that now. I'm now porting our process to do similarly
> for CentOS 7. However, after our process is complete, certain
> directories/symlinks have abnormal SELinux contexts assigned to them.
> This causes the system to fail to boot since we have SELinux enforcing
> by default and one of the problematic symlinks is /lib64.
> 
> Here is what we see in the CentOS 7 build tree root directory, right
> after a fresh install of CentOS 7 from the full updates repo:
> 
> |# ls -alZ /
> dr-xr-xr-x. root root system_u:object_r:root_t:s0  .
> dr-xr-xr-x. root root system_u:object_r:root_t:s0  ..
> drwxr-xr-x. root root system_u:object_r:auditd_log_t:s0 audit
> lrwxrwxrwx. root root system_u:object_r:bin_t:s0   bin -> usr/bin
> dr-xr-xr-x. root root system_u:object_r:boot_t:s0  boot
> drwxr-xr-x. root root unconfined_u:object_r:unlabeled_t:s0 dev
> drwxr-xr-x. root root system_u:object_r:etc_t:s0   etc
> drwxr-xr-x. root root system_u:object_r:home_root_t:s0 home
> lrwxrwxrwx. root root /usr/lib lib -> usr/lib
> lrwxrwxrwx. root root /usr/lib lib64 -> usr/lib64
> drwxr-xr-x. root root system_u:object_r:mnt_t:s0   media
> drwxr-xr-x. root root system_u:object_r:mnt_t:s0   mnt
> drwxr-xr-x. root root system_u:object_r:usr_t:s0   opt
> drwxr-xr-x. root root unconfined_u:object_r:unlabeled_t:s0 proc
> dr-xr-x---. root root system_u:object_r:admin_home_t:s0 root
> drwxr-xr-x. root root /var/run run
> lrwxrwxrwx. root root system_u:object_r:bin_t:s0   sbin -> usr/sbin
> drwxr-xr-x. root root system_u:object_r:var_t:s0   srv
> drwxr-xr-x. root root unconfined_u:object_r:unlabeled_t:s0 sys
> drwxrwxrwt. root root system_u:object_r:tmp_t:s0   tmp
> drwxr-xr-x. root root system_u:object_r:usr_t:s0   usr
> drwxr-xr-x. root root system_u:object_r:var_t:s0   var
> 
> |As you can see, the SELinux context for "lib", is "/usr/lib"!!! and
> similarly, for "lib64", it is "/usr/lib" ... those are not even valid
> context labels!
> 
> How can an invalid string like "/usr/lib" even be assigned as a SELinux
> label in the first place?
> 
> I can workaround this with a manual fix using 'chcon
> system_u:object_r:type_label:s0 path', but I'm just wondering how this
> can happen in the first place? When I try to manually reproduce the
> invalid label, I get this:
> 
> # chcon /usr/lib lib
> chcon: invalid context: /usr/lib
> 
> Any insights would be appreciated...
> Bond

I'm assuming you are running the labeling process in a domain that has
CAP_MAC_ADMIN and mac_admin in SELinux policy, e.g. setfiles_mac_t or
livecd_t.

You would have done that in order to allow it to set labels on files
that may not be defined in the build host policy.

A process with that permission can set any arbitrary string value it
wants as the security.selinux attribute; by design, SELinux on the build
host OS will ignore it and just treat it as if it has the unlabeled context.

So if there is a bug in your labeling program such that it is passing a
pathname rather than a label, then you would get the results you are seeing.

When you try to do it by hand above, you are presumably not running in a
domain with mac_admin permission, and therefore it won't let you set an
unknown value.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Protect Xen Virtualization via SElinux.

2016-06-20 Thread Stephen Smalley
On 06/20/2016 11:06 AM, Jason Long wrote:
> Can you show me some examples for both ?

I already pointed you to OpenXT; it is a worked example of both.

> On Monday, June 20, 2016 5:13 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
> On 06/19/2016 09:15 AM, Jason Long wrote:
> 
>> Hello.
>> How can I protect my Xen VM via SElinux? Can you show me some useful 
>> examples?
> 
> I'm not entirely sure what you are asking, but possible answers:
> 
> 1. If you want to apply SELinux-like controls over Xen virtual machines
> (domains), then you can use Xen Security Modules and the Flask security
> module (commonly abbreviated XSM/Flask) to define and enforce a policy
> over the hypervisor objects and operations.
> 
> 2. If you want to use SELinux to harden the Xen domain-0 or specific
> domUs, you can just enable it in those domains and configure your policy
> accordingly.
> 
> If you want a worked example of applying both XSM/Flask and SELinux,
> have a look at OpenXT,
> http://openxt.org/
> ___
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Selectively assigning SELinux policies to permissive and enforcement mode

2016-06-20 Thread Stephen Smalley
On 06/19/2016 07:16 PM, Taeho Kgil wrote:
> Hi SELinux community, 
> 
> I'm relatively new to this mailing list and not sure if this is the
> appropriate place to raise this question. 
> 
> I am trying to see if we can selectively assign policies to permissive
> and enforcement. Is this a possible capability available today?

Yes, SELinux permissive domains allow you to do this.  On Linux
distributions, you can configure specific domains to be permissive via
the semanage permissive command, see:
https://selinuxproject.org/page/PermissiveDomainRecipe

On Android, you can achieve the same effect by adding a permissive
declaration for the domain to the corresponding .te file under
external/sepolicy (or system/sepolicy in master) or your
device///sepolicy directory, and then rebuilding your
image.  Of course, for your final production image, there must not be
any permissive domains, or it will fail the CTS.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH 1/2] Modify audit2why analyze function to use loaded policy

2016-06-20 Thread Stephen Smalley
On 06/03/2016 11:09 AM, Joshua Brindle wrote:
> Class and perms should come from the policy being used for analysis,
> not the system policy so use sepol_ interfaces
> 
> Change-Id: Ia0590ed2514249fd98810a8d4fe87f8bf5280561
> Signed-off-by: Joshua Brindle 
> ---
>  libselinux/src/audit2why.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Thanks, applied.

> 
> diff --git a/libselinux/src/audit2why.c b/libselinux/src/audit2why.c
> index 12745b3..abe1701 100644
> --- a/libselinux/src/audit2why.c
> +++ b/libselinux/src/audit2why.c
> @@ -343,8 +343,8 @@ static PyObject *analyze(PyObject *self 
> __attribute__((unused)) , PyObject *args
>   if (rc < 0)
>   RETURN(BADTCON)
>  
> - tclass = string_to_security_class(tclassstr);
> - if (!tclass)
> + rc = sepol_string_to_security_class(tclassstr, );
> + if (rc < 0)
>   RETURN(BADTCLASS)
>  
>   /* Convert the permission list to an AV. */
> @@ -365,8 +365,8 @@ static PyObject *analyze(PyObject *self 
> __attribute__((unused)) , PyObject *args
>   permstr = PyString_AsString( strObj );
>  #endif
>   
> - perm = string_to_av_perm(tclass, permstr);
> - if (!perm)
> + rc = sepol_string_to_av_perm(tclass, permstr, );
> + if (rc < 0)
>   RETURN(BADPERM)
>  
>   av |= perm;
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] Correctly detect unknown classes in sepol_string_to_security_class

2016-06-20 Thread Stephen Smalley
On 06/03/2016 11:17 AM, Joshua Brindle wrote:
> Bail before running off the end of the class index
> 
> Change-Id: I47c4eaac3c7d789f8d85047e34e37e3f0bb38b3a
> Signed-off-by: Joshua Brindle 

Applied this one and then rewrote it to use hashtab_search().
Not sure why it wasn't that way in the first place.

> ---
>  libsepol/src/services.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libsepol/src/services.c b/libsepol/src/services.c
> index d64a8e8..665fcaa 100644
> --- a/libsepol/src/services.c
> +++ b/libsepol/src/services.c
> @@ -1155,7 +1155,7 @@ int hidden sepol_string_to_security_class(const char 
> *class_name,
>   char *class = NULL;
>   sepol_security_class_t id;
>  
> - for (id = 1;; id++) {
> + for (id = 1; id <= policydb->p_classes.nprim; id++) {
>   class = policydb->p_class_val_to_name[id - 1];
>   if (class == NULL) {
>   ERR(NULL, "could not convert %s to class id", 
> class_name);
> @@ -1166,6 +1166,8 @@ int hidden sepol_string_to_security_class(const char 
> *class_name,
>   return STATUS_SUCCESS;
>   }
>   }
> + ERR(NULL, "unrecognized class %s", class_name);
> + return -EINVAL;
>  }
>  
>  /*
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


[PATCH] libsepol: rewrite sepol_string_to_security_class to use hashtab_search

2016-06-20 Thread Stephen Smalley
Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
---
 libsepol/src/services.c | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/libsepol/src/services.c b/libsepol/src/services.c
index 665fcaa..d2b80b4 100644
--- a/libsepol/src/services.c
+++ b/libsepol/src/services.c
@@ -1152,22 +1152,16 @@ int hidden sepol_compute_av(sepol_security_id_t ssid,
 int hidden sepol_string_to_security_class(const char *class_name,
sepol_security_class_t *tclass)
 {
-   char *class = NULL;
-   sepol_security_class_t id;
-
-   for (id = 1; id <= policydb->p_classes.nprim; id++) {
-   class = policydb->p_class_val_to_name[id - 1];
-   if (class == NULL) {
-   ERR(NULL, "could not convert %s to class id", 
class_name);
-   return STATUS_ERR;
-   }
-   if ((strcmp(class, class_name)) == 0) {
-   *tclass = id;
-   return STATUS_SUCCESS;
-   }
+   class_datum_t *tclass_datum;
+
+   tclass_datum = hashtab_search(policydb->p_classes.table,
+ (hashtab_key_t) class_name);
+   if (!tclass_datum) {
+   ERR(NULL, "unrecognized class %s", class_name);
+   return STATUS_ERR;
}
-   ERR(NULL, "unrecognized class %s", class_name);
-   return -EINVAL;
+   *tclass = tclass_datum->s.value;
+   return STATUS_SUCCESS;
 }
 
 /*
-- 
2.5.5

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: New rules on restrict kernel module loading

2016-06-23 Thread Stephen Smalley
On 06/23/2016 01:01 PM, Ravi Kumar wrote:
> Hi Jeffrey,
> I tried to do the same , 
> added the allow rule in system_server as 
> / allow system_server system_file:system module_load;/
> 
> But still seeing issue  as of the wlan.ko is a symlink as below  
> wlan.ko -> /system/lib/modules/vendor_wlan.ko 
> 
> Wlan.ko   or  vendor_wlan.ko are with   u:object_r:system_file:s0
> 
> But still  i see there is some issue where it show up this denial .
> 
> W WifiStateMachin: type=1400 audit(0.0:2074): avc: denied { module_load
> } for scontext=u:r:system_server:s0 tcontext=u:r:system_server:s0
> tclass=system permissive=0
> in the above denial  i see the tcontext as system_server.
> 
> I had not debugged much into will do  but looks like there is some thing
> which we are missing . 

hardware/libhardware_legacy/wifi/wifi.c needs to be updated to use
open() + finit_module() rather than load_file() + init_module().

And bionic needs to export finit_module?



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] libselinux: compare absolute pathname in matchpathcon -V

2016-06-23 Thread Stephen Smalley
On 06/20/2016 10:10 AM, Petr Lautrbach wrote:
> filepath needs to be resolved first in order to be correctly found by
> selabel_lookup_raw()
> 
> Fixes:
> $ matchpathcon -V passwd
> passwd has context system_u:object_r:passwd_file_t:s0, should be
> system_u:object_r:passwd_file_t:s0
> 
> $ echo $?
> 1
> 
> Signed-off-by: Petr Lautrbach 

Thanks, applied.

> ---
>  libselinux/src/matchpathcon.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c
> index 3868711..a2f2c3e 100644
> --- a/libselinux/src/matchpathcon.c
> +++ b/libselinux/src/matchpathcon.c
> @@ -471,6 +471,17 @@ int selinux_file_context_verify(const char *path, mode_t 
> mode)
>   char * con = NULL;
>   char * fcontext = NULL;
>   int rc = 0;
> + char stackpath[PATH_MAX + 1];
> + char *p = NULL;
> +
> + if (S_ISLNK(mode)) {
> + if (!realpath_not_final(path, stackpath))
> + path = stackpath;
> + } else {
> + p = realpath(path, stackpath);
> + if (p)
> + path = p;
> + }
>  
>   rc = lgetfilecon_raw(path, );
>   if (rc == -1) {
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] libselinux: add selinux_snapperd_contexts_path()

2016-06-23 Thread Stephen Smalley
On 06/20/2016 07:09 AM, Petr Lautrbach wrote:
> Snapper needs a way how to set a proper selinux context on btrfs
> subvolumes originating in snapshot create command. Fs can't handle it on
> its own so snapper will enforce .snapshots subvolume relabeling
> according to a file returned by selinux_snapperd_contexts_path().
> 
> The format of the file will be similar to other contexts file:
> 
> snapperd_data = system_u:object_r:snapperd_data_t:s0
> 
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1247530
> https://bugzilla.redhat.com/show_bug.cgi?id=1247532

Thanks, applied.  I would recommend that a bug be opened against the
kernel / btrfs about the fact that the inodes are initially unlabeled,
as otherwise snapper will always need permissions to relabel unlabeled
files and generally we would prefer that unlabeled be inaccessible.

> 
> Signed-off-by: Petr Lautrbach 
> ---
>  libselinux/include/selinux/selinux.h |  1 +
>  libselinux/src/file_path_suffixes.h  |  1 +
>  libselinux/src/selinux_config.c  | 10 +-
>  libselinux/src/selinux_internal.h|  1 +
>  4 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/libselinux/include/selinux/selinux.h 
> b/libselinux/include/selinux/selinux.h
> index 2262086..3d8673f 100644
> --- a/libselinux/include/selinux/selinux.h
> +++ b/libselinux/include/selinux/selinux.h
> @@ -544,6 +544,7 @@ extern const char *selinux_lxc_contexts_path(void);
>  extern const char *selinux_x_context_path(void);
>  extern const char *selinux_sepgsql_context_path(void);
>  extern const char *selinux_openssh_contexts_path(void);
> +extern const char *selinux_snapperd_contexts_path(void);
>  extern const char *selinux_systemd_contexts_path(void);
>  extern const char *selinux_contexts_path(void);
>  extern const char *selinux_securetty_types_path(void);
> diff --git a/libselinux/src/file_path_suffixes.h 
> b/libselinux/src/file_path_suffixes.h
> index d1f9b48..95b228b 100644
> --- a/libselinux/src/file_path_suffixes.h
> +++ b/libselinux/src/file_path_suffixes.h
> @@ -24,6 +24,7 @@ S_(BINPOLICY, "/policy/policy")
>  S_(VIRTUAL_IMAGE, "/contexts/virtual_image_context")
>  S_(LXC_CONTEXTS, "/contexts/lxc_contexts")
>  S_(OPENSSH_CONTEXTS, "/contexts/openssh_contexts")
> +S_(SNAPPERD_CONTEXTS, "/contexts/snapperd_contexts")
>  S_(SYSTEMD_CONTEXTS, "/contexts/systemd_contexts")
>  S_(FILE_CONTEXT_SUBS, "/contexts/files/file_contexts.subs")
>  S_(FILE_CONTEXT_SUBS_DIST, "/contexts/files/file_contexts.subs_dist")
> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> index bec5f3b..c519a77 100644
> --- a/libselinux/src/selinux_config.c
> +++ b/libselinux/src/selinux_config.c
> @@ -50,7 +50,8 @@
>  #define BOOLEAN_SUBS  27
>  #define OPENSSH_CONTEXTS  28
>  #define SYSTEMD_CONTEXTS  29
> -#define NEL   30
> +#define SNAPPERD_CONTEXTS 30
> +#define NEL   31
>  
>  /* Part of one-time lazy init */
>  static pthread_once_t once = PTHREAD_ONCE_INIT;
> @@ -499,6 +500,13 @@ const char *selinux_openssh_contexts_path(void)
>  
>  hidden_def(selinux_openssh_contexts_path)
>  
> +const char *selinux_snapperd_contexts_path(void)
> +{
> +return get_path(SNAPPERD_CONTEXTS);
> +}
> +
> +hidden_def(selinux_snapperd_contexts_path)
> +
>  const char *selinux_systemd_contexts_path(void)
>  {
>   return get_path(SYSTEMD_CONTEXTS);
> diff --git a/libselinux/src/selinux_internal.h 
> b/libselinux/src/selinux_internal.h
> index 46566f6..9b9145c 100644
> --- a/libselinux/src/selinux_internal.h
> +++ b/libselinux/src/selinux_internal.h
> @@ -84,6 +84,7 @@ hidden_proto(selinux_mkload_policy)
>  hidden_proto(selinux_x_context_path)
>  hidden_proto(selinux_sepgsql_context_path)
>  hidden_proto(selinux_openssh_contexts_path)
> +hidden_proto(selinux_snapperd_contexts_path)
>  hidden_proto(selinux_systemd_contexts_path)
>  hidden_proto(selinux_path)
>  hidden_proto(selinux_check_passwd_access)
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH] checkpolicy: Fix typos in test/dispol

2016-06-23 Thread Stephen Smalley
On 06/20/2016 07:50 AM, Petr Lautrbach wrote:
> Reported-By: Milos Malik 
> Signed-off-by: Petr Lautrbach 
> ---
>  checkpolicy/test/dispol.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Thanks, applied.

> 
> diff --git a/checkpolicy/test/dispol.c b/checkpolicy/test/dispol.c
> index 86f5688..a78ce81 100644
> --- a/checkpolicy/test/dispol.c
> +++ b/checkpolicy/test/dispol.c
> @@ -252,11 +252,11 @@ int display_cond_expressions(policydb_t * p, FILE * fp)
>  int display_handle_unknown(policydb_t * p, FILE * out_fp)
>  {
>   if (p->handle_unknown == ALLOW_UNKNOWN)
> - fprintf(out_fp, "Allow unknown classes and permisions\n");
> + fprintf(out_fp, "Allow unknown classes and permissions\n");
>   else if (p->handle_unknown == DENY_UNKNOWN)
> - fprintf(out_fp, "Deny unknown classes and permisions\n");
> + fprintf(out_fp, "Deny unknown classes and permissions\n");
>   else if (p->handle_unknown == REJECT_UNKNOWN)
> - fprintf(out_fp, "Reject unknown classes and permisions\n");
> + fprintf(out_fp, "Reject unknown classes and permissions\n");
>   return 0;
>  }
>  
> @@ -349,7 +349,7 @@ int menu(void)
>   printf("\nSelect a command:\n");
>   printf("1)  display unconditional AVTAB\n");
>   printf("2)  display conditional AVTAB (entirely)\n");
> - printf("3)  display conditional AVTAG (only ENABLED rules)\n");
> + printf("3)  display conditional AVTAB (only ENABLED rules)\n");
>   printf("4)  display conditional AVTAB (only DISABLED rules)\n");
>   printf("5)  display conditional bools\n");
>   printf("6)  display conditional expressions\n");
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH 2/2] libselinux: man: Clarify is_selinux_mls_enabled() description

2016-06-23 Thread Stephen Smalley
On 06/20/2016 11:41 AM, Petr Lautrbach wrote:
> From: David King 
> 
> Improve the description by mentioning that if is_selinux_mls_enabled(),
> it simply means that the kernel has MLS support and the policy contains
> MLS features. To check whether MLS support is enabled on the running
> system, use selinux_getpolicytype().
> 
> Signed-off-by: David King 

Thanks, applied both.

> ---
>  libselinux/man/man3/is_selinux_enabled.3 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libselinux/man/man3/is_selinux_enabled.3 
> b/libselinux/man/man3/is_selinux_enabled.3
> index b2df562..df62c22 100644
> --- a/libselinux/man/man3/is_selinux_enabled.3
> +++ b/libselinux/man/man3/is_selinux_enabled.3
> @@ -18,7 +18,9 @@ returns 1 if SELinux is running or 0 if it is not.
>  On error, \-1 is returned.
>  
>  .BR is_selinux_mls_enabled ()
> -returns 1 if SELinux is running in MLS mode or 0 if it is not. 
> +returns 1 if SELinux is capable of running in MLS mode or 0 if it is not. To
> +determine the policy in use on the system, use
> +.BR selinux_getpolicytype (3).
>  .
>  .SH "SEE ALSO"
>  .BR selinux "(8)"
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: New rules on restrict kernel module loading

2016-06-23 Thread Stephen Smalley
On 06/22/2016 03:02 PM, Jeffrey Vander Stoep wrote:
> selinux@tycho.nsa.gov  to bcc
> 
> Hi Ravi,
> 
> The intent is not to restrict which processes may load modules, but to
> place restrictions on the origin of the module itself. Modules, like the
> kernel, should live on a verity protected partition.
> 
> If you want system apps to load a kernel module from the system
> partition you just need to add an allow rule. e.g.
> 
> # system_app loads /system/lib/module/wlan.ko
> allow system_app system_file:system module_load;
> 
> Similar rules may be added for platform_app or system_server. 

Actually, that probably won't work for any app domains, as they can't
pass the sys_module capability check.  So hopefully you only truly need
it for system_server.



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Copying/setting security.selinux xattr explicitly

2016-02-10 Thread Stephen Smalley

On 02/10/2016 05:59 AM, Laurent Bigonville wrote:

Hello,

I've a question concerning copying the security.selinux xattr explicitly.

In you opinion what should happen in an implementation if it cannot be
reset security.selinux on the target file?

Apparently GNU cp -a ignore failures (while cp --preserve=context fails).

In some python helper function (_copyxattr(), see
https://bugs.python.org/issue14082), it will return an exception if the
copy of any of the xattr is failing, there is no special case for
security.selinux.

What do you think should be the behavior here?


The rationale for the difference in behavior between cp -a and cp 
--preserve=context is that cp -a usage predates SELinux (and extended 
attributes) and treating the inability to set the SELinux attribute on 
the destination file as a hard failure would have broken many existing 
uses of cp -a.


That is in fact generally true for all extended attributes, since trying 
to set any of them could fail due to lack of permission (except perhaps 
user.*) or due to lack of extended attribute support in the destination 
filesystem.


Looking at the patches in the bug/issue you cited, it looks like they 
actually ignore errno.EPERM, errno.ENOTSUP, and errno.ENODATA.  So the 
only one they don't ignore that SELinux might return would be errno.EACCES.


I'm a bit unclear on the intended semantics of shutil.copy2(), as on the 
one hand they say it is supposed to be like cp -p (which does not copy 
extended attributes at all) but on the other hand they now say that it 
copies all metadata that it can.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: selinux_set_callback() problem

2016-02-05 Thread Stephen Smalley

On 02/04/2016 04:32 PM, Russell Coker wrote:

type=USER_AVC msg=audit(1454447396.743:48359): pid=1 uid=0 auid=4294967295
ses=4294967295 subj=system_u:system_r:init_t:s0 msg='avc:  denied  { status }
for auid=0 uid=0 gid=0 path="/lib/systemd/system/reboot.target"
cmdline="reboot" scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:systemd_unit_file_t:SystemLow tclass=service
exe="/lib/systemd/systemd" sauid=0 hostname=? addr=? terminal=?'

I'm seeing entries like the above from the Debian/Jessie systemd in audit.log.
Below is the relevant code from the systemd source:

_printf_(2, 3) static int log_callback(int type, const char *fmt, ...) {
 va_list ap;

#ifdef HAVE_AUDIT
 if (get_audit_fd() >= 0) {
 _cleanup_free_ char *buf = NULL;
 int r;

 va_start(ap, fmt);
 r = vasprintf(, fmt, ap);
 va_end(ap);

 if (r >= 0) {
 audit_log_user_avc_message(get_audit_fd(),
AUDIT_USER_AVC, buf,
NULL, NULL, NULL, 0);
 return 0;
 }
 }
#endif

 va_start(ap, fmt);
 log_metav(LOG_USER | LOG_INFO, __FILE__, __LINE__, __FUNCTION__, fmt,
ap);
 va_end(ap);

 return 0;
}

Then the following line is in the access_init() function to enable it:

selinux_set_callback(SELINUX_CB_LOG, (union selinux_callback) log_callback);

Any suggestions as to where I should start working on this?

Sorry if it's a newbie question, I haven't worked on SE Linux library code for
a while.


What exactly is the problem?  Is it that the scontext has a raw context 
and the tcontext has a translated context?  Or is it that it was denied 
when it should have been allowed?


The callback itself is obviously being executed or you wouldn't have the 
audit message at all.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Newbie question on fixfiles

2016-01-29 Thread Stephen Smalley

On 01/29/2016 12:25 PM, Thomas Downing wrote:

Hi,

I need to get SELinux running on an appliance we are building, not based on a
distro that already supports SELinux.

I've got all the userspace stuff built, (including setools3) without any
warnings or errors. I followed instructions for installing and loading
refpolicy, no warnings or errors.  (Except the python tools, which all import
selinux.py, which does not seem to be included in the source tree.)

I'm booting with kernel options "security=selinux selinux=1", and dmesg shows
SELinux initializing, and no errors or warnings.

sestatus output:

SELinux status: enabled
SELinuxfs mount:/sys/fs/selinux
SELinux root directory: /etc/selinux
Loaded policy name: refpolicy
Current mode:   permissive
Mode from config file:  permissive
Policy MLS status:  disabled
Policy deny_unknown status: denied
Max kernel policy version:  30

Problem is: fixfiles does not actually label anything, and the underlying reason
is that none of the mounted disk filesystems (all ext4) have option 'seclabel'.

Any pointers?

Also, given the absence of the seclabel option, I question if the kernel part
of SELinux is in fact really happy...and if it isn't, I'm dead in the water
anyway.


This implies that you haven't loaded a policy into the kernel. Normally 
this is done by init; both sysvinit and systemd should already include 
the necessary bits but you may have to enable them in your configure.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: kernel-4.3.3-303.fc23.x86_64 and selinux-policy

2016-01-28 Thread Stephen Smalley

On 01/28/2016 02:13 PM, Bill wrote:

Is anyone else having a problem booting
kernel-4.3.3-303.fc23.x86_64 and
selinux-policy-3.13.1-158.2.fc23.noarch?


Seems fine here.  What kind of a problem?


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: genhomedircon uid template

2016-02-01 Thread Stephen Smalley

On 02/01/2016 04:36 AM, Jason Zaman wrote:

Hi all,

XDG_RUNTIME_DIR is usually /run/user/$UID but there is no way to label
that in an fcontext file. It used to be /run/user/USER which is easy but
not UID.

What template keyword should be used for such an entry? UID? USERID?

USERID is perhaps more obvious but has to be replaced before USER but
that should be doable.
https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/genhomedircon.c#L76

UID does not conflict with USER but this line exists in refpol which
is problematic:
contrib/fetchmail.fc:13:/var/mail/\.fetchmail-UIDL-cache -- 
gen_context(system_u:object_r:fetchmail_uidl_cache_t,s0)

This could also be used for several fcontexts in kerberos. It stores the
tickets in /tmp/krbcc_UID for example.

If we choose a template name I can put together a patch to add it.


No strong preferences from me on the particular name, e.g. USERID is 
fine.  I think it highlights however the problems with the current 
approach; maybe we ought to be using ${USER} and ${UID} in .fc files 
instead?

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: SELinux file context matching

2016-02-02 Thread Stephen Smalley

On 02/02/2016 12:48 PM, Mark Steele wrote:

Hi list,

I've got some file contexts setup for an application, and can't get the
file context matching to work as I would expect.

[root@dev1 policy]# cat
/etc/selinux/targeted/contexts/files/file_contexts | grep cinched
/etc/cinched(/.*)?  system_u:object_r:ts_etc_t:s0
/var/log/cinched(/.*)?  system_u:object_r:ts_log_t:s0
/var/lib/cinched(/.*)?  system_u:object_r:ts_t:s0
*/usr/lib64/cinched(/.*)?system_u:object_r:ts_lib_t:s0*
/etc/bash_completion.d/cinched_bash_completions
system_u:object_r:ts_etc_t:s0
/var/log/cinched/audit(/.*)?system_u:object_r:ts_audit_log_t:s0
/usr/sbin/cinched   system_u:object_r:ts_exec_t:s0

[root@dev1 policy]# matchpathcon /usr/lib64/cinched/
*/usr/lib64/cinched  system_u:object_r:lib_t:s0*

[root@dev1 policy]# findcon
/etc/selinux/targeted/contexts/files/file_contexts -p /usr/lib64/cinched
/.* system_u:object_r:default_t:s0
/usr/.* system_u:object_r:usr_t:s0
*/usr/lib64/cinched(/.*)?system_u:object_r:ts_lib_t:s0*


This is running on CentOS 7. I was assuming that since my rule has the
longest stem, it would be applied.

Any suggestions?


It would help to see the complete file_contexts file.
Do you have anything in file_contexts.local that could be overriding it?




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Newbie question on fixfiles

2016-01-29 Thread Stephen Smalley

On 01/29/2016 02:03 PM, Thomas Downing wrote:

On Friday, January 29, 2016 13:02:42 Stephen Smalley wrote:

On 01/29/2016 12:25 PM, Thomas Downing wrote:

Hi,

I need to get SELinux running on an appliance we are building, not based
on a distro that already supports SELinux.

I've got all the userspace stuff built, (including setools3) without any
warnings or errors. I followed instructions for installing and loading
refpolicy, no warnings or errors.  (Except the python tools, which all
import selinux.py, which does not seem to be included in the source
tree.)

I'm booting with kernel options "security=selinux selinux=1", and dmesg
shows SELinux initializing, and no errors or warnings.

sestatus output:

SELinux status: enabled
SELinuxfs mount:/sys/fs/selinux
SELinux root directory: /etc/selinux
Loaded policy name: refpolicy
Current mode:   permissive
Mode from config file:  permissive
Policy MLS status:  disabled
Policy deny_unknown status: denied
Max kernel policy version:  30

Problem is: fixfiles does not actually label anything, and the underlying
reason is that none of the mounted disk filesystems (all ext4) have
option 'seclabel'.

Any pointers?

Also, given the absence of the seclabel option, I question if the kernel
part of SELinux is in fact really happy...and if it isn't, I'm dead in
the water anyway.


This implies that you haven't loaded a policy into the kernel. Normally
this is done by init; both sysvinit and systemd should already include
the necessary bits but you may have to enable them in your configure.


Okay, my bad, I thought I had done "make load" in
/etc/selinux/refpolicy/src/policy, but I guess I missed that.  So now
"seclabel" shows up on all ext4 file systems in /proc/mounts, so that is good.

Now running "fixfiles -F -f -v -l fixfiles.log relabel" does not complain.

But now I've got two other problems:

1. Looking at the log file produced, only a few files are said to be labeled,
outside of /run/udev, /dev etc.  What happened to everything else in
file_contexts?

2. None of the files that the log file claims were relabeled, are in fact
labeled, according to 'ls -Z'.

There is no sysvinit script for selinux stuff for this distro, I need to create
all that.  Looking at Fedora 22 that is current SELinux enabled, I can't find
the systemd unit file that does the load, or I would use that as a reference.

On the other hand, I seems I should be able to use what "make load" does as a
reference as well.  Is that a valid assuption?


SELinux initialization is normally done directly from init code, not 
from a script file or unit file, because we need init to load policy and 
then re-exec itself or dynamically switch contexts to get init into its 
own security context (otherwise it will be left in the kernel's domain). 
 sysvinit and systemd source code already include that support (as does 
Android init); if using them, you might just need to rebuild with the 
appropriate configure flags.


Alternatively, you could invoke "load_policy -i" from an initramfs 
script after switching to the real root and before executing init.


If you run restorecon -v /path/to/file for one of these files that 
wasn't labeled, what does it say?  What does ls -Z show for the file 
before and after?

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: genhomedircon uid template

2016-02-02 Thread Stephen Smalley

On 02/02/2016 01:26 AM, Jason Zaman wrote:

On Mon, Feb 01, 2016 at 02:30:37PM -0500, Stephen Smalley wrote:

On 02/01/2016 04:36 AM, Jason Zaman wrote:

Hi all,

XDG_RUNTIME_DIR is usually /run/user/$UID but there is no way to label
that in an fcontext file. It used to be /run/user/USER which is easy but
not UID.

What template keyword should be used for such an entry? UID? USERID?

USERID is perhaps more obvious but has to be replaced before USER but
that should be doable.
https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/genhomedircon.c#L76

UID does not conflict with USER but this line exists in refpol which
is problematic:
contrib/fetchmail.fc:13:/var/mail/\.fetchmail-UIDL-cache -- 
gen_context(system_u:object_r:fetchmail_uidl_cache_t,s0)

This could also be used for several fcontexts in kerberos. It stores the
tickets in /tmp/krbcc_UID for example.

If we choose a template name I can put together a patch to add it.


No strong preferences from me on the particular name, e.g. USERID is
fine.  I think it highlights however the problems with the current
approach; maybe we ought to be using ${USER} and ${UID} in .fc files
instead?


Yes there are definitely problems but fixing would mean refpol and
probably a lot of other things would need to be updated at the same
time.

HOME_DIR and HOME_ROOT are not really problems since they are only
allowed in the beginning of an fcontext line and other lines start with
a /.

USER, USERID, and possibly other things in future (GROUP, GROUPID?) can
appear at any point in the the line so a more unique token might be
better. %USERID might be better than $USERID since thats a thing in
shells.

If we do go down this path, what are the steps? and what tokens do we
want?


I think we would provide backward compatibility for the existing tokens, 
at least for some time.  As far as I know, we only need to modify the 
refpolicy build process and libsemanage to support the new tokens.


You need a way to mark the end of the token, either %USERID% or %{USERID}.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Newbie question on fixfiles

2016-01-29 Thread Stephen Smalley

On 01/29/2016 02:41 PM, Thomas Downing wrote:

On Friday, January 29, 2016 14:25:43 Stephen Smalley wrote:
[snip]

This implies that you haven't loaded a policy into the kernel. Normally
this is done by init; both sysvinit and systemd should already include
the necessary bits but you may have to enable them in your configure.


Okay, my bad, I thought I had done "make load" in
/etc/selinux/refpolicy/src/policy, but I guess I missed that.  So now
"seclabel" shows up on all ext4 file systems in /proc/mounts, so that is
good.

Now running "fixfiles -F -f -v -l fixfiles.log relabel" does not complain.

But now I've got two other problems:

1. Looking at the log file produced, only a few files are said to be
labeled, outside of /run/udev, /dev etc.  What happened to everything
else in file_contexts?

2. None of the files that the log file claims were relabeled, are in fact
labeled, according to 'ls -Z'.

There is no sysvinit script for selinux stuff for this distro, I need to
create all that.  Looking at Fedora 22 that is current SELinux enabled, I
can't find the systemd unit file that does the load, or I would use that
as a reference.

On the other hand, I seems I should be able to use what "make load" does
as a reference as well.  Is that a valid assuption?


SELinux initialization is normally done directly from init code, not
from a script file or unit file, because we need init to load policy and
then re-exec itself or dynamically switch contexts to get init into its
own security context (otherwise it will be left in the kernel's domain).
   sysvinit and systemd source code already include that support (as does
Android init); if using them, you might just need to rebuild with the
appropriate configure flags.

Alternatively, you could invoke "load_policy -i" from an initramfs
script after switching to the real root and before executing init.

If you run restorecon -v /path/to/file for one of these files that
wasn't labeled, what does it say?  What does ls -Z show for the file
before and after?


About init, duh, just not thinking.  I will indeed need to rebuild init.

restorecon -v /home/tdowning/.viminfo:

restorecon reset /home/tdowning/.viminfo context
system_u:object_r:user_home_dir_t->system_u:object_r:user_home_t

But ls -aZ:

? .viminfo

(~/.viminfo is the only file under /home that fixfiles even tried to relabel).

It occurs to me that maybe all of fileutils, coreutils,sysutils, libnss*, pam*
and such like might need to be rebuilt?  Maybe ls is just not build right.  I
note that 'id -Z' complains "works only on an SELinux-enabled kernel",
indicating the need to rebuild all that stuff.


Yes, you need to rebuild your userspace with SELinux enabled.  You may 
be able to see the actual file context by using getfattr directly, e.g.

getfattr -n security.selinux /path/to/file

I assume you aren't using openembedded / yocto for your appliance? 
Because that already has a meta-selinux layer for enabling SELinux support.




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH 1/2] libselinux: procattr: return error on invalid pid_t input.

2016-02-24 Thread Stephen Smalley

On 02/23/2016 03:23 PM, Daniel Cashman wrote:

From: dcashman 

Signed-off-by: Daniel Cashman 


Thanks, applied.


---
  libselinux/src/procattr.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c
index 527a0a5..c20f003 100644
--- a/libselinux/src/procattr.c
+++ b/libselinux/src/procattr.c
@@ -70,9 +70,9 @@ static int openattr(pid_t pid, const char *attr, int flags)
char *path;
pid_t tid;

-   if (pid > 0)
+   if (pid > 0) {
rc = asprintf(, "/proc/%d/attr/%s", pid, attr);
-   else {
+   } else if (pid == 0) {
rc = asprintf(, "/proc/thread-self/attr/%s", attr);
if (rc < 0)
return -1;
@@ -82,6 +82,9 @@ static int openattr(pid_t pid, const char *attr, int flags)
free(path);
tid = gettid();
rc = asprintf(, "/proc/self/task/%d/attr/%s", tid, attr);
+   } else {
+   errno = EINVAL;
+   return -1;
}
if (rc < 0)
return -1;



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: getpidcon with pid == 0 (Was: Re: [PATCH 2/2] libselinux: procattr: return einval for <= 0 pid args.)

2016-02-24 Thread Stephen Smalley

On 02/24/2016 10:25 AM, Nick Kralevich wrote:

A quick Google search for "getpidcon(0" shows only the Android bug.

https://www.google.com/webhp#q=%22getpidcon(0%22

-- Nick


Yes, I looked through searchcode.com results for getpidcon (not just 
with a hardcoded 0 but also looking for any cases where they assume that 
setting a variable pid == 0 degenerates to getcon behavior), and didn't 
see anything.


I've also asked the Fedora SELinux maintainers if they know of anything 
that would break.




On Wed, Feb 24, 2016 at 6:49 AM, Stephen Smalley <s...@tycho.nsa.gov
<mailto:s...@tycho.nsa.gov>> wrote:

On 02/23/2016 03:24 PM, Daniel Cashman wrote:

From: dcashman <dcash...@android.com <mailto:dcash...@android.com>>

getpidcon documentation does not specify that a pid of 0 refers
to the
current process, and getcon exists specifically to provide this
functionality, and getpidcon(getpid()) would provide it as well.
Disallow pid values <= 0 that may lead to unintended behavior in
userspace object managers.


I'll try to see if there are any legitimate users of getpidcon with
pid == 0.  If anyone on the list knows of one, please speak up.


Signed-off-by: Daniel Cashman <dcash...@android.com
<mailto:dcash...@android.com>>
---
   libselinux/src/procattr.c | 14 --
   1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c
index c20f003..eee4612 100644
--- a/libselinux/src/procattr.c
+++ b/libselinux/src/procattr.c
@@ -306,11 +306,21 @@ static int setprocattrcon(const char *
context,
   #define getpidattr_def(fn, attr) \
 int get##fn##_raw(pid_t pid, char **c)  \
 { \
-   return getprocattrcon_raw(c, pid, #attr); \
+   if (pid <= 0) { \
+   errno = EINVAL; \
+   return -1; \
+   } else { \
+   return getprocattrcon_raw(c, pid, #attr); \
+   } \
 } \
 int get##fn(pid_t pid, char **c)\
 { \
-   return getprocattrcon(c, pid, #attr); \
+   if (pid <= 0) { \
+   errno = EINVAL; \
+   return -1; \
+   } else { \
+   return getprocattrcon(c, pid, #attr); \
+   } \
 }

   all_selfattr_def(con, current)





--
Nick Kralevich | Android Security | n...@google.com
<mailto:n...@google.com> | 650.214.4037


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


getpidcon with pid == 0 (Was: Re: [PATCH 2/2] libselinux: procattr: return einval for <= 0 pid args.)

2016-02-24 Thread Stephen Smalley

On 02/23/2016 03:24 PM, Daniel Cashman wrote:

From: dcashman 

getpidcon documentation does not specify that a pid of 0 refers to the
current process, and getcon exists specifically to provide this
functionality, and getpidcon(getpid()) would provide it as well.
Disallow pid values <= 0 that may lead to unintended behavior in
userspace object managers.


I'll try to see if there are any legitimate users of getpidcon with pid 
== 0.  If anyone on the list knows of one, please speak up.




Signed-off-by: Daniel Cashman 
---
  libselinux/src/procattr.c | 14 --
  1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c
index c20f003..eee4612 100644
--- a/libselinux/src/procattr.c
+++ b/libselinux/src/procattr.c
@@ -306,11 +306,21 @@ static int setprocattrcon(const char * context,
  #define getpidattr_def(fn, attr) \
int get##fn##_raw(pid_t pid, char **c)  \
{ \
-   return getprocattrcon_raw(c, pid, #attr); \
+   if (pid <= 0) { \
+   errno = EINVAL; \
+   return -1; \
+   } else { \
+   return getprocattrcon_raw(c, pid, #attr); \
+   } \
} \
int get##fn(pid_t pid, char **c)\
{ \
-   return getprocattrcon(c, pid, #attr); \
+   if (pid <= 0) { \
+   errno = EINVAL; \
+   return -1; \
+   } else { \
+   return getprocattrcon(c, pid, #attr); \
+   } \
}

  all_selfattr_def(con, current)



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Strange AVC with latest rawhide kernel.

2016-02-25 Thread Stephen Smalley

On 02/25/2016 01:02 PM, Daniel J Walsh wrote:

audit2allow -wla
type=AVC msg=audit(1456422969.279:1434): avc:  denied  { entrypoint }
for  pid=23847 comm="exe" path="/usr/bin/bash" dev="dm-2" ino=25165968
scontext=system_u:system_r:svirt_lxc_net_t:s0:c337,c895
tcontext=system_u:object_r:svirt_sandbox_file_t:s0:c337,c895
tclass=file permissive=0
Was caused by:
Unknown - would be allowed by active policy
Possible mismatch between this policy and the one under
which the audit message was generated.

Possible mismatch between current in-memory boolean
settings vs. permanent ones.

When trying to run a docker container on Rawhide, I am seeing this AVC.
The policy as audit2allow -w shows allows svirt_sandbox_file_t as an
entrypoint for svirt_lxc_net_t.

# sesearch -A -s svirt_lxc_net_t -t svirt_sandbox_file_t -c file -p
entrypoint
Found 1 semantic av rules:
allow svirt_sandbox_domain file_type : file entrypoint ;

But when I run try to start the container, docker blocks the access.  I
don't see any constraints that would block this, and don't think
NO_NEW_PRIV is enabled any way, and I don't think it would be involved
here.

Any idea why SELinux is blocking the access?


kernel version?


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Strange AVC with latest rawhide kernel.

2016-02-25 Thread Stephen Smalley

On 02/25/2016 01:59 PM, Daniel J Walsh wrote:

On Thu, 2016-02-25 at 13:18 -0500, Stephen Smalley wrote:

On 02/25/2016 01:02 PM, Daniel J Walsh wrote:


audit2allow -wla
type=AVC msg=audit(1456422969.279:1434): avc:  denied  { entrypoint
}
for  pid=23847 comm="exe" path="/usr/bin/bash" dev="dm-2"
ino=25165968
scontext=system_u:system_r:svirt_lxc_net_t:s0:c337,c895
tcontext=system_u:object_r:svirt_sandbox_file_t:s0:c337,c895
tclass=file permissive=0
Was caused by:
Unknown - would be allowed by active policy
Possible mismatch between this policy and the one under
which the audit message was generated.

Possible mismatch between current in-memory boolean
settings vs. permanent ones.

When trying to run a docker container on Rawhide, I am seeing this
AVC.
The policy as audit2allow -w shows allows svirt_sandbox_file_t as
an
entrypoint for svirt_lxc_net_t.

# sesearch -A -s svirt_lxc_net_t -t svirt_sandbox_file_t -c file -p
entrypoint
Found 1 semantic av rules:
 allow svirt_sandbox_domain file_type : file entrypoint ;

But when I run try to start the container, docker blocks the
access.  I
don't see any constraints that would block this, and don't think
NO_NEW_PRIV is enabled any way, and I don't think it would be
involved
here.

Any idea why SELinux is blocking the access?

Also, what does compute_av report for that (scontext, tcontext,
tclass)
triple?




uname -r
4.5.0-0.rc5.git0.1.fc24.x86_64

./compute_av system_u:system_r:svirt_lxc_net_t:s0:c337,c895
system_u:object_r:svirt_sandbox_file_t:s0:c337,c895 file
allowed= { ioctl read write create getattr setattr lock relabelfrom
relabelto append unlink link rename execute execute_no_trans execmod
open }
auditdeny { ioctl read write create setattr lock relabelfrom relabelto
append unlink link rename execute swapon quotaon mounton
execute_no_trans entrypoint execmod open audit_access 0xffc0 }

Looks like it is auditdeny, but I have no idea why.


No, auditdeny is fine (default is to audit all denials, so all-bits-set, 
even for undefined permissions).  The question is why isn't entrypoint 
listed in allowed - that shows that your kernel is indeed saying that 
entrypoint isn't allowed.


Running the same kernel, with selinux-policy-3.13.1.171.fc24, when I run 
the same compute_av command as above, entrypoint is listed in allowed 
(in between execute_no_trans and execmod).  Your policy is what?



___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: Strange AVC with latest rawhide kernel.

2016-02-25 Thread Stephen Smalley

On 02/25/2016 03:28 PM, Daniel J Walsh wrote:

On Thu, 2016-02-25 at 14:47 -0500, Stephen Smalley wrote:

On 02/25/2016 02:37 PM, Eric Paris wrote:


You added a type bounds right before this broke...  Does the parent
type have entrypoint? If not, maybe that's where it got stripped...

That would match the behavior he described (although he should get
an
audit message of the form op=security_compute_av reason=bounds.. in
audit.log or dmesg in that case).  The kernel automatically reduces
permissions as required by typebounds.  The corresponding logic
never
made its way into the libsepol compute_av code, so audit2why
wouldn't
know about it, and sesearch merely searches for TE rules; it doesn't
do
anything about typebounds.  We should probably update libsepol
compute_av (for that, and eventually for xperms).



That would make sense, and then when I blew it away everything works
again.

So if I do a typebounds on docker_t svirt_lxc_net_t, I have to make
sure docker_t can use svirt_sandbox_file_t as an entrypoint?


As currently defined/implemented, yes - a bounded type cannot be allowed 
anything that is not allowed to its parent.  If you had 
neverallow/hierarchy checking enabled in your policy build (e.g. 
expand-check=1 or manual semodule_expand test), then it would have 
triggered at link time.  Otherwise the kernel prunes it for you.




BTW we have a problem with type bounds, which only allows one.  We
would like to be able to say

typebounds unconfined_t svirt_lxc_net_t;
typebounds docker_t svirt_lxc_net_t;

This would allow runc and docker to transition to svirt_lxc_net_t, if
the user specified prctl(NO_NEW_PRIVS)

Currently typebounds only allows one instance.


It is a hierarchy, where each child has a single parent.  So you can 
define hierarchies like:

typebounds unconfined_t docker_t;
typebounds docker_t svirt_lxc_net_t;
and then they can both transition because they are both ancestors.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


[PATCH] libselinux: only mount /proc if necessary

2016-02-29 Thread Stephen Smalley
Commit 9df498884665d ("libselinux: Mount procfs before checking
/proc/filesystems") changed selinuxfs_exists() to always try
mounting /proc before reading /proc/filesystems.  However, this is
unnecessary if /proc is already mounted and can produce avc denials
if the process is not allowed to perform the mount.  Check first
to see if /proc is already present and only try the mount if it is not.

Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
---
 libselinux/src/init.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libselinux/src/init.c b/libselinux/src/init.c
index 3db4de0..3530594 100644
--- a/libselinux/src/init.c
+++ b/libselinux/src/init.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "dso.h"
 #include "policy.h"
@@ -57,13 +58,19 @@ static int verify_selinuxmnt(const char *mnt)
 
 int selinuxfs_exists(void)
 {
-   int exists = 0, mnt_rc = 0;
+   int exists = 0, mnt_rc = -1, rc;
+   struct statfs sb;
FILE *fp = NULL;
char *buf = NULL;
size_t len;
ssize_t num;
 
-   mnt_rc = mount("proc", "/proc", "proc", 0, 0);
+   do {
+   rc = statfs("/proc", );
+   } while (rc < 0 && errno == EINTR);
+
+   if (rc == 0 && ((uint32_t)sb.f_type != (uint32_t)PROC_SUPER_MAGIC))
+   mnt_rc = mount("proc", "/proc", "proc", 0, 0);
 
fp = fopen("/proc/filesystems", "r");
if (!fp) {
-- 
2.4.3

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: [PATCH 2/2] libselinux: procattr: return einval for <= 0 pid args.

2016-02-29 Thread Stephen Smalley

On 02/23/2016 04:22 PM, Nick Kralevich wrote:

Thanks for proposing this patch Dan.

As you said, the current API feels error prone, as it has two entirely
different behaviors depending on whether the pid is zero or non-zero.
Your patch corrects this error prone API and clearly separates out a
query by PID vs a query of the process itself.

This patch helps provide robustness against bugs similar to:

   https://code.google.com/p/google-security-research/issues/detail?id=727
   https://code.google.com/p/android/issues/detail?id=200617

(note that the Android code in question was reviewed by Stephen,
myself, and others within Google, and we all missed this particular
bug)

I would recommend the upstream SELinux community accept the patch,
even at the potential expense of breaking compatibility with other
apps.


As I haven't heard or seen of anything that would break with this 
change, I've merged this patch.  You'll need to apply it separately to 
Android libselinux since that is still a fork.




___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


  1   2   3   4   5   6   7   8   9   10   >