Fix TLS port binding behavior as well as reload behavior:

* Removes references to tlsbindport from http.conf.sample and manager.conf.sample
* Properly bind to port specified in tlsbindaddr, using the default port if specified.
* On a reload, properly close socket if the service has been disabled.

A note has been added to UPGRADE.txt to indicate how ports must be set for TLS.

(closes issue ASTERISK-16959)
reported by Olaf Holthausen

(closes issue ASTERISK-19201)
reported by Chris Mylonas

(closes issue ASTERISK-19204)
reported by Chris Mylonas

Review: https://reviewboard.asterisk.org/r/1709

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@353770 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/UPGRADE.txt b/UPGRADE.txt
index 45edd03444b13f9105410a8c7d92ca662f913e85..f2c3a7c2dca2abd6f12c87b7a8a8bb0c7bd6500b 100644 (file)
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -20,6 +20,12 @@
 
 From 1.6.2 to 1.8:
 
+* When using TLS with Manager and the HTTP server, the desired port
+  must be specified in the tlsbindaddr setting. If no port is specified,
+  then the default port will be used. See the sample config file to know
+  the default ports. Settings like "sslbindport" and "tlsbindport" have
+  no effect.
+
 * chan_sip no longer sets HASH(SIP_CAUSE,<chan name>) on channels by default.
   This must now be enabled by setting 'sipstorecause' to 'yes' in sip.conf.
   This carries a performance penalty.

diff --git a/configs/http.conf.sample b/configs/http.conf.sample
index 7893cafec4a05ae85a8e9958793e970490328d19..7d4db43f27452b83a6746f8e7e8cc52eae148ec4 100644 (file)
--- a/configs/http.conf.sample
+++ b/configs/http.conf.sample
@@ -54,8 +54,7 @@ bindaddr=127.0.0.1
 ; explicitly enable tls, define the port to use,
 ; and have a certificate somewhere.
 ;tlsenable=yes          ; enable tls - default no.
-;tlsbindport=4433       ; port to use - default is 8089
-;tlsbindaddr=0.0.0.0    ; address to bind to - default is bindaddr.
+;tlsbindaddr=0.0.0.0:8089    ; address and port to bind to - default is bindaddr and port 8089.
 ;
 ;tlscertfile=</path/to/certificate.pem>  ; path to the certificate file (*.pem) only.
 ;tlsprivatekey=</path/to/private.pem>    ; path to private key file (*.pem) only.

diff --git a/configs/manager.conf.sample b/configs/manager.conf.sample
index 2d43360f6a883307f6ecc6d204c7a66df6171d61..fb44e74d4d3958cb76d5ae27f7d45078d3c4d6ac 100644 (file)
--- a/configs/manager.conf.sample
+++ b/configs/manager.conf.sample
@@ -33,8 +33,7 @@ bindaddr = 0.0.0.0
 ;      openssl s_client -connect my_host:5039
 ;
 ;tlsenable=no          ; set to YES to enable it
-;tlsbindport=5039              ; the port to bind to
-;tlsbindaddr=0.0.0.0           ; address to bind to, default to bindaddr
+;tlsbindaddr=0.0.0.0:5039              ; address and port to bind to, default to bindaddr and port 5039
 ;tlscertfile=/tmp/asterisk.pem ; path to the certificate.
 ;tlsprivatekey=/tmp/private.pem ; path to the private key, if no private given,
                                 ; if no tlsprivatekey is given, default is to search

diff --git a/include/asterisk/manager.h b/include/asterisk/manager.h
index 51018e79f1f9eb73c17db18098b6ef100aca8a7a..f0168ef4d2e16551a5b321eff59bf33bd72e5f14 100644 (file)
--- a/include/asterisk/manager.h
+++ b/include/asterisk/manager.h
@@ -56,6 +56,7 @@
 
 #define AMI_VERSION                     "1.1"
 #define DEFAULT_MANAGER_PORT 5038      /* Default port for Asterisk management via TCP */
+#define DEFAULT_MANAGER_TLS_PORT 5039  /* Default port for Asterisk management via TCP */
 
 /*! \name Constant return values
  *\note Currently, returning anything other than zero causes the session to terminate.

diff --git a/main/http.c b/main/http.c
index 53c2eb76c14d720a4e002c48b1e20ac7bd0cc6b2..88e36c7a1f1791a6c27015e799cecc955f60e645 100644 (file)
--- a/main/http.c
+++ b/main/http.c
@@ -61,6 +61,9 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #define MAX_PREFIX 80
 #define DEFAULT_SESSION_LIMIT 100
 
+#define DEFAULT_HTTP_PORT 8080
+#define DEFAULT_HTTPS_PORT 8089
+
 /* See http.h for more information about the SSL implementation */
 #if defined(HAVE_OPENSSL) && (defined(HAVE_FUNOPEN) || defined(HAVE_FOPENCOOKIE))
 #define        DO_SSL  /* comment in/out if you want to support ssl */
@@ -1022,21 +1025,19 @@ static int __ast_http_load(int reload)
        struct ast_flags config_flags = { reload ? CONFIG_FLAG_FILEUNCHANGED : 0 };
        struct sockaddr_in tmp = {0,};
        struct sockaddr_in tmp2 = {0,};
+       int http_tls_was_enabled = 0;
 
        cfg = ast_config_load2("http.conf", "http", config_flags);
        if (cfg == CONFIG_STATUS_FILEMISSING || cfg == CONFIG_STATUS_FILEUNCHANGED || cfg == CONFIG_STATUS_FILEINVALID) {
                return 0;
        }
 
-       /* default values */
+       http_tls_was_enabled = (reload && http_tls_cfg.enabled);
+
        tmp.sin_family = AF_INET;
-       tmp.sin_port = htons(8088);
+       tmp.sin_port = htons(DEFAULT_HTTP_PORT);
        ast_sockaddr_from_sin(&http_desc.local_address, &tmp);
 
-       tmp2.sin_family = AF_INET;
-       tmp2.sin_port = htons(8089);
-       ast_sockaddr_from_sin(&https_desc.local_address, &tmp2);
-
        http_tls_cfg.enabled = 0;
        if (http_tls_cfg.certfile) {
                ast_free(http_tls_cfg.certfile);
@@ -1059,6 +1060,8 @@ static int __ast_http_load(int reload)
        }
        AST_RWLIST_UNLOCK(&uri_redirects);
 
+       ast_sockaddr_setnull(&https_desc.local_address);
+
        if (cfg) {
                v = ast_variable_browse(cfg, "general");
                for (; v; v = v->next) {
@@ -1107,13 +1110,16 @@ static int __ast_http_load(int reload)
 
                ast_config_destroy(cfg);
        }
-       /* if the https addres has not been set, default is the same as non secure http */
+       /* if the https address has not been set, default is the same as non secure http */
        ast_sockaddr_to_sin(&http_desc.local_address, &tmp);
        ast_sockaddr_to_sin(&https_desc.local_address, &tmp2);
        if (!tmp2.sin_addr.s_addr) {
                tmp2.sin_addr = tmp.sin_addr;
-               ast_sockaddr_from_sin(&https_desc.local_address, &tmp2);
        }
+       if (!tmp2.sin_port) {
+               tmp2.sin_port = htons(DEFAULT_HTTPS_PORT);
+       }
+       ast_sockaddr_from_sin(&https_desc.local_address, &tmp2);
        if (!enabled) {
                ast_sockaddr_setnull(&http_desc.local_address);
                ast_sockaddr_setnull(&https_desc.local_address);
@@ -1123,7 +1129,10 @@ static int __ast_http_load(int reload)
        }
        enablestatic = newenablestatic;
        ast_tcptls_server_start(&http_desc);
-       if (ast_ssl_setup(https_desc.tls_cfg)) {
+       /* If https was enabled previously but now is not, then stop the service */
+       if (http_tls_was_enabled && !http_tls_cfg.enabled) {
+               ast_tcptls_server_stop(&https_desc);
+       } else if (ast_ssl_setup(https_desc.tls_cfg)) {
                ast_tcptls_server_start(&https_desc);
        }
 

diff --git a/main/manager.c b/main/manager.c
index b3d6a5b48cc119e19b99bc948616a65fe8927a3c..249c6565af6eded8ae9cde8ac2b6d5e4bd46a75c 100644 (file)
--- a/main/manager.c
+++ b/main/manager.c
@@ -6491,6 +6491,7 @@ static int __init_manager(int reload)
        char a1_hash[256];
        struct sockaddr_in ami_desc_local_address_tmp = { 0, };
        struct sockaddr_in amis_desc_local_address_tmp = { 0, };
+       int tls_was_enabled = 0;
 
        if (!registered) {
                /* Register default actions */
@@ -6556,11 +6557,16 @@ static int __init_manager(int reload)
 
        /* default values */
        ast_copy_string(global_realm, S_OR(ast_config_AST_SYSTEM_NAME, DEFAULT_REALM), sizeof(global_realm));
-       memset(&ami_desc.local_address, 0, sizeof(struct sockaddr_in));
-       memset(&amis_desc.local_address, 0, sizeof(amis_desc.local_address));
-       amis_desc_local_address_tmp.sin_port = htons(5039);
+       ast_sockaddr_setnull(&ami_desc.local_address);
+       ast_sockaddr_setnull(&amis_desc.local_address);
+
+       ami_desc_local_address_tmp.sin_family = AF_INET;
+       amis_desc_local_address_tmp.sin_family = AF_INET;
+
        ami_desc_local_address_tmp.sin_port = htons(DEFAULT_MANAGER_PORT);
 
+       tls_was_enabled = (reload && ami_tls_cfg.enabled);
+
        ami_tls_cfg.enabled = 0;
        if (ami_tls_cfg.certfile) {
                ast_free(ami_tls_cfg.certfile);
@@ -6634,8 +6640,7 @@ static int __init_manager(int reload)
                }
        }
 
-       ami_desc_local_address_tmp.sin_family = AF_INET;
-       amis_desc_local_address_tmp.sin_family = AF_INET;
+       ast_sockaddr_to_sin(&amis_desc.local_address, &amis_desc_local_address_tmp);
 
        /* if the amis address has not been set, default is the same as non secure ami */
        if (!amis_desc_local_address_tmp.sin_addr.s_addr) {
@@ -6643,6 +6648,10 @@ static int __init_manager(int reload)
                    ami_desc_local_address_tmp.sin_addr;
        }
 
+       if (!amis_desc_local_address_tmp.sin_port) {
+               amis_desc_local_address_tmp.sin_port = htons(DEFAULT_MANAGER_TLS_PORT);
+       }
+
        if (manager_enabled) {
                ast_sockaddr_from_sin(&ami_desc.local_address, &ami_desc_local_address_tmp);
                ast_sockaddr_from_sin(&amis_desc.local_address, &amis_desc_local_address_tmp);
@@ -6897,7 +6906,9 @@ static int __init_manager(int reload)
        manager_event(EVENT_FLAG_SYSTEM, "Reload", "Module: Manager\r\nStatus: %s\r\nMessage: Manager reload Requested\r\n", manager_enabled ? "Enabled" : "Disabled");
 
        ast_tcptls_server_start(&ami_desc);
-       if (ast_ssl_setup(amis_desc.tls_cfg)) {
+       if (tls_was_enabled && !ami_tls_cfg.enabled) {
+               ast_tcptls_server_stop(&amis_desc);
+       } else if (ast_ssl_setup(amis_desc.tls_cfg)) {
                ast_tcptls_server_start(&amis_desc);
        }
        return 0;