Hi Reyk, Thank you for your help on the kernel includes. I have applied the patch to vmd -current and it works well on my setup. A non-wheel user can indeed start/stop a VM and connect to the console.
Thanx!! Mischa On 19 Jun at 20:38, Reyk Floeter <r...@openbsd.org> wrote: > Hi, > > On Sun, Jun 17, 2018 at 10:35:27PM +0200, obs...@high5.nl wrote: > > >Synopsis: VMM owner needs to be part of group wheel in order to run vmctl > > >console|start|stop > > the solution is not that easy as it seemed. > > 1. Change the umask and let everyone access vmd, restrict the commands > internally. > > While this is a possible solution, I also agree that this allows any > user (including system/privsep users) to trigger actions and imsgs in > vmd; even if the result is permission denied as this is checked fairly > late. > > 2. Change the default owner group to root:_vmd. > > It would be possible to define a hardcoded group, or use group _vmd, > but this doesn't feel right. > > 3. Let the user configure the owner of the control socket. > > This allows you to configure your own group, like "devops" (hrhr), or > fetch the group from YP/LDA, and let them mess with your VMs. I think > this is much more viable in a multi-user environment. Add the > following to the top/global section of /etc/vm.conf: "socket owner :devops" > > privsep/pledge also makes it a bit more complicated for us because I > don't want to allow the control process to chown the unix socket; so > it is done by the parent and some messaging. > > The attached diff implements 3. ... comments? OK? Should we use 2. instead? > > Reyk > > Index: usr.sbin/vmd/control.c > =================================================================== > RCS file: /cvs/src/usr.sbin/vmd/control.c,v > retrieving revision 1.23 > diff -u -p -u -p -r1.23 control.c > --- usr.sbin/vmd/control.c 13 May 2018 22:48:11 -0000 1.23 > +++ usr.sbin/vmd/control.c 19 Jun 2018 18:27:55 -0000 > @@ -103,6 +103,7 @@ control_dispatch_vmd(int fd, struct priv > break; > case IMSG_VMDOP_CONFIG: > config_getconfig(ps->ps_env, imsg); > + proc_compose(ps, PROC_PARENT, IMSG_VMDOP_DONE, NULL, 0); > break; > case IMSG_CTL_RESET: > config_getreset(ps->ps_env, imsg); > @@ -169,6 +170,18 @@ control_init(struct privsep *ps, struct > > cs->cs_fd = fd; > cs->cs_env = ps; > + > + proc_compose(ps, PROC_PARENT, IMSG_VMDOP_DONE, NULL, 0); > + > + return (0); > +} > + > +int > +control_reset(struct control_sock *cs) > +{ > + /* Updating owner of the control socket */ > + if (chown(cs->cs_name, cs->cs_uid, cs->cs_gid) == -1) > + return (-1); > > return (0); > } > Index: usr.sbin/vmd/parse.y > =================================================================== > RCS file: /cvs/src/usr.sbin/vmd/parse.y,v > retrieving revision 1.35 > diff -u -p -u -p -r1.35 parse.y > --- usr.sbin/vmd/parse.y 19 Jun 2018 17:12:34 -0000 1.35 > +++ usr.sbin/vmd/parse.y 19 Jun 2018 18:27:55 -0000 > @@ -119,7 +119,8 @@ typedef struct { > > %token INCLUDE ERROR > %token ADD BOOT CDROM DISABLE DISK DOWN ENABLE GROUP INTERFACE LLADDR > LOCAL > -%token LOCKED MEMORY NIFS OWNER PATH PREFIX RDOMAIN SIZE SWITCH UP VM > VMID > +%token LOCKED MEMORY NIFS OWNER PATH PREFIX RDOMAIN SIZE SOCKET SWITCH > UP > +%token VM VMID > %token <v.number> NUMBER > %token <v.string> STRING > %type <v.lladdr> lladdr > @@ -190,6 +191,10 @@ main : LOCAL PREFIX STRING { > > memcpy(&env->vmd_cfg.cfg_localprefix, &h, sizeof(h)); > } > + | SOCKET OWNER owner_id { > + env->vmd_ps.ps_csock.cs_uid = $3.uid; > + env->vmd_ps.ps_csock.cs_gid = $3.gid == -1 ? 0 : $3.gid; > + } > ; > > switch : SWITCH string { > @@ -678,6 +683,7 @@ lookup(char *s) > { "prefix", PREFIX }, > { "rdomain", RDOMAIN }, > { "size", SIZE }, > + { "socket", SOCKET }, > { "switch", SWITCH }, > { "up", UP }, > { "vm", VM } > Index: usr.sbin/vmd/proc.h > =================================================================== > RCS file: /cvs/src/usr.sbin/vmd/proc.h,v > retrieving revision 1.12 > diff -u -p -u -p -r1.12 proc.h > --- usr.sbin/vmd/proc.h 27 Mar 2017 00:28:04 -0000 1.12 > +++ usr.sbin/vmd/proc.h 19 Jun 2018 18:27:55 -0000 > @@ -62,6 +62,8 @@ struct control_sock { > int cs_fd; > int cs_restricted; > void *cs_env; > + uid_t cs_uid; > + gid_t cs_gid; > > TAILQ_ENTRY(control_sock) cs_entry; > }; > @@ -192,6 +194,7 @@ int proc_flush_imsg(struct privsep *, e > /* control.c */ > void control(struct privsep *, struct privsep_proc *); > int control_init(struct privsep *, struct control_sock *); > +int control_reset(struct control_sock *); > int control_listen(struct control_sock *); > void control_cleanup(struct control_sock *); > > Index: usr.sbin/vmd/vm.conf.5 > =================================================================== > RCS file: /cvs/src/usr.sbin/vmd/vm.conf.5,v > retrieving revision 1.29 > diff -u -p -u -p -r1.29 vm.conf.5 > --- usr.sbin/vmd/vm.conf.5 18 Jun 2018 06:04:25 -0000 1.29 > +++ usr.sbin/vmd/vm.conf.5 19 Jun 2018 18:27:55 -0000 > @@ -100,6 +100,16 @@ in the > section below. > The default is > .Ar 100.64.0.0/10 . > +.It Cm socket owner Ar user Ns Op : Ns Ar group > +Set the control socket owner to the specified user or group. > +Users with access to the control socket will be allowed to use > +.Nm vmctl > +for restricted access to > +.Nm vmd. > +The default is > +.Ar root:wheel . > +.It Cm socket owner Pf : Ar group > +Set the control socket owner to the specified group. > .El > .Sh VM CONFIGURATION > Each > Index: usr.sbin/vmd/vmd.c > =================================================================== > RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v > retrieving revision 1.86 > diff -u -p -u -p -r1.86 vmd.c > --- usr.sbin/vmd/vmd.c 19 Jun 2018 17:12:34 -0000 1.86 > +++ usr.sbin/vmd/vmd.c 19 Jun 2018 18:27:55 -0000 > @@ -85,6 +85,7 @@ vmd_dispatch_control(int fd, struct priv > struct vmd_vm *vm = NULL; > char *str = NULL; > uint32_t id = 0; > + struct control_sock *rcs; > > switch (imsg->hdr.type) { > case IMSG_VMDOP_START_VM_REQUEST: > @@ -274,6 +275,12 @@ vmd_dispatch_control(int fd, struct priv > IMSG_VMDOP_RECEIVE_VM_END, vm->vm_vmid, imsg->fd, > NULL, 0); > } > + break; > + case IMSG_VMDOP_DONE: > + control_reset(&ps->ps_csock); > + TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry) > + control_reset(rcs); > + cmd = 0; > break; > default: > return (-1); > Index: usr.sbin/vmd/vmd.h > =================================================================== > RCS file: /cvs/src/usr.sbin/vmd/vmd.h,v > retrieving revision 1.68 > diff -u -p -u -p -r1.68 vmd.h > --- usr.sbin/vmd/vmd.h 27 Apr 2018 12:15:10 -0000 1.68 > +++ usr.sbin/vmd/vmd.h 19 Jun 2018 18:27:55 -0000 > @@ -105,7 +105,8 @@ enum imsg_type { > IMSG_VMDOP_PRIV_IFRDOMAIN, > IMSG_VMDOP_VM_SHUTDOWN, > IMSG_VMDOP_VM_REBOOT, > - IMSG_VMDOP_CONFIG > + IMSG_VMDOP_CONFIG, > + IMSG_VMDOP_DONE > }; > > struct vmop_result {