[reSIProcate] CSeq is incorrect in NOTIFY generated by DUM on SUBSCRIBE

Adam Roach adam at nostrum.com
Tue Feb 24 13:15:33 CST 2009


Hmmm... given that initial CSeq must be in the range of [0,2^31), and 
max CSeq is in the range of [0,2^32), you have somewhat more than 2 
billion transactions before you roll the CSeq space. I know the US 
Congress has kinda numbed us to the real meaning of "billion," but that 
would take a long time... about 68 years, if you send one transaction 
per second.

/a

Scott Godin wrote:
> I just did a quick scan of the code - it seems that the only time we 
> check CSeq ordering is for ClientISubscriptions when receiving NOTIFY 
> requests.
>
> Perhaps we should add a CSeq order check to Dialog.cxx, instead of 
> just removing mRemoteCSeq.  We will just need to be careful about 
> being tolerant of CSeq roll overs.
>
> Scott
>
> On Tue, Feb 24, 2009 at 12:06 PM, Robert Sparks <rjsparks at nostrum.com 
> <mailto:rjsparks at nostrum.com>> wrote:
>
>     We don't check for out-of-order requests?
>
>
>     On Feb 24, 2009, at 10:59 AM, Adam Roach wrote:
>
>         I think (hope) Jason was talking about removing the "remote
>         cseq" field. I'd wait for him to get to a non-virtual keyboard
>         -- where he can examine the code himself -- for clarification.
>
>         /a
>
>
>         Robert Sparks wrote:
>
>             Well, if there was to be a change, it would not be to use
>             the CSeq from the other side, but rather to start at
>             somewhere random instead of always at 1.
>
>             While I don't really see a need to change to this random
>             start (because I don't see any benefit), I'm really
>             uncomfortable with the claim you're making here that it
>             might do harm.
>
>             (I agree with watching out for unintended consequences in
>             general, but for this particular instance, if the change
>             broke some peer, then introducing a challenging proxy, for
>             instance,
>             would break that same peer).
>
>             RjS
>
>             On Feb 24, 2009, at 9:52 AM, Jason Fischl wrote:
>
>                 My gut feeling here is to leave it unchanged. These
>                 kind of changes may have unintended consequences.
>
>                 Sent from my iPhone
>
>                 On Feb 24, 2009, at 6:55, Adam Roach <adam at nostrum.com
>                 <mailto:adam at nostrum.com>> wrote:
>
>                     Volodymyr:
>
>                     Thanks for the suggestion.
>
>                     In terms of the local CSeq: According to RFC 3261,
>                     The CSeq number space is unique in each direction.
>                     In other words, the CSeq in the NOTIFY is
>                     completely unrelated to the CSeq in the SUBSCRIBE.
>                     The original code is correct.
>
>                     I agree with you that the remote CSeq is not used,
>                     and can likely be removed. I'd be interested to
>                     have Scott, Jason, and Derek weigh in on whether
>                     this can be safely removed, or if there are some
>                     future plans for it.
>
>                     /a
>
>                     Volodymyr Tarasenko wrote:
>
>                         Hi All,
>
>                         As I see dum is incorrectly generates CSeq in
>                         NOTIFY in case when SUBSCRIBE's SCeq was not
>                         1. NOTIFY's CSeq always starts from 2 in spite
>                         of CSeq in SUBSCRIBE was not 1.
>                         The fast patch is very simple:
>
>                         --- resip/dum/Dialog.cxx
>                         +++ resip/dum/Dialog.cxx
>                         @@ -136,7 +136,7 @@
>                            }
>
>                            mRemoteCSeq =
>                         request.header(h_CSeq).sequence();
>                         -      mLocalCSeq = 1;
>                         +      mLocalCSeq =
>                         request.header(h_CSeq).sequence();
>
>                            DebugLog ( << "************** Created
>                         Dialog as UAS **************" );
>                            DebugLog ( << "mRemoteNameAddr: " <<
>                         mRemoteNameAddr );
>
>                         Also, after code review I've found that
>                         mRemoteCSeq is never used and looks like it
>                         could be safety removed.
>
>                         Regards,
>                         Volodymyr!
>
>
>                           _______________________________________________
>                         resiprocate-devel mailing list
>                         resiprocate-devel at resiprocate.org
>                         <mailto:resiprocate-devel at resiprocate.org>
>                         https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
>
>                     _______________________________________________
>                     resiprocate-devel mailing list
>                     resiprocate-devel at resiprocate.org
>                     <mailto:resiprocate-devel at resiprocate.org>
>                     https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
>                 _______________________________________________
>                 resiprocate-devel mailing list
>                 resiprocate-devel at resiprocate.org
>                 <mailto:resiprocate-devel at resiprocate.org>
>                 https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
>
>
>
>     _______________________________________________
>     resiprocate-devel mailing list
>     resiprocate-devel at resiprocate.org
>     <mailto:resiprocate-devel at resiprocate.org>
>     https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
>




More information about the resiprocate-devel mailing list