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

[rmudgett]

> 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?
> 
> Ashley Sanders wrote:
>     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.

Since you are making a common function in this patch you already know the callers and can declare the sizes involved.  You can allocate the larger for each buffer from the two callers.

ast_str_create() mallocs the initial buffer size.  If you put a larger string into it then the buffer is automatically increased.  Passing in different buffer sizes doesn't buy much except to potentially avoid a realloc.


> 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.
> 
> Ashley Sanders wrote:
>     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.

I made it say Auth OOM and error OOM earlier to distinguish which ast_debug output the message.  Since you are combining the two into a common function, there is only one ast_debug so OOM is sufficient.

On the console, the debug messages display the function and line number.  In the log file that information is not output which is unfortunate.


- rmudgett


-----------------------------------------------------------
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/d3fcff96/attachment.html>