[asterisk-dev] [Code Review] 2684: Fix exposure of template-only config sections

[Matt Jordan]

> On July 18, 2013, 1:45 p.m., Russell Bryant wrote:
> > The sucky thing about this is that fixing this will probably break existing deployments.  :-(
> 
> Russell Bryant wrote:
>     so maybe fix in trunk only and note it in UPGRADE.txt?
> 
> Matt Jordan wrote:
>     +1 for trunk only.
> 
> Mark Michelson wrote:
>     I'd also recommend changing asterisk.conf.sample and doing an audit to find if any other sample configs have one-off sections that are defined as templates and fix those too. It makes no sense that "directories" was a template-only section.
> 
> Tilghman Lesher wrote:
>     Given the possible use of templates in places like sip.conf and other remote peer channel specifications, this has possible security implications.  Therefore, I'd suggest that the change be made in all release branches and a security advisory released.

Offending .conf files:

* asterisk.conf.sample (directories)
* oss.conf.sample (my_skin)

If you squint hard, you can blame sip.conf as well, as the 'phones' that inherit from ulaw-phone and my-codecs aren't very complete.

There is a risk with doing this as a security advisory. While it is conceivable that someone could misconfigure their system and accidentally create a SIP peer that they didn't want created, this behavior has been around for a long time. Changing behavior in midstream tends to bite us pretty hard.

Does anyone else think this warrants a change in 1.8/10/11 and a security notice?


- Matt


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


On July 18, 2013, 1:44 p.m., Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2684/
> -----------------------------------------------------------
> 
> (Updated July 18, 2013, 1:44 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> While working on a deployment, I had to change the [directories] section of asterisk.conf from the defaults.  That worked.  Later I noticed that the directories section was defined as a template-only section like so:
> 
>     [directories](!)
> 
> which means my changes should *not* have taken effect.  This one line change fixes that.
> 
> As a side note, while looking at this, I noticed multiple cases of comparing against a category name like this throughout the file, which seems wrong:
> 
> from ast_category_delete()
> 
>                 if (cat->name == category) {
> 
> from ast_variable_browse()
> 
>         if (config->last_browse && (config->last_browse->name == category)) {
> 
> etc.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/config.c 394685 
> 
> Diff: https://reviewboard.asterisk.org/r/2684/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Russell Bryant
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130805/f478af75/attachment.htm>