lminiero
commented
1 year ago The SIP plugin only supported IPv4 before, now it supports both IPv4 and IPv6, and we use an IPv6 socket to bind to both if that's needed. local_media_ip could be either, and so would need additional and custom code to only bind to that, a bit like the Streaming plugin does. I'm not against that, of course, but to be honest I don't have time to work on that now (vacations are at the door). If you're willing to write a pull request, I'll be happy to review it though.
pawnnail
Have started looking at the multistream implementation of SIP gateway. Have noticed that the SIP plugin now unconditionally binds to any network interface for media. The singelstream version binds to the address specified by _local_mediaip (otherwise, to a single local address).
It may be a security risk since the media sockets unsuspectingly receive traffic from any network interface unrelated to the _local_mediaip advertised by SDP. Additionally, the procedure requires a target UDP port to be available on all interfaces that may be resource wasteful if successful or more prone to the bind failure otherwise.
The change was introduced by this commit. I read through the comments but couldn't see a compelling reason for the change of behaviour. It may well be I am missing something.
Prior to the commit we had what the singlestream version still has:
inet_pton(AF_INET, (local_media_ip ? local_media_ip : local_ip), &audio_rtp_address.sin_addr.s_addr);This has changed to
audio_rtp_address.sin6_addr = in6addr_any;and INADDR_ANY for ipv4.Could we please review the change again and consider an appropriate correction unless there is a good reason for not to. Thanks