From: Jeffrey Altman <jaltman@your-file-system.com>
Date: Thu, 10 Mar 2016 02:38:10 +0000 (-0600)
Subject: OPENAFS-SA-2016-001 group creation by foreign users
X-Git-Tag: upstream/1.8.0_pre1^2~154
X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=d853866c56a114616ecb68f06a914aaea0e5c7c7;p=packages%2Fo%2Fopenafs.git

OPENAFS-SA-2016-001 group creation by foreign users

CVE-2016-2860:

In AFS 3.3 as part of the addition of the cross-cell support for foreign
user auto-registration a bug was introduced that permits foreign users
to create arbitrary groups as if they were system administrators.  This
permits the groups to be created without any group quota checks, and
using group names that non-administrators would not normally be able to
create, such as groups with the "system:" prefix or groups with no colon
(that is, in the namespace for users).

Additionally, all entries created using the auto-registration service
were marked as being created by system:administrators.  This behavior
should not be changed on the stable release branch, but for the next
release the behavior will change to show these entries as being
self-created, to better reflect reality.

FIXES 132822

[kaduk@mit.edu: reword commit message, minor style adjustments]

Change-Id: I54ddca3e4e1339f76ed320f0d6c53d8820aed89c
---

diff --git a/src/ptserver/ptprocs.c b/src/ptserver/ptprocs.c
index 7d1d6ddb9..039077ac2 100644
--- a/src/ptserver/ptprocs.c
+++ b/src/ptserver/ptprocs.c
@@ -314,6 +314,7 @@ newEntry(struct rx_call *call, char aname[], afs_int32 flag, afs_int32 oid,
     afs_int32 code;
     struct ubik_trans *tt;
     int admin;
+    int foreign = 0;
     char cname[PR_MAXNAMELEN];
     stolower(aname);
 
@@ -326,18 +327,31 @@ newEntry(struct rx_call *call, char aname[], afs_int32 flag, afs_int32 oid,
      * automatic id assignment.
      */
     code = WhoIsThisWithName(call, tt, cid, cname);
-    if (code != 2) {		/* 2 specifies that this is a foreign cell request */
-	if (code)
-	    ABORT_WITH(tt, PRPERM);
-	admin = IsAMemberOf(tt, *cid, SYSADMINID);
-    } else {
-	admin = ((!restricted && !strcmp(aname, cname))) || IsAMemberOf(tt, *cid, SYSADMINID);
-	oid = *cid = SYSADMINID;
+    if (code && code != 2)
+	ABORT_WITH(tt, PRPERM);
+    admin = IsAMemberOf(tt, *cid, SYSADMINID);
+    if (code == 2 /* foreign cell request */) {
+	foreign = 1;
+
+	if (!restricted && (strcmp(aname, cname) == 0)) {
+	    /* can't autoregister while providing an owner id */
+	    if (oid != 0)
+		ABORT_WITH(tt, PRPERM);
+
+	    admin = 1;
+	    oid = SYSADMINID;
+	}
     }
     if (!CreateOK(tt, *cid, oid, flag, admin))
 	ABORT_WITH(tt, PRPERM);
 
     code = CreateEntry(tt, aname, aid, 0, flag, oid, *cid);
+    /*
+     * If this was an autoregistration then be sure to audit log
+     * the proper id as the creator.
+     */
+    if (foreign && code == 0 && *aid > 0)
+	*cid = *aid;
     if (code != PRSUCCESS)
 	ABORT_WITH(tt, code);
 
diff --git a/src/ptserver/ptutils.c b/src/ptserver/ptutils.c
index 32b34c746..511f57af5 100644
--- a/src/ptserver/ptutils.c
+++ b/src/ptserver/ptutils.c
@@ -338,8 +338,11 @@ CreateEntry(struct ubik_trans *at, char aname[PR_MAXNAMELEN], afs_int32 *aid, af
 
     admin = pr_noAuth || IsAMemberOf(at, creator, SYSADMINID);
 
-    if (oid == 0 || oid == ANONYMOUSID)
+    if (oid == 0 || oid == ANONYMOUSID) {
+	if (!admin && creator == 0)
+	    return PRBADARG;
 	oid = creator;
+    }
 
     if (flag & PRGRP) {
 	code = CorrectGroupName(at, aname, creator, oid, admin, tentry.name);
@@ -456,7 +459,10 @@ CreateEntry(struct ubik_trans *at, char aname[PR_MAXNAMELEN], afs_int32 *aid, af
 	    return PRDBFAIL;
 
 	/* Now add the new user entry to the database */
-	tentry.creator = creator;
+	if (creator == 0)
+	    tentry.creator = tentry.id;
+	else
+	    tentry.creator = creator;
 	*aid = tentry.id;
 	code = pr_WriteEntry(at, 0, newEntry, &tentry);
 	if (code)
@@ -558,7 +564,10 @@ CreateEntry(struct ubik_trans *at, char aname[PR_MAXNAMELEN], afs_int32 *aid, af
 	tentry.ngroups = tentry.nusers = 20;
     }
 
-    tentry.creator = creator;
+    if (creator == 0)
+	tentry.creator = tentry.id;
+    else
+	tentry.creator = creator;
     *aid = tentry.id;
     code = pr_WriteEntry(at, 0, newEntry, &tentry);
     if (code)