<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/2684/">https://reviewboard.asterisk.org/r/2684/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 18th, 2013, 1:45 p.m. UTC, <b>Russell Bryant</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The sucky thing about this is that fixing this will probably break existing deployments. :-(</pre>
</blockquote>
<p>On July 18th, 2013, 1:46 p.m. UTC, <b>Russell Bryant</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">so maybe fix in trunk only and note it in UPGRADE.txt?</pre>
</blockquote>
<p>On July 18th, 2013, 2:17 p.m. UTC, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">+1 for trunk only.</pre>
</blockquote>
<p>On July 18th, 2013, 2:22 p.m. UTC, <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On July 21st, 2013, 4:29 p.m. UTC, <b>Tilghman Lesher</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?</pre>
<br />
<p>- Matt</p>
<br />
<p>On July 18th, 2013, 1:44 p.m. UTC, Russell Bryant wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Russell Bryant.</div>
<p style="color: grey;"><i>Updated July 18, 2013, 1:44 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/main/config.c <span style="color: grey">(394685)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2684/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>