On Sun, May 02, 2021 at 09:00:21PM -0400, Ashton Fagg wrote:
> "Theo de Raadt" <dera...@openbsd.org> writes:
>
> > Showing the symbolic name is not doing anywhere else in the tree.
> >
> > Most likely they should be
> >
> > err(1, "unveil: %s", path);
>
> Per Theo's advice, updated diffs are attached.
>
There are 4 or five cases how unveil is called, depending on how
you count. The permission seems to be always a string literal or NULL.
The path can be:
1) a string literal
2) a #define
3) a variable
4) the empty string literal ""
5) NULL
> diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
> index 993c829f2d2..ba88d9f5f67 100644
> --- a/sbin/dhclient/dhclient.c
> +++ b/sbin/dhclient/dhclient.c
> @@ -2334,11 +2334,11 @@ fork_privchld(struct interface_info *ifi, int fd, int
> fd2)
> fatal("socket(AF_ROUTE, SOCK_RAW)");
>
> if (unveil(_PATH_RESCONF, "wc") == -1)
> - fatal("unveil");
> + fatal("unveil %s", _PATH_RESCONF);
> if (unveil("/etc/resolv.conf.tail", "r") == -1)
> - fatal("unveil");
> + fatal("unveil /etc/resolve.conf.tail");
> if (unveil(NULL, NULL) == -1)
> - fatal("unveil");
> + fatal("unveil(NULL,NULL)");
>
In this hunk alone you have three out of five and you log them all
differently. I think this should be unified as
fatal("unveil(\"%s\", \"%s\")", _PATH_RESCONF, "wc");
fatal("unveil(\"%s\", \"%s\")", /etc/resolv.conf.tail, "r");
fatal("unveil(\"%s\", \"%s\")", "NULL", "NULL");
This will also do the right thing for unveil("", ""):
fatal("unveil(\"%s\", \"%s\")", "", "");
What I did to slaacd and dhcpleased was wrong in that sense, in my
defense I wasn't looking at the big picture.
--
I'm not entirely sure you are real.
