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 !''
>
>
>
>
>
