Index | Thread | Search

From:
Lloyd <ng2d68@proton.me>
Subject:
Re: httpd(8): patch to disable/modify the server banner
To:
"tech@openbsd.org" <tech@openbsd.org>
Date:
Mon, 07 Apr 2025 18:51:22 +0000

Download raw body.

Thread
Hi tech@

Any further interest in testing this patch?

FWIW it also fixes a bug in httpd's debugging output.

Regards
Lloyd

On Saturday, February 15th, 2025, Kirill A. Korinsky wrote:

> On Wed, 05 Feb 2025 21:38:56 +0100,
> Lloyd wrote:
> 
> > Stuart Henderson wrote:
> > 
> > > "disablebanner" and especially "no disablebanner" is not nice syntax.
> > > considering other config options, "[no] banner" would be idiomatic.
> > 
> > Hi, I reworked this patch option parsing as Stu suggested:
> 
> 
> Lightly tested and I'd like to confirm what it works as advertised.
> 
> OK kirill@
> 
> > Index: config.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/httpd/config.c,v
> > retrieving revision 1.65
> > diff -u -p -u -p -r1.65 config.c
> > --- config.c 17 Jan 2024 08:22:40 -0000 1.65
> > +++ config.c 5 Feb 2025 20:22:29 -0000
> > @@ -591,6 +591,8 @@ config_getserver_config(struct httpd *en
> > srv_conf->flags |= parent->flags & SRVFLAG_ERRDOCS;
> > (void)strlcpy(srv_conf->errdocroot, parent->errdocroot,
> > sizeof(srv_conf->errdocroot));
> > +
> > + srv_conf->flags |= parent->flags & SRVFLAG_NO_BANNER;
> > 
> > DPRINTF("%s: %s %d location \"%s\", "
> > "parent \"%s[%u]\", flags: %s",
> > Index: httpd.conf.5
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> > retrieving revision 1.125
> > diff -u -p -u -p -r1.125 httpd.conf.5
> > --- httpd.conf.5 3 Nov 2023 13:03:02 -0000 1.125
> > +++ httpd.conf.5 5 Feb 2025 20:22:29 -0000
> > @@ -166,6 +166,14 @@ If not specified, it defaults to
> > within the
> > .Xr chroot 2
> > directory.
> > +.It Ic no banner
> > +Do not send the
> > +.Va Server
> > +HTTP response header, and hide the server software name in error documents.
> > +The
> > +.Va SERVER_SOFTWARE
> > +CGI environment variable is always set in accordance with
> > +.%R RFC 3875 .
> > .It Ic prefork Ar number
> > Run the specified number of server processes.
> > This increases the performance and prevents delays when connecting
> > @@ -227,6 +235,20 @@ and must be readable by the www user.
> > Use the
> > .Ic no authenticate
> > directive to disable authentication in a location.
> > +.It Oo Ic no Oc Ic banner
> > +When prefixed with the
> > +.Ic no
> > +keyword,
> > +suppress the
> > +.Va Server
> > +HTTP response header, and hide the server software name in error documents for
> > +the current
> > +.Ic server.
> > +If
> > +.Ic no
> > +is omitted, enable the banner for the current
> > +.Ic server
> > +if it was disabled globally.
> > .It Ic block drop
> > Drop the connection without sending an error page.
> > .It Ic block Op Ic return Ar code Op Ar uri
> > Index: httpd.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> > retrieving revision 1.165
> > diff -u -p -u -p -r1.165 httpd.h
> > --- httpd.h 8 Oct 2024 05:28:11 -0000 1.165
> > +++ httpd.h 5 Feb 2025 20:22:29 -0000
> > @@ -389,6 +389,7 @@ SPLAY_HEAD(client_tree, client);
> > #define SRVFLAG_PATH_REWRITE 0x01000000
> > #define SRVFLAG_NO_PATH_REWRITE 0x02000000
> > #define SRVFLAG_GZIP_STATIC 0x04000000
> > +#define SRVFLAG_NO_BANNER 0x08000000
> > #define SRVFLAG_LOCATION_FOUND 0x40000000
> > #define SRVFLAG_LOCATION_NOT_FOUND 0x80000000
> > 
> > @@ -398,7 +399,7 @@ SPLAY_HEAD(client_tree, client);
> > "\14SYSLOG\15NO_SYSLOG\16TLS\17ACCESS_LOG\20ERROR_LOG" \
> > "\21AUTH\22NO_AUTH\23BLOCK\24NO_BLOCK\25LOCATION_MATCH" \
> > "\26SERVER_MATCH\27SERVER_HSTS\30DEFAULT_TYPE\31PATH\32NO_PATH" \
> > - "\37LOCATION_FOUND\40LOCATION_NOT_FOUND"
> > + "\33GZIP_STATIC\34NO_BANNER\37LOCATION_FOUND\40LOCATION_NOT_FOUND"
> > 
> > #define TCPFLAG_NODELAY 0x01
> > #define TCPFLAG_NNODELAY 0x02
> > Index: parse.y
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> > retrieving revision 1.128
> > diff -u -p -u -p -r1.128 parse.y
> > --- parse.y 27 Feb 2022 20:30:30 -0000 1.128
> > +++ parse.y 5 Feb 2025 20:22:29 -0000
> > @@ -141,9 +141,9 @@ typedef struct {
> > %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 PARAM FORWARDED FOUND NOT
> > -%token ERRDOCS GZIPSTATIC
> > +%token ERRDOCS GZIPSTATIC BANNER
> > %token <v.string> STRING
> > -%token <v.number> NUMBER
> > +%token <v.number> NUMBER
> > %type <v.port> port
> > %type <v.string> fcgiport
> > %type <v.number> opttls optmatch optfound
> > @@ -227,6 +227,9 @@ main : PREFORK NUMBER {
> > | LOGDIR STRING {
> > conf->sc_logdir = $2;
> > }
> > + | NO BANNER {
> > + conf->sc_flags |= SRVFLAG_NO_BANNER;
> > + }
> > | DEFAULT TYPE mediastring {
> > memcpy(&conf->sc_default_type, &media,
> > sizeof(struct media_type));
> > @@ -300,6 +303,9 @@ server : SERVER optmatch STRING {
> > 
> > s->srv_conf.hsts_max_age = SERVER_HSTS_DEFAULT_AGE;
> > 
> > + if (conf->sc_flags & SRVFLAG_NO_BANNER)
> > + s->srv_conf.flags |= SRVFLAG_NO_BANNER;
> > +
> > (void)strlcpy(s->srv_conf.errdocroot,
> > conf->sc_errdocroot,
> > sizeof(s->srv_conf.errdocroot));
> > @@ -550,6 +556,7 @@ serveroptsl : LISTEN ON STRING opttls po
> > | request
> > | root
> > | directory
> > + | banner
> > | logformat
> > | fastcgi
> > | authenticate
> > @@ -687,6 +694,22 @@ serveroptsl : LISTEN ON STRING opttls po
> > }
> > ;
> > 
> > +banner : BANNER {
> > + if (parentsrv != NULL) {
> > + yyerror("banner inside location");
> > + YYERROR;
> > + }
> > + srv->srv_conf.flags &= ~SRVFLAG_NO_BANNER;
> > + }
> > + | NO BANNER {
> > + if (parentsrv != NULL) {
> > + yyerror("no banner inside location");
> > + YYERROR;
> > + }
> > + srv->srv_conf.flags |= SRVFLAG_NO_BANNER;
> > + }
> > + ;
> > +
> > optfound : /* empty */ { $$ = 0; }
> > | FOUND { $$ = 1; }
> > | NOT FOUND { $$ = -1; }
> > @@ -1428,6 +1451,7 @@ lookup(char *s)
> > { "authenticate", AUTHENTICATE},
> > { "auto", AUTO },
> > { "backlog", BACKLOG },
> > + { "banner", BANNER },
> > { "block", BLOCK },
> > { "body", BODY },
> > { "buffer", BUFFER },
> > Index: server_fcgi.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> > retrieving revision 1.97
> > diff -u -p -u -p -r1.97 server_fcgi.c
> > --- server_fcgi.c 8 Nov 2023 19:19:10 -0000 1.97
> > +++ server_fcgi.c 5 Feb 2025 20:22:29 -0000
> > @@ -345,6 +345,7 @@ server_fcgi(struct httpd *env, struct cl
> > goto fail;
> > }
> > 
> > + /* RFC 3875 requires this variable always be present */
> > if (fcgi_add_param(&param, "SERVER_SOFTWARE", HTTPD_SERVERNAME,
> > clt) == -1) {
> > errstr = "failed to encode param";
> > @@ -659,9 +660,12 @@ server_fcgi_header(struct client *clt, u
> > kv_set(&resp->http_pathquery, "%s", error) == -1)
> > return (-1);
> > 
> > - /* Add headers /
> > - if (kv_add(&resp->http_headers, "Server", HTTPD_SERVERNAME) == NULL)
> > - return (-1);
> > + / Add server banner header to response unless suppressed */
> > + if ((srv_conf->flags & SRVFLAG_NO_BANNER) == 0) {
> > + if (kv_add(&resp->http_headers, "Server",
> > + HTTPD_SERVERNAME) == NULL)
> > + return (-1);
> > + }
> > 
> > if (clt->clt_fcgi.type == FCGI_END_REQUEST ||
> > EVBUFFER_LENGTH(clt->clt_srvevb) == 0) {
> > Index: server_http.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> > retrieving revision 1.155
> > diff -u -p -u -p -r1.155 server_http.c
> > --- server_http.c 22 Dec 2024 13:51:42 -0000 1.155
> > +++ server_http.c 5 Feb 2025 20:22:29 -0000
> > @@ -890,6 +890,7 @@ server_abort_http(struct client *clt, un
> > char *httpmsg, *body = NULL, *extraheader = NULL;
> > char tmbuf[32], hbuf[128], *hstsheader = NULL;
> > char *clenheader = NULL;
> > + char *bannerheader = NULL, *bannertoken = NULL;
> > char buf[IBUF_READ_SIZE];
> > char *escapedmsg = NULL;
> > char cstr[5];
> > @@ -981,7 +982,11 @@ server_abort_http(struct client *clt, un
> > 
> > body = replace_var(body, "$HTTP_ERROR", httperr);
> > body = replace_var(body, "$RESPONSE_CODE", cstr);
> > - body = replace_var(body, "$SERVER_SOFTWARE", HTTPD_SERVERNAME);
> > + /* Check if server banner is suppressed */
> > + if ((srv_conf->flags & SRVFLAG_NO_BANNER) == 0)
> > + body = replace_var(body, "$SERVER_SOFTWARE", HTTPD_SERVERNAME);
> > + else
> > + body = replace_var(body, "$SERVER_SOFTWARE", "");
> > bodylen = strlen(body);
> > goto send;
> > 
> > @@ -994,6 +999,11 @@ server_abort_http(struct client *clt, un
> > "body { background-color: #1E1F21; color: #EEEFF1; }\n"
> > "a { color: #BAD7FF; }\n}";
> > 
> > + /* If banner is suppressed, don't write it to the error document /
> > + if ((srv_conf->flags & SRVFLAG_NO_BANNER) == 0)
> > + asprintf(&bannertoken, "<hr>\n<address>%s</address>\n",
> > + HTTPD_SERVERNAME);
> > +
> > / Generate simple HTML error document */
> > if ((bodylen = asprintf(&body,
> > "<!DOCTYPE html>\n"
> > @@ -1005,10 +1015,11 @@ server_abort_http(struct client *clt, un
> > "</head>\n"
> > "<body>\n"
> > "<h1>%03d %s</h1>\n"
> > - "<hr>\n<address>%s</address>\n"
> > + "%s"
> > "</body>\n"
> > "</html>\n",
> > - code, httperr, style, code, httperr, HTTPD_SERVERNAME)) == -1) {
> > + code, httperr, style, code, httperr,
> > + bannertoken == NULL ? "" : bannertoken)) == -1) {
> > body = NULL;
> > goto done;
> > }
> > @@ -1037,11 +1048,16 @@ server_abort_http(struct client *clt, un
> > }
> > }
> > 
> > + /* If banner is suppressed, don't write a Server header /
> > + if ((srv_conf->flags & SRVFLAG_NO_BANNER) == 0)
> > + asprintf(&bannerheader, "Server: %s\r\n",
> > + HTTPD_SERVERNAME);
> > +
> > / Add basic HTTP headers */
> > if (asprintf(&httpmsg,
> > "HTTP/1.0 %03d %s\r\n"
> > "Date: %s\r\n"
> > - "Server: %s\r\n"
> > + "%s"
> > "Connection: close\r\n"
> > "Content-Type: text/html\r\n"
> > "%s"
> > @@ -1049,7 +1065,8 @@ server_abort_http(struct client *clt, un
> > "%s"
> > "\r\n"
> > "%s",
> > - code, httperr, tmbuf, HTTPD_SERVERNAME,
> > + code, httperr, tmbuf,
> > + bannerheader == NULL ? "" : bannerheader,
> > clenheader == NULL ? "" : clenheader,
> > extraheader == NULL ? "" : extraheader,
> > hstsheader == NULL ? "" : hstsheader,
> > @@ -1558,10 +1575,12 @@ server_response_http(struct client *clt,
> > kv_set(&resp->http_pathquery, "%s", error) == -1)
> > return (-1);
> > 
> > - /* Add headers /
> > - if (kv_add(&resp->http_headers, "Server", HTTPD_SERVERNAME) == NULL)
> > - return (-1);
> > -
> > + / Add server banner header to response unless suppressed /
> > + if ((srv_conf->flags & SRVFLAG_NO_BANNER) == 0) {
> > + if (kv_add(&resp->http_headers, "Server",
> > + HTTPD_SERVERNAME) == NULL)
> > + return (-1);
> > + }
> > / Is it a persistent connection? */
> > if (clt->clt_persist) {
> > if (kv_add(&resp->http_headers,
> 
> 
> --
> wbr, Kirill