[asterisk-bugs] [Asterisk 0012963]: [patch] chan_iax2 will create multiple sessions when receiving retransmitted NEW

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=12963 
====================================================================== 
Reported By:                jpgrayson
Assigned To:                
====================================================================== 
Project:                    Asterisk
Issue ID:                   12963
Category:                   Channels/chan_iax2
Reproducibility:            always
Severity:                   major
Priority:                   normal
Status:                     new
Asterisk Version:           1.4.21 
SVN Branch (only for SVN checkouts, not tarball releases): N/A 
SVN Revision (number only!):  
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             06-30-2008 18:48 CDT
Last Modified:              07-02-2008 10:07 CDT
====================================================================== 
Summary:                    [patch] chan_iax2 will create multiple sessions when
receiving retransmitted NEW
Description: 
When a client initiates a call with a NEW frame, frequently the client will
send a retransmitted NEW frame prior to receiving the ACCEPT frame from
asterisk. This may result in asterisk receiving duplicate NEW frames from
the same client in short succession.

As of at least 1.4.20, asterisk will ACCEPT and create new call sessions
for both of these NEW frames originating from the same client. This results
in much protocol confusion between asterisk and the client and ultimately
leads to asterisk and/or the client terminating the call.

The correct behavior would be for asterisk to recognize second (and third
and fourth) NEW frames from the same source (socket and callno) and ignore
them.

====================================================================== 

---------------------------------------------------------------------- 
 jpgrayson - 07-02-08 10:07  
---------------------------------------------------------------------- 
> I need three things changed with this patch before I'm comfortable with
> committing it.
> 
> a) Remove gratuitous changes in spacing, so that the code change
> reflects just that.

Okay. I'll submit any whitespace-oriented stuff as supplemental patches.

> b) Anywhere where you've changed possible semantics, I need to see a
> comment in the code, just in case the semantics are incorrect. This
will
> make finding those cases easier in the future.

I really don't know what you want here. Any non-whitespace change has
the possibility to change semantics. The code says what the semantic is;
beyond that comments are for the "why" not the "what". If there are
specific places you have in mind, I'll do my best to comment them.

> c) In draft 4 of the IETF IAX2 specification, section 6.2.2, the
> document states that the destination call number is not required. It
> does NOT say what value it should be, so a value other than zero is
> possible. Asterisk should therefore not require this field to be zero.
> That needs to be changed in your patch to submit an explicit zero in
the
> dcallno parameter.

I see that interpretation now. I might suggest that the spec could be
clarified. e.g. "for a NEW frame ... the destination call identifier in
the header must be ignored by the receiver". Also, it might be
reasonable for the spec to demand that the sender set the destination
call identifier to 0, but maybe there are historical reasons why that is
not the case.

> Additionally, I'm interested in confirming that your IAX2 client is
> following the specification for retransmission timeout by using twice
> the last POKE/PONG response time (Section 7.2.1).

Nowhere in the spec does it specify what value to assume for the initial
round trip time. When that first NEW frame goes out, the client will not
have had an actual PING/PONG interaction with the server yet, so an
arbitrary number needs to be chosen. I am using iaxclient, it sets the
initial round trip time to 100ms.

In terms of protocol, none of this matters of course because UDP frames
have no guarantee to arrive on time or in order. Asterisk needs to be
able to deal with duplicate NEW frames.

Another problem with the patch:

Upon closer reading of the section 6.2.2, it appears that asterisk must
send an ACCEPT frame in response to this duplicate NEW frame. The patch
as it stands does not yield this behavior. Asterisk will just ignore the
duplicate NEW frame. The spec seems correct in this case. When asterisk
encounters a duplicate NEW frame, it cannot assume that the client
received its first ACCEPT frame.

I will rework the patch to take all of these things into account. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
07-02-08 10:07  jpgrayson      Note Added: 0089601                          
======================================================================