Date: Mon, 10 May 2010 11:57:03 GMT From: Efstratios Karatzas <gpf@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 178033 for review Message-ID: <201005101157.o4ABv3Nc081677@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201005101157.o4ABv3Nc081677>