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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: simplify gotwebd server matching and fix manpage lie
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 22 Jun 2023 15:53:26 +0200

Download raw body.

Thread
On Mon, Jun 19, 2023 at 06:41:54PM +0200, Omar Polo wrote:
> This is an attempt at simplifying how gotwebd select the server to
> use.
> 
> Currently, if there is not a match on the server name, it attempt to
> match the "subdomain" (the first component of the URL, e.g. "git"
> should the Host header be "git.example.com") against the server names,
> and falls back to the first server defined.
> 
> The server name is taken from the SERVER_NAME fastcgi parameter, the
> subdomain by taking the first component of the HTTP_HOST, i.e. "www"
> should the host be "www.example.com", but also "example" if the host
> name is "example.com", which I find a bit confusing.
> 
> Finally, updates the manpage explaning how the matching works.  I'm
> dropping the lie about SNI as gotwebd doesn't do TLS: it's the server'
> responsability to do TLS and check SNI, not a fastcgi application one.
> gotwebd only looks at the SERVER_NAME (and currently at part of
> HTTP_HOST).

Is this related deeply to the httpd diff you sent to tech@?
Would gotwebd depend on -current httpd if this gets committed?
I doubt it, but just wanted to make sure.

In any case, I prefer the simpler matching rules, so ok by me.

> diff /home/op/w/got
> commit - 3bf0e21f50b11c683f08a06c8ab362fe220adc2b
> path + /home/op/w/got
> blob - 439dc44795355724c9f4c3a27e86dd2db4e3539b
> file + gotwebd/fcgi.c
> --- gotwebd/fcgi.c
> +++ gotwebd/fcgi.c
> @@ -182,7 +182,7 @@ fcgi_parse_params(uint8_t *buf, uint16_t n, struct req
>  fcgi_parse_params(uint8_t *buf, uint16_t n, struct request *c, uint16_t id)
>  {
>  	uint32_t name_len, val_len;
> -	uint8_t *sd, *val;
> +	uint8_t *val;
>  
>  	if (!c->request_started) {
>  		log_warn("FCGI_PARAMS without FCGI_BEGIN_REQUEST, ignoring");
> @@ -245,23 +245,6 @@ fcgi_parse_params(uint8_t *buf, uint16_t n, struct req
>  			c->querystring[val_len] = '\0';
>  		}
>  
> -		if (c->http_host[0] == '\0' &&
> -		    val_len < GOTWEBD_MAXTEXT &&
> -		    name_len == 9 &&
> -		    strncmp(buf, "HTTP_HOST", 9) == 0) {
> -			memcpy(c->http_host, val, val_len);
> -			c->http_host[val_len] = '\0';
> -
> -			/*
> -			 * lazily get subdomain
> -			 * will only get domain if no subdomain exists
> -			 * this can still work if gotweb server name is the same
> -			 */
> -			sd = strchr(c->http_host, '.');
> -			if (sd)
> -				*sd = '\0';
> -		}
> -
>  		if (c->document_uri[0] == '\0' &&
>  		    val_len < MAX_DOCUMENT_URI &&
>  		    name_len == 12 &&
> blob - c775066084109268a1ca7a0f0a2c8cf87ecb209a
> file + gotwebd/gotweb.c
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -98,7 +98,7 @@ struct server *gotweb_get_server(uint8_t *, uint8_t *)
>  static void gotweb_free_querystring(struct querystring *);
>  static void gotweb_free_repo_dir(struct repo_dir *);
>  
> -struct server *gotweb_get_server(uint8_t *, uint8_t *);
> +struct server *gotweb_get_server(const char *);
>  
>  static int
>  gotweb_reply(struct request *c, int status, const char *ctype,
> @@ -162,7 +162,7 @@ gotweb_process_request(struct request *c)
>  	if (c->sock->client_status == CLIENT_DISCONNECT)
>  		return;
>  	/* get the gotwebd server */
> -	srv = gotweb_get_server(c->server_name, c->http_host);
> +	srv = gotweb_get_server(c->server_name);
>  	if (srv == NULL) {
>  		log_warnx("%s: error server is NULL", __func__);
>  		goto err;
> @@ -387,28 +387,18 @@ gotweb_get_server(uint8_t *server_name, uint8_t *subdo
>  }
>  
>  struct server *
> -gotweb_get_server(uint8_t *server_name, uint8_t *subdomain)
> +gotweb_get_server(const char *server_name)
>  {
> -	struct server *srv = NULL;
> +	struct server *srv;
>  
>  	/* check against the server name first */
>  	if (*server_name != '\0')
>  		TAILQ_FOREACH(srv, &gotwebd_env->servers, entry)
>  			if (strcmp(srv->name, server_name) == 0)
> -				goto done;
> +				return srv;
>  
> -	/* check against subdomain second */
> -	if (*subdomain != '\0')
> -		TAILQ_FOREACH(srv, &gotwebd_env->servers, entry)
> -			if (strcmp(srv->name, subdomain) == 0)
> -				goto done;
> -
> -	/* if those fail, send first server */
> -	TAILQ_FOREACH(srv, &gotwebd_env->servers, entry)
> -		if (srv != NULL)
> -			break;
> -done:
> -	return srv;
> +	/* otherwise, use the first server */
> +	return TAILQ_FIRST(&gotwebd_env->servers);
>  };
>  
>  const struct got_error *
> blob - 765208e819180a987e11496151b42a2800b2aed9
> file + gotwebd/gotwebd.conf.5
> --- gotwebd/gotwebd.conf.5
> +++ gotwebd/gotwebd.conf.5
> @@ -80,13 +80,8 @@ followed by server-specific configuration directives i
>  .Pp
>  .Ic server Ar name Brq ...
>  .Pp
> -.Xr gotwebd 8
> -is compatible with TLS Server Name Indication (SNI), provided the
> -.Ar name
> -of a server defined in
> -.Nm
> -corresponds to the name of a server defined in
> -.Xr httpd.conf 5 .
> +The first server defined is used if the requested hostname is not
> +matched by any server block.
>  .Pp
>  The available server configuration directives are as follows:
>  .Bl -tag -width Ds
> blob - 31bcbde9268d51c10f8ed32e6080e1691e0473ec
> file + gotwebd/gotwebd.h
> --- gotwebd/gotwebd.h
> +++ gotwebd/gotwebd.h
> @@ -243,7 +243,6 @@ struct request {
>  	size_t				 outbuf_len;
>  
>  	char				 querystring[MAX_QUERYSTRING];
> -	char				 http_host[GOTWEBD_MAXTEXT];
>  	char				 document_uri[MAX_DOCUMENT_URI];
>  	char				 server_name[MAX_SERVER_NAME];
>  	int				 https;
> 
>