Subject: commoning up code that changes uids and gids
To: None <tech-kern@netbsd.org>
From: David Laight <david@l8s.co.uk>
List: tech-kern
Date: 03/03/2003 22:25:56
Currently the tests for the legality of setuid and setgid calls are
repeated in each of the functions that can set new values, as is any
ancillary action that must be done in order to keep the data structures
consistent (eg the chgproccnt() calls).
The change below uses two 'helper' functions do_setres{u,g}id to
perform the actual change, allowing the individual system calls to
just specify any restrictions on the legal changes.
(non-superuser can never introduce a value that isn't the real,
effective of saved one).
This also stops the 'compat' functions getting out of step with
any future changes.
Unless anyone objects I'll commit the change later in the week
(probably Wednesday evening or Thursday morning).
David
Index: kern/kern_prot.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_prot.c,v
retrieving revision 1.75
diff -u -p -r1.75 kern_prot.c
--- kern/kern_prot.c 2003/02/28 23:24:40 1.75
+++ kern/kern_prot.c 2003/03/03 22:09:50
@@ -312,6 +312,124 @@ sys_setpgid(struct lwp *l, void *v, regi
return (enterpgrp(targp, SCARG(uap, pgid), 0));
}
+/*
+ * Set real, effective and saved uids to the requested values.
+ * non-root callers can only ever change uids to values that match
+ * one of the processes current uid values.
+ * This is further restricted by the flags argument.
+ */
+
+int
+do_setresuid(struct lwp *l, uid_t r, uid_t e, uid_t sv, u_int flags)
+{
+ int error;
+ struct proc *p = l->l_proc;
+ struct pcred *pcred = p->p_cred;
+ struct ucred *cred = pcred->pc_ucred;
+
+ /* Superuser can do anything it wants to.... */
+ error = suser(cred, &p->p_acflag);
+ if (error) {
+ /* Otherwise check new value is one of the allowed
+ existing values. */
+ if (r != -1 && !((flags & ID_R_EQ_R) && r == pcred->p_ruid)
+ && !((flags & ID_R_EQ_E) && r == cred->cr_uid)
+ && !((flags & ID_R_EQ_S) && r == pcred->p_svuid))
+ return error;
+ if (e != -1 && !((flags & ID_E_EQ_R) && e == pcred->p_ruid)
+ && !((flags & ID_E_EQ_E) && e == cred->cr_uid)
+ && !((flags & ID_E_EQ_S) && e == pcred->p_svuid))
+ return error;
+ if (sv != -1 && !((flags & ID_S_EQ_R) && sv == pcred->p_ruid)
+ && !((flags & ID_S_EQ_E) && sv == cred->cr_uid)
+ && !((flags & ID_S_EQ_S) && sv == pcred->p_svuid))
+ return error;
+ }
+
+ /* If nothing has changed, short circuit the request */
+ if ((r == -1 || r == pcred->p_ruid)
+ && (e == -1 || e == cred->cr_uid)
+ && (sv == -1 || sv == pcred->p_svuid))
+ /* nothing to do */
+ return 0;
+
+ /* The pcred structure is not actually shared... */
+ if (r != -1 && r != pcred->p_ruid) {
+ /* Update count of processes for this user */
+ (void)chgproccnt(pcred->p_ruid, -1);
+ (void)chgproccnt(r, 1);
+ pcred->p_ruid = r;
+ }
+ if (sv != -1)
+ pcred->p_svuid = sv;
+ if (e != -1 && e != cred->cr_uid) {
+ /* Update a clone of the current credentials */
+ pcred->pc_ucred = cred = crcopy(cred);
+ cred->cr_uid = e;
+ }
+
+ /* Mark process as having changed credentials, stops tracing etc */
+ p_sugid(p);
+ return 0;
+}
+
+/*
+ * Set real, effective and saved gids to the requested values.
+ * non-root callers can only ever change gids to values that match
+ * one of the processes current gid values.
+ * This is further restricted by the flags argument.
+ */
+
+int
+do_setresgid(struct lwp *l, gid_t r, gid_t e, gid_t sv, u_int flags)
+{
+ int error;
+ struct proc *p = l->l_proc;
+ struct pcred *pcred = p->p_cred;
+ struct ucred *cred = pcred->pc_ucred;
+
+ /* Superuser can do anything it wants to.... */
+ error = suser(cred, &p->p_acflag);
+ if (error) {
+ /* Otherwise check new value is one of the allowed
+ existing values. */
+ if (r != -1 && !((flags & ID_R_EQ_R) && r == pcred->p_rgid)
+ && !((flags & ID_R_EQ_E) && r == cred->cr_gid)
+ && !((flags & ID_R_EQ_S) && r == pcred->p_svgid))
+ return error;
+ if (e != -1 && !((flags & ID_E_EQ_R) && e == pcred->p_rgid)
+ && !((flags & ID_E_EQ_E) && e == cred->cr_gid)
+ && !((flags & ID_E_EQ_S) && e == pcred->p_svgid))
+ return error;
+ if (sv != -1 && !((flags & ID_S_EQ_R) && sv == pcred->p_rgid)
+ && !((flags & ID_S_EQ_E) && sv == cred->cr_gid)
+ && !((flags & ID_S_EQ_S) && sv == pcred->p_svgid))
+ return error;
+ }
+
+ /* If nothing has changed, short circuit the request */
+ if ((r == -1 || r == pcred->p_rgid)
+ && (e == -1 || e == cred->cr_gid)
+ && (sv == -1 || sv == pcred->p_svgid))
+ /* nothing to do */
+ return 0;
+
+ /* The pcred structure is not actually shared... */
+ if (r != -1)
+ pcred->p_rgid = r;
+ if (sv != -1)
+ pcred->p_svgid = sv;
+ if (e != -1 && e != cred->cr_gid) {
+ /* Update a clone of the current credentials */
+ pcred->pc_ucred = cred = crcopy(cred);
+ cred->cr_gid = e;
+ }
+
+ /* Mark process as having changed credentials, stops tracing etc */
+ p_sugid(p);
+ return 0;
+}
+
/* ARGSUSED */
int
sys_setuid(struct lwp *l, void *v, register_t *retval)
@@ -319,34 +437,10 @@ sys_setuid(struct lwp *l, void *v, regis
struct sys_setuid_args /* {
syscallarg(uid_t) uid;
} */ *uap = v;
- struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
- uid_t uid;
- int error;
+ uid_t uid = SCARG(uap, uid);
- uid = SCARG(uap, uid);
- if (uid != pc->p_ruid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
- /*
- * Check if we are all set, and this is a no-op.
- */
- if (pc->p_ruid == uid && pc->p_svuid == uid &&
- pc->pc_ucred->cr_uid == uid)
- return (0);
- /*
- * Everything's okay, do it.
- * Transfer proc count to new user.
- * Copy credentials so other references do not see our changes.
- */
- (void)chgproccnt(pc->p_ruid, -1);
- (void)chgproccnt(uid, 1);
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_uid = uid;
- pc->p_ruid = uid;
- pc->p_svuid = uid;
- p_sugid(p);
- return (0);
+ return do_setresuid(l, uid, uid, uid,
+ ID_R_EQ_R | ID_E_EQ_R | ID_S_EQ_R);
}
/* ARGSUSED */
@@ -356,29 +450,8 @@ sys_seteuid(struct lwp *l, void *v, regi
struct sys_seteuid_args /* {
syscallarg(uid_t) euid;
} */ *uap = v;
- struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
- uid_t euid;
- int error;
- euid = SCARG(uap, euid);
- if (euid != pc->p_ruid && euid != pc->p_svuid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
- /*
- * Check if we are all set, and this is a no-op.
- */
- if (pc->pc_ucred->cr_uid == euid)
- return (0);
-
- /*
- * Everything's okay, do it. Copy credentials so other references do
- * not see our changes.
- */
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_uid = euid;
- p_sugid(p);
- return (0);
+ return do_setresuid(l, -1, SCARG(uap, euid), -1, ID_E_EQ_R | ID_E_EQ_S);
}
int
@@ -389,42 +462,21 @@ sys_setreuid(struct lwp *l, void *v, reg
syscallarg(uid_t) euid;
} */ *uap = v;
struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
- uid_t ruid, euid;
- int error, changed = 0;
+ uid_t ruid, euid, svuid;
ruid = SCARG(uap, ruid);
euid = SCARG(uap, euid);
-
- if (ruid != (uid_t)-1 &&
- ruid != pc->p_ruid && ruid != pc->pc_ucred->cr_uid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- if (euid != (uid_t)-1 &&
- euid != pc->p_ruid && euid != pc->pc_ucred->cr_uid &&
- euid != pc->p_svuid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- if (euid != (uid_t)-1 && euid != pc->pc_ucred->cr_uid) {
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_uid = euid;
- changed++;
- }
-
- if (ruid != (uid_t)-1 &&
- (pc->p_ruid != ruid || pc->p_svuid != pc->pc_ucred->cr_uid)) {
- (void)chgproccnt(pc->p_ruid, -1);
- (void)chgproccnt(ruid, 1);
- pc->p_ruid = ruid;
- pc->p_svuid = pc->pc_ucred->cr_uid;
- changed++;
- }
-
- if (changed)
- p_sugid(p);
- return (0);
+ if (ruid == -1)
+ ruid = p->p_cred->p_ruid;
+ if (euid == -1)
+ euid = p->p_ucred->cr_uid;
+ /* Saved uid is set to the new euid if the ruid changed */
+ svuid = ruid == p->p_cred->p_ruid ? -1 : euid;
+
+ return do_setresuid(l, ruid, euid, svuid,
+ ID_R_EQ_R | ID_R_EQ_E |
+ ID_E_EQ_R | ID_E_EQ_E | ID_E_EQ_S |
+ ID_S_EQ_R | ID_S_EQ_E | ID_S_EQ_S);
}
/* ARGSUSED */
@@ -434,28 +486,10 @@ sys_setgid(struct lwp *l, void *v, regis
struct sys_setgid_args /* {
syscallarg(gid_t) gid;
} */ *uap = v;
- struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
- gid_t gid;
- int error;
+ gid_t gid = SCARG(uap, gid);
- gid = SCARG(uap, gid);
- if (gid != pc->p_rgid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
- /*
- * Check if we are all set, and this is a no-op.
- */
- if (pc->pc_ucred->cr_gid == gid && pc->p_rgid == gid &&
- pc->p_svgid == gid)
- return (0);
-
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_gid = gid;
- pc->p_rgid = gid;
- pc->p_svgid = gid;
- p_sugid(p);
- return (0);
+ return do_setresgid(l, gid, gid, gid,
+ ID_R_EQ_R | ID_E_EQ_R | ID_S_EQ_R);
}
/* ARGSUSED */
@@ -465,25 +499,8 @@ sys_setegid(struct lwp *l, void *v, regi
struct sys_setegid_args /* {
syscallarg(gid_t) egid;
} */ *uap = v;
- struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
- gid_t egid;
- int error;
-
- egid = SCARG(uap, egid);
- if (egid != pc->p_rgid && egid != pc->p_svgid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
- /*
- * Check if we are all set, and this is a no-op.
- */
- if (pc->pc_ucred->cr_gid == egid)
- return (0);
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_gid = egid;
- p_sugid(p);
- return (0);
+ return do_setresgid(l, -1, SCARG(uap, egid), -1, ID_E_EQ_R | ID_E_EQ_S);
}
int
@@ -494,40 +511,21 @@ sys_setregid(struct lwp *l, void *v, reg
syscallarg(gid_t) egid;
} */ *uap = v;
struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
- gid_t rgid, egid;
- int error, changed = 0;
+ gid_t rgid, egid, svgid;
rgid = SCARG(uap, rgid);
egid = SCARG(uap, egid);
-
- if (rgid != (gid_t)-1 &&
- rgid != pc->p_rgid && rgid != pc->pc_ucred->cr_gid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- if (egid != (gid_t)-1 &&
- egid != pc->p_rgid && egid != pc->pc_ucred->cr_gid &&
- egid != pc->p_svgid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- if (egid != (gid_t)-1 && pc->pc_ucred->cr_gid != egid) {
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_gid = egid;
- changed++;
- }
-
- if (rgid != (gid_t)-1 &&
- (pc->p_rgid != rgid || pc->p_svgid != pc->pc_ucred->cr_gid)) {
- pc->p_rgid = rgid;
- pc->p_svgid = pc->pc_ucred->cr_gid;
- changed++;
- }
-
- if (changed)
- p_sugid(p);
- return (0);
+ if (rgid == -1)
+ rgid = p->p_cred->p_rgid;
+ if (egid == -1)
+ egid = p->p_ucred->cr_gid;
+ /* Saved gid is set to the new egid if the rgid changed */
+ svgid = rgid == p->p_cred->p_rgid ? -1 : egid;
+
+ return do_setresgid(l, rgid, egid, svgid,
+ ID_R_EQ_R | ID_R_EQ_E |
+ ID_E_EQ_R | ID_E_EQ_E | ID_E_EQ_S |
+ ID_S_EQ_R | ID_S_EQ_E | ID_S_EQ_S);
}
int
Index: sys/ucred.h
===================================================================
RCS file: /cvsroot/src/sys/sys/ucred.h,v
retrieving revision 1.15
diff -u -p -r1.15 ucred.h
--- sys/ucred.h 2003/02/18 08:37:43 1.15
+++ sys/ucred.h 2003/03/03 22:09:50
@@ -65,6 +65,19 @@ struct ucred {
#ifdef _KERNEL
#define crhold(cr) (cr)->cr_ref++
+/* flags that control when do_setres{u,g}id will do anything */
+#define ID_E_EQ_E 0x001 /* effective equals effective */
+#define ID_E_EQ_R 0x002 /* effective equals real */
+#define ID_E_EQ_S 0x004 /* effective equals saved */
+#define ID_R_EQ_E 0x010 /* real equals effective */
+#define ID_R_EQ_R 0x020 /* real equals real */
+#define ID_R_EQ_S 0x040 /* real equals saved */
+#define ID_S_EQ_E 0x100 /* saved equals effective */
+#define ID_S_EQ_R 0x200 /* saved equals real */
+#define ID_S_EQ_S 0x400 /* saved equals saved */
+
+int do_setresuid(struct lwp *, uid_t, uid_t, uid_t, u_int);
+int do_setresgid(struct lwp *, gid_t, gid_t, gid_t, u_int);
struct ucred *crcopy(struct ucred *);
struct ucred *crdup(const struct ucred *);
Index: compat/linux/common/linux_misc.c
===================================================================
RCS file: /cvsroot/src/sys/compat/linux/common/linux_misc.c,v
retrieving revision 1.116
diff -u -p -r1.116 linux_misc.c
--- compat/linux/common/linux_misc.c 2003/01/18 08:02:53 1.116
+++ compat/linux/common/linux_misc.c 2003/03/03 22:09:56
@@ -1203,64 +1203,18 @@ linux_sys_setresuid(l, v, retval)
syscallarg(uid_t) euid;
syscallarg(uid_t) suid;
} */ *uap = v;
- struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
- uid_t ruid, euid, suid;
- int error;
- ruid = SCARG(uap, ruid);
- euid = SCARG(uap, euid);
- suid = SCARG(uap, suid);
-
/*
* Note: These checks are a little different than the NetBSD
* setreuid(2) call performs. This precisely follows the
* behavior of the Linux kernel.
*/
- if (ruid != (uid_t)-1 &&
- ruid != pc->p_ruid &&
- ruid != pc->pc_ucred->cr_uid &&
- ruid != pc->p_svuid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- if (euid != (uid_t)-1 &&
- euid != pc->p_ruid &&
- euid != pc->pc_ucred->cr_uid &&
- euid != pc->p_svuid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- if (suid != (uid_t)-1 &&
- suid != pc->p_ruid &&
- suid != pc->pc_ucred->cr_uid &&
- suid != pc->p_svuid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- /*
- * Now assign the new real, effective, and saved UIDs.
- * Note that Linux, unlike NetBSD in setreuid(2), does not
- * set the saved UID in this call unless the user specifies
- * it.
- */
- if (ruid != (uid_t)-1) {
- (void)chgproccnt(pc->p_ruid, -1);
- (void)chgproccnt(ruid, 1);
- pc->p_ruid = ruid;
- }
-
- if (euid != (uid_t)-1) {
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_uid = euid;
- }
-
- if (suid != (uid_t)-1)
- pc->p_svuid = suid;
- if (ruid != (uid_t)-1 && euid != (uid_t)-1 && suid != (uid_t)-1)
- p->p_flag |= P_SUGID;
- return (0);
+ return do_setresuid(l, SCARG(uap, ruid), SCARG(uap, euid),
+ SCARG(uap, suid),
+ ID_R_EQ_R | ID_R_EQ_E | ID_R_EQ_S |
+ ID_E_EQ_R | ID_E_EQ_E | ID_E_EQ_S |
+ ID_S_EQ_R | ID_S_EQ_E | ID_S_EQ_S );
}
int
Index: compat/linux/common/linux_misc_notalpha.c
===================================================================
RCS file: /cvsroot/src/sys/compat/linux/common/linux_misc_notalpha.c,v
retrieving revision 1.66
diff -u -p -r1.66 linux_misc_notalpha.c
--- compat/linux/common/linux_misc_notalpha.c 2003/02/23 23:36:35 1.66
+++ compat/linux/common/linux_misc_notalpha.c 2003/03/03 22:09:57
@@ -339,61 +339,17 @@ linux_sys_setresgid(l, v, retval)
syscallarg(gid_t) egid;
syscallarg(gid_t) sgid;
} */ *uap = v;
- struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
- gid_t rgid, egid, sgid;
- int error;
- rgid = SCARG(uap, rgid);
- egid = SCARG(uap, egid);
- sgid = SCARG(uap, sgid);
-
/*
* Note: These checks are a little different than the NetBSD
* setregid(2) call performs. This precisely follows the
* behavior of the Linux kernel.
*/
- if (rgid != (gid_t)-1 &&
- rgid != pc->p_rgid &&
- rgid != pc->pc_ucred->cr_gid &&
- rgid != pc->p_svgid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- if (egid != (gid_t)-1 &&
- egid != pc->p_rgid &&
- egid != pc->pc_ucred->cr_gid &&
- egid != pc->p_svgid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- if (sgid != (gid_t)-1 &&
- sgid != pc->p_rgid &&
- sgid != pc->pc_ucred->cr_gid &&
- sgid != pc->p_svgid &&
- (error = suser(pc->pc_ucred, &p->p_acflag)))
- return (error);
-
- /*
- * Now assign the real, effective, and saved GIDs.
- * Note that Linux, unlike NetBSD in setregid(2), does not
- * set the saved UID in this call unless the user specifies
- * it.
- */
- if (rgid != (gid_t)-1)
- pc->p_rgid = rgid;
-
- if (egid != (gid_t)-1) {
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_gid = egid;
- }
-
- if (sgid != (gid_t)-1)
- pc->p_svgid = sgid;
-
- if (rgid != (gid_t)-1 && egid != (gid_t)-1 && sgid != (gid_t)-1)
- p->p_flag |= P_SUGID;
- return (0);
+ return do_setresgid(l, SCARG(uap,rgid), SCARG(uap, egid),
+ SCARG(uap, sgid),
+ ID_R_EQ_R | ID_R_EQ_E | ID_R_EQ_S |
+ ID_E_EQ_R | ID_E_EQ_E | ID_E_EQ_S |
+ ID_S_EQ_R | ID_S_EQ_E | ID_S_EQ_S );
}
int
Index: compat/osf1/osf1_prot.c
===================================================================
RCS file: /cvsroot/src/sys/compat/osf1/osf1_prot.c,v
retrieving revision 1.5
diff -u -p -r1.5 osf1_prot.c
--- compat/osf1/osf1_prot.c 2003/01/18 08:32:04 1.5
+++ compat/osf1/osf1_prot.c 2003/03/03 22:09:59
@@ -86,22 +86,13 @@ osf1_sys_setgid(l, v, retval)
{
struct osf1_sys_setgid_args *uap = v;
struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
gid_t gid = SCARG(uap, gid);
int error;
- if ((error = suser(pc->pc_ucred, &p->p_acflag)) != 0 &&
- gid != pc->p_rgid && gid != pc->p_svgid)
- return (error);
-
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_gid = gid;
- if (error == 0) {
- pc->p_rgid = gid;
- pc->p_svgid = gid;
- }
- p->p_flag |= P_SUGID;
- return (0);
+ error = do_setresgid(l, gid, gid, gid, 0);
+ if (error != 0)
+ error = do_setresgid(l, -1, gid, -1, ID_E_EQ_R | ID_E_EQ_S );
+ return error;
}
/*
@@ -126,22 +117,11 @@ osf1_sys_setuid(l, v, retval)
{
struct osf1_sys_setuid_args *uap = v;
struct proc *p = l->l_proc;
- struct pcred *pc = p->p_cred;
uid_t uid = SCARG(uap, uid);
int error;
-
- if ((error = suser(pc->pc_ucred, &p->p_acflag)) != 0 &&
- uid != pc->p_ruid && uid != pc->p_svuid)
- return (error);
- pc->pc_ucred = crcopy(pc->pc_ucred);
- pc->pc_ucred->cr_uid = uid;
- if (error == 0) {
- (void)chgproccnt(pc->p_ruid, -1);
- (void)chgproccnt(uid, 1);
- pc->p_ruid = uid;
- pc->p_svuid = uid;
- }
- p->p_flag |= P_SUGID;
- return (0);
+ error = do_setresuid(l, uid, uid, uid, 0);
+ if (error != 0)
+ error = do_setresuid(l, -1, uid, -1, ID_E_EQ_R | ID_E_EQ_S );
+ return error;
}
--
David Laight: david@l8s.co.uk