From 476d73d100cee30df751178c194252b2c9cc1cfd Mon Sep 17 00:00:00 2001 From: reyk Date: Fri, 13 Jul 2018 08:42:49 +0000 Subject: [PATCH] Add "allow instance" option. This allows users to create VM instances and change desired options, for example a user can be allowed to run a VM with all the pre-configured options but specify an own disk image. (mlarkin@ was fine with iterating over it) OK ccardenas@ --- usr.sbin/vmd/control.c | 6 +- usr.sbin/vmd/parse.y | 42 +++++-- usr.sbin/vmd/vm.conf.5 | 79 ++++++++++-- usr.sbin/vmd/vmd.c | 277 ++++++++++++++++++++++++++++++++++------- usr.sbin/vmd/vmd.h | 30 +++-- usr.sbin/vmd/vmm.c | 4 +- 6 files changed, 358 insertions(+), 80 deletions(-) diff --git a/usr.sbin/vmd/control.c b/usr.sbin/vmd/control.c index 1162c0c207d..9d2ec09e20e 100644 --- a/usr.sbin/vmd/control.c +++ b/usr.sbin/vmd/control.c @@ -1,4 +1,4 @@ -/* $OpenBSD: control.c,v 1.27 2018/07/11 13:19:47 reyk Exp $ */ +/* $OpenBSD: control.c,v 1.28 2018/07/13 08:42:49 reyk Exp $ */ /* * Copyright (c) 2010-2015 Reyk Floeter @@ -400,8 +400,8 @@ control_dispatch_imsg(int fd, short event, void *arg) if (IMSG_DATA_SIZE(&imsg) < sizeof(vmc)) goto fail; memcpy(&vmc, imsg.data, sizeof(vmc)); - vmc.vmc_uid = c->peercred.uid; - vmc.vmc_gid = -1; + vmc.vmc_owner.uid = c->peercred.uid; + vmc.vmc_owner.gid = -1; if (proc_compose_imsg(ps, PROC_PARENT, -1, imsg.hdr.type, fd, -1, &vmc, sizeof(vmc)) == -1) { diff --git a/usr.sbin/vmd/parse.y b/usr.sbin/vmd/parse.y index a17079c4358..2b159d9b866 100644 --- a/usr.sbin/vmd/parse.y +++ b/usr.sbin/vmd/parse.y @@ -1,4 +1,4 @@ -/* $OpenBSD: parse.y,v 1.41 2018/07/12 12:04:49 reyk Exp $ */ +/* $OpenBSD: parse.y,v 1.42 2018/07/13 08:42:49 reyk Exp $ */ /* * Copyright (c) 2007-2016 Reyk Floeter @@ -117,9 +117,9 @@ typedef struct { %token INCLUDE ERROR -%token ADD BOOT CDROM DISABLE DISK DOWN ENABLE GROUP INSTANCE INTERFACE LLADDR -%token LOCAL LOCKED MEMORY NIFS OWNER PATH PREFIX RDOMAIN SIZE SOCKET SWITCH -%token UP VM VMID +%token ADD ALLOW BOOT CDROM DISABLE DISK DOWN ENABLE GROUP INSTANCE INTERFACE +%token LLADDR LOCAL LOCKED MEMORY NIFS OWNER PATH PREFIX RDOMAIN SIZE SOCKET +%token SWITCH UP VM VMID %token NUMBER %token STRING %type lladdr @@ -319,8 +319,8 @@ vm : VM string vm_instance { } /* set default user/group permissions */ - vmc.vmc_uid = 0; - vmc.vmc_gid = -1; + vmc.vmc_owner.uid = 0; + vmc.vmc_owner.gid = -1; } '{' optnl vm_opts_l '}' { struct vmd_vm *vm; int ret; @@ -486,8 +486,29 @@ vm_opts : disable { vmc.vmc_flags |= VMOP_CREATE_MEMORY; } | OWNER owner_id { - vmc.vmc_uid = $2.uid; - vmc.vmc_gid = $2.gid; + vmc.vmc_owner.uid = $2.uid; + vmc.vmc_owner.gid = $2.gid; + } + | instance + ; + +instance : ALLOW INSTANCE '{' optnl instance_l '}' + | ALLOW INSTANCE instance_flags + ; + +instance_l : instance_flags optcommanl instance_l + | instance_flags optnl + ; + +instance_flags : BOOT { vmc.vmc_insflags |= VMOP_CREATE_KERNEL; } + | MEMORY { vmc.vmc_insflags |= VMOP_CREATE_MEMORY; } + | INTERFACE { vmc.vmc_insflags |= VMOP_CREATE_NETWORK; } + | DISK { vmc.vmc_insflags |= VMOP_CREATE_DISK; } + | CDROM { vmc.vmc_insflags |= VMOP_CREATE_CDROM; } + | INSTANCE { vmc.vmc_insflags |= VMOP_CREATE_INSTANCE; } + | OWNER owner_id { + vmc.vmc_insowner.uid = $2.uid; + vmc.vmc_insowner.gid = $2.gid; } ; @@ -650,6 +671,10 @@ optnl : '\n' optnl | ; +optcommanl : ',' optnl + | nl + ; + nl : '\n' optnl ; @@ -688,6 +713,7 @@ lookup(char *s) /* this has to be sorted always */ static const struct keywords keywords[] = { { "add", ADD }, + { "allow", ALLOW }, { "boot", BOOT }, { "cdrom", CDROM }, { "disable", DISABLE }, diff --git a/usr.sbin/vmd/vm.conf.5 b/usr.sbin/vmd/vm.conf.5 index b16bda1a3df..093311c09b4 100644 --- a/usr.sbin/vmd/vm.conf.5 +++ b/usr.sbin/vmd/vm.conf.5 @@ -1,4 +1,4 @@ -.\" $OpenBSD: vm.conf.5,v 1.32 2018/07/12 12:04:49 reyk Exp $ +.\" $OpenBSD: vm.conf.5,v 1.33 2018/07/13 08:42:49 reyk Exp $ .\" .\" Copyright (c) 2015 Mike Larkin .\" Copyright (c) 2015 Reyk Floeter @@ -15,7 +15,7 @@ .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. .\" -.Dd $Mdocdate: July 12 2018 $ +.Dd $Mdocdate: July 13 2018 $ .Dt VM.CONF 5 .Os .Sh NAME @@ -121,20 +121,15 @@ section starts with a declaration of the virtual machine The name can be any alphanumeric string along with '.', '-', and '_' characters. However, it cannot start with '.', '-', or '_'. Typically the name is a hostname. -.It Ic vm Ar parent Ic instance Ar name Brq ... -A virtual machine can be created as an instance of any other -configured VM. -The new instance will inherit settings from the VM -.Ar parent , -except for exclusive options such as -.Ic disk , -.Ic interface lladdr , -or -.Ic interface name . .El .Pp Followed by a block of parameters that is enclosed in curly brackets: .Bl -tag -width Ds +.It Cm allow instance Oc Op Brq ... +Set the permissions to create VM instances. +See +.Sx VM INSTANCES +for the supported options. .It Cm boot Ar path Kernel or BIOS image to load when booting the VM. If not specified, the default is to boot using the BIOS image in @@ -231,6 +226,66 @@ and open the VM's console. .It Cm owner Pf : Ar group Set the owner to the specified group. .El +.Sh VM INSTANCES +It is possible to use configured or running VMs as a template for +additional instances of the VM. +An instance is just like a normal +.Ic vm +and is configured with the following declaration of the virtual machine: +.Ar name : +.Bl -tag -width Ds +.It Ic vm Ar parent Ic instance Ar name Brq ... +A virtual machine can be created as an instance of any other configured VM. +.El +.Pp +The new instance will inherit settings from the VM +.Ar parent , +except for exclusive options such as +.Ic disk , +.Ic interface lladdr , +or +.Ic interface name . +The configuration options are identical to the +.Sx VM CONFIGURATION , +but restricted to the allowed instance options. +.Pp +The allowed instance options are configured in the +.Ar parent +VM: +.Bl -tag -width Ds +.It Cm allow instance Oc Op Brq ... +Allow users to use this VM as a template for VM instances. +By default, the root user can always create instances without +restrictions and users or non-root owners cannot create instances. +An instance will inherit the configuration from the VM and the user, +if permitted, will be allowed to configure individual VM options. +.Pp +Valid options are: +.Bl -tag -width Ds +.It Cm boot +Allow to configure the kernel or BIOS image. +The user needs read access to the image. +.It Cm cdrom +Allow to configure the ISO file. +The user needs read access to the file. +.It Cm disk +Allow to configure the disk images. +The user needs read and write access to image and instances are not +allowed to reuse disks from the parent VM. +.It Cm instance +Allow to create additional instances from the instances. +.It Cm memory +Allow to configure the memory size. +.It Cm network +Allow to change network interface settings. +.It Cm owner Ar user Ns Op : Ns Ar group +Allow the specified user or group to create the instances. +The owner will be allowed to create instances VMs, start or stop the +instances, pause or unpause the instances, and open the instances' +consoles. +.It Cm owner Pf : Ar group +Set the owner to the specified group. +.El .Sh SWITCH CONFIGURATION A virtual switch allows VMs to communicate with other network interfaces on the host system via either diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c index 7fcef0d4a1e..9a148e288e6 100644 --- a/usr.sbin/vmd/vmd.c +++ b/usr.sbin/vmd/vmd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vmd.c,v 1.95 2018/07/12 12:04:49 reyk Exp $ */ +/* $OpenBSD: vmd.c,v 1.96 2018/07/13 08:42:49 reyk Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -60,6 +60,7 @@ int vmd_check_vmh(struct vm_dump_header *); int vm_instance(struct privsep *, struct vmd_vm **, struct vmop_create_params *, uid_t); +int vm_checkinsflag(struct vmop_create_params *, unsigned int, uid_t); struct vmd *env; @@ -94,7 +95,7 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg) case IMSG_VMDOP_START_VM_REQUEST: IMSG_SIZE_CHECK(imsg, &vmc); memcpy(&vmc, imsg->data, sizeof(vmc)); - ret = vm_register(ps, &vmc, &vm, 0, vmc.vmc_uid); + ret = vm_register(ps, &vmc, &vm, 0, vmc.vmc_owner.uid); if (vmc.vmc_flags == 0) { /* start an existing VM with pre-configured options */ if (!(ret == -1 && errno == EALREADY && @@ -108,7 +109,7 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg) } if (res == 0 && config_setvm(ps, vm, - imsg->hdr.peerid, vm->vm_params.vmc_uid) == -1) { + imsg->hdr.peerid, vm->vm_params.vmc_owner.uid) == -1) { res = errno; cmd = IMSG_VMDOP_START_VM_RESPONSE; } @@ -140,7 +141,8 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg) cmd = IMSG_VMDOP_TERMINATE_VM_RESPONSE; break; } - if (vm_checkperm(vm, vid.vid_uid) != 0) { + if (vm_checkperm(vm, &vm->vm_params.vmc_owner, + vid.vid_uid) != 0) { res = EPERM; cmd = IMSG_VMDOP_TERMINATE_VM_RESPONSE; break; @@ -201,7 +203,8 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg) cmd = IMSG_VMDOP_PAUSE_VM_RESPONSE; break; } - if (vm_checkperm(vm, vid.vid_uid) != 0) { + if (vm_checkperm(vm, &vm->vm_params.vmc_owner, + vid.vid_uid) != 0) { res = EPERM; cmd = IMSG_VMDOP_PAUSE_VM_RESPONSE; break; @@ -270,14 +273,15 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg) sizeof(vmc.vmc_params.vcp_name)); vmc.vmc_params.vcp_id = 0; - ret = vm_register(ps, &vmc, &vm, 0, vmc.vmc_uid); + ret = vm_register(ps, &vmc, &vm, 0, vmc.vmc_owner.uid); if (ret != 0) { res = errno; cmd = IMSG_VMDOP_START_VM_RESPONSE; close(imsg->fd); } else { vm->vm_received = 1; - config_setvm(ps, vm, imsg->hdr.peerid, vmc.vmc_uid); + config_setvm(ps, vm, imsg->hdr.peerid, + vmc.vmc_owner.uid); log_debug("%s: sending fd to vmm", __func__); proc_compose_imsg(ps, PROC_VMM, -1, IMSG_VMDOP_RECEIVE_VM_END, vm->vm_vmid, imsg->fd, @@ -466,7 +470,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struct imsg *imsg) } /* get the user id who started the vm */ vir.vir_uid = vm->vm_uid; - vir.vir_gid = vm->vm_params.vmc_gid; + vir.vir_gid = vm->vm_params.vmc_owner.gid; } if (proc_compose_imsg(ps, PROC_CONTROL, -1, imsg->hdr.type, imsg->hdr.peerid, -1, &vir, sizeof(vir)) == -1) { @@ -495,8 +499,8 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struct imsg *imsg) vir.vir_info.vir_ncpus = vm->vm_params.vmc_params.vcp_ncpus; /* get the configured user id for this vm */ - vir.vir_uid = vm->vm_params.vmc_uid; - vir.vir_gid = vm->vm_params.vmc_gid; + vir.vir_uid = vm->vm_params.vmc_owner.uid; + vir.vir_gid = vm->vm_params.vmc_owner.gid; if (proc_compose_imsg(ps, PROC_CONTROL, -1, IMSG_VMDOP_GET_INFO_VM_DATA, imsg->hdr.peerid, -1, &vir, @@ -883,7 +887,7 @@ vmd_configure(void) continue; } if (config_setvm(&env->vmd_ps, vm, - -1, vm->vm_params.vmc_uid) == -1) + -1, vm->vm_params.vmc_owner.uid) == -1) return (-1); } @@ -961,7 +965,7 @@ vmd_reload(unsigned int reset, const char *filename) continue; } if (config_setvm(&env->vmd_ps, vm, - -1, vm->vm_params.vmc_uid) == -1) + -1, vm->vm_params.vmc_owner.uid) == -1) return (-1); } else { log_debug("%s: not creating vm \"%s\": " @@ -1146,6 +1150,7 @@ vm_register(struct privsep *ps, struct vmop_create_params *vmc, { struct vmd_vm *vm = NULL, *vm_parent = NULL; struct vm_create_params *vcp = &vmc->vmc_params; + struct vmop_owner *vmo = NULL; uint32_t rng; unsigned int i; struct vmd_switch *sw; @@ -1160,7 +1165,8 @@ vm_register(struct privsep *ps, struct vmop_create_params *vmc, if ((vm = vm_getbyname(vcp->vcp_name)) != NULL || (vm = vm_getbyvmid(vcp->vcp_id)) != NULL) { - if (vm_checkperm(vm, uid) != 0) { + if (vm_checkperm(vm, &vm->vm_params.vmc_owner, + uid) != 0) { errno = EPERM; goto fail; } @@ -1169,8 +1175,12 @@ vm_register(struct privsep *ps, struct vmop_create_params *vmc, goto fail; } + if (vm_parent != NULL) + vmo = &vm_parent->vm_params.vmc_insowner; + /* non-root users can only start existing VMs or instances */ - if (vm_checkperm(vm_parent, uid) != 0) { + if (vm_checkperm(NULL, vmo, uid) != 0) { + log_warnx("permission denied"); errno = EPERM; goto fail; } @@ -1293,13 +1303,8 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, vmcp = &(*vm_parent)->vm_params; vcpp = &vmcp->vmc_params; - /* - * Are we allowed to create an instance from this VM? - * - * XXX This is currently allowed for root but will check *vm_parent - * XXX once we defined instance permissions and quota. - */ - if (vm_checkperm(NULL, uid) != 0) { + /* Are we allowed to create an instance from this VM? */ + if (vm_checkperm(NULL, &vmcp->vmc_insowner, uid) != 0) { log_warnx("vm \"%s\" no permission to create vm instance", vcpp->vcp_name); errno = ENAMETOOLONG; @@ -1315,14 +1320,35 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, return (-1); } - /* machine options */ + /* CPU */ if (vcp->vcp_ncpus == 0) vcp->vcp_ncpus = vcpp->vcp_ncpus; + if (vm_checkinsflag(vmcp, VMOP_CREATE_CPU, uid) != 0 && + vcp->vcp_ncpus != vcpp->vcp_ncpus) { + log_warnx("vm \"%s\" no permission to set cpus", name); + errno = EPERM; + return (-1); + } + + /* memory */ if (vcp->vcp_memranges[0].vmr_size == 0) vcp->vcp_memranges[0].vmr_size = vcpp->vcp_memranges[0].vmr_size; + if (vm_checkinsflag(vmcp, VMOP_CREATE_MEMORY, uid) != 0 && + vcp->vcp_memranges[0].vmr_size != + vcpp->vcp_memranges[0].vmr_size) { + log_warnx("vm \"%s\" no permission to set memory", name); + errno = EPERM; + return (-1); + } /* disks cannot be inherited */ + if (vm_checkinsflag(vmcp, VMOP_CREATE_DISK, uid) != 0 && + vcp->vcp_ndisks) { + log_warnx("vm \"%s\" no permission to set disks", name); + errno = EPERM; + return (-1); + } for (i = 0; i < vcp->vcp_ndisks; i++) { /* Check if this disk is already used in the parent */ for (j = 0; j < vcpp->vcp_ndisks; j++) { @@ -1334,9 +1360,22 @@ 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); + } } /* interfaces */ + if (vcp->vcp_nnics > 0 && + vm_checkinsflag(vmcp, VMOP_CREATE_NETWORK, uid) != 0 && + vcp->vcp_nnics != vcpp->vcp_nnics) { + log_warnx("vm \"%s\" no permission to set interfaces", name); + errno = EPERM; + return (-1); + } for (i = 0; i < vcpp->vcp_nnics; i++) { /* Interface got overwritten */ if (i < vcp->vcp_nnics) @@ -1378,8 +1417,20 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, } /* kernel */ - if (strlen(vcp->vcp_kernel) == 0 && - strlcpy(vcp->vcp_kernel, vcpp->vcp_kernel, + if (strlen(vcp->vcp_kernel) > 0) { + if (vm_checkinsflag(vmcp, VMOP_CREATE_KERNEL, uid) != 0) { + log_warnx("vm \"%s\" no permission to set boot image", + name); + 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); + } + } 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); errno = EINVAL; @@ -1387,8 +1438,19 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, } /* cdrom */ - if (strlen(vcp->vcp_cdrom) == 0 && - strlcpy(vcp->vcp_cdrom, vcpp->vcp_cdrom, + if (strlen(vcp->vcp_cdrom) > 0) { + if (vm_checkinsflag(vmcp, VMOP_CREATE_CDROM, uid) != 0) { + log_warnx("vm \"%s\" no permission to set cdrom", name); + 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); + } + } 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); errno = EINVAL; @@ -1396,23 +1458,40 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, } /* user */ - if (vmc->vmc_uid == 0) - vmc->vmc_uid = vmcp->vmc_uid; - else if (vmc->vmc_uid != vmcp->vmc_uid) { + if (vmc->vmc_owner.uid == 0) + vmc->vmc_owner.uid = vmcp->vmc_owner.uid; + else if (vmc->vmc_owner.uid != uid && + vmc->vmc_owner.uid != vmcp->vmc_owner.uid) { log_warnx("vm \"%s\" user mismatch", name); errno = EPERM; return (-1); } /* group */ - if (vmc->vmc_gid == 0) - vmc->vmc_gid = vmcp->vmc_gid; - else if (vmc->vmc_gid != vmcp->vmc_gid) { + if (vmc->vmc_owner.gid == 0) + vmc->vmc_owner.gid = vmcp->vmc_owner.gid; + else if (vmc->vmc_owner.gid != vmcp->vmc_owner.gid) { log_warnx("vm \"%s\" group mismatch", name); errno = EPERM; return (-1); } + /* child instances */ + if (vmc->vmc_insflags) { + log_warnx("vm \"%s\" cannot change instance permissions", name); + errno = EPERM; + return (-1); + } + if (vmcp->vmc_insflags & VMOP_CREATE_INSTANCE) { + vmc->vmc_insowner.gid = vmcp->vmc_insowner.gid; + vmc->vmc_insowner.uid = vmcp->vmc_insowner.gid; + vmc->vmc_insflags = vmcp->vmc_insflags; + } else { + vmc->vmc_insowner.gid = 0; + vmc->vmc_insowner.uid = 0; + vmc->vmc_insflags = 0; + } + /* finished, remove instance flags */ vmc->vmc_flags &= ~VMOP_CREATE_INSTANCE; @@ -1428,6 +1507,7 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, * * Parameters: * vm: the VM whose permission is to be checked + * vmo: the required uid/gid to be checked * uid: the user ID of the user making the request * * Return values: @@ -1435,7 +1515,7 @@ vm_instance(struct privsep *ps, struct vmd_vm **vm_parent, * -1: the permission check failed (also returned if vm == null) */ int -vm_checkperm(struct vmd_vm *vm, uid_t uid) +vm_checkperm(struct vmd_vm *vm, struct vmop_owner *vmo, uid_t uid) { struct group *gr; struct passwd *pw; @@ -1445,23 +1525,130 @@ vm_checkperm(struct vmd_vm *vm, uid_t uid) if (uid == 0) return (0); - if (vm == NULL) + if (vmo == NULL) return (-1); - /* check supplementary groups */ - if (vm->vm_params.vmc_gid != -1 && - (pw = getpwuid(uid)) != NULL && - (gr = getgrgid(vm->vm_params.vmc_gid)) != NULL) { - for (grmem = gr->gr_mem; *grmem; grmem++) - if (strcmp(*grmem, pw->pw_name) == 0) - return (0); + /* check user */ + if (vm == NULL) { + if (vmo->uid == uid) + return (0); + } else { + /* + * check user of running vm (the owner of a running vm can + * be different to (or more specific than) the configured owner. + */ + if ((vm->vm_running && vm->vm_uid == uid) || + (!vm->vm_running && vmo->uid == uid)) + return (0); + } + + /* check groups */ + if (vmo->gid != -1) { + if ((pw = getpwuid(uid)) == NULL) + return (-1); + if (pw->pw_gid == vmo->gid) + return (0); + if ((gr = getgrgid(vmo->gid)) != NULL) { + for (grmem = gr->gr_mem; *grmem; grmem++) + if (strcmp(*grmem, pw->pw_name) == 0) + return (0); + } } + return (-1); +} + +/* + * vm_checkinsflag + * + * Checks wheter the non-root user is allowed to set an instance option. + * + * Parameters: + * vmc: the VM create parameters + * flag: the flag to be checked + * uid: the user ID of the user making the request + * + * Return values: + * 0: the permission should be granted + * -1: the permission check failed (also returned if vm == null) + */ +int +vm_checkinsflag(struct vmop_create_params *vmc, unsigned int flag, uid_t uid) +{ + /* root has no restrictions */ + if (uid == 0) + return (0); + + if ((vmc->vmc_insflags & flag) == 0) + return (-1); + + return (0); +} + +/* + * vm_checkaccess + * + * Checks if the user represented by the 'uid' parameter is allowed to + * access the file described by the 'path' parameter. + * + * Parameters: + * path: the path of a file + * uid: the user ID of the user making the request + * + * Return values: + * 0: the permission should be granted + * -1: the permission check failed + */ +int +vm_checkaccess(const char *path, uid_t uid, int amode) +{ + struct group *gr; + struct passwd *pw; + char **grmem; + struct stat st; + mode_t mode; + + /* root has no restrictions */ + if (uid == 0) + return (0); + + if (path == NULL || strlen(path) == 0) + 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)) + return (-1); + + /* check other */ + mode = amode == W_OK ? S_IWOTH : 0; + mode |= amode == R_OK ? S_IROTH : 0; + if ((st.st_mode & mode) == mode) + return (0); + /* check user */ - if ((vm->vm_running && vm->vm_uid == uid) || - (!vm->vm_running && vm->vm_params.vmc_uid == uid)) + 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; + if ((st.st_mode & mode) != mode) + return (-1); + if ((pw = getpwuid(uid)) == NULL) + return (-1); + if (pw->pw_gid == st.st_gid) + return (0); + if ((gr = getgrgid(st.st_gid)) != NULL) { + for (grmem = gr->gr_mem; *grmem; grmem++) + if (strcmp(*grmem, pw->pw_name) == 0) + return (0); + } + return (-1); } @@ -1495,9 +1682,9 @@ vm_opentty(struct vmd_vm *vm) goto fail; uid = vm->vm_uid; - gid = vm->vm_params.vmc_gid; + gid = vm->vm_params.vmc_owner.gid; - if (vm->vm_params.vmc_gid != -1) { + if (vm->vm_params.vmc_owner.gid != -1) { mode = 0660; } else if ((gr = getgrnam("tty")) != NULL) { gid = gr->gr_gid; diff --git a/usr.sbin/vmd/vmd.h b/usr.sbin/vmd/vmd.h index 036557413fa..7da53185887 100644 --- a/usr.sbin/vmd/vmd.h +++ b/usr.sbin/vmd/vmd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: vmd.h,v 1.75 2018/07/12 12:04:49 reyk Exp $ */ +/* $OpenBSD: vmd.h,v 1.76 2018/07/13 08:42:49 reyk Exp $ */ /* * Copyright (c) 2015 Mike Larkin @@ -133,15 +133,21 @@ struct vmop_ifreq { struct ifaliasreq vfr_ifra; }; +struct vmop_owner { + uid_t uid; + int64_t gid; +}; + struct vmop_create_params { struct vm_create_params vmc_params; unsigned int vmc_flags; -#define VMOP_CREATE_KERNEL 0x01 -#define VMOP_CREATE_MEMORY 0x02 -#define VMOP_CREATE_NETWORK 0x04 -#define VMOP_CREATE_DISK 0x08 -#define VMOP_CREATE_CDROM 0x10 -#define VMOP_CREATE_INSTANCE 0x20 +#define VMOP_CREATE_CPU 0x01 +#define VMOP_CREATE_KERNEL 0x02 +#define VMOP_CREATE_MEMORY 0x04 +#define VMOP_CREATE_NETWORK 0x08 +#define VMOP_CREATE_DISK 0x10 +#define VMOP_CREATE_CDROM 0x20 +#define VMOP_CREATE_INSTANCE 0x40 /* userland-only part of the create params */ unsigned int vmc_ifflags[VMM_MAX_NICS_PER_VM]; @@ -154,9 +160,12 @@ struct vmop_create_params { char vmc_ifswitch[VMM_MAX_NICS_PER_VM][VM_NAME_MAX]; char vmc_ifgroup[VMM_MAX_NICS_PER_VM][IF_NAMESIZE]; unsigned int vmc_ifrdomain[VMM_MAX_NICS_PER_VM]; + struct vmop_owner vmc_owner; + + /* instance template params */ char vmc_instance[VMM_MAX_NAME_LEN]; - uid_t vmc_uid; - int64_t vmc_gid; + struct vmop_owner vmc_insowner; + unsigned int vmc_insflags; }; struct vm_dump_header_cpuid { @@ -311,7 +320,8 @@ void vm_stop(struct vmd_vm *, int, const char *); 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 *, uid_t); +int vm_checkperm(struct vmd_vm *, struct vmop_owner *, uid_t); +int vm_checkaccess(const char *, uid_t, int); int vm_opentty(struct vmd_vm *); void vm_closetty(struct vmd_vm *); void switch_remove(struct vmd_switch *); diff --git a/usr.sbin/vmd/vmm.c b/usr.sbin/vmd/vmm.c index 12ce8cc991f..4e96fc25e42 100644 --- a/usr.sbin/vmd/vmm.c +++ b/usr.sbin/vmd/vmm.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vmm.c,v 1.87 2018/07/12 12:04:49 reyk Exp $ */ +/* $OpenBSD: vmm.c,v 1.88 2018/07/13 08:42:49 reyk Exp $ */ /* * Copyright (c) 2015 Mike Larkin @@ -285,7 +285,7 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg) IMSG_SIZE_CHECK(imsg, &vmc); memcpy(&vmc, imsg->data, sizeof(vmc)); ret = vm_register(ps, &vmc, &vm, - imsg->hdr.peerid, vmc.vmc_uid); + imsg->hdr.peerid, vmc.vmc_owner.uid); vm->vm_tty = imsg->fd; vm->vm_received = 1; break; -- 2.20.1