On Mon, Feb 18, 2019 at 08:05:14PM +0100, Florian Obser wrote: > On Mon, Feb 18, 2019 at 12:17:31PM +0200, Paul Irofti wrote: > > On Sun, Feb 17, 2019 at 11:57:20AM +0100, Florian Obser wrote: > > > On Tue, Feb 12, 2019 at 05:02:47PM +0200, Paul Irofti wrote: > > > > Hi, > > > > > > > > This patch adds support for FastCGI parameters. > > > > Grammar change is very simple. > > > > > > > > fastcgi param NAME VALUE > > > > > > > > Example: > > > > %------------------------------------------------------------------------------ > > > > server "default" { > > > > listen on $ext_addr port 80 > > > > > > > > location "/cgi-bin/*" { > > > > fastcgi socket "/run/slowcgi.sock" > > > > fastcgi param TEST1 hello > > > > fastcgi param TEST2 world > > > > > > > > # The /cgi-bin directory is outside of the document root > > > > root "/" > > > > } > > > > } > > > > %------------------------------------------------------------------------------ > > > > > > > > Output: > > > > %------------------------------------------------------------------------------ > > > > CGI/1.1 test script report: > > > > > > > > GATEWAY_INTERFACE = CGI/1.1 > > > > SERVER_SOFTWARE = OpenBSD httpd > > > > SERVER_PROTOCOL = HTTP/1.1 > > > > SERVER_NAME = default > > > > SERVER_ADDR = 127.0.0.1 > > > > SERVER_PORT = 80 > > > > REQUEST_METHOD = GET > > > > HTTP_ACCEPT = > > > > text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8 > > > > SCRIPT_NAME = /cgi-bin/testcgi > > > > REQUEST_URI = /cgi-bin/testcgi > > > > PATH_INFO = > > > > QUERY_STRING = > > > > REMOTE_ADDR = 127.0.0.1 > > > > REMOTE_PORT = 28054 > > > > CONTENT_TYPE = > > > > CONTENT_LENGTH = > > > > TEST1 = hello > > > > TEST2 = world > > > > PATH = > > > > %------------------------------------------------------------------------------ > > > > > > > > Initially discussed with florian@. > > > > Thoughts? Silly bike-shedding about the grammar? OKs? > > > > > > > > Manual page changes after grammar is OK'ed. > > > > > > I'm mostly OK with it, config_getserver_fcgiparams() needs a bit of > > > polishing, see inline. > > > > Like this? I did the message length check in two steps as I don't know > > how long it will be until I read how many items were sent. > > kinda, except you can't do it like this, see inline.
Heh, silly me. Thanks for catching that! I also added the manual bits in my commit. If you dislike them let me know and I will change them or go ahead and modify them. Thanks again for your review! > > > > > > > Index: config.c > > =================================================================== > > RCS file: /cvs/src/usr.sbin/httpd/config.c,v > > retrieving revision 1.55 > > diff -u -p -u -p -r1.55 config.c > > --- config.c 20 Jun 2018 16:43:05 -0000 1.55 > > +++ config.c 18 Feb 2019 10:15:22 -0000 > > @@ -231,6 +231,9 @@ config_setserver(struct httpd *env, stru > > __func__, srv->srv_conf.name); > > return (-1); > > } > > + > > + /* Configure FCGI parmeters if necessary. */ > > + config_setserver_fcgiparams(env, srv); > > } > > } > > > > @@ -289,6 +292,102 @@ config_settls(struct httpd *env, struct > > tls.tls_chunk_offset += tls.tls_chunk_len; > > data += tls.tls_chunk_len; > > len -= tls.tls_chunk_len; > > + } > > + > > + return (0); > > +} > > + > > +int > > +config_getserver_fcgiparams(struct httpd *env, struct imsg *imsg) > > +{ > > + struct server *srv; > > + struct server_config *srv_conf, *iconf; > > + struct fastcgi_param *fp; > > + uint32_t id; > > + size_t c, nc, len; > > + uint8_t *p = imsg->data; > > + > > + len = sizeof(nc) + sizeof(id); > > + if ((IMSG_DATA_SIZE(imsg) - len) < 0) { > > these are all unsigned data types, this is always false > > please write it like this: > if (IMSG_DATA_SIZE(imsg) < len) { > > > + log_debug("%s: invalid message length", __func__); > > + return (-1); > > + } > > + > > + memcpy(&nc, p, sizeof(nc)); /* number of params */ > > + p += sizeof(nc); > > + > > + memcpy(&id, p, sizeof(id)); /* server conf id */ > > + srv_conf = serverconfig_byid(id); > > + p += sizeof(id); > > + > > + len += nc*sizeof(*fp); > > + if ((IMSG_DATA_SIZE(imsg) - len) < 0) { > > and here. > With that fixed OK florian@ > > > + log_debug("%s: invalid message length", __func__); > > + return (-1); > > + } > > + > > + /* Find associated server config */ > > + TAILQ_FOREACH(srv, env->sc_servers, srv_entry) { > > + if (srv->srv_conf.id == id) { > > + srv_conf = &srv->srv_conf; > > + break; > > + } > > + TAILQ_FOREACH(iconf, &srv->srv_hosts, entry) { > > + if (iconf->id == id) { > > + srv_conf = iconf; > > + break; > > + } > > + } > > + } > > + > > + /* Fetch FCGI parameters */ > > + for (c = 0; c < nc; c++) { > > + if ((fp = calloc(1, sizeof(*fp))) == NULL) > > + fatalx("fcgiparams out of memory"); > > + memcpy(fp, p, sizeof(*fp)); > > + TAILQ_INSERT_HEAD(&srv_conf->fcgiparams, fp, entry); > > + > > + p += sizeof(*fp); > > + } > > + > > + return (0); > > +} > > + > > +int > > +config_setserver_fcgiparams(struct httpd *env, struct server *srv) > > +{ > > + struct privsep *ps = env->sc_ps; > > + struct server_config *srv_conf = &srv->srv_conf; > > + struct fastcgi_param *fp; > > + struct iovec *iov; > > + size_t c = 0, nc = 0; > > + > > + DPRINTF("%s: sending fcgiparam for \"%s[%u]\" to %s fd %d", __func__, > > + srv_conf->name, srv_conf->id, ps->ps_title[PROC_SERVER], > > + srv->srv_s); > > + > > + if (TAILQ_EMPTY(&srv_conf->fcgiparams)) /* nothing to do */ > > + return (0); > > + > > + TAILQ_FOREACH(fp, &srv_conf->fcgiparams, entry) { > > + nc++; > > + } > > + if ((iov = calloc(nc + 2, sizeof(*iov))) == NULL) > > + return (-1); > > + > > + iov[c].iov_base = &nc; /* number of params */ > > + iov[c++].iov_len = sizeof(nc); > > + iov[c].iov_base = &srv_conf->id; /* server config id */ > > + iov[c++].iov_len = sizeof(srv_conf->id); > > + > > + TAILQ_FOREACH(fp, &srv_conf->fcgiparams, entry) { /* push FCGI > > params */ > > + iov[c].iov_base = fp; > > + iov[c++].iov_len = sizeof(*fp); > > + } > > + if (proc_composev(ps, PROC_SERVER, IMSG_CFG_FCGI, iov, c) != 0) { > > + log_warn("%s: failed to compose IMSG_CFG_FCGI imsg for " > > + "`%s'", __func__, srv_conf->name); > > + return (-1); > > } > > > > return (0); > > Index: httpd.h > > =================================================================== > > RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v > > retrieving revision 1.142 > > diff -u -p -u -p -r1.142 httpd.h > > --- httpd.h 11 Oct 2018 09:52:22 -0000 1.142 > > +++ httpd.h 18 Feb 2019 10:15:22 -0000 > > @@ -64,6 +64,8 @@ > > #define HTTPD_TLS_CIPHERS "compat" > > #define HTTPD_TLS_DHE_PARAMS "none" > > #define HTTPD_TLS_ECDHE_CURVES "default" > > +#define HTTPD_FCGI_NAME_MAX 511 > > +#define HTTPD_FCGI_VAL_MAX 511 > > #define FD_RESERVE 5 > > > > #define SERVER_MAX_CLIENTS 1024 > > @@ -225,6 +227,7 @@ enum imsg_type { > > IMSG_CFG_TLS, > > IMSG_CFG_MEDIA, > > IMSG_CFG_AUTH, > > + IMSG_CFG_FCGI, > > IMSG_CFG_DONE, > > IMSG_LOG_ACCESS, > > IMSG_LOG_ERROR, > > @@ -467,6 +470,14 @@ struct server_tls_ticket { > > unsigned char tt_key[TLS_TICKET_KEY_SIZE]; > > }; > > > > +struct fastcgi_param { > > + char name[HTTPD_FCGI_NAME_MAX]; > > + char value[HTTPD_FCGI_VAL_MAX]; > > + > > + TAILQ_ENTRY(fastcgi_param) entry; > > +}; > > +TAILQ_HEAD(server_fcgiparams, fastcgi_param); > > + > > struct server_config { > > uint32_t id; > > uint32_t parent_id; > > @@ -534,6 +545,8 @@ struct server_config { > > int hsts_max_age; > > uint8_t hsts_flags; > > > > + struct server_fcgiparams fcgiparams; > > + > > TAILQ_ENTRY(server_config) entry; > > }; > > TAILQ_HEAD(serverhosts, server_config); > > @@ -818,8 +831,10 @@ int config_getreset(struct httpd *, str > > int config_getcfg(struct httpd *, struct imsg *); > > int config_setserver(struct httpd *, struct server *); > > int config_setserver_tls(struct httpd *, struct server *); > > +int config_setserver_fcgiparams(struct httpd *, struct server *); > > int config_getserver(struct httpd *, struct imsg *); > > int config_getserver_tls(struct httpd *, struct imsg *); > > +int config_getserver_fcgiparams(struct httpd *, struct imsg *); > > int config_setmedia(struct httpd *, struct media_type *); > > int config_getmedia(struct httpd *, struct imsg *); > > int config_setauth(struct httpd *, struct auth *); > > Index: parse.y > > =================================================================== > > RCS file: /cvs/src/usr.sbin/httpd/parse.y,v > > retrieving revision 1.108 > > diff -u -p -u -p -r1.108 parse.y > > --- parse.y 8 Jan 2019 18:35:27 -0000 1.108 > > +++ parse.y 18 Feb 2019 10:15:22 -0000 > > @@ -140,7 +140,7 @@ typedef struct { > > %token PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG > > TCP TICKET > > %token TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD > > REQUEST > > %token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE > > -%token CA CLIENT CRL OPTIONAL > > +%token CA CLIENT CRL OPTIONAL PARAM > > %token <v.string> STRING > > %token <v.number> NUMBER > > %type <v.port> port > > @@ -290,6 +290,7 @@ server : SERVER optmatch STRING { > > > > SPLAY_INIT(&srv->srv_clients); > > TAILQ_INIT(&srv->srv_hosts); > > + TAILQ_INIT(&srv_conf->fcgiparams); > > > > TAILQ_INSERT_TAIL(&srv->srv_hosts, srv_conf, entry); > > } '{' optnl serveropts_l '}' { > > @@ -658,6 +659,36 @@ fcgiflags : SOCKET STRING { > > free($2); > > srv_conf->flags |= SRVFLAG_SOCKET; > > } > > + | PARAM STRING STRING { > > + struct fastcgi_param *param; > > + > > + if ((param = calloc(1, sizeof(*param))) == NULL) > > + fatal("out of memory"); > > + > > + if (strlcpy(param->name, $2, sizeof(param->name)) >= > > + sizeof(param->name)) { > > + yyerror("fastcgi_param name truncated"); > > + free($2); > > + free($3); > > + free(param); > > + YYERROR; > > + } > > + if (strlcpy(param->value, $3, sizeof(param->value)) >= > > + sizeof(param->value)) { > > + yyerror("fastcgi_param value truncated"); > > + free($2); > > + free($3); > > + free(param); > > + YYERROR; > > + } > > + free($2); > > + free($3); > > + > > + DPRINTF("[%s,%s,%d]: adding param \"%s\" value \"%s\"", > > + srv_conf->location, srv_conf->name, srv_conf->id, > > + param->name, param->value); > > + TAILQ_INSERT_HEAD(&srv_conf->fcgiparams, param, entry); > > + } > > ; > > > > connection : CONNECTION '{' optnl conflags_l '}' > > @@ -1282,6 +1313,7 @@ lookup(char *s) > > { "ocsp", OCSP }, > > { "on", ON }, > > { "optional", OPTIONAL }, > > + { "param", PARAM }, > > { "pass", PASS }, > > { "port", PORT }, > > { "prefork", PREFORK }, > > Index: server.c > > =================================================================== > > RCS file: /cvs/src/usr.sbin/httpd/server.c,v > > retrieving revision 1.117 > > diff -u -p -u -p -r1.117 server.c > > --- server.c 8 Jan 2019 18:35:27 -0000 1.117 > > +++ server.c 18 Feb 2019 10:15:23 -0000 > > @@ -491,6 +491,8 @@ server_purge(struct server *srv) > > void > > serverconfig_free(struct server_config *srv_conf) > > { > > + struct fastcgi_param *param, *tparam; > > + > > free(srv_conf->return_uri); > > free(srv_conf->tls_ca_file); > > free(srv_conf->tls_ca); > > @@ -502,6 +504,9 @@ serverconfig_free(struct server_config * > > free(srv_conf->tls_ocsp_staple); > > freezero(srv_conf->tls_cert, srv_conf->tls_cert_len); > > freezero(srv_conf->tls_key, srv_conf->tls_key_len); > > + > > + TAILQ_FOREACH_SAFE(param, &srv_conf->fcgiparams, entry, tparam) > > + free(param); > > } > > > > void > > @@ -519,6 +524,7 @@ serverconfig_reset(struct server_config > > srv_conf->tls_key_file = NULL; > > srv_conf->tls_ocsp_staple = NULL; > > srv_conf->tls_ocsp_staple_file = NULL; > > + TAILQ_INIT(&srv_conf->fcgiparams); > > } > > > > struct server * > > @@ -1369,6 +1375,9 @@ server_dispatch_parent(int fd, struct pr > > break; > > case IMSG_CFG_TLS: > > config_getserver_tls(httpd_env, imsg); > > + break; > > + case IMSG_CFG_FCGI: > > + config_getserver_fcgiparams(httpd_env, imsg); > > break; > > case IMSG_CFG_DONE: > > config_getcfg(httpd_env, imsg); > > Index: server_fcgi.c > > =================================================================== > > RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v > > retrieving revision 1.77 > > diff -u -p -u -p -r1.77 server_fcgi.c > > --- server_fcgi.c 15 Oct 2018 08:16:17 -0000 1.77 > > +++ server_fcgi.c 18 Feb 2019 10:15:23 -0000 > > @@ -92,6 +92,7 @@ server_fcgi(struct httpd *env, struct cl > > struct http_descriptor *desc = clt->clt_descreq; > > struct fcgi_record_header *h; > > struct fcgi_begin_request_body *begin; > > + struct fastcgi_param *fcgiparam; > > char hbuf[HOST_NAME_MAX+1]; > > size_t scriptlen; > > int pathlen; > > @@ -290,6 +291,13 @@ server_fcgi(struct httpd *env, struct cl > > if (srv_conf->tls_flags != 0 && fcgi_add_param(¶m, > > "TLS_PEER_VERIFY", printb_flags(srv_conf->tls_flags, > > TLSFLAG_BITS), clt) == -1) { > > + errstr = "failed to encode param"; > > + goto fail; > > + } > > + } > > + > > + TAILQ_FOREACH(fcgiparam, &srv_conf->fcgiparams, entry) { > > + if (fcgi_add_param(¶m, fcgiparam->name, fcgiparam->value, > > clt) == -1) { > > errstr = "failed to encode param"; > > goto fail; > > } > > -- > I'm not entirely sure you are real.