Nevermind about the getpw change, I ran start/stop and show in between and
didn't see that issue since I was running the old bin without my change. Here's
the new diff to just sort those promises to the canonical order.
And yes for now the unveil(2) change is just for correctness, it doesn't
actually fix anything.
Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
retrieving revision 1.72
diff -u -p -u -r1.72 main.c
--- main.c 30 Jul 2022 14:17:42 -0000 1.72
+++ main.c 31 Aug 2022 15:45:18 -0000
@@ -171,7 +171,7 @@ parse(int argc, char *argv[])
if (!ctl->has_pledge) {
/* pledge(2) default if command doesn't have its own pledge */
- if (pledge("stdio rpath exec unix getpw unveil", NULL) == -1)
+ if (pledge("stdio rpath unix getpw exec unveil", NULL) == -1)
err(1, "pledge");
}
if (ctl->main(&res, argc, argv) != 0)
@@ -196,7 +196,7 @@ vmmaction(struct parse_result *res)
unsigned int flags;
if (ctl_sock == -1) {
- if (unveil(SOCKET_NAME, "r") == -1)
+ if (unveil(SOCKET_NAME, "rw") == -1)
err(1, "unveil %s", SOCKET_NAME);
if ((ctl_sock = socket(AF_UNIX,
SOCK_STREAM|SOCK_CLOEXEC, 0)) == -1)
On 11:21 Wed 31 Aug , Dave Voutila wrote:
>
> Ricardo Mestre <[email protected]> writes:
>
> > After discussing with martijn@ if we should keep using unveil(2) (or not),
> > in
> > his latest diff of further privsep'ing snmpd(8), and also after he also
> > spoke
> > with deraadt@, it turns out that unix's promise in pledge(2) is not
> > enforcing
> > the unix socket's file permissions as it should.
> >
> > e.g. Unveiling the socket only with read permissions, having
> > pledge(...unix...)
> > and then call connect(2), which needs read/write, actually won't fail.
> > One specific place where this happens is on vmctl(8) hence the diff below.
> >
>
> Is this a correctness change? Am I understanding that correctly?
>
> > Unveiling the socket and calling connect(2) will need rw, while here also
> > remove
> > getpw promise from pledge(2) since nowadays it's possible to start/stop the
> > vms
> > with the owner's account without needing this promise, and sort the
> > remaining
> > promises to their canonical order.
>
> getpw is needed for user_from_{u,g}id(3) calls, otherwise `vmctl status`
> prints just the uids and not usernames.
>
> >
> > Comments? OK? OK with splitting in 2 diffs (one for unveil, another for
> > pledge)?
> >
> > Note: The pledge and unveil calls in this program are all over the place and
> > need to be glued together as much as possible in the same place, I have a
> > diff
> > for that but need to test it first, this is just the quick&dirty version.
> >
