On 2022/12/29 20:58:55 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> On Thu, Dec 29, 2022 at 07:27:00PM +0100, Stefan Sperling wrote:
> > Remove filesystem access via bind(2) from gotd's auth process.
> > See the added comment for rationale, and keep in mind that
> > AF_UNIX bind(2) requires unveil "w".
> > 
> > ok?
> 
> Omar pointed out on IRC that we need to add an unveil rule
> before locking unveil.

ok for me too.

> diff 365cf0f34d08316d433e730a8663283029f729b3 fdc9c76e0b23708faf82df8e055ddf5895344150
> commit - 365cf0f34d08316d433e730a8663283029f729b3
> commit + fdc9c76e0b23708faf82df8e055ddf5895344150
> blob - 05f659daea632d0e305556351e4d6a5e97519fa0
> blob + b79d7d9818993976319266976df74331b6ba4d71
> --- gotd/gotd.c
> +++ gotd/gotd.c
> @@ -2371,6 +2371,16 @@ apply_unveil(void)
>  }
>  
>  static void
> +apply_unveil_none(void)
> +{
> +	if (unveil("/", "") == -1)
> +		fatal("unveil");
> +
> +	if (unveil(NULL, NULL) == -1)
> +		fatal("unveil");
> +}
> +
> +static void
>  apply_unveil(void)
>  {
>  	struct gotd_repo *repo;
> @@ -2582,9 +2592,17 @@ main(int argc, char **argv)
>  		break;
>  	case PROC_AUTH:
>  #ifndef PROFILE
> -		if (pledge("stdio getpw recvfd unix", NULL) == -1)
> +		if (pledge("stdio getpw recvfd unix unveil", NULL) == -1)
>  			err(1, "pledge");
>  #endif
> +		/*
> +		 * We need the "unix" pledge promise for getpeername(2) only.
> +		 * Ensure that AF_UNIX bind(2) cannot be used by revoking all
> +		 * filesystem access via unveil(2). Access to password database
> +		 * files will still work since "getpw" bypasses unveil(2).
> +		 */
> +		apply_unveil_none();
> +
>  		auth_main(title, &gotd.repos, repo_path);
>  		/* NOTREACHED */
>  		break;