From: Kirill A. Korinsky <kirill@korins.ky>
Subject: Re: httpd(8): patch to disable/modify the server banner
To: Lloyd <ng2d68@proton.me>
Cc: "tech@openbsd.org" <tech@openbsd.org>
Date: Wed, 29 Jan 2025 10:05:03 +0100

On Sun, 26 Jan 2025 21:20:35 +0100,
Lloyd <ng2d68@proton.me> wrote:
> 
> I have created and tested the following patch for httpd(8) which adds
> two new options to httpd.conf(5):
> 
> banner <string> which allows you to customize the server name string.
> 
> [no] disablebanner which allows you to suppress the Server header and
> output in errdocs entirely e.g.:
> 
> banner "Microsoft-IIS/5.0"
> 
> server "foo" {
> 	listen on * port 80
> 	disablebanner
> }
> 
> server "bar" {
> 	listen on * port 80
> 	banner "OpenBSD httpd"
> }
> 
> These directives are available both in global and server contexts and
> you can mix and match (e.g. suppress globally but allow publication
> on certain virtual servers only).
> 
> Allowing configuration of the Server header is suggested in RFC 2616.
> Suppression of the header entirely is preferred by some operators but
> often difficult to do in most server software without recompiling.
> 
> This patch also fixes a bug introduced when gzip-static was merged,
> but the bug only affected debugging output so went unnoticed.
> 
> Tested on amd64.
> 
> 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	26 Jan 2025 19:55:19 -0000
> @@ -54,6 +54,9 @@ config_init(struct httpd *env)
>  	(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?

>  	/* Other configuration */
>  	what = ps->ps_what[privsep_process];
>  
> @@ -591,6 +594,10 @@ 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;
> +		strlcpy(srv_conf->banner, parent->banner,
> +		    sizeof(srv_conf->banner));

and here?

>  
>  		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	26 Jan 2025 19:55:19 -0000
> @@ -105,6 +105,16 @@ server "example.com" {
>  .Sh GLOBAL CONFIGURATION
>  Here are the settings that can be set globally:
>  .Bl -tag -width Ds
> +.It Ic banner Ar string
> +Set a custom server software name for
> +.Xr httpd 8 .
> +This is the value sent to clients in the
> +.Ic Server
> +HTTP response header, the
> +.Ic SERVER_SOFTWARE
> +environment variable for CGI scripts, and the
> +.Ic $SERVER_SOFTWARE
> +macro used to create custom error documents.
>  .It Ic chroot Ar directory
>  Set the
>  .Xr chroot 2
> @@ -121,6 +131,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
> @@ -227,6 +245,12 @@ and must be readable by the www user.
>  Use the
>  .Ic no authenticate
>  directive to disable authentication in a location.
> +.It Ic banner Ar string
> +Set a custom
> +.Xr httpd 8
> +software name for the current
> +.Ic server ,
> +overriding the global setting if set.
>  .It Ic block drop
>  Drop the connection without sending an error page.
>  .It Ic block Op Ic return Ar code Op Ar uri
> @@ -323,6 +347,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	26 Jan 2025 19:55:19 -0000
> @@ -46,6 +46,7 @@
>  #define CONF_FILE		"/etc/httpd.conf"
>  #define HTTPD_USER		"www"
>  #define HTTPD_SERVERNAME	"OpenBSD httpd"
> +#define HTTPD_SERVERNAME_MAX	64
>  #define HTTPD_DOCROOT		"/htdocs"
>  #define HTTPD_ERRDOCTEMPLATE	"err" /* 3-char name */
>  #define HTTPD_ERRDOCROOT_MAX	(PATH_MAX - sizeof("000.html"))
> @@ -389,6 +390,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 +400,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
> @@ -539,6 +541,7 @@ struct server_config {
>  	struct server_fcgiparams fcgiparams;
>  	int			 fcgistrip;
>  	char			 errdocroot[HTTPD_ERRDOCROOT_MAX];
> +	char			 banner[HTTPD_SERVERNAME_MAX];
>  
>  	TAILQ_ENTRY(server_config) entry;
>  };
> @@ -600,6 +603,7 @@ struct httpd {
>  
>  	int			 sc_custom_errdocs;
>  	char			 sc_errdocroot[HTTPD_ERRDOCROOT_MAX];
> +	char			 sc_banner[HTTPD_SERVERNAME_MAX];
>  };
>  
>  #define HTTPD_OPT_VERBOSE		0x01
> 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	26 Jan 2025 19:55:19 -0000
> @@ -141,7 +141,7 @@ 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 DISABLEBANNER
>  %token	<v.string>	STRING
>  %token  <v.number>	NUMBER
>  %type	<v.port>	port
> @@ -210,9 +210,16 @@ main		: PREFORK NUMBER	{
>  			}
>  			conf->sc_prefork_server = $2;
>  		}
> +		| BANNER STRING		{
> +			strlcpy(conf->sc_banner, $2, sizeof(conf->sc_banner));
> +			free($2);
> +		}
>  		| 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 +307,12 @@ server		: SERVER optmatch STRING	{
>  
>  			s->srv_conf.hsts_max_age = SERVER_HSTS_DEFAULT_AGE;
>  
> +			strlcpy(s->srv_conf.banner, conf->sc_banner,
> +			    sizeof(s->srv_conf.banner));

and here?

> +
> +			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));
> @@ -495,6 +508,11 @@ serveroptsl	: LISTEN ON STRING opttls po
>  
>  			TAILQ_INSERT_TAIL(&srv->srv_hosts, alias, entry);
>  		}
> +		| BANNER STRING		{
> +			strlcpy(srv->srv_conf.banner, $2,
> +			    sizeof(srv->srv_conf.banner));

and here?

> +			free($2);
> +		}
>  		| ERRDOCS STRING	{
>  			if (parentsrv != NULL) {
>  				yyerror("errdocs inside location");
> @@ -550,6 +568,7 @@ serveroptsl	: LISTEN ON STRING opttls po
>  		| request
>  		| root
>  		| directory
> +		| disablebanner
>  		| logformat
>  		| fastcgi
>  		| authenticate
> @@ -687,6 +706,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; }
> @@ -1428,6 +1463,7 @@ lookup(char *s)
>  		{ "authenticate",	AUTHENTICATE},
>  		{ "auto",		AUTO },
>  		{ "backlog",		BACKLOG },
> +		{ "banner",		BANNER },
>  		{ "block",		BLOCK },
>  		{ "body",		BODY },
>  		{ "buffer",		BUFFER },
> @@ -1443,6 +1479,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	26 Jan 2025 19:55:19 -0000
> @@ -345,7 +345,8 @@ server_fcgi(struct httpd *env, struct cl
>  		goto fail;
>  	}
>  
> -	if (fcgi_add_param(&param, "SERVER_SOFTWARE", HTTPD_SERVERNAME,
> +	/* RFC 3875 requires this variable always be present */
> +	if (fcgi_add_param(&param, "SERVER_SOFTWARE", srv_conf->banner,

Not sure that sending a custom banner is safe, because it can be an empty
string. Am I wrong?

>  	    clt) == -1) {
>  		errstr = "failed to encode param";
>  		goto fail;
> @@ -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",
> +		    srv_conf->banner) == 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	26 Jan 2025 19:55:19 -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", srv_conf->banner);
> +	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",
> +		    srv_conf->banner);
> +
>  	/* 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",
> +		    srv_conf->banner);
> +
>  	/* 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 */

I think we should keep this comment.

> -	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",
> +		    srv_conf->banner) == NULL)
> +			return (-1);
> +	}
>  	/* Is it a persistent connection? */
>  	if (clt->clt_persist) {
>  		if (kv_add(&resp->http_headers,
> 

-- 
wbr, Kirill