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.

> 
> 
> 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(&param,
>                   "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(&param, fcgiparam->name, fcgiparam->value, 
> clt) == -1) {
>                       errstr = "failed to encode param";
>                       goto fail;
>               }

-- 
I'm not entirely sure you are real.

Reply via email to