commit 610dd8c9f454e1625b0eddc16af87b765279b0a5
from: Stefan Sperling <stsp@stsp.name>
date: Fri Aug 19 16:29:51 2022 UTC

gotwebd: struct socket_conf can only use a single address, so don't use a list

ok tracey

commit - 01498c42e0fc9fb6355312a236656cf2f36cebc0
commit + 610dd8c9f454e1625b0eddc16af87b765279b0a5
blob - d908471eeceff6cfcb6cf32db263b9fc5e018ba4
blob + 4a98f544d07275e73edfe3df1b42a22bc24af9c0
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -289,7 +289,7 @@ enum client_action {
 };
 
 struct socket_conf {
-	struct addresslist	al;
+	struct address	 addr;
 
 	char		 name[GOTWEBD_MAXTEXT];
 	char		 srv_name[GOTWEBD_MAXTEXT];
blob - bde51dbff1e8cbc18fd20d36fe3701aa4ab66e53
blob + 836377578903d72f44f5cc1ae1fecf1e440af00c
--- gotwebd/sockets.c
+++ gotwebd/sockets.c
@@ -73,12 +73,14 @@ void	 sockets_rlimit(int);
 
 int	 sockets_dispatch_gotwebd(int, struct privsep_proc *, struct imsg *);
 int	 sockets_unix_socket_listen(struct privsep *, struct socket *);
-int	 sockets_create_socket(struct addresslist *, in_port_t);
+int	 sockets_create_socket(struct address *, in_port_t);
 int	 sockets_accept_reserve(int, struct sockaddr *, socklen_t *, int,
 	    volatile int *);
 
-struct socket *sockets_conf_new_socket(struct gotwebd *, struct server *,
-    int, int);
+struct socket *sockets_conf_new_socket_unix(struct gotwebd *, struct server *,
+    int);
+struct socket *sockets_conf_new_socket_fcgi(struct gotwebd *, struct server *,
+    int, struct address *);
 
 int cgi_inflight = 0;
 
@@ -117,75 +119,64 @@ void
 sockets_parse_sockets(struct gotwebd *env)
 {
 	struct server *srv;
+	struct address *a;
 	struct socket *new_sock = NULL;
-	int sock_id = 0, ipv4 = 0, ipv6 = 0;
+	int sock_id = 1;
 
 	TAILQ_FOREACH(srv, &env->servers, entry) {
 		if (srv->unix_socket) {
-			sock_id++;
-			new_sock = sockets_conf_new_socket(env, srv,
-			    sock_id, AF_UNIX);
-			/* Should always succeed. */
-			TAILQ_INSERT_TAIL(&env->sockets, new_sock, entry);
-		}
-
-		if (srv->fcgi_socket) {
-			sock_id++;
-			new_sock = sockets_conf_new_socket(env, srv, sock_id,
-			    AF_INET);
+			new_sock = sockets_conf_new_socket_unix(env, srv,
+			    sock_id);
 			if (new_sock) {
+				sock_id++;
 				TAILQ_INSERT_TAIL(&env->sockets, new_sock,
 				    entry);
-				ipv4 = 1;
-				sock_id++;
 			}
+		}
 
-			new_sock = sockets_conf_new_socket(env, srv, sock_id,
-			    AF_INET6);
-			if (new_sock) {
-				TAILQ_INSERT_TAIL(&env->sockets,
-				    new_sock, entry);
-				ipv6 = 1;
+		if (srv->fcgi_socket) {
+			if (TAILQ_EMPTY(&srv->al)) {
+				fatalx("%s: server %s has no IP addresses to "
+				    "listen for FCGI connections", __func__,
+				    srv->name);
 			}
-
-			if (ipv4 == 0 && ipv6 == 0) {
-				fatalx("%s: have no IP addresses to listen "
-				    "for FCGI connections", __func__);
+			TAILQ_FOREACH(a, &srv->al, entry) {
+				if (a->ss.ss_family != AF_INET &&
+				    a->ss.ss_family != AF_INET6)
+					continue;
+				new_sock = sockets_conf_new_socket_fcgi(env,
+				    srv, sock_id, a);
+				if (new_sock) {
+					sock_id++;
+					TAILQ_INSERT_TAIL(&env->sockets,
+					    new_sock, entry);
+				}
 			}
 		}
 	}
 }
 
-/* Returns NULL if no addresses in the requested family are configured. */
 struct socket *
-sockets_conf_new_socket(struct gotwebd *env, struct server *srv, int id,
-    int af_type)
+sockets_conf_new_socket_unix(struct gotwebd *env, struct server *srv, int id)
 {
 	struct socket *sock;
-	struct address *a, *acp;
 	int n;
 
 	if ((sock = calloc(1, sizeof(*sock))) == NULL)
 		fatalx("%s: calloc", __func__);
 
-	TAILQ_INIT(&sock->conf.al);
-
 	sock->conf.id = id;
 	sock->fd = -1;
-	sock->conf.af_type = af_type;
+	sock->conf.af_type = AF_UNIX;
 
-	if (af_type == AF_UNIX) {
-		if (strlcpy(sock->conf.unix_socket_name,
-		    srv->unix_socket_name,
-		    sizeof(sock->conf.unix_socket_name)) >=
-		    sizeof(sock->conf.unix_socket_name)) {
-			free(sock);
-			fatalx("%s: strlcpy", __func__);
-		}
+	if (strlcpy(sock->conf.unix_socket_name,
+	    srv->unix_socket_name,
+	    sizeof(sock->conf.unix_socket_name)) >=
+	    sizeof(sock->conf.unix_socket_name)) {
+		free(sock);
+		fatalx("%s: strlcpy", __func__);
 	}
 
-	sock->conf.fcgi_socket_port = srv->fcgi_socket_port;
-
 	n = snprintf(sock->conf.name, GOTWEBD_MAXTEXT, "%s_parent",
 	    srv->name);
 	if (n < 0 || (size_t)n >= GOTWEBD_MAXTEXT) {
@@ -199,35 +190,53 @@ sockets_conf_new_socket(struct gotwebd *env, struct se
 		fatalx("%s: strlcpy", __func__);
 	}
 
-	TAILQ_FOREACH(a, &srv->al, entry) {
-		if (a->ss.ss_family != af_type)
-			continue;
+	return sock;
+}
 
-		if ((acp = calloc(1, sizeof(*acp))) == NULL) {
-			free(sock);
-			fatal("%s: calloc", __func__);
-		}
-		memcpy(&acp->ss, &a->ss, sizeof(acp->ss));
-		acp->ipproto = a->ipproto;
-		acp->prefixlen = a->prefixlen;
-		acp->port = a->port;
-		if (strlen(a->ifname) != 0) {
-			if (strlcpy(acp->ifname, a->ifname,
-			    sizeof(acp->ifname)) >= sizeof(acp->ifname)) {
-				fatalx("%s: interface name truncated",
-				    __func__);
-			}
-		}
+struct socket *
+sockets_conf_new_socket_fcgi(struct gotwebd *env, struct server *srv, int id,
+    struct address *a)
+{
+	struct socket *sock;
+	struct address *acp;
+	int n;
 
-		TAILQ_INSERT_TAIL(&sock->conf.al, acp, entry);
-	}
+	if ((sock = calloc(1, sizeof(*sock))) == NULL)
+		fatalx("%s: calloc", __func__);
 
-	if ((af_type == AF_INET || af_type == AF_INET6) &&
-	    TAILQ_EMPTY(&sock->conf.al)) {
+	sock->conf.id = id;
+	sock->fd = -1;
+	sock->conf.af_type = a->ss.ss_family;
+
+	sock->conf.fcgi_socket_port = srv->fcgi_socket_port;
+
+	n = snprintf(sock->conf.name, GOTWEBD_MAXTEXT, "%s_parent",
+	    srv->name);
+	if (n < 0 || (size_t)n >= GOTWEBD_MAXTEXT) {
 		free(sock);
-		return NULL;
+		fatalx("%s: snprintf", __func__);
 	}
 
+	if (strlcpy(sock->conf.srv_name, srv->name,
+	    sizeof(sock->conf.srv_name)) >= sizeof(sock->conf.srv_name)) {
+		free(sock);
+		fatalx("%s: strlcpy", __func__);
+	}
+
+	acp = &sock->conf.addr;
+
+	memcpy(&acp->ss, &a->ss, sizeof(acp->ss));
+	acp->ipproto = a->ipproto;
+	acp->prefixlen = a->prefixlen;
+	acp->port = a->port;
+	if (strlen(a->ifname) != 0) {
+		if (strlcpy(acp->ifname, a->ifname,
+		    sizeof(acp->ifname)) >= sizeof(acp->ifname)) {
+			fatalx("%s: interface name truncated",
+			    __func__);
+		}
+	}
+
 	return (sock);
 }
 
@@ -378,10 +387,10 @@ sockets_privinit(struct gotwebd *env, struct socket *s
 		log_debug("%s: initializing %s FCGI socket on port %d for %s",
 		    __func__, sock->conf.af_type == AF_INET ? "inet" : "inet6",
 		    sock->conf.fcgi_socket_port, sock->conf.name);
-		sock->fd = sockets_create_socket(&sock->conf.al,
+		sock->fd = sockets_create_socket(&sock->conf.addr,
 		    sock->conf.fcgi_socket_port);
 		if (sock->fd == -1) {
-			log_warnx("%s: create unix socket failed", __func__);
+			log_warnx("%s: create FCGI socket failed", __func__);
 			return -1;
 		}
 	}
@@ -465,10 +474,9 @@ sockets_unix_socket_listen(struct privsep *ps, struct 
 }
 
 int
-sockets_create_socket(struct addresslist *al, in_port_t port)
+sockets_create_socket(struct address *a, in_port_t port)
 {
 	struct addrinfo hints;
-	struct address *a;
 	int fd = -1, o_val = 1, flags;
 
 	memset(&hints, 0, sizeof(hints));
@@ -476,53 +484,55 @@ sockets_create_socket(struct addresslist *al, in_port_
 	hints.ai_socktype = SOCK_STREAM;
 	hints.ai_flags |= AI_PASSIVE;
 
-	TAILQ_FOREACH(a, al, entry) {
-		switch (a->ss.ss_family) {
-		case AF_INET:
-			((struct sockaddr_in *)(&a->ss))->sin_port = port;
-			break;
-		case AF_INET6:
-			((struct sockaddr_in6 *)(&a->ss))->sin6_port = port;
-			break;
-		default:
-			log_warnx("%s: unknown address family", __func__);
-			goto fail;
-		}
+	switch (a->ss.ss_family) {
+	case AF_INET:
+		((struct sockaddr_in *)(&a->ss))->sin_port = port;
+		break;
+	case AF_INET6:
+		((struct sockaddr_in6 *)(&a->ss))->sin6_port = port;
+		break;
+	default:
+		log_warnx("%s: unknown address family", __func__);
+		return -1;
+	}
 
-		fd = socket(a->ss.ss_family, hints.ai_socktype,
-		    a->ipproto);
-			log_debug("%s: opening socket (%d) for %s", __func__,
-			    fd, a->ifname);
+	fd = socket(a->ss.ss_family, hints.ai_socktype, a->ipproto);
+	if (fd == -1)
+		return -1;
 
-		if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &o_val,
-		    sizeof(int)) == -1) {
-			log_warn("%s: setsockopt error", __func__);
-			return -1;
-		}
+	log_debug("%s: opened socket (%d) for %s", __func__,
+	    fd, a->ifname);
 
-		/* non-blocking */
-		flags = fcntl(fd, F_GETFL);
-		flags |= O_NONBLOCK;
-		fcntl(fd, F_SETFL, flags);
+	if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &o_val,
+	    sizeof(int)) == -1) {
+		log_warn("%s: setsockopt error", __func__);
+		close(fd);
+		return -1;
+	}
 
-		if (bind(fd, (struct sockaddr *)&a->ss, a->ss.ss_len) == -1) {
-			close(fd);
-			log_info("%s: can't bind to port %d", __func__,
-			    ntohs(port));
-			goto fail;
-		}
+	/* non-blocking */
+	flags = fcntl(fd, F_GETFL);
+	flags |= O_NONBLOCK;
+	if (fcntl(fd, F_SETFL, flags) == -1) {
+		log_info("%s: could not enable non-blocking I/O", __func__);
+		close(fd);
+		return -1;
+	}
 
-		if (listen(fd, SOMAXCONN) == -1) {
-			log_warn("%s, unable to listen on socket", __func__);
-			goto fail;
-		}
+	if (bind(fd, (struct sockaddr *)&a->ss, a->ss.ss_len) == -1) {
+		close(fd);
+		log_info("%s: can't bind to port %d", __func__,
+		    ntohs(port));
+		return -1;
 	}
 
-	free(a);
+	if (listen(fd, SOMAXCONN) == -1) {
+		log_warn("%s, unable to listen on socket", __func__);
+		close(fd);
+		return -1;
+	}
+
 	return (fd);
-fail:
-	free(a);
-	return -1;
 }
 
 int