[Home]

Summary:ASTERISK-19389: Sending ACK in CANCLE dialog to the wrong destination
Reporter:Karsten Wemheuer (kwemheuer)Labels:
Date Opened:2012-02-20 04:49:53.000-0600Date Closed:2012-02-23 09:39:19.000-0600
Priority:BlockerRegression?
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:1.8.9.2 Frequency of
Occurrence		Constant
Related
Issues:
must be completed before resolvingASTERISK-19128 Asterisk 1.8.10 Blockers
must be completed before resolvingASTERISK-19129 Asterisk 10.2.0 Blockers
Environment:Debian Squeeze, Asterisk 1.8.9.2, patched with solution for ASTERISK-19358, OpenSIPSAttachments:( 0) ASTERISK-19389.patch
( 1) ASTERISK-19389v2.patch
( 2) ngrep-commented.txt
Description:I got a problem with asterisk 1.8.9.2. The same scenario is working fine in 1.8.8.2. My main problem described in ASTERISK-19358 is fixed by the solution for the issue 19358. For the following scenario I use asterisk 1.8.9.2 with the patch of issue #19358 already integrated.

Asterisk calls a SIP phone via a proxy. Proxy, phone and asterisk are on the same LAN, no NAT.

As in issue #19358 an INVITE is sent by asterisk via proxy to a phone. The phone is ringing. Asterisk then cancels the call (originator giving up). The CANCEL is sent via proxy to the phone (Thanks to the patch). The phone sends "200 OK" and "487 Request terminated" to the proxy. The proxy sends ACK to the phone and all is fine on this side. The proxy also sends "200 cancelling" and 487 Request terminated" to asterisk. And here starts the diff to asterisk 1.8.8.2: In the former version asterisk sends ACK to the proxy but in 1.8.9.2 asterisk sends ACK to the phone. The proxy repeats his 487 message many times before giving up. This is no big thing, because there is no failure in communication. Only resources at the proxy are bound for a longer time, I think.

Is this a bug in asterisk or am I doing something wrong (that was tolerated in 1.8.8.2)?
Comments:By: Karsten Wemheuer (kwemheuer) 2012-02-20 04:51:30.282-0600

Trace created with ngrep, commented and shortend to the (hopefully) relevant parts.
By: Mark Michelson (mmichelson) 2012-02-20 09:12:44.264-0600

Have you tried applying both patches from 19358? There are two different patches (although they have the same name) and both are needed in order to fix the problems you're experiencing.
By: Karsten Wemheuer (kwemheuer) 2012-02-20 09:29:19.788-0600

Yes, both patches are applied.
By: Mark Michelson (mmichelson) 2012-02-20 12:51:49.127-0600

For my reference, here are some passages from RFC 3261 that explain this:

Section 17.1.1.2:

"...reception of a response with status code from 300-699 MUST cause the client transaction to transition to "Completed." ... and the client transaction must generate an ACK request ... The ACK MUST be sent to the same address, port, and transport to which teh original request was sent."

So this is saying that an ACK for a final response that is non-2XX needs to be sent to the same place as the INVITE was.

Section 17.1.1.3:
"If the INVITE request whose response is being acknowledged had Route header fields, those header fields MUST appear in the ACK. This is to ensure that the ACK can be routed properly through any downstream stateless proxies."

This is in the section describing UAC ACK construction upon receipt of a 2xx response.

My proposed fix for this is to alter build_route so that when a non-2xx final response is received, we will not build a route set. I will have a patch up as soon as I can.
By: Mark Michelson (mmichelson) 2012-02-20 14:00:06.479-0600

This should take care of the problem. I decided to go a bit differently from what I described in my previous comment. The reason is that I realized there was a possible other bug in the code regarding where requests are routed. So I have now isolated the two conditions for which we should not use the route set rather than trying to build the route set only other special conditions. See if ASTERISK-19389.patch solves the issue for you.
By: Karsten Wemheuer (kwemheuer) 2012-02-21 02:34:57.395-0600

Unfortunately the patch is not working. I do some debugging and digging in the code. The received 487 response is handled in function handle_response_invite(). In the following big switch statement, 487 leads to a call of function transmit_request() to send the ACK. At the beginning of this function the statements
{code}
if (sipmethod == SIP_ACK) {
p->invitestate = INV_CONFIRMED;
}
{code}
leads to the result, that Your statement
{code}
if (sipmethod == SIP_CANCEL ||
(sipmethod == SIP_ACK && p->invitestate == INV_COMPLETED)) {
set_destination(p, ast_strdupa(p->uri));
} ...
{code}
never gets executed for the ACK.

If I change INV_COMPLETED to INV_CONFIRMED in Your piece of code, the ACK will be sent to the proxy. But I am not sure, that this is a valid solution. I am not sure, that this would not break anything else.
By: Stefan Schmidt (schmidts) 2012-02-21 06:06:35.652-0600

INV_COMPLETED looks wrong to me cause pending to the definition in sip.h it should be INV_CONFIRMED:
       INV_COMPLETED = 4,    /*!< Got final response with error. Wait for ACK, then CONFIRMED */
       INV_CONFIRMED = 5,    /*!< Confirmed response - we've got an ack (Incoming calls only) */


By: Mark Michelson (mmichelson) 2012-02-21 11:22:57.079-0600

Ah, I see the problem. The state is set to INV_COMPLETED when the INVITE response is received. The problem is that it gets changed to INV_CONFIRMED right before we are about to call reqprep to send the ACK. I can't simply change to using INV_CONFIRMED now because we only want to ignore the route set if we are sending an ACK for a non-2xx final response. The state will be INV_CONFIRMED no matter what the response to our INVITE was. I will have to come up with a different method of ignoring the route set for this particular ACK scenario.
By: Mark Michelson (mmichelson) 2012-02-21 12:03:51.785-0600

I have uploaded ASTERISK-19389v2.patch that should hopefully do the trick. Please see if it works.
By: Karsten Wemheuer (kwemheuer) 2012-02-22 07:43:27.780-0600

No, unfortunately the patch is not working as expected. I put some debug statements in the code. The value of p->invitestate is now 7 in case of the ACK being processed. It sounds reasonable to me, as the call is actually cancelled. So I _think_ changing the if statement to
{code}
if (sipmethod == SIP_CANCEL ||
(sipmethod == SIP_ACK && p->invitestate == INV_CANCELLED)) {
set_destination(p, ast_strdupa(p->uri));
} else {
...
{code}
would do the trick. The patch is working with this modification, but I am not sure about any side effects. I am not very familiar with this big peace of code, so I may be wrong.
By: Mark Michelson (mmichelson) 2012-02-23 09:24:04.115-0600

That won't work for the general case where an ACK is sent in response to a a non 2xx final response. So I think you've found another case that needs to be tested. So it should be

{code}
if (sipmethod == SIP_CANCEL ||
   (sipmethod == SIP_ACK && (p->invitestate == INV_CANCELLED || p->invitestate == INV_COMPLETED))) {
   set_destination(p, ast_strdupa(p->uri);
} else {
...
{code}

That would handle sending the ACK to the proper place in a normal INVITE rejection scenario as well as when responding to a 487 due to a cancellation.
By: Mark Michelson (mmichelson) 2012-02-23 09:24:41.264-0600

Since you have confirmed that adding the INV_CANCELLED fixes the issue, I'm going to go ahead and commit the change that has the code change I mentioned in my previous comment.