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

[Mark Michelson]

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

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/7f87313c/attachment-0001.htm>