Fix RTPCHECK functionality regressions

[jeannotlanglois]

This PR resolves regressions in the RTPCHECK functionality which were introduced right when it was merged in SIPp-3.7.0~rc1 back in 2023.

• Fixed crash occurring on RE-INVITEs when processing SRTP crypto parameters (was caused by incorrect substitution of strncat() instead of memset()+strncpy() when RTPCHECK functionality was first introduced in 3.7.0~rc1).
• Restored code to use timestamps in RTP echo action log file names in rtpstream_rtpecho_startaudio() -- this had been mistakenly taken out when RTPCHECK functionality was first instroduced in 3.7.0~rc1.
• Restored code to use timestamps in RTP echo action log file names in rtpstream_rtpecho_startvideo() -- this had been mistakenly taken out when RTPCHECK functionality was first instroduced in 3.7.0~rc1.
• Modified rtpstream_rtpecho_startaudio() to perform use safe open()/fdopen() APIs to restrict access to RTP echo action log file.
• Modified rtpstream_rtpecho_startvideo() to perform use safe open()/fdopen() APIs to restrict access to RTP echo action log file.
• Added dummy custom JLSRTP class constructor to allow providing (SSRC, IP, port) at construction time when using non-SSL builds (so that encapsulation can be maintained without the need for externs).
• Modified call class to use custom JLSRTP class constructor (to maintain encapsulation without the need for externs).
• Modified rtpstream code to use custom JLSRTP constructor (to maintain encapsulation without the need for externs).
• Parameterized global_ssrc_id to allow either maintaining legacy SIPP behavior (default constant SSRC required by RTPCHECK functionality) or randomly generating them (varying SSRC).
• Modified main sipp code to properly apply parameterized global_ssrc_id parameter.

[jeannotlanglois]

Hello @wdoekes :

I'm finally back working full time in the IP telephony domain - which means that I'm back using/testing/improving SIPP :).

As I've had time to finally look, use and test the RTPCHECK functionality which was merged in SIPp-3.7.0 in 2023, I've noticed that several regressions were introduced right when my code was initially imported in SIPp-3.7.0~rc1 -- so the RTPCHECK functionality code that got included starting with SIPP-3.7.0 never quite fully functioned properly.

This PR resolves ALL these known issues.

I would like to suggest that the current PR's fix is used for a new SIPp-3.7.4 bugfix version.

I'm running out of time today but within the next day or two I expect to write more details at the top of this PR to explain every item addressed.

It's good to be back in IP telephony :)

Cheers!

[orgads]

@jeannotlanglois Thanks a lot for your contribution! Can you please address the comments so we can push this forward?

[jeannotlanglois]

Hi there: Sure, I just got delayed by a long work week... Still trying to get back to this as soon as I can this weekend... Jeannot Langlois Senior Software Developer ***@***.***

…

On Sun, Jul 20, 2025, 06:43 Orgad Shaneh ***@***.***> wrote: *orgads* left a comment (SIPp/sipp#798) <#798 (comment)> @jeannotlanglois <https://github.com/jeannotlanglois> Thanks a lot for your contribution! Can you please address the comments so we can push this forward? — Reply to this email directly, view it on GitHub <#798 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJK3K5KSWRFU66AUKDOFLQ33JNXGPAVCNFSM6AAAAACBNPLEEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAOJUGQZTIMRVGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jeannotlanglois]

Commit containing changes applied during round #1 pending...

[jeannotlanglois]

@orgads :

Hello -- I've responded to all questions and addressed most ( if not all ) concerns -- let me know what you think about what remains. Within the next week I plan to help finalize this if anything else needs addressing.

Cheers :)

[orgads]

Thank you!

[orgads]

Ok, instead of continuing this ping-pong, I just pushed my proposal. If you approve, I'll go ahead and merge it.

[Copilot]

Pull Request Overview

This PR fixes critical regressions in the RTPCHECK functionality that were introduced in SIPp-3.7.0~rc1. The key changes include fixing a crash caused by incorrect string handling, restoring timestamp functionality in log filenames, implementing safer file operations, and improving encapsulation through a new SrtpChannel class.

Key changes:

• Fixed SRTP crypto parameter processing crash by correcting string operations from unsafe strncat() to safe strcpy()
• Restored timestamp-based naming for RTP echo log files that was accidentally removed
• Added new SrtpChannel wrapper class to improve encapsulation and eliminate extern dependencies

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/sipp.cpp	Added random_base_ssrc parameter and proper global_ssrc_id initialization
src/rtpstream.cpp	Fixed log filename timestamps, corrected mutex unlock, and updated SRTP type usage
src/jlsrtp.cpp	Fixed constructor parameter type and added dummy constructor for non-SSL builds
src/call.cpp	Replaced unsafe strncat() with strcpy() throughout SRTP parameter handling
sipp_scenarios/pfca_uas_both_crypto_simple.xml	Reorganized RTP echo actions within send element
include/srtp_channel.hpp	New header defining SrtpChannel wrapper class
include/rtpstream.hpp	Updated to use forward declaration instead of direct include
include/jlsrtp.hpp	Updated constructor signature and version number
include/call.hpp	Updated to use SrtpChannel instead of JLSRTP directly

[jeannotlanglois]

Looks mostly good - I only spotted two small items regarding RTP echo audio/video filenames which I'd like to be kept as-is.
Once these two are fixed the rest can be approved+merged.

I will be away on a short one-week vacation but can check for any updates by Sunday afternoon once I regain access to a computer.

Thanks!

[jeannotlanglois]

No these changes are fine ans timestamps are needed, rather see the file names that were changed around the murex scope change cide areas. Tget should be "debugrefileaudio" and "debugrefilevideo". Jeannot Langlois Senior Software Developer ***@***.***

…

On Sat, Aug 2, 2025, 14:39 Orgad Shaneh ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/rtpstream.cpp <#798 (comment)>: > if (debugrefileaudio == nullptr) { - debugrefileaudio = fopen("debugrefileaudio", "w"); + debugrefileaudio = fopen(build_rtpecho_filename("audio").c_str(), "w"); Do you mean you don't want timestamp in the file name? This is your own change, I only reverted it to keep using fopen. — Reply to this email directly, view it on GitHub <#798 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJK3K5PVA2WKOHBUZEL56LD3LUAUNAVCNFSM6AAAAACBNPLEEGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTAOBRGE4DMMRQGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[orgads]

No these changes are fine ans timestamps are needed, rather see the file names that were changed around the murex scope change cide areas. Tget should be "debugrefileaudio" and "debugrefilevideo".

But this is already done here, by passing audio/video to build_rtpecho_filename. What do you find missing?

[jeannotlanglois]

Ohhh... you are right, I had forgot that part... which I did myself... d'oh! :) Patch is good in its current state then -- as far I'm concerned it can now be merged. Thanks! Jeannot Langlois Senior Software Developer ***@***.***

…

On Sun, Aug 3, 2025, 00:58 Orgad Shaneh ***@***.***> wrote: *orgads* left a comment (SIPp/sipp#798) <#798 (comment)> No these changes are fine ans timestamps are needed, rather see the file names that were changed around the murex scope change cide areas. Tget should be "debugrefileaudio" and "debugrefilevideo". But this is already done here, by passing audio/video to build_rtpecho_filename. What do you find missing? — Reply to this email directly, view it on GitHub <#798 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJK3K5PKAJA6XM2CJ4RILXD3LWJHFAVCNFSM6AAAAACBNPLEEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNBWHE4DGNJWHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>