Download raw body.
httpd: don't send the fastcgi param struct over imsg and tweaks
Based on the "custom header" idea and found issues.
Send one imsg per param with a small fixed header (fastcgi_param_imsg)
followed by the name and value bytes, and read it back with imsg_get_ibuf()
and ibuf_get_string(). The struct now uses dynamic buffer instead of fixed,
and the value limit is raised to 8192. Parse order is now preserved end-to-end
instead of being reversed twice. This follows the config order:
...
fcgi_add_param: HTTP_USER_AGENT[15] => curl/8.21.0[11], total_len: 189
fcgi_add_param: FCGI_TEST_A[11] => A[1], total_len: 217
fcgi_add_param: FCGI_TEST_B[11] => B[1], total_len: 231
fcgi_add_param: FCGI_TEST_C[11] => C[1], total_len: 245
fcgi_add_param: FCGI_TEST_D[11] => D[1], total_len: 259
fcgi_add_param: FCGI_TEST_E[11] => E[1], total_len: 273
fcgi_add_param: FCGI_TEST_BB[12] => BB[2], total_len: 287
...
This diff not include the change in "httpd: do NOT send location
fcgiparams ...": https://marc.info/?l=openbsd-tech&m=178397190636339&w=2
Feedback, OK?
Rafael
diff --git a/config.c b/config.c
index 146ed2c..ec1ec26 100644
--- a/config.c
+++ b/config.c
@@ -348,67 +348,52 @@ config_settls(struct httpd *env, struct server *srv, enum tls_config_type type,
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) {
- log_debug("%s: invalid message length", __func__);
+ struct server_config *srv_conf;
+ struct fastcgi_param *fp;
+ struct fastcgi_param_imsg fpmsg;
+ struct ibuf ibuf;
+
+ if (imsg_get_ibuf(imsg, &ibuf) == -1 ||
+ ibuf_get(&ibuf, &fpmsg, sizeof(fpmsg)) == -1) {
+ log_debug("%s: invalid message", __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) {
- log_debug("%s: invalid message length", __func__);
+ if ((srv_conf = serverconfig_byid(fpmsg.id)) == NULL) {
+ log_debug("%s: invalid config id", __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;
- }
- }
+ if (fpmsg.namelen > HTTPD_FCGI_NAME_MAX - 1 ||
+ fpmsg.vallen > HTTPD_FCGI_VAL_MAX - 1) {
+ log_debug("%s: fastcgi_param too long", __func__);
+ return (-1);
}
- /* 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);
+ if ((fp = calloc(1, sizeof(*fp))) == NULL)
+ fatal("fastcgi_param out of memory");
- p += sizeof(*fp);
+ fp->name = ibuf_get_string(&ibuf, fpmsg.namelen);
+ fp->value = ibuf_get_string(&ibuf, fpmsg.vallen);
+ if (fp->name == NULL || fp->value == NULL) {
+ free(fp->name);
+ free(fp->value);
+ free(fp);
+ return (-1);
}
+ TAILQ_INSERT_TAIL(&srv_conf->fcgiparams, fp, entry);
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;
+ struct privsep *ps = env->sc_ps;
+ struct server_config *srv_conf = &srv->srv_conf;
+ struct fastcgi_param *fp;
+ struct fastcgi_param_imsg fpmsg;
+ struct iovec iov[3];
DPRINTF("%s: sending fcgiparam for \"%s[%u]\" to %s fd %d", __func__,
srv_conf->name, srv_conf->id, ps->ps_title[PROC_SERVER],
@@ -418,28 +403,24 @@ config_setserver_fcgiparams(struct httpd *env, struct server *srv)
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);
- free(iov);
- return (-1);
+ fpmsg.id = srv_conf->id;
+ fpmsg.namelen = strlen(fp->name);
+ fpmsg.vallen = strlen(fp->value);
+
+ iov[0].iov_base = &fpmsg;
+ iov[0].iov_len = sizeof(fpmsg);
+ iov[1].iov_base = fp->name;
+ iov[1].iov_len = fpmsg.namelen;
+ iov[2].iov_base = fp->value;
+ iov[2].iov_len = fpmsg.vallen;
+
+ if (proc_composev(ps, PROC_SERVER, IMSG_CFG_FCGI, iov, 3)
+ != 0) {
+ log_warn("%s: failed to compose IMSG_CFG_FCGI "
+ "for `%s'", __func__, srv_conf->name);
+ return (-1);
+ }
}
- free(iov);
-
return (0);
}
@@ -727,6 +708,8 @@ config_getserver(struct httpd *env, struct imsg *imsg)
memcpy(&srv->srv_conf, &srv_conf, sizeof(srv->srv_conf));
srv->srv_s = fd;
+ TAILQ_INIT(&srv->srv_conf.fcgiparams);
+
if (config_getserver_auth(env, &srv->srv_conf) != 0)
goto fail;
diff --git a/httpd.h b/httpd.h
index 740721e..3cce5d1 100644
--- a/httpd.h
+++ b/httpd.h
@@ -65,8 +65,8 @@
#define HTTPD_TLS_CIPHERS "secure"
#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 HTTPD_FCGI_NAME_MAX 512
+#define HTTPD_FCGI_VAL_MAX 8192
#define FD_RESERVE 5
#define SERVER_MAX_CLIENTS 1024
@@ -440,9 +440,15 @@ struct server_tls_ticket {
unsigned char tt_key[TLS_TICKET_KEY_SIZE];
};
+struct fastcgi_param_imsg {
+ uint32_t id; /* server conf id */
+ uint16_t namelen;
+ uint16_t vallen;
+};
+
struct fastcgi_param {
- char name[HTTPD_FCGI_NAME_MAX];
- char value[HTTPD_FCGI_VAL_MAX];
+ char *name;
+ char *value;
TAILQ_ENTRY(fastcgi_param) entry;
};
diff --git a/parse.y b/parse.y
index 65664e1..257ecb7 100644
--- a/parse.y
+++ b/parse.y
@@ -813,29 +813,33 @@ fcgiflags : SOCKET STRING {
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");
+ if (strlen($2) > HTTPD_FCGI_NAME_MAX - 1) {
+ yyerror("fastcgi param name too long (max %d)",
+ HTTPD_FCGI_NAME_MAX - 1);
free($2);
free($3);
- free(param);
YYERROR;
}
- if (strlcpy(param->value, $3, sizeof(param->value)) >=
- sizeof(param->value)) {
- yyerror("fastcgi_param value truncated");
+
+ if (strlen($3) > HTTPD_FCGI_VAL_MAX - 1) {
+ yyerror("fastcgi param value too long (max %d)",
+ HTTPD_FCGI_VAL_MAX - 1);
free($2);
free($3);
- free(param);
YYERROR;
}
+
+ if ((param->name = strdup($2)) == NULL ||
+ (param->value = strdup($3)) == NULL)
+ fatal("out of memory");
+
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);
+ TAILQ_INSERT_TAIL(&srv_conf->fcgiparams, param, entry);
}
| STRIP NUMBER {
if ($2 < 0 || $2 > INT_MAX) {
diff --git a/server.c b/server.c
index 0731730..a12d6f9 100644
--- a/server.c
+++ b/server.c
@@ -483,8 +483,11 @@ serverconfig_free(struct server_config *srv_conf)
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)
+ TAILQ_FOREACH_SAFE(param, &srv_conf->fcgiparams, entry, tparam) {
+ free(param->name);
+ free(param->value);
free(param);
+ }
}
void
httpd: don't send the fastcgi param struct over imsg and tweaks