Re-resolve the STUN address if a STUN poll fails for res_stun_monitor.

The STUN socket must remain open between polls or the external address
seen by the STUN server is likely to change.  However, if the STUN request
poll fails then the STUN server address needs to be re-resolved and the
STUN socket needs to be closed and reopened.

* Re-resolve the STUN server address and create a new socket if the STUN
request poll fails.

* Fix ast_stun_request() return value consistency.

* Fix ast_stun_request() to check the received packet for expected message
type and transaction ID.

* Fix ast_stun_request() to read packets until timeout or an associated
response packet is found.  The stun_purge_socket() hack is no longer
required.

* Reduce ast_stun_request() error messages to debug output.

* No longer pass in the destination address to ast_stun_request() if the
socket is already bound or connected to the destination.

(closes issue ASTERISK-18327)
Reported by: Wolfram Joost
Tested by: rmudgett

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

Merged revisions 346700 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 346701 from http://svn.asterisk.org/svn/asterisk/branches/10


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

4 files changed, 311 insertions, 177 deletions

diff --git a/configs/res_stun_monitor.conf.sample b/configs/res_stun_monitor.conf.sample
index 9237799c5..12d32a4cd 100644
--- a/configs/res_stun_monitor.conf.sample
+++ b/configs/res_stun_monitor.conf.sample
@@ -2,11 +2,11 @@
 ; Configuration file for the res_stun_monitor module
 ;
 ; The res_stun_monitor module sends STUN requests to a configured STUN server
-; periodically.  If the monitor detects a change in the external ip or port
+; periodically.  If the monitor detects a change in the external IP address or port
 ; provided by the STUN server an event is sent out internally within Asterisk
 ; to alert all listeners to that event of the change.
 
-; The current default listeners for the netork change event include chan_sip
+; The current default listeners for the network change event include chan_sip
 ; and chan_iax.  Both of these channel drivers by default react to this event
 ; by renewing all outbound registrations.  This allows the endpoints Asterisk
 ; is registering with to become aware of the address change and know the new
@@ -15,8 +15,13 @@
 [general]
 ;
 ; ---- STUN Server configuration ---
-;  Setting the 'stunaddr' option to a valid address enables the stun monitor.
-;
-; stunaddr = mystunserver.com ; address of the stun server to query.
-; stunrefresh = 30     ; number of seconds between stun refreshes. default is 30
+;  Setting the 'stunaddr' option to a valid address enables the STUN monitor.
 ;
+;stunaddr = mystunserver.com    ; Address of the STUN server to query.
+                                ; Valid form:
+                                ;   [(hostname | IP-address) [':' port]]
+                                ; The port defaults to the standard STUN port (3478).
+                                ; Set to an empty value to disable STUN monitoring.
+                                ;   Default is disabled.
+;stunrefresh = 30               ; Number of seconds between STUN refreshes.
+                                ;   Default is 30.
diff --git a/include/asterisk/stun.h b/include/asterisk/stun.h
index 1fda0c33d..f0f9d0e34 100644
--- a/include/asterisk/stun.h
+++ b/include/asterisk/stun.h
@@ -41,28 +41,53 @@ enum ast_stun_result {
 
 struct stun_attr;
 
-/*! \brief Generic STUN request
- * send a generic stun request to the server specified.
- * \param s the socket used to send the request
- * \param dst the address of the STUN server
- * \param username if non null, add the username in the request
- * \param answer if non null, the function waits for a response and
+/*!
+ * \brief Generic STUN request.
+ *
+ * \param s The socket used to send the request.
+ * \param dst If non null, the address of the STUN server.
+ *    Only needed if the socket is not bound or connected.
+ * \param username If non null, add the username in the request.
+ * \param answer If non null, the function waits for a response and
  *    puts here the externally visible address.
- * \return 0 on success, other values on error.
- * The interface it may change in the future.
+ *
+ * \details
+ * Send a generic STUN request to the server specified, possibly
+ * waiting for a reply and filling the answer parameter with the
+ * externally visible address.  Note that in this case the
+ * request will be blocking.
+ *
+ * \note The interface may change slightly in the future.
+ *
+ * \retval 0 on success.
+ * \retval <0 on error.
+ * \retval >0 on timeout.
  */
 int ast_stun_request(int s, struct sockaddr_in *dst, const char *username, struct sockaddr_in *answer);
 
 /*! \brief callback type to be invoked on stun responses. */
 typedef int (stun_cb_f)(struct stun_attr *attr, void *arg);
 
-/*! \brief handle an incoming STUN message.
+/*!
+ * \brief handle an incoming STUN message.
  *
+ * \param s Socket to send any response to.
+ * \param src Address where packet came from.
+ * \param data STUN packet buffer to process.
+ * \param len Length of packet
+ * \param stun_cb If not NULL, callback for each STUN attribute.
+ * \param arg Arg to pass to callback.
+ *
+ * \details
  * Do some basic sanity checks on packet size and content,
  * try to extract a bit of information, and possibly reply.
  * At the moment this only processes BIND requests, and returns
  * the externally visible address of the request.
  * If a callback is specified, invoke it with the attribute.
+ *
+ * \retval AST_STUN_ACCEPT if responed to a STUN request
+ * \retval AST_STUN_IGNORE
+ * \retval -1 on error
  */
 int ast_stun_handle_packet(int s, struct sockaddr_in *src, unsigned char *data, size_t len, stun_cb_f *stun_cb, void *arg);
 
diff --git a/main/stun.c b/main/stun.c
index 3ad326ee8..a1474156f 100644
--- a/main/stun.c
+++ b/main/stun.c
@@ -234,6 +234,21 @@ static int stun_send(int s, struct sockaddr_in *dst, struct stun_header *resp)
 		      (struct sockaddr *)dst, sizeof(*dst));
 }
 
+/*!
+ * \internal
+ * \brief Compare the STUN tranaction IDs.
+ *
+ * \param left Transaction ID.
+ * \param right Transaction ID.
+ *
+ * \retval 0 if match.
+ * \retval non-zero if not match.
+ */
+static int stun_id_cmp(stun_trans_id *left, stun_trans_id *right)
+{
+	return memcmp(left, right, sizeof(*left));
+}
+
 /*! \brief helper function to generate a random request id */
 static void stun_req_id(struct stun_header *req)
 {
@@ -242,14 +257,6 @@ static void stun_req_id(struct stun_header *req)
 		req->id.id[x] = ast_random();
 }
 
-/*! \brief handle an incoming STUN message.
- *
- * Do some basic sanity checks on packet size and content,
- * try to extract a bit of information, and possibly reply.
- * At the moment this only processes BIND requests, and returns
- * the externally visible address of the request.
- * If a callback is specified, invoke it with the attribute.
- */
 int ast_stun_handle_packet(int s, struct sockaddr_in *src, unsigned char *data, size_t len, stun_cb_f *stun_cb, void *arg)
 {
 	struct stun_header *hdr = (struct stun_header *)data;
@@ -359,75 +366,98 @@ static int stun_get_mapped(struct stun_attr *attr, void *arg)
 	return 0;
 }
 
-/*! \brief Generic STUN request
- * Send a generic stun request to the server specified,
- * possibly waiting for a reply and filling the 'reply' field with
- * the externally visible address. Note that in this case the request
- * will be blocking.
- * (Note, the interface may change slightly in the future).
- *
- * \param s the socket used to send the request
- * \param dst the address of the STUN server
- * \param username if non null, add the username in the request
- * \param answer if non null, the function waits for a response and
- *    puts here the externally visible address.
- * \return 0 on success, other values on error.
- */
 int ast_stun_request(int s, struct sockaddr_in *dst,
 	const char *username, struct sockaddr_in *answer)
 {
 	struct stun_header *req;
-	unsigned char reqdata[1024];
+	struct stun_header *rsp;
+	unsigned char req_buf[1024];
+	unsigned char rsp_buf[1024];
 	int reqlen, reqleft;
 	struct stun_attr *attr;
-	int res = 0;
+	int res = -1;
 	int retry;
 
-	req = (struct stun_header *)reqdata;
+	if (answer) {
+		/* Always clear answer in case the request fails. */
+		memset(answer, 0, sizeof(struct sockaddr_in));
+	}
+
+	/* Create STUN bind request */
+	req = (struct stun_header *) req_buf;
 	stun_req_id(req);
 	reqlen = 0;
-	reqleft = sizeof(reqdata) - sizeof(struct stun_header);
+	reqleft = sizeof(req_buf) - sizeof(struct stun_header);
 	req->msgtype = 0;
 	req->msglen = 0;
-	attr = (struct stun_attr *)req->ies;
-	if (username)
+	attr = (struct stun_attr *) req->ies;
+	if (username) {
 		append_attr_string(&attr, STUN_USERNAME, username, &reqlen, &reqleft);
+	}
 	req->msglen = htons(reqlen);
 	req->msgtype = htons(STUN_BINDREQ);
-	for (retry = 0; retry < 3; retry++) {	/* XXX make retries configurable */
+
+	for (retry = 0; retry++ < 3;) {	/* XXX make retries configurable */
 		/* send request, possibly wait for reply */
-		unsigned char reply_buf[1024];
-		struct pollfd pfds = { .fd = s, .events = POLLIN };
 		struct sockaddr_in src;
 		socklen_t srclen;
 
+		/* Send STUN message. */
 		res = stun_send(s, dst, req);
 		if (res < 0) {
-			ast_log(LOG_WARNING, "ast_stun_request send #%d failed error %d, retry\n",
-				retry, res);
-			continue;
+			ast_debug(1, "stun_send try %d failed: %s\n", retry, strerror(errno));
+			break;
 		}
-		if (answer == NULL)
+		if (!answer) {
+			/* Successful send since we don't care about any response. */
+			res = 0;
 			break;
-		res = ast_poll(&pfds, 1, 3000);
-		if (res <= 0)	/* timeout or error */
-			continue;
+		}
+
+try_again:
+		/* Wait for response. */
+		{
+			struct pollfd pfds = { .fd = s, .events = POLLIN };
+
+			res = ast_poll(&pfds, 1, 3000);
+			if (res < 0) {
+				/* Error */
+				continue;
+			}
+			if (!res) {
+				/* No response, timeout */
+				res = 1;
+				continue;
+			}
+		}
+
+		/* Read STUN response. */
 		memset(&src, 0, sizeof(src));
 		srclen = sizeof(src);
-		/* XXX pass -1 in the size, because stun_handle_packet might
+		/* XXX pass sizeof(rsp_buf) - 1 in the size, because stun_handle_packet might
 		 * write past the end of the buffer.
 		 */
-		res = recvfrom(s, reply_buf, sizeof(reply_buf) - 1,
-			0, (struct sockaddr *)&src, &srclen);
+		res = recvfrom(s, rsp_buf, sizeof(rsp_buf) - 1,
+			0, (struct sockaddr *) &src, &srclen);
 		if (res < 0) {
-			ast_log(LOG_WARNING, "ast_stun_request recvfrom #%d failed error %d, retry\n",
-				retry, res);
-			continue;
+			ast_debug(1, "recvfrom try %d failed: %s\n", retry, strerror(errno));
+			break;
 		}
-		memset(answer, 0, sizeof(struct sockaddr_in));
-		ast_stun_handle_packet(s, &src, reply_buf, res,
-			stun_get_mapped, answer);
-		res = 0; /* signal regular exit */
+
+		/* Process the STUN response. */
+		rsp = (struct stun_header *) rsp_buf;
+		if (ast_stun_handle_packet(s, &src, rsp_buf, res, stun_get_mapped, answer)
+			|| (rsp->msgtype != htons(STUN_BINDRESP)
+				&& rsp->msgtype != htons(STUN_BINDERR))
+			|| stun_id_cmp(&req->id, &rsp->id)) {
+			/* Bad STUN packet, not right type, or transaction ID did not match. */
+			memset(answer, 0, sizeof(struct sockaddr_in));
+
+			/* Was not a resonse to our request. */
+			goto try_again;
+		}
+		/* Success.  answer contains the external address if available. */
+		res = 0;
 		break;
 	}
 	return res;
diff --git a/res/res_stun_monitor.c b/res/res_stun_monitor.c
index 64ca73eda..b9854e60f 100644
--- a/res/res_stun_monitor.c
+++ b/res/res_stun_monitor.c
@@ -38,107 +38,137 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include "asterisk/stun.h"
 #include "asterisk/netsock2.h"
 #include "asterisk/lock.h"
+#include "asterisk/acl.h"
 #include <fcntl.h>
 
-static const int DEFAULT_MONITOR_REFRESH = 30;
+#define DEFAULT_MONITOR_REFRESH 30	/*!< Default refresh period in seconds */
 
 static const char stun_conf_file[] = "res_stun_monitor.conf";
 static struct ast_sched_context *sched;
 
 static struct {
-	struct sockaddr_in stunaddr;      /*!< The stun address we send requests to*/
-	struct sockaddr_in externaladdr;  /*!< current perceived external address. */
+	/*! STUN monitor protection lock. */
 	ast_mutex_t lock;
+	/*! Current perceived external address. */
+	struct sockaddr_in external_addr;
+	/*! STUN server host name. */
+	const char *server_hostname;
+	/*! Port of STUN server to use */
+	unsigned int stun_port;
+	/*! Number of seconds between polls to the STUN server for the external address. */
 	unsigned int refresh;
-	int stunsock;
+	/*! Monitoring STUN socket. */
+	int stun_sock;
+	/*! TRUE if the STUN monitor is enabled. */
 	unsigned int monitor_enabled:1;
-	unsigned int externaladdr_known:1;
+	/*! TRUE if the perceived external address is valid/known. */
+	unsigned int external_addr_known:1;
+	/*! TRUE if we have already griped about a STUN poll failing. */
+	unsigned int stun_poll_failed_gripe:1;
 } args;
 
-static inline void stun_close_sock(void)
+static void stun_close_sock(void)
 {
-	if (args.stunsock != -1) {
-		close(args.stunsock);
-		args.stunsock = -1;
-		memset(&args.externaladdr, 0, sizeof(args.externaladdr));
-		args.externaladdr_known = 0;
+	if (0 <= args.stun_sock) {
+		close(args.stun_sock);
+		args.stun_sock = -1;
 	}
 }
 
-/* \brief purge the stun socket's receive buffer before issuing a new request
- *
- * XXX Note that this is somewhat of a hack.  This function is essentially doing
- * a cleanup on the socket rec buffer to handle removing any STUN responses we have not
- * handled.  This is called before sending out a new STUN request so we don't read
- * a latent previous response thinking it is new.
- */
-static void stun_purge_socket(void)
-{
-	int flags = fcntl(args.stunsock, F_GETFL);
-	int res = 0;
-	unsigned char reply_buf[1024];
-
-	fcntl(args.stunsock, F_SETFL, flags | O_NONBLOCK);
-	while (res != -1) {
-		/* throw away everything in the buffer until we reach the end. */
-		res = recv(args.stunsock, reply_buf, sizeof(reply_buf), 0);
-	}
-	fcntl(args.stunsock, F_SETFL, flags & ~O_NONBLOCK);
-}
-
 /* \brief called by scheduler to send STUN request */
 static int stun_monitor_request(const void *blarg)
 {
 	int res;
-	int generate_event = 0;
-	struct sockaddr_in answer = { 0, };
-
+	struct sockaddr_in answer;
+	static const struct sockaddr_in no_addr = { 0, };
 
-	/* once the stun socket goes away, this scheduler item will go away as well */
 	ast_mutex_lock(&args.lock);
-	if (args.stunsock == -1) {
-		ast_log(LOG_ERROR, "STUN monitor: can not send STUN request, socket is not open\n");
+	if (!args.monitor_enabled) {
 		goto monitor_request_cleanup;
 	}
 
-	stun_purge_socket();
-
-	if (!(ast_stun_request(args.stunsock, &args.stunaddr, NULL, &answer)) &&
-		(memcmp(&args.externaladdr, &answer, sizeof(args.externaladdr)))) {
-		const char *newaddr = ast_strdupa(ast_inet_ntoa(answer.sin_addr));
-		int newport = ntohs(answer.sin_port);
-
-		ast_log(LOG_NOTICE, "STUN MONITOR: Old external address/port %s:%d now seen as %s:%d \n",
-			ast_inet_ntoa(args.externaladdr.sin_addr), ntohs(args.externaladdr.sin_port),
-			newaddr, newport);
-
-		memcpy(&args.externaladdr, &answer, sizeof(args.externaladdr));
+	if (args.stun_sock < 0) {
+		struct ast_sockaddr stun_addr;
 
-		if (args.externaladdr_known) {
-			/* the external address was already known, and has changed... generate event. */
-			generate_event = 1;
+		/* STUN socket not open.  Refresh the server DNS address resolution. */
+		if (!args.server_hostname) {
+			/* No STUN hostname? */
+			goto monitor_request_cleanup;
+		}
 
-		} else {
-			/* this was the first external address we found, do not alert listeners
-			 * until this address changes to something else. */
-			args.externaladdr_known = 1;
+		/* Lookup STUN address. */
+		memset(&stun_addr, 0, sizeof(stun_addr));
+		stun_addr.ss.ss_family = AF_INET;
+		if (ast_get_ip(&stun_addr, args.server_hostname)) {
+			/* Lookup failed. */
+			ast_log(LOG_WARNING, "Unable to lookup STUN server '%s'\n",
+				args.server_hostname);
+			goto monitor_request_cleanup;
 		}
-	}
+		ast_sockaddr_set_port(&stun_addr, args.stun_port);
 
-	if (generate_event) {
-		struct ast_event *event = ast_event_new(AST_EVENT_NETWORK_CHANGE, AST_EVENT_IE_END);
-		if (!event) {
-			ast_log(LOG_ERROR, "STUN monitor: could not create AST_EVENT_NETWORK_CHANGE event.\n");
+		/* open socket binding */
+		args.stun_sock = socket(AF_INET, SOCK_DGRAM, 0);
+		if (args.stun_sock < 0) {
+			ast_log(LOG_WARNING, "Unable to create STUN socket: %s\n", strerror(errno));
 			goto monitor_request_cleanup;
 		}
-		if (ast_event_queue(event)) {
-			ast_event_destroy(event);
-			event = NULL;
-			ast_log(LOG_ERROR, "STUN monitor: could not queue AST_EVENT_NETWORK_CHANGE event.\n");
+		if (ast_connect(args.stun_sock, &stun_addr)) {
+			ast_log(LOG_WARNING, "STUN Failed to connect to %s: %s\n",
+				ast_sockaddr_stringify(&stun_addr), strerror(errno));
+			stun_close_sock();
 			goto monitor_request_cleanup;
 		}
 	}
 
+	res = ast_stun_request(args.stun_sock, NULL, NULL, &answer);
+	if (res) {
+		/*
+		 * STUN request timed out or errored.
+		 *
+		 * Refresh the server DNS address resolution next time around.
+		 */
+		if (!args.stun_poll_failed_gripe) {
+			args.stun_poll_failed_gripe = 1;
+			ast_log(LOG_WARNING, "STUN poll %s. Re-evaluating STUN server address.\n",
+				res < 0 ? "failed" : "got no response");
+		}
+		stun_close_sock();
+	} else {
+		args.stun_poll_failed_gripe = 0;
+		if (memcmp(&no_addr, &answer, sizeof(no_addr))
+			&& memcmp(&args.external_addr, &answer, sizeof(args.external_addr))) {
+			const char *newaddr = ast_strdupa(ast_inet_ntoa(answer.sin_addr));
+			int newport = ntohs(answer.sin_port);
+
+			ast_log(LOG_NOTICE, "Old external address/port %s:%d now seen as %s:%d.\n",
+				ast_inet_ntoa(args.external_addr.sin_addr),
+				ntohs(args.external_addr.sin_port), newaddr, newport);
+
+			args.external_addr = answer;
+
+			if (args.external_addr_known) {
+				struct ast_event *event;
+
+				/*
+				 * The external address was already known, and has changed...
+				 * generate event.
+				 */
+				event = ast_event_new(AST_EVENT_NETWORK_CHANGE, AST_EVENT_IE_END);
+				if (!event) {
+					ast_log(LOG_ERROR, "Could not create AST_EVENT_NETWORK_CHANGE event.\n");
+				} else if (ast_event_queue(event)) {
+					ast_event_destroy(event);
+					ast_log(LOG_ERROR, "Could not queue AST_EVENT_NETWORK_CHANGE event.\n");
+				}
+			} else {
+				/* this was the first external address we found, do not alert listeners
+				 * until this address changes to something else. */
+				args.external_addr_known = 1;
+			}
+		}
+	}
+
 monitor_request_cleanup:
 	/* always refresh this scheduler item.  It will be removed elsewhere when
 	 * it is supposed to go away */
@@ -148,46 +178,40 @@ monitor_request_cleanup:
 	return res;
 }
 
-/* \brief stops the stun monitor thread
+/*!
+ * \internal
+ * \brief Stops the STUN monitor thread.
+ *
  * \note do not hold the args->lock while calling this
+ *
+ * \return Nothing
  */
 static void stun_stop_monitor(void)
 {
+	ast_mutex_lock(&args.lock);
+	args.monitor_enabled = 0;
+	ast_free((char *) args.server_hostname);
+	args.server_hostname = NULL;
+	stun_close_sock();
+	ast_mutex_unlock(&args.lock);
+
 	if (sched) {
 		ast_sched_context_destroy(sched);
 		sched = NULL;
 		ast_log(LOG_NOTICE, "STUN monitor stopped\n");
 	}
-	/* it is only safe to destroy the socket without holding arg->lock
-	 * after the sched thread is destroyed */
-	stun_close_sock();
 }
 
-/* \brief starts the stun monitor thread
+/*!
+ * \internal
+ * \brief Starts the STUN monitor thread.
+ *
  * \note The args->lock MUST be held when calling this function
+ *
+ * \return Nothing
  */
 static int stun_start_monitor(void)
 {
-	struct ast_sockaddr dst;
-	/* clean up any previous open socket */
-	stun_close_sock();
-
-	/* create destination ast_sockaddr */
-	ast_sockaddr_from_sin(&dst, &args.stunaddr);
-
-	/* open new socket binding */
-	args.stunsock = socket(AF_INET, SOCK_DGRAM, 0);
-	if (args.stunsock < 0) {
-		ast_log(LOG_WARNING, "Unable to create STUN socket: %s\n", strerror(errno));
-		return -1;
-	}
-
-	if (ast_connect(args.stunsock, &dst) != 0) {
-		ast_log(LOG_WARNING, "SIP STUN Failed to connect to %s\n", ast_sockaddr_stringify(&dst));
-		stun_close_sock();
-		return -1;
-	}
-
 	/* if scheduler thread is not started, make sure to start it now */
 	if (sched) {
 		return 0; /* already started */
@@ -195,7 +219,6 @@ static int stun_start_monitor(void)
 
 	if (!(sched = ast_sched_context_create())) {
 		ast_log(LOG_ERROR, "Failed to create stun monitor scheduler context\n");
-		stun_close_sock();
 		return -1;
 	}
 
@@ -210,7 +233,6 @@ static int stun_start_monitor(void)
 		ast_log(LOG_ERROR, "Unable to schedule STUN network monitor \n");
 		ast_sched_context_destroy(sched);
 		sched = NULL;
-		stun_close_sock();
 		return -1;
 	}
 
@@ -219,6 +241,65 @@ static int stun_start_monitor(void)
 	return 0;
 }
 
+/*!
+ * \internal
+ * \brief Parse and setup the stunaddr parameter.
+ *
+ * \param value Configuration parameter variable value.
+ *
+ * \retval 0 on success.
+ * \retval -1 on error.
+ */
+static int setup_stunaddr(const char *value)
+{
+	char *val;
+	char *host_str;
+	char *port_str;
+	unsigned int port;
+	struct ast_sockaddr stun_addr;
+
+	if (ast_strlen_zero(value)) {
+		/* Setting to an empty value disables STUN monitoring. */
+		args.monitor_enabled = 0;
+		return 0;
+	}
+
+	val = ast_strdupa(value);
+	if (!ast_sockaddr_split_hostport(val, &host_str, &port_str, 0)
+		|| ast_strlen_zero(host_str)) {
+		return -1;
+	}
+
+	/* Determine STUN port */
+	if (ast_strlen_zero(port_str)
+		|| 1 != sscanf(port_str, "%30u", &port)) {
+		port = STANDARD_STUN_PORT;
+	}
+
+	host_str = ast_strdup(host_str);
+	if (!host_str) {
+		return -1;
+	}
+
+	/* Lookup STUN address. */
+	memset(&stun_addr, 0, sizeof(stun_addr));
+	stun_addr.ss.ss_family = AF_INET;
+	if (ast_get_ip(&stun_addr, host_str)) {
+		ast_log(LOG_WARNING, "Unable to lookup STUN server '%s'\n", host_str);
+		ast_free(host_str);
+		return -1;
+	}
+
+	/* Save STUN server information. */
+	ast_free((char *) args.server_hostname);
+	args.server_hostname = host_str;
+	args.stun_port = port;
+
+	/* Enable STUN monitor */
+	args.monitor_enabled = 1;
+	return 0;
+}
+
 static int load_config(int startup)
 {
 	struct ast_flags config_flags = { 0, };
@@ -229,39 +310,37 @@ static int load_config(int startup)
 		ast_set_flag(&config_flags, CONFIG_FLAG_FILEUNCHANGED);
 	}
 
-	if (!(cfg = ast_config_load2(stun_conf_file, "res_stun_monitor", config_flags)) ||
-		cfg == CONFIG_STATUS_FILEINVALID) {
+	cfg = ast_config_load2(stun_conf_file, "res_stun_monitor", config_flags);
+	if (!cfg || cfg == CONFIG_STATUS_FILEINVALID) {
 		ast_log(LOG_WARNING, "Unable to load config %s\n", stun_conf_file);
 		return -1;
 	}
-
-	if (cfg == CONFIG_STATUS_FILEUNCHANGED && !startup) {
+	if (cfg == CONFIG_STATUS_FILEUNCHANGED) {
 		return 0;
 	}
 
+	/* clean up any previous open socket */
+	stun_close_sock();
+	args.stun_poll_failed_gripe = 0;
+
 	/* set defaults */
 	args.monitor_enabled = 0;
-	memset(&args.stunaddr, 0, sizeof(args.stunaddr));
 	args.refresh = DEFAULT_MONITOR_REFRESH;
 
 	for (v = ast_variable_browse(cfg, "general"); v; v = v->next) {
 		if (!strcasecmp(v->name, "stunaddr")) {
-			args.stunaddr.sin_port = htons(STANDARD_STUN_PORT);
-			if (ast_parse_arg(v->value, PARSE_INADDR, &args.stunaddr)) {
-				ast_log(LOG_WARNING, "Invalid STUN server address: %s\n", v->value);
-			} else {
-				ast_log(LOG_NOTICE, "STUN monitor enabled: %s\n", v->value);
-				args.monitor_enabled = 1;
+			if (setup_stunaddr(v->value)) {
+				ast_log(LOG_WARNING, "Invalid STUN server address: %s at line %d\n",
+					v->value, v->lineno);
 			}
 		} else if (!strcasecmp(v->name, "stunrefresh")) {
 			if ((sscanf(v->value, "%30u", &args.refresh) != 1) || !args.refresh) {
 				ast_log(LOG_WARNING, "Invalid stunrefresh value '%s', must be an integer > 0 at line %d\n", v->value, v->lineno);
 				args.refresh = DEFAULT_MONITOR_REFRESH;
-			} else {
-				ast_log(LOG_NOTICE, "STUN Monitor set to refresh every %d seconds\n", args.refresh);
 			}
 		} else {
-			ast_log(LOG_WARNING, "SIP STUN: invalid config option %s at line %d\n", v->value, v->lineno);
+			ast_log(LOG_WARNING, "Invalid config option %s at line %d\n",
+				v->value, v->lineno);
 		}
 	}
 
@@ -280,8 +359,7 @@ static int __reload(int startup)
 	}
 	ast_mutex_unlock(&args.lock);
 
-	if ((res == -1) || !args.monitor_enabled) {
-		args.monitor_enabled = 0;
+	if (res < 0 || !args.monitor_enabled) {
 		stun_stop_monitor();
 	}
 
@@ -303,12 +381,8 @@ static int unload_module(void)
 static int load_module(void)
 {
 	ast_mutex_init(&args.lock);
-	args.stunsock = -1;
-	memset(&args.externaladdr, 0, sizeof(args.externaladdr));
-	args.externaladdr_known = 0;
-	sched = NULL;
+	args.stun_sock = -1;
 	if (__reload(1)) {
-		stun_stop_monitor();
 		ast_mutex_destroy(&args.lock);
 		return AST_MODULE_LOAD_DECLINE;
 	}