Yeah, memcached already knows how to drop privileges and it's entirely possible to jail it without having to tie it to a super-specific config. Not sure this patch is needed (no offense)
On Wed, Jul 21, 2010 at 1:30 AM, dormando <[email protected]> wrote: > I'm not really sure how to delicately explain this so please have some > forgiveness :) > > You can't really hardcode either of those things... That's replacing a > flexible feature with the inflexible sort you'd expect out of a > proprietary appliance. > > I'm pretty sure the -u feature works the way you need it to, and > chroot'ing an application is perfectly doable with an init script. I think > we could take a patch with an example init script for chroot'ing it > (perhaps along with some directions). > > A bit on the fence about adding an outright chroot command, since > different OS's have different ways of doing that, and hardcoding it > doesn't seem to be the best use here (tho someone correct me if I'm > wrong). > > On Wed, 21 Jul 2010, Loganaden Velvindron wrote: > > > Hi, > > > > _memcached is a dedicated user with home directory /var/empty, > > and the login shell is /sbin/nologin. > > > > /var/empty could be created by the package manager or install script. > > > > //Logan > > C-x-C-c > > > > > > On Wed, Jul 21, 2010 at 1:25 AM, Trond Norbye <[email protected]> > wrote: > > > > Why do you remove the ability for the user to specify the username it > should run as, and instead hardcode it to run as _memcached ?? In addition > this patch require /var/empty to exists, and I know of > > a number of platforms that don't have a /var/empty directory... > > > > Just my 0.5NOK > > > > Trond > > > > > > On 20. juli 2010, at 20.54, Loganaden Velvindron wrote: > > > > Greetings, > > > > I've investigated further, and this diff seems to be ok. > > > > What do you think ? > > > > //Logan > > C-x-C-c > > > > diff --git a/memcached.c b/memcached.c > > index 750c8b3..1d56a8f 100644 > > --- a/memcached.c > > +++ b/memcached.c > > @@ -22,6 +22,8 @@ > > #include <sys/uio.h> > > #include <ctype.h> > > #include <stdarg.h> > > +#include <unistd.h> > > +#include <grp.h> > > > > /* some POSIX systems need the following definition > > * to get mlockall flags out of sys/mman.h. */ > > @@ -4539,22 +4541,6 @@ int main (int argc, char **argv) { > > } > > } > > > > - /* lose root privileges if we have them */ > > - if (getuid() == 0 || geteuid() == 0) { > > - if (username == 0 || *username == '\0') { > > - fprintf(stderr, "can't run as root without the -u > switch\n"); > > - exit(EX_USAGE); > > - } > > - if ((pw = getpwnam(username)) == 0) { > > - fprintf(stderr, "can't find the user %s to switch > to\n", username); > > - exit(EX_NOUSER); > > - } > > - if (setgid(pw->pw_gid) < 0 || setuid(pw->pw_uid) < 0) { > > - fprintf(stderr, "failed to assume identity of user > %s\n", username); > > - exit(EX_OSERR); > > - } > > - } > > - > > /* Initialize Sasl if -S was specified */ > > if (settings.sasl) { > > init_sasl(); > > @@ -4675,6 +4661,30 @@ int main (int argc, char **argv) { > > } > > > > /* Drop privileges no longer needed */ > > + if (getuid()==0 || geteuid()==0) { > > + if ((pw=getpwnam("_memcached")) == NULL) { > > + fprintf(stderr,"user _memcached not found"); > > + exit(EX_NOUSER); > > + } > > + > > + if((chroot("/var/empty") == -1)) { > > + fprintf(stderr,"check permissions on /var/empty"); > > + exit(EX_OSERR); > > + } > > + > > + if(chdir("/") == -1) { > > + fprintf(stderr," Cannot set new root"); > > + exit(EX_OSERR); > > + } > > + > > + if(setgroups(1, &pw->pw_gid) || > > + setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) || > > + setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid)) { > > + fprintf(stderr," failed to switch to correct > user"); > > + exit(EX_NOUSER); > > + } > > + > > + } > > drop_privileges(); > > > > /* enter the event loop */ > > > > On Tue, Jul 20, 2010 at 10:53 AM, Loganaden Velvindron < > [email protected]> wrote: > > yep it makes sense. > > > > In this case, could we not remove this part and drop root at > the other location > > to gain the jail benefit ? > > > > > > //Logan > > C-x-C-c > > > > On Tue, Jul 20, 2010 at 10:24 AM, dormando <[email protected]> wrote: > > You don't need to run memcached as root to do that, you need to > *start* it > > as root. > > > > If you look just under the setrlimit(RLIMIT_NOFILE code you see > that the > > privilege dropping happens. > > > > So you fire up memcached *from* root, specifying -u memcached pand > it will > > do its root-y things and then drop privileges to that user already. > > > > On Tue, 20 Jul 2010, Loganaden Velvindron wrote: > > > > > It's useful when you need to run memcached as root (-u root). > > > > > > > > > if (setrlimit(RLIMIT_NOFILE, &rlim) != 0) { > > > fprintf(stderr, "failed to set rlimit for open files. Try > running a$ > > > exit(EX_OSERR); > > > } > > > > > > for upping rlimit. > > > > > > Once it's done setting rlimit, root privileges are no longer needed. > > > > > > Additionally, it chroots the process to /var/empty. If the attacker > somehow > > > succeeds in finding an exploit, he cannot execute commands like > /bin/sh, since > > > he's jailed inside the /var/empty. > > > > > > > > > //Logan > > > C-x-C-c > > > On Tue, Jul 20, 2010 at 2:38 AM, dormando <[email protected]> wrote: > > > > > > > Greetings, > > > > > > > > We are a small company who are increasingly relying on > > > > memcached for our big projects. We are very pleased with > > > > its performance. > > > > > > > > I've put this patch that > > > > > > > > 1) chroots to /var/empty > > > > 2) change from root to a simple user. > > > > > > > > It effectively jails the process once it no longer needs root > > > > privilege and allows an attacker very little room to play. > > > > > > > > The patch has been working fine on our gentoo server for > > > > quite some time. > > > > > > > > Feedback is most welcomed, and we are more than willing to > > > > improve the patch to fit your standards. > > > > > > I'm a little confused; there is already a method for memcached to drop > > > user privileges, by specifying the -u option? What's the purpose of > this > > > that the other function doesn't do? > > > > > > > > > > > > > > > -- > > > `` Real men run current !'' > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > `` Real men run current !'' > > > > > > > > > > > > > > > > > > -- > > `` Real men run current !'' > > > > > > > > > > > > > > > > > > > > -- > > `` Real men run current !'' > > > > > > > > > > > > > > > -- awl
