From: reyk Date: Fri, 13 Jul 2018 10:26:57 +0000 (+0000) Subject: Check the disk/kernel/cdrom file permissions after openening the fd. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=a882c86a5b5b202cf1ed6c410d93fd52b778cf4e;p=openbsd Check the disk/kernel/cdrom file permissions after openening the fd. This prevents time of TOCTOU attacks for instances. OK mlarkin@ --- diff --git a/usr.sbin/vmd/config.c b/usr.sbin/vmd/config.c index fc8114a9e5e..5f6db1afba8 100644 --- a/usr.sbin/vmd/config.c +++ b/usr.sbin/vmd/config.c @@ -1,4 +1,4 @@ -/* $OpenBSD: config.c,v 1.46 2018/07/11 13:19:47 reyk Exp $ */ +/* $OpenBSD: config.c,v 1.47 2018/07/13 10:26:57 reyk Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -173,7 +173,6 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) struct vmd_if *vif; struct vmop_create_params *vmc = &vm->vm_params; struct vm_create_params *vcp = &vmc->vmc_params; - struct stat stat_buf; unsigned int i; int fd = -1, vmboot = 0; int kernfd = -1, *diskfds = NULL, *tapfds = NULL; @@ -241,6 +240,15 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) errno = VMD_BIOS_MISSING; goto fail; } + + if (vm_checkaccess(kernfd, + vmc->vmc_checkaccess & VMOP_CREATE_KERNEL, + uid, R_OK) == -1) { + log_warnx("vm \"%s\" no read access to %s", + vcp->vcp_name, vcp->vcp_kernel); + errno = EPERM; + goto fail; + } } /* Open CDROM image for child */ @@ -253,16 +261,13 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) errno = VMD_CDROM_MISSING; goto fail; } - if (fstat(cdromfd, &stat_buf) == -1) { - log_warn("%s: can't open cdrom %s", __func__, - vcp->vcp_cdrom); - errno = VMD_CDROM_MISSING; - goto fail; - } - if (S_ISREG(stat_buf.st_mode) == 0) { - log_warnx("%s: cdrom %s is not a regular file", - __func__, vcp->vcp_cdrom); - errno = VMD_CDROM_INVALID; + + if (vm_checkaccess(cdromfd, + vmc->vmc_checkaccess & VMOP_CREATE_CDROM, + uid, R_OK) == -1) { + log_warnx("vm \"%s\" no read access to %s", + vcp->vcp_name, vcp->vcp_cdrom); + errno = EPERM; goto fail; } } @@ -277,16 +282,13 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) errno = VMD_DISK_MISSING; goto fail; } - if (fstat(diskfds[i], &stat_buf) == -1) { - log_warn("%s: can't open disk %s", __func__, - vcp->vcp_disks[i]); - errno = VMD_DISK_INVALID; - goto fail; - } - if (S_ISREG(stat_buf.st_mode) == 0) { - log_warnx("%s: disk %s is not a regular file", __func__, - vcp->vcp_disks[i]); - errno = VMD_DISK_INVALID; + + if (vm_checkaccess(diskfds[i], + vmc->vmc_checkaccess & VMOP_CREATE_DISK, + uid, R_OK|W_OK) == -1) { + log_warnx("vm \"%s\" no read access to %s", + vcp->vcp_name, vcp->vcp_kernel); + errno = EPERM; goto fail; } } diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c index 9a148e288e6..da184629dc5 100644 --- a/usr.sbin/vmd/vmd.c +++ b/usr.sbin/vmd/vmd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vmd.c,v 1.96 2018/07/13 08:42:49 reyk Exp $ */ +/* $OpenBSD: vmd.c,v 1.97 2018/07/13 10:26:57 reyk Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -1360,12 +1360,7 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, return (-1); } } - if (vm_checkaccess(vcp->vcp_disks[i], uid, R_OK|W_OK) == -1) { - log_warnx("vm \"%s\" no read/write access to %s", name, - vcp->vcp_disks[i]); - errno = EPERM; - return (-1); - } + vmc->vmc_checkaccess |= VMOP_CREATE_DISK; } /* interfaces */ @@ -1424,12 +1419,7 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, errno = EPERM; return (-1); } - if (vm_checkaccess(vcp->vcp_kernel, uid, R_OK) == -1) { - log_warnx("vm \"%s\" no read access to %s", name, - vcp->vcp_kernel); - errno = EPERM; - return (-1); - } + vmc->vmc_checkaccess |= VMOP_CREATE_KERNEL; } else if (strlcpy(vcp->vcp_kernel, vcpp->vcp_kernel, sizeof(vcp->vcp_kernel)) >= sizeof(vcp->vcp_kernel)) { log_warnx("vm \"%s\" kernel name too long", name); @@ -1444,12 +1434,7 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, errno = EPERM; return (-1); } - if (vm_checkaccess(vcp->vcp_cdrom, uid, R_OK) == -1) { - log_warnx("vm \"%s\" no read access to %s", name, - vcp->vcp_cdrom); - errno = EPERM; - return (-1); - } + vmc->vmc_checkaccess |= VMOP_CREATE_CDROM; } else if (strlcpy(vcp->vcp_cdrom, vcpp->vcp_cdrom, sizeof(vcp->vcp_cdrom)) >= sizeof(vcp->vcp_cdrom)) { log_warnx("vm \"%s\" cdrom name too long", name); @@ -1592,15 +1577,17 @@ vm_checkinsflag(struct vmop_create_params *vmc, unsigned int flag, uid_t uid) * access the file described by the 'path' parameter. * * Parameters: - * path: the path of a file + * fd: the file descriptor of the opened file + * uflag: check if the userid has access to the file * uid: the user ID of the user making the request + * amode: the access flags of R_OK and W_OK * * Return values: * 0: the permission should be granted * -1: the permission check failed */ int -vm_checkaccess(const char *path, uid_t uid, int amode) +vm_checkaccess(int fd, unsigned int uflag, uid_t uid, int amode) { struct group *gr; struct passwd *pw; @@ -1608,35 +1595,34 @@ vm_checkaccess(const char *path, uid_t uid, int amode) struct stat st; mode_t mode; - /* root has no restrictions */ - if (uid == 0) - return (0); - - if (path == NULL || strlen(path) == 0) + if (fd == -1) return (-1); /* * File has to be accessible and a regular file - * (symlinks would allow TOCTTOU-like attacks). */ - if (stat(path, &st) == -1 && !S_ISREG(st.st_mode)) + if (fstat(fd, &st) == -1 || !S_ISREG(st.st_mode)) return (-1); + /* root has no restrictions */ + if (uid == 0 || uflag == 0) + return (0); + /* check other */ - mode = amode == W_OK ? S_IWOTH : 0; - mode |= amode == R_OK ? S_IROTH : 0; + mode = amode & W_OK ? S_IWOTH : 0; + mode |= amode & R_OK ? S_IROTH : 0; if ((st.st_mode & mode) == mode) return (0); /* check user */ - mode = amode == W_OK ? S_IWUSR : 0; - mode |= amode == R_OK ? S_IRUSR : 0; + mode = amode & W_OK ? S_IWUSR : 0; + mode |= amode & R_OK ? S_IRUSR : 0; if (uid == st.st_uid && (st.st_mode & mode) == mode) return (0); /* check groups */ - mode = amode == W_OK ? S_IWGRP : 0; - mode |= amode == R_OK ? S_IRGRP : 0; + mode = amode & W_OK ? S_IWGRP : 0; + mode |= amode & R_OK ? S_IRGRP : 0; if ((st.st_mode & mode) != mode) return (-1); if ((pw = getpwuid(uid)) == NULL) diff --git a/usr.sbin/vmd/vmd.h b/usr.sbin/vmd/vmd.h index 7da53185887..bf670f5eb95 100644 --- a/usr.sbin/vmd/vmd.h +++ b/usr.sbin/vmd/vmd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: vmd.h,v 1.76 2018/07/13 08:42:49 reyk Exp $ */ +/* $OpenBSD: vmd.h,v 1.77 2018/07/13 10:26:57 reyk Exp $ */ /* * Copyright (c) 2015 Mike Larkin @@ -149,6 +149,9 @@ struct vmop_create_params { #define VMOP_CREATE_CDROM 0x20 #define VMOP_CREATE_INSTANCE 0x40 + /* same flags; check for access to these resources */ + unsigned int vmc_checkaccess; + /* userland-only part of the create params */ unsigned int vmc_ifflags[VMM_MAX_NICS_PER_VM]; #define VMIFF_UP 0x01 @@ -321,7 +324,7 @@ void vm_remove(struct vmd_vm *, const char *); int vm_register(struct privsep *, struct vmop_create_params *, struct vmd_vm **, uint32_t, uid_t); int vm_checkperm(struct vmd_vm *, struct vmop_owner *, uid_t); -int vm_checkaccess(const char *, uid_t, int); +int vm_checkaccess(int, unsigned int, uid_t, int); int vm_opentty(struct vmd_vm *); void vm_closetty(struct vmd_vm *); void switch_remove(struct vmd_switch *);