selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Adding audit messge to newrole
@ 2005-12-20 22:14 Steve G
  2005-12-20 22:39 ` Timothy R. Chavez
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Steve G @ 2005-12-20 22:14 UTC (permalink / raw)
  To: SE-Linux

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

Hi,

I am attaching a patch that lets newrole send messages to the audit system. This
patch needs review as it changes newrole to be setuid root. The patch drops
capabilities very soon after startup. 

You will need recent audit package in order to compile newrole since it uses the
USER_ROLE_CHANGE message type. It is available in version 1.1.2.

Thanks,
-Steve

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

[-- Attachment #2: 4077945729-policycoreutils-1.29.2-newrole-audit.patch --]
[-- Type: application/octet-stream, Size: 4576 bytes --]

diff -urp policycoreutils-1.29.2.orig/newrole/Makefile policycoreutils-1.29.2/newrole/Makefile
--- policycoreutils-1.29.2.orig/newrole/Makefile	2005-12-20 10:51:42.000000000 -0500
+++ policycoreutils-1.29.2/newrole/Makefile	2005-12-20 16:58:50.000000000 -0500
@@ -8,7 +8,7 @@ PAMH = $(shell ls /usr/include/security/
 
 CFLAGS ?= -Werror -Wall -W
 override CFLAGS += $(LDFLAGS) -I$(PREFIX)/include -DUSE_NLS -DLOCALEDIR="\"$(LOCALEDIR)\"" -DPACKAGE="\"policycoreutils\""
-LDLIBS += -lselinux -lsepol -L$(PREFIX)/lib
+LDLIBS += -lselinux -lsepol -L$(PREFIX)/lib -lcap -laudit
 ifeq (${PAMH}, /usr/include/security/pam_appl.h)
 	override CFLAGS += -DUSE_PAM
 	LDLIBS += -lpam -lpam_misc
@@ -26,7 +26,7 @@ install: all
 	test -d $(BINDIR)      || install -m 755 -d $(BINDIR)
 	test -d $(ETCDIR)/pam.d || install -m 755 -d $(ETCDIR)/pam.d
 	test -d $(MANDIR)/man1 || install -m 755 -d $(MANDIR)/man1
-	install -m 555 newrole $(BINDIR)
+	install -m 4555 newrole $(BINDIR)
 	install -m 644 newrole.1 $(MANDIR)/man1/
 ifeq (${PAMH}, /usr/include/security/pam_appl.h)
 	test -d $(ETCDIR)/pam.d || install -m 755 -d $(ETCDIR)/pam.d
diff -urp policycoreutils-1.29.2.orig/newrole/newrole.c policycoreutils-1.29.2/newrole/newrole.c
--- policycoreutils-1.29.2.orig/newrole/newrole.c	2005-12-20 15:29:44.000000000 -0500
+++ policycoreutils-1.29.2/newrole/newrole.c	2005-12-20 17:06:12.000000000 -0500
@@ -62,6 +62,9 @@
 #include <selinux/get_default_type.h>
 #include <selinux/get_context_list.h> /* for SELINUX_DEFAULTUSER */
 #include <signal.h>
+#include <sys/prctl.h>
+#include <sys/capability.h>
+#include <libaudit.h>
 #ifdef USE_NLS
 #include <locale.h>			    /* for setlocale() */
 #include <libintl.h>			    /* for gettext() */
@@ -332,6 +335,48 @@ static int verify_shell(const char *shel
   return found;
 }
 
+/*
+ * This function will drop the capabilities so that we are left
+ * only with access to the audit system. If the user is root, we leave
+ * the capabilities alone since they already should have access to the
+ * audit netlink socket.
+ */
+static void drop_capabilities(void)
+{
+  uid_t uid = getuid();
+
+  if (uid) { /* Non-root path */
+    cap_t new_caps;
+    cap_value_t cap_list[] = { CAP_AUDIT_WRITE };
+
+    /* Keep capabilities across uid change */
+    prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
+
+    /* Change uid */
+    if (setuid(uid)) {
+      fprintf(stderr, _("Error changing uid, aborting.\n"));
+      exit(-1);
+    }
+
+    /* Now get rid of this ability */
+    prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0);
+
+    /* We should still have root's caps, so let drop them. */
+    new_caps = cap_init();
+    if (!new_caps) {
+      fprintf(stderr, _("Error initing capabilities, aborting.\n"));
+      exit(-1);
+    }
+    cap_set_flag(new_caps, CAP_PERMITTED, 1, cap_list, CAP_SET);
+    cap_set_flag(new_caps, CAP_EFFECTIVE, 1, cap_list, CAP_SET);
+    if (cap_set_proc(new_caps)) {
+      fprintf(stderr, _("Error dropping capabilities, aborting"));
+      exit(-1);
+    }
+    cap_free(new_caps);
+  }
+}
+
 /************************************************************************
  *
  * All code used for both PAM and shadow passwd goes in this section.
@@ -401,6 +446,8 @@ int main( int argc, char *argv[] ) {
     exit(-1);
   }
 
+  drop_capabilities();
+
   while (1) {
     clflag=getopt_long(argc,argv,"r:t:l:",long_options,&flag_index);
     if (clflag == -1)
@@ -496,7 +543,6 @@ int main( int argc, char *argv[] ) {
     exit(-1);
   }
 
-  freecon(old_context);
   /* Make `pw' point to a structure containing the data              *
    * from our user's line in the passwd file.  If the current user's
    * SELinux user identity is the default (SELINUX_DEFAULTUSER), then
@@ -753,6 +799,23 @@ int main( int argc, char *argv[] ) {
        fprintf(stderr, _("Could not set exec context to %s.\n"), new_context);
        exit(-1);
   }
+  /* Send audit message */
+  {
+    char msg[128];  /* This number is a guess */
+    int audit_fd = audit_open();
+    if (audit_fd < 0) {
+       fprintf(stderr, _("Error connecting to audit system.\n"));
+       exit(-1);
+    }
+    snprintf(msg, sizeof(msg), "newrole: old-context=%s new-context=%s",
+             old_context, new_context);
+    if (audit_log_user_message(audit_fd, AUDIT_USER_ROLE_CHANGE,
+                               msg, NULL, NULL, ttyn, 1) <= 0) {
+       fprintf(stderr, _("Error sending audit message.\n"));
+       exit(-1);
+    }
+    close(audit_fd);
+  }
   execv(argv[optind-1],argv+optind-1);
 
   /* If we reach here, then we failed to exec the new shell. */

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-20 22:14 Adding audit messge to newrole Steve G
@ 2005-12-20 22:39 ` Timothy R. Chavez
  2005-12-20 22:42 ` Timothy R. Chavez
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Timothy R. Chavez @ 2005-12-20 22:39 UTC (permalink / raw)
  To: Steve G; +Cc: SE-Linux

On Tuesday 20 December 2005 16:14, Steve G wrote:
> Hi,
> 
> I am attaching a patch that lets newrole send messages to the audit system. This
> patch needs review as it changes newrole to be setuid root. The patch drops
> capabilities very soon after startup. 

I'm lazy and would prefer something inline.  Oh and the patch seems to be empty... hrm.

-tim

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-20 22:14 Adding audit messge to newrole Steve G
  2005-12-20 22:39 ` Timothy R. Chavez
@ 2005-12-20 22:42 ` Timothy R. Chavez
  2005-12-21 16:10 ` Stephen Smalley
  2005-12-21 16:19 ` Stephen Smalley
  3 siblings, 0 replies; 13+ messages in thread
From: Timothy R. Chavez @ 2005-12-20 22:42 UTC (permalink / raw)
  To: Steve G; +Cc: SE-Linux

On Tuesday 20 December 2005 16:14, Steve G wrote:
> 4077945729-policycoreutils-1.29.2-newrole-audit.patch

Oh nevermind, my bad... I was looking at this... hm.

-tim

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-20 22:14 Adding audit messge to newrole Steve G
  2005-12-20 22:39 ` Timothy R. Chavez
  2005-12-20 22:42 ` Timothy R. Chavez
@ 2005-12-21 16:10 ` Stephen Smalley
  2005-12-21 16:42   ` Stephen Smalley
                     ` (2 more replies)
  2005-12-21 16:19 ` Stephen Smalley
  3 siblings, 3 replies; 13+ messages in thread
From: Stephen Smalley @ 2005-12-21 16:10 UTC (permalink / raw)
  To: Steve G; +Cc: Chris Wright, SE-Linux

On Tue, 2005-12-20 at 14:14 -0800, Steve G wrote:
> Hi,
> 
> I am attaching a patch that lets newrole send messages to the audit system. This
> patch needs review as it changes newrole to be setuid root. The patch drops
> capabilities very soon after startup. 
> 
> You will need recent audit package in order to compile newrole since it uses the
> USER_ROLE_CHANGE message type. It is available in version 1.1.2.

cc'd Chris for review/discussion of the capability dropping code.

diff -urp policycoreutils-1.29.2.orig/newrole/newrole.c policycoreutils-1.29.2/newrole/newrole.c
--- policycoreutils-1.29.2.orig/newrole/newrole.c	2005-12-20 15:29:44.000000000 -0500
+++ policycoreutils-1.29.2/newrole/newrole.c	2005-12-20 17:06:12.000000000 -0500
@@ -332,6 +335,48 @@ static int verify_shell(const char *shel
   return found;
 }
 
+/*
+ * This function will drop the capabilities so that we are left
+ * only with access to the audit system. If the user is root, we leave
+ * the capabilities alone since they already should have access to the
+ * audit netlink socket.
+ */
+static void drop_capabilities(void)
+{
+  uid_t uid = getuid();
+
+  if (uid) { /* Non-root path */
+    cap_t new_caps;
+    cap_value_t cap_list[] = { CAP_AUDIT_WRITE };
+
+    /* Keep capabilities across uid change */
+    prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
+
+    /* Change uid */
+    if (setuid(uid)) {
+      fprintf(stderr, _("Error changing uid, aborting.\n"));
+      exit(-1);
+    }
+
+    /* Now get rid of this ability */
+    prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0);
+
+    /* We should still have root's caps, so let drop them. */
+    new_caps = cap_init();
+    if (!new_caps) {
+      fprintf(stderr, _("Error initing capabilities, aborting.\n"));
+      exit(-1);
+    }
+    cap_set_flag(new_caps, CAP_PERMITTED, 1, cap_list, CAP_SET);
+    cap_set_flag(new_caps, CAP_EFFECTIVE, 1, cap_list, CAP_SET);
+    if (cap_set_proc(new_caps)) {
+      fprintf(stderr, _("Error dropping capabilities, aborting"));
+      exit(-1);
+    }
+    cap_free(new_caps);
+  }
+}

Why don't you drop capabilities first, then switch to the real uid?
Otherwise, you have a task with the caller's uid and full capabilities
for an interval of time, right?

@@ -401,6 +446,8 @@ int main( int argc, char *argv[] ) {
     exit(-1);
   }
 
+  drop_capabilities();
+
   while (1) {
     clflag=getopt_long(argc,argv,"r:t:l:",long_options,&flag_index);
     if (clflag == -1)

Any reason we can't move this up earlier in main()?  Ideally, it should
be the first thing in main() to ensure that everything else runs under
the caller's uid as before.

@@ -753,6 +799,23 @@ int main( int argc, char *argv[] ) {
        fprintf(stderr, _("Could not set exec context to %s.\n"), new_context);
        exit(-1);
   }
+  /* Send audit message */
+  {
+    char msg[128];  /* This number is a guess */
+    int audit_fd = audit_open();
+    if (audit_fd < 0) {
+       fprintf(stderr, _("Error connecting to audit system.\n"));
+       exit(-1);
+    }
+    snprintf(msg, sizeof(msg), "newrole: old-context=%s new-context=%s",
+             old_context, new_context);

Why not just dynamically allocate the msg buffer to be of the right
size?  There is the potential for memory allocation failure, but we
should just abort in that case, and audit_log_user_message can fail
anyway.

+    if (audit_log_user_message(audit_fd, AUDIT_USER_ROLE_CHANGE,
+                               msg, NULL, NULL, ttyn, 1) <= 0) {
+       fprintf(stderr, _("Error sending audit message.\n"));
+       exit(-1);
+    }
+    close(audit_fd);
+  }
   execv(argv[optind-1],argv+optind-1);
 
   /* If we reach here, then we failed to exec the new shell. */

Also, I think that the audit-related code should be separately #ifdef'd
and made optional (and newrole shouldn't be made setuid unless building
with that option).  At the very least, it needs to be optional because
not every distro that uses SELinux has audit integrated yet, and some
people may specifically not want to require a setuid newrole (despite
the capability dropping measures).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-20 22:14 Adding audit messge to newrole Steve G
                   ` (2 preceding siblings ...)
  2005-12-21 16:10 ` Stephen Smalley
@ 2005-12-21 16:19 ` Stephen Smalley
  2005-12-21 17:56   ` Steve G
  3 siblings, 1 reply; 13+ messages in thread
From: Stephen Smalley @ 2005-12-21 16:19 UTC (permalink / raw)
  To: Steve G; +Cc: SE-Linux

On Tue, 2005-12-20 at 14:14 -0800, Steve G wrote:
> Hi,
> 
> I am attaching a patch that lets newrole send messages to the audit system. This
> patch needs review as it changes newrole to be setuid root. The patch drops
> capabilities very soon after startup. 
> 
> You will need recent audit package in order to compile newrole since it uses the
> USER_ROLE_CHANGE message type. It is available in version 1.1.2.

In addition to the comments on the patch, it doesn't seem to work for me
on rawhide, either as root or a non-root user:

# /usr/bin/newrole -r sysadm_r -t sysadm_t
Authenticating root.
Password:
Error sending audit message.

$ newrole -r sysadm_r -t sysadm_t
Authenticating sds.
Password:
Error sending audit message.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-21 16:10 ` Stephen Smalley
@ 2005-12-21 16:42   ` Stephen Smalley
  2005-12-21 17:41   ` Steve G
  2005-12-22  4:10   ` Chris Wright
  2 siblings, 0 replies; 13+ messages in thread
From: Stephen Smalley @ 2005-12-21 16:42 UTC (permalink / raw)
  To: Steve G; +Cc: Chris Wright, SE-Linux

On Wed, 2005-12-21 at 11:10 -0500, Stephen Smalley wrote:
> Why don't you drop capabilities first, then switch to the real uid?
> Otherwise, you have a task with the caller's uid and full capabilities
> for an interval of time, right?

Hmm...I see - you need CAP_SETUID around still in the effective set to
perform the setuid() call to the real uid.  Looking about for how others
handle this same logic, I did see:
http://lists.samba.org/archive/rsync/2004-January/008403.html

Looks like they shed the capabilities in the permitted and inheritable
sets before the setuid(), then drop from the effective set afterward.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-21 16:10 ` Stephen Smalley
  2005-12-21 16:42   ` Stephen Smalley
@ 2005-12-21 17:41   ` Steve G
  2005-12-21 18:09     ` Stephen Smalley
  2005-12-22  4:10   ` Chris Wright
  2 siblings, 1 reply; 13+ messages in thread
From: Steve G @ 2005-12-21 17:41 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Chris Wright, SE-Linux


>Why don't you drop capabilities first, then switch to the real uid?

You answered yourself. Need setuid capabilities.

@@ -401,6 +446,8 @@ int main( int argc, char *argv[] ) {
     exit(-1);
   }
 
+  drop_capabilities();
+
   while (1) {
     clflag=getopt_long(argc,argv,"r:t:l:",long_options,&flag_index);
     if (clflag == -1)

Any reason we can't move this up earlier in main()?

I suppose we could move it above the selinux enabled call.

>Ideally, it should be the first thing in main() to ensure that everything 
>else runs under the caller's uid as before.

But after the bindtext call for localization?

>@@ -753,6 +799,23 @@ int main( int argc, char *argv[] ) {
>        fprintf(stderr, _("Could not set exec context to %s.\n"), >new_context);
>        exit(-1);
>   }
>+  /* Send audit message */
>+  {
>+    char msg[128];  /* This number is a guess */
>
>Why not just dynamically allocate the msg buffer to be of the right
>size?

I generally prefer to use the stack on small programs. Its less complicated and
runs faster. We aren't short for stack space in this program. My comment may be
misleading you. i wanted to say that its an arbitrary number and can be changed
if someone decideds its safe to make it bigger or smaller. Is there a define that
has the maximum string representation of a context?

>Also, I think that the audit-related code should be separately #ifdef'd
>and made optional (and newrole shouldn't be made setuid unless building
>with that option). 

I can do that. I didn't want to clutter the patch on the first go around. I knew
there would be changes to it.

-Steve

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-21 16:19 ` Stephen Smalley
@ 2005-12-21 17:56   ` Steve G
  2005-12-21 18:26     ` Stephen Smalley
  0 siblings, 1 reply; 13+ messages in thread
From: Steve G @ 2005-12-21 17:56 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: SE-Linux


>In addition to the comments on the patch, it doesn't seem to work for me
>on rawhide, either as root or a non-root user:

You may need an appropriately patched kernel. I am testing on the lspp.4 kernel
from:  http://david.woodhou.se/lspp/kernel/

this is what I get:

[root@localhost ~]# newrole -r system_r -t unconfined_t
Authenticating root.
Password:
[root@localhost ~]# ausearch -ts 12:44:00 -m USER_ROLE_CHANGE
----
time->Wed Dec 21 12:45:08 2005
type=USER_ROLE_CHANGE msg=audit(1135187108.964:67): user pid=2549 uid=0 auid=0
msg='newrole: old context=root:system_r:unconfined_t:SystemLow-SystemHigh new
context=root:system_r:unconfined_t:SystemLow-SystemHig: exe="/usr/bin/newrole"
(hostname=?, addr=?, terminal=/dev/tty1 res=success)'

-Steve

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-21 17:41   ` Steve G
@ 2005-12-21 18:09     ` Stephen Smalley
  2005-12-22  4:19       ` Chris Wright
  0 siblings, 1 reply; 13+ messages in thread
From: Stephen Smalley @ 2005-12-21 18:09 UTC (permalink / raw)
  To: Steve G; +Cc: Chris Wright, SE-Linux

On Wed, 2005-12-21 at 09:41 -0800, Steve G wrote:
> >Why don't you drop capabilities first, then switch to the real uid?
> 
> You answered yourself. Need setuid capabilities.

Yes, but I'm still not sure about the implications.  Not all kernel
operations compare capability sets, e.g. signals only compare the uids
of the relevant tasks.  So if you switch to the caller's uid while still
possessing all capabilities, you may be opening yourself to manipulation
by the caller.  ptrace does compare the permitted sets for a subset
relationship.  Might still be safer to shed everything you can first,
and then drop CAP_SETUID last after the setuid.

> Any reason we can't move this up earlier in main()?
> 
> I suppose we could move it above the selinux enabled call.
> 
> >Ideally, it should be the first thing in main() to ensure that everything 
> >else runs under the caller's uid as before.
> 
> But after the bindtext call for localization?

I would assume before, as you otherwise risk still having uid 0 and
capabilities at that point if there is some locale-related exploit.
Purging the environment on entry to main() wouldn't hurt either.
http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/

> I generally prefer to use the stack on small programs. Its less complicated and
> runs faster. We aren't short for stack space in this program. My comment may be
> misleading you. i wanted to say that its an arbitrary number and can be changed
> if someone decideds its safe to make it bigger or smaller. Is there a define that
> has the maximum string representation of a context?

No, there is no maximum imposed by SELinux itself.  Certain kernel
interfaces (/proc/pid/attr, selinuxfs) presently limit them to no more
than PAGE_SIZE, but the core SELinux code doesn't bound them.  xattrs
are only "limited" to 64K.  In any event, with the fixed value, you risk
truncation of the audit message (especially the new context). 

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-21 17:56   ` Steve G
@ 2005-12-21 18:26     ` Stephen Smalley
  0 siblings, 0 replies; 13+ messages in thread
From: Stephen Smalley @ 2005-12-21 18:26 UTC (permalink / raw)
  To: Steve G; +Cc: SE-Linux

On Wed, 2005-12-21 at 09:56 -0800, Steve G wrote:
> >In addition to the comments on the patch, it doesn't seem to work for me
> >on rawhide, either as root or a non-root user:
> 
> You may need an appropriately patched kernel. I am testing on the lspp.4 kernel
> from:  http://david.woodhou.se/lspp/kernel/

Ok, just another reason to make it all conditional so that people can
still build and use newrole on unpatched kernels (or older kernels once
the kernel patches go upstream).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-21 16:10 ` Stephen Smalley
  2005-12-21 16:42   ` Stephen Smalley
  2005-12-21 17:41   ` Steve G
@ 2005-12-22  4:10   ` Chris Wright
  2 siblings, 0 replies; 13+ messages in thread
From: Chris Wright @ 2005-12-22  4:10 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Steve G, Chris Wright, SE-Linux

* Stephen Smalley (sds@tycho.nsa.gov) wrote:
> On Tue, 2005-12-20 at 14:14 -0800, Steve G wrote:
> +static void drop_capabilities(void)
> +{
> +  uid_t uid = getuid();
> +
> +  if (uid) { /* Non-root path */
> +    cap_t new_caps;
> +    cap_value_t cap_list[] = { CAP_AUDIT_WRITE };
> +
> +    /* Keep capabilities across uid change */
> +    prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
> +
> +    /* Change uid */
> +    if (setuid(uid)) {
> +      fprintf(stderr, _("Error changing uid, aborting.\n"));
> +      exit(-1);
> +    }
> +
> +    /* Now get rid of this ability */
> +    prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0);
> +
> +    /* We should still have root's caps, so let drop them. */
> +    new_caps = cap_init();
> +    if (!new_caps) {
> +      fprintf(stderr, _("Error initing capabilities, aborting.\n"));
> +      exit(-1);
> +    }
> +    cap_set_flag(new_caps, CAP_PERMITTED, 1, cap_list, CAP_SET);
> +    cap_set_flag(new_caps, CAP_EFFECTIVE, 1, cap_list, CAP_SET);
> +    if (cap_set_proc(new_caps)) {
> +      fprintf(stderr, _("Error dropping capabilities, aborting"));

While moot, it's a leak ;-)

> +      exit(-1);
> +    }
> +    cap_free(new_caps);
> +  }
> +}
> 
> Why don't you drop capabilities first, then switch to the real uid?
> Otherwise, you have a task with the caller's uid and full capabilities
> for an interval of time, right?

As you noted later, you must retain at least CAP_SETUID (and
CAP_AUDIT_WRITE) in order to drop uid.  It should be done as early as
possible during startup.

thanks,
-chris

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Adding audit messge to newrole
  2005-12-21 18:09     ` Stephen Smalley
@ 2005-12-22  4:19       ` Chris Wright
  0 siblings, 0 replies; 13+ messages in thread
From: Chris Wright @ 2005-12-22  4:19 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Steve G, Chris Wright, SE-Linux

* Stephen Smalley (sds@tycho.nsa.gov) wrote:
> Yes, but I'm still not sure about the implications.  Not all kernel
> operations compare capability sets, e.g. signals only compare the uids
> of the relevant tasks.  So if you switch to the caller's uid while still
> possessing all capabilities, you may be opening yourself to manipulation
> by the caller.  ptrace does compare the permitted sets for a subset
> relationship.  Might still be safer to shed everything you can first,
> and then drop CAP_SETUID last after the setuid.

It's definitely safer.  Regarding signals, given it's setuid, you can
still send it signals before it has dropped uid.

> > Any reason we can't move this up earlier in main()?
> > 
> > I suppose we could move it above the selinux enabled call.
> > 
> > >Ideally, it should be the first thing in main() to ensure that everything 
> > >else runs under the caller's uid as before.
> > 
> > But after the bindtext call for localization?
> 
> I would assume before, as you otherwise risk still having uid 0 and
> capabilities at that point if there is some locale-related exploit.
> Purging the environment on entry to main() wouldn't hurt either.
> http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/

Agreed.

thanks,
-chris

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Adding audit messge to newrole
@ 2006-01-25 17:46 Steve G
  0 siblings, 0 replies; 13+ messages in thread
From: Steve G @ 2006-01-25 17:46 UTC (permalink / raw)
  To: SE-Linux

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

Hi,

I am attaching a patch that lets newrole send messages to the audit system. This
patch needs review as it changes newrole to be setuid root. The patch drops
capabilities immediately after startup. This patch depends on USE_AUDIT already
being defined by the Makefile.

You will need recent audit package in order to compile newrole since it uses the
USER_ROLE_CHANGE message type. It is available in version 1.1.2 and later. 

Thanks,
-Steve

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 374981619-policycoreutils-1.29.9-newrole-audit.patch --]
[-- Type: text/x-patch; name="policycoreutils-1.29.9-newrole-audit.patch", Size: 5210 bytes --]

diff -ur policycoreutils-1.29.9.orig/newrole/Makefile policycoreutils-1.29.9/newrole/Makefile
--- policycoreutils-1.29.9.orig/newrole/Makefile	2006-01-25 11:24:30.000000000 -0500
+++ policycoreutils-1.29.9/newrole/Makefile	2006-01-25 11:24:50.000000000 -0500
@@ -8,7 +8,7 @@
 
 CFLAGS ?= -Werror -Wall -W
 override CFLAGS += $(LDFLAGS) -I$(PREFIX)/include -DUSE_NLS -DLOCALEDIR="\"$(LOCALEDIR)\"" -DPACKAGE="\"policycoreutils\""
-LDLIBS += -lselinux -lsepol -L$(PREFIX)/lib
+LDLIBS += -lselinux -lsepol -L$(PREFIX)/lib -lcap -laudit
 ifeq (${PAMH}, /usr/include/security/pam_appl.h)
 	override CFLAGS += -DUSE_PAM
 	LDLIBS += -lpam -lpam_misc
@@ -26,7 +26,7 @@
 	test -d $(BINDIR)      || install -m 755 -d $(BINDIR)
 	test -d $(ETCDIR)/pam.d || install -m 755 -d $(ETCDIR)/pam.d
 	test -d $(MANDIR)/man1 || install -m 755 -d $(MANDIR)/man1
-	install -m 555 newrole $(BINDIR)
+	install -m 4555 newrole $(BINDIR)
 	install -m 644 newrole.1 $(MANDIR)/man1/
 ifeq (${PAMH}, /usr/include/security/pam_appl.h)
 	test -d $(ETCDIR)/pam.d || install -m 755 -d $(ETCDIR)/pam.d
diff -ur policycoreutils-1.29.9.orig/newrole/newrole.c policycoreutils-1.29.9/newrole/newrole.c
--- policycoreutils-1.29.9.orig/newrole/newrole.c	2006-01-25 11:24:30.000000000 -0500
+++ policycoreutils-1.29.9/newrole/newrole.c	2006-01-25 12:20:18.000000000 -0500
@@ -56,6 +58,10 @@
 #include <fcntl.h>
 #include <string.h>
 #include <errno.h>
+#ifdef USE_AUDIT
+#include <sys/prctl.h>
+#include <sys/capability.h>
+#endif
 #include <selinux/selinux.h>      /* for is_selinux_enabled() */
 #include <selinux/flask.h>        /* for SECCLASS_CHR_FILE */
 #include <selinux/context.h>      /* for context-mangling functions */
@@ -322,6 +328,65 @@
   return found;
 }
 
+/*
+ * This function will drop the capabilities so that we are left
+ * only with access to the audit system. If the user is root, we leave
+ * the capabilities alone since they already should have access to the
+ * audit netlink socket.
+ */
+#ifdef USE_AUDIT
+static void drop_capabilities(void)
+{
+  uid_t uid = getuid();
+
+  if (uid) { /* Non-root path */
+    cap_t new_caps, tmp_caps;
+    cap_value_t cap_list[] = { CAP_AUDIT_WRITE };
+    cap_value_t tmp_cap_list[] = { CAP_AUDIT_WRITE, CAP_SETUID };
+
+    new_caps = cap_init();
+    tmp_caps = cap_init();
+    if (!new_caps || !tmp_caps) {
+      fprintf(stderr, _("Error initing capabilities, aborting.\n"));
+      exit(-1);
+    }
+    cap_set_flag(new_caps, CAP_PERMITTED, 1, cap_list, CAP_SET);
+    cap_set_flag(new_caps, CAP_EFFECTIVE, 1, cap_list, CAP_SET);
+    cap_set_flag(tmp_caps, CAP_PERMITTED, 1, tmp_cap_list, CAP_SET);
+    cap_set_flag(tmp_caps, CAP_EFFECTIVE, 1, tmp_cap_list, CAP_SET);
+
+    /* Keep capabilities across uid change */
+    prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
+
+    /* We should still have root's caps, so drop most capabilities now */
+    if (cap_set_proc(tmp_caps)) {
+      fprintf(stderr, _("Error dropping capabilities, aborting\n"));
+      exit(-1);
+    }
+    cap_free(tmp_caps);
+
+    /* Change uid */
+    if (setuid(uid)) {
+      fprintf(stderr, _("Error changing uid, aborting.\n"));
+      exit(-1);
+    }
+
+    /* Now get rid of this ability */
+    if (prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0) < 0) {
+      fprintf(stderr, _("Error resetting KEEPCAPS, aborting\n"));
+      exit(-1);
+    }
+
+    /* Finish dropping capabilities. */
+    if (cap_set_proc(new_caps)) {
+      fprintf(stderr, _("Error dropping SETUID capability, aborting\n"));
+      exit(-1);
+    }
+    cap_free(new_caps);
+  }
+}
+#endif
+
 /************************************************************************
  *
  * All code used for both PAM and shadow passwd goes in this section.
@@ -360,6 +425,10 @@
   int enforcing;
   sigset_t empty;
 
+#ifdef USE_AUDIT
+  drop_capabilities();
+#endif
+
   /* Empty the signal mask in case someone is blocking a signal */
   sigemptyset( &empty );
   (void) sigprocmask( SIG_SETMASK, &empty, NULL );
@@ -486,7 +555,6 @@
     exit(-1);
   }
 
-  freecon(old_context);
   /* Make `pw' point to a structure containing the data              *
    * from our user's line in the passwd file.  If the current user's
    * SELinux user identity is the default (SELINUX_DEFAULTUSER), then
@@ -743,6 +811,31 @@
        fprintf(stderr, _("Could not set exec context to %s.\n"), new_context);
        exit(-1);
   }
+#ifdef USE_AUDIT
+  /* Send audit message */
+  {
+    char *msg;
+    int audit_fd = audit_open();
+    if (audit_fd < 0) {
+       fprintf(stderr, _("Error connecting to audit system.\n"));
+       exit(-1);
+    }
+    if (asprintf(&msg, "newrole: old-context=%s new-context=%s",
+             old_context, new_context) < 0) {
+       fprintf(stderr, _("Error allocating memory.\n"));
+       exit(-1);
+    }
+    if (audit_log_user_message(audit_fd, AUDIT_USER_ROLE_CHANGE,
+                               msg, NULL, NULL, ttyn, 1) <= 0) {
+       fprintf(stderr, _("Error sending audit message.\n"));
+       free(msg);
+       exit(-1);
+    }
+    free(msg);
+    close(audit_fd);
+  }
+#endif
+  freecon(old_context);
   execv(argv[optind-1],argv+optind-1);
 
   /* If we reach here, then we failed to exec the new shell. */

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2006-01-25 17:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-12-20 22:14 Adding audit messge to newrole Steve G
2005-12-20 22:39 ` Timothy R. Chavez
2005-12-20 22:42 ` Timothy R. Chavez
2005-12-21 16:10 ` Stephen Smalley
2005-12-21 16:42   ` Stephen Smalley
2005-12-21 17:41   ` Steve G
2005-12-21 18:09     ` Stephen Smalley
2005-12-22  4:19       ` Chris Wright
2005-12-22  4:10   ` Chris Wright
2005-12-21 16:19 ` Stephen Smalley
2005-12-21 17:56   ` Steve G
2005-12-21 18:26     ` Stephen Smalley
2006-01-25 17:46 Steve G

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).