meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.17k stars 2.47k forks source link

[1.x] Sip plugin crash on Options #3388

Closed do-not-set-2fa closed 3 months ago

do-not-set-2fa commented 3 months ago

Version: sip-trunk branch

I have noticed that it keeps crashing on my OPTIONS messages.

These are the last logs and gdb before/after SEGV:

[janus sip trunk crash and log: https://pastebin.com/RJmUvb3C

and OPTIONS are as normal as it can get :slight_smile:

https://pastebin.com/p8tbM4HJ

Only thing is that SIP routers that are not know in sip-trunk settings on janus are also sending options to it, but Janus do respond to them, but in log I do see mention of (null)

lminiero commented 3 months ago

It looks like an issue looking up the To in the hashtable. I see the OPTIONS has the To set to

To: <sip:JANUS_IP:5080>.

This means that our

const char *to = sip->sip_to->a_url[0].url_user;

is probably NULL or empty, which makes the g_hash_table_lookup(sip_trunk->sessions, to) call crash. I'll try to improve that check, so that you can see if it fixes things for you.

lminiero commented 3 months ago

@do-not-set-2fa can you check the commit above?

do-not-set-2fa commented 3 months ago

Hey Lorenzo, It is indeed fixed, surviving already 10min of sip options hammering (6 messages per minute :)

Options for trunk type are a bit loose, no need for proper to i guess.

Thanks for the quick fix, we will do more poc/testing on this branch soon i guess.

One unrelated question, maybe more for a discord: Now that there is a trunk, can there be a bypass media mode? For example you have another sip sbc that can talk to user so no need for janus to ne in the media path (less jumps, less delay and potential breakpoints), just to do webrtc json to sip signaling conversion and keep original sdp in offer and answer intact? Rtpengine does the kernel udp forwarding and can bring it back to user space when needed, so it is much faster and can handle more media streams that way. (Also would be wery cool to have kernalization in janus ofcourse, that would be even better, set up a stream in user space and move it to kernel space if there is no need for transcoding and such)

Thanks, Mirko

lminiero commented 3 months ago

It is indeed fixed, surviving already 10min of sip options hammering (6 messages per minute :)

Is Janus sending a 200 in response to those OPTIONS, or a 404? If it's a 404, there still might be something we need to fix I guess.

Now that there is a trunk, can there be a bypass media mode?

No, Janus and the SIP plugin will alwaye be on the media path.

Also would be wery cool to have kernalization in janus ofcourse

I don't think this is viable, due to the general purpose nature of Janus. Different plugins will need to handle media in different ways, there's not just a core relaying media from A to B and viceversa. As such, it's not planned for now.

do-not-set-2fa commented 3 months ago

No, all my sip replies were 200 OKed, the known and unknown ips.

Olk thanks for the info and fast replies as always :)

lminiero commented 3 months ago

No, all my sip replies were 200 OKed, the known and unknown ips.

Mh, maybe it shouldn't be 200 for unknown IPs, but we'll see in the future. Thanks for the feedback!