Skip to content

Conversation

@ibc
Copy link
Member

@ibc ibc commented Jun 13, 2025

Details

@ibc
Copy link
Member Author

ibc commented Jun 13, 2025

TODO 1 (solved)

Wow, it looks like if the public srtp_protect() API and so on now requires extra arguments for the target. Note that in SrtpSession.cpp we do this:

std::memcpy(encryptBuffer, *data, *len);

const srtp_err_status_t err = srtp_protect(this->session, encryptBuffer, len);

But now it requires 6 args instead:

../../../src/RTC/SrtpSession.cpp:228:33: error: no matching function for call to 'srtp_protect'
  228 |                 const srtp_err_status_t err = srtp_protect(this->session, encryptBuffer, len);
      |                                               ^~~~~~~~~~~~
../../../subprojects/libsrtp-3.0.0-beta/include/srtp.h:433:19: note: candidate function not viable: requires 6 arguments, but 3 were provided
  433 | srtp_err_status_t srtp_protect(srtp_t ctx,
      |                   ^            ~~~~~~~~~~~
  434 |                                const uint8_t *rtp,
      |                                ~~~~~~~~~~~~~~~~~~~
  435 |                                size_t rtp_len,
      |                                ~~~~~~~~~~~~~~~
  436 |                                uint8_t *srtp,
      |                                ~~~~~~~~~~~~~~
  437 |                                size_t *srtp_len,
      |                                ~~~~~~~~~~~~~~~~~
  438 |                                size_t mki_index);
      |                                ~~~~~~~~~~~~~~~~
../../../src/RTC/SrtpSession.cpp:247:33: error: no matching function for call to 'srtp_unprotect'
  247 |                 const srtp_err_status_t err = srtp_unprotect(this->session, data, len);
      |                                               ^~~~~~~~~~~~~~
../../../subprojects/libsrtp-3.0.0-beta/include/srtp.h:485:19: note: candidate function not viable: requires 5 arguments, but 3 were provided
  485 | srtp_err_status_t srtp_unprotect(srtp_t ctx,
      |                   ^              ~~~~~~~~~~~
  486 |                                  const uint8_t *srtp,
      |                                  ~~~~~~~~~~~~~~~~~~~~
  487 |                                  size_t srtp_len,
      |                                  ~~~~~~~~~~~~~~~~
  488 |                                  uint8_t *rtp,
      |                                  ~~~~~~~~~~~~~
  489 |                                  size_t *rtp_len);
      |                                  ~~~~~~~~~~~~~~~
../../../src/RTC/SrtpSession.cpp:273:33: error: no matching function for call to 'srtp_protect_rtcp'
  273 |                 const srtp_err_status_t err = srtp_protect_rtcp(this->session, EncryptBuffer, len);
      |                                               ^~~~~~~~~~~~~~~~~
../../../subprojects/libsrtp-3.0.0-beta/include/srtp.h:1162:19: note: candidate function not viable: requires 6 arguments, but 3 were provided
 1162 | srtp_err_status_t srtp_protect_rtcp(srtp_t ctx,
      |                   ^                 ~~~~~~~~~~~
 1163 |                                     const uint8_t *rtcp,
      |                                     ~~~~~~~~~~~~~~~~~~~~
 1164 |                                     size_t rtcp_len,
      |                                     ~~~~~~~~~~~~~~~~
 1165 |                                     uint8_t *srtcp,
      |                                     ~~~~~~~~~~~~~~~
 1166 |                                     size_t *srtcp_len,
      |                                     ~~~~~~~~~~~~~~~~~~
 1167 |                                     size_t mki_index);
      |                                     ~~~~~~~~~~~~~~~~
../../../src/RTC/SrtpSession.cpp:292:33: error: no matching function for call to 'srtp_unprotect_rtcp'
  292 |                 const srtp_err_status_t err = srtp_unprotect_rtcp(this->session, data, len);
      |                                               ^~~~~~~~~~~~~~~~~~~
../../../subprojects/libsrtp-3.0.0-beta/include/srtp.h:1212:19: note: candidate function not viable: requires 5 arguments, but 3 were provided
 1212 | srtp_err_status_t srtp_unprotect_rtcp(srtp_t ctx,
      |                   ^                   ~~~~~~~~~~~
 1213 |                                       const uint8_t *srtcp,
      |                                       ~~~~~~~~~~~~~~~~~~~~~
 1214 |                                       size_t srtcp_len,
      |                                       ~~~~~~~~~~~~~~~~~
 1215 |                                       uint8_t *rtcp,
      |                                       ~~~~~~~~~~~~~~
 1216 |                                       size_t *rtcp_len);
      |                                       ~~~~~~~~~~~~~~~~

Signature of srtp_protect():

/**
 * @brief srtp_protect() is the Secure RTP sender-side packet processing
 * function.
 *
 * The function call srtp_protect(ctx, rtp, rtp_len, srtp, srtp_len, mki_index)
 * applies SRTP protection to the RTP packet rtp (which has length rtp_len)
 * using the SRTP context ctx.  If srtp_err_status_ok is returned, then srtp
 * points to the resulting SRTP packet and *srtp_len is the number of
 * octets in that packet; otherwise, no assumptions should be made
 * about the value of either data elements.
 *
 * The sequence numbers of the RTP packets presented to this function
 * need not be consecutive, but they @b must be out of order by less
 * than 2^15 = 32,768 packets.
 *
 * @warning This function assumes that the RTP packet is aligned on a 32-bit
 * boundary.
 *
 * @param ctx is the SRTP context to use in processing the packet.
 *
 * @param rtp is a pointer to the RTP packet.
 *
 * @param rtp_len is the length in octets of the complete RTP
 * packet (header and body).
 *
 * @param srtp is a pointer to a buffer that after the function returns will
 * contain the complete SRTP packet. The value of srtp can be the same as rtp to
 * support in-place io.
 *
 * @param srtp_len is a pointer to the length in octets of the srtp buffer
 * before the function call, and of the complete SRTP packet after the call, if
 * srtp_err_status_ok was returned. Otherwise, the value of the data to which it
 * points is undefined.
 *
 * @param mki_index integer value specifying which set of session keys should be
 * used if use_mki in the policy was set to true. Otherwise ignored.
 *
 * @return
 *    - srtp_err_status_ok            no problems
 *    - srtp_err_status_replay_fail   rtp sequence number was non-increasing
 *    - srtp_err_status_buffer_small  the srtp buffer is too small for the SRTP
 * packet
 *    - @e other                 failure in cryptographic mechanisms
 */
srtp_err_status_t srtp_protect(srtp_t ctx,
                               const uint8_t *rtp,
                               size_t rtp_len,
                               uint8_t *srtp,
                               size_t *srtp_len,
                               size_t mki_index);

@jmillan
Copy link
Member

jmillan commented Jun 13, 2025

This enhancement of us is present in libsrtp main branch which should come to v3 sometime, but new libsrtp v2 releases come out of 2_x_dev branch. In main the public API is being changed.

The problem is that I don't think it is well known when v3 is coming out. So we could keep using main and apply those API changes or port the enhancement to 2_x_dev so it's integrated into the next 2.X release.

Check this thread for more details cisco/libsrtp#674 (comment)

@ibc
Copy link
Member Author

ibc commented Jun 13, 2025

So you mean that we should merge changes from mainstream 2_x_dev branch instead of main branch?

The problem I see is that in our fork we applied changed directly in main branch is its hard to manage commits from mainstream.

@jmillan
Copy link
Member

jmillan commented Jun 13, 2025

I just mean that we could merge specifically this PR and few other changes made on top of it afterwards, to 2_x_dev so the changes will be present in the next v2 release. This way once next v2 is released we won't have to have any custom branch and will use the official libsrtp.

The changes will be present in v3 anyway but the problem is that I don't think there is a timeline for v3 release.

@ibc
Copy link
Member Author

ibc commented Jun 13, 2025

I just mean that we could merge specifically this PR and few other changes made on top of it afterwards, to 2_x_dev so the changes will be present in the next v2 release. This way once next v2 is released we won't have to have any custom branch and will use the official libsrtp.

The changes will be present in v3 anyway but the problem is that I don't think there is a timeline for v3 release.

That makes sense but we already diverged (literally your PR went to libsrtp main branch rather than 2_x_dev branch) and I don't think that moving back to libsrtp v2 (and adopt the needed changes) is easier than staying in main branch.

Anyway, as per our call today I've pushed to this PR with the required API changes to make libsrtp v3 work, meaning that yes, we are using libsrtp main branch here (AKA v3).

@ibc ibc merged commit 86f5aec into v3 Jun 13, 2025
71 checks passed
@ibc ibc deleted the update-libsrtp-to-3.0.0-beta branch June 13, 2025 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants