setuid patch

[Brad Fitzpatrick]

Checked in.

Lisa, your patch was okay, but it was a little too Perl-ish (char* foo = "")
and Avva's is just smaller, which is always nicer.  In any case, I gave
you credit too for doing the bulk of the effort/push.

- Brad


On Sun, 7 Sep 2003, Anatoly Vorobey wrote:

> On Fri, Sep 05, 2003 at 06:13:29PM -0400, Lisa Seelye wrote:
> > Attached is a patch made against the v1.38 memcached.c file to make
> > dropping root privileges possible (with the -u flag).
> >
> > The patch is also available at:
> > http://dev.gentoo.org/~lisa/memcached/setuid.patch
>
> Well... I have to admit it still looks much too large to me, considering
> its rather small (albeit significant) role in the larger scheme of
> things. It still takes up two source files, adds a few error codes used
> just in one place, and hardcodes a username ("root" - I don't know if
> there're admins out there sick enough to use a different username with
> userid 0, but it's possible. I think your testing getuid() after
> setuid() returned success is a shade too paranoid. If we can't trust
> the system functions, who *can* we trust? ;) ).
>
> Consider the following patch, which I've done by basically using your
> code, changing the root-detection logic a little as described in my
> previous mail, condensing and moving to memcached.c. It adds about
> 15 lines of code, defines no new constants or functions, and, I think,
> gets the job done (note that in case the program isn't run as root,
> the -u switch is ignored, which is documented in -h, and seems to be
> adequate).
>
> Note: I've only tested it on Linux, someone should test on a BSD.
>
> main -> wcmtools          src/memcached/memcached.c
> --- cvs/wcmtools/memcached/memcached.c	Fri Sep  5 11:37:49 2003
> +++ src/memcached/memcached.c	Sat Sep  6 14:04:03 2003
> @@ -28,6 +28,7 @@
>  #ifndef _P1003_1B_VISIBLE
>  #define _P1003_1B_VISIBLE
>  #endif
> +#include <pwd.h>
>  #include <sys/mman.h>
>  #include <fcntl.h>
>  #include <stdlib.h>
> @@ -1161,6 +1162,7 @@
>      printf("-p <num>      port number to listen on\n");
>      printf("-l <ip_addr>  interface to listen on, default is INDRR_ANY\n");
>      printf("-d            run as a daemon\n");
> +    printf("-u <username> assume identity of <username> (only when run as root)\n");
>      printf("-m <num>      max memory to use for items in megabytes, default is 64 MB\n");
>      printf("-c <num>      max simultaneous connections, default is 1024\n");
>      printf("-k            lock down all paged memory\n");
> @@ -1250,12 +1252,14 @@
>      struct in_addr addr;
>      int lock_memory = 0;
>      int daemonize = 0;
> +    char *username = 0;
> +    struct passwd *pw;
>
>      /* init settings */
>      settings_init();
>
>      /* process arguments */
> -    while ((c = getopt(argc, argv, "p:m:c:khivdl:")) != -1) {
> +    while ((c = getopt(argc, argv, "p:m:c:khivdl:u:")) != -1) {
>          switch (c) {
>          case 'p':
>              settings.port = atoi(optarg);
> @@ -1289,6 +1293,9 @@
>          case 'd':
>              daemonize = 1;
>              break;
> +        case 'u':
> +            username = optarg;
> +            break;
>          default:
>              fprintf(stderr, "Illegal argument \"%c\"\n", c);
>              return 1;
> @@ -1310,6 +1317,22 @@
>  #else
>          fprintf(stderr, "warning: mlockall() not supported on this platform.  proceeding without.\n");
>  #endif
> +    }
> +
> +    /* 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");
> +            return 1;
> +        }
> +        if ((pw = getpwnam(username)) == 0) {
> +            fprintf(stderr, "can't find the user %s to switch to\n", username);
> +            return 1;
> +        }
> +        if (setgid(pw->pw_gid)<0 || setuid(pw->pw_uid)<0) {
> +            fprintf(stderr, "failed to assume identity of user %s\n", username);
> +            return 1;
> +        }
>      }
>
>      if (daemonize) {
>
>