From: Thomas Veerman <thomas@minix3.org>
Date: Fri, 27 Jan 2012 16:06:43 +0000 (+0000)
Subject: Add permission test for bind and socket
X-Git-Tag: v3.2.0~98
X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/named.html?a=commitdiff_plain;h=1fc399a5c14b9c031447360be363c347c5dcb5ab;p=minix.git

Add permission test for bind and socket

Also, apply forbidden patch to VFS from AVFS (fixes hanging test56 if
it has the permission test).
---

diff --git a/servers/vfs/exec.c b/servers/vfs/exec.c
index 43af7d6a2..e91cb6cee 100644
--- a/servers/vfs/exec.c
+++ b/servers/vfs/exec.c
@@ -126,7 +126,7 @@ PUBLIC int pm_exec(int proc_e, char *path, vir_bytes path_len, char *frame,
 
 	if ((vp->v_mode & I_TYPE) != I_REGULAR) 
 		r = ENOEXEC;
-	else if ((r1 = forbidden(vp, X_BIT)) != OK)
+	else if ((r1 = forbidden(fp, vp, X_BIT)) != OK)
 		r = r1;
 	else
 		r = req_stat(vp->v_fs_e, vp->v_inode_nr, VFS_PROC_NR,
diff --git a/servers/vfs/link.c b/servers/vfs/link.c
index 4000097e8..5a1f81564 100644
--- a/servers/vfs/link.c
+++ b/servers/vfs/link.c
@@ -50,7 +50,7 @@ PUBLIC int do_link()
   if(vp->v_fs_e != vp_d->v_fs_e)
   	r = EXDEV;
   else 
-	r = forbidden(vp_d, W_BIT | X_BIT);
+	r = forbidden(fp, vp_d, W_BIT | X_BIT);
 
   if (r == OK)
 	r = req_link(vp->v_fs_e, vp_d->v_inode_nr, user_fullpath,
@@ -85,7 +85,7 @@ PUBLIC int do_unlink()
   }
 
   /* The caller must have both search and execute permission */
-  if ((r = forbidden(vldirp, X_BIT | W_BIT)) != OK) {
+  if ((r = forbidden(fp, vldirp, X_BIT | W_BIT)) != OK) {
 	put_vnode(vldirp);
 	return(r);
   }
@@ -169,8 +169,8 @@ PUBLIC int do_rename()
   if(old_dirp->v_fs_e != new_dirp->v_fs_e) r = EXDEV; 
 
   /* Parent dirs must be writable, searchable and on a writable device */
-  if ((r1 = forbidden(old_dirp, W_BIT|X_BIT)) != OK ||
-      (r1 = forbidden(new_dirp, W_BIT|X_BIT)) != OK) r = r1;
+  if ((r1 = forbidden(fp, old_dirp, W_BIT|X_BIT)) != OK ||
+      (r1 = forbidden(fp, new_dirp, W_BIT|X_BIT)) != OK) r = r1;
   
   if(r == OK)
 	  r = req_rename(old_dirp->v_fs_e, old_dirp->v_inode_nr, old_name,
@@ -201,7 +201,7 @@ PUBLIC int do_truncate()
   if ((vp = eat_path(PATH_NOFLAGS, fp)) == NULL) return(err_code);
   
   /* Ask FS to truncate the file */
-  if ((r = forbidden(vp, W_BIT)) == OK)
+  if ((r = forbidden(fp, vp, W_BIT)) == OK)
   	r = truncate_vnode(vp, m_in.flength);
   
   put_vnode(vp);
@@ -261,7 +261,7 @@ PUBLIC int do_slink()
   if(fetch_name(m_in.name2, m_in.name2_length, M1) != OK) return(err_code);
   if ((vp = last_dir(fp)) == NULL) return(err_code);
 
-  if ((r = forbidden(vp, W_BIT|X_BIT)) == OK) {
+  if ((r = forbidden(fp, vp, W_BIT|X_BIT)) == OK) {
 	r = req_slink(vp->v_fs_e, vp->v_inode_nr, user_fullpath, who_e,
 		      m_in.name1, m_in.name1_length - 1, fp->fp_effuid,
 		      fp->fp_effgid);
diff --git a/servers/vfs/open.c b/servers/vfs/open.c
index fc6698f26..ae47d048e 100644
--- a/servers/vfs/open.c
+++ b/servers/vfs/open.c
@@ -116,13 +116,13 @@ PUBLIC int common_open(register int oflags, mode_t omode)
   /* Only do the normal open code if we didn't just create the file. */
   if(exist) {
 	/* Check protections. */
-	if ((r = forbidden(vp, bits)) == OK) {
+	if ((r = forbidden(fp, vp, bits)) == OK) {
 		/* Opening reg. files, directories, and special files differ */
 		switch (vp->v_mode & I_TYPE) {
 		   case I_REGULAR:
 		   	/* Truncate regular file if O_TRUNC. */
 		   	if (oflags & O_TRUNC) {
-		   		if ((r = forbidden(vp, W_BIT)) != OK)
+		   		if ((r = forbidden(fp, vp, W_BIT)) != OK)
 		   			break;
 		   		truncate_vnode(vp, 0);
 		   	}
@@ -266,7 +266,7 @@ PRIVATE struct vnode *new_node(int oflags, mode_t bits)
 		put_vnode(dirp);	
 		return(NULL);
 	}
-	if ((r = forbidden(dirp, W_BIT|X_BIT)) != OK ||
+	if ((r = forbidden(fp, dirp, W_BIT|X_BIT)) != OK ||
 	    (r = req_create(dirp->v_fs_e, dirp->v_inode_nr,bits, fp->fp_effuid,
 			    fp->fp_effgid, user_fullpath, &res)) != OK ) {
 		/* Can't create inode either due to permissions or some other
@@ -410,7 +410,7 @@ PUBLIC int do_mknod()
 	  return(ENOTDIR);
   }
 
-  if ((r = forbidden(vp, W_BIT|X_BIT)) == OK) {
+  if ((r = forbidden(fp, vp, W_BIT|X_BIT)) == OK) {
 	r = req_mknod(vp->v_fs_e, vp->v_inode_nr, user_fullpath, fp->fp_effuid,
 		      fp->fp_effgid, bits, m_in.mk_z0);
   }
@@ -443,7 +443,7 @@ PUBLIC int do_mkdir()
 	  return(ENOTDIR);
   }
 
-  if ((r = forbidden(vp, W_BIT|X_BIT)) == OK) {
+  if ((r = forbidden(fp, vp, W_BIT|X_BIT)) == OK) {
 	r = req_mkdir(vp->v_fs_e, vp->v_inode_nr, user_fullpath, fp->fp_effuid,
 		      fp->fp_effgid, bits);
   }
diff --git a/servers/vfs/path.c b/servers/vfs/path.c
index a58fd0b08..834ba6b2b 100644
--- a/servers/vfs/path.c
+++ b/servers/vfs/path.c
@@ -524,7 +524,7 @@ int pathlen;
   }
 
   /* check permissions */
-  r = forbidden(vp, (R_BIT | W_BIT));
+  r = forbidden(rfp, vp, (R_BIT | W_BIT));
 
   put_vnode(vp);
   return(r);
diff --git a/servers/vfs/protect.c b/servers/vfs/protect.c
index 01a0555d4..958cfe8ac 100644
--- a/servers/vfs/protect.c
+++ b/servers/vfs/protect.c
@@ -155,7 +155,7 @@ PUBLIC int do_access()
   if (fetch_name(m_in.name, m_in.name_length, M3) != OK) return(err_code);
   if ((vp = eat_path(PATH_NOFLAGS, fp)) == NULL) return(err_code);
 
-  r = forbidden(vp, m_in.mode);
+  r = forbidden(fp, vp, m_in.mode);
   put_vnode(vp);
   return(r);
 }
@@ -164,7 +164,7 @@ PUBLIC int do_access()
 /*===========================================================================*
  *				forbidden				     *
  *===========================================================================*/
-PUBLIC int forbidden(struct vnode *vp, mode_t access_desired)
+PUBLIC int forbidden(struct fproc *rfp, struct vnode *vp, mode_t access_desired)
 {
 /* Given a pointer to an vnode, 'vp', and the access desired, determine
  * if the access is allowed, and if not why not.  The routine looks up the
@@ -181,8 +181,8 @@ PUBLIC int forbidden(struct vnode *vp, mode_t access_desired)
 
   /* Isolate the relevant rwx bits from the mode. */
   bits = vp->v_mode;
-  uid = (call_nr == ACCESS ? fp->fp_realuid : fp->fp_effuid);
-  gid = (call_nr == ACCESS ? fp->fp_realgid : fp->fp_effgid);
+  uid = (call_nr == ACCESS ? rfp->fp_realuid : rfp->fp_effuid);
+  gid = (call_nr == ACCESS ? rfp->fp_realgid : rfp->fp_effgid);
 
   if (uid == SU_UID) {
 	/* Grant read and write permission.  Grant search permission for
diff --git a/servers/vfs/proto.h b/servers/vfs/proto.h
index 3c9841d20..3c933df04 100644
--- a/servers/vfs/proto.h
+++ b/servers/vfs/proto.h
@@ -166,7 +166,8 @@ _PROTOTYPE( int do_access, (void)					);
 _PROTOTYPE( int do_chmod, (void)					);
 _PROTOTYPE( int do_chown, (void)					);
 _PROTOTYPE( int do_umask, (void)					);
-_PROTOTYPE( int forbidden, (struct vnode *vp, mode_t access_desired)	);
+_PROTOTYPE( int forbidden, (struct fproc *rfp, struct vnode *vp,
+			    mode_t access_desired)			);
 _PROTOTYPE( int read_only, (struct vnode *vp)				);
 
 /* read.c */
diff --git a/servers/vfs/stadir.c b/servers/vfs/stadir.c
index 2517a7a6d..8f8da3059 100644
--- a/servers/vfs/stadir.c
+++ b/servers/vfs/stadir.c
@@ -99,7 +99,7 @@ struct vnode *vp;		/* this is what the inode has to become */
   if ((vp->v_mode & I_TYPE) != I_DIRECTORY)
   	r = ENOTDIR;
   else
-	r = forbidden(vp, X_BIT);	/* Check if dir is searchable*/
+	r = forbidden(fp, vp, X_BIT);	/* Check if dir is searchable*/
 
   /* If error, return vnode */
   if (r != OK) {
diff --git a/servers/vfs/time.c b/servers/vfs/time.c
index 749581cba..491adeb5d 100644
--- a/servers/vfs/time.c
+++ b/servers/vfs/time.c
@@ -37,7 +37,7 @@ PUBLIC int do_utime()
   /* Only the owner of a file or the super user can change its name. */  
   r = OK;
   if (vp->v_uid != fp->fp_effuid && fp->fp_effuid != SU_UID) r = EPERM;
-  if (m_in.utime_length == 0 && r != OK) r = forbidden(vp, W_BIT);
+  if (m_in.utime_length == 0 && r != OK) r = forbidden(fp, vp, W_BIT);
   if (read_only(vp) != OK) r = EROFS; /* Not even su can touch if R/O */ 
   if (r == OK) {
 	/* Issue request */
diff --git a/test/Makefile b/test/Makefile
index e02032497..1ccc2c7bc 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -16,10 +16,10 @@ OBJ=	test1  test2  test3  test4  test5  test6  test7  test8  test9  \
 	test30 test31 test32        test34 test35 test36 test37 test38 \
 	test39 t10a t11a t11b test40 t40a t40b t40c t40d t40e t40f test41 \
 	test42 test45 test47 test49 test50 test51 test52 test53 \
-	test54 test56 test58 t60a t60b
+	test54 test58 t60a t60b
 
 BIGOBJ=  test20 test24
-ROOTOBJ= test11 test33 test43 test44 test46 test60 test61
+ROOTOBJ= test11 test33 test43 test44 test46 test56 test60 test61
 GCCOBJ=  test45-gcc test48 test49-gcc test55
 GCCFPUOBJ= test51-gcc test52-gcc
 OTHEROBJ= test57 test59
diff --git a/test/test56.c b/test/test56.c
index 0d350c5e7..2f678639a 100644
--- a/test/test56.c
+++ b/test/test56.c
@@ -436,8 +436,6 @@ void test_getsockname(void)
 	addr.sun_family = AF_UNIX;
 	strncpy(addr.sun_path, TEST_SUN_PATH, sizeof(addr.sun_path) - 1);
 
-	debug("Test bind() success");
-
 	SOCKET(sd, PF_UNIX, SOCK_STREAM, 0);
 	rc = bind(sd, (struct sockaddr *) &addr, sizeof(struct sockaddr_un));
 	if (rc == -1) {
@@ -2473,6 +2471,94 @@ void test_fd_passing_parent(int sd)
 	}
 }
 
+void test_permissions(void) {
+	/* Test bind and connect for permission verification
+	 *
+	 * After creating a UDS socket we change user credentials. At that
+	 * point we should not be allowed to bind or connect to the UDS socket
+	 */
+
+	pid_t pid;
+	int sd, rc, status;
+	struct sockaddr_un addr, client_addr;
+	char buf[10];
+	socklen_t client_addr_size;
+
+	client_addr_size = sizeof(struct sockaddr_un);
+
+	memset(&addr, '\0', sizeof(struct sockaddr_un));
+	addr.sun_family = AF_UNIX;
+	strncpy(addr.sun_path, TEST_SUN_PATH, sizeof(addr.sun_path) - 1);
+
+	UNLINK(TEST_SUN_PATH);
+
+	pid = fork();
+	if (pid < 0) test_fail("unable to fork");
+	else if (pid == 0) {
+		SOCKET(sd, PF_UNIX, SOCK_STREAM, 0);
+		if (setuid(999) != 0) test_fail("unable to chance uid");
+		rc = bind(sd, (struct sockaddr *) &addr,
+				 sizeof(struct sockaddr_un));
+		if (rc != -1) {
+			test_fail("bind() should not have worked");
+		}
+		exit(errct);
+	} else {
+		rc = waitpid(pid, &status, 0);
+		errct += WEXITSTATUS(status);
+	}
+
+	/* the signal handler is only used by the client, but we have to
+	 * install it now. if we don't the server may signal the client
+	 * before the handler is installed.
+	 */
+	debug("installing signal handler");
+	if (signal(SIGUSR1, test_xfer_sighdlr) == SIG_ERR) {
+		test_fail("signal(SIGUSR1, test_xfer_sighdlr) failed");
+	}
+
+	debug("signal handler installed");
+
+	server_ready = 0;
+
+	pid = fork();
+	if (pid < 0) test_fail("unable to fork");
+	else if (pid == 0) {
+		while (server_ready == 0) {
+			debug("[client] waiting for the server to signal");
+			sleep(1);
+		}
+		SOCKET(sd, PF_UNIX, SOCK_STREAM, 0);
+		if (setuid(999) != 0) test_fail("unable to chance uid");
+		rc = connect(sd, (struct sockaddr *) &addr,
+						sizeof(struct sockaddr_un));
+		if (rc != -1)
+			test_fail("connect should not have worked");
+		exit(errct);
+	} else {
+		int client_sd;
+		SOCKET(sd, PF_UNIX, SOCK_STREAM, 0);
+		rc = bind(sd, (struct sockaddr *) &addr,
+				 sizeof(struct sockaddr_un));
+		if (rc == -1) {
+			test_fail("bind() should have worked");
+		}
+
+		rc = listen(sd, 8);
+		if (rc == -1) {
+			test_fail("listen(sd, 8) should have worked");
+		}
+		kill(pid, SIGUSR1);
+		sleep(1);
+		CLOSE(sd);
+
+		rc = waitpid(pid, &status, 0);
+		errct += WEXITSTATUS(status);
+	}
+
+	UNLINK(TEST_SUN_PATH);
+}
+
 void test_fd_passing(void) {
 	int status;
 	int sv[2];
@@ -2536,11 +2622,13 @@ int main(int argc, char *argv[])
 	start(56);
 
 	test_socket();
+	test_bind();
 	test_listen();
 	test_getsockname();
 	test_header();
 	test_shutdown();
 	test_close();
+	test_permissions();
 	test_dup();
 	test_dup2();
 	test_socketpair();
@@ -2549,7 +2637,6 @@ int main(int argc, char *argv[])
 	test_write();
 	test_sockopts();
 	test_ucred();
-	test_bind();
 	test_xfer();
 
 	for (i = 0; i < 3; i++) {
@@ -2564,7 +2651,6 @@ int main(int argc, char *argv[])
 	test_multiproc_write();
 	test_scm_credentials();
 	test_fd_passing();
-
 	quit();
 
 	return -1;	/* we should never get here */