On Fri, Nov 01, 2002 at 05:15:30PM +1300, Gregory McGarry wrote:
> Thanks for the feedback David.  I've adopted your recommendations.
> 
> David Laight wrote:
> 
> > NFS loads user credentials directly, to be absolutely sure
> > root privileged are not checked by mistake the 'e' and 'sv'
> > fields should be set to be the same as the 'r' one.
> 
> I was hoping to develop an interface in conjunction with ACLs
> that would allow the credentials to be checked without frobbing
> the inner details of the ucred structure.  Little steps...

Isn't that backwards?
IIRC items (like files) have ACLs processes have credentials.
What you might want to look at is implementing DAC (and MAC?)
(Discretionary Access Controls - basically replacing suser()
with a bit pattern).

It is crcvt() that needs to ensure it populates all the cred
structure.

> > In sys_execve (kern_exec.c) and fork1 (kern_fork.c) you are
> > calling crcopy() when the cred structure is unchanged.
> > This stops structure being shared.....
> 
> You're right about fork1().  sys_execve() preserves existing
> behaviour.

I doubt it! the existing code reads:

	if ((p->p_flag & P_TRACED) == 0 &&

	    (((attr.va_mode & S_ISUID) != 0 &&
	      p->p_ucred->cr_uid != attr.va_uid) ||

	     ((attr.va_mode & S_ISGID) != 0 &&
	      p->p_ucred->cr_gid != attr.va_gid))) {
		...
		p->p_ucred = crcopy(cred);
		...
		if (attr.va_mode & S_ISUID)
			p->p_ucred->cr_uid = attr.va_uid;
		if (attr.va_mode & S_ISGID)
			p->p_ucred->cr_gid = attr.va_gid;
	} else
		p->p_flag &= ~P_SUGID;
	p->p_cred->p_svuid = p->p_ucred->cr_uid;
	p->p_cred->p_svgid = p->p_ucred->cr_gid;

So crcopy is only called if the structure changes.
(You probably noticed that the reference count on p_cred is only
ever one - so that structure can be safely stomped all over.)

You might want to look at by kernel.diff file (www.l8s.co.uk
then netbsd), I've deleted some separate changes, but the uid stuff
gets everywhere - so you have my alternate pid allocater.

You might want to look at the do_setu/gid() functions.  They
remove knowedge of uid changing from all the compat code without
really bloating the normal kernel (it might even shrink).

int
do_setuid(struct proc *p, uid_t e, uid_t r, uid_t sv, u_int fl)
{
	int error;
	struct ucred *cr = p->p_ucred;
		 
	/* superuser can do anything it wants to.... */
	error = suser(cr, &p->p_acflag);
	if (error) {
		if (r != -1 && !((fl & R_EQUALS_R) && r == cr->cr_ruid)
			    && !((fl & R_EQUALS_E) && r == cr->cr_uid)
			    && !((fl & R_EQUALS_S) && r == cr->cr_svuid))
			return error;
		if (e != -1 && !((fl & E_EQUALS_R) && e == cr->cr_ruid)
			    && !((fl & E_EQUALS_E) && e == cr->cr_uid)
			    && !((fl & E_EQUALS_S) && e == cr->cr_svuid))
			return error;
		if (sv != -1 && !((fl & S_EQUALS_R) && sv == cr->cr_ruid)
			    && !((fl & S_EQUALS_E) && sv == cr->cr_uid)
			    && !((fl & S_EQUALS_S) && sv == cr->cr_svuid))
			return error;
	}

	/* If nothing has changed, short circuit the request */
	if ((r == -1 || r == cr->cr_ruid)
	    && (e == -1 || e == cr->cr_uid)
	    && (sv == -1 || sv == cr->cr_svuid))
		/* nothing to do */
		return 0;

	/* Update a clone of the current credentials */
	p->p_ucred = cr = crcopy(cr);
	if (r != -1) {
		/* Update count of processes for this user */
		(void)chgproccnt(p, -1);
		cr->cr_ruid = r;
		(void)chgproccnt(p, 1);
	}
	if (e != -1)
		cr->cr_uid = e;
	if (sv != -1)
		cr->cr_svuid = sv;
	/* Mark process as having changed credentials, stops tracing etc */
	p_sugid(p);
	return 0;
}

sys_setuid, for example is then just:
	return do_setuid(p, uid, uid, uid, E_EQUALS_R|R_EQUALS_R|S_EQUALS_R);

	David

-- 
David Laight: david@l8s.co.uk