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

1 files changed, 78 insertions, 48 deletions

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;