"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
gotwebd: what if listen was a top-level setting?
To:
gameoftrees@openbsd.org
Date:
Thu, 16 May 2024 19:13:27 +0200

Download raw body.

Thread
My plan for this follow-up was to refine gotweb_get_server() to filter
servers available per-address: for e.g. if the server "foo" is only
listening on localhost:9001 and the server "bar" is listening only on a
local socket, gotwebd will refuse to serve "foo" for connections from
the unix socket.  i.e. to duplicate what httpd does.

This however made me think about the usefulness of having the `listen'
parameter being per-server instead of it being a global config.

For an http server it totally makes sense to make virtual hosts
reachable only from where they're explicitly configured to be so, but
for gotwebd?

gotwebd already sits behind an http server, by design, so it's less
important to filter the domains here.  It would also simplify the
configuration.  The unix_socket_name option can be romevod, as now it is
just `listen on socket "..."`.

How many are sharing the same gotwebd instance between different hosts
and don't trust their frontend http server to not filter virtual-hosts?
Do we care about this use-case?  (I'm propending to not care.)

To make things clearer: gotwebd already works like this.  It only looks
at the server name (i.e. the Host header in the request) and has done so
since when it was imported.  My proposed diff just makes this more
obvious from the config file too.


diffstat /home/op/w/got
 M  gotwebd/gotwebd.conf.5  |  30+  22-
 M  gotwebd/gotwebd.h       |   0+   3-
 M  gotwebd/parse.y         |  46+  81-

3 files changed, 76 insertions(+), 106 deletions(-)

diff /home/op/w/got
commit - 0f88252c795396951ad9bcf40c534b5852dc5e28
path + /home/op/w/got
blob - 1358883401f1dd6e5ee11d565febab287239d368
file + gotwebd/gotwebd.conf.5
--- gotwebd/gotwebd.conf.5
+++ gotwebd/gotwebd.conf.5
@@ -58,23 +58,42 @@ environment of
 If not specified then
 .Pa /var/www
 will be used.
+.It Ic listen on Ar address Ic port Ar number
+Configure an address and port for incoming FastCGI connections.
+Valid
+.Ar address
+arguments are hostnames, IPv4 and IPv6 addresses.
+The
+.Ar port
+argument may be number or a service name defined in
+.Xr services 5 .
+May be specified multiple times to build up a list of listening sockets.
+.It Ic listen on socket Ar path
+Configure a
+.Ux Ns -domain
+socket for incoming FastCGI connections.
+May be specified multiple times to build up a list of listening sockets.
 .It Ic prefork Ar number
 Run the specified number of server processes.
 .Xr gotwebd 8
 runs 3 server processes by default.
-.It Ic unix_socket_name Ar path
-Set the path to the default unix socket.
-Defaults to
+.El
+.Pp
+If no
+.Ic listen
+directive is used,
+.Xr gotwebd 8
+will listen on the
+.Ux Ns -domain
+socket at
 .Pa /var/www/run/gotweb.sock .
-.El
 .Sh SERVER CONFIGURATION
 At least one server context must exist for
 .Xr gotwebd 8
 to function.
 In case no server context is defined in the configuration file, a default
-server context will be used, which listens on a unix socket at
-.Pa /var/www/run/gotweb.sock
-and uses default parameters for all applicable settings.
+server context will be used which uses default parameters for all
+applicable settings.
 .Pp
 A server context is declared with a unique
 .Ar name ,
@@ -92,19 +111,6 @@ Set the path to a custom Cascading Style Sheet (CSS) t
 If this option is not specified then the default style sheet
 .Sq gotweb.css
 will be used.
-.It Ic listen on Ar address Ic port Ar number
-Configure an address and port for incoming FastCGI connections.
-Valid
-.Ar address
-arguments are hostnames, IPv4 and IPv6 addresses.
-The
-.Ar port
-argument may be number or a service name defined in
-.Xr services 5 .
-.Pp
-May be specified multiple times to build up a list of listening sockets.
-.It Ic listen on socket Ar path
-Set the path to the unix socket used by the server.
 .It Ic logo Ar path
 Set the path to an image file containing a logo to be displayed.
 Defaults to
@@ -203,9 +209,11 @@ implicit
 .Ux
 socket.
 .Bd -literal -offset indent
+listen on 127.0.0.1 port 9000
+listen on ::1 port 9000
+
 server "localhost" {
-	listen on 127.0.0.1 port 9000
-	listen on ::1 port 9000
+	site_name "my public repos"
 }
 .Ed
 .Sh SEE ALSO
blob - 1657f8a62dd05c0eea153e252b2a93ddc1aeb1dc
file + gotwebd/gotwebd.h
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -285,7 +285,6 @@ TAILQ_HEAD(addresslist, address);
 
 struct server {
 	TAILQ_ENTRY(server)	 entry;
-	struct addresslist	al;
 
 	char		 name[GOTWEBD_MAXTEXT];
 
@@ -364,8 +363,6 @@ struct gotwebd {
 	int		 server_cnt;
 
 	char		 httpd_chroot[PATH_MAX];
-
-	char		 unix_socket_name[PATH_MAX];
 };
 
 /*
blob - 83ff26c00de4deadcb16dcb4494799c8d0bad002
file + gotwebd/parse.y
--- gotwebd/parse.y
+++ gotwebd/parse.y
@@ -94,10 +94,10 @@ static struct server		*conf_new_server(const char *);
 int				 getservice(const char *);
 int				 n;
 
-int		 get_addrs(const char *, const char *, struct server *);
-int		 get_unix_addr(const char *, struct server *);
+int		 get_addrs(const char *, const char *);
+int		 get_unix_addr(const char *);
 int		 addr_dup_check(struct addresslist *, struct address *);
-int		 add_addr(struct server *, struct address *);
+int		 add_addr(struct address *);
 
 typedef struct {
 	union {
@@ -113,7 +113,7 @@ typedef struct {
 %token	LOGO_URL SHOW_REPO_OWNER SHOW_REPO_AGE SHOW_REPO_DESCRIPTION
 %token	MAX_REPOS_DISPLAY REPOS_PATH MAX_COMMITS_DISPLAY ON ERROR
 %token	SHOW_SITE_OWNER SHOW_REPO_CLONEURL PORT PREFORK RESPECT_EXPORTOK
-%token	UNIX_SOCKET_NAME SERVER CHROOT CUSTOM_CSS SOCKET
+%token	SERVER CHROOT CUSTOM_CSS SOCKET
 %token	SUMMARY_COMMITS_DISPLAY SUMMARY_TAGS_DISPLAY
 
 %token	<v.string>	STRING
@@ -202,19 +202,38 @@ main		: PREFORK NUMBER {
 			}
 			free($2);
 		}
-		| UNIX_SOCKET_NAME STRING {
-			n = snprintf(gotwebd->unix_socket_name,
-			    sizeof(gotwebd->unix_socket_name), "%s%s",
-			    gotwebd->httpd_chroot, $2);
-			if (n < 0 ||
-			    (size_t)n >= sizeof(gotwebd->unix_socket_name)) {
-				yyerror("%s: unix_socket_name truncated",
-				    __func__);
-				free($2);
+		| LISTEN ON listen_addr PORT STRING {
+			if (get_addrs($3, $5) == -1) {
+				yyerror("could not get addrs");
 				YYERROR;
 			}
-			free($2);
+			free($3);
+			free($5);
 		}
+		| LISTEN ON listen_addr PORT NUMBER {
+			char portno[32];
+			int n;
+
+			n = snprintf(portno, sizeof(portno), "%lld",
+			    (long long)$5);
+			if (n < 0 || (size_t)n >= sizeof(portno))
+				fatalx("port number too long: %lld",
+				    (long long)$5);
+
+			if (get_addrs($3, portno) == -1) {
+				yyerror("could not get addrs");
+				YYERROR;
+			}
+			free($3);
+		}
+		| LISTEN ON SOCKET STRING {
+			if (get_unix_addr($4) == -1) {
+				yyerror("can't listen on %s", $4);
+				free($4);
+				YYERROR;
+			}
+			free($4);
+		}
 		;
 
 server		: SERVER STRING {
@@ -319,38 +338,6 @@ serveropts1	: REPOS_PATH STRING {
 			}
 			free($2);
 		}
-		| LISTEN ON listen_addr PORT STRING {
-			if (get_addrs($3, $5, new_srv) == -1) {
-				yyerror("could not get addrs");
-				YYERROR;
-			}
-			free($3);
-			free($5);
-		}
-		| LISTEN ON listen_addr PORT NUMBER {
-			char portno[32];
-			int n;
-
-			n = snprintf(portno, sizeof(portno), "%lld",
-			    (long long)$5);
-			if (n < 0 || (size_t)n >= sizeof(portno))
-				fatalx("port number too long: %lld",
-				    (long long)$5);
-
-			if (get_addrs($3, portno, new_srv) == -1) {
-				yyerror("could not get addrs");
-				YYERROR;
-			}
-			free($3);
-		}
-		| LISTEN ON SOCKET STRING {
-			if (get_unix_addr($4, new_srv) == -1) {
-				yyerror("can't listen on %s", $4);
-				free($4);
-				YYERROR;
-			}
-			free($4);
-		}
 		| SHOW_SITE_OWNER boolean {
 			new_srv->show_site_owner = $2;
 		}
@@ -472,7 +459,6 @@ lookup(char *s)
 		{ "socket",			SOCKET },
 		{ "summary_commits_display",	SUMMARY_COMMITS_DISPLAY },
 		{ "summary_tags_display",	SUMMARY_TAGS_DISPLAY },
-		{ "unix_socket_name",		UNIX_SOCKET_NAME },
 	};
 	const struct keywords *p;
 
@@ -805,19 +791,12 @@ int
 parse_config(const char *filename, struct gotwebd *env)
 {
 	struct sym *sym, *next;
-	struct server *srv;
-	int n;
 
 	if (config_init(env) == -1)
 		fatalx("failed to initialize configuration");
 
 	gotwebd = env;
 
-	n = snprintf(env->unix_socket_name, sizeof(env->unix_socket_name),
-	    "%s%s", D_HTTPD_CHROOT, D_UNIX_SOCKET);
-	if (n < 0 || (size_t)n >= sizeof(env->unix_socket_name))
-		fatalx("%s: snprintf", __func__);
-
 	file = newfile(filename, 0);
 	if (file == NULL) {
 		add_default_server();
@@ -847,12 +826,11 @@ parse_config(const char *filename, struct gotwebd *env
 	if (gotwebd->server_cnt == 0)
 		add_default_server();
 
-	/* add the implicit listen on socket where missing */
-	TAILQ_FOREACH(srv, &gotwebd->servers, entry) {
-		if (!TAILQ_EMPTY(&srv->al))
-			continue;
-		if (get_unix_addr(env->unix_socket_name, srv) == -1)
-			yyerror("can't listen on %s", env->unix_socket_name);
+	/* add the implicit listen on socket */
+	if (TAILQ_EMPTY(&gotwebd->addresses)) {
+		const char *path = D_HTTPD_CHROOT D_UNIX_SOCKET;
+		if (get_unix_addr(path) == -1)
+			yyerror("can't listen on %s", path);
 	}
 
 	if (errors)
@@ -915,7 +893,6 @@ conf_new_server(const char *name)
 	srv->summary_commits_display = D_MAXSLCOMMDISP;
 	srv->summary_tags_display = D_MAXSLTAGDISP;
 
-	TAILQ_INIT(&srv->al);
 	TAILQ_INSERT_TAIL(&gotwebd->servers, srv, entry);
 	gotwebd->server_cnt++;
 
@@ -998,7 +975,7 @@ symget(const char *nam)
 }
 
 int
-get_addrs(const char *hostname, const char *servname, struct server *new_srv)
+get_addrs(const char *hostname, const char *servname)
 {
 	struct addrinfo hints, *res0, *res;
 	int error;
@@ -1053,7 +1030,7 @@ get_addrs(const char *hostname, const char *servname, 
 			fatalx("unknown address family %d", res->ai_family);
 		}
 
-		if (add_addr(new_srv, h) == -1) {
+		if (add_addr(h) == -1) {
 			freeaddrinfo(res0);
 			return -1;
 		}
@@ -1063,7 +1040,7 @@ get_addrs(const char *hostname, const char *servname, 
 }
 
 int
-get_unix_addr(const char *path, struct server *new_srv)
+get_unix_addr(const char *path)
 {
 	struct address *h;
 	struct sockaddr_un *sun;
@@ -1084,7 +1061,7 @@ get_unix_addr(const char *path, struct server *new_srv
 		return (-1);
 	}
 
-	return add_addr(new_srv, h);
+	return add_addr(h);
 }
 
 int
@@ -1106,25 +1083,13 @@ addr_dup_check(struct addresslist *al, struct address 
 }
 
 int
-add_addr(struct server *new_srv, struct address *h)
+add_addr(struct address *h)
 {
-	struct server *srv;
-	struct address *dup;
-
-	TAILQ_FOREACH(srv, &gotwebd->servers, entry) {
-		if (addr_dup_check(&srv->al, h) == -1) {
-			free(h);
-			return 0;
-		}
-	}
-
 	if (addr_dup_check(&gotwebd->addresses, h) == 0) {
-		if ((dup = calloc(1, sizeof(*dup))) == NULL)
-			fatal("%s: calloc", __func__);
-		memcpy(dup, h, sizeof(*dup));
-		TAILQ_INSERT_TAIL(&gotwebd->addresses, dup, entry);
+		TAILQ_INSERT_TAIL(&gotwebd->addresses, h, entry);
+		return (0);
 	}
 
-	TAILQ_INSERT_TAIL(&new_srv->al, h, entry);
+	free(h);
 	return (0);
 }