From 1fc399a5c14b9c031447360be363c347c5dcb5ab Mon Sep 17 00:00:00 2001 From: Thomas Veerman Date: Fri, 27 Jan 2012 16:06:43 +0000 Subject: [PATCH] Add permission test for bind and socket Also, apply forbidden patch to VFS from AVFS (fixes hanging test56 if it has the permission test). --- servers/vfs/exec.c | 2 +- servers/vfs/link.c | 12 +++--- servers/vfs/open.c | 10 ++--- servers/vfs/path.c | 2 +- servers/vfs/protect.c | 8 ++-- servers/vfs/proto.h | 3 +- servers/vfs/stadir.c | 2 +- servers/vfs/time.c | 2 +- test/Makefile | 4 +- test/test56.c | 94 +++++++++++++++++++++++++++++++++++++++++-- 10 files changed, 113 insertions(+), 26 deletions(-) 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 */ -- 2.44.0