On Fri, Mar 12, 2021 at 08:41:59AM +0100, Sebastien Marie wrote:
> Hi,
>
> The following diff is a cleanup to remove two leftover checks, which
> were used when ni_unveil was used with UNVEIL_INSPECT:
>
> it was used by:
> - readlink(2) - removed 2019-08-31
>
> Make readlink require UNVEIL_READ instead of UNVEIL_INSPECT only
> since realpath() is now a system call
>
> - stat(2) and access(2) - removed 2019-03-24
>
> Make stat(2) and access(2) need UNVEIL_READ instead of UNVEIL_INSPECT
>
> UNVEIL_INSPECT is a hack we added to get chrome/glib working. It silently
> adds permission for stat(2), access(2), and readlink(2) to be used on
> all path components of any unveil'ed path. robert@ has sucessfully now
> fixed chrome/glib to not require exessive TOC vs TOU stat(2) and access(2)
> calls on the paths it uses, so that this no longer needed there.
>
> readlink(2) is the sole call that is now permitted by UNVEIL_INSPECT,
> and this is only needed so that realpath(3) can work. Going forward we will
> likely make a realpath(2), after which we can completely deprecate
> UNVEIL_INSPECT.
>
>
> I audited the values sets in ni_unveil, and UNVEIL_INSPECT is
> effectively not used anywhere in this variable.
>
> The diff removes two checks that were done:
> - one in unveil_flagmatch(), for a debug printf
> - one in pledge_namei(), for "getpw" usage when using
> access("/var/run/ypbind.lock")
>
> Comments or OK ?
OK claudio@
I think all of UNVEIL_INSPECT can go. I have a diff that does that on top
of this one.
> --
> Sebastien Marie
>
> diff 48cf7af2deddb13b1f53f18782fd5612c3fdc34a /home/semarie/repos/openbsd/src
> blob - 2de0d500e39367046a93c951aeded70bcdeb097d
> file + sys/kern/kern_pledge.c
> --- sys/kern/kern_pledge.c
> +++ sys/kern/kern_pledge.c
> @@ -619,8 +619,7 @@ pledge_namei(struct proc *p, struct nameidata *ni, cha
> /* when avoiding YP mode, getpw* functions touch this */
> if (ni->ni_pledge == PLEDGE_RPATH &&
> strcmp(path, "/var/run/ypbind.lock") == 0) {
> - if ((p->p_p->ps_pledge & PLEDGE_GETPW) ||
> - (ni->ni_unveil == UNVEIL_INSPECT)) {
> + if (p->p_p->ps_pledge & PLEDGE_GETPW) {
> ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
> return (0);
> } else
> blob - 0822248e435b45baf4fa2640cc1a89d85f632cad
> file + sys/kern/kern_unveil.c
> --- sys/kern/kern_unveil.c
> +++ sys/kern/kern_unveil.c
> @@ -720,11 +720,6 @@ unveil_flagmatch(struct nameidata *ni, u_char flags)
> return 0;
> }
> }
> - if (ni->ni_unveil & UNVEIL_INSPECT) {
> -#ifdef DEBUG_UNVEIL
> - printf("any unveil allows UNVEIL_INSPECT\n");
> -#endif
> - }
> return 1;
> }
>
>
--
:wq Claudio
