Proof of concept patch, add dropping privileges to a non root user
Serge E. Hallyn
serue at us.ibm.com
Tue Oct 20 15:14:53 UTC 2009
Quoting corentin.labbe (corentin.labbe at geomatys.fr):
> Hello
>
> This is a patch that add a -u parameter to auditd.
> This parameter permit to auditd to drop to an unprivilegied UID after initialization.
>
> Any comment will be appreciated.
>
> Cordially
>
>
>
> --- src/auditd.c.orig 2009-10-05 14:18:52.000000000 +0200
> +++ src/auditd.c 2009-10-05 14:55:36.000000000 +0200
> @@ -471,9 +471,10 @@
> struct ev_signal sigusr2_watcher;
> struct ev_signal sigchld_watcher;
> int rc;
> + int auditd_uid=0;
>
> /* Get params && set mode */
> - while ((c = getopt(argc, argv, "flns:")) != -1) {
> + while ((c = getopt(argc, argv, "flns:u:")) != -1) {
> switch (c) {
> case 'f':
> opt_foreground = 1;
> @@ -481,6 +482,17 @@
> case 'l':
> opt_allow_links=1;
> break;
> + case 'u':
> + auditd_uid = atoi(optarg);
> + if (auditd_uid > 65535) {
> + fprintf(stderr, "Invalid UID '%s' > 65535\n", optarg);
> + usage();
> + }
> + if (auditd_uid < 0) {
> + fprintf(stderr, "Invalid UID '%s' < 0\n", optarg);
> + usage();
> + }
> + break;
> case 'n':
> do_fork = 0;
> break;
> @@ -522,7 +534,7 @@
>
> #ifndef DEBUG
> /* Make sure we are root */
> - if (getuid() != 0) {
> + if (getuid() != 0 && auditd_uid == 0) {
I don't have the original source in front of me, but I think what
you'd really want to do here is check that
if (geteuid() != 0) {
...
}
or better yet do a detailed check for the capabilities you need,
which I suppose are something like
if (!got_caps(CAP_AUDIT_CONTROL | CAP_AUDIT_WRITE))
complain();
if (getuid() != auditd_uid && !got_caps(CAP_SETUID))
complain();
> fprintf(stderr, "You must be root to run this program.\n");
> return 4;
> }
> @@ -690,6 +702,14 @@
> shutdown_dispatcher();
> return 1;
> }
> +
> + if (auditd_uid > 0)
> + if (setuid(auditd_uid) == -1) {
> + fprintf(stderr, "setuid error() %d.\n", errno);
> + shutdown_dispatcher();
> + return 1;
> + }
I think it's always worthwhile to follow this by a
getresuid(&r, &e, &s);
if (r != auditd_uid || e != auditd_uid || s != auditd_uid)
bail();
I don't really know that an attacker could set things up so that
uid and suid wouldn't get set (i.e. !CAP_SETUID, and uid==auditd_uid,
but it's conceivable - i.e. finds a way to drop CAP_SETUID from the
bounding set through another vulnerability, then runs a setuid root
auditd using 'auditd -u `id -u`'. That's not quite it, as saveduid
would have to be 0, and i can't recall offhand whether execve() of
a setuid-root binary sets saved_uid to 0 or not. But hopefully this
rant is scary enough to convince you that it's worth just making
sure :)
> +
> audit_msg(LOG_NOTICE,
> "Init complete, auditd %s listening for events (startup state %s)",
> VERSION,
> --
> Linux-audit mailing list
> Linux-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
More information about the Linux-audit
mailing list