[Oisf-devel] [RFC] change accounts when starting up

[Will Metcalf]

Heya Steve,

I will have a look tomorrow. I see one thing right away that we will
have to address. Certain run modes will require root access such as
inline mode that uses NFQUEUE.  I think the file handles are also
opened as root before we drop privs.  Like I said though I will have a
look tomorrow as it is something that we need to support at least for
pcap live mode.

Regards,

Will

On Mon, Feb 22, 2010 at 10:31 AM, Steve Grubb <sgrubb at redhat.com> wrote:
> Hello,
>
> This is a prototype for a patch. Suricata is likely to run across some evil
> packets as it runs. If there is a parser error of any kind, you really don't
> want suricata to be an attack point since its running as root. This patch
> would add a -u and -g command line option.
>
> However...I noticed when writing it that -u is taken by the optional unit
> tests. For the end user, it would be more intuitive if -u was allowed to be
> the user account. So, I guess I'm asking if allowing -u to the be reassigned
> might be permissible? Or do you have other suggestions for their names?
>
> I also have not idea if Windows supports setuid() so I #ifndef'ed it for now.
>
> Thanks,
> -Steve
>
>
> diff -urp oisf.orig/src/suricata.c oisf/src/suricata.c
> --- oisf.orig/src/suricata.c    2010-02-22 09:28:22.000000000 -0500
> +++ oisf/src/suricata.c 2010-02-22 11:23:55.000000000 -0500
> @@ -5,7 +5,10 @@
>  #include <getopt.h>
>  #include <signal.h>
>  #include <pthread.h>
> -
> +#ifndef OS_WIN32
> +#include <pwd.h>
> +#include <grp.h>
> +#endif
>  #include "suricata.h"
>  #include "decode.h"
>  #include "detect.h"
> @@ -369,7 +372,14 @@ void usage(const char *progname)
>     printf("\t-U, --unittest-filter=REGEX  : filter unittests with a regex\n");
>     printf("\t--list-unittests             : list unit tests\n");
>     printf("\t--fatal-unittests            : enable fatal failure on unittest error\n");
> +#else
> +#ifndef OS_WIN32
> +    printf("\t-u                           : run under the given user account\n");
> +#endif
>  #endif /* UNITTESTS */
> +#ifndef OS_WIN32
> +    printf("\t-g                           : run under the given group account\n");
> +#endif /*OS_WIN32 */
>     printf("\t--init-errors-fatal          : enable fatal failure on signature init error\n");
>     printf("\t--dump-config                : show the running configuration\n");
>     printf("\t--pfring-int <dev>           : run in pfring mode\n");
> @@ -396,6 +406,10 @@ int main(int argc, char **argv)
>     int dump_config = 0;
>     int list_unittests = 0;
>     int daemon = 0;
> +#ifndef OS_WIN32
> +    uid_t uid = 0;
> +    gid_t gid = 0;
> +#endif
>
>     char *log_dir;
>     struct stat buf;
> @@ -428,7 +442,11 @@ int main(int argc, char **argv)
>     /* getopt_long stores the option index here. */
>     int option_index = 0;
>
> -    char short_opts[] = "c:Dhi:l:q:d:r:us:U:V";
> +#ifdef UNITTESTS
> +    char short_opts[] = "c:Dhi:l:q:d:r:us:U:Vg:";
> +#else
> +    char short_opts[] = "c:Dhi:l:q:d:r:u:s:U:Vg:";
> +#endif
>
>     while ((opt = getopt_long(argc, argv, short_opts, long_opts, &option_index)) != -1) {
>         switch (opt) {
> @@ -556,8 +574,29 @@ int main(int argc, char **argv)
>                 exit(EXIT_SUCCESS);
>             }
>  #else
> +#ifndef OS_WIN32
> +            if (isdigit(optarg[0])) {
> +                errno = 0;
> +                uid = strtoul(optarg,NULL,10);
> +                if (errno) {
> +                    fprintf(stderr,
> +                        "Numeric user ID conversion error (%s) for %s\n",
> +                        strerror(errno), optarg);
> +                    exit(EXIT_FAILURE);
> +                }
> +            } else {
> +                struct passwd *pw;
> +                pw = getpwnam(optarg);
> +                if (pw == NULL) {
> +                    fprintf(stderr, "User name is unknown (%s)\n", optarg);
> +                    exit(EXIT_FAILURE);
> +                }
> +                uid = pw->pw_uid;
> +            }
> +#else
>             fprintf(stderr, "ERROR: Unit tests not enabled. Make sure to pass --enable-unittests to configure when building.\n");
>             exit(EXIT_FAILURE);
> +#endif
>  #endif /* UNITTESTS */
>             break;
>         case 'U':
> @@ -571,6 +610,28 @@ int main(int argc, char **argv)
>         case 'V':
>             printf("\nThis is %s version %s\n\n", PROG_NAME, PROG_VER);
>             exit(EXIT_SUCCESS);
> +#ifndef OS_WIN32
> +        case 'g':
> +            if (isdigit(optarg[0])) {
> +                errno = 0;
> +                gid = strtoul(optarg,NULL,10);
> +                if (errno) {
> +                    fprintf(stderr,
> +                        "Numeric group ID conversion error (%s) for %s\n",
> +                        strerror(errno), optarg);
> +                    exit(EXIT_FAILURE);
> +                }
> +            } else {
> +                struct group *gr;
> +                gr = getgrnam(optarg);
> +                if (gr == NULL) {
> +                    fprintf(stderr, "Group name is unknown (%s)\n", optarg);
> +                   exit(EXIT_FAILURE);
> +                }
> +                gid = gr->gr_gid;
> +            }
> +            break;
> +#endif
>         default:
>             usage(argv[0]);
>             exit(EXIT_FAILURE);
> @@ -792,6 +853,15 @@ int main(int argc, char **argv)
>     SignalHandlerSetup(SIGINT, SignalHandlerSigint);
>     SignalHandlerSetup(SIGTERM, SignalHandlerSigterm);
>     SignalHandlerSetup(SIGHUP, SignalHandlerSighup);
> +
> +    if (gid != 0 && setgid(gid)) {
> +        SCLogInfo("Failed changing group id");
> +        exit(EXIT_FAILURE);
> +    }
> +    if (uid != 0 && setuid(uid)) {
> +        SCLogInfo("Failed changing user id");
> +        exit(EXIT_FAILURE);
> +    }
>  #endif /* OS_WIN32 */
>
>     /* pre allocate packets */
> _______________________________________________
> Oisf-devel mailing list
> Oisf-devel at openinfosecfoundation.org
> http://lists.openinfosecfoundation.org/mailman/listinfo/oisf-devel
>