From owner-p4-projects@FreeBSD.ORG Mon May 10 11:57:04 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id E5E001065673; Mon, 10 May 2010 11:57:03 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 91AF31065670 for ; Mon, 10 May 2010 11:57:03 +0000 (UTC) (envelope-from gpf@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [69.147.83.41]) by mx1.freebsd.org (Postfix) with ESMTP id 766A08FC17 for ; Mon, 10 May 2010 11:57:03 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id o4ABv30b081680 for ; Mon, 10 May 2010 11:57:03 GMT (envelope-from gpf@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id o4ABv3Nc081677 for perforce@freebsd.org; Mon, 10 May 2010 11:57:03 GMT (envelope-from gpf@FreeBSD.org) Date: Mon, 10 May 2010 11:57:03 GMT Message-Id: <201005101157.o4ABv3Nc081677@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to gpf@FreeBSD.org using -f From: Efstratios Karatzas To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 178033 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 May 2010 11:57:04 -0000 http://p4web.freebsd.org/@@178033?ac=10 Change 178033 by gpf@gpf_desktop on 2010/05/10 11:56:53 * log the credentials of the user that initiated the nfs rpc instead of the nfsd thread's root credentials. There is a slight problem: If a user with user_id X does not exist in the server, we just audit the user_id without providing a username which is fine. But it's the same as with doing a `ps -aux` outside of a jail and having user foo inside the jail executing something. If user foo has the same user_id with user bar, who is outside the jail, ps will show that user bar is executing something. Same goes for group ids. It's not a bug, it's a feature. Affected files ... .. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/nfsserver/nfs_srvkrpc.c#3 edit .. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit.c#4 edit .. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit.h#3 edit Differences ... ==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/nfsserver/nfs_srvkrpc.c#3 (text+ko) ==== @@ -352,7 +352,7 @@ } nfsrvstats.srvrpccnt[nd.nd_procnum]++; - AUDIT_NFS_ENTER(procnum, td); + AUDIT_NFS_ENTER(procnum, nd.nd_cr, td); error = proc(&nd, NULL, &mrep); AUDIT_NFS_EXIT(error, td); ==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit.c#4 (text) ==== @@ -730,8 +730,8 @@ * audit_new() will fill in basic thread/credential properties. */ void -audit_nfs_enter(unsigned int proc, struct thread *td) -{ +audit_nfs_enter(unsigned int proc, struct ucred *user_cr, struct thread *td) +{ struct au_mask *aumask; au_class_t class; au_event_t event; @@ -740,7 +740,8 @@ KASSERT(td->td_ar == NULL, ("audit_nfs_enter: td->td_ar != NULL")); KASSERT((td->td_pflags & TDP_AUDITREC) == 0, ("audit_nfs_enter: TDP_AUDITREC set")); - + KASSERT(user_cr != NULL, ("audit_nfs_enter: user_cr == NULL")); + /* XXXgpf: perhaps log a failure to match a rpc to an audit event? */ audit_nfs_proc_to_event(proc, &event); @@ -780,16 +781,38 @@ priv_check(td, PRIV_AUDIT_FAILSTOP) != 0) { cv_wait(&audit_fail_cv, &audit_mtx); panic("audit_failing_stop: thread continued"); - } - td->td_ar = audit_new(event, td); + } + td->td_ar = audit_new(event, td); if (td->td_ar != NULL) td->td_pflags |= TDP_AUDITREC; - } else if (audit_pipe_preselect(auid, event, class, AU_PRS_BOTH, 0)) { + } else if (audit_pipe_preselect(auid, event, class, AU_PRS_BOTH, 0)) { td->td_ar = audit_new(event, td); if (td->td_ar != NULL) td->td_pflags |= TDP_AUDITREC; } else td->td_ar = NULL; + + /* + * Write over the credentials that audit_record_ctor() exported. + * These are the credentials of the user that initiated the NFS RPC. + */ + /* XXXgpf: seems better than having to use another uma_zone_t with a + * different contructor. + * + * I also tried temporarily replacing td's ucred with + * user_cr just before the call to audit_new() like this, but it + * results in bogus behaviour (no nfs events in my log): + * orig_cr = td->td_ucred; + * td->td_ucred = user_cr; + * td->ar = audit_new(...); + * td->td_ucred = orig_cr; + */ + if (td->td_ar != NULL) { + cru2x(user_cr, &td->td_ar->k_ar.ar_subj_cred); + td->td_ar->k_ar.ar_subj_ruid = user_cr->cr_ruid; + td->td_ar->k_ar.ar_subj_rgid = user_cr->cr_rgid; + td->td_ar->k_ar.ar_subj_egid = user_cr->cr_groups[0]; + } } /* ==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit.h#3 (text) ==== @@ -59,7 +59,7 @@ void audit_syscall_enter(unsigned short code, struct thread *td); void audit_syscall_exit(int error, struct thread *td); -void audit_nfs_enter(unsigned int proc, struct thread *td); +void audit_nfs_enter(unsigned int proc, struct ucred *user_cr, struct thread *td); void audit_nfs_exit(int error, struct thread *td); /* @@ -314,9 +314,9 @@ audit_syscall_exit(error, td); \ } while (0) -#define AUDIT_NFS_ENTER(proc, td) do { \ +#define AUDIT_NFS_ENTER(proc, user_cr, td) do { \ if (audit_enabled) { \ - audit_nfs_enter(proc, td); \ + audit_nfs_enter(proc, user_cr, td); \ } \ } while (0)