[Asterisk-code-review] chan_sip: Return 503 if we're out of RTP ports (asterisk[16])

Joshua Colp asteriskteam at digium.com
Thu Feb 6 07:24:30 CST 2020


Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13761 )

Change subject: chan_sip: Return 503 if we're out of RTP ports
......................................................................

chan_sip: Return 503 if we're out of RTP ports

If you're for some reason out of RTP ports, chan_sip would previously
responde to an INVITE with a 403, which will fail the call.

Now, it returns a 503, allowing the device/proxy to retry the call on a
different machine.

ASTERISK-28718

Change-Id: I968dcf6c1e30ecddcce397dcda36db727c83ca90
---
M channels/chan_sip.c
1 file changed, 53 insertions(+), 11 deletions(-)

Approvals:
  Sean Bright: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Approved for Submit



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 8c3e607..2a3fdb7 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -19625,6 +19625,54 @@
 	return check_user_full(p, req, sipmethod, uri, reliable, addr, NULL);
 }
 
+static void send_check_user_failure_response(struct sip_pvt *p, struct sip_request *req, int res, enum xmittype reliable)
+{
+	const char *response;
+
+	switch (res) {
+	case AUTH_SECRET_FAILED:
+	case AUTH_USERNAME_MISMATCH:
+	case AUTH_NOT_FOUND:
+	case AUTH_UNKNOWN_DOMAIN:
+	case AUTH_PEER_NOT_DYNAMIC:
+	case AUTH_BAD_TRANSPORT:
+		ast_log(LOG_NOTICE, "Failed to authenticate device %s for %s, code = %d\n",
+			sip_get_header(req, "From"), sip_methods[p->method].text, res);
+		response = "403 Forbidden";
+		break;
+	case AUTH_SESSION_LIMIT:
+		/* Unexpected here, actually. As it's handled elsewhere. */
+		ast_log(LOG_NOTICE, "Call limit reached for device %s for %s, code = %d\n",
+			sip_get_header(req, "From"), sip_methods[p->method].text, res);
+		response = "480 Temporarily Unavailable";
+		break;
+	case AUTH_RTP_FAILED:
+		/* We don't want to send a 403 in the RTP_FAILED case.
+		 * The cause could be any one of:
+		 * - out of memory or rtp ports
+		 * - dtls/srtp requested but not loaded/invalid
+		 * Neither of them warrant a 403. A 503 makes more
+		 * sense, as this node is broken/overloaded. */
+		ast_log(LOG_NOTICE, "RTP init failure for device %s for %s, code = %d\n",
+			sip_get_header(req, "From"), sip_methods[p->method].text, res);
+		response = "503 Service Unavailable";
+		break;
+	case AUTH_SUCCESSFUL:
+	case AUTH_CHALLENGE_SENT:
+		/* These should have been handled elsewhere. */
+	default:
+		ast_log(LOG_NOTICE, "Unexpected error for device %s for %s, code = %d\n",
+			sip_get_header(req, "From"), sip_methods[p->method].text, res);
+		response = "503 Service Unavailable";
+	}
+
+	if (reliable == XMIT_RELIABLE) {
+		transmit_response_reliable(p, response, req);
+	} else if (reliable == XMIT_UNRELIABLE) {
+		transmit_response(p, response, req);
+	}
+}
+
 static int set_message_vars_from_req(struct ast_msg *msg, struct sip_request *req)
 {
 	size_t x;
@@ -19741,8 +19789,7 @@
 			return;
 		}
 		if (res < 0) { /* Something failed in authentication */
-			ast_log(LOG_NOTICE, "Failed to authenticate device %s\n", sip_get_header(req, "From"));
-			transmit_response(p, "403 Forbidden", req);
+			send_check_user_failure_response(p, req, res, XMIT_UNRELIABLE);
 			sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
 			return;
 		}
@@ -25857,8 +25904,7 @@
 			return 0;
 		}
 		if (res < 0) { /* Something failed in authentication */
-			ast_log(LOG_NOTICE, "Failed to authenticate device %s\n", sip_get_header(req, "From"));
-			transmit_response(p, "403 Forbidden", req);
+			send_check_user_failure_response(p, req, res, XMIT_UNRELIABLE);
 			sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
 			return 0;
 		}
@@ -26651,8 +26697,7 @@
 			goto request_invite_cleanup;
 		}
 		if (res < 0) { /* Something failed in authentication */
-			ast_log(LOG_NOTICE, "Failed to authenticate device %s\n", sip_get_header(req, "From"));
-			transmit_response_reliable(p, "403 Forbidden", req);
+			send_check_user_failure_response(p, req, res, XMIT_RELIABLE);
 			p->invitestate = INV_COMPLETED;
 			sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
 			goto request_invite_cleanup;
@@ -28375,8 +28420,7 @@
 		p->lastinvite = seqno;
 		return 0;
 	} else if (auth_result < 0) {
-		ast_log(LOG_NOTICE, "Failed to authenticate device %s\n", sip_get_header(req, "From"));
-		transmit_response(p, "403 Forbidden", req);
+		send_check_user_failure_response(p, req, auth_result, XMIT_UNRELIABLE);
 		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
 		ast_string_field_set(p, theirtag, NULL);
 		return 0;
@@ -28598,9 +28642,7 @@
 		if (res == AUTH_CHALLENGE_SENT)	/* authpeer = NULL here */
 			return 0;
 		if (res != AUTH_SUCCESSFUL) {
-			ast_log(LOG_NOTICE, "Failed to authenticate device %s for SUBSCRIBE\n", sip_get_header(req, "From"));
-			transmit_response(p, "403 Forbidden", req);
-
+			send_check_user_failure_response(p, req, res, XMIT_UNRELIABLE);
 			pvt_set_needdestroy(p, "authentication failed");
 			return 0;
 		}

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13761
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I968dcf6c1e30ecddcce397dcda36db727c83ca90
Gerrit-Change-Number: 13761
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200206/7e8de1ea/attachment-0001.html>


More information about the asterisk-code-review mailing list