[asterisk-dev] [Code Review]: Fix TLS port settings for Manager and HTTP

[Mark Michelson]

> On Feb. 2, 2012, 10:06 a.m., Paul Belanger wrote:
> > /branches/1.8/configs/http.conf.sample, lines 57-58
> > <https://reviewboard.asterisk.org/r/1709/diff/1/?file=23870#file23870line57>
> >
> >     might be worth adding a note in CHANGES or UPGRADE.txt that this setting was removed / never worked.

Yeah I had thought about this, but it's a bit of a weird situation since tlsbindport was never actually in an Asterisk release. It was added and removed prior to the release of 1.8.0. So it doesn't really belong in UPGRADE.txt since it has nothing to do with previous releases. On the other hand, a note in UPGRADE.txt indicating that preferred port must now be specified as part of the tlsbindaddr makes sense though. Do you think that's enough?


- Mark


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


On Feb. 2, 2012, 9:57 a.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1709/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2012, 9:57 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> During the formative stages of Asterisk 1.8, a common set of TLS settings was added to the TCP/TLS API, with the function ast_tls_read_conf() used to set common attributes. When this API was added, a "tlsbindport" option was present, and its presence was made known in http.conf.sample and manager.conf.sample.
> 
> Later, IPv6 support was added, and as part of it, the "tlsbindport" setting was removed from the common set of TLS settings. It was removed in favor of setting the port as part of the "tlsbindaddr" setting. Unfortunately, there were two problems. Firstly, the sample config files were not updated to indicate this setting had been removed. Secondly, the IPv6 merge introduced some screwy logic that caused the port to not get set to the proper default when not specified.
> 
> This patch has three fixes:
> 1. http.conf.sample and manager.conf.sample no longer reference the tlsbindport option, instead indicating to set the port in the tlsbindaddr setting instead.
> 2. Both manager.c and http.c will set the port to the proper default if none is specified. The code was previously attempting to pre-seed the default port into the settings. The problem is that this was being overwritten later in the config. The fix is to set the default port later, and only do so if no port was specified in the config file.
> 3. Based on notes from ASTERISK-19204, I also fixed behavior when TLS is initially enabled, Asterisk is started, then TLS is disabled in the config file, and either manager or http is reloaded. Previously, we would not actually close the TCP socket. Now we do!
> 
> 
> This addresses bugs ASTERISK-16959, ASTERISK-19201 and ASTERISK-19204.
>     https://issues.asterisk.org/jira/browse/ASTERISK-16959
>     https://issues.asterisk.org/jira/browse/ASTERISK-19201
>     https://issues.asterisk.org/jira/browse/ASTERISK-19204
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/configs/http.conf.sample 352913 
>   /branches/1.8/configs/manager.conf.sample 352913 
>   /branches/1.8/include/asterisk/manager.h 352913 
>   /branches/1.8/main/http.c 352913 
>   /branches/1.8/main/manager.c 352913 
> 
> Diff: https://reviewboard.asterisk.org/r/1709/diff
> 
> 
> Testing
> -------
> 
> Ran manager and http through their paces. For each, I did the following:
> 
> 1. Started Asterisk with TLS disabled. Verified that we did not start TLS service.
> 2. Started Asterisk with TLS enabled and no tlsbindaddr. Verified that service was started on the same address as non-TLS service and used the default TLS port.
> 3. Started Asterisk with TLS enabled and a tlsbindaddr with no port. Verified that service was started on the specified tlsbindaddr and on default TLS port.
> 4. Started Asterisk with TLS enabled and a tlsbindaddr with port. Verified that service was started on the specified tlsbindaddr and port.
> 5. Did step 4. Changed config file to not indicate a port in tlsbindaddr. Reloaded service. Verified that previous socket was closed and that service was restarted on default port.
> 5. Did step 4. Changed config file so TLS was disabled. Reloaded service. Verified that previous socket was closed and no new TLS service was started.
> 
> 
> Thanks,
> 
> Mark
> 
>

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