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.
>
> 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 13:58:17 -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 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)
