[asterisk-dev] [Code Review] 4374: Asterisk: For httpd server, need option to define server name for security purposes

[Ashley Sanders]

> On Jan. 26, 2015, 5:44 p.m., rmudgett wrote:
> > ./branches/13/main/http.c, lines 608-615
> > <https://reviewboard.asterisk.org/r/4374/diff/1/?file=71085#file71085line608>
> >
> >     Yuck on toothbrush formatting.

I think this looks better, personally. It's easier to read and glean intent.


> On Jan. 26, 2015, 5:44 p.m., rmudgett wrote:
> > ./branches/13/main/http.c, lines 626-635
> > <https://reviewboard.asterisk.org/r/4374/diff/1/?file=71085#file71085line626>
> >
> >     Can you say uninitialized pointer?

Thank you for finding this.


> On Jan. 26, 2015, 5:44 p.m., rmudgett wrote:
> > ./branches/13/configs/samples/http.conf.sample, lines 18-19
> > <https://reviewboard.asterisk.org/r/4374/diff/1/?file=71084#file71084line18>
> >
> >     This statement is not true.  "Asterisk Server" is no longer sent out if servername is not specified.  It is now "Asterisk/<verion>" by default for both locations.

This is the exact text that was provided in the issue instructions. I will revise it, though, as you are correct.


> On Jan. 26, 2015, 5:44 p.m., rmudgett wrote:
> > ./branches/13/main/http.c, lines 567-568
> > <https://reviewboard.asterisk.org/r/4374/diff/1/?file=71085#file71085line567>
> >
> >     Why are the allocation sizes passed in?

To account for this handling the error and auth functions. They both created a header and out buffer, but of different sizes. In a fair and just world, I would declare a size inside of the ast_http_create_response function; however, because I do not want to introduce any regressions, I am leaving the sizes of the each buffer up to the individual calling functions.


> On Jan. 26, 2015, 5:44 p.m., rmudgett wrote:
> > ./branches/13/main/http.c, line 577
> > <https://reviewboard.asterisk.org/r/4374/diff/1/?file=71085#file71085line577>
> >
> >     Just "OOM" here.  It was "Auth OOM" and "error OOM" before you made a common function.

This is just a debug message to show where the OOM occurred (e.g. "Auth" for the auth function and "error" for the error function); is it not equally sufficient to say that the error occurred in this function and then use the debug logs back trace the source? That would have had to happen before this change, also. 


- Ashley


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4374/#review14289
-----------------------------------------------------------


On Jan. 26, 2015, 2:03 p.m., Ashley Sanders wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4374/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 2:03 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24316
>     https://issues.asterisk.org/jira/browse/ASTERISK-24316
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Currently, all responses from the Asterisk HTTP server contain a [Server] header that identifies Asterisk and its version (e.g. "Server:Asterisk/<version>", where <version> is the currently running version of Asterisk). The preferred behavior is to allow the user to configure an alternate name to use for the value returned in the [Server] header for HTTP responses (e.g. "Server:SomeSuperAwesomeServerName").
> 
> This patch provides a new configuration property, [servername], in http.conf, that gives users the ability to modify the value that Asterisk uses when identifying itself. 
> 
> By default, the new property is unused, which means that out-of-the-box, the HTTP server behaves just like it did prior to the patch. Requests to the HTTP server will generate responses with the old-style [Server] header (e.g. "Server:Asterisk/<version>", where <version> is the currently running version of Asterisk). To see the new behavior, you must add the configuration property, [servername] with some value (an empty value will work, also) to http.conf.
> 
> Whatever value the HTTP server is holding for the server name can now be seen through the httpstatus web page (http://<bindaddr>:<bindport>/<prefix>/httpstatus) (where [bindaddr], [bindport], and [prefix] are all values configured in http.conf) and the CLI command: http show status.
> 
> ***Note*** This is just the patch to the Asterisk source. You can find the review for the Testsuite at: https://reviewboard.asterisk.org/r/4377/
> 
> 
> Diffs
> -----
> 
>   ./branches/13/main/http.c 431112 
>   ./branches/13/configs/samples/http.conf.sample 431112 
> 
> Diff: https://reviewboard.asterisk.org/r/4374/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashley Sanders
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150127/b841fd74/attachment-0001.html>