<div dir="ltr"><div>Hi<br><br>The attached patch fixes RLS module use of pres_delete_shtable - its fairly straight forward as pres_delete_shtable is only used once and not exported as I thought it might be.<br><br></div>If everyone is in agreement I will commit this change and the presence module change to the master branch.<br>
<br>Regards<br>Richard.<br><br><br><br><br><br><div class="gmail_extra"><div class="gmail_quote"> 24 January 2014 18:04, Andrew Mortensen <span dir="ltr"><<a href="mailto:admorten@isc.upenn.edu" target="_blank">admorten@isc.upenn.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im"><br>
On Jan 24, 2014, at 10:54 AM, Richard Good <<a href="mailto:richard.good@smilecoms.com">richard.good@smilecoms.com</a>> wrote:<br>
<br>
> Hi<br>
><br>
> The attached patch fixes our problem.<br>
><br>
> As you suggested delete_shtable takes a subs_t pointer as an argument and compares the dialog's full tag set before deleting.<br>
><br>
> 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.<br>
<br>
</div>Looks good to me. Thanks.<br>
<br>
andrew<br>
<div><div class="h5"><br>
<br>
<br>
><br>
> On 23 January 2014 17:24, Andrew Mortensen <<a href="mailto:admorten@isc.upenn.edu">admorten@isc.upenn.edu</a>> wrote:<br>
><br>
> On Jan 23, 2014, at 10:06 AM, Jason Penton <<a href="mailto:jason.penton@gmail.com">jason.penton@gmail.com</a>> wrote:<br>
><br>
> > Hey Andrew,<br>
> ><br>
> > 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.<br>
> ><br>
> > Can I go ahead?<br>
><br>
> You have my support. Post the patch here for review.<br>
><br>
> andrew<br>
><br>
><br>
><br>
> ><br>
> > On Thu, Jan 23, 2014 at 5:00 PM, Andrew Mortensen <<a href="mailto:admorten@isc.upenn.edu">admorten@isc.upenn.edu</a>> wrote:<br>
> ><br>
> > On Jan 23, 2014, at 9:19 AM, Jason Penton <<a href="mailto:jason.penton@gmail.com">jason.penton@gmail.com</a>> wrote:<br>
> ><br>
> > > Hey all,<br>
> > ><br>
> > > 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:<br>
> > > [snip]<br>
> > > 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...<br>
> ><br>
> > At least nominally the tag should be globally unique, per RFC3261:<br>
> ><br>
> > 19.3 ... When a tag is generated by a UA for insertion into a request or<br>
> > response, it MUST be globally unique and cryptographically random<br>
> > with at least 32 bits of randomness.<br>
> ><br>
> > But I agree that poorly behaved UAs could cause problems here. (We actually ran into duplicate to-tags recently with vendor handsets booted simultaneously.)<br>
> ><br>
> > 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.<br>
> ><br>
> > 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.<br>
> ><br>
> > andrew<br>
> > _______________________________________________<br>
> > sr-dev mailing list<br>
> > <a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
> > <a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
> ><br>
> > _______________________________________________<br>
> > sr-dev mailing list<br>
> > <a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
> > <a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
><br>
><br>
> _______________________________________________<br>
> sr-dev mailing list<br>
> <a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
> <a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
><br>
</div></div>> This email is subject to the disclaimer of Smile Communications at <a href="http://www.smilecoms.com/disclaimer" target="_blank">http://www.smilecoms.com/disclaimer</a><br>
><br>
><br>
> <presence_mod.diff>_______________________________________________<br>
<div class=""><div class="h5">> sr-dev mailing list<br>
> <a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
> <a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
<br>
<br>
_______________________________________________<br>
sr-dev mailing list<br>
<a href="mailto:sr-dev@lists.sip-router.org">sr-dev@lists.sip-router.org</a><br>
<a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev</a><br>
</div></div></blockquote></div><br></div></div>
<pre>This email is subject to the disclaimer of Smile Communications at http://www.smilecoms.com/disclaimer