Change the way call-IDs are tracked in the SIP plugin (fixes #3404)

[lminiero]

This PR is an attempt to address the problem @ycherniavskyi reported in issue #3404. We have to thank Yurii for this fix as well, as his company, @WebTrit, sponsored the effort to investigate the issue and come up with a potential solution. Thanks, guys!

Coming to the core of the issue, Yurii identified an inconsistency in how Call-IDs were used as ways to address sessions in the SIP plugin, that would occur specifically when caller and callee were handled by the same Janus instance, and so the map would be overwritten. This could cause problems in two specific cases:

1. When using event handlers to notify incoming/outgoing SIP messages to an external backend (e.g., Homer) and mapping them to the plugin session (and so Janus handle) they belonged to.
2. When doing attended SIP transfers to fill in the Replaces header properly.

This PR introduces a new kind of mapping where we don't map the call-ID to a single session, but instead map it to a new structure that can keep track of both a caller and a callee, by keeping track of caller tags too. That structure is only updated with info on sessions handled by the Janus instance, and allows us to track both parties in a call from the same call-ID. This allowed us to solve the two problems above in two separate ways:

1. For event handlers, we find the mapping from call-ID, and then check From/To headers to find the right session looking at tags in the request.
2. For attended transfers, we limit the scope of lookups to sessions handled by the session implementing the transfer: in fact, transfers in Janus rely on the concept of master/helper sessions to implement multiple lines, and limiting the scope to those lines allows us to avoid ambiguities.

As usual, while this PR is currently for master, I'll backport it to 0.x once it's merged. I tested the PR briefly and it seems to work, but of course my ability to test SIP deployments (and those use cases in particular) is limited. I'm looking forward to feedback, to ensure that first of all this fixes the two problems we mentioned, but also that we're not introducing regressions with these changes. If you use SIP extensively in your deployments, please make sure to test this and give us all the feedback we need to make sure it's merged as soon as possible.

[lminiero]

Pinging @lmangani as well, since before this fix SIP messages sent to Homer may have been misattributed by the core.

[ycherniavskyi]

@lminiero We've successfully built Janus with this PR and are beginning our testing process. Please expect a detailed feedback report within several days.

[lminiero]

@ycherniavskyi any feedback on this?

[ycherniavskyi]

@lminiero, unfortunately, we encountered two problematic cases that appear to be related to the same root issue. In both instances, Janus crashed (in our environment, the Janus container exited with code 139).

First case

Two accounts, Client A and Client B, are registered to the same Janus instance via our prospect softswitch.

Steps to reproduce:

1. Client A initiates a call to Client B.
2. Client B receives the incoming call from Client A.
3. Client B answers the call from Client A.
4. The call is successfully connected and proceeds normally.
5. Client A drops the call with Client B.
6. Upon receiving the BYE message for Client B, Janus crashes.

Here is the call flow and detailed SIP message log for this case: Link to Gist (IP addresses, numbers, and names have been anonymized).

It’s also worth noting that this issue does not occur with all softswitches we’ve tested.

Second case

In this scenario, three accounts are registered on our prospect softswitch:

• Account 1 (Linphone)
• Account 2 (Janus)
• Account 3 (any other device)

Steps to reproduce:

1. Account 1 calls Account 2.
2. Account 1 transfers (either unattended or attended) the call to Account 3.
3. Account 2 and Account 3 are successfully connected and the call proceeds.
4. When either Account 2 or Account 3 ends the call, Janus crashes.

The only requirement for this crash is that a non-Janus initiates the transfer.

Since both cases seem related to the same issue, I didn’t collect a separate SIP dump for the second case. However, if needed, I can gather it ASAP.

Let me know if you need additional information or further logs.

[lminiero]

If there's crashes, I'll need a gdb stacktrace or, even better, a libasan dump, to see where it crashes. Just the SIP messages (which is what I see in that gist) won't help unfortunately.

[lminiero]

@ycherniavskyi while we wait for those crash logs I mentioned, the only thing I noticed in your log is that there seem to be many SIP BYEs that are never answered, I guess because, as you pointed out, Janus seems to be crashing when it gets the first one. That said, we don't do anything in nua_i_bye except save the reason for the BYE, and that wasn't changed in this PR. I personally can't replicate this crash, so I'll need debugging info from you to look into it.

[lminiero]

@ycherniavskyi I should have fixed the crash in the commit above: it was a double free, caused by the fact we were not strdup-ing the call-id when adding it as a key to the callids table in some cases, and so the string was freed twice (once by us, once by the table itself on removal).

[lminiero]

Merging ✌️