From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Serge E. Hallyn" Subject: Re: Proof of concept patch, add dropping privileges to a non root user Date: Tue, 20 Oct 2009 10:14:53 -0500 Message-ID: <20091020151453.GA5848@us.ibm.com> References: <4ADDC422.3000108@geomatys.fr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mx1.redhat.com (ext-mx08.extmail.prod.ext.phx2.redhat.com [10.5.110.12]) by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id n9L0dU72010854 for ; Tue, 20 Oct 2009 20:39:31 -0400 Received: from e39.co.us.ibm.com (e39.co.us.ibm.com [32.97.110.160]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n9L0dIat011363 for ; Tue, 20 Oct 2009 20:39:18 -0400 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by e39.co.us.ibm.com (8.14.3/8.13.1) with ESMTP id n9L0XNiA014264 for ; Tue, 20 Oct 2009 18:33:23 -0600 Received: from d03av01.boulder.ibm.com (d03av01.boulder.ibm.com [9.17.195.167]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id n9L0d6bp098462 for ; Tue, 20 Oct 2009 18:39:10 -0600 Received: from d03av01.boulder.ibm.com (loopback [127.0.0.1]) by d03av01.boulder.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id n9L0d5Jc023607 for ; Tue, 20 Oct 2009 18:39:06 -0600 Content-Disposition: inline In-Reply-To: <4ADDC422.3000108@geomatys.fr> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: "corentin.labbe" Cc: linux-audit@redhat.com List-Id: linux-audit@redhat.com Quoting corentin.labbe (corentin.labbe@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@redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit