Hi
The attached patch fixes our problem.
As you suggested delete_shtable takes a subs_t pointer as an argument and
compares the dialog's full tag set before deleting.
If everyone is OK with this patch, on Monday I will look at a patch for the
RLS module - it looks a little bit tricky as I think RLS exports the
pres_delete_shtable through its own API as rls_delete_shtable, though I'm
not 100% sure on this.
Regards
Richard.
On 23 January 2014 17:24, Andrew Mortensen <admorten(a)isc.upenn.edu> wrote:
On Jan 23, 2014, at 10:06 AM, Jason Penton <jason.penton(a)gmail.com> wrote:
Hey Andrew,
Shot for the prompt response. I am happy to make these changes if we all
agree...
I was concerned like you of the impact on the consumers, but as
you say just RLS.
Can I go ahead?
You have my support. Post the patch here for review.
andrew
On Thu, Jan 23, 2014 at 5:00 PM, Andrew Mortensen <
admorten(a)isc.upenn.edu>
wrote:
On Jan 23, 2014, at 9:19 AM, Jason Penton <jason.penton(a)gmail.com>
wrote:
> Hey all,
>
> I was taking a look at the presence module API code, specfically the
API code
for the hash table. I see that when we delete a subscription
according to hash.c:
> [snip]
> Why are we only searching on to-tag? What if there is a collision on
the hash
AND the to-tag is the same for 2+ different subscriptions. This is
even more likely of happening considering that the hash calculation is
based only on the callid and to-tag...
At least nominally the tag should be globally unique, per RFC3261:
19.3 ... When a tag is generated by a UA for insertion into a request or
response, it MUST be globally unique and cryptographically random
with at least 32 bits of randomness.
But I agree that poorly behaved UAs could cause problems here. (We
actually ran
into duplicate to-tags recently with vendor handsets booted
simultaneously.)
It looks to me like we should be calling search_shtable, which compares
the
dialog's full tag set. The problem is that unlike other similar calls,
delete_shtable doesn't take a subs_t pointer as an argument, so the only
point of reference is the to-tag. Fortunately it looks like there's only
one place delete_shtable's called in the presence module, so it shouldn't
be much work to start using search_shtable to find the subscription to
delete.
To complicate things further, though, delete_shtable is exported as part
of the
presence API, so we'd need to update all API consumers as well. That
doesn't seem unreasonable, as a quick git grep indicates the only current
user of the presence API is the rls module.
andrew
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev