Download raw body.
(return of) the httpd banner suppressing patch
I have a few comments on this
On Fri, May 09, 2025 at 15:27:29 PM -0400, Lloyd <ng2d68@proton.me> wrote:
> Patch has been freshly retested against OpenBSD 7.7:
>
> 1. Adds a "[no] banner" directive to the global and server sections
> of httpd.conf to selectively suppress the "Server:" HTTP header and
> tokens in the error documents
>
> 2. Fixes a bug in debugging output introduced in a previous merge
>
> 3. Fixes a formatting nit in existing code
>
> 4. Relevant updates to man page documentation
>
> 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;
> + }
> + ;
> +
This directive should be listed along with the other directives that
are not allowed in location blocks in the httpd.conf(5) manpage.
> 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(¶m, "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);
> +
Since HTTPD_SERVERNAME is a constant I don't think you really need
to use asprintf(3) here (could just use string literal concatenation).
If if makes sense to use asprintf(3) here then you should check
if it fails, and at some point free bannertoken.
> /* 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);
> +
Same as above
> /* 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,
(return of) the httpd banner suppressing patch