Bug#929063: SELinux integration in sysVinit

2019-05-24 Thread Stephen Smalley

On 5/24/19 9:00 AM, Jason Zaman wrote:

Bigon asked me to forward this so its part of the bug tracker.

On Fri, May 24, 2019 at 08:55:22PM +0800, Jason Zaman wrote:

On Fri, May 24, 2019 at 01:17:00PM +0200, Laurent Bigonville wrote:

Hello,

There is currently some discussion at [0] about SELinux integration in
sysVinit and the fact that somebody wants to delegate the loading of the
policy to an other binary than PID1.

Has somebody a remark or an objection to that proposal?


I object too. There is a *huge* change in functionality. Originally if
you boot with SELinux enforcing, there are only two things that can
happen. Either you end up with the policy loaded or the machine halts.

In the new patch, an attacker can just chmod -x /sbin/selinux-check then
next boot there will be no selinux enabled.

if (access(SELINUX_CHECK, X_OK) != 0) fails, the machine will continue
to boot without SELinux enabled. There is no difference between a user
without /sbin/selinux-check on purpose and an attacker just chmod -x'd
the tool.


NB even amending the patch to use F_OK still leaves one vulnerable to an 
attacker doing rm /sbin/selinux-check to silently disable SELinux on 
reboot.  And you can't assume that anyone who can do that can already 
modify or replace /sbin/init; under existing policies /sbin/init has its 
own distinct SELinux type, which would not be assigned to 
/sbin/selinux-check by default (and probably shouldn't be even in newer 
policies since it is the entrypoint type for the init domain).




SELinux does not protect /sbin anywhere near as much as /etc/selinux
(and doing that would probably be impossible). You'd need to check if
selinux is enabled and enforcing before skipping the loading ... which
is done by calling is_selinux_enabled() which needs linking to
libselinux. The important part of the original design is not
selinux_init_load_policy(), its is_selinux_enabled().

If someone really wants to run sysvinit without libselinux then just
switch to Gentoo, if you're on a non-selinux profile then you dont have
libselinux.so. But I'd definitely not want to have this patch in Gentoo
either.

-- Jason



I already gave my POV regarding SELinux integration in debian.

Kind regards,

Laurent Bigonville

[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=929063





Bug#857660: SELinux: cannot sent policyload notice

2017-03-21 Thread Stephen Smalley
On Tue, 2017-03-14 at 00:11 +0100, cgzones wrote:
> Hi list,
> I created bug report against dbus 1.10 on Debian [1] due to failing
> to
> send policyload notices.
> Are there any objections or comments on the upstream patch[2]?

Also, the patch looks correct to me.

> The patch works for me:
> 
> Mar 14 00:01:36 debianSE audit[441]: USER_AVC pid=441 uid=105
> auid=4294967295 ses=4294967295
> subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 msg='avc:
> received policyload notice (seqno=3)
>  exe="/usr/bin/dbus-daemon"
> sauid=105 hostname=? addr=? terminal=?'
> Mar 14 00:01:36 debianSE dbus[441]: [system] Reloaded configuration
> 
> Best regards,
>    Christian Göttsche
> 
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=857660
> [2] https://cgit.freedesktop.org/dbus/dbus/commit/?id=a3a5935a0a038c3
> b44c61ce5719f0f7e647b96c6



Bug#857660: SELinux: cannot sent policyload notice

2017-03-21 Thread Stephen Smalley
On Tue, 2017-03-14 at 00:11 +0100, cgzones wrote:
> Hi list,
> I created bug report against dbus 1.10 on Debian [1] due to failing
> to
> send policyload notices.
> Are there any objections or comments on the upstream patch[2]?

The patch has been working correctly in dbus 1.11 in Fedora for quite
some time.

> The patch works for me:
> 
> Mar 14 00:01:36 debianSE audit[441]: USER_AVC pid=441 uid=105
> auid=4294967295 ses=4294967295
> subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 msg='avc:
> received policyload notice (seqno=3)
>  exe="/usr/bin/dbus-daemon"
> sauid=105 hostname=? addr=? terminal=?'
> Mar 14 00:01:36 debianSE dbus[441]: [system] Reloaded configuration
> 
> Best regards,
>    Christian Göttsche
> 
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=857660
> [2] https://cgit.freedesktop.org/dbus/dbus/commit/?id=a3a5935a0a038c3
> b44c61ce5719f0f7e647b96c6



Bug#734806: [PATCH] Set self.sename to sename after calling semanage_seuser_set_sename()

2015-07-16 Thread Stephen Smalley
On 07/14/2015 01:07 PM, Laurent Bigonville wrote:
 From: Laurent Bigonville bi...@bigon.be
 
 This fixes audit information that are being logged and a crash when the
 python-audit binding is not installed.
 
 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=734806

Thanks, applied.

 ---
  policycoreutils/semanage/seobject.py | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/policycoreutils/semanage/seobject.py 
 b/policycoreutils/semanage/seobject.py
 index 568ebfd..2edb050 100644
 --- a/policycoreutils/semanage/seobject.py
 +++ b/policycoreutils/semanage/seobject.py
 @@ -575,6 +575,7 @@ class loginRecords(semanageRecords):
  
   if sename != :
   semanage_seuser_set_sename(self.sh, u, sename)
 +self.sename = sename
   else:
   self.sename = self.oldsename
   
 


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#510466: [PATCH] Add SELinux support to run jobs in the proper domain

2015-07-09 Thread Stephen Smalley
On 07/09/2015 07:42 AM, Laurent Bigonville wrote:
 From: Marcela Mašláňová mmasl...@redhat.com
 
 Currently, jobs run by at are run in the crond_t domain and not
 transitioned outside of it.
 
 With this patch, the jobs are transitioned in the same domain as the
 jobs that are run by the cron daemon:
 
 - When cron_userdomain_transition is set to off, a process for an
   unconfined user will transition to unconfined_cronjob_t. For confined
   user, the job is run as cronjob_t.
 
 - When cron_userdomain_transition is set to on, the processes are run
   under the user default context.
 
 Tested-by: Laurent Bigonville bi...@bigon.be
 ---
  Makefile.in  |  3 ++-
  atd.c| 81 
 
  config.h.in  |  3 +++
  configure.ac |  8 ++
  4 files changed, 94 insertions(+), 1 deletion(-)
 
 diff --git a/Makefile.in b/Makefile.in
 index 5dd2767..2bddc13 100644
 --- a/Makefile.in
 +++ b/Makefile.in
 @@ -40,6 +40,7 @@ LIBS= @LIBS@
  LIBOBJS  = @LIBOBJS@
  INSTALL  = @INSTALL@
  PAMLIB  = @PAMLIB@
 +SELINUXLIB  = @SELINUXLIB@
  
  CLONES   = atq atrm
  ATOBJECTS= at.o panic.o perm.o posixtm.o y.tab.o lex.yy.o
 @@ -73,7 +74,7 @@ at: $(ATOBJECTS)
   $(LN_S) -f at atrm
  
  atd: $(RUNOBJECTS)
 - $(CC) $(LDFLAGS) -o atd $(RUNOBJECTS) $(LIBS) $(PAMLIB)
 + $(CC) $(LDFLAGS) -o atd $(RUNOBJECTS) $(LIBS) $(PAMLIB) $(SELINUXLIB)
  
  y.tab.c y.tab.h: parsetime.y
   $(YACC) -d parsetime.y
 diff --git a/atd.c b/atd.c
 index d0b422e..55f6f8d 100644
 --- a/atd.c
 +++ b/atd.c
 @@ -83,6 +83,14 @@
  #include getloadavg.h
  #endif
  
 +#ifdef WITH_SELINUX
 +#include selinux/selinux.h
 +#include selinux/get_context_list.h
 +int selinux_enabled=0;
 +#include selinux/flask.h
 +#include selinux/av_permissions.h

Use of these headers (flask.h and av_permissions.h) is deprecated.
See below.

 +#endif
 +
  /* Macros */
  
  #define BATCH_INTERVAL_DEFAULT 60
 @@ -195,6 +203,68 @@ myfork()
  #define fork myfork
  #endif
  
 +#ifdef WITH_SELINUX
 +static int set_selinux_context(const char *name, const char *filename) {
 +   security_context_t user_context=NULL;
 +   security_context_t  file_context=NULL;
 +   struct av_decision avd;
 +   int retval=-1;
 +   char *seuser=NULL;
 +   char *level=NULL;
 +
 +   if (getseuserbyname(name, seuser, level) == 0) {
 +   retval=get_default_context_with_level(seuser, level, NULL, 
 user_context);
 +   free(seuser);
 +   free(level);
 +   if (retval) {
 +   if (security_getenforce()==1) {
 +   perr(execle: couldn't get security context 
 for user %s\n, name);
 +   } else {
 +   syslog(LOG_ERR, execle: couldn't get 
 security context for user %s\n, name);
 +   return -1;
 +   }
 +   }
 +   }
 +
 +   /*
 +   * Since crontab files are not directly executed,
 +   * crond must ensure that the crontab file has
 +   * a context that is appropriate for the context of
 +   * the user cron job.  It performs an entrypoint
 +   * permission check for this purpose.
 +   */
 +   if (fgetfilecon(STDIN_FILENO, file_context)  0)
 +   perr(fgetfilecon FAILED %s, filename);
 +
 +   retval = security_compute_av(user_context,
 +file_context,
 +SECCLASS_FILE,
 +FILE__ENTRYPOINT,
 +avd);
 +   freecon(file_context);
 +   if (retval || ((FILE__ENTRYPOINT  avd.allowed) != FILE__ENTRYPOINT)) 
 {
 +   if (security_getenforce()==1) {
 +   perr(Not allowed to set exec context to %s for user  
 %s\n, user_context,name);
 +   } else {
 +   syslog(LOG_ERR, Not allowed to set exec context to 
 %s for user  %s\n, user_context,name);
 +   retval = -1;
 +   goto err;
 +   }
 +   }

Please switch to using selinux_check_access() rather than
security_compute_av() in new code.  This handles dynamically looking up
the file class and entrypoint permission values, checks and uses
security_deny_unknown() to handle unknown classes/permissions, uses the
AVC, checks enforcing mode (both global and per-domain), and ensures
that denials are audited/logged in a standardized format.  You will also
want to use selinux_set_callback(SELINUX_CB_LOG, ...) and possibly
selinux_set_callback(SELINUX_CB_AUDIT, ...) to redirect the logging to
your preferred destination and to supply any auxiliary audit data in the
message.

 +   if (setexeccon(user_context)  0) {
 +   if (security_getenforce()==1) {
 +   perr(Could not set exec context to %s for 

Bug#786956: getpidcon() behaviour when other LSM is enabled

2015-05-28 Thread Stephen Smalley
On 05/28/2015 02:06 PM, Laurent Bigonville wrote:
 Hello,
 
 In procps(-ng)[0] when the use of libselinux is enabled at build time,
 it always uses getpidcon() even if an other (or no) LSM is enabled.
 
 I tried to use getpidcon() (via the cmd tool getpidcon) with apparmor
 enabled instead of selinux, and it returned the apparmor context.
 Is this expected and can we rely on this?

Fundamentally, getpidcon() just reads the value of
/proc/pid/attr/current into a dynamically allocated buffer and returns
it.  That part should work for any security module.  The only other
thing getpidcon() does is pass the context to mcstransd for context
translation if mcstransd is running.  That could potentially break if
you happen to be running mcstransd on a non-SELinux system, although I
don't know why anyone would.  Possibly we ought to have mcstransd test
is_selinux_enabled() and bail immediately if it is disabled just to
preclude that.

 Otherwise, I've prepared the attached patch. Would this patch be
 acceptable?
 
 Cheers,
 
 Laurent Bigonville
 
 [0] https://gitlab.com/procps-ng/procps/blob/master/ps/output.c#L1237
 
 
 
 ___
 Selinux mailing list
 seli...@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.
 


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#770492: [RFC PATCH RESEND] vfs: Move security_inode_killpriv() after permission checks

2015-01-21 Thread Stephen Smalley
On 01/20/2015 06:17 PM, James Morris wrote:
 On Sat, 17 Jan 2015, Ben Hutchings wrote:
 
 chown() and write() should clear all privilege attributes on
 a file - setuid, setgid, setcap and any other extended
 privilege attributes.

 However, any attributes beyond setuid and setgid are managed by the
 LSM and not directly by the filesystem, so they cannot be set along
 with the other attributes.

 Currently we call security_inode_killpriv() in notify_change(),
 but in case of a chown() this is too early - we have not called
 inode_change_ok() or made any filesystem-specific permission/sanity
 checks.

 Add a new function setattr_killpriv() which calls
 security_inode_killpriv() if necessary, and change the setattr()
 implementation to call this in each filesystem that supports xattrs.
 This assumes that extended privilege attributes are always stored in
 xattrs.
 
 It'd be useful to get some input from LSM module maintainers on this. 
 
 e.g. doesn't SELinux already handle this via policy directives?

There have been a couple postings of a similar patch set [1] by Jan
Kara, although I don't believe that series addressed chown().

If I am reading the patches correctly, they (correctly) don't affect
SELinux or Smack labels; they are just calling the existing
security_inode_killpriv() hook, which is only implemented for the
capability module to remove the security.capability xattr.

[1] http://marc.info/?l=linux-security-modulem=141890696325054w=2


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#543420: upstart: SELinux support

2009-09-18 Thread Stephen Smalley
On Fri, 2009-09-11 at 18:04 +0200, Michael Biebl wrote:
 Russell Coker wrote:
 
  
  Philipp, thanks for your bug report.  I'm a bit short of time this week, I 
  would appreciate if it you could do some tests with upstart compiled with 
  this patch.  I will of course do all ongoing maintenance on this patch to 
  keep it up to date.
 
 
 Regarding this patch: Is there a reason why init needs to be linked against
 libsepol? From what I can see in the diff, it only uses functions from 
 libselinux.

I don't think that is needed anymore; it was a legacy of older
libselinux.  Modern libselinux will dlopen libsepol upon the call to
selinux_init_load_policy().

-- 
Stephen Smalley
National Security Agency




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#474339: dpkg bug related to SE Linux

2008-04-08 Thread Stephen Smalley

On Tue, 2008-04-08 at 10:30 +1100, Russell Coker wrote:
 On Tuesday 08 April 2008 02:55, Stephen Smalley [EMAIL PROTECTED] wrote:
  On Mon, 2008-04-07 at 12:48 -0400, Stephen Smalley wrote:
   On Sat, 2008-04-05 at 13:47 +1100, Russell Coker wrote:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=474339
   
When the file_contexts file has an invalid context then dpkg will
crash, see the above bug report.
   
dpkg is statically linked against libselinux.
  
   Can you reproduce with a simpler caller, e.g. compile matchpathcon from
   libselinux/utils statically and run it on your corrupted file_contexts
   file, passing in a pathname that will match the invalid line?
 
 Restorecon doesn't have this problem, neither when dynamically nor statically 
 linked.

In that case, I'd recommend looking more closely at the dpkg selinux
code itself rather than libselinux.  For example, does dpkg try to free
the context from matchpathcon() upon an error return?  That would be a
bug in it rather than libselinux.  Or is dpkg double free'ing a returned
context?

-- 
Stephen Smalley
National Security Agency




-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]