From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotwebd: set chroot once and use it
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Thu, 15 Jun 2023 15:37:32 +0200

On Thu, Jun 15, 2023 at 01:20:06PM +0200, Omar Polo wrote:
> continuing the strlen() quest brought me here.  We have a lot of
> strlen(env->httpd_chroot) ? env->httpd_chroot : D_HTTPD_CHROOT that
> I'd like to get rid of.  I made parse.y strictier in that `chroot ""`
> is now rejected, so that we can fill the default chroot path in
> config_init(), optionally update in parse.y, and we're done.
> 
> sockets_run() in sockets.c will reset the chroot path since it calls
> config_init() again but it's not an issue since reached that stage we
> don't care anymore about the chroot path.  I'll address it in a future
> commit anyway however since I'd prefer if we call config_init() only
> once.
> 
> ok?

Seems reasonable to me. ok.

> diff /home/op/w/got
> commit - 23c35dae297d2eecd13e17118a547761314a578d
> path + /home/op/w/got
> blob - a6a1722df89866f0ad5a9adb12cf2820cd9831bb
> file + gotwebd/config.c
> --- gotwebd/config.c
> +++ gotwebd/config.c
> @@ -48,6 +48,8 @@ config_init(struct gotwebd *env)
>  	struct privsep *ps = env->gotwebd_ps;
>  	unsigned int what;
>  
> +	strlcpy(env->httpd_chroot, D_HTTPD_CHROOT, sizeof(env->httpd_chroot));
> +
>  	/* Global configuration. */
>  	if (privsep_process == PROC_GOTWEBD)
>  		env->prefork_gotwebd = GOTWEBD_NUMPROC;
> blob - 3633c55f9eed4080f24d13cadb5b1fb99b89cf9c
> file + gotwebd/gotwebd.c
> --- gotwebd/gotwebd.c
> +++ gotwebd/gotwebd.c
> @@ -213,8 +213,7 @@ main(int argc, char **argv)
>  		ps->ps_title[proc_id] = title;
>  
>  	for (proc = 0; proc < nitems(procs); proc++)
> -		procs[proc].p_chroot = strlen(env->httpd_chroot) ?
> -		    env->httpd_chroot : D_HTTPD_CHROOT;
> +		procs[proc].p_chroot = env->httpd_chroot;
>  
>  	/* only the gotwebd returns */
>  	proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
> @@ -251,8 +250,7 @@ main(int argc, char **argv)
>  		err(1, "gmon.out");
>  #endif
>  
> -	if (unveil(strlen(env->httpd_chroot) > 0 ? env->httpd_chroot :
> -	    D_HTTPD_CHROOT, "rwc") == -1)
> +	if (unveil(env->httpd_chroot, "rwc") == -1)
>  		err(1, "unveil");
>  
>  	if (unveil(GOT_TMPDIR_STR, "rw") == -1)
> blob - a20548a36dab44bcdc56af91c0847fc938ea5df6
> file + gotwebd/parse.y
> --- gotwebd/parse.y
> +++ gotwebd/parse.y
> @@ -231,8 +231,7 @@ main		: PREFORK NUMBER {
>  		| UNIX_SOCKET_NAME STRING {
>  			n = snprintf(gotwebd->unix_socket_name,
>  			    sizeof(gotwebd->unix_socket_name), "%s%s",
> -			    strlen(gotwebd->httpd_chroot) ?
> -			    gotwebd->httpd_chroot : D_HTTPD_CHROOT, $2);
> +			    gotwebd->httpd_chroot, $2);
>  			if (n < 0 ||
>  			    (size_t)n >= sizeof(gotwebd->unix_socket_name)) {
>  				yyerror("%s: unix_socket_name truncated",
> @@ -364,8 +363,7 @@ serveropts1	: REPOS_PATH STRING {
>  
>  			n = snprintf(new_srv->unix_socket_name,
>  			    sizeof(new_srv->unix_socket_name), "%s%s",
> -			    strlen(gotwebd->httpd_chroot) ?
> -			    gotwebd->httpd_chroot : D_HTTPD_CHROOT, $4);
> +			    gotwebd->httpd_chroot, $4);
>  			if (n < 0 ||
>  			    (size_t)n >= sizeof(new_srv->unix_socket_name)) {
>  				yyerror("%s: unix_socket_name truncated",
> 
>