Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
Did you see the patch John Spencer sent me to make it actually work? About three dozen more lines of code. John Spencer's patch focuses on making ping work without root privileges. My code focuses on giving root privileges to applets that need it (such as current ping) without making the whole busybox binary setuid root. Not the same approach, not the same thing. -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
n Fri, Jan 10, 2014 at 1:59 PM, Laurent Bercot ska-dietl...@skarnet.org wrote: Did you see the patch John Spencer sent me to make it actually work? About three dozen more lines of code. John Spencer's patch focuses on making ping work without root privileges. My code focuses on giving root privileges to applets that need it (such as current ping) without making the whole busybox binary setuid root. Not the same approach, not the same thing. Ah, I see. Sorry. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On 1/10/2014 12:37 AM, Laurent Bercot wrote: You're performing too much work copying your argument list. :P The wrapper should be entirely transparent: busybox shouldn't even notice it has been run through it, so it should be called with the exact same argv. Here's what I do [...] If you didn't want to have to maintain the list within the binary, and want to depend on the filesystem to declare which applets are allowed, you could write the wrapper to stat /bin/$NAME to verify that it (and / and /bin) are * owned by root * not writable by other users * on the root filesystem (compare device number to device number of /) * has set-uid bit This would be authorization-by-the-filesystem that you could rely on, and is intuitive to configure. It's a lot more code to write, but would still be a small binary. -Mike ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Fri, Jan 10, 2014 at 12:06:27PM -0500, Michael Conrad wrote: On 1/10/2014 12:37 AM, Laurent Bercot wrote: You're performing too much work copying your argument list. :P The wrapper should be entirely transparent: busybox shouldn't even notice it has been run through it, so it should be called with the exact same argv. Here's what I do [...] If you didn't want to have to maintain the list within the binary, and want to depend on the filesystem to declare which applets are allowed, you could write the wrapper to stat /bin/$NAME to verify that it (and / and /bin) are * owned by root * not writable by other users * on the root filesystem (compare device number to device number of /) * has set-uid bit This would be authorization-by-the-filesystem that you could rely on, and is intuitive to configure. It's a lot more code to write, but would still be a small binary. Note that this kind of approach STILL does not protect you from vulnerabilities in the dynamic linker (avoiding them would require making both the wrapper and busybox binary static-linked) or libc startup code (inevitable). Unlike other methods (e.g. logging into a daemon via a local or network socket) by which initially non-privileged users/clients get control of a privileged process, the legacy setuid mechanism simply has too many potentially dangerous inputs to account for: - environment variables - inherited file descriptors (possibly to files in /proc which behave differently depending on the uid of the process accessing them) - resource limits (which can cause failure in syscalls the program expected not to be able to fail). - controlling terminal and other process group and session mess. - ... Playing whack-a-mole trying to make sure your suids are safe against all of these is an exercise in futility. If you want a system that's secure against local attacks, you have to ensure that no user account that might be compromised is able to execute an suid binary. And the easiest way to do this is not to have any. This is why I want to see a ping that works without suid. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On 2014-01-10 19:27, Rich Felker wrote: Note that this kind of approach STILL does not protect you from vulnerabilities in the dynamic linker (avoiding them would require making both the wrapper and busybox binary static-linked) Which is the case for me. or libc startup code (inevitable). I'm using musl, it looked like a good, paranoid libc; maybe I was lied to ? :-O [dangers of suid] This is why I want to see a ping that works without suid. So do I. I also want to write a simple user database backend (with its own getpwent() implementation) so that passwd doesn't need to be setuid root. And a Unix-socket-based su daemon with credential passing, and terminal passing too. And rewrite qmail-queue as a Unix-socket-based daemon. And a non-setuid traceroute. And a pony. In the meantime, I also want a usable, working system. As Denys noted, cleansing the existing codebase of setuid is an energy- and time-consuming practice; in the name of good compromise between practicality and security, I will still use the setuid binaries I need until I've replaced them (or, better, until you and John have done all the hard work for me), while making sure privileges are only gained when they are strictly required. -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Fri, Jan 10, 2014 at 09:33:56PM +, Laurent Bercot wrote: On 2014-01-10 19:27, Rich Felker wrote: Note that this kind of approach STILL does not protect you from vulnerabilities in the dynamic linker (avoiding them would require making both the wrapper and busybox binary static-linked) Which is the case for me. or libc startup code (inevitable). I'm using musl, it looked like a good, paranoid libc; maybe I was lied to ? :-O Part of being good and paranoid is not trusting even yourself that much. :-) But I was thinking more of other libcs that have more complicated startup code. [dangers of suid] This is why I want to see a ping that works without suid. So do I. I also want to write a simple user database backend (with its own getpwent() implementation) so that passwd doesn't need to be setuid root. These are much harder problems. The difficulty of solving a harder problem is not an argument for not solving easy ones. BTW, musl also supports /etc/tcb/ shadow passwords, another feature from Owl, whereby, depending on how you set permissions, it's trivial to write a passwd utility that does not need root to change your password. And a Unix-socket-based su daemon with credential passing, and terminal passing too. alias su=ssh root@localhost makes a decent substitute. And rewrite qmail-queue as a Unix-socket-based daemon. Now we're well outside the scope of things in Busybox. And a non-setuid traceroute. Non-Busybox traceroute already does it. Busybox traceroute --help implies it supports UDP-based trace, so I don't know why it tries to open a raw socket and aborts when it fails. Ideally the same method proposed for ping could also be supported by traceroute to allow ICMP-based trace by non-root, but I think this is lower-priority than support in ping since traceroute generally works fine with UDP or TCP. And a pony. IIRC we have some MLP fans in #musl who might could help. :-) In the meantime, I also want a usable, working system. As Denys noted, cleansing the existing codebase of setuid is an energy- and time-consuming practice; in the name of good compromise between practicality and security, I will still use the setuid binaries I need until I've replaced them (or, better, until you and John have done all the hard work for me), while making sure privileges are only gained when they are strictly required. *nod* It's still rare that I use systems with absolutely no suids, but I'd like to move more in that direction, and lack of ping is a big point of frustration that would be easy to fix. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Mon, Jan 6, 2014 at 5:03 PM, John Spencer maillist-busy...@barfooze.de wrote: i've been able to get the SOCK_DGRAM stuff to work with a little help of Vasily, author of the kernel patch. see attached proof-of-concept patch. i'm aware that it doesnt use xbind() and other busybox replacement funcs. getting it into a more busybox-ish shape is something i leave as a task for someone more familiar with the busybox internals. note that i didn't test if the added getsockopt calls are strictly needed, i added them because they were in the original iputils patch. in general the following differences exist between SOCK_RAW and SOCK_DGRAM handling: 1) the received packet is a raw icmp packet, not an IP one, so it lacks the header and is shorter. 2) the ident (myid) of the packet is sin(6)_port of the sockaddr struct after doing a bind() and getsockname() on the dgram socket. the patch works for both fancy ping(6), and non-fancy ping(6) (the latter just sends one packet and displays whether it was successful or not, without further info). This seems to lead to a significantly larger code. Making ping suid wasn't such a big problem before, so why should we have all these complications now? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
Denys Vlasenko wrote: On Mon, Jan 6, 2014 at 5:03 PM, John Spencer maillist-busy...@barfooze.de wrote: i've been able to get the SOCK_DGRAM stuff to work with a little help of Vasily, author of the kernel patch. see attached proof-of-concept patch. i'm aware that it doesnt use xbind() and other busybox replacement funcs. getting it into a more busybox-ish shape is something i leave as a task for someone more familiar with the busybox internals. note that i didn't test if the added getsockopt calls are strictly needed, i added them because they were in the original iputils patch. in general the following differences exist between SOCK_RAW and SOCK_DGRAM handling: 1) the received packet is a raw icmp packet, not an IP one, so it lacks the header and is shorter. 2) the ident (myid) of the packet is sin(6)_port of the sockaddr struct after doing a bind() and getsockname() on the dgram socket. the patch works for both fancy ping(6), and non-fancy ping(6) (the latter just sends one packet and displays whether it was successful or not, without further info). This seems to lead to a significantly larger code. Making ping suid wasn't such a big problem before, so why should we have all these complications now? making ping suid in the context of busybox basically means make the entire busybox binary suid and that is definitely a bad idea (an example that comes to mind is the wall vulnerability discovered recently). you can only build ping as a separate applet when you use dynamic linking, which leads to dozens new attack vectors and gives the attacker access to the entire libc. in my distribution, there is only a single (static) suid binary, and that is a selfwritten su implementation that's selfcontained in a single TU with ~150 LOC. so it is very simple to audit for security bugs. one could even get rid of that and make a su script that just calls ssh localhost with appropriate parameters. the openwall distribution owl, from which the kernel patch originated, has zero suid binaries. i think if a busybox specialist reshapes the POC code slightly (for example by putting the bind code to get myid from the port into a common function used by v4/v6 code), the size increase could be brought down to less than 200 bytes. maybe using an optional config feature for CONFIG_PING_DGRAM. --JS ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Thursday 09 January 2014 13:35:59 John Spencer wrote: Denys Vlasenko wrote: On Mon, Jan 6, 2014 at 5:03 PM, John Spencer maillist-busy...@barfooze.de wrote: i've been able to get the SOCK_DGRAM stuff to work with a little help of Vasily, author of the kernel patch. see attached proof-of-concept patch. i'm aware that it doesnt use xbind() and other busybox replacement funcs. getting it into a more busybox-ish shape is something i leave as a task for someone more familiar with the busybox internals. note that i didn't test if the added getsockopt calls are strictly needed, i added them because they were in the original iputils patch. in general the following differences exist between SOCK_RAW and SOCK_DGRAM handling: 1) the received packet is a raw icmp packet, not an IP one, so it lacks the header and is shorter. 2) the ident (myid) of the packet is sin(6)_port of the sockaddr struct after doing a bind() and getsockname() on the dgram socket. the patch works for both fancy ping(6), and non-fancy ping(6) (the latter just sends one packet and displays whether it was successful or not, without further info). This seems to lead to a significantly larger code. Making ping suid wasn't such a big problem before, so why should we have all these complications now? making ping suid in the context of busybox basically means make the entire busybox binary suid and that is definitely a bad idea (an example that comes to mind is the wall vulnerability discovered recently). Hi, Busybox drops suid privileges for applets that don't require it even before the applet code is called. you can only build ping as a separate applet when you use dynamic linking, which leads to dozens new attack vectors and gives the attacker access to the entire libc. Why? You can build a static version of busybox with the ping applet or only the applets that need suid privileges. in my distribution, there is only a single (static) suid binary, and that is a selfwritten su implementation that's selfcontained in a single TU with ~150 LOC. so it is very simple to audit for security bugs. one could even get rid of that and make a su script that just calls ssh localhost with appropriate parameters. So you just shift the suid problem towards the ssh binary. You have to allow root login to ssh. What is the difference? the openwall distribution owl, from which the kernel patch originated, has zero suid binaries. Just out of curiosity how do they manage shadow passwords? Ciao, Tito i think if a busybox specialist reshapes the POC code slightly (for example by putting the bind code to get myid from the port into a common function used by v4/v6 code), the size increase could be brought down to less than 200 bytes. maybe using an optional config feature for CONFIG_PING_DGRAM. --JS ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Thursday 09 January 2014 15:07:23 Laurent Bercot wrote: making ping suid in the context of busybox basically means make the entire busybox binary suid and that is definitely a bad idea (an example that comes to mind is the wall vulnerability discovered recently). Hi, Busybox drops suid privileges for applets that don't require it even before the applet code is called. I never understood all the fuss about that or the chosen Busybox solution. Gaining privileges is the single most dangerous thing in Unix ; gaining privileges then dropping them if you didn't need them after all is playing with fire for no reason. Here is what I do: * make a single busybox binary with all the applets I need. My busybox binary is NEVER setuid. * compile a separate small C program that tests whether `basename $0` is in a list of accepted words, and if it is the case, execs into /bin/busybox `basename $0` $@. Make that separate binary setuid root. Hi, basename is a link to which one of the busybox binaries? Ciao, Tito * the utilities that need to be setuid root are symlinks to that binary, the other ones are direct symlinks to busybox. This solution makes me trust 4 lines of code instead of the whole busybox binary, and privileges are only gained if they are really needed. Sure, I have to edit the list of setuid applets in an additional place; this is a small price to pay for correctness. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Thu, Jan 9, 2014 at 1:35 PM, John Spencer maillist-busy...@barfooze.de wrote: This seems to lead to a significantly larger code. Making ping suid wasn't such a big problem before, so why should we have all these complications now? making ping suid in the context of busybox basically means make the entire busybox binary suid and that is definitely a bad idea (an example that comes to mind is the wall vulnerability discovered recently). If you want to use wall applet, you will need to setuid the entire binary anyway. Having ping applet to not need root privs won't help one iota in avoiding triggering a bug in other applets (e.g. wall). The only thing which you save yourself from are possible undiscovered bugs in ping applet. A security-paranoid project conceivably would be willing to trade more code and complexity for added security wrt bugs. We are size-paranoid project, not security-paranoid one. If you are concerned about posiible bugs in ping applet, feel free to audit its code and let me know if you find one. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Thu, Jan 9, 2014 at 3:07 PM, Laurent Bercot ska-dietl...@skarnet.org wrote: I never understood all the fuss about that or the chosen Busybox solution. Gaining privileges is the single most dangerous thing in Unix An attacker who only manages to subvert your user account, of course, can't get at the precious things like /usr/bin/* files and modify or delete them. He can only read your locally saved emails, browser's cache and saved passwords of your bank website login. Oh, wait... ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
Denys Vlasenko wrote: On Thu, Jan 9, 2014 at 1:35 PM, John Spencer maillist-busy...@barfooze.de wrote: This seems to lead to a significantly larger code. Making ping suid wasn't such a big problem before, so why should we have all these complications now? making ping suid in the context of busybox basically means make the entire busybox binary suid and that is definitely a bad idea (an example that comes to mind is the wall vulnerability discovered recently). If you want to use wall applet, you will need to setuid the entire why ? you can use wall as root. in fact using the applet as non-root seems pretty insane. that way random users can spam your terminal with nonsense. binary anyway. Having ping applet to not need root privs won't help one iota in avoiding triggering a bug in other applets (e.g. wall). apart from ping and su there isn't anything that needs setuid or other raised privileges, and shrinking that list down to one, just su, is definitely an improvement. and as in my case, you can just as well use another su replacement so your busybox binary is entirely suid-free. The only thing which you save yourself from are possible undiscovered bugs in ping applet. no, i save myself from yet another tool that needs elevated privs despite there being a kernel solution for not requiring them since 2.5 years. A security-paranoid project conceivably would be willing to trade more code and complexity for added security wrt bugs. We are size-paranoid project, not security-paranoid one. If you are concerned about posiible bugs in ping applet, feel free to audit its code and let me know if you find one. well if you cant be convinced to trade 100-200 bytes for a secure ping implementation, than you should at least remove the existing bogus SOCK_DGRAM support which doesn't work at all, but adds bloat (10 bytes or so). ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
An attacker who only manages to subvert your user account, of course, can't get at the precious things like /usr/bin/* files and modify or delete them. He can only read your locally saved emails, browser's cache and saved passwords of your bank website login. Oh, wait... Eh, I didn't pretend that security holes weren't serious to begin with. But an attacker who finds a hole in a setuid root binary can gain access to *every user*'s personal data, and cover his tracks, and so on. Root exploits are an order of magnitude more problematic, which doesn't mean that user exploits are fine. -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
* make a single busybox binary with all the applets I need. My busybox binary is NEVER setuid. * compile a separate small C program that tests whether `basename $0` is in a list of accepted words, and if it is the case, execs into /bin/busybox `basename $0` $@. Make that separate binary setuid root. Hi, basename is a link to which one of the busybox binaries? basename is the standard busybox, of course, it doesn't need setuid privileges. But when I wrote `basename $0` here, it was a shortcut: the actual code is C, so it's more like the basename() function. The additional binary does not depend on busybox at all - except for the final execve(). -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Thu, Jan 9, 2014 at 7:15 PM, Laurent Bercot ska-dietl...@skarnet.org wrote: An attacker who only manages to subvert your user account, of course, can't get at the precious things like /usr/bin/* files and modify or delete them. He can only read your locally saved emails, browser's cache and saved passwords of your bank website login. Oh, wait... Eh, I didn't pretend that security holes weren't serious to begin with. But an attacker who finds a hole in a setuid root binary can gain access to *every user*'s personal data, Which often means the only user of this machine. and cover his tracks, and so on. Root exploits are an order of magnitude more problematic, which doesn't mean that user exploits are fine. They were order of magnitude more problematic when multi-user machines were the norm. Today, the difference in the level of impact is less pronounced. That's my point: It is not logical anymore to see root exploits as orders of magnitude more dangerous than user-level ones, and spend much more efforts to prevent specifically these exploits to be used. If you are afraid that ping may have a bug, spend time auditing ping, not making it more ugly just because you can make such bug impact only lowly user. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
They were order of magnitude more problematic when multi-user machines were the norm. True enough, but it is still the case, for a good definition of user. Most machines today only have one human user, but there are a lot of uids and gids used to run daemons with separate privileges. It is just as likely that an exploitable hole will be found in some daemon code than in some code directly run under the human user's uid - and there, a user exploit won't be a major problem, whereas a root exploit will. It is not logical anymore to see root exploits as orders of magnitude more dangerous than user-level ones, and spend much more efforts to prevent specifically these exploits to be used. If you are afraid that ping may have a bug, spend time auditing ping, not making it more ugly just because you can make such bug impact only lowly user. I understand what you're saying, and agree with it, but my point is that my solution: * isn't much more effort. I probably spent 5-10 minutes writing the additional 4 lines of C code. And theoretically, the privileged applet list could be automatically generated from the Kconfig, to avoid any additional configuration effort. * isn't more ugly. Actually, there's less code in total than with the busybox setuid-then-drop-privileges thing, and the general case execution path is shorter. It could totally be integrated into busybox itself and benefit everyone. I just don't have time to work on a patch right now, so I'm just mentioning the idea around. -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
RE: [PATCH] ping: try SOCK_DGRAM if no root privileges
A separate suid-exec relay utility is in fact exactly what
we use on our BB installation in our product, and have
for years. BB itself is not suid, I/we didn't trust it
enough. Here it is:
/*
** Relay execution program to setuid-root selected busybox functions.
** (We are expected to be suid-root, busybox is not.)
*/
#include unistd.h
#include malloc.h
#include string.h
int
main(int argc, char **argv, char **envp)
{
int ii;
char **nargv, *cp;
for (ii=0; argv[ii]; ii++)
;
nargv = calloc(ii + 2, sizeof *nargv);
while (ii = 0) {
nargv[ii + 1] = argv[ii];
ii--;
}
nargv[0] = busybox;
if ((cp = strrchr(argv[0], '/')))
nargv[1] = cp + 1;
execve(/bin/busybox, nargv, envp);
return 1;
}
-- Jim
-Original Message-
From: busybox-boun...@busybox.net [mailto:busybox-boun...@busybox.net] On
Behalf Of Laurent Bercot
Sent: Thursday, January 09, 2014 12:10 PM
To: Denys Vlasenko
Cc: busybox
Subject: Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
They were order of magnitude more problematic
when multi-user machines were the norm.
True enough, but it is still the case, for a good definition of user.
Most machines today only have one human user, but there are a lot
of uids and gids used to run daemons with separate privileges. It is
just as likely that an exploitable hole will be found in some daemon
code than in some code directly run under the human user's uid - and
there, a user exploit won't be a major problem, whereas a root
exploit will.
It is not logical anymore to see root exploits as orders of magnitude
more dangerous than user-level ones, and spend much more efforts
to prevent specifically these exploits to be used.
If you are afraid that ping may have a bug, spend time auditing ping,
not making it more ugly just because you can make such bug
impact only lowly user.
I understand what you're saying, and agree with it, but my point is
that my solution:
* isn't much more effort. I probably spent 5-10 minutes writing the
additional 4 lines of C code. And theoretically, the privileged applet
list could be automatically generated from the Kconfig, to avoid any
additional configuration effort.
* isn't more ugly. Actually, there's less code in total than with the
busybox setuid-then-drop-privileges thing, and the general case execution
path is shorter.
It could totally be integrated into busybox itself and benefit everyone.
I just don't have time to work on a patch right now, so I'm just mentioning
the idea around.
--
Laurent
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
Cathey, == Cathey, Jim jcat...@ciena.com writes: This is on a read-only root filesystem that is built the way we want it. (Squashfs, I believe.) There aren't any, and can't be, any links to names we don't wish to give suid permission to. And no writable storage anywhere (E.G. a tmpfs for /tmp or similar) where you can add a handy symlink to the wrapper? -- Bye, Peter Korsgaard ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
RE: [PATCH] ping: try SOCK_DGRAM if no root privileges
This is on a read-only root filesystem that is built the way we want it. (Squashfs, I believe.) There aren't any, and can't be, any links to names we don't wish to give suid permission to. -- Jim -Original Message- Where's the check for what applets are allowed? What stops you from running wrapper ash? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
RE: [PATCH] ping: try SOCK_DGRAM if no root privileges
Doesn't really do you much good since BB is still doing its own name checks. No, it's not perfect. Making this kind of relay program a real part of BB, as a user-selectable option probably, would be best. -- Jim -Original Message- From: Peter Korsgaard [mailto:jac...@gmail.com] On Behalf Of Peter Korsgaard Sent: Thursday, January 09, 2014 12:46 PM To: Cathey, Jim Cc: Laurent Bercot; Denys Vlasenko; busybox Subject: Re: [PATCH] ping: try SOCK_DGRAM if no root privileges Cathey, == Cathey, Jim jcat...@ciena.com writes: This is on a read-only root filesystem that is built the way we want it. (Squashfs, I believe.) There aren't any, and can't be, any links to names we don't wish to give suid permission to. And no writable storage anywhere (E.G. a tmpfs for /tmp or similar) where you can add a handy symlink to the wrapper? -- Bye, Peter Korsgaard ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
RE: [PATCH] ping: try SOCK_DGRAM if no root privileges
Oh, and it helps to know that users on this particular system do not get access to shells. Or rather, the account shell _is_ our custom application. Bash (or ash) is there, but not exposed. It's used by admin scripts, booting, etc. -- Jim -Original Message- From: busybox-boun...@busybox.net [mailto:busybox-boun...@busybox.net] On Behalf Of Cathey, Jim Sent: Thursday, January 09, 2014 1:24 PM To: Peter Korsgaard Cc: busybox Subject: RE: [PATCH] ping: try SOCK_DGRAM if no root privileges Doesn't really do you much good since BB is still doing its own name checks. No, it's not perfect. Making this kind of relay program a real part of BB, as a user-selectable option probably, would be best. -- Jim -Original Message- From: Peter Korsgaard [mailto:jac...@gmail.com] On Behalf Of Peter Korsgaard Sent: Thursday, January 09, 2014 12:46 PM To: Cathey, Jim Cc: Laurent Bercot; Denys Vlasenko; busybox Subject: Re: [PATCH] ping: try SOCK_DGRAM if no root privileges Cathey, == Cathey, Jim jcat...@ciena.com writes: This is on a read-only root filesystem that is built the way we want it. (Squashfs, I believe.) There aren't any, and can't be, any links to names we don't wish to give suid permission to. And no writable storage anywhere (E.G. a tmpfs for /tmp or similar) where you can add a handy symlink to the wrapper? -- Bye, Peter Korsgaard ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Thu, Jan 09, 2014 at 02:04:12PM +0100, Tito wrote: On Thursday 09 January 2014 13:35:59 John Spencer wrote: Denys Vlasenko wrote: On Mon, Jan 6, 2014 at 5:03 PM, John Spencer maillist-busy...@barfooze.de wrote: i've been able to get the SOCK_DGRAM stuff to work with a little help of Vasily, author of the kernel patch. see attached proof-of-concept patch. i'm aware that it doesnt use xbind() and other busybox replacement funcs. getting it into a more busybox-ish shape is something i leave as a task for someone more familiar with the busybox internals. note that i didn't test if the added getsockopt calls are strictly needed, i added them because they were in the original iputils patch. in general the following differences exist between SOCK_RAW and SOCK_DGRAM handling: 1) the received packet is a raw icmp packet, not an IP one, so it lacks the header and is shorter. 2) the ident (myid) of the packet is sin(6)_port of the sockaddr struct after doing a bind() and getsockname() on the dgram socket. the patch works for both fancy ping(6), and non-fancy ping(6) (the latter just sends one packet and displays whether it was successful or not, without further info). This seems to lead to a significantly larger code. Making ping suid wasn't such a big problem before, so why should we have all these complications now? making ping suid in the context of busybox basically means make the entire busybox binary suid and that is definitely a bad idea (an example that comes to mind is the wall vulnerability discovered recently). Hi, Busybox drops suid privileges for applets that don't require it even before the applet code is called. This is not entirely true. Some apps, such as the infamous vulnerable wall, are in the list that require it even though, from a standpoint of sanity, wall should not have suid-root and should be restricted to use by only root and group-tty users. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Thu, Jan 09, 2014 at 08:03:49PM +0100, Denys Vlasenko wrote: If you are afraid that ping may have a bug, spend time auditing ping, not making it more ugly just because you can make such bug impact only lowly user. The concern is not that ping may have a bug. The concern is that the presence of ANY suid binaries on a system vastly increases the risk of having a vulnerability (even in the dynamic linker, for example, if the suid program is dynamic-linked). Good policy is not to have any suids, and even to mount all filesystems with the nosuid option. The whole point of adding SOCK_DGRAM support to ping is to allow the use of ping (by non-root users) on such a properly configured system. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
You're performing too much work copying your argument list. :P
The wrapper should be entirely transparent: busybox shouldn't
even notice it has been run through it, so it should be called
with the exact same argv. Here's what I do.
Notes:
* untested, please check carefully. The actual code I use depends
on skalibs; I modified it to remove the dependency to post it here.
(Which increased both the source code size and binary code size: yes,
the standard libc APIs suck that much and I wrote skalibs for a reason.)
* BUSYBOX and LIST should be auto-generated to match the final path
to the busybox binary and the list of authorized privileged applets.
* It is necessary to copy argv[0] because basename() can modify its
argument. (Have I ever mentioned that the standard libc APIs suck ?)
I'm doing it with a VLA to avoid pulling in malloc. The VLA can't smash
the stack, because the kernel limits argv.
* No, Rich, I won't do a dichotomic search on a list that size. ;)
-
#include string.h
#include libgen.h
#include stdio.h
#include unistd.h
#define BUSYBOX /bin/busybox
static char const *LIST[] =
{
passwd,
ping,
ping6,
su,
0
} ;
static int okay (char const *s)
{
register char const **p = LIST ;
for (; *p ; p++) if (!strcmp(s, *p)) return 1 ;
return 0 ;
}
int main (int argc, char const *const *argv, char const *const *envp)
{
{
char tmp[strlen(argv[0]) + 1] ;
strcpy(tmp, argv[0]) ;
if (!okay(basename(tmp)))
{
fprintf(stderr, busybox-setuid: unable to run %s: unauthorized
applet\n, argv[0]) ;
return 1 ;
}
}
execve(BUSYBOX, argv, envp) ;
perror(busybox-setuid: unable to exec BUSYBOX) ;
return 2 ;
}
-
--
Laurent
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
On Thursday 09 January 2014 21:09, Laurent Bercot wrote: It is not logical anymore to see root exploits as orders of magnitude more dangerous than user-level ones, and spend much more efforts to prevent specifically these exploits to be used. If you are afraid that ping may have a bug, spend time auditing ping, not making it more ugly just because you can make such bug impact only lowly user. I understand what you're saying, and agree with it, but my point is that my solution: * isn't much more effort. I probably spent 5-10 minutes writing the additional 4 lines of C code. Did you see the patch John Spencer sent me to make it actually work? About three dozen more lines of code. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
i've been able to get the SOCK_DGRAM stuff to work with a little help of
Vasily, author of the kernel patch.
see attached proof-of-concept patch.
i'm aware that it doesnt use xbind() and other busybox replacement
funcs. getting it into a more busybox-ish shape is something i leave as
a task for someone more familiar with the busybox internals.
note that i didn't test if the added getsockopt calls are strictly
needed, i added them because they were in the original iputils patch.
in general the following differences exist between SOCK_RAW and
SOCK_DGRAM handling:
1) the received packet is a raw icmp packet, not an IP one, so it lacks
the header and is shorter.
2) the ident (myid) of the packet is sin(6)_port of the sockaddr struct
after doing a bind() and getsockname() on the dgram socket.
the patch works for both fancy ping(6), and non-fancy ping(6) (the
latter just sends one packet and displays whether it was successful or
not, without further info).
note that the iputils patch is not upstream yet, so if busybox merges
it, it's the first official ping implementation that supports SOCK_DGRAM.
--JS
John Spencer wrote:
Denys Vlasenko wrote:
Applied, thanks.
i just tested this new functionality, and it hangs at recv and does
nothing until the alarm is triggered:
c = recv(pingsock, G.packet, sizeof(G.packet), 0);
(note: in order to test one has to echo groupid groupid
/proc/sys/net/ipv4/ping_group_range as described in the kernel commit:
https://lkml.org/lkml/2011/5/13/382 )
OTOH using the iputils source tarball provided on the kernel patch
authors ping info page
( http://openwall.info/wiki/people/segoon/ping ) + the patch there, the
resulting ping binary successfully is able to ping as an ordinary user
without special privileges, as long as the groupid matches the range)
i fixed a couple compile errors in that version of iputils and squashed
the needed ping code from that tarball into a single standalone 2KLOC C
file - see attachment.
On Tue, Nov 26, 2013 at 10:18 PM, Daniel Borca dbo...@yahoo.com wrote:
Allow non-setuid ping.
Reference:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c319b4d76b9e583a5d88d6bf190e079c4e43213d
-dborca
--- a/networking/ping.c
+++ b/networking/ping.c
@@ -469,6 +1053,7 @@
pingsock = 0,
};
+static int using_dgram;
static void
#if ENABLE_PING6
create_icmp_socket(len_and_sockaddr *lsa)
@@ -501,6 +1086,7 @@
if (sock 0)
#endif
bb_error_msg_and_die(bb_msg_perm_denied_are_you_root);
+ using_dgram = 1;
}
xmove_fd(sock, pingsock);
@@ -551,10 +1137,12 @@
bb_perror_msg(recvfrom);
continue;
}
- if (c = 76) { /* ip + icmp */
- struct iphdr *iphdr = (struct iphdr *) G.packet;
+ if (c = 76 || using_dgram (c == 64)) {
/* ip + icmp */
+ if(!using_dgram) {
+ struct iphdr *iphdr = (struct iphdr *) G.packet;
- pkt = (struct icmp *) (G.packet + (iphdr-ihl 2));
/* skip ip hdr */
+ pkt = (struct icmp *) (G.packet + (iphdr-ihl
2)); /* skip ip hdr */
+ } else pkt = (struct icmp *) G.packet;
if (pkt-icmp_type == ICMP_ECHOREPLY)
break;
}
@@ -946,19 +1533,21 @@
}
static void unpack4(char *buf, int sz, struct sockaddr_in *from)
{
- struct icmp *icmppkt;
struct iphdr *iphdr;
+ struct icmp *icmppkt;
int hlen;
/* discard if too short */
if (sz (datalen + ICMP_MINLEN))
return;
+ if(!using_dgram) {
+ /* check IP header */
+ iphdr = (struct iphdr *) buf;
+ hlen = iphdr-ihl 2;
+ sz -= hlen;
+ icmppkt = (struct icmp *) (buf + hlen);
+ } else icmppkt = (struct icmp *) buf;
- /* check IP header */
- iphdr = (struct iphdr *) buf;
- hlen = iphdr-ihl 2;
- sz -= hlen;
- icmppkt = (struct icmp *) (buf + hlen);
if (icmppkt-icmp_id != myid)
return; /* not our ping */
@@ -970,7 +1559,7 @@
tp = (uint32_t *) icmppkt-icmp_data;
unpack_tail(sz, tp,
inet_ntoa(*(struct in_addr *) from-sin_addr.s_addr),
- recv_seq, iphdr-ttl);
+ recv_seq, using_dgram ? 42 : iphdr-ttl);
} else if (icmppkt-icmp_type != ICMP_ECHO) {
bb_error_msg(warning: got ICMP %d (%s),
icmppkt-icmp_type,
@@ -1014,11 +1603,31 @@
int sockopt;
pingaddr.sin = lsa-u.sin;
- if (source_lsa) {
+ if (source_lsa !using_dgram) {
Re: [PATCH] ping: try SOCK_DGRAM if no root privileges
Applied, thanks. On Tue, Nov 26, 2013 at 10:18 PM, Daniel Borca dbo...@yahoo.com wrote: Allow non-setuid ping. Reference: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c319b4d76b9e583a5d88d6bf190e079c4e43213d -dborca ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ping: try SOCK_DGRAM if no root privileges
Allow non-setuid ping.
Reference:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c319b4d76b9e583a5d88d6bf190e079c4e43213d
-dborca
Signed-off-by: Daniel Borca dbo...@yahoo.com
---
networking/ping.c | 19 ---
1 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/networking/ping.c b/networking/ping.c
index 5e4771f..5d71fe8 100644
--- a/networking/ping.c
+++ b/networking/ping.c
@@ -168,9 +168,22 @@ create_icmp_socket(void)
#endif
sock = socket(AF_INET, SOCK_RAW, 1); /* 1 == ICMP */
if (sock 0) {
- if (errno == EPERM)
- bb_error_msg_and_die(bb_msg_perm_denied_are_you_root);
- bb_perror_msg_and_die(bb_msg_can_not_create_raw_socket);
+ if (errno != EPERM)
+ bb_perror_msg_and_die(bb_msg_can_not_create_raw_socket);
+#if defined(__linux__) || defined(__APPLE__)
+ /* We don't have root privileges. Try SOCK_DGRAM instead.
+* Linux needs net.ipv4.ping_group_range for this to work.
+* MacOSX allows ICMP_ECHO, ICMP_TSTAMP or ICMP_MASKREQ
+*/
+#if ENABLE_PING6
+ if (lsa-u.sa.sa_family == AF_INET6)
+ sock = socket(AF_INET6, SOCK_DGRAM, IPPROTO_ICMPV6);
+ else
+#endif
+ sock = socket(AF_INET, SOCK_DGRAM, 1); /* 1 == ICMP */
+ if (sock 0)
+#endif
+ bb_error_msg_and_die(bb_msg_perm_denied_are_you_root);
}
xmove_fd(sock, pingsock);
--
1.7.4.4
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
