From 476d73d100cee30df751178c194252b2c9cc1cfd Mon Sep 17 00:00:00 2001 From: reyk Date: Fri, 13 Jul 2018 08:42:49 +0000 Subject: 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/vmd.c | 277 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 232 insertions(+), 45 deletions(-) (limited to 'usr.sbin/vmd/vmd.c') 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; -- cgit v1.2.3-59-g8ed1b