Rework local client id finding code to be more uniform

configure.ac

@@ -179,7 +179,7 @@ dnl Checks for library functions.
 AC_FUNC_VPRINTF
 AC_CHECK_FUNCS([geteuid getuid link memmove memset mkstemp strchr strrchr \
 		strtol getopt getopt_long vsnprintf walkcontext backtrace \
-		getisax])
+		getisax getzoneid])
 AC_FUNC_ALLOCA
 dnl Old HAS_* names used in os/*.c.
 AC_CHECK_FUNC([getdtablesize],

include/dix-config.h.in

@@ -136,6 +136,9 @@
 /* Define to 1 if you have the `getuid' function. */
 #undef HAVE_GETUID
 
+/* Define to 1 if you have the `getzoneid' function. */
+#undef HAVE_GETZONEID
+
 /* Define to 1 if you have the <inttypes.h> header file. */
 #undef HAVE_INTTYPES_H
 

include/os.h

@@ -323,6 +323,24 @@ extern int LocalClient(ClientPtr /* client */);
 
 extern int LocalClientCred(ClientPtr, int *, int *);
 
+#define LCC_UID_SET	(1 << 0)
+#define LCC_GID_SET	(1 << 1)
+#define LCC_PID_SET	(1 << 2)
+#define LCC_ZID_SET	(1 << 3)
+
+typedef struct {
+    int fieldsSet;	/* Bit mask of fields set */
+    int	euid;		/* Effective uid */
+    int egid;		/* Primary effective group id */
+    int nSuppGids;	/* Number of supplementary group ids */
+    int *pSuppGids;	/* Array of supplementary group ids */
+    int pid;		/* Process id */
+    int zoneid;		/* Only set on Solaris 10 & later */
+} LocalClientCredRec;
+
+extern int GetLocalClientCreds(ClientPtr, LocalClientCredRec **);
+extern void FreeLocalClientCreds(LocalClientCredRec *); 
+
 extern int ChangeAccessControl(ClientPtr /*client*/, int /*fEnabled*/);
 
 extern int GetAccessControl(void);

os/access.c

@@ -234,10 +234,6 @@ static Bool NewHost(int /*family*/,
 		    int /*len*/,
 		    int /* addingLocalHosts */);
 
-static int LocalClientCredAndGroups(ClientPtr client, int *pUid, int *pGid, 
-				    int **pSuppGids, int *nSuppGids);
-
-
 /* XFree86 bug #156: To keep track of which hosts were explicitly requested in
    /etc/X<display>.hosts, we've added a requested field to the HOST struct,
    and a LocalHostRequested variable.  These default to FALSE, but are set
@@ -1383,38 +1379,51 @@ _X_EXPORT Bool LocalClient(ClientPtr client)
 
 /*
  * Return the uid and gid of a connected local client
- * or the uid/gid for nobody those ids cannot be determined
  * 
  * Used by XShm to test access rights to shared memory segments
  */
 int
 LocalClientCred(ClientPtr client, int *pUid, int *pGid)
 {
-    return LocalClientCredAndGroups(client, pUid, pGid, NULL, NULL);
+    LocalClientCredRec *lcc;
+    int ret = GetLocalClientCreds(client, &lcc);
+
+    if (ret == 0) {
+#ifdef HAVE_GETZONEID /* only local if in the same zone */
+	if ((lcc->fieldsSet & LCC_ZID_SET) && (lcc->zoneid != getzoneid())) {
+	    FreeLocalClientCreds(lcc);
+	    return -1;
+	}	    
+#endif
+	if ((lcc->fieldsSet & LCC_UID_SET) && (pUid != NULL))
+	    *pUid = lcc->euid;
+	if ((lcc->fieldsSet & LCC_GID_SET) && (pGid != NULL))
+	    *pGid = lcc->egid;
+	FreeLocalClientCreds(lcc);
+    }
+    return ret;
 }
 
 /*
  * Return the uid and all gids of a connected local client
- * or the uid/gid for nobody those ids cannot be determined
+ * Allocates a LocalClientCredRec - caller must call FreeLocalClientCreds
  * 
- * If the caller passes non-NULL values for pSuppGids & nSuppGids,
- * they are responsible for calling XFree(*pSuppGids) to release the
- * memory allocated for the supplemental group ids list.
- *
  * Used by localuser & localgroup ServerInterpreted access control forms below
+ * Used by AuthAudit to log who local connections came from
  */
-static int
+int
-LocalClientCredAndGroups(ClientPtr client, int *pUid, int *pGid, 
+GetLocalClientCreds(ClientPtr client, LocalClientCredRec **lccp)
-			 int **pSuppGids, int *nSuppGids)
 {
 #if defined(HAS_GETPEEREID) || defined(HAS_GETPEERUCRED) || defined(SO_PEERCRED)
     int fd;
     XtransConnInfo ci;
+    LocalClientCredRec *lcc;
 #ifdef HAS_GETPEEREID
     uid_t uid;
     gid_t gid;
 #elif defined(HAS_GETPEERUCRED)
     ucred_t *peercred = NULL;
+    const gid_t *gids;
 #elif defined(SO_PEERCRED)
     struct ucred peercred;
     socklen_t so_len = sizeof(peercred);
@@ -1433,57 +1442,65 @@ LocalClientCredAndGroups(ClientPtr client, int *pUid, int *pGid,
     }
 #endif
 
-    if (pSuppGids != NULL)
+    *lccp = Xcalloc(sizeof(LocalClientCredRec));
-	*pSuppGids = NULL;
+    if (*lccp == NULL)
-    if (nSuppGids != NULL)
+	return -1;
-	*nSuppGids = 0;
+    lcc = *lccp;
-
+        
     fd = _XSERVTransGetConnectionNumber(ci);
 #ifdef HAS_GETPEEREID
-    if (getpeereid(fd, &uid, &gid) == -1) 
+    if (getpeereid(fd, &uid, &gid) == -1) {
-	    return -1;
+	FreeLocalClientCreds(lcc);
-    if (pUid != NULL)
-	    *pUid = uid;
-    if (pGid != NULL)
-	    *pGid = gid;
-    return 0;
-#elif defined(HAS_GETPEERUCRED)
-    if (getpeerucred(fd, &peercred) < 0)
-    	return -1;
-#ifdef sun /* Ensure process is in the same zone */
-    if (getzoneid() != ucred_getzoneid(peercred)) {
-	ucred_free(peercred);
 	return -1;
     }
+    lcc->euid = uid;
+    lcc->egid = gid;
+    lcc->fieldsSet = LCC_UID_SET | LCC_GID_SET;
+    return 0;
+#elif defined(HAS_GETPEERUCRED)
+    if (getpeerucred(fd, &peercred) < 0) {
+	FreeLocalClientCreds(lcc);
+    	return -1;
+    }
+    lcc->euid = ucred_geteuid(peercred);
+    if (lcc->euid != -1)
+	lcc->fieldsSet |= LCC_UID_SET;
+    lcc->egid = ucred_getegid(peercred);
+    if (lcc->egid != -1)
+	lcc->fieldsSet |= LCC_GID_SET;
+    lcc->pid = ucred_getpid(peercred);
+    if (lcc->pid != -1)
+	lcc->fieldsSet |= LCC_PID_SET;
+#ifdef HAVE_GETZONEID
+    lcc->zoneid = ucred_getzoneid(peercred);
+    if (lcc->zoneid != -1)
+	lcc->fieldsSet |= LCC_ZID_SET;
 #endif
-    if (pUid != NULL)
+    lcc->nSuppGids = ucred_getgroups(peercred, &gids);
-	*pUid = ucred_geteuid(peercred);
+    if (lcc->nSuppGids > 0) {
-    if (pGid != NULL)
+	lcc->pSuppGids = Xcalloc((lcc->nSuppGids) * sizeof(int));
-	*pGid = ucred_getegid(peercred);
+	if (lcc->pSuppGids == NULL) {
-    if (pSuppGids != NULL && nSuppGids != NULL) {
+	    lcc->nSuppGids = 0;
-	const gid_t *gids;
+	} else {
-	*nSuppGids = ucred_getgroups(peercred, &gids);
+	    int i;
-	if (*nSuppGids > 0) {
+	    for (i = 0 ; i < lcc->nSuppGids; i++) {
-	    *pSuppGids = xalloc(sizeof(int) * (*nSuppGids));
+		(lcc->pSuppGids)[i] = (int) gids[i];
-	    if (*pSuppGids == NULL) {
-		*nSuppGids = 0;
-	    } else {
-		int i;
-		for (i = 0 ; i < *nSuppGids; i++) {
-		    (*pSuppGids)[i] = (int) gids[i];
-		}
 	    }
 	}
+    } else {
+	lcc->nSuppGids = 0;
     }
     ucred_free(peercred);
     return 0;
 #elif defined(SO_PEERCRED)
-    if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &peercred, &so_len) == -1) 
+    if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &peercred, &so_len) == -1) {
-	    return -1;
+	FreeLocalClientCreds(lcc);
-    if (pUid != NULL)
+	return -1;
-	    *pUid = peercred.uid;
+    }
-    if (pGid != NULL)
+    lcc->euid = peercred.uid;
-	    *pGid = peercred.gid;
+    lcc->egid = peercred.gid;
+    lcc->pid = peercred.pid;
+    lcc->fieldsSet = LCC_UID_SET | LCC_GID_SET | LCC_PID_SET;
     return 0;
 #endif
 #else
@@ -1493,6 +1510,17 @@ LocalClientCredAndGroups(ClientPtr client, int *pUid, int *pGid,
 #endif
 }
 
+void
+FreeLocalClientCreds(LocalClientCredRec *lcc)
+{
+    if (lcc != NULL) {
+	if (lcc->nSuppGids > 0) {
+	    Xfree(lcc->pSuppGids);
+	}
+	Xfree(lcc);
+    }
+}
+
 static Bool
 AuthorizedClient(ClientPtr client)
 {
@@ -2321,38 +2349,48 @@ static Bool
 siLocalCredAddrMatch(int family, pointer addr, int len,
   const char *siAddr, int siAddrlen, ClientPtr client, void *typePriv)
 {
-    int connUid, connGid, *connSuppGids, connNumSuppGids, siAddrId;
+    int siAddrId;
+    LocalClientCredRec *lcc;
     siLocalCredPrivPtr lcPriv = (siLocalCredPrivPtr) typePriv;
 
-    if (LocalClientCredAndGroups(client, &connUid, &connGid,
+    if (GetLocalClientCreds(client, &lcc) == -1) {
-      &connSuppGids, &connNumSuppGids) == -1) {
 	return FALSE;
     }
 
+#ifdef HAVE_GETZONEID /* Ensure process is in the same zone */
+    if ((lcc->fieldsSet & LCC_ZID_SET) && (lcc->zoneid != getzoneid())) {
+	FreeLocalClientCreds(lcc);
+	return FALSE;
+    }
+#endif
+
     if (siLocalCredGetId(siAddr, siAddrlen, lcPriv, &siAddrId) == FALSE) {
+	FreeLocalClientCreds(lcc);
 	return FALSE;
     }
 
     if (lcPriv->credType == LOCAL_USER) {
-	if (connUid == siAddrId) {
+	if ((lcc->fieldsSet & LCC_UID_SET) && (lcc->euid == siAddrId)) {
+	    FreeLocalClientCreds(lcc);
 	    return TRUE;
 	}
     } else {
-	if (connGid == siAddrId) {
+	if ((lcc->fieldsSet & LCC_GID_SET) && (lcc->egid == siAddrId)) {
+	    FreeLocalClientCreds(lcc);
 	    return TRUE;
 	}
-	if (connSuppGids != NULL) {
+	if (lcc->pSuppGids != NULL) {
 	    int i;
 
-	    for (i = 0 ; i < connNumSuppGids; i++) {
+	    for (i = 0 ; i < lcc->nSuppGids; i++) {
-		if (connSuppGids[i] == siAddrId) {
+		if (lcc->pSuppGids[i] == siAddrId) {
-		    xfree(connSuppGids);
+		    FreeLocalClientCreds(lcc);
 		    return TRUE;
 		}
 	    }
-	    xfree(connSuppGids);
 	}
     }
+    FreeLocalClientCreds(lcc);
     return FALSE;
 }
 

os/connection.c

@@ -539,10 +539,8 @@ AuthAudit (ClientPtr client, Bool letin,
     char *out = addr;
     int client_uid;
     char client_uid_string[64];
-#ifdef HAS_GETPEERUCRED
+    LocalClientCredRec *lcc;
-    ucred_t *peercred = NULL;
+#ifdef XSERVER_DTRACE
-#endif
-#if defined(HAS_GETPEERUCRED) || defined(XSERVER_DTRACE)    
     pid_t client_pid = -1;
     zoneid_t client_zid = -1;
 #endif
@@ -583,23 +581,50 @@ AuthAudit (ClientPtr client, Bool letin,
 	    strcpy(out, "unknown address");
 	}
 
-#ifdef HAS_GETPEERUCRED
+    if (GetLocalClientCreds(client, &lcc) != -1) {
-    if (getpeerucred(((OsCommPtr)client->osPrivate)->fd, &peercred) >= 0) {
+	int slen; /* length written to client_uid_string */
-	client_uid = ucred_geteuid(peercred);
-	client_pid = ucred_getpid(peercred);
-	client_zid = ucred_getzoneid(peercred);
 
-	ucred_free(peercred);
+	strcpy(client_uid_string, " ( ");
-	snprintf(client_uid_string, sizeof(client_uid_string),
+	slen = 3;
-		 " (uid %ld, pid %ld, zone %ld)",
+
-		 (long) client_uid, (long) client_pid, (long) client_zid);
+	if (lcc->fieldsSet & LCC_UID_SET) {
-    }
+	    snprintf(client_uid_string + slen,
-#else    
+		     sizeof(client_uid_string) - slen,
-    if (LocalClientCred(client, &client_uid, NULL) != -1) {
+		     "uid=%ld ", (long) lcc->euid);
-	snprintf(client_uid_string, sizeof(client_uid_string),
+	    slen = strlen(client_uid_string);
-		 " (uid %d)", client_uid);
+	}
-    }
+
+	if (lcc->fieldsSet & LCC_GID_SET) {
+	    snprintf(client_uid_string + slen,
+		     sizeof(client_uid_string) - slen,
+		     "gid=%ld ", (long) lcc->egid);
+	    slen = strlen(client_uid_string);
+	}
+
+	if (lcc->fieldsSet & LCC_PID_SET) {
+#ifdef XSERVER_DTRACE	    
+	    client_pid = lcc->pid;
 #endif
+	    snprintf(client_uid_string + slen,
+		     sizeof(client_uid_string) - slen,
+		     "pid=%ld ", (long) lcc->pid);
+	    slen = strlen(client_uid_string);
+	}
+	
+	if (lcc->fieldsSet & LCC_ZID_SET) {
+#ifdef XSERVER_DTRACE
+	    client_zid = lcc->zoneid;
+#endif	    
+	    snprintf(client_uid_string + slen,
+		     sizeof(client_uid_string) - slen,
+		     "zoneid=%ld ", (long) lcc->zoneid);
+	    slen = strlen(client_uid_string);
+	}
+
+	snprintf(client_uid_string + slen, sizeof(client_uid_string) - slen,
+		 ")");
+	FreeLocalClientCreds(lcc);
+    }
     else {
 	client_uid_string[0] = '\0';
     }