Hi,
On Sun, Jun 17, 2018 at 10:35:27PM +0200, [email protected] 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 {