I would be happy to open a ticket for this, but given that I'm a newb when
it comes to the source, I'd like some confirmation of what I've found
before I do.
According to RFC3261 8.1.1.5 "The sequence number value MUST be expressible
as a 32-bit unsigned integer and MUST be less than 2**31"
I also checked RFC2543 just for fun and similar wording is there.
A client just informed me that he is seeing errors in the field where
FreeSWITCH is generating CSeq headers with a negative sequence number and
those requests are getting rejected by other endpoints.
=====================
In digging through the code, I found
MASTER/src/mod/endpoints/mod_sofia/sofia_presence.c
line 2134 defines a function sofia_presence_get_cseq() which appears to be
responsible for creating the sequence value that ends up in the CSeq
headers.
A cursory glance at this logic leads me to believe that it is definitely
capable of creating a sequence number with bit 31 set, which would be in
violation of the RFC.
I believe a simple fix with little side-effect would be to add the
following line right before last_cseq is set ( line 2149 in MASTER ):
}
}
+ callsequence &= 0x7FFFFFFF; // RFC3261 8.1.1.5 prohibits bit 31 to be set
profile->last_cseq = callsequence;
=====================
Another small thing is that FreeSWITCH is generating a negative value in
certain scenarios when outputting a CSeq header. This means that the
sequence value is improperly being interpreted as a signed integer. This
shouldn't matter because the high order bit isn't supposed to be set, but
regardless the code is still technically wrong.
In MASTER/libs/sofia-sip/libsofia-sip-ua/sip/sip_basic.c I found a
definition of sip_cseq_e() which correctly interprets the value as unsigned.
There are a few logging examples that are wrong that I found using a simple
grep, although these line #'s are probably off because it's from an old
checkout. The "CSeq %d" in each of the following probably needs to be a
"CSeq %u"
nta\sl_utils_log.c:76 "%s%s "URL_FORMAT_STRING" (CSeq %d %s)\n",
nta\sl_utils_log.c:100 "%s%03u %s (CSeq %d %s)\n",
nta\sl_utils_print.c:69 "%s%s "URL_FORMAT_STRING" (CSeq %d %s)\n",
nta\sl_utils_print.c:86 "%s%03u %s (CSeq %d %s)\n",
However in MASTER/src/mod/endpoints/mod_sofia/sofia.c I found the following:
line 381:
-switch_snprintf(sip_cseq, sizeof(sip_cseq), "%d", sip->sip_cseq->cs_seq);
+switch_snprintf(sip_cseq, sizeof(sip_cseq), "%u", sip->sip_cseq->cs_seq);
line 1707:
-switch_snprintf(sip_cseq, sizeof(sip_cseq), "%d", sip->sip_cseq->cs_seq);
+switch_snprintf(sip_cseq, sizeof(sip_cseq), "%u", sip->sip_cseq->cs_seq);
=====================
Something like the following might also be a consideration, but probably
only in 1.8:
In MASTER/libs/sofia-sip/libsofia-sip-ua/sip/sip_basic.c line 1288 add
something like the following:
if ( seq & 0x80000000 )
{
SU_DEBUG_1(("%s: CSeq header creation failed - sequence value %u
violates RFC3261/8.1.1.5", __func__, seq ));
return NULL;
}
P.S. I'd really like to see at least the first 2 changes proposed above (
callsequence masking and format string fixes ) back-ported to 1.6.
Royce Mitchell, IT Consultant
ITAS Solutions
royce3(a)itas-solutions.com