PERFORCE change 178033 for review

[Efstratios Karatzas]

http://p4web.freebsd.org/@@178033?ac=10

Change 178033 by gpf at 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)