[Asterisk-code-review] udptl: Don't eat sequence numbers until OK is received (asterisk[certified/13.1])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: udptl:  Don't eat sequence numbers until OK is received
......................................................................


udptl:  Don't eat sequence numbers until OK is received

Scenario:
Local fax -> Asterisk w/ firewall -> Provider -> Remote fax

* Local fax starts rtp call to remote fax
* Remote fax starts t38 call back to local fax.
* Local fax sends t38 no-signal to Asterisk before sending an OK.
* udptl processes the frame and increments the expected sequence number.
* chan_sip drops the frame because the call isn't up so nothing goes out
  the external interface to open the port for incoming packets.
* Local fax sends OK and Asterisk sends OK to the remote fax.
* Remote fax sends t38 packets which are dropped by the firewall.
* Local fax re-sends t38 no-signal with the same sequence number.
* udptl drops the frame because it thinks it's a dup.
* Still no outgoing packets to open the firewall.
* t38 negotiation fails.

The patch drops frames t38 received before udptl sequence processing
when the call hasn't been answered yet.  The second no-signal frame
is then seen as new and is relayed out the external interface which
opens the port and allows negotiation to continue.

ASTERISK-26034 #close

Change-Id: I11744b39748bd2ecbbe8ea84cdb4f3c5943c5af9
---
M main/udptl.c
1 file changed, 12 insertions(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve; Verified



diff --git a/main/udptl.c b/main/udptl.c
index ba1f6da..347793e 100644
--- a/main/udptl.c
+++ b/main/udptl.c
@@ -772,6 +772,18 @@
 		return &ast_null_frame;
 	}
 
+	/*
+	 * If early media isn't turned on for the channel driver, it's going to
+	 * drop this frame.  By that time though, udptl has already incremented
+	 * the expected sequence number so if the CPE re-sends, the second frame
+	 * will be dropped as a dup even though the first frame never went through.
+	 * So we drop the frame here if the channel isn't up. 'tag' is set by the
+	 * channel drivers on T38_ENABLED or T38_PEER_REINVITE.
+	 */
+	if (udptl->tag == NULL) {
+		return &ast_null_frame;
+	}
+
 	if (udptl->nat) {
 		/* Send to whoever sent to us */
 		if (ast_sockaddr_cmp(&udptl->them, &addr)) {

-- 
To view, visit https://gerrit.asterisk.org/2871
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I11744b39748bd2ecbbe8ea84cdb4f3c5943c5af9
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>