You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@gmail.com> on 2006/08/30 15:18:02 UTC

[PATCH] split ap_get_server_version() into two functions...

... so that ServerTokens doesn't affect what gets logged to the error
log at startup (or any other place where we want the description of
the server instead of the banner to be written over the network).

This patch axes ap_get_server_version() so that third-party modules
will be forced to make a choice in post-2.2.x versions -- call
ap_get_server_banner() to retrieve the string suitable for sending
over the network (as controlled by ServerTokens) or call
ap_get_server_description() to retrieve the string that describes the
server version and certain plug-in modules.

I haven't actually changed the returned strings with this patch, but
instead would like to see any comments on the direction.  The two
functions are present and all httpd code has been modified to call the
proper function, which is also subject to a useful review.  What about
status pages?  Is the server version and module configuration somehow
more private than the status page itself, such that they should use
ap_get_server_banner() as in the current patch?

Re: [PATCH] split ap_get_server_version() into two functions...

Posted by Jeff Trawick <tr...@gmail.com>.
On 9/1/06, Joe Orton <jo...@redhat.com> wrote:
> On Wed, Aug 30, 2006 at 09:18:02AM -0400, Jeff Trawick wrote:
> > ... so that ServerTokens doesn't affect what gets logged to the error
> > log at startup (or any other place where we want the description of
> > the server instead of the banner to be written over the network).
> >
> > This patch axes ap_get_server_version() so that third-party modules
> > will be forced to make a choice in post-2.2.x versions -- call
> > ap_get_server_banner() to retrieve the string suitable for sending
> > over the network (as controlled by ServerTokens) or call
> > ap_get_server_description() to retrieve the string that describes the
> > server version and certain plug-in modules.
>
> Looks good to me.
>
> > I haven't actually changed the returned strings with this patch, but
> > instead would like to see any comments on the direction.  The two
> > functions are present and all httpd code has been modified to call the
> > proper function, which is also subject to a useful review.  What about
> > status pages?  Is the server version and module configuration somehow
> > more private than the status page itself, such that they should use
> > ap_get_server_banner() as in the current patch?
>
> I'd agree with Sebastian here, the mod_status output should just use the
> more descriptive version, if you are revealing the server state in such
> detail you certainly have nothing to hide.

Thanks to both for commenting and to Sebastian for carrying the patch
further.  I'll play with Sebastian's logic to return the proper
strings with the plan to commit (but still no ServerTokens None at
that point).

Re: [PATCH] split ap_get_server_version() into two functions...

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Aug 30, 2006 at 09:18:02AM -0400, Jeff Trawick wrote:
> ... so that ServerTokens doesn't affect what gets logged to the error
> log at startup (or any other place where we want the description of
> the server instead of the banner to be written over the network).
> 
> This patch axes ap_get_server_version() so that third-party modules
> will be forced to make a choice in post-2.2.x versions -- call
> ap_get_server_banner() to retrieve the string suitable for sending
> over the network (as controlled by ServerTokens) or call
> ap_get_server_description() to retrieve the string that describes the
> server version and certain plug-in modules.

Looks good to me.

> I haven't actually changed the returned strings with this patch, but
> instead would like to see any comments on the direction.  The two
> functions are present and all httpd code has been modified to call the
> proper function, which is also subject to a useful review.  What about
> status pages?  Is the server version and module configuration somehow
> more private than the status page itself, such that they should use
> ap_get_server_banner() as in the current patch?

I'd agree with Sebastian here, the mod_status output should just use the 
more descriptive version, if you are revealing the server state in such 
detail you certainly have nothing to hide.

Regards,

joe

Re: [PATCH] split ap_get_server_version() into two functions...

Posted by Sebastian Nohn <se...@nohn.net>.
Sebastian Nohn wrote:
> Jeff Trawick wrote:
> 
>> ... so that ServerTokens doesn't affect what gets logged to the error
>> log at startup (or any other place where we want the description of
>> the server instead of the banner to be written over the network).
> 
> As far as I can see, your patch does not distinguish between those yet:
> 
> +AP_DECLARE(const char *) ap_get_server_description(void)
> +{
> +    return get_server_version();
> +}
> +
> +AP_DECLARE(const char *) ap_get_server_banner(void)
> +{
> +    return get_server_version();
> +}

Please find attached a patch that implements the desired behaviour in a
very rudimentary, but probably suffiecient manner (and includes my
ServerTokens Off patch, which could be removed easily but is included
for convenience).

Best regards,
  Sebastian Nohn
-- 
Sebastian Nohn · Wolfstraße 29 · 53111 Bonn · Germany
+49-228-4097103 · http://nohn.net/ · sebastian@nohn.net
http://pgpkeys.pca.dfn.de:11371/pks/lookup?op=get&fingerprint=on&search=0xD47D55E0

Re: [PATCH] split ap_get_server_version() into two functions...

Posted by Sebastian Nohn <se...@nohn.net>.
Jeff Trawick wrote:

> ... so that ServerTokens doesn't affect what gets logged to the error
> log at startup (or any other place where we want the description of
> the server instead of the banner to be written over the network).

As far as I can see, your patch does not distinguish between those yet:

+AP_DECLARE(const char *) ap_get_server_description(void)
+{
+    return get_server_version();
+}
+
+AP_DECLARE(const char *) ap_get_server_banner(void)
+{
+    return get_server_version();
+}

> I haven't actually changed the returned strings with this patch, but
> instead would like to see any comments on the direction.  The two
> functions are present and all httpd code has been modified to call the

I should have read that ;)

> proper function, which is also subject to a useful review.  What about
> status pages?  Is the server version and module configuration somehow
> more private than the status page itself, such that they should use
> ap_get_server_banner() as in the current patch?

I don't think so. Someone who makes his status page public most likely
would also make his Server version and configured modules public. And
someone who protects his server status somehow does most likely not care
if Server version and module information is published on that page, so
in my opinion, the full information should be displayed on status pages.

Best regards,
  Sebastian Nohn
-- 
Sebastian Nohn · Wolfstraße 29 · 53111 Bonn · Germany
+49-228-4097103 · http://nohn.net/ · sebastian@nohn.net
http://pgpkeys.pca.dfn.de:11371/pks/lookup?op=get&fingerprint=on&search=0xD47D55E0