2006-10-11 18:58:36

by Serge Aleynikov

[permalink] [raw]
Subject: non-critical security bug fix

To Maintainers of the linux/security/commoncap.c:

Patch description:
==================
This bug-fix ensures that if a process with root access sets
keep_capabilities flag, current capabilities get preserved when the
process switches from root to another effective user. It looks like
this was intended from the way capabilities are documented, but the
current->keep_capabilities flag is not being checked.

Regards,

Serge

----------------------------[ Begin patch ]---------------------------

--- linux/security/commoncap.c.orig 2005-10-29 16:00:58.656572231 -0400
+++ linux/security/commoncap.c 2005-10-29 16:04:45.093411424 -0400
@@ -217,6 +217,10 @@
* Keeping uid 0 is not an option because uid 0 owns too many vital
* files..
* Thanks to Olaf Kirch and Peter Benie for spotting this.
+ *
+ * Serge Aleynikov <[email protected]> IDT Corp, Oct 2005
+ * Control the case (old_euid==0 && current->euid!=0) via
+ * current->keep_capabilities as well.
*/
static inline void cap_emulate_setxuid (int old_ruid, int old_euid,
int old_suid)
@@ -227,7 +231,8 @@
cap_clear (current->cap_permitted);
cap_clear (current->cap_effective);
}
- if (old_euid == 0 && current->euid != 0) {
+ if (old_euid == 0 && current->euid != 0 &&
+ !current->keep_capabilities) {
cap_clear (current->cap_effective);
}
if (old_euid != 0 && current->euid == 0) {

----------------------------[ End patch ]---------------------------



----------------------------[ Begin test ]--------------------------
Change the EFFECTIVE_UID value, compile and run the following as root.
This test tries to set the capability cap_net_raw, then switch from root
to an effective user, and open a raw socket being not a root.

#include <sys/socket.h>
#include <sys/types.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <linux/if_ether.h>
#include <netinet/in.h>
#include <unistd.h>
#include <sys/capability.h>
#include <sys/prctl.h>

#define EFFECTIVE_UID 501

int main()
{
int s1, s2, rc;
cap_t caps;
char cmd [1024];

if (prctl (PR_SET_KEEPCAPS, 1, 0, 0, 0) < 0)
printf ("\nCould not SET_KEEPCAPS\n");
printf ("Keeping the Capabilities: %d\n",
prctl (PR_GET_KEEPCAPS, 0,0,0,0));

printf ("\nReal UID=%d, Real GID=%d, Eff UID=%d, Eff GID=%d\n",
getuid(), getgid(), geteuid(), getegid());

caps = cap_get_proc ();
printf ("\nInitial Capabilities: %s\n",
cap_to_text (caps, NULL));

sprintf (cmd, "cap_setuid,cap_setgid,cap_net_raw=eip");
printf ("\nSetting the Capabilities %s\n", cmd);
rc = cap_set_proc (cap_from_text (cmd));
if (rc != 0)
printf ("Failed to set the Capabilities: %s\n",
strerror(errno));

caps = cap_get_proc ();
printf ("\nPrivileged Capabilities: %s\n",
cap_to_text(caps, NULL));

setegid (EFFECTIVE_UID);
seteuid (EFFECTIVE_UID);

printf ("\nReal UID=%d, Real GID=%d, Eff UID=%d, Eff GID=%d\n",
getuid(), getgid(), geteuid(), getegid());

caps = cap_get_proc ();
printf ("\nUnPrivileged Capabilities: %s\n",
cap_to_text(caps, NULL));

s1 = socket (PF_INET, SOCK_RAW, IPPROTO_RAW);
if (s1 < 0)
printf ("PF_INET error: %s\n", strerror (errno));

s2 = socket (PF_PACKET, SOCK_RAW, htonl(ETH_P_IP));
if (s2 < 0)
printf ("PF_INET error: %s\n", strerror (errno));

return 0;
}
----------------------------[ End test ]----------------------------

Regards,

Serge

--
Serge Aleynikov
Routing R&D, IDT Telecom
Tel: +1 (973) 438-3436
Fax: +1 (973) 438-1464


2006-10-12 19:06:51

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: non-critical security bug fix

Quoting Serge Aleynikov ([email protected]):
> To Maintainers of the linux/security/commoncap.c:
>
> Patch description:
> ==================
> This bug-fix ensures that if a process with root access sets
> keep_capabilities flag, current capabilities get preserved when the
> process switches from root to another effective user. It looks like
> this was intended from the way capabilities are documented, but the
> current->keep_capabilities flag is not being checked.

Note that without your patch, the permitted set is maintained, so that
you can regain the caps into your effective set after setuid if you
need. i.e.

prctl(PR_SET_KEEPCAPS, 1);
setresuid(1000, 1000, 1000);
caps = cap_from_text("cap_net_admin,cap_sys_admin,cap_dac_override=ep");
ret = cap_set_proc(caps);

So this patch will change the default behavior, but does not add
features or change what is possible.

Ordinarely I'd say changing default behavior wrt security is a bad
thing, but given that this is "default behavior when doing prctl(PR_SET_KEEPCAPS)",
I don't know how much it matters.

Still, I like the current behavior, where setuid means drop effective
caps no matter what.

-serge

2006-10-12 19:25:59

by Serge Aleynikov

[permalink] [raw]
Subject: Re: non-critical security bug fix

Serge (what a nice name! ;-) ),

Let me give you an example where we found this patch very useful.

A 3rd party library that we bought implemented a user-level SCTP
protocol by opening raw sockets. This required our application to run
as root. However, we didn't want for it to run as root, and wanted to
set the CAP_NET_RAW option and have the interaction with the raw socket
survive after when we switch the effective user away from root.

When reading "man capabilities" we found:

"If a process that has a 0 value for one or more of its user IDs
wants to prevent its permitted capability set being cleared when it
resets all of its user IDs to non-zero values, it can do so using
the prctl() PR_SET_KEEPCAPS operation."

Correct me if I am wrong, but I believe that this sentence says just
what I described above. If so, the previously attached patch has the
behavior described in the man page.

Regards,

--
Serge Aleynikov
Routing R&D, IDT Telecom
Tel: +1 (973) 438-3436
Fax: +1 (973) 438-1464


Serge E. Hallyn wrote:
> Quoting Serge Aleynikov ([email protected]):
>> To Maintainers of the linux/security/commoncap.c:
>>
>> Patch description:
>> ==================
>> This bug-fix ensures that if a process with root access sets
>> keep_capabilities flag, current capabilities get preserved when the
>> process switches from root to another effective user. It looks like
>> this was intended from the way capabilities are documented, but the
>> current->keep_capabilities flag is not being checked.
>
> Note that without your patch, the permitted set is maintained, so that
> you can regain the caps into your effective set after setuid if you
> need. i.e.
>
> prctl(PR_SET_KEEPCAPS, 1);
> setresuid(1000, 1000, 1000);
> caps = cap_from_text("cap_net_admin,cap_sys_admin,cap_dac_override=ep");
> ret = cap_set_proc(caps);
>
> So this patch will change the default behavior, but does not add
> features or change what is possible.
>
> Ordinarely I'd say changing default behavior wrt security is a bad
> thing, but given that this is "default behavior when doing prctl(PR_SET_KEEPCAPS)",
> I don't know how much it matters.
>
> Still, I like the current behavior, where setuid means drop effective
> caps no matter what.
>
> -serge
>



2006-10-12 20:29:30

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: non-critical security bug fix

Quoting Serge Aleynikov ([email protected]):
> Serge (what a nice name! ;-) ),
>
> Let me give you an example where we found this patch very useful.
>
> A 3rd party library that we bought implemented a user-level SCTP
> protocol by opening raw sockets. This required our application to run
> as root. However, we didn't want for it to run as root, and wanted to
> set the CAP_NET_RAW option and have the interaction with the raw socket
> survive after when we switch the effective user away from root.
>
> When reading "man capabilities" we found:
>
> "If a process that has a 0 value for one or more of its user IDs
> wants to prevent its permitted capability set being cleared when it

Right, *permitted*.

> resets all of its user IDs to non-zero values, it can do so using
> the prctl() PR_SET_KEEPCAPS operation."
>
> Correct me if I am wrong, but I believe that this sentence says just
> what I described above.

No, you're asking for the effective set to also not be cleared, but the
manpage only says the permitted set is maintained.

> If so, the previously attached patch has the
> behavior described in the man page.

Are you able to modify the source of the application you bought? If so,
you can trivially fix it to do what you need by doing a cap_set_proc
after the suid as I described before, i.e.

ret = setuid();
caps = cap_from_text("cap_net_raw=e");
ret = cap_set_proc(caps);

Is that an option?

-serge

2006-10-12 21:11:42

by Serge Aleynikov

[permalink] [raw]
Subject: Re: non-critical security bug fix

Serge E. Hallyn wrote:
> Are you able to modify the source of the application you bought? If so,
> you can trivially fix it to do what you need by doing a cap_set_proc
> after the suid as I described before, i.e.
>
> ret = setuid();
> caps = cap_from_text("cap_net_raw=e");
> ret = cap_set_proc(caps);
>
> Is that an option?

Unfortunately that product doesn't come with sources. :-(

--
Serge Aleynikov
Routing R&D, IDT Telecom
Tel: +1 (973) 438-3436
Fax: +1 (973) 438-1464