From: Omar Polo <op@omarpolo.com>
Subject: Re: gotsh: simplify unix_socket_path handling
To: gameoftrees@openbsd.org
Date: Sun, 22 Jan 2023 15:24:03 +0100

On 2023/01/22 15:17:16 +0100, Omar Polo <op@omarpolo.com> wrote:
> noticed while was investigating that gotd issue.  there's no need to
> copy the path to a temporary buffer, and if it's too long we will fail
> anyway right before connect.
> 
> ok?
> 
> diff /home/op/w/got
> commit - 87724ba023ff5063a868e68ac90b991ae1cf7ec8
> path + /home/op/w/got
> blob - fb5f331b5c32503b2fe1e1c4de73a8af9959be9d
> file + gotsh/gotsh.c
> --- gotsh/gotsh.c
> +++ gotsh/gotsh.c
> @@ -63,8 +63,7 @@ main(int argc, char *argv[])
>  main(int argc, char *argv[])
>  {
>  	const struct got_error *error;
> -	char unix_socket_path[PATH_MAX];
> -	char *unix_socket_path_env = getenv("GOTD_UNIX_SOCKET");
> +	const char *unix_socket_path;
>  	int gotd_sock = -1;
>  	struct sockaddr_un	 sun;
>  	char *gitcmd = NULL, *command = NULL, *repo_path = NULL;
> @@ -90,14 +89,9 @@ main(int argc, char *argv[])
>  	if (error)
>  		goto done;
>  
> -	if (unix_socket_path_env) {
> -		if (strlcpy(unix_socket_path, unix_socket_path_env,
> -		    sizeof(unix_socket_path)) >= sizeof(unix_socket_path))
> -			errx(1, "gotd socket path too long");
> -	} else {
> -		strlcpy(unix_socket_path, GOTD_UNIX_SOCKET,
> -		    sizeof(unix_socket_path));
> -	}
> +	unix_socket_path = getenv("GOTD_UNIX_SOCKET");
> +	if (unix_socket_path == NULL)
> +		unix_socket_path = GOTD_UNIX_SOCKET;
>  
>  	error = apply_unveil(unix_socket_path);
>  	if (error)

Actually we can simplify it further.

 - there's no need to call pledge the second time to remove the
   `unveil' promise, it's already implicit by the unveil(NULL, NULL)
   done at the end of apply_unveil.

 - i may be too paranoic but I'd move the socket unveiling at the
   start, right after the first pledge call.  it becomes clearer IMHO
   that unveil is immediately locked.

Diff on top of previous:

diff /home/op/w/got
commit - 3962236da9a5be363965c0439fc0a4bd4534d43b
path + /home/op/w/got
blob - b35ab29a7bf7de38825cb9a4cb6b51c291725f48
file + gotsh/gotsh.c
--- gotsh/gotsh.c
+++ gotsh/gotsh.c
@@ -72,6 +72,15 @@ main(int argc, char *argv[])
 	if (pledge("stdio recvfd unix unveil", NULL) == -1)
 		err(1, "pledge");
 #endif
+
+	unix_socket_path = getenv("GOTD_UNIX_SOCKET");
+	if (unix_socket_path == NULL)
+		unix_socket_path = GOTD_UNIX_SOCKET;
+
+	error = apply_unveil(unix_socket_path);
+	if (error)
+		goto done;
+
 	if (strcmp(argv[0], GOT_SERVE_CMD_SEND) == 0 ||
 	    strcmp(argv[0], GOT_SERVE_CMD_FETCH) == 0) {
 		if (argc != 2)
@@ -89,18 +98,6 @@ main(int argc, char *argv[])
 	if (error)
 		goto done;
 
-	unix_socket_path = getenv("GOTD_UNIX_SOCKET");
-	if (unix_socket_path == NULL)
-		unix_socket_path = GOTD_UNIX_SOCKET;
-
-	error = apply_unveil(unix_socket_path);
-	if (error)
-		goto done;
-
-#ifndef PROFILE
-	if (pledge("stdio recvfd unix", NULL) == -1)
-		err(1, "pledge");
-#endif
 	if ((gotd_sock = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
 		err(1, "socket");