Hi Alexandre, Sorry for the lack of response earlier. Thanks for the patch. Unfortunately, by allowing multiple listeners, it affects other APIs as well, such as pjsip_tpmgr_acquire_transport() which will decide which transport it should use. When there are multiple listeners, it could make the hash table currently used to store the transports not sufficient to accommodate all the transports (the hash table will only store the last transport of a certain type to a certain destination), hence causing the transport selection process possibly inaccurate. Thus, a modification in the transport hash table key and the hash table lookup may be required as well. Regards, Ming On Tue, Apr 26, 2016 at 4:14 AM, Alexandre Viau <aviau@xxxxxxxxxx> wrote: > Hello, > > This patch was submitted two times in the past: > > October 2014: > http://lists.pjsip.org/pipermail/pjsip_lists.pjsip.org/2014-October/017905.html > > January 2015: > http://lists.pjsip.org/pipermail/pjsip_lists.pjsip.org/2015-January/018072.html > > On behalf of Adrien, I am trying again. > > Could you guys please take a look? I am sure Adrien and the folks at > Savoir-faire Linux will take the time to modify the patch should it be > needed. > > Ring (https://ring.cx) is going to be shipped in Debian. At the moment, > we have no choice but to ship a patched copy of pjsip along with Ring. > This is an unfortunate situation and we would love to get it sorted out. > > This is not the only pjsip patch that is needed for Ring. Other patches > were not submitted to this list because of the lack of response. It > would be a good start to get this one merged and then move on with others. > > Is there anything we can do to bring attention to this issue? > > Cheers, > > -- > Alexandre Viau > aviau@xxxxxxxxxx > > > ----- Mail original ----- > > Hello, > > I sent this message a few months ago, but got no reply :-( > > Could you please take a look ? > > Thanks > Adrien > > > ----- Mail original ----- > > De: "Adrien Béraud" <adrien.beraud at savoirfairelinux.com> > À: pjsip at lists.pjsip.org > Envoyé: Samedi 25 Octobre 2014 18:02:52 > Objet: Multiple transport listeners patch > > Hello, > > We noticed PJSIP only supports a single transport listener (tpfactory) > by transport type. > For instance, only a single TLS listener can be created for a given > PJSIP endpoint (or two with IPv4 + IPv6). > > This issue was reported multiple times over the years > (e.g https://trac.pjsip.org/repos/ticket/1019 > https://trac.pjsip.org/repos/ticket/1083 etc.) > > TLS was not heavily used before, but since last years it's kind of a big > priority for many to be able to setup secure communications. > UDP is not affected because it's connectionless, so there is no "listener". > > We looked at the sip_transport code and the fix was so simple that it > looked too good to be true. Maybe it is ? > > The patch is bellow, so please take a look and merge if you think it's > good. > For security reasons, the patch forces providing an explicit listener to > create a new TLS / encrypted transport, to avoid data being encrypted > with the wrong keys if the listener/factory was selected automatically. > > Cheers, > > Adrien Béraud, > SFLphone developper ( sflphone.org ) > Savoir-faire Linux Inc. > > > diff --git a/pjproject/pjsip/src/pjsip/sip_transport.c > b/pjproject_new/pjsip/src/pjsip/sip_transport.c > index 4b2cc1a..22e3603 100644 > --- a/pjsip/src/pjsip/sip_transport.c > +++ b/pjsip/src/pjsip/sip_transport.c > @@ -1179,22 +1179,22 @@ PJ_DEF(pj_status_t) > pjsip_tpmgr_register_tpfactory( pjsip_tpmgr *mgr, > > pj_lock_acquire(mgr->lock); > > - /* Check that no factory with the same type has been registered. */ > + /* Check that no factory with the same type and bound address has > been registered. */ > status = PJ_SUCCESS; > for (p=mgr->factory_list.next; p!=&mgr->factory_list; p=p->next) { > - if (p->type == tpf->type) { > - status = PJSIP_ETYPEEXISTS; > - break; > - } > - if (p == tpf) { > - status = PJ_EEXISTS; > - break; > - } > + if (p->type == tpf->type && !pj_sockaddr_cmp(&tpf->local_addr, > &p->local_addr)) { > + status = PJSIP_ETYPEEXISTS; > + break; > + } > + if (p == tpf) { > + status = PJ_EEXISTS; > + break; > + } > } > > if (status != PJ_SUCCESS) { > - pj_lock_release(mgr->lock); > - return status; > + pj_lock_release(mgr->lock); > + return status; > } > > pj_list_insert_before(&mgr->factory_list, tpf); > @@ -1909,13 +1909,11 @@ PJ_DEF(pj_status_t) > pjsip_tpmgr_acquire_transport2(pjsip_tpmgr *mgr, > pj_memcpy(&key.rem_addr, remote, addr_len); > > transport = (pjsip_transport*) > - pj_hash_get(mgr->table, &key, key_len, NULL); > - > + pj_hash_get(mgr->table, &key, key_len, NULL); > + unsigned flag = pjsip_transport_get_flag_from_type(type); > if (transport == NULL) { > - unsigned flag = pjsip_transport_get_flag_from_type(type); > const pj_sockaddr *remote_addr = (const pj_sockaddr*)remote; > > - > /* Ignore address for loop transports. */ > if (type == PJSIP_TRANSPORT_LOOP || > type == PJSIP_TRANSPORT_LOOP_DGRAM) > @@ -1954,6 +1952,11 @@ PJ_DEF(pj_status_t) > pjsip_tpmgr_acquire_transport2(pjsip_tpmgr *mgr, > return PJ_SUCCESS; > } > > + if (flag & PJSIP_TRANSPORT_SECURE) { > + TRACE_((THIS_FILE, "Can't create new TLS transport with no > provided suitable TLS listener.")); > + return PJSIP_ETPNOTSUITABLE; > + } > + > /* > * Transport not found! > * Find factory that can create such transport. > > > > _______________________________________________ > Visit our blog: http://blog.pjsip.org > > pjsip mailing list > pjsip@xxxxxxxxxxxxxxx > http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org > _______________________________________________ Visit our blog: http://blog.pjsip.org pjsip mailing list pjsip@xxxxxxxxxxxxxxx http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
