From: Lloyd Subject: Re: httpd(8): patch to disable/modify the server banner To: "Kirill A. Korinsky" Cc: "tech@openbsd.org" , "tom.smyth@wirelessconnect.eu" , "stu@spacehopper.org" Date: Wed, 29 Jan 2025 19:47:15 +0000 Kirill A. Korinsky wrote: > > (void)strlcpy(env->sc_errdocroot, "", > > sizeof(env->sc_errdocroot)); > > > > + /* Load the default server banner into the config if unconfigured */ > > + strlcpy(env->sc_banner, HTTPD_SERVERNAME, sizeof(env->sc_banner)); > > + > > Shouldn't you use (void)strlcpy like a line above? Is there a rule on this or just personal coding style of the original author? The only reason to cast strlcpy to void is to silence compliler warnings, no? FWIW I count 7 other instances in parse.y unrelated to the diff where is is not used - perhaps those should be updated as well for consistency's sake? Kind of a moot point because I eliminated these calls in the new diff but I am curious regardless. > Not sure that sending a custom banner is safe, because it can be an empty > string. Am I wrong? You bring up a good point. Discussion at link below indicates errata to RFC 7230 now allows zero-length string to HTTP header values (it was ambiguous for some time), though I removed it as Stu/Tom suggested below. https://github.com/whatwg/fetch/issues/332 > > - /* Add headers */ > > I think we should keep this comment. The comment wasn't removed but rewritten with more clarity: > > + /* Add server banner header to response unless suppressed / Stuart Henderson wrote: > I do see some use for disabling the header, but changing it from the > default seems a bit of a niche feature for an http daemon that is > intentionally quite barebones. Tom Smyth wrote: > Fwiw i think stuarts suggestion re syntax makes sense.. it is useful > to be able to remove the banner in httpd with out inserting relayd / > loadbalancers / reverse proxies into the equation I tend to agree with both comments. Other HTTP servers allow changing the header to different preset values, but httpd(8) already sends a minimalist Server header, which purposely does not include the version number. The ability to disable the header altogether without a reverse proxy kludge is the practical feature here and I feel adds security value. The simplified diff below reverts those changes and only adds the 'disablebanner' directive to httpd.conf. A small spacing error in parse.y was also noted and corrected. Regards Lloyd 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 29 Jan 2025 19:34:30 -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 29 Jan 2025 19:34:30 -0000 @@ -121,6 +121,14 @@ see the section below. If not specified, the default type is set to .Ar application/octet-stream . +.It Ic disablebanner +Do not send the +.Ic Server +HTTP response header, and hide the server software name in error documents. +The +.Ic SERVER_SOFTWARE +CGI environment variable is always set in accordance with +.Pa RFC 3875 . .It Ic errdocs Ar directory Let .Xr httpd 8 @@ -323,6 +331,13 @@ Disable the directory index. .Xr httpd 8 will neither display nor generate a directory index. .El +.It Oo Ic no Oc Ic disablebanner +Suppresses the server software name in HTTP headers and error documents +for the current +.Ic server. +The +.Ic no +keyword can be used to override the global setting. .It Oo Ic no Oc Ic errdocs Ar directory Overrides or, if the .Ic no 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 29 Jan 2025 19:34:30 -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 29 Jan 2025 19:34:30 -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 DISABLEBANNER %token STRING -%token NUMBER +%token NUMBER %type port %type fcgiport %type opttls optmatch optfound @@ -213,6 +213,9 @@ main : PREFORK NUMBER { | CHROOT STRING { conf->sc_chroot = $2; } + | DISABLEBANNER { + conf->sc_flags |= SRVFLAG_NO_BANNER; + } | ERRDOCS STRING { if ($2 != NULL && strlcpy(conf->sc_errdocroot, $2, sizeof(conf->sc_errdocroot)) >= @@ -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 + | disablebanner | logformat | fastcgi | authenticate @@ -687,6 +694,22 @@ serveroptsl : LISTEN ON STRING opttls po } ; +disablebanner : NO DISABLEBANNER { + if (parentsrv != NULL) { + yyerror("disablebanner inside location"); + YYERROR; + } + srv->srv_conf.flags &= ~SRVFLAG_NO_BANNER; + } + | DISABLEBANNER { + if (parentsrv != NULL) { + yyerror("disablebanner inside location"); + YYERROR; + } + srv->srv_conf.flags |= SRVFLAG_NO_BANNER; + } + ; + optfound : /* empty */ { $$ = 0; } | FOUND { $$ = 1; } | NOT FOUND { $$ = -1; } @@ -1443,6 +1466,7 @@ lookup(char *s) { "default", DEFAULT }, { "dhe", DHE }, { "directory", DIRECTORY }, + { "disablebanner", DISABLEBANNER }, { "drop", DROP }, { "ecdhe", ECDHE }, { "errdocs", ERRDOCS }, 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 29 Jan 2025 19:34:30 -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 29 Jan 2025 19:34:30 -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, "
\n
%s
\n", + HTTPD_SERVERNAME); + /* Generate simple HTML error document */ if ((bodylen = asprintf(&body, "\n" @@ -1005,10 +1015,11 @@ server_abort_http(struct client *clt, un "\n" "\n" "

%03d %s

\n" - "
\n
%s
\n" + "%s" "\n" "\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,